From f6714c2cb9aad10cc33a468d3103d919852aa912 Mon Sep 17 00:00:00 2001 From: martonp Date: Wed, 6 Oct 2021 11:31:18 -0400 Subject: [PATCH 1/2] client/eth: FundOrder, ReturnCoins, FundingCoins This PR implements locking in the ETH wallet. Since there are no "coins" in the ETH account system as there are in UTXO based coins, the amount of ETH that has been reserved for orders is kept track of in the ETH wallet. This PR also includes a refactoring of the CoinID code in the server. The changes in the client code needed a new coin id type which represented coins in an address that had not yet been spent. --- client/asset/eth/eth.go | 174 +++++++++++++++++-- client/asset/eth/eth_test.go | 250 ++++++++++++++++++++++++++- server/asset/eth/common.go | 236 +++++++++++++++++++------ server/asset/eth/eth.go | 12 +- server/asset/eth/eth_test.go | 325 +++++++++++------------------------ 5 files changed, 700 insertions(+), 297 deletions(-) diff --git a/client/asset/eth/eth.go b/client/asset/eth/eth.go index 00e0cb4adc..c95c7df2d9 100644 --- a/client/asset/eth/eth.go +++ b/client/asset/eth/eth.go @@ -93,7 +93,11 @@ func (d *Driver) Setup(cfg *asset.WalletConfig, logger dex.Logger, network dex.N // DecodeCoinID creates a human-readable representation of a coin ID for Ethereum. func (d *Driver) DecodeCoinID(coinID []byte) (string, error) { - return dexeth.CoinIDToString(coinID) + id, err := dexeth.DecodeCoinID(coinID) + if err != nil { + return "", nil + } + return id.String(), nil } // Info returns basic information about the wallet and asset. @@ -164,6 +168,9 @@ type ExchangeWallet struct { cachedInitGas uint64 cachedInitGasMtx sync.Mutex + + lockedFunds uint64 // gwei + lockedFundsMtx sync.RWMutex } // Info returns basic information about the wallet and asset. @@ -284,6 +291,15 @@ func (eth *ExchangeWallet) OwnsAddress(address string) (bool, error) { // // TODO: Return Immature and Locked values. func (eth *ExchangeWallet) Balance() (*asset.Balance, error) { + eth.lockedFundsMtx.Lock() + defer eth.lockedFundsMtx.Unlock() + + return eth.balanceImpl() +} + +// balanceImpl returns the total available funds in the account. +// This function expects eth.lockedFundsMtx to be held. +func (eth *ExchangeWallet) balanceImpl() (*asset.Balance, error) { if eth.acct == nil { return nil, errors.New("account not set") } @@ -296,9 +312,9 @@ func (eth *ExchangeWallet) Balance() (*asset.Balance, error) { return nil, err } bal := &asset.Balance{ - Available: gwei, + Available: gwei - eth.lockedFunds, + Locked: eth.lockedFunds, // Immature: , How to know? - // Locked: , Not lockable? } return bal, nil } @@ -424,28 +440,154 @@ func (*ExchangeWallet) PreRedeem(req *asset.PreRedeemForm) (*asset.PreRedeem, er }, nil } +// coin implements the asset.Coin interface for ETH +type coin struct { + id dexeth.AmountCoinID +} + +// ID is the ETH coins ID. It includes the address the coins came from (20 bytes) +// and the value of the coin (8 bytes). +func (c *coin) ID() dex.Bytes { + serializedBytes := c.id.Encode() + return dex.Bytes(serializedBytes) +} + +// String is a string representation of the coin. +func (c *coin) String() string { + return c.id.String() +} + +// Value returns the value in gwei of the coin. +func (c *coin) Value() uint64 { + return c.id.Amount +} + +var _ asset.Coin = (*coin)(nil) + +// decodeCoinID decodes a coin id into a coin object. The coin id +// must contain an AmountCoinID. +func decodeCoinID(coinID []byte) (*coin, error) { + id, err := dexeth.DecodeCoinID(coinID) + if err != nil { + return nil, err + } + + amountCoinID, ok := id.(*dexeth.AmountCoinID) + if !ok { + return nil, + fmt.Errorf("coinID is expected to be an amount coin id") + } + + return &coin{ + id: *amountCoinID, + }, nil +} + // FundOrder selects coins for use in an order. The coins will be locked, and // will not be returned in subsequent calls to FundOrder or calculated in calls // to Available, unless they are unlocked with ReturnCoins. -// The returned []dex.Bytes contains the redeem scripts for the selected coins. -// Equal number of coins and redeemed scripts must be returned. A nil or empty -// dex.Bytes should be appended to the redeem scripts collection for coins with -// no redeem script. -func (*ExchangeWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, error) { - return nil, nil, asset.ErrNotImplemented +// In UTXO based coins, the returned []dex.Bytes contains the redeem scripts for the +// selected coins, but since there are no redeem scripts in Ethereum, nil is returned. +// Equal number of coins and redeem scripts must be returned. +func (eth *ExchangeWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, error) { + maxFees := ord.DEXConfig.MaxFeeRate * ord.DEXConfig.SwapSize * ord.MaxSwapCount + fundsNeeded := ord.Value + maxFees + err := eth.lockFunds(fundsNeeded) + if err != nil { + return nil, nil, err + } + + var nonce [8]byte + copy(nonce[:], encode.RandomBytes(8)) + var address [20]byte + copy(address[:], eth.acct.Address.Bytes()) + coin := coin{ + id: dexeth.AmountCoinID{ + Address: address, + Amount: fundsNeeded, + Nonce: nonce, + }, + } + + return asset.Coins{&coin}, []dex.Bytes{nil}, nil } // ReturnCoins unlocks coins. This would be necessary in the case of a // canceled order. -func (*ExchangeWallet) ReturnCoins(unspents asset.Coins) error { - return asset.ErrNotImplemented +func (eth *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { + var fundsToUnlock uint64 + for _, coin := range unspents { + coin, err := decodeCoinID(coin.ID()) + if err != nil { + return err + } + + if !bytes.Equal(coin.id.Address.Bytes(), eth.acct.Address.Bytes()) { + return fmt.Errorf("ReturnCoins: coin address: %v != wallet address: %v", + coin.id.Address, eth.acct.Address) + } + + fundsToUnlock += coin.id.Amount + } + + return eth.unlockFunds(fundsToUnlock) } // FundingCoins gets funding coins for the coin IDs. The coins are locked. This // method might be called to reinitialize an order from data stored externally. -// This method will only return funding coins, e.g. unspent transaction outputs. -func (*ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { - return nil, asset.ErrNotImplemented +func (eth *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { + var amountFunded uint64 + coins := make([]asset.Coin, 0, len(ids)) + for _, id := range ids { + coin, err := decodeCoinID(id) + if err != nil { + return nil, err + } + if !bytes.Equal(coin.id.Address.Bytes(), eth.acct.Address.Bytes()) { + return nil, fmt.Errorf("FundingCoins: coin address: %v != wallet address: %v", + coin.id.Address, eth.acct.Address) + } + + amountFunded += coin.id.Amount + coins = append(coins, coin) + } + + err := eth.lockFunds(amountFunded) + if err != nil { + return nil, err + } + + return coins, nil +} + +func (eth *ExchangeWallet) lockFunds(gwei uint64) error { + eth.lockedFundsMtx.Lock() + defer eth.lockedFundsMtx.Unlock() + + balance, err := eth.balanceImpl() + if err != nil { + return err + } + + if balance.Available < gwei { + return fmt.Errorf("currently available %v < locking %v", + balance.Available, gwei) + } + + eth.lockedFunds += gwei + return nil +} + +func (eth *ExchangeWallet) unlockFunds(gwei uint64) error { + eth.lockedFundsMtx.Lock() + defer eth.lockedFundsMtx.Unlock() + + if eth.lockedFunds < gwei { + return fmt.Errorf("currently locked %v < unlocking %v", eth.lockedFunds, gwei) + } + + eth.lockedFunds -= gwei + return nil } // Swap sends the swaps in a single transaction. The Receipts returned can be @@ -510,6 +652,10 @@ func (eth *ExchangeWallet) Unlock(pw string) error { // Lock locks the exchange wallet. func (eth *ExchangeWallet) Lock() error { + eth.lockedFundsMtx.Lock() + eth.lockedFunds = 0 + eth.lockedFundsMtx.Unlock() + return eth.node.lock(eth.ctx, eth.acct) } diff --git a/client/asset/eth/eth_test.go b/client/asset/eth/eth_test.go index 2ad9af307e..9f06717cf1 100644 --- a/client/asset/eth/eth_test.go +++ b/client/asset/eth/eth_test.go @@ -7,6 +7,7 @@ package eth import ( "context" + "encoding/hex" "errors" "math/big" "testing" @@ -14,6 +15,7 @@ import ( "decred.org/dcrdex/client/asset" "decred.org/dcrdex/dex" + "decred.org/dcrdex/dex/encode" swap "decred.org/dcrdex/dex/networks/eth" dexeth "decred.org/dcrdex/server/asset/eth" "github.com/ethereum/go-ethereum" @@ -67,7 +69,9 @@ func (n *testNode) accounts() []*accounts.Account { return nil } func (n *testNode) balance(ctx context.Context, acct *common.Address) (*big.Int, error) { - return n.bal, n.balErr + balCopy := new(big.Int) + balCopy.Set(n.bal) + return balCopy, n.balErr } func (n *testNode) sendTransaction(ctx context.Context, tx map[string]string) (common.Hash, error) { return common.Hash{}, nil @@ -356,6 +360,250 @@ func TestBalance(t *testing.T) { } } +// badCoin fulfills the asset.Coin interface, but the ID does not match the ID expected in the +// ETH wallet code. +type badCoin uint64 + +func (*badCoin) ID() dex.Bytes { + return []byte{123} +} +func (*badCoin) String() string { + return "abc" +} +func (b *badCoin) Value() uint64 { + return uint64(*b) +} + +func TestFundOrderReturnCoinsFundingCoins(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + node := &testNode{} + walletBalanceGwei := uint64(dexeth.GweiFactor) + node.bal = big.NewInt(int64(walletBalanceGwei) * dexeth.GweiFactor) + address := "0xB6De8BB5ed28E6bE6d671975cad20C03931bE981" + account := accounts.Account{ + Address: common.HexToAddress(address), + } + eth := &ExchangeWallet{ + node: node, + ctx: ctx, + log: tLogger, + acct: &account, + } + + checkBalance := func(wallet *ExchangeWallet, expectedAvailable, expectedLocked uint64, testName string) { + balance, err := wallet.Balance() + if err != nil { + t.Fatalf("%v: unexpected error %v", testName, err) + } + if balance.Available != expectedAvailable { + t.Fatalf("%v: expected %v funds to be available but got %v", testName, expectedAvailable, balance.Available) + } + if balance.Locked != expectedLocked { + t.Fatalf("%v: expected %v funds to be locked but got %v", testName, expectedLocked, balance.Locked) + } + } + + type fundOrderTest struct { + testName string + wantErr bool + coinValue uint64 + coinAddress string + } + checkFundOrderResult := func(coins asset.Coins, redeemScripts []dex.Bytes, err error, test fundOrderTest) { + if test.wantErr && err == nil { + t.Fatalf("%v: expected error but didn't get", test.testName) + } + if test.wantErr { + return + } + if err != nil { + t.Fatalf("%v: unexpected error: %v", test.testName, err) + } + if len(coins) != 1 { + t.Fatalf("%v: expected 1 coins but got %v", test.testName, len(coins)) + } + if len(redeemScripts) != 1 { + t.Fatalf("%v: expected 1 redeem script but got %v", test.testName, len(redeemScripts)) + } + coin, err := decodeCoinID(coins[0].ID()) + if err != nil { + t.Fatalf("%v: unexpected error: %v", test.testName, err) + } + if coin.id.Address.String() != test.coinAddress { + t.Fatalf("%v: coin address expected to be %v, but got %v", test.testName, test.coinAddress, coin.id.Address.String()) + } + if coins[0].Value() != test.coinValue { + t.Fatalf("%v: expected %v but got %v", test.testName, test.coinValue, coins[0].Value()) + } + } + + order := asset.Order{ + Value: 500000000, + MaxSwapCount: 2, + DEXConfig: &dex.Asset{ + ID: 60, + Symbol: "ETH", + Version: 0, + MaxFeeRate: 150, + SwapSize: 180000, + }, + } + + // Test fund order with less than available funds + coins1, redeemScripts1, err := eth.FundOrder(&order) + expectedOrderFees := order.DEXConfig.SwapSize * order.DEXConfig.MaxFeeRate * order.MaxSwapCount + expectedCoinValue := order.Value + expectedOrderFees + checkFundOrderResult(coins1, redeemScripts1, err, fundOrderTest{ + testName: "more than enough", + coinValue: expectedCoinValue, + coinAddress: address, + }) + checkBalance(eth, walletBalanceGwei-expectedCoinValue, expectedCoinValue, "more than enough") + + // Test fund order with 1 more than available funds + order.Value = walletBalanceGwei - expectedCoinValue - expectedOrderFees + 1 + coins, redeemScripts, err := eth.FundOrder(&order) + checkFundOrderResult(coins, redeemScripts, err, fundOrderTest{ + testName: "not enough", + wantErr: true, + }) + checkBalance(eth, walletBalanceGwei-expectedCoinValue, expectedCoinValue, "not enough") + + // Test fund order with funds equal to available + order.Value = order.Value - 1 + coins2, redeemScripts2, err := eth.FundOrder(&order) + checkFundOrderResult(coins2, redeemScripts2, err, fundOrderTest{ + testName: "just enough", + coinValue: order.Value + expectedOrderFees, + coinAddress: address, + }) + checkBalance(eth, 0, walletBalanceGwei, "just enough") + + // Test returning too much funds fails + err = eth.ReturnCoins([]asset.Coin{coins1[0], coins1[0]}) + if err == nil { + t.Fatalf("expected error but did not get") + } + checkBalance(eth, 0, walletBalanceGwei, "after redeem too much") + + // Test returning coin with invalid ID + var badCoin badCoin + err = eth.ReturnCoins([]asset.Coin{&badCoin}) + if err == nil { + t.Fatalf("expected error but did not get") + } + + // Test returning correct coins returns all funds + err = eth.ReturnCoins([]asset.Coin{coins1[0], coins2[0]}) + if err != nil { + t.Fatalf("unexpected error") + } + checkBalance(eth, walletBalanceGwei, 0, "returned correct amount") + + node.balErr = errors.New("") + _, _, err = eth.FundOrder(&order) + if err == nil { + t.Fatalf("balance error should cause error but did not") + } + node.balErr = nil + + eth2 := &ExchangeWallet{ + node: node, + ctx: ctx, + log: tLogger, + acct: &account, + } + + // Test reloading coins from first order + coins, err = eth2.FundingCoins([]dex.Bytes{coins1[0].ID()}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(coins) != 1 { + t.Fatalf("expected 1 coins but got %v", len(coins)) + } + if coins[0].Value() != coins1[0].Value() { + t.Fatalf("funding coin value %v != expected %v", coins[0].Value(), coins[1].Value()) + } + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "funding1") + + // Test reloading more coins than are available in balance + _, err = eth2.FundingCoins([]dex.Bytes{coins1[0].ID()}) + if err == nil { + t.Fatalf("expected error but didn't get one") + } + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 1") + + // Test funding coins with bad coin ID + _, err = eth2.FundingCoins([]dex.Bytes{badCoin.ID()}) + if err == nil { + t.Fatalf("expected error but did not get") + } + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 2") + + // Test funding coins with coin from different address + var differentAddress [20]byte + decodedHex, _ := hex.DecodeString("345853e21b1d475582E71cC269124eD5e2dD3422") + copy(differentAddress[:], decodedHex) + var nonce [8]byte + copy(nonce[:], encode.RandomBytes(8)) + differentAddressCoin := coin{ + id: dexeth.AmountCoinID{ + Address: differentAddress, + Amount: 100000, + Nonce: nonce, + }, + } + _, err = eth2.FundingCoins([]dex.Bytes{differentAddressCoin.ID()}) + if err == nil { + t.Fatalf("expected error but did not get") + } + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 3") + + // Test funding coins with balance error + node.balErr = errors.New("") + _, err = eth2.FundingCoins([]dex.Bytes{badCoin.ID()}) + if err == nil { + t.Fatalf("expected error but did not get") + } + node.balErr = nil + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 4") + + // Reloading coins from second order + coins, err = eth2.FundingCoins([]dex.Bytes{coins2[0].ID()}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(coins) != 1 { + t.Fatalf("expected 1 coins but got %v", len(coins)) + } + if coins[0].Value() != coins2[0].Value() { + t.Fatalf("funding coin value %v != expected %v", coins[0].Value(), coins[1].Value()) + } + checkBalance(eth2, 0, walletBalanceGwei, "funding2") + + // return coin with incorrect address + err = eth2.ReturnCoins([]asset.Coin{&differentAddressCoin}) + if err == nil { + t.Fatalf("expected error but did not get") + } + + // return all coins + err = eth2.ReturnCoins([]asset.Coin{coins1[0], coins2[0]}) + if err != nil { + t.Fatalf("unexpected error") + } + checkBalance(eth2, walletBalanceGwei, 0, "return coins after funding") + + // Test funding coins with two coins at the same time + _, err = eth2.FundingCoins([]dex.Bytes{coins1[0].ID(), coins2[0].ID()}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + checkBalance(eth2, 0, walletBalanceGwei, "funding3") +} func TestGetInitGas(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) node := &testNode{} diff --git a/server/asset/eth/common.go b/server/asset/eth/common.go index 6bb268e6e7..1cd7592abb 100644 --- a/server/asset/eth/common.go +++ b/server/asset/eth/common.go @@ -54,12 +54,188 @@ const ( // contract address and secret hash used to fetch data about a swap // from the live contract. CIDSwap + // CIDAmount indicates that this coin ID represents a coin which has + // not yet been submitted to the swap contract. It contains the address + // which holds the ETH, an amount of ETH in gwei, and a random nonce to + // avoid duplicate coin IDs. + CIDAmount ) +// CoinID is an interface that objects which represent different types of ETH +// coin identifiers must implement. +type CoinID interface { + String() string + Encode() []byte +} + +const ( + // coin type id (2) + tx id (32) = 34 + txCoinIDSize = 34 + // coin type id (2) + address (20) + secret has (32) = 54 + swapCoinIDSize = 54 + // coin type id (2) + address (20) + amount (8) + nonce (8) = 38 + amountCoinIDSize = 38 +) + +// TxCoinID identifies a coin by the transaction ID that was used to send it +// to the smart contract. This type of ID is useful to identify coins that +// were sent in transactions that have not yet been mined. +type TxCoinID struct { + TxID [32]byte +} + +// String creates a human readable string. +func (c *TxCoinID) String() string { + return fmt.Sprintf("tx: %x", c.TxID) +} + +// Encode creates a byte slice that can be decoded with DecodeCoinID. +func (c *TxCoinID) Encode() []byte { + b := make([]byte, txCoinIDSize) + binary.BigEndian.PutUint16(b[:2], uint16(CIDTxID)) + copy(b[2:], c.TxID[:]) + return b +} + +var _ CoinID = (*TxCoinID)(nil) + +// decodeTxCoinID decodes a byte slice into an TxCoinID struct. +func decodeTxCoinID(coinID []byte) (*TxCoinID, error) { + if len(coinID) != txCoinIDSize { + return nil, fmt.Errorf("decodeTxCoinID: length expected %v, got %v", + txCoinIDSize, len(coinID)) + } + + flag := binary.BigEndian.Uint16(coinID[:2]) + if CoinIDFlag(flag) != CIDTxID { + return nil, fmt.Errorf("decodeTxCoinID: flag expected %v, got %v", + flag, CIDTxID) + } + + var txID [32]byte + copy(txID[:], coinID[2:]) + return &TxCoinID{ + TxID: txID, + }, nil +} + +// SwapCoinID identifies a coin in a swap contract. +type SwapCoinID struct { + ContractAddress common.Address + SecretHash [32]byte +} + +// String creates a human readable string. +func (c *SwapCoinID) String() string { + return fmt.Sprintf("contract: %v, secret hash:%x", + c.ContractAddress, c.SecretHash) +} + +// Encode creates a byte slice that can be decoded with DecodeCoinID. +func (c *SwapCoinID) Encode() []byte { + b := make([]byte, swapCoinIDSize) + binary.BigEndian.PutUint16(b[:2], uint16(CIDSwap)) + copy(b[2:22], c.ContractAddress[:]) + copy(b[22:], c.SecretHash[:]) + return b +} + +var _ CoinID = (*SwapCoinID)(nil) + +// decodeSwapCoinID decodes a byte slice into an SwapCoinID struct. +func decodeSwapCoinID(coinID []byte) (*SwapCoinID, error) { + if len(coinID) != swapCoinIDSize { + return nil, fmt.Errorf("decodeSwapCoinID: length expected %v, got %v", + txCoinIDSize, len(coinID)) + } + + flag := binary.BigEndian.Uint16(coinID[:2]) + if CoinIDFlag(flag) != CIDSwap { + return nil, fmt.Errorf("decodeSwapCoinID: flag expected %v, got %v", + flag, CIDSwap) + } + + var contractAddress [20]byte + var secretHash [32]byte + copy(contractAddress[:], coinID[2:22]) + copy(secretHash[:], coinID[22:]) + return &SwapCoinID{ + ContractAddress: contractAddress, + SecretHash: secretHash, + }, nil +} + +// AmountCoinID is an identifier for a coin which has not yet been sent to the +// swap contract. +type AmountCoinID struct { + Address common.Address + Amount uint64 + Nonce [8]byte +} + +// String creates a human readable string. +func (c *AmountCoinID) String() string { + return fmt.Sprintf("address: %v, amount:%x, nonce:%x", + c.Address, c.Amount, c.Nonce) +} + +// Encode creates a byte slice that can be decoded with DecodeCoinID. +func (c *AmountCoinID) Encode() []byte { + b := make([]byte, amountCoinIDSize) + binary.BigEndian.PutUint16(b[:2], uint16(CIDAmount)) + copy(b[2:22], c.Address[:]) + binary.BigEndian.PutUint64(b[22:30], c.Amount) + copy(b[30:], c.Nonce[:]) + return b +} + +var _ CoinID = (*AmountCoinID)(nil) + +// decodeAmountCoinID decodes a byte slice into an AmountCoinID struct. +func decodeAmountCoinID(coinID []byte) (*AmountCoinID, error) { + if len(coinID) != amountCoinIDSize { + return nil, fmt.Errorf("DecodeAmountCoinID: length expected %v, got %v", + txCoinIDSize, len(coinID)) + } + + flag := binary.BigEndian.Uint16(coinID[:2]) + if CoinIDFlag(flag) != CIDAmount { + return nil, fmt.Errorf("DecodeAmountCoinID: flag expected %v, got %v", + flag, CIDAmount) + } + + var address [20]byte + var nonce [8]byte + copy(address[:], coinID[2:22]) + copy(nonce[:], coinID[30:]) + return &AmountCoinID{ + Address: address, + Amount: binary.BigEndian.Uint64(coinID[22:30]), + Nonce: nonce, + }, nil +} + +// DecodeCoinID decodes the coin id byte slice into an object implementing the +// CoinID interface. +func DecodeCoinID(coinID []byte) (CoinID, error) { + if len(coinID) < 2 { + return nil, + fmt.Errorf("DecodeCoinID: coinID length must be > 2, but got %v", + len(coinID)) + } + flag := CoinIDFlag(binary.BigEndian.Uint16(coinID[:2])) + if flag == CIDTxID { + return decodeTxCoinID(coinID) + } else if flag == CIDSwap { + return decodeSwapCoinID(coinID) + } else if flag == CIDAmount { + return decodeAmountCoinID(coinID) + } else { + return nil, fmt.Errorf("DecodeCoinID: invalid coin id flag: %v", flag) + } +} + const ( - // coinIdSize = flags (2) + smart contract address where funds are - // locked (20) + secret hash map key (32) - coinIDSize = 54 // MaxBlockInterval is the number of seconds since the last header came // in over which we consider the chain to be out of sync. MaxBlockInterval = 180 @@ -106,57 +282,3 @@ func (ss SwapState) String() string { } return "unknown" } - -// DecodeCoinID decodes the coin ID into flags, a contract address, and hash -// that represents either a secret or txid depending on flags. The CIDTxID flag -// indicates that this coinID represents a simple txid. The address return is -// unused and bytes are a 32 byte txid. Bytes are in the same order as the txid. -// The CIDSwap flag indicates that this coinID represents a swap contract. The -// address is the swap contract's address and the bytes return is the 32 byte -// secret hash of a swap. Errors if the passed coinID is not the expected length. -func DecodeCoinID(coinID []byte) (CoinIDFlag, *common.Address, []byte, error) { - if len(coinID) != coinIDSize { - return 0, nil, nil, fmt.Errorf("coin ID wrong length. expected %d, got %d", - coinIDSize, len(coinID)) - } - hash, addr := make([]byte, 32), new(common.Address) - copy(hash, coinID[22:]) - copy(addr[:], coinID[2:22]) - return CoinIDFlag(binary.BigEndian.Uint16(coinID[:2])), - addr, hash, nil -} - -// CoinIDToString converts coinID into a human readable string. -func CoinIDToString(coinID []byte) (string, error) { - flags, addr, hash, err := DecodeCoinID(coinID) - if err != nil { - return "", err - } - return fmt.Sprintf("%x:%x:%x", flags, addr, hash), nil -} - -// ToCoinID converts the address and secret hash, or txid to a coin ID. hash is -// expected to be 32 bytes long and represent either a txid or a secret hash. A -// hash longer than 32 bytes is truncated at 32. If a txid, bytes should be -// kept in the same order as the hex string representation. -func ToCoinID(flags CoinIDFlag, addr *common.Address, hash []byte) []byte { - b := make([]byte, coinIDSize) - binary.BigEndian.PutUint16(b[:2], uint16(flags)) - if IsCIDSwap(flags) { - copy(b[2:], addr[:]) - } - copy(b[22:], hash[:]) - return b -} - -// IsCIDTxID returns whether the passed flags indicate the associated coin ID's -// hash portion represents a transaction hash. -func IsCIDTxID(flags CoinIDFlag) bool { - return flags == CIDTxID -} - -// IsCIDSwap returns whether the passed flags indicate the associated coin ID -// represents a swap with an address and secret hash. -func IsCIDSwap(flags CoinIDFlag) bool { - return flags == CIDSwap -} diff --git a/server/asset/eth/eth.go b/server/asset/eth/eth.go index 4e71256b34..e4d4d1b2d7 100644 --- a/server/asset/eth/eth.go +++ b/server/asset/eth/eth.go @@ -55,7 +55,11 @@ func (d *Driver) Setup(configPath string, logger dex.Logger, network dex.Network // DecodeCoinID creates a human-readable representation of a coin ID for Ethereum. func (d *Driver) DecodeCoinID(coinID []byte) (string, error) { - return CoinIDToString(coinID) + coinId, err := DecodeCoinID(coinID) + if err != nil { + return "", err + } + return coinId.String(), nil } // ethFetcher represents a blockchain information fetcher. In practice, it is @@ -245,7 +249,11 @@ func (eth *Backend) FundingCoin(ctx context.Context, coinID []byte, redeemScript // ValidateCoinID attempts to decode the coinID. func (eth *Backend) ValidateCoinID(coinID []byte) (string, error) { - return CoinIDToString(coinID) + coinId, err := DecodeCoinID(coinID) + if err != nil { + return "", err + } + return coinId.String(), nil } // ValidateContract ensures that the swap contract is constructed properly, and diff --git a/server/asset/eth/eth_test.go b/server/asset/eth/eth_test.go index 95295ef229..d0d26ca441 100644 --- a/server/asset/eth/eth_test.go +++ b/server/asset/eth/eth_test.go @@ -8,16 +8,15 @@ package eth import ( "bytes" "context" - "encoding/hex" + "encoding/binary" "errors" - "fmt" "math/big" - "reflect" "testing" "time" "decred.org/dcrdex/dex" "decred.org/dcrdex/dex/calc" + "decred.org/dcrdex/dex/encode" "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -121,129 +120,110 @@ func TestLoad(t *testing.T) { } } -func TestDecodeCoinID(t *testing.T) { - tests := []struct { - name string - wantFlags CoinIDFlag - wantAddr *common.Address - coinID, wantSecretHash []byte - wantErr bool - }{{ - name: "ok", - coinID: []byte{ - 0x00, 0x01, // 2 byte flags - 0x18, 0xd6, 0x5f, 0xb8, 0xd6, 0x0c, 0x11, 0x99, 0xbb, - 0x1a, 0xd3, 0x81, 0xbe, 0x47, 0xaa, 0x69, 0x2b, 0x48, - 0x26, 0x05, // 20 byte addr - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, 0x3c, // 32 byte secret hash - }, - wantFlags: CIDTxID, - wantAddr: &common.Address{ - 0x18, 0xd6, 0x5f, 0xb8, 0xd6, 0x0c, 0x11, 0x99, 0xbb, - 0x1a, 0xd3, 0x81, 0xbe, 0x47, 0xaa, 0x69, 0x2b, 0x48, - 0x26, 0x05, - }, - wantSecretHash: []byte{ - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, 0x3c, // 32 byte secret hash - }, - }, { - name: "wrong length", - coinID: []byte{ - 0xFF, 0x01, // 2 byte flags - 0x18, 0xd6, 0x5f, 0xb8, 0xd6, 0x0c, 0x11, 0x99, 0xbb, - 0x1a, 0xd3, 0x81, 0xbe, 0x47, 0xaa, 0x69, 0x2b, 0x48, - 0x26, 0x05, // 20 byte addr - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, // 31 bytes - }, - wantErr: true, - }} +func TestCoinIDs(t *testing.T) { + // Decode and encode TxCoinID + var txID [32]byte + copy(txID[:], encode.RandomBytes(32)) + originalTxCoin := TxCoinID{ + TxID: txID, + } + encodedTxCoin := originalTxCoin.Encode() + decodedCoin, err := DecodeCoinID(encodedTxCoin) + if err != nil { + t.Fatalf("unexpected error decoding tx coin: %v", err) + } + decodedTxCoin, ok := decodedCoin.(*TxCoinID) + if !ok { + t.Fatalf("expected coin to be a TxCoin") + } + if !bytes.Equal(originalTxCoin.TxID[:], decodedTxCoin.TxID[:]) { + t.Fatalf("expected txIds to be equal before and after decoding") + } - for _, test := range tests { - flags, addr, secretHash, err := DecodeCoinID(test.coinID) - if test.wantErr { - if err == nil { - t.Fatalf("expected error for test %v", test.name) - } - continue - } - if err != nil { - t.Fatalf("unexpected error for test %v: %v", test.name, err) - } - if flags != test.wantFlags { - t.Fatalf("want flags value of %v but got %v for test %v", - test.wantFlags, flags, test.name) - } - if *addr != *test.wantAddr { - t.Fatalf("want addr value of %v but got %v for test %v", - test.wantAddr, addr, test.name) - } - if !reflect.DeepEqual(secretHash, test.wantSecretHash) { - t.Fatalf("want secret hash value of %v but got %v for test %v", - test.wantSecretHash, secretHash, test.name) - } + // Decode tx coin id with incorrect length + txCoinID := make([]byte, 33) + binary.BigEndian.PutUint16(txCoinID[:2], uint16(CIDTxID)) + copy(txCoinID[2:], encode.RandomBytes(30)) + if _, err := DecodeCoinID(txCoinID); err == nil { + t.Fatalf("expected error decoding tx coin ID with incorrect length") } -} -func TestCoinIDToString(t *testing.T) { - flags := CIDSwap - addr := "18d65fb8d60c1199bb1ad381be47aa692b482605" - secretHash := "71d810d39333296b518c846a3e49eca55f998fd7994998bb3e5048567f2f073c" - tests := []struct { - name, wantCoinID string - coinID []byte - wantErr bool - }{{ - name: "ok", - coinID: []byte{ - 0x00, 0x02, // 2 byte flags - 0x18, 0xd6, 0x5f, 0xb8, 0xd6, 0x0c, 0x11, 0x99, 0xbb, - 0x1a, 0xd3, 0x81, 0xbe, 0x47, 0xaa, 0x69, 0x2b, 0x48, - 0x26, 0x05, // 20 byte addr - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, 0x3c, // 32 byte secret hash - }, - wantCoinID: fmt.Sprintf("%d:%s:%s", flags, addr, secretHash), - }, { - name: "wrong length", - coinID: []byte{ - 0x00, 0x01, // 2 byte flags - 0x18, 0xd6, 0x5f, 0xb8, 0xd6, 0x0c, 0x11, 0x99, 0xbb, - 0x1a, 0xd3, 0x81, 0xbe, 0x47, 0xaa, 0x69, 0x2b, 0x48, - 0x26, 0x05, // 20 byte addr - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, // 31 bytes - }, - wantErr: true, - }} + // Decode and encode SwapCoinID + var contractAddress [20]byte + var secretHash [32]byte + copy(contractAddress[:], encode.RandomBytes(20)) + copy(secretHash[:], encode.RandomBytes(32)) + originalSwapCoin := SwapCoinID{ + ContractAddress: contractAddress, + SecretHash: secretHash, + } + encodedSwapCoin := originalSwapCoin.Encode() + decodedCoin, err = DecodeCoinID(encodedSwapCoin) + if err != nil { + t.Fatalf("unexpected error decoding swap coin: %v", err) + } + decodedSwapCoin, ok := decodedCoin.(*SwapCoinID) + if !ok { + t.Fatalf("expected coin to be a SwapCoinID") + } + if !bytes.Equal(originalSwapCoin.ContractAddress[:], decodedSwapCoin.ContractAddress[:]) { + t.Fatalf("expected contract address to be equal before and after decoding") + } + if !bytes.Equal(originalSwapCoin.SecretHash[:], decodedSwapCoin.SecretHash[:]) { + t.Fatalf("expected secret hash to be equal before and after decoding") + } - for _, test := range tests { - coinID, err := CoinIDToString(test.coinID) - if test.wantErr { - if err == nil { - t.Fatalf("expected error for test %v", test.name) - } - continue - } - if err != nil { - t.Fatalf("unexpected error for test %v: %v", test.name, err) - } - if coinID != test.wantCoinID { - t.Fatalf("want coinID value of %v but got %v for test %v", - test.wantCoinID, coinID, test.name) - } + // Decode swap coin id with incorrect length + swapCoinID := make([]byte, 53) + binary.BigEndian.PutUint16(swapCoinID[:2], uint16(CIDSwap)) + copy(swapCoinID[2:], encode.RandomBytes(50)) + if _, err := DecodeCoinID(swapCoinID); err == nil { + t.Fatalf("expected error decoding swap coin ID with incorrect length") + } + + // Decode and encode AmountCoinID + var address [20]byte + var nonce [8]byte + copy(address[:], encode.RandomBytes(20)) + copy(nonce[:], encode.RandomBytes(8)) + originalAmountCoin := AmountCoinID{ + Address: address, + Amount: 100, + Nonce: nonce, + } + encodedAmountCoin := originalAmountCoin.Encode() + decodedCoin, err = DecodeCoinID(encodedAmountCoin) + if err != nil { + t.Fatalf("unexpected error decoding swap coin: %v", err) + } + decodedAmountCoin, ok := decodedCoin.(*AmountCoinID) + if !ok { + t.Fatalf("expected coin to be a AmounCoinID") + } + if !bytes.Equal(originalAmountCoin.Address[:], decodedAmountCoin.Address[:]) { + t.Fatalf("expected address to be equal before and after decoding") + } + if !bytes.Equal(originalAmountCoin.Nonce[:], decodedAmountCoin.Nonce[:]) { + t.Fatalf("expected nonce to be equal before and after decoding") + } + if originalAmountCoin.Amount != decodedAmountCoin.Amount { + t.Fatalf("expected amount to be equal before and after decoding") + } + + // Decode amount coin id with incorrect length + amountCoinId := make([]byte, 37) + binary.BigEndian.PutUint16(amountCoinId[:2], uint16(CIDAmount)) + copy(amountCoinId[2:], encode.RandomBytes(35)) + if _, err := DecodeCoinID(amountCoinId); err == nil { + t.Fatalf("expected error decoding amount coin ID with incorrect length") + } + + // Decode coin id with non existant flag + nonExistantCoinID := make([]byte, 37) + binary.BigEndian.PutUint16(nonExistantCoinID[:2], uint16(5)) + copy(nonExistantCoinID, encode.RandomBytes(35)) + if _, err := DecodeCoinID(nonExistantCoinID); err == nil { + t.Fatalf("expected error decoding coin id with non existant flag") } } @@ -399,107 +379,6 @@ func TestSynced(t *testing.T) { } } -func TestIsCIDTxID(t *testing.T) { - tests := []struct { - name string - flag CoinIDFlag - want bool - }{{ - name: "true", - flag: CIDTxID, - want: true, - }, { - name: "false zero", - }, { - name: "false other", - flag: CIDSwap, - }, { - name: "false txid and other", - flag: CIDTxID | CIDSwap, - }} - - for _, test := range tests { - got := IsCIDTxID(test.flag) - if got != test.want { - t.Fatalf("want %v but got %v for test %v", test.want, got, test.name) - } - } -} - -func TestCIDSwap(t *testing.T) { - tests := []struct { - name string - flag CoinIDFlag - want bool - }{{ - name: "true", - flag: CIDSwap, - want: true, - }, { - name: "false zero", - }, { - name: "false other", - flag: CIDTxID, - }, { - name: "false txid and other", - flag: CIDTxID | CIDSwap, - }} - - for _, test := range tests { - got := IsCIDSwap(test.flag) - if got != test.want { - t.Fatalf("want %v but got %v for test %v", test.want, got, test.name) - } - } -} - -func TestToCoinID(t *testing.T) { - a := common.HexToAddress("18d65fb8d60c1199bb1ad381be47aa692b482605") - addr := &a - secretHash, err := hex.DecodeString("71d810d39333296b518c846a3e49eca55f998fd7994998bb3e5048567f2f073c") - if err != nil { - panic(err) - } - tests := []struct { - name string - flag CoinIDFlag - want []byte - }{{ - name: "txid", - flag: CIDTxID, - want: []byte{ - 0x00, 0x01, // 2 byte flags - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, // 20 byte addr - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, 0x3c, // 32 byte secret hash - }, - }, { - name: "swap", - flag: CIDSwap, - want: []byte{ - 0x00, 0x02, // 2 byte flags - 0x18, 0xd6, 0x5f, 0xb8, 0xd6, 0x0c, 0x11, 0x99, 0xbb, - 0x1a, 0xd3, 0x81, 0xbe, 0x47, 0xaa, 0x69, 0x2b, 0x48, - 0x26, 0x05, // 20 byte addr - 0x71, 0xd8, 0x10, 0xd3, 0x93, 0x33, 0x29, 0x6b, 0x51, - 0x8c, 0x84, 0x6a, 0x3e, 0x49, 0xec, 0xa5, 0x5f, 0x99, - 0x8f, 0xd7, 0x99, 0x49, 0x98, 0xbb, 0x3e, 0x50, 0x48, - 0x56, 0x7f, 0x2f, 0x07, 0x3c, // 32 byte secret hash - }, - }} - - for _, test := range tests { - got := ToCoinID(test.flag, addr, secretHash) - if !bytes.Equal(got, test.want) { - t.Fatalf("want %x but got %x for test %v", test.want, got, test.name) - } - } -} - // TestRequiredOrderFunds ensures that a fee calculation in the calc package // will come up with the correct required funds. func TestRequiredOrderFunds(t *testing.T) { From 85f8c02f4061241de435bd51c39d118246d0aecd Mon Sep 17 00:00:00 2001 From: martonp Date: Thu, 7 Oct 2021 16:44:34 -0400 Subject: [PATCH 2/2] Updates based on review. --- client/asset/eth/eth.go | 103 +++++++++++++++++++++-------------- client/asset/eth/eth_test.go | 37 +++++++++---- 2 files changed, 88 insertions(+), 52 deletions(-) diff --git a/client/asset/eth/eth.go b/client/asset/eth/eth.go index c95c7df2d9..7a76396edc 100644 --- a/client/asset/eth/eth.go +++ b/client/asset/eth/eth.go @@ -169,7 +169,7 @@ type ExchangeWallet struct { cachedInitGas uint64 cachedInitGasMtx sync.Mutex - lockedFunds uint64 // gwei + lockedFunds map[string]uint64 // gwei lockedFundsMtx sync.RWMutex } @@ -198,6 +198,7 @@ func NewWallet(assetCFG *asset.WalletConfig, logger dex.Logger, network dex.Netw log: logger, tipChange: assetCFG.TipChange, internalNode: node, + lockedFunds: make(map[string]uint64), }, nil } @@ -307,13 +308,23 @@ func (eth *ExchangeWallet) balanceImpl() (*asset.Balance, error) { if err != nil { return nil, err } - gwei, err := dexeth.ToGwei(bigBal) + gweiBal, err := dexeth.ToGwei(bigBal) if err != nil { return nil, err } + + var amountLocked uint64 + for _, value := range eth.lockedFunds { + amountLocked += value + } + if amountLocked > gweiBal { + return nil, + fmt.Errorf("amount locked: %v > available: %v", amountLocked, gweiBal) + } + bal := &asset.Balance{ - Available: gwei - eth.lockedFunds, - Locked: eth.lockedFunds, + Available: gweiBal - amountLocked, + Locked: amountLocked, // Immature: , How to know? } return bal, nil @@ -492,10 +503,6 @@ func decodeCoinID(coinID []byte) (*coin, error) { func (eth *ExchangeWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, error) { maxFees := ord.DEXConfig.MaxFeeRate * ord.DEXConfig.SwapSize * ord.MaxSwapCount fundsNeeded := ord.Value + maxFees - err := eth.lockFunds(fundsNeeded) - if err != nil { - return nil, nil, err - } var nonce [8]byte copy(nonce[:], encode.RandomBytes(8)) @@ -508,35 +515,25 @@ func (eth *ExchangeWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes Nonce: nonce, }, } + coins := asset.Coins{&coin} - return asset.Coins{&coin}, []dex.Bytes{nil}, nil + err := eth.lockFunds(coins) + if err != nil { + return nil, nil, err + } + + return coins, []dex.Bytes{nil}, nil } // ReturnCoins unlocks coins. This would be necessary in the case of a // canceled order. func (eth *ExchangeWallet) ReturnCoins(unspents asset.Coins) error { - var fundsToUnlock uint64 - for _, coin := range unspents { - coin, err := decodeCoinID(coin.ID()) - if err != nil { - return err - } - - if !bytes.Equal(coin.id.Address.Bytes(), eth.acct.Address.Bytes()) { - return fmt.Errorf("ReturnCoins: coin address: %v != wallet address: %v", - coin.id.Address, eth.acct.Address) - } - - fundsToUnlock += coin.id.Amount - } - - return eth.unlockFunds(fundsToUnlock) + return eth.unlockFunds(unspents) } // FundingCoins gets funding coins for the coin IDs. The coins are locked. This // method might be called to reinitialize an order from data stored externally. func (eth *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { - var amountFunded uint64 coins := make([]asset.Coin, 0, len(ids)) for _, id := range ids { coin, err := decodeCoinID(id) @@ -544,15 +541,14 @@ func (eth *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { return nil, err } if !bytes.Equal(coin.id.Address.Bytes(), eth.acct.Address.Bytes()) { - return nil, fmt.Errorf("FundingCoins: coin address: %v != wallet address: %v", + return nil, fmt.Errorf("FundingCoins: coin address %v != wallet address %v", coin.id.Address, eth.acct.Address) } - amountFunded += coin.id.Amount coins = append(coins, coin) } - err := eth.lockFunds(amountFunded) + err := eth.lockFunds(coins) if err != nil { return nil, err } @@ -560,33 +556,62 @@ func (eth *ExchangeWallet) FundingCoins(ids []dex.Bytes) (asset.Coins, error) { return coins, nil } -func (eth *ExchangeWallet) lockFunds(gwei uint64) error { +// lockFunds adds coins to the map of locked funds. +func (eth *ExchangeWallet) lockFunds(coins asset.Coins) error { eth.lockedFundsMtx.Lock() defer eth.lockedFundsMtx.Unlock() + currentlyLocking := make(map[string]bool) + var amountToLock uint64 + for _, coin := range coins { + hexID := hex.EncodeToString(coin.ID()) + if _, ok := eth.lockedFunds[hexID]; ok { + return fmt.Errorf("cannot lock funds that are already locked: %v", hexID) + } + if _, ok := currentlyLocking[hexID]; ok { + return fmt.Errorf("attempting to lock duplicate coins: %v", hexID) + } + currentlyLocking[hexID] = true + amountToLock += coin.Value() + } + balance, err := eth.balanceImpl() if err != nil { return err } - if balance.Available < gwei { + if balance.Available < amountToLock { return fmt.Errorf("currently available %v < locking %v", - balance.Available, gwei) + balance.Available, amountToLock) } - eth.lockedFunds += gwei + for _, coin := range coins { + eth.lockedFunds[hex.EncodeToString(coin.ID())] = coin.Value() + } return nil } -func (eth *ExchangeWallet) unlockFunds(gwei uint64) error { +// lockFunds removes coins from the map of locked funds. +func (eth *ExchangeWallet) unlockFunds(coins asset.Coins) error { eth.lockedFundsMtx.Lock() defer eth.lockedFundsMtx.Unlock() - if eth.lockedFunds < gwei { - return fmt.Errorf("currently locked %v < unlocking %v", eth.lockedFunds, gwei) + currentlyUnlocking := make(map[string]bool) + for _, coin := range coins { + hexID := hex.EncodeToString(coin.ID()) + if _, ok := eth.lockedFunds[hexID]; !ok { + return fmt.Errorf("cannot unlock coin ID that is not locked: %v", coin.ID()) + } + if _, ok := currentlyUnlocking[hexID]; ok { + return fmt.Errorf("attempting to unlock duplicate coins: %v", hexID) + } + currentlyUnlocking[hexID] = true + } + + for _, coin := range coins { + delete(eth.lockedFunds, hex.EncodeToString(coin.ID())) } - eth.lockedFunds -= gwei return nil } @@ -652,10 +677,6 @@ func (eth *ExchangeWallet) Unlock(pw string) error { // Lock locks the exchange wallet. func (eth *ExchangeWallet) Lock() error { - eth.lockedFundsMtx.Lock() - eth.lockedFunds = 0 - eth.lockedFundsMtx.Unlock() - return eth.node.lock(eth.ctx, eth.acct) } diff --git a/client/asset/eth/eth_test.go b/client/asset/eth/eth_test.go index 9f06717cf1..593b17989c 100644 --- a/client/asset/eth/eth_test.go +++ b/client/asset/eth/eth_test.go @@ -386,10 +386,11 @@ func TestFundOrderReturnCoinsFundingCoins(t *testing.T) { Address: common.HexToAddress(address), } eth := &ExchangeWallet{ - node: node, - ctx: ctx, - log: tLogger, - acct: &account, + node: node, + ctx: ctx, + log: tLogger, + acct: &account, + lockedFunds: make(map[string]uint64), } checkBalance := func(wallet *ExchangeWallet, expectedAvailable, expectedLocked uint64, testName string) { @@ -510,10 +511,11 @@ func TestFundOrderReturnCoinsFundingCoins(t *testing.T) { node.balErr = nil eth2 := &ExchangeWallet{ - node: node, - ctx: ctx, - log: tLogger, - acct: &account, + node: node, + ctx: ctx, + log: tLogger, + acct: &account, + lockedFunds: make(map[string]uint64), } // Test reloading coins from first order @@ -536,12 +538,25 @@ func TestFundOrderReturnCoinsFundingCoins(t *testing.T) { } checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 1") + // Double the available balance + node.bal.Mul(node.bal, big.NewInt(2)) + + // Test funding two coins with the same id + coins, err = eth2.FundingCoins([]dex.Bytes{coins2[0].ID(), coins2[0].ID()}) + if err == nil { + t.Fatalf("expected error but did not get") + } + checkBalance(eth2, walletBalanceGwei*2-coins1[0].Value(), coins1[0].Value(), "after funding error 2") + + // Return to original available balance + node.bal.Div(node.bal, big.NewInt(2)) + // Test funding coins with bad coin ID _, err = eth2.FundingCoins([]dex.Bytes{badCoin.ID()}) if err == nil { t.Fatalf("expected error but did not get") } - checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 2") + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 3") // Test funding coins with coin from different address var differentAddress [20]byte @@ -560,7 +575,7 @@ func TestFundOrderReturnCoinsFundingCoins(t *testing.T) { if err == nil { t.Fatalf("expected error but did not get") } - checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 3") + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 4") // Test funding coins with balance error node.balErr = errors.New("") @@ -569,7 +584,7 @@ func TestFundOrderReturnCoinsFundingCoins(t *testing.T) { t.Fatalf("expected error but did not get") } node.balErr = nil - checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 4") + checkBalance(eth2, walletBalanceGwei-coins1[0].Value(), coins1[0].Value(), "after funding error 5") // Reloading coins from second order coins, err = eth2.FundingCoins([]dex.Bytes{coins2[0].ID()})