diff --git a/execution/state/log_arena.go b/execution/state/log_arena.go index 734b0f926e5..62778d6fc06 100644 --- a/execution/state/log_arena.go +++ b/execution/state/log_arena.go @@ -21,7 +21,6 @@ import ( "slices" "github.com/erigontech/erigon/common" - "github.com/erigontech/erigon/common/dbg" "github.com/erigontech/erigon/common/hexutil" "github.com/erigontech/erigon/execution/protocol/params" "github.com/erigontech/erigon/execution/types" @@ -37,8 +36,8 @@ const ( maxLogBytesPerTxn = int(params.MaxTxnGasLimit / params.LogDataGas) // 2MB // Fractions of those, because the pool never shrinks: the whole ceiling would - // park 13MB of entries per arena, while a tenth still holds the p99 block of - // 1706 logs that a caller resetting per block pools in one go. + // park 13MB of entries per arena, while a tenth still holds the ~1700 logs of + // a p99 block that a caller resetting per block pools in one go. maxPooledLogEntries = maxLogsPerTxn / 10 maxPooledLogBytes = maxLogBytesPerTxn / 2 @@ -49,7 +48,7 @@ const ( // Slots the arena keeps between resets. The run is a block for a caller that // resets per block, so this is bounded by the memory it costs rather than by - // a transaction's budget: 32KB of pointers, holding the p99 block of 1706. + // a transaction's budget: 32KB of pointers, holding a p99 block's ~1700. maxRetainedLogSlots = 32 * 1024 / 8 ) @@ -150,14 +149,15 @@ func (a *logArena) revertLast(txIndex int) { } // forTx returns the entries txIndex emitted, owned by the arena. They are held -// in one run rather than grouped, and a transaction's own are the tail of it, -// so any other transaction reads as empty. +// in one run rather than grouped, and a transaction's own are the tail of it, so +// a transaction newer than the tail reads as empty and an older one panics. func (a *logArena) forTx(txIndex int) types.Logs { entries := a.entries i := len(entries) - if dbg.AssertEnabled && i > 0 && txIndex < int(entries[i-1].TxIndex) { + if i > 0 && txIndex < int(entries[i-1].TxIndex) { // Newer than the tail is a transaction that emitted nothing, which is - // how most of them end. Older is a caller reading what it has left. + // how most of them end. Older is a caller reading what it has left: + // answering empty would read as "emitted no logs", so say so instead. panic(fmt.Sprintf("logs of tx %d asked for, the run has reached tx %d", txIndex, int(entries[i-1].TxIndex))) } diff --git a/execution/state/log_arena_test.go b/execution/state/log_arena_test.go index 24cdbf2b501..52d3cbc179d 100644 --- a/execution/state/log_arena_test.go +++ b/execution/state/log_arena_test.go @@ -655,12 +655,11 @@ func TestLogIndexIsBlockWide(t *testing.T) { require.Equal(t, hexutil.Uint(0), ibs.GetRawLogs(0)[0].Index, "next block restarts at zero") } -// Reading a transaction the run has moved past answers empty, which a receipt -// records as "emitted no logs". A transaction that simply emitted nothing is -// newer than the tail, not older, so it stays legal. -func TestGetLogsOfPastTxAsserts(t *testing.T) { - defer func(prev bool) { dbg.AssertEnabled = prev }(dbg.AssertEnabled) - dbg.AssertEnabled = true +// Reading a transaction the run has moved past would answer empty, which a +// receipt records as "emitted no logs" - so it panics instead. A transaction that +// simply emitted nothing is newer than the tail, not older, so it stays legal. +func TestGetLogsOfPastTxPanics(t *testing.T) { + t.Parallel() ibs := New(nil) ibs.SetTxContext(1, 0) @@ -673,6 +672,20 @@ func TestGetLogsOfPastTxAsserts(t *testing.T) { require.Panics(t, func() { ibs.GetRawLogs(0) }, "the run has moved past tx 0") } +// The past-transaction check guards production, not just assert builds: a caller +// that reads what the run has left must not be handed the tail's logs as if they +// were its own. Not parallel - it writes the global assert flag. +func TestGetLogsOfPastTxPanicsWithoutAsserts(t *testing.T) { + defer func(prev bool) { dbg.AssertEnabled = prev }(dbg.AssertEnabled) + dbg.AssertEnabled = false + + ibs := New(nil) + ibs.SetTxContext(1, 3) + ibs.AddLog(&types.Log{Address: common.HexToAddress("0x1")}) + + require.Panics(t, func() { ibs.GetRawLogs(2) }, "tx 2 is older than the tail") +} + // Whatever the traffic looks like, one arena holds a bounded amount: the pool, // and the one array the run left behind. This is the invariant a shape-specific // leak breaks — logs parked per tx index, an array kept because one block was