Skip to content
41 changes: 26 additions & 15 deletions client/asset/eth/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func init() {
asset.Register(BipID, &Driver{})
// Test token
registerToken(simnetTokenID, "A token wallet for the DEX test token. Used for testing DEX software.", dex.Simnet)
registerToken(usdcTokenID, "The USDC Ethereum ERC20 token.", dex.Testnet)
registerToken(usdcTokenID, "The USDC Ethereum ERC20 token.", dex.Mainnet, dex.Testnet)
}

const (
Expand Down Expand Up @@ -455,11 +455,11 @@ type baseWallet struct {
// assetWallet satisfies the dex.Wallet interface.
type assetWallet struct {
*baseWallet
assetID uint32
tipChange func(error)
log dex.Logger
atomicUnit string
connected atomic.Bool
assetID uint32
tipChange func(error)
log dex.Logger
ui dex.UnitInfo
connected atomic.Bool

lockedFunds struct {
mtx sync.RWMutex
Expand Down Expand Up @@ -684,11 +684,14 @@ func NewWallet(assetCFG *asset.WalletConfig, logger dex.Logger, net dex.Network)
contractors: make(map[uint32]contractor),
evmify: dexeth.GweiToWei,
atomize: dexeth.WeiToGwei,
atomicUnit: dexeth.UnitInfo.AtomicUnit,
ui: dexeth.UnitInfo,
maxSwapsInTx: perTxGasLimit / maxSwapGas,
maxRedeemsInTx: perTxGasLimit / maxRedeemGas,
}

logger.Infof("ETH wallet will support a maximum of %d swaps and %d redeems per transaction.",
aw.maxSwapsInTx, aw.maxRedeemsInTx)

aw.wallets = map[uint32]*assetWallet{
BipID: aw,
}
Expand Down Expand Up @@ -985,7 +988,7 @@ func (w *ETHWallet) OpenTokenWallet(tokenCfg *asset.TokenConfig) (asset.Wallet,
contractors: make(map[uint32]contractor),
evmify: token.AtomicToEVM,
atomize: token.EVMToAtomic,
atomicUnit: token.UnitInfo.AtomicUnit,
ui: token.UnitInfo,
maxSwapsInTx: perTxGasLimit / maxSwapGas,
maxRedeemsInTx: perTxGasLimit / maxRedeemGas,
}
Expand Down Expand Up @@ -1016,6 +1019,10 @@ func (eth *baseWallet) OwnsDepositAddress(address string) (bool, error) {
return addr == eth.addr, nil
}

func (w *assetWallet) amtString(amt uint64) string {
return fmt.Sprintf("%s %s", w.ui.ConventionalString(amt), w.ui.Conventional.Unit)
}

// fundReserveType represents the various uses for which funds need to be locked:
// initiations, redemptions, and refunds.
type fundReserveType uint32
Expand Down Expand Up @@ -1063,7 +1070,7 @@ func (w *assetWallet) lockFunds(amt uint64, t fundReserveType) error {

if balance.Available < amt {
return fmt.Errorf("attempting to lock more %s for %s than is currently available. %d > %d %s",
dex.BipIDSymbol(w.assetID), t, amt, balance.Available, w.atomicUnit)
dex.BipIDSymbol(w.assetID), t, amt, balance.Available, w.ui.AtomicUnit)
}

w.lockedFunds.mtx.Lock()
Expand Down Expand Up @@ -1387,6 +1394,8 @@ func (w *ETHWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, error
// some work for the caller as well. We can't just always do it that way and
// remove RedeemN, since we can't guarantee that the redemption asset is in
// our fee-family. though it could still be an AccountRedeemer.
w.log.Debugf("Locking %s to swap %s in up to %d swaps at a fee rate of %d gwei/gas using up to %d gas per swap",
w.amtString(ethToLock), w.amtString(ord.Value), ord.MaxSwapCount, ord.MaxFeeRate, g.Swap)

coin := w.createFundingCoin(ethToLock)

Expand Down Expand Up @@ -1428,6 +1437,8 @@ func (w *TokenWallet) FundOrder(ord *asset.Order) (asset.Coins, []dex.Bytes, err
}
}()

w.log.Debugf("Locking %s to swap %s in up to %d swaps at a fee rate of %d gwei/gas using up to %d gas per swap",
w.parent.amtString(ethToLock), w.amtString(ord.Value), ord.MaxSwapCount, ord.MaxFeeRate, g.Swap)
if err := w.parent.lockFunds(ethToLock, initiationReserve); err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -1554,7 +1565,7 @@ func (w *assetWallet) swapGas(n int, ver uint32) (oneSwap, nSwap uint64, approve
}
}
if gasEst > nSwap {
w.log.Warnf("Swap gas estimate %d is greater than the server's configured value %d. Using live estimate + 10%.", gasEst, nSwap)
w.log.Warnf("Swap gas estimate %d is greater than the server's configured value %d. Using live estimate + 10%%.", gasEst, nSwap)
nSwap = gasEst * 11 / 10 // 10% buffer
if n == 1 && nSwap > oneSwap {
oneSwap = nSwap
Expand Down Expand Up @@ -2150,7 +2161,7 @@ func (w *TokenWallet) maybeApproveTokenSwapContract(ver uint32, maxFeeRate, swap
return fmt.Errorf("parent balance %d doesn't cover contract approval (%d) and tx fees (%d)",
ethBal.Available, approveGas*maxFeeRate, swapReserves)
}
tx, err := w.approveToken(unlimitedAllowance, maxFeeRate, ver)
tx, err := w.approveToken(unlimitedAllowance, maxFeeRate, approveGas, ver)
if err != nil {
return fmt.Errorf("token contract approval error (using max fee rate %d): %w", maxFeeRate, err)
}
Expand Down Expand Up @@ -2178,10 +2189,10 @@ func (w *assetWallet) tokenAllowance() (allowance *big.Int, err error) {

// approveToken approves the token swap contract to spend tokens on behalf of
// account handled by the wallet.
func (w *assetWallet) approveToken(amount *big.Int, maxFeeRate uint64, contractVer uint32) (tx *types.Transaction, err error) {
func (w *assetWallet) approveToken(amount *big.Int, maxFeeRate, gasLimit uint64, contractVer uint32) (tx *types.Transaction, err error) {
w.nonceSendMtx.Lock()
defer w.nonceSendMtx.Unlock()
txOpts, err := w.node.txOpts(w.ctx, 0, approveGas, dexeth.GweiToWei(maxFeeRate), nil)
txOpts, err := w.node.txOpts(w.ctx, 0, gasLimit, dexeth.GweiToWei(maxFeeRate), nil)
if err != nil {
return nil, fmt.Errorf("addSignerToOpts error: %w", err)
}
Expand Down Expand Up @@ -2744,7 +2755,7 @@ func (w *TokenWallet) canSend(value uint64, isPreEstimate bool) (uint64, *big.In
}
avail := bal.Available
if avail < value {
return 0, nil, fmt.Errorf("not enough tokens: have %[1]d %[3]s need %[2]d %[3]s", avail, value, w.atomicUnit)
return 0, nil, fmt.Errorf("not enough tokens: have %[1]d %[3]s need %[2]d %[3]s", avail, value, w.ui.AtomicUnit)
}

ethBal, err := w.parent.Balance()
Expand Down Expand Up @@ -3417,7 +3428,7 @@ func (w *TokenWallet) ConfirmRedemption(coinID dex.Bytes, redemption *asset.Rede
}

const (
txConfsNeededToConfirm = 10
txConfsNeededToConfirm = 3
blocksToWaitBeforeCoinNotFound = 10
blocksToWaitBeforeCheckingIfReplaced = 10
)
Expand Down
56 changes: 29 additions & 27 deletions client/asset/eth/eth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ func newTestNode(assetID uint32) *testNode {
approveTx: types.NewTransaction(4, common.Address{0x34}, big.NewInt(1e9), defaultGasFeeLimit, big.NewInt(2e9), nil),
}
if assetID != BipID {
ttc.tContractor.gasEstimates = &tokenGases
c = ttc
}

Expand Down Expand Up @@ -3778,10 +3779,10 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
blockSubmitted: 9,
},
},
bestBlock: 13,
bestBlock: 11, // tx.height + txConfsNeededToConfirm - 2
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 4,
Req: 10,
Confs: txConfsNeededToConfirm - 1,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(3, 200, redeem0Data),
},
baseFee: dexeth.GweiToWei(100),
Expand Down Expand Up @@ -3810,10 +3811,10 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
},
},
expectedMonitoredTxs: map[common.Hash]*monitoredTx{},
bestBlock: 19,
bestBlock: 12, // tx.height + txConfsNeededToConfirm
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 10,
Req: 10,
Confs: txConfsNeededToConfirm,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(3, 200, redeem0Data),
},
receipt: &types.Receipt{
Expand Down Expand Up @@ -3861,7 +3862,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 19,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(4, 123, redeem0Data),
},
redeemTx: toEthTx(4, 123, redeem0Data),
Expand Down Expand Up @@ -3927,14 +3928,14 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 13,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(4, 123, redeem0Data),
},
redeemTx: toEthTx(4, 123, redeem0Data),
baseFee: dexeth.GweiToWei(100),
},
{
name: "not in monitored txs, found by geth, < 10 confirmations",
name: "not in monitored txs, found by geth, < 3 confirmations",
coinID: toEthTxCoinID(3, 200, redeem0Data),
redemption: assetRedemption(secretHashes[0], secrets[0]),
getTxResMap: map[common.Hash]*txData{
Expand All @@ -3951,11 +3952,11 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
},
},
monitoredTxs: map[common.Hash]*monitoredTx{},
bestBlock: 13,
bestBlock: 11,

expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 4,
Req: 10,
Confs: 2,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(3, 200, redeem0Data),
},
baseFee: dexeth.GweiToWei(100),
Expand Down Expand Up @@ -3993,7 +3994,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 13,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(4, 123, redeem0and1Data),
},
expectedResubmittedRedemptions: []*asset.Redemption{
Expand Down Expand Up @@ -4036,7 +4037,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 13,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(4, 123, redeem0Data),
},
expectedResubmittedRedemptions: []*asset.Redemption{
Expand Down Expand Up @@ -4087,7 +4088,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 22,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 2,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(5, 200, redeem0Data),
},
baseFee: dexeth.GweiToWei(100),
Expand All @@ -4109,10 +4110,10 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
State: dexeth.SSRedeemed,
},
},
bestBlock: 13,
bestBlock: 3, // txConfsNeededToConfirm + tx.height + 1
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 10,
Req: 10,
Confs: txConfsNeededToConfirm,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(3, 200, redeem0Data),
},
baseFee: dexeth.GweiToWei(100),
Expand Down Expand Up @@ -4154,7 +4155,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 13,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(4, 123, redeem0Data),
},
expectedResubmittedRedemptions: []*asset.Redemption{
Expand Down Expand Up @@ -4201,7 +4202,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 13,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(4, 123, redeem0Data),
},
expectedResubmittedRedemptions: []*asset.Redemption{
Expand Down Expand Up @@ -4243,7 +4244,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
bestBlock: 13,
expectedResult: &asset.ConfirmRedemptionStatus{
Confs: 0,
Req: 10,
Req: txConfsNeededToConfirm,
CoinID: toEthTxCoinID(3, 200, redeem0Data),
},
expectSentSignedTransaction: toEthTx(3, 200, redeem0Data),
Expand Down Expand Up @@ -4281,6 +4282,7 @@ func testConfirmRedemption(t *testing.T, assetID uint32) {
}

for _, test := range tests {
fmt.Printf("###### %s ###### \n", test.name)
node.getTxResMap = make(map[common.Hash]*tGetTxRes)
for hash, txData := range test.getTxResMap {
node.getTxResMap[hash] = &tGetTxRes{
Expand Down Expand Up @@ -4601,18 +4603,18 @@ func testMaxSwapRedeemLots(t *testing.T, assetID uint32) {

info := wallet.Info()
if assetID == BipID {
if info.MaxSwapsInTx != 37 {
t.Fatalf("expected 37 for max swaps but got %d", info.MaxSwapsInTx)
if info.MaxSwapsInTx != 28 {
t.Fatalf("expected 28 for max swaps but got %d", info.MaxSwapsInTx)
}
if info.MaxRedeemsInTx != 79 {
t.Fatalf("expected 119 for max redemptions but got %d", info.MaxRedeemsInTx)
if info.MaxRedeemsInTx != 63 {
t.Fatalf("expected 63 for max redemptions but got %d", info.MaxRedeemsInTx)
}
} else {
if info.MaxSwapsInTx != 28 {
t.Fatalf("expected 43 for max swaps but got %d", info.MaxSwapsInTx)
t.Fatalf("expected 28 for max swaps but got %d", info.MaxSwapsInTx)
}
if info.MaxRedeemsInTx != 71 {
t.Fatalf("expected 107 for max redemptions but got %d", info.MaxRedeemsInTx)
t.Fatalf("expected 71 for max redemptions but got %d", info.MaxRedeemsInTx)
}
}
}
Expand Down
9 changes: 3 additions & 6 deletions client/asset/eth/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ func prepareNode(cfg *nodeConfig) (*node.Node, error) {
case dex.Testnet:
urls = params.GoerliBootnodes
case dex.Mainnet:
// urls = params.MainnetBootnodes
// TODO: Allow.
return nil, fmt.Errorf("eth cannot be used on mainnet")
urls = params.MainnetBootnodes
default:
return nil, fmt.Errorf("unknown network ID: %d", uint8(cfg.net))
}
Expand Down Expand Up @@ -210,9 +208,8 @@ func ethChainConfig(network dex.Network) (c ethconfig.Config, err error) {
ethCfg.Genesis = core.DefaultGoerliGenesisBlock()
ethCfg.NetworkId = params.GoerliChainConfig.ChainID.Uint64()
case dex.Mainnet:
// urls = params.MainnetBootnodes
// TODO: Allow.
return c, fmt.Errorf("eth cannot be used on mainnet")
ethCfg.Genesis = core.DefaultGenesisBlock()
ethCfg.NetworkId = params.MainnetChainConfig.ChainID.Uint64()
default:
return c, fmt.Errorf("unknown network ID: %d", uint8(network))
}
Expand Down
5 changes: 1 addition & 4 deletions client/asset/eth/nodeclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,7 @@ import (
"github.com/ethereum/go-ethereum/consensus/misc"
)

const (
contractVersionNewest = ^uint32(0)
approveGas = 4e5
)
const contractVersionNewest = ^uint32(0)

var (
// https://github.com/ethereum/go-ethereum/blob/16341e05636fd088aa04a27fca6dc5cda5dbab8f/eth/backend.go#L110-L113
Expand Down
5 changes: 1 addition & 4 deletions client/core/bookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -984,10 +984,7 @@ func handlePriceUpdateNote(c *Core, dc *dexConnection, msg *msgjson.Message) err
}
mktName, err := dex.MarketName(spot.BaseID, spot.QuoteID)
if err != nil {
// It's possible for the dex server to have a market with coin
// ID's we do not know, especially in the case of eth tokens.
c.log.Debugf("error parsing market for base = %d, quote = %d: %v", spot.BaseID, spot.QuoteID, err)
return nil
return nil // it's just an asset we don't support, don't insert, but don't spam
}
dc.spotsMtx.Lock()
dc.spots[mktName] = spot
Expand Down
15 changes: 6 additions & 9 deletions dex/networks/eth/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ var (

ContractAddresses = map[uint32]map[dex.Network]common.Address{
0: {
dex.Mainnet: common.Address{},
dex.Mainnet: common.HexToAddress("0x8C17e4968B6903E1601be82Ca989c5B5E2c7b400"),
dex.Simnet: common.HexToAddress("0x2f68e723b8989ba1c6a9f03e42f33cb7dc9d606f"),
dex.Testnet: common.HexToAddress("0x198463496037754564e9bea5418Bf4117Db0520C"),
},
}
)

var v0Gases = &Gases{
Swap: 135000,
SwapAdd: 113000,
Redeem: 63000,
RedeemAdd: 32000,
Refund: 43000,
Swap: 174500, // 134,500 actual -- https://goerli.etherscan.io/tx/0xa17b6edeaf79791b5fc9232dc05a56d43f3a67845f3248e763b77162fae9b181, verified on mainnet
SwapAdd: 146400, // 112,600 actual (247,100 for 2) -- https://goerli.etherscan.io/tx/0xa4fc65b8001bf8c44f1079b3d97adf42eb1097658e360b9033596253b0cbbd04, verified on mainnet
Redeem: 78600, // 60,456 actual -- https://goerli.etherscan.io/tx/0x5b22c48052df4a8ecd03a31b62e5015e6afe18c9ffb05e6cdd77396dfc3ca917, verified on mainnet
RedeemAdd: 41000, // 31,672 actual (92,083 for 2, 123,724 for 3) -- https://goerli.etherscan.io/tx/0xae424cc9b0d43bf934112245cb74ab9eca9c2611eabcd6257b6ec258b071c1e6, https://goerli.etherscan.io/tx/0x7ba7cb945da108d39a5a0ac580d4841c4017a32cd0e244f26845c6ed501d2475, verified on mainnet
Refund: 57000, // 43,014 actual -- https://goerli.etherscan.io/tx/0x586ed4cb7dab043f98d4cc08930d9eb291b0052d140d949b20232ceb6ad15f25
Comment on lines +55 to +59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running the tests in #2050 with the new contract, on testnet.

  First swap used 134446 gas 
    4 additional swaps averaged 112615 gas each
    [134446 247061 359676 472291 584906] 
  First redeem used 60466 gas 
    4 additional redeems averaged 31629 gas each
    [60466 92095 123712 155353 186982] 
  Average of 5 refunds: 42712 
    [42712 42712 42712 42712 42712] 

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refund discrepancy here is smaller, but still there

https://goerli.etherscan.io/tx/0x586ed4cb7dab043f98d4cc08930d9eb291b0052d140d949b20232ceb6ad15f25
43,014

Copy link
Member Author

@chappjc chappjc Feb 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoeGruffins I'm confused because I don't see your refunds on the swap contract: https://goerli.etherscan.io/address/0x198463496037754564e9bea5418bf4117db0520c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoeGruffins I'm confused because I don't see your refunds on the swap contract: https://goerli.etherscan.io/address/0x198463496037754564e9bea5418bf4117db0520c

I hadn't noticed til now, but it seems the gas estimate app does not refund, it only does estimateRefundGas

}

// LoadGenesisFile loads a Genesis config from a json file.
Expand Down Expand Up @@ -240,9 +240,6 @@ var testTokenID, _ = dex.BipSymbolID("dextt.eth")
var usdcTokenID, _ = dex.BipSymbolID("usdc.eth")

// Gases lists the expected gas required for various DEX and wallet operations.
// ★★ IMPORTANT ★★ By policy, clients should allow servers to adjust Swap and
// Redeem gas values in their *dex.Asset up to but not over 2x the value listed
// in VersionedGases or Tokens.
Comment on lines -243 to -245
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No?

Copy link
Member Author

@chappjc chappjc Feb 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the dex.Asset doesn't even make it into the wallet backends anymore:

f950443
#1866 (comment)

Server can specify fee rates, but not gas caps to use the in transactions. That would be like telling the BTC backend what size in bytes when planning the transaction. The client is in charge of authoring transactions the way they think is right (other than fee rates, payment amounts and recipients, etc.).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually have the size fields deprecated from msgjson.Asset now:

dcrdex/dex/msgjson/types.go

Lines 1068 to 1073 in f950443

// The swap/redeem size fields are DEPRECATED. They are implied by version.
// The values provided by the server in these fields should not be used by
// client wallets, which know the structure of their own transactions.
SwapSize uint64 `json:"swapsize"`
SwapSizeBase uint64 `json:"swapsizebase"`
RedeemSize uint64 `json:"redeemsize"`

type Gases struct {
// Approve is the amount of gas needed to approve the swap contract for
// transferring tokens.
Expand Down
Loading