From d1fd46993ac4e14acf005c6b551aa23c9c5b1b0f Mon Sep 17 00:00:00 2001 From: Attila Lendvai Date: Sat, 28 Sep 2024 16:24:32 +0200 Subject: [PATCH] fix: eliminate local transaction nonce cache Stop keeping the next nonce in our local state. Instead we always calculate it from the blockchain nonce and the local pending transactions. Fixes issue #4826 where the local nonce cache was out of sync with the blockchain for an unknown reason and because of that no transactions got incorporated into the blockchain. This made unstaking or participation in the reserve incentives impossible. Tests adjusted by @martinconic. --- pkg/transaction/transaction.go | 39 +++++++--------- pkg/transaction/transaction_test.go | 69 ----------------------------- 2 files changed, 16 insertions(+), 92 deletions(-) diff --git a/pkg/transaction/transaction.go b/pkg/transaction/transaction.go index 560136822ac..fc7a4904510 100644 --- a/pkg/transaction/transaction.go +++ b/pkg/transaction/transaction.go @@ -193,11 +193,6 @@ func (t *transactionService) Send(ctx context.Context, request *TxRequest, boost return common.Hash{}, err } - err = t.putNonce(nonce + 1) - if err != nil { - return common.Hash{}, err - } - txHash = signedTx.Hash() err = t.store.Put(storedTransactionKey(txHash), StoredTransaction{ @@ -353,10 +348,6 @@ func (t *transactionService) suggestedFeeAndTip(ctx context.Context, gasPrice *b } -func (t *transactionService) nonceKey() string { - return fmt.Sprintf("%s%x", noncePrefix, t.sender) -} - func storedTransactionKey(txHash common.Hash) string { return fmt.Sprintf("%s%x", storedTransactionPrefix, txHash) } @@ -371,26 +362,28 @@ func (t *transactionService) nextNonce(ctx context.Context) (uint64, error) { return 0, err } - var nonce uint64 - err = t.store.Get(t.nonceKey(), &nonce) + pendingTxs, err := t.PendingTransactions() if err != nil { - // If no nonce was found locally used whatever we get from the backend. - if errors.Is(err, storage.ErrNotFound) { - return onchainNonce, nil - } return 0, err } - // If the nonce onchain is larger than what we have there were external - // transactions and we need to update our nonce. - if onchainNonce > nonce { - return onchainNonce, nil + pendingTxs = t.filterPendingTransactions(t.ctx, pendingTxs) + + // PendingNonceAt returns the nonce we should use, but we will + // compare this to our pending tx list, therefore the -1. + var maxNonce uint64 = onchainNonce - 1 + for _, txHash := range pendingTxs { + trx, _, err := t.backend.TransactionByHash(ctx, txHash) + + if err != nil { + t.logger.Error(err, "pending transaction not found", "tx", txHash) + return 0, err + } + + maxNonce = max(maxNonce, trx.Nonce()) } - return nonce, nil -} -func (t *transactionService) putNonce(nonce uint64) error { - return t.store.Put(t.nonceKey(), nonce) + return maxNonce + 1, nil } // WaitForReceipt waits until either the transaction with the given hash has diff --git a/pkg/transaction/transaction_test.go b/pkg/transaction/transaction_test.go index 41cf94f0112..d105738afbf 100644 --- a/pkg/transaction/transaction_test.go +++ b/pkg/transaction/transaction_test.go @@ -30,10 +30,6 @@ import ( "github.com/ethersphere/bee/v2/pkg/util/testutil" ) -func nonceKey(sender common.Address) string { - return fmt.Sprintf("transaction_nonce_%x", sender) -} - func signerMockForTransaction(t *testing.T, signedTx *types.Transaction, sender common.Address, signerChainID *big.Int) crypto.Signer { t.Helper() return signermock.New( @@ -66,10 +62,6 @@ func signerMockForTransaction(t *testing.T, signedTx *types.Transaction, sender t.Fatalf("signing transaction with wrong gasprice. wanted %d, got %d", signedTx.GasPrice(), transaction.GasPrice()) } - if transaction.Nonce() != signedTx.Nonce() { - t.Fatalf("signing transaction with wrong nonce. wanted %d, got %d", signedTx.Nonce(), transaction.Nonce()) - } - return signedTx, nil }), signermock.WithEthereumAddressFunc(func() (common.Address, error) { @@ -112,10 +104,6 @@ func TestTransactionSend(t *testing.T) { Value: value, } store := storemock.NewStateStore() - err := store.Put(nonceKey(sender), nonce) - if err != nil { - t.Fatal(err) - } transactionService, err := transaction.NewService(logger, sender, backendmock.New( @@ -167,15 +155,6 @@ func TestTransactionSend(t *testing.T) { t.Fatal("returning wrong transaction hash") } - var storedNonce uint64 - err = store.Get(nonceKey(sender), &storedNonce) - if err != nil { - t.Fatal(err) - } - if storedNonce != nonce+1 { - t.Fatalf("nonce not stored correctly: want %d, got %d", nonce+1, storedNonce) - } - storedTransaction, err := transactionService.StoredTransaction(txHash) if err != nil { t.Fatal(err) @@ -238,10 +217,6 @@ func TestTransactionSend(t *testing.T) { MinEstimatedGasLimit: estimatedGasLimit, } store := storemock.NewStateStore() - err := store.Put(nonceKey(sender), nonce) - if err != nil { - t.Fatal(err) - } transactionService, err := transaction.NewService(logger, sender, backendmock.New( @@ -287,15 +262,6 @@ func TestTransactionSend(t *testing.T) { t.Fatal("returning wrong transaction hash") } - var storedNonce uint64 - err = store.Get(nonceKey(sender), &storedNonce) - if err != nil { - t.Fatal(err) - } - if storedNonce != nonce+1 { - t.Fatalf("nonce not stored correctly: want %d, got %d", nonce+1, storedNonce) - } - storedTransaction, err := transactionService.StoredTransaction(txHash) if err != nil { t.Fatal(err) @@ -363,10 +329,6 @@ func TestTransactionSend(t *testing.T) { Value: value, } store := storemock.NewStateStore() - err := store.Put(nonceKey(sender), nonce) - if err != nil { - t.Fatal(err) - } transactionService, err := transaction.NewService(logger, sender, backendmock.New( @@ -418,15 +380,6 @@ func TestTransactionSend(t *testing.T) { t.Fatal("returning wrong transaction hash") } - var storedNonce uint64 - err = store.Get(nonceKey(sender), &storedNonce) - if err != nil { - t.Fatal(err) - } - if storedNonce != nonce+1 { - t.Fatalf("nonce not stored correctly: want %d, got %d", nonce+1, storedNonce) - } - storedTransaction, err := transactionService.StoredTransaction(txHash) if err != nil { t.Fatal(err) @@ -534,15 +487,6 @@ func TestTransactionSend(t *testing.T) { if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { t.Fatal("returning wrong transaction hash") } - - var storedNonce uint64 - err = store.Get(nonceKey(sender), &storedNonce) - if err != nil { - t.Fatal(err) - } - if storedNonce != nonce+1 { - t.Fatalf("did not store nonce correctly. wanted %d, got %d", nonce+1, storedNonce) - } }) t.Run("send_skipped_nonce", func(t *testing.T) { @@ -565,10 +509,6 @@ func TestTransactionSend(t *testing.T) { Value: value, } store := storemock.NewStateStore() - err := store.Put(nonceKey(sender), nonce) - if err != nil { - t.Fatal(err) - } transactionService, err := transaction.NewService(logger, sender, backendmock.New( @@ -614,15 +554,6 @@ func TestTransactionSend(t *testing.T) { if !bytes.Equal(txHash.Bytes(), signedTx.Hash().Bytes()) { t.Fatal("returning wrong transaction hash") } - - var storedNonce uint64 - err = store.Get(nonceKey(sender), &storedNonce) - if err != nil { - t.Fatal(err) - } - if storedNonce != nextNonce+1 { - t.Fatalf("did not store nonce correctly. wanted %d, got %d", nextNonce+1, storedNonce) - } }) }