Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions hook/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,39 +15,46 @@ import (
"github.com/ava-labs/libevm/core/types"
"github.com/ava-labs/libevm/params"

"github.com/ava-labs/strevm/blocks"
"github.com/ava-labs/strevm/gastime"
"github.com/ava-labs/strevm/intmath"
saeparams "github.com/ava-labs/strevm/params"
)

// Points define user-injected hook points.
type Points interface {
GasTarget(parent *types.Block) gas.Gas
SubSecondBlockTime(*types.Block) gas.Gas
// GasTarget returns the amount of gas per second that the chain should
// target to consume after executing the provided block.
GasTarget(*types.Header) gas.Gas
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// GasTarget returns the amount of gas per second that the chain should
// target to consume after executing the provided block.
GasTarget(*types.Header) gas.Gas
// GasTargetAfter returns the amount of gas per second that the chain should
// target to consume after executing the provided block.
GasTargetAfter(*types.Header) gas.Gas

Also (nit), something about the wording of "to consume after executing the provided block" feels off and it confused me for a split second. Perhaps "target to consume in subsequent blocks."? The current phrasing suggests immediacy, not long-term horizon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reworded it, lmk what you think

// SubSecondBlockTime returns the sub-second portion of the block time based
// on the provided gas rate.
//
// For example, if the block timestamp is 10.75 seconds and the gas rate is
// 100 gas/second, then this method should return 75 gas.
SubSecondBlockTime(*types.Header) gas.Gas
// BeforeBlock is called immediately prior to executing the block.
BeforeBlock(params.Rules, *state.StateDB, *types.Block) error
// AfterBlock is called immediately after executing the block.
AfterBlock(*state.StateDB, *types.Block, types.Receipts)
}

