core/txpool/legacypool: Logic race in reorg loop incorrectly gapping transactions
System information
OS & Version: macOS Tahoe 26.6.2
Commit hash : 2dfea58b921acd6bd17771a43871c7392bc23054
Expected behaviour
Assume that acct has tx1 and tx2 in the mempool and are executable.. A third tx3 has also been added, but is still in dirtyAccounts in the reorg loop. Assume the number of the transaction is also its nonce. Block B containing tx1 is executed, and the pool receives a notification to do a chain reorg via the subscription. In this case, tx3 will be incorrectly gapped until it is removed from the mempool, the account's nonce is incremented in state, or a tx4 is issued. I would expect that after running a chain reorg, both tx2 and tx3` would be executable.
Actual behaviour
In runReorg, dirtyAccounts and reset are non-nil in the case above. Because reset is non-nil, pool.reset will reset the noncer to be the state associated with the new head, but it DOES NOT clear the pool.pending mempool. Because of this, the noncer will now report acct's nonce to be 2, even though there is a pending transaction with nonce 2. Next, in pool.queue.promoteExecutables, all addresses from the queue were added, including acct. Since the nonce associated in the noncer is 2, tx3 will still be marked as not executable, and remain in the queue. tx2 is still pending.
Steps to reproduce the behaviour
The unit test is technically racy:
func TestRaceInReorg(t *testing.T) {
pool, _ := setupPool()
defer pool.Close()
key, _ := crypto.GenerateKey()
addr := crypto.PubkeyToAddress(key.PublicKey)
testAddBalance(pool, addr, big.NewInt(1000000000000000))
// acct starts at nonce 1. The reset is needed because the noncer holds its
// own copy of the state, so it has to be rebuilt to see the change.
testSetNonce(pool, addr, 1)
<-pool.requestReset(nil, nil)
// tx1 and tx2 are pending and executable.
for _, tx := range []*types.Transaction{transaction(1, 100000, key), transaction(2, 100000, key)} {
if err := pool.addRemoteSync(tx); err != nil {
t.Fatalf("failed to add setup transaction %d: %v", tx.Nonce(), err)
}
}
if pending, queued := pool.Stats(); pending != 2 || queued != 0 {
t.Fatalf("setup: pending %d (want 2), queued %d (want 0)", pending, queued)
}
// Occupy the reorg loop, so that the two requests below can accumulate
// instead of being launched one at a time.
busy := pool.requestReset(nil, nil)
// tx3 is added to the pool. Its promotion request goes to dirtyAccounts.
pool.Add([]*types.Transaction{transaction(3, 100000, key)}, false)
// Block B, containing tx1, is executed: acct's state nonce becomes 2, and
// the pool is notified. If this reset joins tx3's promotion request in the
// same batch, the follow-up reorg runs with both reset != nil and
// dirtyAccounts != nil, and tx3 is stranded.
testSetNonce(pool, addr, 2)
<-pool.requestReset(nil, nil)
<-busy
// tx1 is gone, tx2 and tx3 should both be pending and executable.
pending, queued := pool.Stats()
if pending != 2 || queued != 0 {
t.Errorf("tx3 stranded in the queue: pending %d (want 2), queued %d (want 0)", pending, queued)
}
// The pool's own invariant check passes either way: the pending nonce is
// correctly 3, it is just never acted upon.
if err := validatePoolInternals(pool); err != nil {
t.Fatalf("pool internals mismatch: %v", err)
}
// A further reset does not rescue tx3 either, since it re-wipes the noncer
// and re-fails the identical readiness check.
<-pool.requestReset(nil, nil)
if pending, queued = pool.Stats(); pending != 2 || queued != 0 {
t.Errorf("tx3 still stranded after a second reset: pending %d (want 2), queued %d (want 0)", pending, queued)
}
}
To guarantee the race, one could add a time.Sleep in runReorg line 1222:
time.Sleep(100 * time.Millisecond)
pool.mu.Lock()
if reset != nil {
Backtrace
If running the UT above with the time.Sleep added (or you are lucky and hit the race):
--- FAIL: TestRaceInReorg (0.61s)
/go/src/github.com/ethereum/go-ethereum/core/txpool/legacypool/legacypool_test.go:629: tx3 stranded in the queue: pending 1 (want 2), queued 1 (want 0)
/go/src/github.com/ethereum/go-ethereum/core/txpool/legacypool/legacypool_test.go:640: tx3 still stranded after a second reset: pending 1 (want 2), queued 1 (want 0)
FAIL
FAIL github.com/ethereum/go-ethereum/core/txpool/legacypool 1.145s
FAIL
Source: ethereum/go-ethereum