-
Notifications
You must be signed in to change notification settings - Fork 1.5k
execution/state: make the past-transaction log check unconditional #23087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1c03eb7
0cfc2ff
38ab6af
7468ce6
aad6b39
efec152
12f0b82
10d8e1f
7e435e1
883e075
0c45cc3
ab04465
90e6d66
a242baa
b46e1dc
855f951
85a402e
d75f691
972c98e
222237c
d38ccc6
eb6d857
0d2ba2d
576e57a
2d3a5b1
667e565
a9c7709
182d5f6
5a31cd0
3b80eca
23fe3d3
178c9c6
e35c914
57896d6
9d57200
4236270
ffe2866
63b85dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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. | ||||||||||||||||
|
Comment on lines
+675
to
+677
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ungated
Suggested change
|
||||||||||||||||
| func TestGetLogsOfPastTxPanicsWithoutAsserts(t *testing.T) { | ||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: with |
||||||||||||||||
| 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 | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forTxcompares only against the current run: afterreset()a past-tx read answers empty again, and once the next block logs at the same index a stale reader gets that block's logs back — no panic in either case. Worth scoping the promise: