Skip to content

Commit

Permalink
client/eth: Tx history recipient bug
Browse files Browse the repository at this point in the history 
The recipient field was being populated with the "to" field of the eth
transaction (which may be a contract address) instead of the recipient
of a send transaction as in other assets.
  • Loading branch information
@martonp
martonp committed Jul 15, 2024
1 parent c75e047 commit 633f79f
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 41 deletions.
67 changes: 33 additions & 34 deletions client/asset/eth/eth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ func (eth *baseWallet) gasFeeLimit() uint64 {

// transactionGenerator is an action that uses a nonce and returns a tx, it's
// type specifier, and its value.
type transactionGenerator func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error)
type transactionGenerator func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error)

// withNonce is called with a function intended to generate a new transaction
// using the next available nonce. If the function returns a non-nil tx, the
Expand Down Expand Up @@ -1110,7 +1110,7 @@ func (w *assetWallet) withNonce(ctx context.Context, f transactionGenerator) (er
w.log.Trace("Nonce chosen for tx generator =", n)

// Make a first attempt with our best-known nonce.
tx, txType, amt, err := f(n)
tx, txType, amt, recipient, err := f(n)
if err != nil && strings.Contains(err.Error(), "nonce too low") {
w.log.Warnf("Too-low nonce detected. Attempting recovery")
confirmedNonceAt, pendingNonceAt, err := w.node.nonce(ctx)
Expand All @@ -1122,7 +1122,7 @@ func (w *assetWallet) withNonce(ctx context.Context, f transactionGenerator) (er
if newNonce := nonce(); newNonce != n {
n = newNonce
// Try again.
tx, txType, amt, err = f(n)
tx, txType, amt, recipient, err = f(n)
if err != nil {
return err
}
Expand All @@ -1133,7 +1133,7 @@ func (w *assetWallet) withNonce(ctx context.Context, f transactionGenerator) (er
}

if tx != nil {
et := w.extendedTx(tx, txType, amt)
et := w.extendedTx(tx, txType, amt, recipient)
w.pendingTxs = append(w.pendingTxs, et)
if n.Cmp(w.pendingNonceAt) >= 0 {
w.pendingNonceAt.Add(n, big.NewInt(1))
Expand Down Expand Up @@ -2530,13 +2530,13 @@ func (w *assetWallet) tokenAllowance(version uint32) (allowance *big.Int, err er
// approveToken approves the token swap contract to spend tokens on behalf of
// account handled by the wallet.
func (w *assetWallet) approveToken(ctx context.Context, amount *big.Int, gasLimit uint64, maxFeeRate, tipRate *big.Int, contractVer uint32) (tx *types.Transaction, err error) {
return tx, w.withNonce(ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error) {
return tx, w.withNonce(ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error) {
txOpts, err := w.node.txOpts(w.ctx, 0, gasLimit, maxFeeRate, tipRate, nonce)
if err != nil {
return nil, 0, 0, fmt.Errorf("addSignerToOpts error: %w", err)
return nil, 0, 0, nil, fmt.Errorf("addSignerToOpts error: %w", err)
}

return tx, asset.ApproveToken, w.atomize(amount), w.withTokenContractor(w.assetID, contractVer, func(c tokenContractor) error {
return tx, asset.ApproveToken, w.atomize(amount), nil, w.withTokenContractor(w.assetID, contractVer, func(c tokenContractor) error {
tx, err = c.approve(txOpts, amount)
if err != nil {
return err
Expand Down Expand Up @@ -4212,7 +4212,7 @@ func (w *ETHWallet) sendToAddr(addr common.Address, amt uint64, maxFeeRate, tipR
// defer w.borkNonce(tx)
// }

return tx, w.withNonce(w.ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error) {
return tx, w.withNonce(w.ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error) {

// Uncomment here and above to test actionTypeMissingNonces.
// if nonceFuturized.CompareAndSwap(false, true) {
Expand All @@ -4229,17 +4229,18 @@ func (w *ETHWallet) sendToAddr(addr common.Address, amt uint64, maxFeeRate, tipR

txOpts, err := w.node.txOpts(w.ctx, amt, defaultSendGasLimit, maxFeeRate, tipRate, nonce)
if err != nil {
return nil, 0, 0, err
return nil, 0, 0, nil, err
}
tx, err = w.node.sendTransaction(w.ctx, txOpts, addr, nil)
if err != nil {
return nil, 0, 0, err
return nil, 0, 0, nil, err
}
txType := asset.Send
if addr == w.addr {
txType = asset.SelfSend
}
return tx, txType, amt, nil
recipient := addr.Hex()
return tx, txType, amt, &recipient, nil
})
}

Expand All @@ -4249,16 +4250,17 @@ func (w *TokenWallet) sendToAddr(addr common.Address, amt uint64, maxFeeRate, ti
if g == nil {
return nil, fmt.Errorf("no gas table")
}
return tx, w.withNonce(w.ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error) {
return tx, w.withNonce(w.ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error) {
txOpts, err := w.node.txOpts(w.ctx, 0, g.Transfer, maxFeeRate, tipRate, nonce)
if err != nil {
return nil, 0, 0, err
return nil, 0, 0, nil, err
}
txType := asset.Send
if addr == w.addr {
txType = asset.SelfSend
}
return tx, txType, amt, w.withTokenContractor(w.assetID, contractVersionNewest, func(c tokenContractor) error {
recipient := addr.Hex()
return tx, txType, amt, &recipient, w.withTokenContractor(w.assetID, contractVersionNewest, func(c tokenContractor) error {
tx, err = c.transfer(txOpts, addr, w.evmify(amt))
if err != nil {
return err
Expand Down Expand Up @@ -4289,13 +4291,13 @@ func (w *assetWallet) initiate(
val += c.Value
}
}
return tx, w.withNonce(ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error) {
return tx, w.withNonce(ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error) {
txOpts, err := w.node.txOpts(ctx, val, gasLimit, maxFeeRate, tipRate, nonce)
if err != nil {
return nil, 0, 0, err
return nil, 0, 0, nil, err
}

return tx, asset.Swap, amt, w.withContractor(contractVer, func(c contractor) error {
return tx, asset.Swap, amt, nil, w.withContractor(contractVer, func(c contractor) error {
tx, err = c.initiate(txOpts, contracts)
return err
})
Expand Down Expand Up @@ -4434,7 +4436,7 @@ func (w *assetWallet) redeem(
// return types.NewTransaction(10, w.addr, big.NewInt(dexeth.GweiFactor), gasLimit, dexeth.GweiToWei(maxFeeRate), nil), nil
// }

return tx, w.withNonce(ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error) {
return tx, w.withNonce(ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error) {
var amt uint64
for _, r := range redemptions {
amt += r.Spends.Coin.Value()
Expand All @@ -4447,9 +4449,9 @@ func (w *assetWallet) redeem(

txOpts, err := w.node.txOpts(ctx, 0, gasLimit, dexeth.GweiToWei(maxFeeRate), tipRate, nonce)
if err != nil {
return nil, 0, 0, err
return nil, 0, 0, nil, err
}
return tx, asset.Redeem, amt, w.withContractor(contractVer, func(c contractor) error {
return tx, asset.Redeem, amt, nil, w.withContractor(contractVer, func(c contractor) error {
tx, err = c.redeem(txOpts, redemptions)
return err
})
Expand All @@ -4464,12 +4466,12 @@ func (w *assetWallet) refund(secretHash [32]byte, amt uint64, maxFeeRate, tipRat
if gas == nil {
return nil, fmt.Errorf("no gas table for asset %d, version %d", w.assetID, contractVer)
}
return tx, w.withNonce(w.ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, error) {
return tx, w.withNonce(w.ctx, func(nonce *big.Int) (*types.Transaction, asset.TransactionType, uint64, *string, error) {
txOpts, err := w.node.txOpts(w.ctx, 0, gas.Refund, maxFeeRate, tipRate, nonce)
if err != nil {
return nil, 0, 0, err
return nil, 0, 0, nil, err
}
return tx, asset.Refund, amt, w.withContractor(contractVer, func(c contractor) error {
return tx, asset.Refund, amt, nil, w.withContractor(contractVer, func(c contractor) error {
tx, err = c.refund(txOpts, secretHash)
return err
})
Expand Down Expand Up @@ -4898,7 +4900,7 @@ func (w *assetWallet) userActionBumpFees(actionB []byte) error {
return fmt.Errorf("error sending bumped-fee transaction: %w", err)
}

newPendingTx := w.extendedTx(newTx, pendingTx.Type, pendingTx.Amount)
newPendingTx := w.extendedTx(newTx, pendingTx.Type, pendingTx.Amount, pendingTx.Recipient)

pendingTx.NonceReplacement = newPendingTx.ID
pendingTx.FeeReplacement = true
Expand Down Expand Up @@ -4971,7 +4973,8 @@ func (w *assetWallet) userActionNonceReplacement(actionB []byte) error {
if replacementTx.Nonce() != pendingTx.Nonce.Uint64() {
return fmt.Errorf("nonce replacement doesn't have the right nonce. %d != %s", replacementTx.Nonce(), pendingTx.Nonce)
}
newPendingTx := w.extendedTx(replacementTx, asset.Unknown, 0)
recipient := w.addr.Hex()
newPendingTx := w.extendedTx(replacementTx, asset.Unknown, 0, &recipient)
pendingTx.NonceReplacement = newPendingTx.ID
var oldTo, newTo common.Address
if oldAddr := oldTx.To(); oldAddr != nil {
Expand Down Expand Up @@ -5039,7 +5042,8 @@ func (w *assetWallet) userActionRecoverNonces(actionB []byte) error {
if skip {
w.log.Warnf("skipping storing underpriced replacement tx for nonce %d", nonce)
} else {
pendingTx := w.extendAndStoreTx(tx, asset.SelfSend, 0, nil)
recipient := w.addr.Hex()
pendingTx := w.extendAndStoreTx(tx, asset.SelfSend, 0, nil, &recipient)
w.emitTransactionNote(pendingTx.WalletTransaction, true)
w.pendingTxs = append(w.pendingTxs, pendingTx)
sort.Slice(w.pendingTxs, func(i, j int) bool {
Expand Down Expand Up @@ -5104,25 +5108,20 @@ func transactionFeeLimit(tx *types.Transaction) *big.Int {

// extendedTx generates an *extendedWalletTx for a newly-broadcasted tx and
// stores it in the DB.
func (w *assetWallet) extendedTx(tx *types.Transaction, txType asset.TransactionType, amt uint64) *extendedWalletTx {
func (w *assetWallet) extendedTx(tx *types.Transaction, txType asset.TransactionType, amt uint64, recipient *string) *extendedWalletTx {
var tokenAssetID *uint32
if w.assetID != w.baseChainID {
tokenAssetID = &w.assetID
}
return w.baseWallet.extendAndStoreTx(tx, txType, amt, tokenAssetID)
return w.baseWallet.extendAndStoreTx(tx, txType, amt, tokenAssetID, recipient)
}

func (w *baseWallet) extendAndStoreTx(tx *types.Transaction, txType asset.TransactionType, amt uint64, tokenAssetID *uint32) *extendedWalletTx {
func (w *baseWallet) extendAndStoreTx(tx *types.Transaction, txType asset.TransactionType, amt uint64, tokenAssetID *uint32, recipient *string) *extendedWalletTx {
nonce := tx.Nonce()
rawTx, err := tx.MarshalBinary()
if err != nil {
w.log.Errorf("Error marshaling tx %q: %v", tx.Hash(), err)
}
var recipient *string
if to := tx.To(); to != nil {
s := to.String()
recipient = &s
}

now := time.Now()

Expand Down
12 changes: 6 additions & 6 deletions client/asset/eth/eth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ func TestCheckPendingTxs(t *testing.T) {

val := dexeth.GweiToWei(1)
extendedTx := func(nonce, blockNum, blockStamp, submissionStamp uint64) *extendedWalletTx {
pendingTx := eth.extendedTx(node.newTransaction(nonce, val), asset.Send, 1)
pendingTx := eth.extendedTx(node.newTransaction(nonce, val), asset.Send, 1, nil)
pendingTx.BlockNumber = blockNum
pendingTx.Confirmed = blockNum > 0 && blockNum <= finalized
pendingTx.Timestamp = blockStamp
Expand Down Expand Up @@ -844,7 +844,7 @@ func TestTakeAction(t *testing.T) {

aGwei := dexeth.GweiToWei(1)

pendingTx := eth.extendedTx(node.newTransaction(0, aGwei), asset.Send, 1)
pendingTx := eth.extendedTx(node.newTransaction(0, aGwei), asset.Send, 1, nil)
eth.pendingTxs = []*extendedWalletTx{pendingTx}

feeCap := new(big.Int).Mul(aGwei, big.NewInt(5))
Expand Down Expand Up @@ -878,7 +878,7 @@ func TestTakeAction(t *testing.T) {
t.Fatal("didn't save to DB")
}

pendingTx = eth.extendedTx(node.newTransaction(1, aGwei), asset.Send, 1)
pendingTx = eth.extendedTx(node.newTransaction(1, aGwei), asset.Send, 1, nil)
eth.pendingTxs = []*extendedWalletTx{pendingTx}
pendingTx.SubmissionTime = 0
// Neglecting to bump should reset submission time.
Expand Down Expand Up @@ -917,15 +917,15 @@ func TestTakeAction(t *testing.T) {
t.Fatalf("replacement tx wasn't accepted")
}
// wrong nonce is an error though
pendingTx = eth.extendedTx(node.newTransaction(5050, aGwei), asset.Send, 1)
pendingTx = eth.extendedTx(node.newTransaction(5050, aGwei), asset.Send, 1, nil)
eth.pendingTxs = []*extendedWalletTx{pendingTx}
lostNonceAction = []byte(fmt.Sprintf(`{"txID":"%s","abandon":false,"replacementID":"%s"}`, pendingTx.ID, replacementTx.Hash()))
if err := eth.TakeAction(actionTypeLostNonce, lostNonceAction); err == nil {
t.Fatalf("no error for wrong nonce")
}

// Missing nonces
tx5 := eth.extendedTx(node.newTransaction(5, aGwei), asset.Send, 1)
tx5 := eth.extendedTx(node.newTransaction(5, aGwei), asset.Send, 1, nil)
eth.pendingTxs = []*extendedWalletTx{tx5}
eth.confirmedNonceAt = big.NewInt(2)
eth.pendingNonceAt = big.NewInt(6)
Expand Down Expand Up @@ -5099,7 +5099,7 @@ func TestSwapOrRedemptionFeesPaid(t *testing.T) {
for _, test := range tests {
var txHash common.Hash
if test.pendingTx != nil {
wt := bw.extendedTx(test.pendingTx, asset.Unknown, 1)
wt := bw.extendedTx(test.pendingTx, asset.Unknown, 1, nil)
wt.BlockNumber = test.pendingTxBlock
wt.Fees = fees
bw.pendingTxs = []*extendedWalletTx{wt}
Expand Down
2 changes: 1 addition & 1 deletion client/asset/eth/txdb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestTxDB(t *testing.T) {
}

newTx := func(nonce uint64) *extendedWalletTx {
return eth.extendedTx(node.newTransaction(nonce, big.NewInt(1)), asset.Send, 1)
return eth.extendedTx(node.newTransaction(nonce, big.NewInt(1)), asset.Send, 1, nil)
}

wt1 := newTx(1)
Expand Down

0 comments on commit 633f79f

Please sign in to comment.