diff --git a/.golangci.yml b/.golangci.yml index 70ea23fa..480f6ae1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -48,7 +48,7 @@ linters-settings: sections: - standard - default - - localmodule + - prefix(github.com/ava-labs/strevm) # The rest of these break developer expections, in increasing order of # divergence, so are at the end to increase the chance of being seen. - dot diff --git a/blocks/blockstest/blocks.go b/blocks/blockstest/blocks.go index 990db990..54c8fd4f 100644 --- a/blocks/blockstest/blocks.go +++ b/blocks/blockstest/blocks.go @@ -8,12 +8,14 @@ package blockstest import ( + "math" "math/big" "slices" "testing" "time" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/vms/components/gas" "github.com/ava-labs/libevm/core" "github.com/ava-labs/libevm/core/state" "github.com/ava-labs/libevm/core/types" @@ -102,7 +104,10 @@ func WithLogger(l logging.Logger) BlockOption { // marked as both executed and synchronous. func NewGenesis(tb testing.TB, db ethdb.Database, config *params.ChainConfig, alloc types.GenesisAlloc, opts ...GenesisOption) *blocks.Block { tb.Helper() - conf := options.ApplyTo(&genesisConfig{}, opts...) + conf := &genesisConfig{ + target: math.MaxUint64, + } + options.ApplyTo(conf, opts...) gen := &core.Genesis{ Config: config, @@ -115,13 +120,18 @@ func NewGenesis(tb testing.TB, db ethdb.Database, config *params.ChainConfig, al require.NoErrorf(tb, tdb.Commit(hash, true), "%T.Commit(core.SetupGenesisBlock(...))", tdb) b := NewBlock(tb, gen.ToBlock(), nil, nil) - require.NoErrorf(tb, b.MarkExecuted(db, gastime.New(gen.Timestamp, 1, 0), time.Time{}, new(big.Int), nil, b.SettledStateRoot()), "%T.MarkExecuted()", b) + require.NoErrorf(tb, b.MarkExecuted(db, gastime.New(gen.Timestamp, conf.gasTarget(), 0), time.Time{}, new(big.Int), nil, b.SettledStateRoot()), "%T.MarkExecuted()", b) require.NoErrorf(tb, b.MarkSynchronous(), "%T.MarkSynchronous()", b) return b } type genesisConfig struct { tdbConfig *triedb.Config + target gas.Gas +} + +func (gc *genesisConfig) gasTarget() gas.Gas { + return gc.target } // A GenesisOption configures [NewGenesis]. @@ -133,3 +143,10 @@ func WithTrieDBConfig(tc *triedb.Config) GenesisOption { gc.tdbConfig = tc }) } + +// WithGasTarget overrides the gas target used by [NewGenesis]. +func WithGasTarget(target gas.Gas) GenesisOption { + return options.Func[genesisConfig](func(gc *genesisConfig) { + gc.target = target + }) +} diff --git a/hook/hook.go b/hook/hook.go index 3db23486..fb913f17 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -15,7 +15,6 @@ 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" @@ -23,38 +22,39 @@ import ( // Points define user-injected hook points. type Points interface { - GasTarget(parent *types.Header) gas.Gas + // GasTargetAfter returns the gas target that should go into effect + // immediately after the provided block. + GasTargetAfter(*types.Header) gas.Gas // SubSecondBlockTime returns the sub-second portion of the block time based - // on the gas rate. + // 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 + SubSecondBlockTime(gasRate gas.Gas, h *types.Header) gas.Gas // BeforeExecutingBlock is called immediately prior to executing the block. BeforeExecutingBlock(params.Rules, *state.StateDB, *types.Block) error // AfterExecutingBlock is called immediately after executing the block. AfterExecutingBlock(*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.Header()), + b.Time(), + pts.SubSecondBlockTime(clock.Rate(), b.Header()), ) - target := pts.GasTarget(b.ParentBlock().Header()) - if err := clock.SetTarget(target); err != nil { - return fmt.Errorf("%T.SetTarget() before block: %w", clock, err) - } - return pts.BeforeExecutingBlock(rules, sdb, b.EthBlock()) + return pts.BeforeExecutingBlock(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.GasTargetAfter(b.Header()) + if err := clock.SetTarget(target); err != nil { + return fmt.Errorf("%T.SetTarget() after block: %w", clock, err) + } pts.AfterExecutingBlock(sdb, b, rs) + return nil } // MinimumGasConsumption MUST be used as the implementation for the respective diff --git a/hook/hook_test.go b/hook/hook_test.go new file mode 100644 index 00000000..406ce806 --- /dev/null +++ b/hook/hook_test.go @@ -0,0 +1,68 @@ +// 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/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/strevm/gastime" + "github.com/ava-labs/strevm/hook/hookstest" + "github.com/ava-labs/strevm/saetest" + + . "github.com/ava-labs/strevm/hook" +) + +// 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) + initialRate := tm.Rate() + + 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()") + // 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, AfterBlock(hook, nil, block, tm, used, nil), "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()") +} diff --git a/hook/hookstest/stub.go b/hook/hookstest/stub.go index 296830b0..5ca4b68c 100644 --- a/hook/hookstest/stub.go +++ b/hook/hookstest/stub.go @@ -20,13 +20,13 @@ type Stub struct { var _ hook.Points = (*Stub)(nil) -// GasTarget ignores its argument and always returns [Stub.Target]. -func (s *Stub) GasTarget(*types.Header) gas.Gas { +// GasTargetAfter ignores its argument and always returns [Stub.Target]. +func (s *Stub) GasTargetAfter(*types.Header) gas.Gas { return s.Target } -// SubSecondBlockTime time ignores its argument and always returns 0. -func (*Stub) SubSecondBlockTime(*types.Header) gas.Gas { +// SubSecondBlockTime time ignores its arguments and always returns 0. +func (*Stub) SubSecondBlockTime(gas.Gas, *types.Header) gas.Gas { return 0 } diff --git a/saexec/execution.go b/saexec/execution.go index 1fba3507..9af858c5 100644 --- a/saexec/execution.go +++ b/saexec/execution.go @@ -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() @@ -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( diff --git a/saexec/saexec_test.go b/saexec/saexec_test.go index 3e45a128..d215989f 100644 --- a/saexec/saexec_test.go +++ b/saexec/saexec_test.go @@ -363,9 +363,9 @@ func TestGasAccounting(t *testing.T) { // Steps are _not_ independent, so the execution time of one is the starting // time of the next. steps := []struct { - target gas.Gas blockTime uint64 numTxs int + targetAfter gas.Gas wantExecutedBy *proxytime.Time[gas.Gas] // Because of the 2:1 ratio between Rate and Target, gas consumption // increases excess by half of the amount consumed, while @@ -374,74 +374,85 @@ func TestGasAccounting(t *testing.T) { wantPriceAfter gas.Price }{ { - target: 5 * gasPerTx, + // Initially set the gasTarget for the next block. + blockTime: 0, + numTxs: 0, + targetAfter: 5 * gasPerTx, + wantExecutedBy: at(0, 0, 10*gasPerTx), + wantExcessAfter: 0, + wantPriceAfter: 1, + }, + { blockTime: 2, numTxs: 3, + targetAfter: 5 * gasPerTx, wantExecutedBy: at(2, 3, 10*gasPerTx), wantExcessAfter: 3 * gasPerTx / 2, wantPriceAfter: 1, // Excess isn't high enough so price is effectively e^0 }, { - target: 5 * gasPerTx, blockTime: 3, // fast-forward numTxs: 12, + targetAfter: 5 * gasPerTx, wantExecutedBy: at(4, 2, 10*gasPerTx), wantExcessAfter: 12 * gasPerTx / 2, wantPriceAfter: 1, }, { - target: 5 * gasPerTx, blockTime: 4, // no fast-forward so starts at last execution time numTxs: 20, + targetAfter: 5 * gasPerTx, wantExecutedBy: at(6, 2, 10*gasPerTx), wantExcessAfter: (12 + 20) * gasPerTx / 2, wantPriceAfter: 1, }, { - target: 5 * gasPerTx, - blockTime: 7, // fast-forward equivalent of 8 txs - numTxs: 16, - wantExecutedBy: at(8, 6, 10*gasPerTx), - wantExcessAfter: (12 + 20 - 8 + 16) * gasPerTx / 2, + blockTime: 7, // fast-forward equivalent of 8 txs + numTxs: 16, + targetAfter: 10 * gasPerTx, // double gas/block --> halve ticking rate + // Doubling the target scales both the ending time and excess to compensate. + wantExecutedBy: at(8, 2*6, 2*10*gasPerTx), + wantExcessAfter: 2 * (12 + 20 - 8 + 16) * gasPerTx / 2, wantPriceAfter: 1, }, { - target: 10 * gasPerTx, // double gas/block --> halve ticking rate - blockTime: 8, // no fast-forward - numTxs: 4, - wantExecutedBy: at(8, (6*2)+4, 20*gasPerTx), // starting point scales - wantExcessAfter: (2*(12+20-8+16) + 4) * gasPerTx / 2, + blockTime: 8, // no fast-forward + numTxs: 4, + targetAfter: 5 * gasPerTx, // back to original + // Halving the target inverts the scaling seen in the last block. + wantExecutedBy: at(8, 6+(4/2), 10*gasPerTx), + wantExcessAfter: ((12 + 20 - 8 + 16) + 4/2) * gasPerTx / 2, wantPriceAfter: 1, }, { - target: 5 * gasPerTx, // back to original blockTime: 8, numTxs: 5, + targetAfter: 5 * gasPerTx, wantExecutedBy: at(8, 6+(4/2)+5, 10*gasPerTx), wantExcessAfter: ((12 + 20 - 8 + 16) + 4/2 + 5) * gasPerTx / 2, wantPriceAfter: 1, }, { - target: 5 * gasPerTx, blockTime: 20, // more than double the last executed-by time, reduces excess to 0 numTxs: 1, + targetAfter: 5 * gasPerTx, wantExecutedBy: at(20, 1, 10*gasPerTx), wantExcessAfter: gasPerTx / 2, wantPriceAfter: 1, }, { - target: 5 * gasPerTx, blockTime: 21, // fast-forward so excess is 0 numTxs: 30 * gastime.TargetToExcessScaling, // deliberate, see below + targetAfter: 5 * gasPerTx, wantExecutedBy: at(21, 30*gastime.TargetToExcessScaling, 10*gasPerTx), wantExcessAfter: 3 * ((5 * gasPerTx /*T*/) * gastime.TargetToExcessScaling /* == K */), // Excess is now 3·K so the price is e^3 wantPriceAfter: gas.Price(math.Floor(math.Pow(math.E, 3 /* <----- NB */))), }, { - target: 5 * gasPerTx, blockTime: 22, // no fast-forward numTxs: 10 * gastime.TargetToExcessScaling, + targetAfter: 5 * gasPerTx, wantExecutedBy: at(21, 40*gastime.TargetToExcessScaling, 10*gasPerTx), wantExcessAfter: 4 * ((5 * gasPerTx /*T*/) * gastime.TargetToExcessScaling /* == K */), wantPriceAfter: gas.Price(math.Floor(math.Pow(math.E, 4 /* <----- NB */))), @@ -451,7 +462,7 @@ func TestGasAccounting(t *testing.T) { e, chain, wallet := sut.Executor, sut.chain, sut.wallet for i, step := range steps { - hooks.Target = step.target + hooks.Target = step.targetAfter txs := make(types.Transactions, step.numTxs) for i := range txs {