review followup

Author: buck54321

client/asset/eth/contractor.go

@@ -221,8 +221,8 @@ func (c *contractorV0) vector(ctx context.Context, locator []byte) (*dexeth.Swap
 		return nil, err
 	}
 	vector := &dexeth.SwapVector{
-		From:       swap.Participant,
-		To:         swap.Initiator,
+		From:       swap.Initiator,
+		To:         swap.Participant,
 		Value:      swap.Value,
 		SecretHash: secretHash,
 		LockTime:   uint64(swap.LockTime.Unix()),
@@ -243,8 +243,8 @@ func (c *contractorV0) statusAndVector(ctx context.Context, locator []byte) (*de
 		return nil, nil, err
 	}
 	vector := &dexeth.SwapVector{
-		From:       swap.Participant,
-		To:         swap.Initiator,
+		From:       swap.Initiator,
+		To:         swap.Participant,
 		Value:      swap.Value,
 		SecretHash: secretHash,
 		LockTime:   uint64(swap.LockTime.Unix()),
@@ -452,7 +452,6 @@ func (c *erc20Contractor) balance(ctx context.Context) (*big.Int, error) {
 // allowance exposes the read-only allowance method of the erc20 token contract.
 func (c *erc20Contractor) allowance(ctx context.Context) (*big.Int, error) {
 	callOpts := &bind.CallOpts{
-		// Pending: true, // Seeing errors on even simnet that say "backend does not support pending state"
 		From:    c.acct,
 		Context: ctx,
 	}
@@ -947,14 +946,6 @@ func (c *tokenContractorV1) tokenAddress() common.Address {
 var _ contractor = (*tokenContractorV1)(nil)
 var _ tokenContractor = (*tokenContractorV1)(nil)
 
-// readOnlyCallOpts is the CallOpts used for read-only contract method calls.
-func readOnlyCallOpts(ctx context.Context) *bind.CallOpts {
-	return &bind.CallOpts{
-		Pending: true,
-		Context: ctx,
-	}
-}
-
 func estimateGas(ctx context.Context, from, to common.Address, abi *abi.ABI, cb bind.ContractBackend, value *big.Int, method string, args ...interface{}) (uint64, error) {
 	data, err := abi.Pack(method, args...)
 	if err != nil {

client/asset/eth/eth.go

@@ -1497,7 +1497,7 @@ func (w *TokenWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, uin
 	g, err := w.initGasEstimate(int(ord.MaxSwapCount), contractVer,
 		ord.RedeemVersion, ord.RedeemAssetID)
 	if err != nil {
-		return nil, nil, 0, fmt.Errorf("error estimating swap gas: %vlaptop apart comic equip remove adult system tuna office discover toddler can keep fury aware amazing injury typical", err)
+		return nil, nil, 0, fmt.Errorf("error estimating swap gas: %v", err)
 	}
 
 	ethToLock := ord.MaxFeeRate * g.Swap * ord.MaxSwapCount
@@ -3446,7 +3446,7 @@ func parseSecretHashes(tx *types.Transaction, contractVer uint32, isInit bool) (
 			}
 			locators = make([][]byte, 0, len(inits))
 			for k := range inits {
-				copyK := k // TODO: Is this really necessary?
+				copyK := k
 				locators = append(locators, copyK[:])
 			}
 		} else {
@@ -4628,7 +4628,7 @@ func (w *assetWallet) loadContractors() error {
 // withContractor runs the provided function with the versioned contractor.
 func (w *assetWallet) withContractor(contractVer uint32, f func(contractor) error) error {
 	if contractVer == dexeth.ContractVersionERC20 {
-		// For ERC02 methods, use the most recent contractor version.
+		// For ERC20 methods, use the most recent contractor version.
 		var bestVer uint32
 		var bestContractor contractor
 		for ver, c := range w.contractors {

client/asset/eth/eth_test.go

@@ -377,8 +377,8 @@ func (c *tContractor) vector(ctx context.Context, locator []byte) (*dexeth.SwapV
 		return nil, errors.New("swap not in map")
 	}
 	v := &dexeth.SwapVector{
-		From:       swap.Participant,
-		To:         swap.Initiator,
+		From:       swap.Initiator,
+		To:         swap.Participant,
 		Value:      swap.Value,
 		SecretHash: secretHash,
 		LockTime:   uint64(swap.LockTime.Unix()),
@@ -399,8 +399,8 @@ func (c *tContractor) statusAndVector(ctx context.Context, locator []byte) (*dex
 		return nil, nil, errors.New("swap not in map")
 	}
 	v := &dexeth.SwapVector{
-		From:       swap.Participant,
-		To:         swap.Initiator,
+		From:       swap.Initiator,
+		To:         swap.Participant,
 		Value:      swap.Value,
 		SecretHash: vector.SecretHash,
 		LockTime:   uint64(swap.LockTime.Unix()),

client/core/simnet_trade.go

@@ -2135,7 +2135,7 @@ func (s *simulationTest) assertBalanceChanges(client *simulationClient, isRefund
 	if isRefund {
 		// NOTE: Gas price may be higher if the eth harness has
 		// had a lot of use. The minimum is the gas tip cap.
-		ethRefundFees := int64(dexeth.RefundGas(0 /*version*/)) * dexeth.MinGasTipCap
+		ethRefundFees := int64(dexeth.RefundGas(1 /*version*/)) * dexeth.MinGasTipCap
 
 		msgTx := wire.NewMsgTx(0)
 		prevOut := wire.NewOutPoint(&chainhash.Hash{}, 0)

dex/networks/erc20/contracts/ERC20SwapV0.sol

@@ -120,7 +120,7 @@ contract ERC20Swap {
     }
 
     // redeem redeems an array of swaps contract. It checks that the sender is
-    // not a contract, and that the secret hash hashes to secretHash. The ERC20
+    // not a contract, and that the secret hashes to secretHash. The ERC20
     // tokens are transferred from the contract to the sender.
     function redeem(Redemption[] calldata redemptions)
         public

dex/networks/erc20/contracts/ERC20SwapV1.sol

@@ -14,11 +14,11 @@ pragma solidity = 0.8.18;
 // When calling initiate, the necessary tokens for swaps are transferred to
 // the swap contract. At this point the funds belong to the contract, and
 // cannot be accessed by anyone else, not even the contract's deployer. The
-// initiator sets a secret hash, a blocktime the funds will be accessible should
-// they not be redeemed, and a participant who can redeem before or after the
-// locktime. The participant can redeem at any time after the initiation
-// transaction is mined if they have the secret that hashes to the secret hash.
-// Otherwise, the initiator can refund funds any time after the locktime.
+// initiator commits to the swap parameters, including a locktime after which
+// the funds will be accessible for refund should they not be redeemed. The
+// participant can redeem at any time after the initiation transaction is mined
+// if they have the secret that hashes to the secret hash. Otherwise, the
+// initiator can refund funds any time after the locktime.
 //
 // This contract has no limits on gas used for any transactions.
 //
@@ -30,7 +30,7 @@ contract ERC20Swap {
 
     address public immutable token_address;
 
-     // Step is a type that hold's a contract's current step. Empty is the
+    // Step is a type that hold's a contract's current step. Empty is the
     // uninitiated or null value.
     enum Step { Empty, Filled, Redeemed, Refunded }
 
@@ -41,6 +41,7 @@ contract ERC20Swap {
     }
 
     bytes32 constant RefundRecord = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
+    bytes32 constant RefundRecordHash = 0xAF9613760F72635FBDB44A5A0A63C39F12AF30F950A6EE5C971BE188E89C4051;
 
     // swaps is a map of contract hashes to the "swap record". The swap record
     // has the following interpretation.
@@ -134,6 +135,7 @@ contract ERC20Swap {
 
             require(v.value > 0, "0 val");
             require(v.refundTimestamp > 0, "0 refundTimestamp");
+            require(v.secretHash != RefundRecordHash, "illegal secret hash (refund record hash)");
 
             bytes32 k = contractKey(v);
             bytes32 record = swaps[k];
@@ -153,7 +155,7 @@ contract ERC20Swap {
         require(success && (data.length == 0 || abi.decode(data, (bool))), 'transfer from failed');
     }
 
-    // isRedeemable returns whether or not a swap identified by secretHash
+    // isRedeemable returns whether or not a swap identified by vector
     // can be redeemed using secret.
     function isRedeemable(Vector calldata v)
         public
@@ -165,7 +167,7 @@ contract ERC20Swap {
     }
 
     // redeem redeems a Vector. It checks that the sender is not a contract,
-    // and that the secret hash hashes to secretHash. msg.value is tranfered
+    // and that the secret hashes to secretHash. msg.value is tranfered
     // from ETHSwap to the sender.
     //
     // To prevent reentry attack, it is very important to check the state of the
@@ -223,14 +225,14 @@ contract ERC20Swap {
         (bytes32 k, bytes32 record, uint256 blockNum) = retrieveStatus(v);
 
         // Is this swap initialized?
+        // This check also guarantees that the swap has not already been
+        // refunded i.e. record != RefundRecord, since RefundRecord is certainly
+        // greater than block.number.
         require(blockNum > 0 && blockNum <= block.number, "swap not active");
 
         // Is it already redeemed?
         require(!secretValidates(record, v.secretHash), "swap already redeemed");
 
-        // Is it already refunded?
-        require(record != RefundRecord, "swap already refunded");
-
         swaps[k] = RefundRecord;
 
         bool success;