From 978d392aa4a061a11a3e4e064ec4d1e0da402388 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Sun, 12 Jan 2025 22:24:14 +0800 Subject: [PATCH 1/5] core/txpool/legacypool, ethclient/simulated: ensure pending nonces are reset by subpool.Clear (TODO: add this for blobpool as well) --- core/txpool/legacypool/legacypool.go | 1 + ethclient/simulated/rollback_test.go | 107 +++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 ethclient/simulated/rollback_test.go diff --git a/core/txpool/legacypool/legacypool.go b/core/txpool/legacypool/legacypool.go index 70be7034eea9..fe8fa0cda182 100644 --- a/core/txpool/legacypool/legacypool.go +++ b/core/txpool/legacypool/legacypool.go @@ -1994,6 +1994,7 @@ func (pool *LegacyPool) Clear() { pool.priced = newPricedList(pool.all) pool.pending = make(map[common.Address]*list) pool.queue = make(map[common.Address]*list) + pool.pendingNonces = newNoncer(pool.currentState) if !pool.config.NoLocals && pool.config.Journal != "" { pool.journal = newTxJournal(pool.config.Journal) diff --git a/ethclient/simulated/rollback_test.go b/ethclient/simulated/rollback_test.go new file mode 100644 index 000000000000..018f3723914a --- /dev/null +++ b/ethclient/simulated/rollback_test.go @@ -0,0 +1,107 @@ +package simulated + +import ( + "context" + "testing" + "time" + + "github.com/ethereum/go-ethereum/core/types" +) + +// TestTransactionRollbackBehavior verifies the behavior of transactions +// in the simulated backend after rollback operations. +// +// The test demonstrates that after a rollback: +// 1. The first test shows normal transaction processing without rollback +// 2. The second test shows that transactions immediately after rollback fail +// 3. The third test shows a workaround: committing an empty block after rollback +// makes subsequent transactions succeed +func TestTransactionRollbackBehavior(t *testing.T) { + sim := simTestBackend(testAddr) + defer sim.Close() + client := sim.Client() + + t.Run("Case 1: Basic Transaction (Control Case)", func(t *testing.T) { + // Demonstrates normal transaction processing works as expected + tx := testSendSignedTx(t, sim) + sim.Commit() + assertSuccessfulReceipt(t, client, tx) + }) + + t.Run("Case 2: Transaction After Rollback (Shows Issue)", func(t *testing.T) { + // First transaction gets rolled back + _ = testSendSignedTx(t, sim) + sim.Rollback() + + // Attempting to process a new transaction immediately after rollback + // Currently, this case fails to get a valid receipt + tx := testSendSignedTx(t, sim) + sim.Commit() + assertSuccessfulReceipt(t, client, tx) + }) + + t.Run("Case 3: Transaction After Rollback with Empty Block (Workaround)", func(t *testing.T) { + // First transaction gets rolled back + _ = testSendSignedTx(t, sim) + sim.Rollback() + + // Workaround: Commit an empty block after rollback + sim.Commit() + + // Now the new transaction succeeds + tx := testSendSignedTx(t, sim) + sim.Commit() + assertSuccessfulReceipt(t, client, tx) + }) +} + +// testSendSignedTx sends a signed transaction to the simulated backend. +// It does not commit the block. +func testSendSignedTx(t *testing.T, sim *Backend) *types.Transaction { + t.Helper() + client := sim.Client() + ctx := context.Background() + + signedTx, err := newTx(sim, testKey) + if err != nil { + t.Fatalf("failed to create transaction: %v", err) + } + + if err = client.SendTransaction(ctx, signedTx); err != nil { + t.Fatalf("failed to send transaction: %v", err) + } + + return signedTx +} + +// assertSuccessfulReceipt verifies that a transaction was successfully processed +// by checking its receipt status. +func assertSuccessfulReceipt(t *testing.T, client Client, tx *types.Transaction) { + t.Helper() + ctx := context.Background() + + var ( + receipt *types.Receipt + err error + ) + + // Poll for receipt with timeout + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + receipt, err = client.TransactionReceipt(ctx, tx.Hash()) + if err == nil && receipt != nil { + break + } + time.Sleep(100 * time.Millisecond) + } + + if err != nil { + t.Fatalf("failed to get transaction receipt: %v", err) + } + if receipt == nil { + t.Fatal("transaction receipt is nil") + } + if receipt.Status != types.ReceiptStatusSuccessful { + t.Fatalf("transaction failed with status: %v", receipt.Status) + } +} From 768a29e3a4599579df13fc5a6934fcf3b74e5e45 Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Mon, 13 Jan 2025 17:53:38 +0800 Subject: [PATCH 2/5] expand test case to cover blob pool --- ethclient/simulated/backend_test.go | 44 ++++++++++++++++++++++ ethclient/simulated/rollback_test.go | 55 ++++++++++------------------ 2 files changed, 64 insertions(+), 35 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index 37f4dbbf7b34..b0a26c5ff235 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -19,6 +19,9 @@ package simulated import ( "context" "crypto/ecdsa" + "crypto/sha256" + "github.com/ethereum/go-ethereum/crypto/kzg4844" + "github.com/holiman/uint256" "math/big" "math/rand" "testing" @@ -46,6 +49,46 @@ func simTestBackend(testAddr common.Address) *Backend { ) } +func newBlobTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) { + client := sim.Client() + + testBlob := &kzg4844.Blob{0x00} + testBlobCommit, _ := kzg4844.BlobToCommitment(testBlob) + testBlobProof, _ := kzg4844.ComputeBlobProof(testBlob, testBlobCommit) + testBlobVHash := kzg4844.CalcBlobHashV1(sha256.New(), &testBlobCommit) + + head, _ := client.HeaderByNumber(context.Background(), nil) // Should be child's, good enough + gasPrice := new(big.Int).Add(head.BaseFee, big.NewInt(params.GWei)) + gasPriceU256, _ := uint256.FromBig(gasPrice) + gasTipCapU256, _ := uint256.FromBig(big.NewInt(params.GWei)) + + addr := crypto.PubkeyToAddress(key.PublicKey) + chainid, _ := client.ChainID(context.Background()) + nonce, err := client.PendingNonceAt(context.Background(), addr) + if err != nil { + return nil, err + } + + chainidU256, _ := uint256.FromBig(chainid) + tx := types.NewTx(&types.BlobTx{ + ChainID: chainidU256, + GasTipCap: gasTipCapU256, + GasFeeCap: gasPriceU256, + BlobFeeCap: uint256.NewInt(1), + Gas: 21000, + Nonce: nonce, + To: addr, + AccessList: nil, + BlobHashes: []common.Hash{testBlobVHash}, + Sidecar: &types.BlobTxSidecar{ + Blobs: []kzg4844.Blob{*testBlob}, + Commitments: []kzg4844.Commitment{testBlobCommit}, + Proofs: []kzg4844.Proof{testBlobProof}, + }, + }) + return types.SignTx(tx, types.LatestSignerForChainID(chainid), key) +} + func newTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) { client := sim.Client() @@ -66,6 +109,7 @@ func newTx(sim *Backend, key *ecdsa.PrivateKey) (*types.Transaction, error) { Gas: 21000, To: &addr, }) + return types.SignTx(tx, types.LatestSignerForChainID(chainid), key) } diff --git a/ethclient/simulated/rollback_test.go b/ethclient/simulated/rollback_test.go index 018f3723914a..d8e3ac1bdd36 100644 --- a/ethclient/simulated/rollback_test.go +++ b/ethclient/simulated/rollback_test.go @@ -21,48 +21,33 @@ func TestTransactionRollbackBehavior(t *testing.T) { defer sim.Close() client := sim.Client() - t.Run("Case 1: Basic Transaction (Control Case)", func(t *testing.T) { - // Demonstrates normal transaction processing works as expected - tx := testSendSignedTx(t, sim) - sim.Commit() - assertSuccessfulReceipt(t, client, tx) - }) - - t.Run("Case 2: Transaction After Rollback (Shows Issue)", func(t *testing.T) { - // First transaction gets rolled back - _ = testSendSignedTx(t, sim) - sim.Rollback() - - // Attempting to process a new transaction immediately after rollback - // Currently, this case fails to get a valid receipt - tx := testSendSignedTx(t, sim) - sim.Commit() - assertSuccessfulReceipt(t, client, tx) - }) - - t.Run("Case 3: Transaction After Rollback with Empty Block (Workaround)", func(t *testing.T) { - // First transaction gets rolled back - _ = testSendSignedTx(t, sim) - sim.Rollback() - - // Workaround: Commit an empty block after rollback - sim.Commit() - - // Now the new transaction succeeds - tx := testSendSignedTx(t, sim) - sim.Commit() - assertSuccessfulReceipt(t, client, tx) - }) + // First transaction gets rolled back + _ = testSendSignedTx(t, sim, true) + sim.Rollback() + + // Attempting to process a new transaction immediately after rollback + // Currently, this case fails to get a valid receipt + tx := testSendSignedTx(t, sim, true) + sim.Commit() + assertSuccessfulReceipt(t, client, tx) } // testSendSignedTx sends a signed transaction to the simulated backend. // It does not commit the block. -func testSendSignedTx(t *testing.T, sim *Backend) *types.Transaction { +func testSendSignedTx(t *testing.T, sim *Backend, isBlobTx bool) *types.Transaction { t.Helper() client := sim.Client() ctx := context.Background() - signedTx, err := newTx(sim, testKey) + var ( + err error + signedTx *types.Transaction + ) + if isBlobTx { + signedTx, err = newBlobTx(sim, testKey) + } else { + signedTx, err = newTx(sim, testKey) + } if err != nil { t.Fatalf("failed to create transaction: %v", err) } @@ -74,7 +59,7 @@ func testSendSignedTx(t *testing.T, sim *Backend) *types.Transaction { return signedTx } -// assertSuccessfulReceipt verifies that a transaction was successfully processed +// assertSuccessfulReceipt verifies that a transaction was successfully included as of the pending state // by checking its receipt status. func assertSuccessfulReceipt(t *testing.T, client Client, tx *types.Transaction) { t.Helper() From 1103cb388b5c2646f94c410fbfa222caf00cea4b Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Mon, 13 Jan 2025 21:32:12 +0800 Subject: [PATCH 3/5] include blob txs in the unit test --- ethclient/simulated/backend_test.go | 6 ++- ethclient/simulated/rollback_test.go | 57 +++++++++++++++++----------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index b0a26c5ff235..a837ebe9a179 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -37,8 +37,10 @@ import ( var _ bind.ContractBackend = (Client)(nil) var ( - testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") - testAddr = crypto.PubkeyToAddress(testKey.PublicKey) + testKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + testAddr = crypto.PubkeyToAddress(testKey.PublicKey) + testKey2, _ = crypto.HexToECDSA("7ee346e3f7efc685250053bfbafbfc880d58dc6145247053d4fb3cb0f66dfcb2") + testAddr2 = crypto.PubkeyToAddress(testKey2.PublicKey) ) func simTestBackend(testAddr common.Address) *Backend { diff --git a/ethclient/simulated/rollback_test.go b/ethclient/simulated/rollback_test.go index d8e3ac1bdd36..391caf0ca3c2 100644 --- a/ethclient/simulated/rollback_test.go +++ b/ethclient/simulated/rollback_test.go @@ -2,39 +2,50 @@ package simulated import ( "context" + "crypto/ecdsa" + "math/big" "testing" "time" "github.com/ethereum/go-ethereum/core/types" ) -// TestTransactionRollbackBehavior verifies the behavior of transactions -// in the simulated backend after rollback operations. -// -// The test demonstrates that after a rollback: -// 1. The first test shows normal transaction processing without rollback -// 2. The second test shows that transactions immediately after rollback fail -// 3. The third test shows a workaround: committing an empty block after rollback -// makes subsequent transactions succeed +// TestTransactionRollbackBehavior tests that calling Rollback on the simulated backend doesn't prevent subsequent +// addition of new transactions func TestTransactionRollbackBehavior(t *testing.T) { - sim := simTestBackend(testAddr) + sim := NewBackend( + types.GenesisAlloc{ + testAddr: {Balance: big.NewInt(10000000000000000)}, + testAddr2: {Balance: big.NewInt(10000000000000000)}, + }, + ) defer sim.Close() client := sim.Client() - // First transaction gets rolled back - _ = testSendSignedTx(t, sim, true) + btx0 := testSendSignedTx(t, testKey, sim, true) + tx0 := testSendSignedTx(t, testKey2, sim, false) + tx1 := testSendSignedTx(t, testKey2, sim, false) + sim.Rollback() - // Attempting to process a new transaction immediately after rollback - // Currently, this case fails to get a valid receipt - tx := testSendSignedTx(t, sim, true) + if pendingStateHasTx(client, btx0) || pendingStateHasTx(client, tx0) || pendingStateHasTx(client, tx1) { + t.Fatalf("all transactions were not rolled back") + } + + btx2 := testSendSignedTx(t, testKey, sim, true) + tx2 := testSendSignedTx(t, testKey2, sim, false) + tx3 := testSendSignedTx(t, testKey2, sim, false) + sim.Commit() - assertSuccessfulReceipt(t, client, tx) + + if !pendingStateHasTx(client, btx2) || !pendingStateHasTx(client, tx2) || !pendingStateHasTx(client, tx3) { + t.Fatalf("all post-rollback transactions were not included") + } } // testSendSignedTx sends a signed transaction to the simulated backend. // It does not commit the block. -func testSendSignedTx(t *testing.T, sim *Backend, isBlobTx bool) *types.Transaction { +func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobTx bool) *types.Transaction { t.Helper() client := sim.Client() ctx := context.Background() @@ -44,9 +55,9 @@ func testSendSignedTx(t *testing.T, sim *Backend, isBlobTx bool) *types.Transact signedTx *types.Transaction ) if isBlobTx { - signedTx, err = newBlobTx(sim, testKey) + signedTx, err = newBlobTx(sim, key) } else { - signedTx, err = newTx(sim, testKey) + signedTx, err = newTx(sim, key) } if err != nil { t.Fatalf("failed to create transaction: %v", err) @@ -61,8 +72,7 @@ func testSendSignedTx(t *testing.T, sim *Backend, isBlobTx bool) *types.Transact // assertSuccessfulReceipt verifies that a transaction was successfully included as of the pending state // by checking its receipt status. -func assertSuccessfulReceipt(t *testing.T, client Client, tx *types.Transaction) { - t.Helper() +func pendingStateHasTx(client Client, tx *types.Transaction) bool { ctx := context.Background() var ( @@ -81,12 +91,13 @@ func assertSuccessfulReceipt(t *testing.T, client Client, tx *types.Transaction) } if err != nil { - t.Fatalf("failed to get transaction receipt: %v", err) + return false } if receipt == nil { - t.Fatal("transaction receipt is nil") + return false } if receipt.Status != types.ReceiptStatusSuccessful { - t.Fatalf("transaction failed with status: %v", receipt.Status) + return false } + return true } From d9639a52c016bc04ad3ee45e8ea5973d60e8b12e Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Mon, 13 Jan 2025 21:35:38 +0800 Subject: [PATCH 4/5] fix comment --- ethclient/simulated/rollback_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ethclient/simulated/rollback_test.go b/ethclient/simulated/rollback_test.go index 391caf0ca3c2..8fc9f5bc863e 100644 --- a/ethclient/simulated/rollback_test.go +++ b/ethclient/simulated/rollback_test.go @@ -70,8 +70,7 @@ func testSendSignedTx(t *testing.T, key *ecdsa.PrivateKey, sim *Backend, isBlobT return signedTx } -// assertSuccessfulReceipt verifies that a transaction was successfully included as of the pending state -// by checking its receipt status. +// pendingStateHasTx returns true if a given transaction was successfully included as of the latest pending state. func pendingStateHasTx(client Client, tx *types.Transaction) bool { ctx := context.Background() From 537270007366a9d3fe3b0bb7c075f28702e9cc5d Mon Sep 17 00:00:00 2001 From: Jared Wasinger Date: Mon, 13 Jan 2025 23:25:24 +0800 Subject: [PATCH 5/5] gofmt --- ethclient/simulated/backend_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ethclient/simulated/backend_test.go b/ethclient/simulated/backend_test.go index a837ebe9a179..8efe93e24357 100644 --- a/ethclient/simulated/backend_test.go +++ b/ethclient/simulated/backend_test.go @@ -20,13 +20,14 @@ import ( "context" "crypto/ecdsa" "crypto/sha256" - "github.com/ethereum/go-ethereum/crypto/kzg4844" - "github.com/holiman/uint256" "math/big" "math/rand" "testing" "time" + "github.com/ethereum/go-ethereum/crypto/kzg4844" + "github.com/holiman/uint256" + "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types"