diff --git a/server/asset/eth/coiner.go b/server/asset/eth/coiner.go index 10c21aed85..2ebe0afedf 100644 --- a/server/asset/eth/coiner.go +++ b/server/asset/eth/coiner.go @@ -159,6 +159,8 @@ func (be *AssetBackend) newRedeemCoin(coinID []byte, contractData []byte) (*rede backend: be, locator: locator, contractVer: contractVer, + gasFeeCap: new(big.Int), + gasTipCap: new(big.Int), } return &redeemCoin{ baseCoin: bc, diff --git a/server/asset/eth/coiner_test.go b/server/asset/eth/coiner_test.go index be9888acd5..5b0a0e26fe 100644 --- a/server/asset/eth/coiner_test.go +++ b/server/asset/eth/coiner_test.go @@ -15,6 +15,7 @@ import ( "decred.org/dcrdex/dex/encode" dexeth "decred.org/dcrdex/dex/networks/eth" + "decred.org/dcrdex/server/asset" "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" @@ -39,13 +40,15 @@ func TestNewRedeemCoin(t *testing.T) { const wantGas = 30 const wantGasTipCap = 2 tests := []struct { - name string - ver uint32 - locator []byte - tx *types.Transaction - swpErr, txErr error - swap *dexeth.SwapState - wantErr bool + name string + ver uint32 + backendVer *uint32 // if nil, defaults to ver (backend and contract data agree) + locator []byte + tx *types.Transaction + swpErr, txErr error + swap *dexeth.SwapState + wantErr bool + wantCoinNotFound bool }{ { name: "ok redeem v0", @@ -79,17 +82,50 @@ func TestNewRedeemCoin(t *testing.T) { swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), locator: secretHash[:], }, { - name: "tx not found, not redeemed", + // Regression test: this is the exact scenario of the bug where + // TokenBackend() left contractVer at its zero value. With the + // backend correctly reporting v1 (matching the contract data), + // the fallback must resolve via on-chain swap status. + name: "tx not found, redeemed, v1 backend matches contract version", + ver: 1, txErr: ethereum.NotFound, - swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSInitiated, &participantAddr), - locator: secretHash[:], - wantErr: true, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), + locator: locator, + }, { + name: "tx not found, not redeemed", + txErr: ethereum.NotFound, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSInitiated, &participantAddr), + locator: secretHash[:], + wantErr: true, + wantCoinNotFound: true, }, { name: "tx not found, swap err", txErr: ethereum.NotFound, swpErr: errors.New("swap not found"), locator: secretHash[:], wantErr: true, + // The raw statusAndVector error propagates as-is; it must not + // be conflated with the CoinNotFoundError retry sentinel. + wantCoinNotFound: false, + }, { + // Backend behind the contract data's version (e.g. a v0-only + // backend seeing a v1 redeem). This must be a hard, + // non-retryable error, not CoinNotFoundError. + name: "tx not found, backend behind contract version", + ver: 1, + backendVer: uint32Ptr(0), + txErr: ethereum.NotFound, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), + locator: locator, + wantErr: true, + }, { + // The reverse: backend ahead of an older contract data version. + name: "tx not found, backend ahead of contract version", + backendVer: uint32Ptr(1), + txErr: ethereum.NotFound, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), + locator: secretHash[:], + wantErr: true, }, } for _, test := range tests { @@ -102,6 +138,10 @@ func TestNewRedeemCoin(t *testing.T) { string(test.locator): test.swap, }, } + backendVer := test.ver + if test.backendVer != nil { + backendVer = *test.backendVer + } eth := &AssetBackend{ baseBackend: &baseBackend{ node: node, @@ -111,12 +151,17 @@ func TestNewRedeemCoin(t *testing.T) { assetID: BipID, log: tLogger, atomize: dexeth.WeiToGwei, + contractVer: backendVer, } rc, err := eth.newRedeemCoin(txHash[:], dexeth.EncodeContractData(test.ver, test.locator)) if test.wantErr { if err == nil { t.Fatalf("expected error for test %q", test.name) } + if got := errors.Is(err, asset.CoinNotFoundError); got != test.wantCoinNotFound { + t.Fatalf("test %q: errors.Is(err, CoinNotFoundError) = %v, want %v (err: %v)", + test.name, got, test.wantCoinNotFound, err) + } continue } if err != nil { @@ -130,6 +175,114 @@ func TestNewRedeemCoin(t *testing.T) { dexeth.WeiToGwei(rc.gasTipCap) != wantGasTipCap) { t.Fatalf("returns do not match expected for test %q / %v", test.name, rc) } + + if test.txErr != nil { + // Coins built via the tx-not-found fallback previously had a + // nil gasFeeCap/gasTipCap, which would panic in FeeRate. + if fr := rc.FeeRate(); fr != 0 { + t.Fatalf("test %q: expected zero fee rate for fallback-constructed coin, got %d", test.name, fr) + } + } + } +} + +// TestNewRedeemCoinRelay covers redeem coins built from relay coin IDs +// (gasless redemptions). Unlike the tx-lookup path, relay coins are verified +// entirely via on-chain swap status in relayBaseCoin, which independently +// enforces the same be.contractVer check that newRedeemCoin's tx-not-found +// fallback does. Before the fix to TokenBackend(), that check always failed +// for token backends (contractVer stuck at 0), so every gasless token +// redemption was rejected server-side with a non-retryable error. +func TestNewRedeemCoinRelay(t *testing.T) { + contractAddr := randomAddress() + secret, locator := secretB, locatorB + + tests := []struct { + name string + ver uint32 + backendVer *uint32 // if nil, defaults to ver (backend and contract data agree) + locator []byte + swap *dexeth.SwapState + wantErr bool + wantCoinNotFound bool + }{ + { + name: "relay v1, redeemed", + ver: 1, + locator: locator, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), + }, { + name: "relay v1, still pending", + ver: 1, + locator: locator, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSInitiated, &participantAddr), + wantErr: true, + wantCoinNotFound: true, + }, { + name: "relay v0 unsupported", + locator: secretHashB[:], + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), + wantErr: true, + }, { + // The pre-fix bug scenario: the backend's contractVer (0, as it + // was left before TokenBackend() set it) disagrees with the + // contract data's version (1). Relay coins never go through the + // tx-not-found fallback, so this must surface as a hard, + // non-retryable error straight out of relayBaseCoin. + name: "relay version mismatch (pre-fix bug scenario)", + ver: 1, + backendVer: uint32Ptr(0), + locator: locator, + swap: tSwap(97, initLocktime, 1000, secret, dexeth.SSRedeemed, &participantAddr), + wantErr: true, + }, + } + + for _, test := range tests { + node := &testNode{ + swp: map[string]*dexeth.SwapState{ + string(test.locator): test.swap, + }, + } + backendVer := test.ver + if test.backendVer != nil { + backendVer = *test.backendVer + } + eth := &AssetBackend{ + baseBackend: &baseBackend{ + node: node, + baseLogger: tLogger, + }, + contractAddr: *contractAddr, + assetID: BipID, + log: tLogger, + atomize: dexeth.WeiToGwei, + contractVer: backendVer, + } + + relayCoinID := encode.RandomBytes(int(dexeth.RelayCoinIDLength)) + rc, err := eth.newRedeemCoin(relayCoinID, dexeth.EncodeContractData(test.ver, test.locator)) + if test.wantErr { + if err == nil { + t.Fatalf("expected error for test %q", test.name) + } + if got := errors.Is(err, asset.CoinNotFoundError); got != test.wantCoinNotFound { + t.Fatalf("test %q: errors.Is(err, CoinNotFoundError) = %v, want %v (err: %v)", + test.name, got, test.wantCoinNotFound, err) + } + continue + } + if err != nil { + t.Fatalf("unexpected error for test %q: %v", test.name, err) + } + if rc.secret != secret { + t.Fatalf("test %q: secret mismatch, got %x", test.name, rc.secret) + } + // relayBaseCoin sets zeroed gas fields; confirm FeeRate is safe and + // reports zero rather than panicking on a nil big.Int. + if fr := rc.FeeRate(); fr != 0 { + t.Fatalf("test %q: expected zero fee rate for relay coin, got %d", test.name, fr) + } } } diff --git a/server/asset/eth/eth.go b/server/asset/eth/eth.go index 9c7c9910c9..d390623936 100644 --- a/server/asset/eth/eth.go +++ b/server/asset/eth/eth.go @@ -532,6 +532,7 @@ func (eth *ETHBackend) TokenBackend(assetID uint32, configPath string) (asset.Ba assetID: assetID, blockChans: make(map[chan *asset.BlockUpdate]struct{}), contractAddr: contractAddr, + contractVer: vToken.ContractVersion, atomize: vToken.EVMToAtomic, gases: &swapContract.Gas, }, diff --git a/server/asset/eth/eth_test.go b/server/asset/eth/eth_test.go index 100cb382c5..0d296bc2cc 100644 --- a/server/asset/eth/eth_test.go +++ b/server/asset/eth/eth_test.go @@ -10,6 +10,7 @@ import ( "crypto/sha256" "encoding/hex" "errors" + "fmt" "math/big" "net/url" "os" @@ -143,6 +144,8 @@ func mustArray32(s string) (b [32]byte) { return } +func uint32Ptr(v uint32) *uint32 { return &v } + type testNode struct { connectErr error bestHdr *types.Header @@ -668,13 +671,16 @@ func TestRedemption(t *testing.T) { const gasTipCap = 2 tests := []struct { - name string - ver uint32 - coinID []byte - locator []byte - swp *dexeth.SwapState - tx *types.Transaction - wantErr bool + name string + ver uint32 + backendVer *uint32 // if nil, defaults to ver (backend and contract data agree) + coinID []byte + locator []byte + swp *dexeth.SwapState + tx *types.Transaction + txErr error + wantErr bool + wantCoinNotFound bool }{ { name: "ok v0", @@ -709,29 +715,81 @@ func TestRedemption(t *testing.T) { coinID: txHash[:], swp: tSwap(0, 0, 0, secret, dexeth.SSRedeemed, receiverAddr), wantErr: true, + }, { + // Regression test: tx-not-found fallback for a v1 token-style + // backend (i.e. AssetBackend.contractVer correctly matches the + // contract data version, as TokenBackend() now sets it). Before + // the fix, token backends left contractVer at its zero value, + // so this case (and any v1 token redeem hitting the fallback) + // failed with "wrong contract version" instead of resolving via + // on-chain swap status. + name: "tx not found, redeemed, v1 backend matches contract version", + ver: 1, + txErr: ethereum.NotFound, + locator: locatorA, + coinID: txHash[:], + swp: tSwap(0, 0, 0, secret, dexeth.SSRedeemed, receiverAddr), + }, { + name: "tx not found, swap still pending", + ver: 1, + txErr: ethereum.NotFound, + locator: locatorA, + coinID: txHash[:], + swp: tSwap(0, 0, 0, secret, dexeth.SSInitiated, receiverAddr), + wantErr: true, + wantCoinNotFound: true, + }, { + // Backend still on v0 (e.g. an evm-protocol-overrides.json drain + // scenario) seeing a v1 contract in the redeem data. Must be + // rejected outright, not treated as CoinNotFoundError, since the + // backend can't validate a contract version it doesn't support. + name: "tx not found, backend behind contract version", + ver: 1, + backendVer: uint32Ptr(0), + txErr: ethereum.NotFound, + locator: locatorA, + coinID: txHash[:], + swp: tSwap(0, 0, 0, secret, dexeth.SSRedeemed, receiverAddr), + wantErr: true, }, } for _, test := range tests { eth, node := tNewBackend(BipID) node.tx = test.tx + node.txErr = test.txErr node.receipt = &types.Receipt{ BlockNumber: big.NewInt(5), } node.swp[string(test.locator)] = test.swp eth.contractAddr = *contractAddr + if test.backendVer != nil { + eth.contractVer = *test.backendVer + } else { + eth.contractVer = test.ver + } contract := dexeth.EncodeContractData(test.ver, test.locator) - _, err := eth.Redemption(test.coinID, nil, contract) + coin, err := eth.Redemption(test.coinID, nil, contract) if test.wantErr { if err == nil { t.Fatalf("expected error for test %q", test.name) } + if got := errors.Is(err, asset.CoinNotFoundError); got != test.wantCoinNotFound { + t.Fatalf("test %q: errors.Is(err, CoinNotFoundError) = %v, want %v (err: %v)", + test.name, got, test.wantCoinNotFound, err) + } continue } if err != nil { t.Fatalf("unexpected error for test %q: %v", test.name, err) } + // Coins built via the tx-not-found fallback previously had a nil + // gasFeeCap/gasTipCap, which would panic in FeeRate. Confirm it's + // safe to call. + if fr := coin.FeeRate(); test.txErr != nil && fr != 0 { + t.Fatalf("test %q: expected zero fee rate for fallback-constructed coin, got %d", test.name, fr) + } } } @@ -851,6 +909,47 @@ func testValidateContract(t *testing.T, assetID uint32) { } } +// TestTokenBackendContractVersion is a regression test for a bug where +// TokenBackend() built the token's AssetBackend without setting contractVer, +// leaving it at the zero value regardless of the token's actual contract +// version. That field is read directly (not via the contract data) by the +// tx-not-found redeem fallback and by relay coin construction, so an unset +// contractVer caused every v1 token to fail those paths with a spurious +// "wrong contract version" error. +func TestTokenBackendContractVersion(t *testing.T) { + for _, ver := range []uint32{0, 1} { + t.Run(fmt.Sprintf("contract version %d", ver), func(t *testing.T) { + vToken := &VersionedToken{ + Token: dexeth.Tokens[usdcID], + ContractVersion: ver, + } + eth := ÐBackend{&AssetBackend{ + baseBackend: &baseBackend{ + ctx: tCtx, + net: dex.Simnet, + node: &testNode{}, + baseLogger: tLogger, + tokens: make(map[uint32]*TokenBackend), + versionedTokens: map[uint32]*VersionedToken{usdcID: vToken}, + }, + contractAddrV1: *randomAddress(), + }} + + be, err := eth.TokenBackend(usdcID, "") + if err != nil { + t.Fatalf("TokenBackend error for version %d: %v", ver, err) + } + tb, ok := be.(*TokenBackend) + if !ok { + t.Fatalf("TokenBackend returned unexpected type %T", be) + } + if tb.contractVer != ver { + t.Fatalf("contractVer = %d, want %d", tb.contractVer, ver) + } + }) + } +} + func TestAccountBalance(t *testing.T) { eth, node := tNewBackend(BipID)