// BeforeBlock is intended to be called before processing a block, with the gas
// target sourced from [Points].
func BeforeBlock(pts Points, rules params.Rules, sdb *state.StateDB, b *blocks.Block, clock *gastime.Time) error {
// BeforeBlock is intended to be called before processing a block.
func BeforeBlock(pts Points, rules params.Rules, sdb *state.StateDB, b *types.Block, clock *gastime.Time) error {
clock.FastForwardTo(
b.BuildTime(),
pts.SubSecondBlockTime(b.EthBlock()),
b.Time(),
pts.SubSecondBlockTime(b.Header()),
)
target := pts.GasTarget(b.ParentBlock().EthBlock())
if err := clock.SetTarget(target); err != nil {
return fmt.Errorf("%T.SetTarget() before block: %w", clock, err)
}
return pts.BeforeBlock(rules, sdb, b.EthBlock())
return pts.BeforeBlock(rules, sdb, b)
}

// AfterBlock is intended to be called after processing a block, with the gas
// sourced from [types.Block.GasUsed] or equivalent.
func AfterBlock(pts Points, sdb *state.StateDB, b *types.Block, clock *gastime.Time, used gas.Gas, rs types.Receipts) {
// AfterBlock is intended to be called after processing a block.
func AfterBlock(pts Points, sdb *state.StateDB, b *types.Block, clock *gastime.Time, used gas.Gas, rs types.Receipts) error {
clock.Tick(used)
target := pts.GasTarget(b.Header())
if err := clock.SetTarget(target); err != nil {
return fmt.Errorf("%T.SetTarget() after block: %w", clock, err)
}
pts.AfterBlock(sdb, b, rs)
return nil
}

// MinimumGasConsumption MUST be used as the implementation for the respective
Expand Down
64 changes: 64 additions & 0 deletions hook/hook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright (C) 2025, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package hook_test

import (
"testing"

"github.com/ava-labs/avalanchego/vms/components/gas"
"github.com/ava-labs/libevm/core/types"
"github.com/ava-labs/libevm/params"
"github.com/ava-labs/strevm/gastime"
"github.com/ava-labs/strevm/hook/hookstest"
"github.com/ava-labs/strevm/saetest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

. "github.com/ava-labs/strevm/hook"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) I'll reconfigure the linter later, but please put this in its own group, at the end. It fundamentally changes how the file should be interpreted, so I think it should be made more obvious to a reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed the linter. localmodule has a higher priority than dot, so it was overriding the dot handling for the local module. I replaced it with what we do in avalanchego.

)

// TestTargetUpdateTiming verifies that the gas target is modified in AfterBlock
// rather than BeforeBlock.
func TestTargetUpdateTiming(t *testing.T) {
const (
initialTime = 42
initialTarget gas.Gas = 1_600_000
initialExcess = 1_234_567_890
)
tm := gastime.New(initialTime, initialTarget, initialExcess)

const (
newTime uint64 = initialTime + 1
newTarget = initialTarget + 100_000
)
hook := &hookstest.Stub{
Target: newTarget,
}
header := &types.Header{
Time: newTime,
}
block := types.NewBlock(header, nil, nil, nil, saetest.TrieHasher())

initialPrice := tm.Price()
require.NoError(t, BeforeBlock(hook, params.TestRules, nil, block, tm), "BeforeBlock()")
assert.Equal(t, newTime, tm.Unix(), "Unix time advanced by BeforeBlock()")
assert.Equal(t, initialTarget, tm.Target(), "Target not changed by BeforeBlock()")

enforcedPrice := tm.Price()
assert.LessOrEqual(t, enforcedPrice, initialPrice, "Price should not increase in BeforeBlock()")
if t.Failed() {
t.FailNow()
}

const (
secondsOfGasUsed = 3
initialRate = initialTarget * gastime.TargetToRate
used gas.Gas = initialRate * secondsOfGasUsed
expectedEndTime = newTime + secondsOfGasUsed
)
require.NoError(t, AfterBlock(hook, nil, block, tm, used, nil), "AfterBlock()")
assert.Equal(t, expectedEndTime, tm.Unix(), "Unix time advanced by AfterBlock()")
assert.Equal(t, newTarget, tm.Target(), "Target updated by AfterBlock()")
assert.GreaterOrEqual(t, tm.Price(), enforcedPrice, "Price should not decrease in AfterBlock()")
}
4 changes: 2 additions & 2 deletions hook/hookstest/stub.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ type Stub struct {
var _ hook.Points = (*Stub)(nil)

// GasTarget ignores its argument and always returns [Stub.Target].
func (s *Stub) GasTarget(parent *types.Block) gas.Gas {
func (s *Stub) GasTarget(*types.Header) gas.Gas {
return s.Target
}

// SubSecondBlockTime time ignores its argument and always returns 0.
func (*Stub) SubSecondBlockTime(*types.Block) gas.Gas {
func (*Stub) SubSecondBlockTime(*types.Header) gas.Gas {
return 0
}

Expand Down
7 changes: 3 additions & 4 deletions saexec/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (e *Executor) execute(b *blocks.Block, logger logging.Logger) error {

rules := e.chainConfig.Rules(b.Number(), true /*isMerge*/, b.BuildTime())
gasClock := parent.ExecutedByGasTime().Clone()
if err := hook.BeforeBlock(e.hooks, rules, stateDB, b, gasClock); err != nil {
if err := hook.BeforeBlock(e.hooks, rules, stateDB, b.EthBlock(), gasClock); err != nil {
return fmt.Errorf("before-block hook: %v", err)
}
perTxClock := gasClock.Time.Clone()
Expand Down Expand Up @@ -162,9 +162,8 @@ func (e *Executor) execute(b *blocks.Block, logger logging.Logger) error {
receipts[ti] = receipt
}
endTime := time.Now()
hook.AfterBlock(e.hooks, stateDB, b.EthBlock(), gasClock, blockGasConsumed, receipts)
if gasClock.Time.Compare(perTxClock) != 0 {
return fmt.Errorf("broken invariant: block-resolution clock @ %s does not match tx-resolution clock @ %s", gasClock.String(), perTxClock.String())
if err := hook.AfterBlock(e.hooks, stateDB, b.EthBlock(), gasClock, blockGasConsumed, receipts); err != nil {
return fmt.Errorf("after-block hook: %v", err)
}

logger.Debug(
Expand Down