cleanup and early review followup

Author: buck54321

client/asset/eth/contractor.go

@@ -195,7 +195,6 @@ func (c *contractorV0) swap(ctx context.Context, secretHash [32]byte) (*dexeth.S
 	if err != nil {
 		return nil, err
 	}
-
 	return &dexeth.SwapState{
 		BlockHeight: state.InitBlockNumber.Uint64(),
 		LockTime:    time.Unix(state.RefundBlockTimestamp.Int64(), 0),
@@ -207,7 +206,6 @@ func (c *contractorV0) swap(ctx context.Context, secretHash [32]byte) (*dexeth.S
 	}, nil
 }
 
-// swap retrieves the swap info from the read-only swap method.
 func (c *contractorV0) status(ctx context.Context, contract *dex.SwapContractDetails) (step dexeth.SwapStep, secret [32]byte, blockNumber uint32, err error) {
 	var secretHash [32]byte
 	copy(secretHash[:], contract.SecretHash)
@@ -218,32 +216,41 @@ func (c *contractorV0) status(ctx context.Context, contract *dex.SwapContractDet
 	return swap.State, swap.Secret, uint32(swap.BlockHeight), nil
 }
 
+func (c *contractorV0) refundImpl(txOpts *bind.TransactOpts, secretHash [32]byte) (*types.Transaction, error) {
+	return c.contractV0.Refund(txOpts, secretHash)
+}
+
 // refund issues the refund command to the swap contract. Use isRefundable first
 // to ensure the refund will be accepted.
 func (c *contractorV0) refund(txOpts *bind.TransactOpts, contract *dex.SwapContractDetails) (*types.Transaction, error) {
 	var secretHash [32]byte
 	copy(secretHash[:], contract.SecretHash)
-	return c.contractV0.Refund(txOpts, secretHash)
+	return c.refundImpl(txOpts, secretHash)
+}
+
+func (c *contractorV0) isRedeemableImpl(secretHash, secret [32]byte) (bool, error) {
+	return c.contractV0.IsRedeemable(&bind.CallOpts{From: c.acctAddr}, secretHash, secret)
 }
 
 // isRedeemable exposes the isRedeemable method of the swap contract.
 func (c *contractorV0) isRedeemable(secret [32]byte, contract *dex.SwapContractDetails) (bool, error) {
 	var secretHash [32]byte
 	copy(secretHash[:], contract.SecretHash)
-	return c.contractV0.IsRedeemable(&bind.CallOpts{From: c.acctAddr}, secretHash, secret)
+	return c.isRedeemableImpl(secretHash, secret)
+}
+
+func (c *contractorV0) isRefundableImpl(secretHash [32]byte) (bool, error) {
+	return c.contractV0.IsRefundable(&bind.CallOpts{From: c.acctAddr}, secretHash)
 }
 
 // isRefundable exposes the isRefundable method of the swap contract.
 func (c *contractorV0) isRefundable(contract *dex.SwapContractDetails) (bool, error) {
 	var secretHash [32]byte
 	copy(secretHash[:], contract.SecretHash)
-	return c.contractV0.IsRefundable(&bind.CallOpts{From: c.acctAddr}, secretHash)
+	return c.isRefundableImpl(secretHash)
 }
 
-// estimateRedeemGas estimates the gas used to redeem. The secret hashes
-// supplied must reference existing swaps, so this method can't be used until
-// the swap is initiated.
-func (c *contractorV0) estimateRedeemGas(ctx context.Context, secrets [][32]byte, _ []*dex.SwapContractDetails) (uint64, error) {
+func (c *contractorV0) estimateRedeemGasImpl(ctx context.Context, secrets [][32]byte) (uint64, error) {
 	redemps := make([]swapv0.ETHSwapRedemption, 0, len(secrets))
 	for _, secret := range secrets {
 		redemps = append(redemps, swapv0.ETHSwapRedemption{
@@ -254,13 +261,24 @@ func (c *contractorV0) estimateRedeemGas(ctx context.Context, secrets [][32]byte
 	return c.estimateGas(ctx, nil, "redeem", redemps)
 }
 
+// estimateRedeemGas estimates the gas used to redeem. The secret hashes
+// supplied must reference existing swaps, so this method can't be used until
+// the swap is initiated.
+func (c *contractorV0) estimateRedeemGas(ctx context.Context, secrets [][32]byte, _ []*dex.SwapContractDetails) (uint64, error) {
+	return c.estimateRedeemGasImpl(ctx, secrets)
+}
+
+func (c *contractorV0) estimateRefundGasImpl(ctx context.Context, secretHash [32]byte) (uint64, error) {
+	return c.estimateGas(ctx, nil, "refund", secretHash)
+}
+
 // estimateRefundGas estimates the gas used to refund. The secret hashes
 // supplied must reference existing swaps that are refundable, so this method
 // can't be used until the swap is initiated and the lock time has expired.
 func (c *contractorV0) estimateRefundGas(ctx context.Context, deets *dex.SwapContractDetails) (uint64, error) {
 	var secretHash [32]byte
 	copy(secretHash[:], deets.SecretHash)
-	return c.estimateGas(ctx, nil, "refund", secretHash)
+	return c.estimateRefundGasImpl(ctx, secretHash)
 }
 
 // estimateInitGas estimates the gas used to initiate n generic swaps. The

client/asset/eth/eth.go

@@ -1482,7 +1482,7 @@ func (w *ETHWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, uint6
 		return fail("unfunded swap: %d < %d", reservedVal, swapVal+fees)
 	}
 
-	tx, err := w.initiate(w.ctx, w.assetID, w.contractsToDetails(swaps.Contracts), swaps.FeeRate, gasLimit, cfg.Version)
+	tx, err := w.initiate(w.ctx, w.assetID, w.contractsToInitDetails(swaps.Contracts), swaps.FeeRate, gasLimit, cfg.Version)
 	if err != nil {
 		return fail("Swap: initiate error: %w", err)
 	}
@@ -1512,7 +1512,7 @@ func (w *ETHWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, uint6
 	return receipts, change, fees, nil
 }
 
-func (w *baseWallet) contractsToDetails(contracts []*asset.Contract) []*dex.SwapContractDetails {
+func (w *baseWallet) contractsToInitDetails(contracts []*asset.Contract) []*dex.SwapContractDetails {
 	details := make([]*dex.SwapContractDetails, len(contracts))
 	for i, c := range contracts {
 		details[i] = &dex.SwapContractDetails{
@@ -1572,7 +1572,7 @@ func (w *TokenWallet) Swap(swaps *asset.Swaps) ([]asset.Receipt, asset.Coin, uin
 		return fail("unfunded token swap fees: %d < %d", reservedParent, fees)
 	}
 
-	tx, err := w.initiate(w.ctx, w.assetID, w.contractsToDetails(swaps.Contracts), swaps.FeeRate, gasLimit, cfg.Version)
+	tx, err := w.initiate(w.ctx, w.assetID, w.contractsToInitDetails(swaps.Contracts), swaps.FeeRate, gasLimit, cfg.Version)
 	if err != nil {
 		return fail("Swap: initiate error: %w", err)
 	}
@@ -2037,8 +2037,7 @@ func (w *assetWallet) AuditContract(coinID, contract, serializedTx dex.Bytes, re
 	}
 
 	var val uint64
-	var participant string
-	var initiator string
+	var participant, initiator string
 	var lockTime time.Time
 	switch version {
 	case 0:

client/asset/eth/eth_test.go

@@ -2485,27 +2485,6 @@ func testAuditContract(t *testing.T, assetID uint32) {
 	}
 
 	for _, test := range tests {
-		// txData, err := packInitiateDataV0(test.initiations)
-		// if err != nil {
-		// 	t.Fatalf("unexpected error: %v", err)
-		// }
-		// if test.badTxData {
-		// 	txData = []byte{0}
-		// }
-
-		// tx := tTx(2, 300, uint64(len(test.initiations)), &node.addr, txData)
-		// chainParams := node.chainConfig()
-		// signer := types.LatestSignerForChainID(chainParams.ChainID)
-		// tx, _ = types.SignTx(tx, signer, privKey)
-
-		// txBinary, err := tx.MarshalBinary()
-		// if err != nil {
-		// 	t.Fatalf(`"%v": failed to marshal binary: %v`, test.name, err)
-		// }
-		// if test.badTxBinary {
-		// 	txBinary = []byte{0}
-		// }
-
 		txData, err := packInitiateDataV0(test.initiations)
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)

client/asset/eth/nodeclient_harness_test.go

@@ -293,9 +293,9 @@ func runSimnet(m *testing.M) (int, error) {
 	participantAddr = participantAcct.Address
 
 	if v1 {
-		prepareV1Contractors()
+		prepareV1SimnetContractors()
 	} else {
-		prepareV0Contractors()
+		prepareV0SimnetContractors()
 	}
 
 	if err := ethClient.unlock(pw); err != nil {
@@ -482,15 +482,15 @@ func runTestnet(m *testing.M) (int, error) {
 	return code, nil
 }
 
-func prepareV0Contractors() (err error) {
-	return prepareContractors(newV0Contractor, newV0TokenContractor)
+func prepareV0SimnetContractors() (err error) {
+	return prepareSimnetContractors(newV0Contractor, newV0TokenContractor)
 }
 
-func prepareV1Contractors() (err error) {
-	return prepareContractors(newV1Contractor, newV1TokenContractor)
+func prepareV1SimnetContractors() (err error) {
+	return prepareSimnetContractors(newV1Contractor, newV1TokenContractor)
 }
 
-func prepareContractors(c contractorConstructor, tc tokenContractorConstructor) (err error) {
+func prepareSimnetContractors(c contractorConstructor, tc tokenContractorConstructor) (err error) {
 	if simnetContractor, err = c(dex.Simnet, simnetAddr, ethClient.contractBackend()); err != nil {
 		return fmt.Errorf("new contractor error: %w", err)
 	}
@@ -1210,21 +1210,9 @@ func testInitiate(t *testing.T, assetID uint32) {
 
 		diff := new(big.Int).Sub(wantBal, bal)
 		if diff.CmpAbs(new(big.Int)) != 0 {
-
-			fmt.Println("Original balance:", fmtBig(originalBal), ", New balance:", fmtBig(bal),
-				", Balance change:", fmtBig(new(big.Int).Sub(bal, originalBal)),
-				", Tx fees:", fmtBig(txFee), ", Swap val:", fmtBig(dexeth.GweiToWei(totalVal)))
-
-			cmd := exec.CommandContext(ctx, "./mine-alpha", "5")
-			cmd.Dir = harnessCtlDir
-			if err := cmd.Run(); err != nil {
-				t.Fatalf("error mining block after funding wallets")
-			}
-
-			bal, _ := balance()
-
-			fmt.Println("Original balance:", fmtBig(originalBal), ", New balance:", fmtBig(bal),
-				", Balance change:", fmtBig(new(big.Int).Sub(bal, originalBal)))
+			fmt.Println("Original balance:", fmtWei(originalBal), ", New balance:", fmtWei(bal),
+				", Balance change:", fmtWei(new(big.Int).Sub(bal, originalBal)),
+				", Tx fees:", fmtWei(txFee), ", Swap val:", fmtWei(dexeth.GweiToWei(totalVal)))
 			t.Fatalf("%s: unexpected balance change: want %d got %d gwei, diff = %.9f gwei",
 				test.name, dexeth.WeiToGwei(wantBal), dexeth.WeiToGwei(bal), float64(diff.Int64())/dexeth.GweiFactor)
 		}
@@ -1441,18 +1429,18 @@ func testRedeem(t *testing.T, assetID uint32) {
 			finalStates:        []dexeth.SwapStep{dexeth.SSRedeemed},
 			addAmt:             true,
 		},
-		// {
-		// 	name:               "bad redeemer",
-		// 	sleepNBlocks:       8,
-		// 	redeemerClient:     ethClient,
-		// 	redeemer:           simnetAcct,
-		// 	redeemerContractor: c,
-		// 	swaps:              []*dex.SwapContractDetails{newDetails(4, 1)},
-		// 	redemptions:        []*asset.Redemption{newRedeem(secrets[4], secretHashes[4], 1, lockTime)},
-		// 	isRedeemable:       []bool{false},
-		// 	finalStates:        []dexeth.SwapStep{dexeth.SSInitiated},
-		// 	addAmt:             false,
-		// },
+		{
+			name:               "bad redeemer",
+			sleepNBlocks:       8,
+			redeemerClient:     ethClient,
+			redeemer:           simnetAcct,
+			redeemerContractor: c,
+			swaps:              []*dex.SwapContractDetails{newDetails(4, 1)},
+			redemptions:        []*asset.Redemption{newRedeem(secrets[4], secretHashes[4], 1, lockTime)},
+			isRedeemable:       []bool{false},
+			finalStates:        []dexeth.SwapStep{dexeth.SSInitiated},
+			addAmt:             false,
+		},
 		{
 			name:               "bad secret",
 			sleepNBlocks:       8,
@@ -1573,9 +1561,9 @@ func testRedeem(t *testing.T, assetID uint32) {
 		expGas := gases.RedeemN(len(test.redemptions))
 		// Ethereum is weird. For v1, if I use 42,000 here, it will often fail,
 		// using all of the gas, so presumably it fails because of insufficient
-		// gas. But if I set a higher limit, the it will succeed, and oddly will
-		// never use 42,000 gas. Why does it use more gas when I use a lower
-		// limit?
+		// gas. But if I set a higher limit, then it will succeed, and oddly
+		// will never use 42,000 gas. Why does it use more gas when I use a
+		// lower limit?
 		txOpts, _ = test.redeemerClient.txOpts(ctx, 0, expGas, dexeth.GweiToWei(maxFeeRate))
 		tx, err = test.redeemerContractor.redeem(txOpts, test.redemptions)
 		if test.expectRedeemErr {
@@ -1603,9 +1591,6 @@ func testRedeem(t *testing.T, assetID uint32) {
 		if err != nil && expSuccess {
 			t.Fatalf("%s: failed redeem transaction status: %v", test.name, err)
 		}
-		if expSuccess && receipt.GasUsed > expGas {
-			t.Fatalf("%s: gas used, %d, exceeds expected max %d", test.name, receipt.GasUsed, expGas)
-		}
 
 		fmt.Printf("Gas used for %d redeems, success = %t: %d \n", len(test.swaps), expSuccess, receipt.GasUsed)
 
@@ -1952,11 +1937,9 @@ func testRefund(t *testing.T, assetID uint32) {
 
 		diff := new(big.Int).Sub(wantBal, bal)
 		if diff.CmpAbs(dexeth.GweiToWei(1)) >= 0 {
-
-			fmt.Println("Original balance:", fmtBig(originalBal), ", New balance:", fmtBig(bal),
-				", Balance change:", fmtBig(new(big.Int).Sub(bal, originalBal)),
-				", Tx fees:", fmtBig(txFee), ", Swap val:", fmtBig(dexeth.GweiToWei(amt)))
-
+			fmt.Println("Original balance:", fmtWei(originalBal), ", New balance:", fmtWei(bal),
+				", Balance change:", fmtWei(new(big.Int).Sub(bal, originalBal)),
+				", Tx fees:", fmtWei(txFee), ", Swap val:", fmtWei(dexeth.GweiToWei(amt)))
 			t.Fatalf("%s: unexpected balance change: want %d got %d, diff = %d",
 				test.name, dexeth.WeiToGwei(wantBal), dexeth.WeiToGwei(bal), dexeth.WeiToGwei(diff))
 		}
@@ -2342,6 +2325,6 @@ func exportAccountsFromNode(node *node.Node) ([]accounts.Account, error) {
 	return ks.Accounts(), nil
 }
 
-func fmtBig(v *big.Int) string {
+func fmtWei(v *big.Int) string {
 	return fmt.Sprintf("%.9f gwei", float64(new(big.Int).Div(v, big.NewInt(dexeth.GweiFactor)).Int64()))
 }

client/asset/interface.go

@@ -182,9 +182,8 @@ type WalletInfo struct {
 	// TODO: Handle unsupported protocols at the UI (instruct user to update
 	// dexc).
 	ProtocolVersions []uint32 `json:"protocolVersions"`
-	// Version is the Wallet's version number, which is used to signal when
-	// major changes are made to internal details such as coin ID encoding and
-	// contract structure that must be common to a server's.
+	// Version is the highest server backend version supported by the client.
+	// Deprecated: Use ProtocolVersions.
 	Version uint32 `json:"version"`
 	// AvailableWallets is an ordered list of available WalletDefinition. The
 	// first WalletDefinition is considered the default, and might, for instance

client/asset/ltc/ltc.go

@@ -119,7 +119,7 @@ var (
 			rpcWalletDefinition,
 			electrumWalletDefinition,
 		},
-		ProtocolVersions: []uint32{0},
+		ProtocolVersions: []uint32{1},
 	}
 )
 

client/core/types.go

@@ -104,7 +104,7 @@ type WalletBalance struct {
 // WalletState is the current status of an exchange wallet.
 type WalletState struct {
 	Symbol       string            `json:"symbol"`
-	AssetID      uint32            `json:"assetID"`
+	AssetID      uint32            `json:"assetID"` // Deprecated: use ProtocolVersions
 	Version      uint32            `json:"version"`
 	WalletType   string            `json:"type"`
 	Traits       asset.WalletTrait `json:"traits"`
@@ -117,7 +117,7 @@ type WalletState struct {
 	PeerCount    uint32            `json:"peerCount"`
 	Synced       bool              `json:"synced"`
 	SyncProgress float32           `json:"syncProgress"`
-	// ProtocolVersions is the supported server protocol versions. Added ~0.6.
+	// ProtocolVersions is the supported server protocol versions.
 	ProtocolVersions []uint32 `json:"protocolVersions"`
 }
 

dex/asset.go

@@ -175,7 +175,7 @@ func IntDivUp(val, div int64) int64 {
 	// return (val + div - 1) / div
 }
 
-// SwapContractDetails is critical information for one side of a trade.
+// SwapContractDetails is critical swap data for one side of a trade.
 type SwapContractDetails struct {
 	// From is the sending address.
 	From string

dex/networks/eth/params.go

@@ -15,7 +15,6 @@ import (
 	"os"
 	"time"
 
-	"decred.org/dcrdex/client/asset"
 	"decred.org/dcrdex/dex"
 	v0 "decred.org/dcrdex/dex/networks/eth/contracts/v0"
 	swapv1 "decred.org/dcrdex/dex/networks/eth/contracts/v1"
@@ -250,15 +249,6 @@ type Initiation struct {
 	ValueGwei   uint64
 }
 
-func (i *Initiation) Contract() *asset.Contract {
-	return &asset.Contract{
-		Address:    i.Participant.String(),
-		Value:      i.ValueGwei,
-		SecretHash: i.SecretHash[:],
-		LockTime:   uint64(i.LockTime.Unix()),
-	}
-}
-
 // Redemption is the data used to redeem a swap.
 type Redemption struct {
 	Secret     [32]byte

server/asset/eth/eth.go

@@ -168,7 +168,6 @@ type ethFetcher interface {
 	receipt(context.Context, common.Hash) (*types.Receipt, error)
 	// token- and asset-specific methods
 	loadToken(ctx context.Context, assetID uint32) error
-	// status(ctx context.Context, assetID uint32, contract *dex.SwapContractDetails) (step dexeth.SwapStep, secret [32]byte, blockNumber uint32, err error)
 	accountBalance(ctx context.Context, assetID uint32, addr common.Address) (*big.Int, error)
 }