-
Notifications
You must be signed in to change notification settings - Fork 1
Remove hook.BeforeBlock and hook.AfterBlock #40
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
Merged
StephenButtolph
merged 26 commits into
main
from
StephenButtolph/fix-gas-target-changes
Dec 11, 2025
+184
−101
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
8b038bf
Move BeforeBlock and AfterBlock into gastime
StephenButtolph af596f7
Replace full blocks with headers in hook.Points
StephenButtolph 178724f
Update gas target immediately after block execution
StephenButtolph 19c3fb9
Add test
StephenButtolph 3b7e488
reduce comments
StephenButtolph 6851d5a
Update hook.Points
StephenButtolph 0d8cb33
Merged
StephenButtolph d8e67f3
merged
StephenButtolph b4450bb
ok
StephenButtolph f34665a
nit
StephenButtolph 0373d0a
lint?
StephenButtolph 94e1fb9
lint
StephenButtolph 5c4a6e1
Update test
StephenButtolph 16badd0
Add genesis block target override
StephenButtolph abe0d6d
merged
StephenButtolph 8a8b9e2
address comments
StephenButtolph 537bc52
re-push execution file
StephenButtolph ad66e07
Default to max gas target
StephenButtolph 2339b16
Merge branch 'main' into StephenButtolph/update-target-sooner
StephenButtolph 73f2f6c
Merge branch 'StephenButtolph/update-target-sooner' into StephenButto…
StephenButtolph cb79047
Merge branch 'main' into StephenButtolph/update-target-sooner
StephenButtolph f206ac4
Merge branch 'StephenButtolph/update-target-sooner' into StephenButto…
StephenButtolph 67171b5
lint
StephenButtolph 6a11f2a
Update gastime/acp176_test.go
StephenButtolph b7f4ce2
align test
StephenButtolph 694b7f3
merged
StephenButtolph File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Copyright (C) 2025, Ava Labs, Inc. All rights reserved. | ||
| // See the file LICENSE for licensing terms. | ||
|
|
||
| package gastime | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/ava-labs/avalanchego/vms/components/gas" | ||
| "github.com/ava-labs/libevm/core/types" | ||
|
|
||
| "github.com/ava-labs/strevm/hook" | ||
| ) | ||
|
|
||
| // BeforeBlock is intended to be called before processing a block, with the | ||
| // timestamp sourced from [hook.Points] and [types.Header]. | ||
| func (tm *Time) BeforeBlock(hooks hook.Points, h *types.Header) { | ||
| tm.FastForwardTo( | ||
| h.Time, | ||
| hooks.SubSecondBlockTime(tm.Rate(), h), | ||
| ) | ||
| } | ||
|
|
||
| // AfterBlock is intended to be called after processing a block, with the target | ||
| // sourced from [hook.Points] and [types.Header]. | ||
| func (tm *Time) AfterBlock(used gas.Gas, hooks hook.Points, h *types.Header) error { | ||
| tm.Tick(used) | ||
| target := hooks.GasTargetAfter(h) | ||
| if err := tm.SetTarget(target); err != nil { | ||
| return fmt.Errorf("%T.SetTarget() after block: %w", tm, err) | ||
| } | ||
| return nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| // Copyright (C) 2025, Ava Labs, Inc. All rights reserved. | ||
| // See the file LICENSE for licensing terms. | ||
|
|
||
| package gastime | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/ava-labs/avalanchego/vms/components/gas" | ||
| "github.com/ava-labs/libevm/core/types" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/ava-labs/strevm/hook/hookstest" | ||
| ) | ||
|
|
||
| // TestTargetUpdateTiming verifies that the gas target is modified in AfterBlock | ||
| // rather than BeforeBlock. | ||
| func TestTargetUpdateTiming(t *testing.T) { | ||
StephenButtolph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const ( | ||
| initialTime = 42 | ||
| initialTarget gas.Gas = 1_600_000 | ||
| initialExcess = 1_234_567_890 | ||
| ) | ||
| tm := New(initialTime, initialTarget, initialExcess) | ||
| initialRate := tm.Rate() | ||
|
|
||
| const ( | ||
| newTime uint64 = initialTime + 1 | ||
| newTarget = initialTarget + 100_000 | ||
| ) | ||
| hook := &hookstest.Stub{ | ||
| Target: newTarget, | ||
| } | ||
| header := &types.Header{ | ||
| Time: newTime, | ||
| } | ||
|
|
||
| initialPrice := tm.Price() | ||
| tm.BeforeBlock(hook, header) | ||
| assert.Equal(t, newTime, tm.Unix(), "Unix time advanced by BeforeBlock()") | ||
| assert.Equal(t, initialTarget, tm.Target(), "Target not changed by BeforeBlock()") | ||
| // While the price technically could remain the same, being more strict | ||
| // ensures the test is meaningful. | ||
| enforcedPrice := tm.Price() | ||
| assert.Less(t, enforcedPrice, initialPrice, "Price should not increase in BeforeBlock()") | ||
| if t.Failed() { | ||
| t.FailNow() | ||
| } | ||
|
|
||
| const ( | ||
| secondsOfGasUsed = 3 | ||
| expectedEndTime = newTime + secondsOfGasUsed | ||
| ) | ||
| used := initialRate * secondsOfGasUsed | ||
| require.NoError(t, tm.AfterBlock(used, hook, header), "AfterBlock()") | ||
| assert.Equal(t, expectedEndTime, tm.Unix(), "Unix time advanced by AfterBlock() due to gas consumption") | ||
| assert.Equal(t, newTarget, tm.Target(), "Target updated by AfterBlock()") | ||
| // While the price technically could remain the same, being more strict | ||
| // ensures the test is meaningful. | ||
| assert.Greater(t, tm.Price(), enforcedPrice, "Price should not decrease in AfterBlock()") | ||
| } | ||
|
|
||
| func FuzzWorstCasePrice(f *testing.F) { | ||
| f.Fuzz(func( | ||
| t *testing.T, | ||
| initTimestamp, initTarget, initExcess, | ||
| time0, timeFrac0, used0, limit0, target0, | ||
StephenButtolph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| time1, timeFrac1, used1, limit1, target1, | ||
| time2, timeFrac2, used2, limit2, target2, | ||
| time3, timeFrac3, used3, limit3, target3 uint64, | ||
| ) { | ||
| initTarget = max(initTarget, 1) | ||
|
|
||
| worstcase := New(initTimestamp, gas.Gas(initTarget), gas.Gas(initExcess)) | ||
| actual := New(initTimestamp, gas.Gas(initTarget), gas.Gas(initExcess)) | ||
|
|
||
| blocks := []struct { | ||
| time uint64 | ||
| timeFrac gas.Gas | ||
| used gas.Gas | ||
| limit gas.Gas | ||
| target gas.Gas | ||
| }{ | ||
| { | ||
| time: time0, | ||
| timeFrac: gas.Gas(timeFrac0), | ||
| used: gas.Gas(used0), | ||
| limit: gas.Gas(limit0), | ||
| target: gas.Gas(target0), | ||
| }, | ||
| { | ||
| time: time1, | ||
| timeFrac: gas.Gas(timeFrac1), | ||
| used: gas.Gas(used1), | ||
| limit: gas.Gas(limit1), | ||
| target: gas.Gas(target1), | ||
| }, | ||
| { | ||
| time: time2, | ||
| timeFrac: gas.Gas(timeFrac2), | ||
| used: gas.Gas(used2), | ||
| limit: gas.Gas(limit2), | ||
| target: gas.Gas(target2), | ||
| }, | ||
| { | ||
| time: time3, | ||
| timeFrac: gas.Gas(timeFrac3), | ||
| used: gas.Gas(used3), | ||
| limit: gas.Gas(limit3), | ||
| target: gas.Gas(target3), | ||
| }, | ||
| } | ||
| for _, block := range blocks { | ||
| block.limit = max(block.used, block.limit) | ||
| block.target = clampTarget(max(block.target, 1)) | ||
| block.timeFrac %= rateOf(block.target) | ||
|
|
||
| header := &types.Header{ | ||
| Time: block.time, | ||
| } | ||
| hook := &hookstest.Stub{ | ||
| Target: block.target, | ||
| SubSecondTime: block.timeFrac, | ||
| } | ||
|
|
||
| worstcase.BeforeBlock(hook, header) | ||
| actual.BeforeBlock(hook, header) | ||
|
|
||
| // The crux of this test lies in the maintaining of this inequality | ||
| // through the use of `limit` instead of `used` in `AfterBlock()` | ||
| require.LessOrEqualf(t, actual.Price(), worstcase.Price(), "actual <= worst-case %T.Price()", actual) | ||
| require.NoError(t, worstcase.AfterBlock(block.limit, hook, header), "worstcase.AfterBlock()") | ||
| require.NoError(t, actual.AfterBlock(block.used, hook, header), "actual.AfterBlock()") | ||
| } | ||
| }) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This precludes the
hookpackage ever importing theblockspackage. Not a problem for now, but flagging it so you're thinking about it. This will be OK if hooks only ever deal with Eth types, but that might not always be the case.