diff --git a/vms/saevm/blocks/BUILD.bazel b/vms/saevm/blocks/BUILD.bazel index 13ec4d2d4670..498304c83d86 100644 --- a/vms/saevm/blocks/BUILD.bazel +++ b/vms/saevm/blocks/BUILD.bazel @@ -13,7 +13,6 @@ go_library( "invariants.go", "settlement.go", "snow.go", - "time.go", ], importpath = "github.com/ava-labs/avalanchego/vms/saevm/blocks", visibility = ["//visibility:public"], @@ -51,7 +50,6 @@ go_test( "block_test.go", "execution_test.go", "settlement_test.go", - "time_test.go", ], data = glob(["testdata/**"]), embed = [":blocks"], diff --git a/vms/saevm/blocks/execution_test.go b/vms/saevm/blocks/execution_test.go index 5d58aa1fff0c..7077fe5bbd02 100644 --- a/vms/saevm/blocks/execution_test.go +++ b/vms/saevm/blocks/execution_test.go @@ -24,7 +24,6 @@ import ( "github.com/ava-labs/avalanchego/vms/components/gas" "github.com/ava-labs/avalanchego/vms/saevm/cmputils" "github.com/ava-labs/avalanchego/vms/saevm/gastime" - "github.com/ava-labs/avalanchego/vms/saevm/hook" "github.com/ava-labs/avalanchego/vms/saevm/saetest" saetypes "github.com/ava-labs/avalanchego/vms/saevm/types" @@ -174,7 +173,7 @@ type selfAsHasher common.Hash func (h selfAsHasher) Hash() common.Hash { return common.Hash(h) } -func mustNewGasTime(tb testing.TB, at time.Time, target, excess gas.Gas, gasPriceConfig hook.GasPriceConfig) *gastime.Time { +func mustNewGasTime(tb testing.TB, at time.Time, target, excess gas.Gas, gasPriceConfig gastime.GasPriceConfig) *gastime.Time { tb.Helper() tm, err := gastime.New(at, target, excess, gasPriceConfig) require.NoError(tb, err, "gastime.New()") diff --git a/vms/saevm/blocks/settlement.go b/vms/saevm/blocks/settlement.go index 1ec1e5f2ad87..dc0d1962b5ef 100644 --- a/vms/saevm/blocks/settlement.go +++ b/vms/saevm/blocks/settlement.go @@ -93,7 +93,7 @@ func (b *Block) MarkSynchronous(hooks hook.Points, db ethdb.Database, xdb types. // would also require them to be received as an argument to MarkSynchronous. target, cfg := hooks.GasConfigAfter(b.Header()) execTime, err := gastime.New( - PreciseTime(hooks, b.Header()), + hooks.BlockTime(b.Header()), // Target, excess, and config _after_ are a requirement of // [Block.MarkExecuted]. target, @@ -283,7 +283,7 @@ func LastToSettleAt(hooks hook.Points, settleAt time.Time, parent *Block) (b *Bl return block, known, nil } - if startsNoEarlierThan := PreciseTime(hooks, block.Header()); startsNoEarlierThan.Compare(settleAt) > 0 { + if startsNoEarlierThan := hooks.BlockTime(block.Header()); startsNoEarlierThan.Compare(settleAt) > 0 { known = true continue } diff --git a/vms/saevm/blocks/settlement_test.go b/vms/saevm/blocks/settlement_test.go index c939970c159f..594e574cd461 100644 --- a/vms/saevm/blocks/settlement_test.go +++ b/vms/saevm/blocks/settlement_test.go @@ -59,7 +59,7 @@ func TestSettlementInvariants(t *testing.T) { db := rawdb.NewMemoryDatabase() xdb := saetest.NewExecutionResultsDB() for _, b := range []*Block{b, parent, lastSettled} { - tm := mustNewGasTime(t, preciseTime(b.Header(), 0), 1, 0, gastime.DefaultGasPriceConfig()) + tm := mustNewGasTime(t, time.Unix(int64(b.Header().Time), 0), 1, 0, gastime.DefaultGasPriceConfig()) //#nosec G115 -- block time is hard-coded above. b.markExecutedForTests(t, db, xdb, tm) } diff --git a/vms/saevm/blocks/time.go b/vms/saevm/blocks/time.go deleted file mode 100644 index cf96920cade3..000000000000 --- a/vms/saevm/blocks/time.go +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (C) 2019, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package blocks - -import ( - "time" - - "github.com/ava-labs/libevm/core/types" - - "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/gastime" - "github.com/ava-labs/avalanchego/vms/saevm/hook" - "github.com/ava-labs/avalanchego/vms/saevm/proxytime" -) - -// PreciseTime calls [hook.Points.SubSecondBlockTime] on the header and returns -// the value, combined with the regular timestamp to provide a full-resolution -// block time. -func PreciseTime(hooks hook.Points, hdr *types.Header) time.Time { - return preciseTime(hdr, hooks.SubSecondBlockTime(hdr)) -} - -func preciseTime(hdr *types.Header, subSec time.Duration) time.Time { //nolint:staticcheck // subSec intentionally communicates that the value is < time.Second - return time.Unix( - int64(hdr.Time), //#nosec G115 -- Won't overflow for a few millennia - subSec.Nanoseconds(), - ) -} - -// GasTime is the gas equivalent of [PreciseTime], deriving the gas rate from -// the parent header and the hooks. -func GasTime(hooks hook.Points, hdr, parent *types.Header) *proxytime.Time[gas.Gas] { - target, _ := hooks.GasConfigAfter(parent) - rate := gastime.SafeRateOfTarget(target) - tm := proxytime.New(hdr.Time, rate) - tm.Tick(gastime.SubSecond(hooks, hdr, rate)) - return tm -} diff --git a/vms/saevm/blocks/time_test.go b/vms/saevm/blocks/time_test.go deleted file mode 100644 index 40febf026252..000000000000 --- a/vms/saevm/blocks/time_test.go +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright (C) 2019, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package blocks - -import ( - "math/big" - "testing" - "time" - - "github.com/ava-labs/libevm/core/types" - "github.com/google/go-cmp/cmp" - "github.com/stretchr/testify/require" - - "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/gastime" - "github.com/ava-labs/avalanchego/vms/saevm/hook/hookstest" - "github.com/ava-labs/avalanchego/vms/saevm/proxytime" -) - -func TestGasTime(t *testing.T) { - const ( - unix = 42 - frac = 12_345 - scale = 250 - target = 1_000_000_000 / gastime.TargetToRate / scale - nanos = frac * scale - ) - rate := gastime.SafeRateOfTarget(target) - - hooks := hookstest.NewStub(target, hookstest.WithNow(func() time.Time { - return time.Unix(unix, nanos) - })) - parent := &types.Header{ - Number: big.NewInt(1), - } - hdr, err := hooks.BuildHeader(parent) - require.NoErrorf(t, err, "%T.BuildHeader()", hooks) - - got := GasTime(hooks, hdr, parent) - want := proxytime.New(unix, rate) - want.Tick(frac) - - if diff := cmp.Diff(want, got, proxytime.CmpOpt[gas.Gas]()); diff != "" { - t.Errorf("GasTime(...) diff (-want +got):\n%s", diff) - } -} - -func FuzzTimeExtraction(f *testing.F) { - // There are two different ways that the gas time of a block can be - // calculated, both of which result in the same value. While neither can - // result in an overflow due to required invariants, this guarantee is much - // easier to reason about when inspecting [GasTime]. The alternative, via - // [proxytime.Of] and then [proxytime.Time.SetRate], is more obviously - // correct but too general-purpose and requires ignoring/checking a returned - // `error`. We can therefore use this equivalence for differential fuzzing. - - f.Fuzz(func(t *testing.T, unix int64, target uint64, subSec int64) { - if subSec < 0 || time.Duration(subSec) >= time.Second { - t.Skip("Invalid sub-second value") - } - if target == 0 { - t.Skip("Zero target") - } - - hooks := hookstest.NewStub(gas.Gas(target), hookstest.WithNow(func() time.Time { - return time.Unix(unix, subSec) - })) - parent := &types.Header{ - Number: big.NewInt(1), - } - hdr, err := hooks.BuildHeader(parent) - require.NoErrorf(t, err, "%T.BuildHeader()", hooks) - - t.Run("PreciseTime", func(t *testing.T) { - got := PreciseTime(hooks, hdr) - want := hooks.Now() - if got != want { - t.Errorf("PreciseTime() = %v; want %v", got, want) - } - }) - - t.Run("GasTime", func(t *testing.T) { - got := GasTime(hooks, hdr, parent) - - want := proxytime.Of[gas.Gas](hooks.Now()) - rate := gastime.SafeRateOfTarget(gas.Gas(target)) - want.SetRate(rate) - - if diff := cmp.Diff(want, got, proxytime.CmpOpt[gas.Gas]()); diff != "" { - t.Errorf("diff (-proxytime.Of +GasTime):\n%s", diff) - } - }) - }) -} diff --git a/vms/saevm/gastime/BUILD.bazel b/vms/saevm/gastime/BUILD.bazel index 75afdb37048f..6c4cf9888425 100644 --- a/vms/saevm/gastime/BUILD.bazel +++ b/vms/saevm/gastime/BUILD.bazel @@ -15,10 +15,8 @@ go_library( visibility = ["//visibility:public"], deps = [ "//vms/components/gas", - "//vms/saevm/hook", "//vms/saevm/intmath", "//vms/saevm/proxytime", - "@com_github_ava_labs_libevm//core/types", "@com_github_google_go_cmp//cmp", "@com_github_google_go_cmp//cmp/cmpopts", "@com_github_holiman_uint256//:uint256", @@ -36,12 +34,9 @@ go_test( embed = [":gastime"], deps = [ "//vms/components/gas", - "//vms/saevm/hook", - "//vms/saevm/hook/hookstest", "//vms/saevm/intmath", "//vms/saevm/proxytime", "@com_github_arr4n_shed//testerr", - "@com_github_ava_labs_libevm//core/types", "@com_github_ava_labs_libevm//params", "@com_github_google_go_cmp//cmp", "@com_github_google_go_cmp//cmp/cmpopts", diff --git a/vms/saevm/gastime/acp176.go b/vms/saevm/gastime/acp176.go index d7793d7f258e..4fdda16ff26b 100644 --- a/vms/saevm/gastime/acp176.go +++ b/vms/saevm/gastime/acp176.go @@ -6,27 +6,41 @@ package gastime import ( "fmt" "math" - - "github.com/ava-labs/libevm/core/types" + "time" "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/hook" + "github.com/ava-labs/avalanchego/vms/saevm/intmath" ) -// 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, - SubSecond(hooks, h, tm.Rate()), +// BeforeBlock is intended to be called before processing a block with the +// provided time. The gastime is advanced to be no earlier than the block time. +func (tm *Time) BeforeBlock(t time.Time) { + tm.FastForwardToTime(t) +} + +// FastForwardToTime is equivalent to [Time.FastForwardTo] except that it +// accepts a [time.Time]. +func (tm *Time) FastForwardToTime(t time.Time) { + g, _, err := intmath.MulDivCeil( + gas.Gas(t.Nanosecond()), //#nosec G115 -- ns is in [0, time.Second) + tm.Rate(), + gas.Gas(time.Second), ) + if err != nil { + // [time.Time.Nanosecond] is documented as only returning values in the + // range [0, time.Second). So either Nanosecond returned an incorrect + // value, or [intmath.MulDivCeil] incorrectly returned an error. + // Regardless, this failure MUST be detected in tests, hence not just + // dropping the error. + panic(fmt.Sprintf("broken invariant: %v", err)) + } + tm.FastForwardTo(uint64(t.Unix()), g) //#nosec G115 -- known non-negative. } // AfterBlock is intended to be called after processing a block, with the -// target and gas configuration sourced from [hook.Points] and [types.Header]. -func (tm *Time) AfterBlock(used gas.Gas, hooks hook.Points, h *types.Header) error { +// target and gas configuration provided. +func (tm *Time) AfterBlock(used gas.Gas, target gas.Gas, cfg GasPriceConfig) error { tm.Tick(used) - target, hookCfg := hooks.GasConfigAfter(h) // Although [Time.SetTarget] scales the excess by the same factor as the // change in target, it rounds when necessary, which might alter the price // by a negligible amount. We therefore take a price snapshot beforehand @@ -35,9 +49,8 @@ func (tm *Time) AfterBlock(used gas.Gas, hooks hook.Points, h *types.Header) err p := tm.Price() tm.SetTarget(target) - cfg, err := newConfig(hookCfg) - if err != nil { - return fmt.Errorf("%T.newConfig() after block: %w", tm, err) + if err := cfg.Validate(); err != nil { + return fmt.Errorf("%T.Validate() after block: %w", cfg, err) } if cfg.equals(tm.config) { return nil @@ -49,8 +62,8 @@ func (tm *Time) AfterBlock(used gas.Gas, hooks hook.Points, h *types.Header) err } // findExcessForPrice uses binary search over uint64 to find the smallest excess -// value that produces targetPrice with the current [config]. This maintains -// price continuity under a change in [config], with the following scenarios: +// value that produces targetPrice with the current [GasPriceConfig]. This maintains +// price continuity under a change in [GasPriceConfig], with the following scenarios: // // - K changes (via TargetToExcessScaling): Scale excess to maintain current price // - StaticPricing is true: Set excess to 0, enabling fixed price mode @@ -61,7 +74,7 @@ func (tm *Time) findExcessForPrice(targetPrice gas.Price) gas.Gas { // We return 0 in case targetPrice < minPrice because we should at least maintain the minimum price // by setting the excess to 0. ( P = M * e^(0 / K) = M ) // Note: Even though we return 0 for excess it won't avoid accumulating excess in the long run. - if targetPrice <= tm.config.minPrice || tm.config.staticPricing { + if targetPrice <= tm.config.MinPrice || tm.config.StaticPricing { return 0 } @@ -71,7 +84,7 @@ func (tm *Time) findExcessForPrice(targetPrice gas.Price) gas.Gas { lo, hi := gas.Gas(0), gas.Gas(math.MaxUint64) for lo < hi { mid := lo + (hi-lo)/2 - if gas.CalculatePrice(tm.config.minPrice, mid, k) >= targetPrice { + if gas.CalculatePrice(tm.config.MinPrice, mid, k) >= targetPrice { hi = mid } else { lo = mid + 1 diff --git a/vms/saevm/gastime/acp176_test.go b/vms/saevm/gastime/acp176_test.go index 336070a98a88..3eb8d1d9b0b2 100644 --- a/vms/saevm/gastime/acp176_test.go +++ b/vms/saevm/gastime/acp176_test.go @@ -9,7 +9,6 @@ import ( "testing" "time" - "github.com/ava-labs/libevm/core/types" "github.com/ava-labs/libevm/params" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -17,8 +16,6 @@ import ( "golang.org/x/text/message" "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/hook" - "github.com/ava-labs/avalanchego/vms/saevm/hook/hookstest" ) // TestInvalidConfigRejected verifies that zero values for TargetToExcessScaling @@ -29,18 +26,18 @@ func TestInvalidConfigRejected(t *testing.T) { tests := []struct { name string - config hook.GasPriceConfig + config GasPriceConfig expected error }{ { "zero_scaling", - hook.GasPriceConfig{TargetToExcessScaling: 0, MinPrice: DefaultGasPriceConfig().MinPrice}, - errInvalidGasPriceConfig, + GasPriceConfig{TargetToExcessScaling: 0, MinPrice: DefaultGasPriceConfig().MinPrice}, + errTargetToExcessScalingZero, }, { "zero_min_price", - hook.GasPriceConfig{TargetToExcessScaling: DefaultGasPriceConfig().TargetToExcessScaling, MinPrice: 0}, - errInvalidGasPriceConfig, + GasPriceConfig{TargetToExcessScaling: DefaultGasPriceConfig().TargetToExcessScaling, MinPrice: 0}, + errMinPriceZero, }, } @@ -48,15 +45,14 @@ func TestInvalidConfigRejected(t *testing.T) { t.Run(tt.name, func(t *testing.T) { tm := mustNew(t, time.Unix(42, 0), target, 0, DefaultGasPriceConfig()) - initialScaling := tm.config.targetToExcessScaling - initialMinPrice := tm.config.minPrice - hooks := hookstest.NewStub(target, hookstest.WithGasPriceConfig(tt.config)) - err := tm.AfterBlock(0, hooks, &types.Header{Time: 42}) + initialScaling := tm.config.TargetToExcessScaling + initialMinPrice := tm.config.MinPrice + err := tm.AfterBlock(0, target, tt.config) require.ErrorIs(t, err, tt.expected, "AfterBlock()") // Config unchanged after rejected update - assert.Equal(t, initialScaling, tm.config.targetToExcessScaling, "targetToExcessScaling changed") - assert.Equal(t, initialMinPrice, tm.config.minPrice, "minPrice changed") + assert.Equal(t, initialScaling, tm.config.TargetToExcessScaling, "targetToExcessScaling changed") + assert.Equal(t, initialMinPrice, tm.config.MinPrice, "minPrice changed") }) } } @@ -73,17 +69,13 @@ func TestTargetUpdateTiming(t *testing.T) { initialRate := tm.Rate() const ( - newTime uint64 = initialTime + 1 - newTarget = initialTarget + 100_000 + newTime = initialTime + 1 + newTarget = initialTarget + 100_000 ) - hook := hookstest.NewStub(newTarget) - header := &types.Header{ - Time: newTime, - } initialPrice := tm.Price() - tm.BeforeBlock(hook, header) - assert.Equal(t, newTime, tm.Unix(), "Unix time advanced by BeforeBlock()") + tm.BeforeBlock(time.Unix(newTime, 0)) + assert.Equal(t, uint64(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. @@ -98,8 +90,8 @@ func TestTargetUpdateTiming(t *testing.T) { 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") + require.NoError(t, tm.AfterBlock(used, newTarget, DefaultGasPriceConfig()), "AfterBlock()") + assert.Equal(t, uint64(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. @@ -116,6 +108,7 @@ func FuzzWorstCasePrice(f *testing.F) { time3, nanos3, used3, limit3, target3 uint64, ) { initTarget = max(initTarget, 1) + gasCfg := DefaultGasPriceConfig() initUnix := int64(min(initTimestamp, math.MaxInt64)) //#nosec G115 -- Clamped to MaxInt64 worstcase := mustNew(t, time.Unix(initUnix, 0), gas.Gas(initTarget), gas.Gas(initExcess), DefaultGasPriceConfig()) @@ -160,25 +153,17 @@ func FuzzWorstCasePrice(f *testing.F) { for _, block := range blocks { block.limit = max(block.used, block.limit) block.target = clampTarget(max(block.target, 1)) + blockSeconds := int64(min(block.time, math.MaxInt64)) //#nosec G115 -- Clamped to MaxInt64 - header := &types.Header{ - Time: block.time, - } - hook := hookstest.NewStub(block.target, hookstest.WithNow(func() time.Time { - return time.Unix( - int64(block.time), //#nosec G115 -- Won't overflow for a few millennia - int64(block.nanos), - ) - })) - - worstcase.BeforeBlock(hook, header) - actual.BeforeBlock(hook, header) + tm := time.Unix(blockSeconds, int64(block.nanos)) + worstcase.BeforeBlock(tm) + actual.BeforeBlock(tm) // 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()") + require.NoError(t, worstcase.AfterBlock(block.limit, block.target, gasCfg), "worstcase.AfterBlock()") + require.NoError(t, actual.AfterBlock(block.used, block.target, gasCfg), "actual.AfterBlock()") } }) } @@ -339,7 +324,7 @@ func FuzzPriceInvarianceAfterBlock(f *testing.F) { t.Skip("New target too low") } - initConfig := hook.GasPriceConfig{ + initConfig := GasPriceConfig{ TargetToExcessScaling: gas.Gas(initScaling), MinPrice: gas.Price(initMinPrice), StaticPricing: initStaticPricing, @@ -353,8 +338,11 @@ func FuzzPriceInvarianceAfterBlock(f *testing.F) { { var wantErrIs error - if initScaling == 0 || initMinPrice == 0 { - wantErrIs = errInvalidGasPriceConfig + switch { + case initScaling == 0: + wantErrIs = errTargetToExcessScalingZero + case initMinPrice == 0: + wantErrIs = errMinPriceZero } require.ErrorIsf(t, err, wantErrIs, "New(... %+v)", initConfig) if wantErrIs != nil { @@ -366,24 +354,24 @@ func FuzzPriceInvarianceAfterBlock(f *testing.F) { initPrice := tm.Price() { - hooks := hookstest.NewStub( - gas.Gas(newTarget), - hookstest.WithGasPriceConfig(hook.GasPriceConfig{ - MinPrice: gas.Price(newMinPrice), - TargetToExcessScaling: gas.Gas(newScaling), - StaticPricing: newStaticPricing, - })) + cfg := GasPriceConfig{ + MinPrice: gas.Price(newMinPrice), + TargetToExcessScaling: gas.Gas(newScaling), + StaticPricing: newStaticPricing, + } var wantErrIs error - if newScaling == 0 || newMinPrice == 0 { - wantErrIs = errInvalidGasPriceConfig + switch { + case newScaling == 0: + wantErrIs = errTargetToExcessScalingZero + case newMinPrice == 0: + wantErrIs = errMinPriceZero } - // Consuming gas increases the excess, which changes the price. // We're only interested in invariance under changes in config. const gasUsed = 0 - err := tm.AfterBlock(gasUsed, hooks, nil) - require.ErrorIsf(t, err, wantErrIs, "AfterBlock([%+v])", hooks) + err := tm.AfterBlock(gasUsed, gas.Gas(newTarget), cfg) + require.ErrorIsf(t, err, wantErrIs, "AfterBlock(%d, [%+v])", newTarget, cfg) if wantErrIs != nil { return } @@ -411,7 +399,7 @@ func FuzzPriceInvarianceAfterBlock(f *testing.F) { case newStaticPricing: require.Equal(t, minP, tm.Price(), "static pricing -> price == min") - case tm.config.equalHookConfig(t, initConfig): + case tm.config.equals(initConfig): // Target-only changes keep the x/K exponent as close to equal as // possible given the resolution of the denominator. newExp := tm.exponent() @@ -440,15 +428,6 @@ func FuzzPriceInvarianceAfterBlock(f *testing.F) { }) } -// equalHookConfig is equivalent to [config.equal] but accepts a -// [hook.GasPriceConfig] that is first converted and validated. -func (c *config) equalHookConfig(tb testing.TB, hookCfg hook.GasPriceConfig) bool { - tb.Helper() - other, err := newConfig(hookCfg) - require.NoError(tb, err) - return c.equals(other) -} - // exponent returns x/K, the exponent of the gas price. For fixed M, equal // exponents result in equal prices. However if scaling results in a new // exponent with insufficient resolution (too low a denominator) to be identical diff --git a/vms/saevm/gastime/cmpopt.go b/vms/saevm/gastime/cmpopt.go index 76eb6c8647be..6cfd9e227459 100644 --- a/vms/saevm/gastime/cmpopt.go +++ b/vms/saevm/gastime/cmpopt.go @@ -17,8 +17,8 @@ import ( // tests. func CmpOpt() cmp.Option { return cmp.Options{ - cmp.AllowUnexported(Time{}, config{}), - cmpopts.IgnoreTypes(canotoData_Time{}, canotoData_config{}), + cmp.AllowUnexported(Time{}, GasPriceConfig{}), + cmpopts.IgnoreTypes(canotoData_Time{}, canotoData_GasPriceConfig{}), proxytime.CmpOpt[gas.Gas](), } } diff --git a/vms/saevm/gastime/config.canoto.go b/vms/saevm/gastime/config.canoto.go index 725b6d4229ed..7a53d641b514 100644 --- a/vms/saevm/gastime/config.canoto.go +++ b/vms/saevm/gastime/config.canoto.go @@ -27,40 +27,40 @@ var ( ) const ( - canoto__config__targetToExcessScaling = 1 - canoto__config__minPrice = 2 - canoto__config__staticPricing = 3 + canoto__GasPriceConfig__TargetToExcessScaling = 1 + canoto__GasPriceConfig__MinPrice = 2 + canoto__GasPriceConfig__StaticPricing = 3 - canoto__config__targetToExcessScaling__tag = "\x08" // canoto.Tag(canoto__config__targetToExcessScaling, canoto.Varint) - canoto__config__minPrice__tag = "\x10" // canoto.Tag(canoto__config__minPrice, canoto.Varint) - canoto__config__staticPricing__tag = "\x18" // canoto.Tag(canoto__config__staticPricing, canoto.Varint) + canoto__GasPriceConfig__TargetToExcessScaling__tag = "\x08" // canoto.Tag(canoto__GasPriceConfig__TargetToExcessScaling, canoto.Varint) + canoto__GasPriceConfig__MinPrice__tag = "\x10" // canoto.Tag(canoto__GasPriceConfig__MinPrice, canoto.Varint) + canoto__GasPriceConfig__StaticPricing__tag = "\x18" // canoto.Tag(canoto__GasPriceConfig__StaticPricing, canoto.Varint) ) -type canotoData_config struct { +type canotoData_GasPriceConfig struct { size uint64 } // CanotoSpec returns the specification of this canoto message. -func (*config) CanotoSpec(...reflect.Type) *canoto.Spec { - var zero config +func (*GasPriceConfig) CanotoSpec(...reflect.Type) *canoto.Spec { + var zero GasPriceConfig s := &canoto.Spec{ - Name: "config", + Name: "GasPriceConfig", Fields: []canoto.FieldType{ { - FieldNumber: canoto__config__targetToExcessScaling, - Name: "targetToExcessScaling", + FieldNumber: canoto__GasPriceConfig__TargetToExcessScaling, + Name: "TargetToExcessScaling", OneOf: "", - TypeUint: canoto.SizeOf(zero.targetToExcessScaling), + TypeUint: canoto.SizeOf(zero.TargetToExcessScaling), }, { - FieldNumber: canoto__config__minPrice, - Name: "minPrice", + FieldNumber: canoto__GasPriceConfig__MinPrice, + Name: "MinPrice", OneOf: "", - TypeUint: canoto.SizeOf(zero.minPrice), + TypeUint: canoto.SizeOf(zero.MinPrice), }, { - FieldNumber: canoto__config__staticPricing, - Name: "staticPricing", + FieldNumber: canoto__GasPriceConfig__StaticPricing, + Name: "StaticPricing", OneOf: "", TypeBool: true, }, @@ -73,7 +73,7 @@ func (*config) CanotoSpec(...reflect.Type) *canoto.Spec { // UnmarshalCanoto unmarshals a Canoto-encoded byte slice into the struct. // // During parsing, the canoto cache is saved. -func (c *config) UnmarshalCanoto(bytes []byte) error { +func (c *GasPriceConfig) UnmarshalCanoto(bytes []byte) error { r := canoto.Reader{ B: bytes, } @@ -86,9 +86,9 @@ func (c *config) UnmarshalCanoto(bytes []byte) error { // During parsing, the canoto cache is saved. // // This function enables configuration of reader options. -func (c *config) UnmarshalCanotoFrom(r canoto.Reader) error { +func (c *GasPriceConfig) UnmarshalCanotoFrom(r canoto.Reader) error { // Zero the struct before unmarshaling. - *c = config{} + *c = GasPriceConfig{} atomic.StoreUint64(&c.canotoData.size, uint64(len(r.B))) var minField uint32 @@ -102,37 +102,37 @@ func (c *config) UnmarshalCanotoFrom(r canoto.Reader) error { } switch field { - case canoto__config__targetToExcessScaling: + case canoto__GasPriceConfig__TargetToExcessScaling: if wireType != canoto.Varint { return canoto.ErrUnexpectedWireType } - if err := canoto.ReadUint(&r, &c.targetToExcessScaling); err != nil { + if err := canoto.ReadUint(&r, &c.TargetToExcessScaling); err != nil { return err } - if canoto.IsZero(c.targetToExcessScaling) { + if canoto.IsZero(c.TargetToExcessScaling) { return canoto.ErrZeroValue } - case canoto__config__minPrice: + case canoto__GasPriceConfig__MinPrice: if wireType != canoto.Varint { return canoto.ErrUnexpectedWireType } - if err := canoto.ReadUint(&r, &c.minPrice); err != nil { + if err := canoto.ReadUint(&r, &c.MinPrice); err != nil { return err } - if canoto.IsZero(c.minPrice) { + if canoto.IsZero(c.MinPrice) { return canoto.ErrZeroValue } - case canoto__config__staticPricing: + case canoto__GasPriceConfig__StaticPricing: if wireType != canoto.Varint { return canoto.ErrUnexpectedWireType } - if err := canoto.ReadBool(&r, &c.staticPricing); err != nil { + if err := canoto.ReadBool(&r, &c.StaticPricing); err != nil { return err } - if canoto.IsZero(c.staticPricing) { + if canoto.IsZero(c.StaticPricing) { return canoto.ErrZeroValue } default: @@ -151,7 +151,7 @@ func (c *config) UnmarshalCanotoFrom(r canoto.Reader) error { // 1. All OneOfs are specified at most once. // 2. All strings are valid utf-8. // 3. All custom fields are ValidCanoto. -func (c *config) ValidCanoto() bool { +func (c *GasPriceConfig) ValidCanoto() bool { return true } @@ -159,16 +159,16 @@ func (c *config) ValidCanoto() bool { // values in the struct. // // It is not safe to copy this struct concurrently. -func (c *config) CalculateCanotoCache() { +func (c *GasPriceConfig) CalculateCanotoCache() { var size uint64 - if !canoto.IsZero(c.targetToExcessScaling) { - size += uint64(len(canoto__config__targetToExcessScaling__tag)) + canoto.SizeUint(c.targetToExcessScaling) + if !canoto.IsZero(c.TargetToExcessScaling) { + size += uint64(len(canoto__GasPriceConfig__TargetToExcessScaling__tag)) + canoto.SizeUint(c.TargetToExcessScaling) } - if !canoto.IsZero(c.minPrice) { - size += uint64(len(canoto__config__minPrice__tag)) + canoto.SizeUint(c.minPrice) + if !canoto.IsZero(c.MinPrice) { + size += uint64(len(canoto__GasPriceConfig__MinPrice__tag)) + canoto.SizeUint(c.MinPrice) } - if !canoto.IsZero(c.staticPricing) { - size += uint64(len(canoto__config__staticPricing__tag)) + canoto.SizeBool + if !canoto.IsZero(c.StaticPricing) { + size += uint64(len(canoto__GasPriceConfig__StaticPricing__tag)) + canoto.SizeBool } atomic.StoreUint64(&c.canotoData.size, size) } @@ -180,7 +180,7 @@ func (c *config) CalculateCanotoCache() { // // If the struct has been modified since the last call to CalculateCanotoCache, // the returned size may be incorrect. -func (c *config) CachedCanotoSize() uint64 { +func (c *GasPriceConfig) CachedCanotoSize() uint64 { return atomic.LoadUint64(&c.canotoData.size) } @@ -189,7 +189,7 @@ func (c *config) CachedCanotoSize() uint64 { // It is assumed that this struct is ValidCanoto. // // It is not safe to copy this struct concurrently. -func (c *config) MarshalCanoto() []byte { +func (c *GasPriceConfig) MarshalCanoto() []byte { c.CalculateCanotoCache() w := canoto.Writer{ B: make([]byte, 0, c.CachedCanotoSize()), @@ -207,17 +207,17 @@ func (c *config) MarshalCanoto() []byte { // It is assumed that this struct is ValidCanoto. // // It is not safe to copy this struct concurrently. -func (c *config) MarshalCanotoInto(w canoto.Writer) canoto.Writer { - if !canoto.IsZero(c.targetToExcessScaling) { - canoto.Append(&w, canoto__config__targetToExcessScaling__tag) - canoto.AppendUint(&w, c.targetToExcessScaling) +func (c *GasPriceConfig) MarshalCanotoInto(w canoto.Writer) canoto.Writer { + if !canoto.IsZero(c.TargetToExcessScaling) { + canoto.Append(&w, canoto__GasPriceConfig__TargetToExcessScaling__tag) + canoto.AppendUint(&w, c.TargetToExcessScaling) } - if !canoto.IsZero(c.minPrice) { - canoto.Append(&w, canoto__config__minPrice__tag) - canoto.AppendUint(&w, c.minPrice) + if !canoto.IsZero(c.MinPrice) { + canoto.Append(&w, canoto__GasPriceConfig__MinPrice__tag) + canoto.AppendUint(&w, c.MinPrice) } - if !canoto.IsZero(c.staticPricing) { - canoto.Append(&w, canoto__config__staticPricing__tag) + if !canoto.IsZero(c.StaticPricing) { + canoto.Append(&w, canoto__GasPriceConfig__StaticPricing__tag) canoto.AppendBool(&w, true) } return w diff --git a/vms/saevm/gastime/config.go b/vms/saevm/gastime/config.go index 59a354d3d85a..2d64995c9d61 100644 --- a/vms/saevm/gastime/config.go +++ b/vms/saevm/gastime/config.go @@ -5,42 +5,69 @@ package gastime import ( "errors" - "fmt" "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/hook" ) //go:generate go run github.com/StephenButtolph/canoto/canoto $GOFILE -//nolint:revive // struct-tag: canoto allows unexported fields -type config struct { - targetToExcessScaling gas.Gas `canoto:"uint,1"` - minPrice gas.Price `canoto:"uint,2"` - staticPricing bool `canoto:"bool,3"` +// GasPriceConfig contains gas-related parameters that can be configured via hooks. +type GasPriceConfig struct { + // TargetToExcessScaling is the ratio between the gas target and the + // reciprocal of the excess coefficient used in price calculation + // (K variable in ACP-176, where K = TargetToExcessScaling * T). + // MUST be non-zero. + TargetToExcessScaling gas.Gas `canoto:"uint,1"` + // MinPrice is the minimum gas price / base fee (M parameter in ACP-176). + // MUST be non-zero. + MinPrice gas.Price `canoto:"uint,2"` + // StaticPricing is a flag indicating whether the gas price should be static + // at the minimum price. + StaticPricing bool `canoto:"bool,3"` - canotoData canotoData_config + canotoData canotoData_GasPriceConfig } -var errInvalidGasPriceConfig = errors.New("invalid gas price config") +// DefaultTargetToExcessScaling is the default ratio between gas target and the +// reciprocal of the excess coefficient used in price calculation (K variable in ACP-176). +const DefaultTargetToExcessScaling = 87 -// newConfig builds and validates an internal config from [hook.GasPriceConfig]. -func newConfig(from hook.GasPriceConfig) (config, error) { - if err := from.Validate(); err != nil { - return config{}, fmt.Errorf("%w: %w", errInvalidGasPriceConfig, err) +// DefaultMinPrice is the default minimum gas price (base fee), i.e. the M +// parameter in ACP-176's price calculation. +const DefaultMinPrice gas.Price = 1 + +// DefaultGasPriceConfig returns the default [GasPriceConfig] values. +func DefaultGasPriceConfig() GasPriceConfig { + return GasPriceConfig{ + TargetToExcessScaling: DefaultTargetToExcessScaling, + MinPrice: DefaultMinPrice, + StaticPricing: false, + } +} + +var ( + errTargetToExcessScalingZero = errors.New("targetToExcessScaling must be non-zero") + errMinPriceZero = errors.New("minPrice must be non-zero") +) + +// Validate checks that the GasPriceConfig fields are valid. +func (c *GasPriceConfig) Validate() error { + if c.TargetToExcessScaling == 0 { + return errTargetToExcessScalingZero } - c := config{ - targetToExcessScaling: from.TargetToExcessScaling, - minPrice: from.MinPrice, - staticPricing: from.StaticPricing, + // TODO (ceyonur): Decide whether we want to allow zero min price exclusive for static pricing, + // to support fee-less networks. + // https://github.com/ava-labs/strevm/issues/266 + if c.MinPrice == 0 { + return errMinPriceZero } - return c, nil + return nil } // equal returns true if the logical fields of c and other are equal. // It ignores canoto internal fields. -func (c config) equals(other config) bool { - return c.targetToExcessScaling == other.targetToExcessScaling && - c.minPrice == other.minPrice && - c.staticPricing == other.staticPricing +func (c GasPriceConfig) equals(other GasPriceConfig) bool { + return c.TargetToExcessScaling == other.TargetToExcessScaling && + c.MinPrice == other.MinPrice && + c.StaticPricing == other.StaticPricing } diff --git a/vms/saevm/gastime/gastime.go b/vms/saevm/gastime/gastime.go index c2f124503934..a8d2f396e936 100644 --- a/vms/saevm/gastime/gastime.go +++ b/vms/saevm/gastime/gastime.go @@ -8,11 +8,9 @@ import ( "math" "time" - "github.com/ava-labs/libevm/core/types" "github.com/holiman/uint256" "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/hook" "github.com/ava-labs/avalanchego/vms/saevm/intmath" "github.com/ava-labs/avalanchego/vms/saevm/proxytime" ) @@ -34,9 +32,9 @@ import ( //nolint:tagliatelle,revive // tagliatelle: can't handle embedded field; struct-tag: canoto allows unexported fields type Time struct { *proxytime.Time[gas.Gas] `canoto:"pointer,1"` - target gas.Gas `canoto:"uint,2"` - excess gas.Gas `canoto:"uint,3"` - config config `canoto:"value,4"` + target gas.Gas `canoto:"uint,2"` + excess gas.Gas `canoto:"uint,3"` + config GasPriceConfig `canoto:"value,4"` canotoData canotoData_Time `canoto:"nocopy"` } @@ -44,9 +42,8 @@ type Time struct { // New returns a new [Time], derived from a [time.Time]. The consumption of // `target` * [TargetToRate] units of [gas.Gas] is equivalent to a tick of 1 // second. -func New(at time.Time, target, startingExcess gas.Gas, gasPriceConfig hook.GasPriceConfig) (*Time, error) { - cfg, err := newConfig(gasPriceConfig) - if err != nil { +func New(at time.Time, target, startingExcess gas.Gas, gasPriceConfig GasPriceConfig) (*Time, error) { + if err := gasPriceConfig.Validate(); err != nil { return nil, err } @@ -58,49 +55,17 @@ func New(at time.Time, target, startingExcess gas.Gas, gasPriceConfig hook.GasPr Time: tm, target: target, excess: startingExcess, - config: cfg, + config: gasPriceConfig, }, nil } -// SubSecond scales the value returned by [hook.Points.SubSecondBlockTime] to -// reflect the given gas rate. -func SubSecond(hooks hook.Points, hdr *types.Header, rate gas.Gas) gas.Gas { - // [hook.Points.SubSecondBlockTime] is required to return values in - // [0,second). The lower bound guarantees that the conversion to unsigned - // [gas.Gas] is safe while the upper bound guarantees that the mul-div - // result can't overflow so we don't have to check the error. - g, _, _ := intmath.MulDivCeil( - gas.Gas(hooks.SubSecondBlockTime(hdr)), //#nosec G115 -- See above - rate, - gas.Gas(time.Second), - ) - return g -} - -// TargetToRate is the ratio between [Time.Target] and [proxytime.Time.Rate]. -const TargetToRate = 2 - -// DefaultTargetToExcessScaling is the default ratio between gas target and the -// reciprocal of the excess coefficient used in price calculation (K variable in ACP-176). -const DefaultTargetToExcessScaling = 87 - -// DefaultMinPrice is the default minimum gas price (base fee), i.e. the M -// parameter in ACP-176's price calculation. -const DefaultMinPrice gas.Price = 1 - -// DefaultGasPriceConfig returns the default [hook.GasPriceConfig] values. -func DefaultGasPriceConfig() hook.GasPriceConfig { - return hook.GasPriceConfig{ - TargetToExcessScaling: DefaultTargetToExcessScaling, - MinPrice: DefaultMinPrice, - StaticPricing: false, - } -} - // MinTarget is the minimum allowable [Time.Target] to avoid division by zero. // Values below this are silently clamped. const MinTarget = gas.Gas(1) +// TargetToRate is the ratio between [Time.Target] and [proxytime.Time.Rate]. +const TargetToRate = 2 + // MaxTarget is the maximum allowable [Time.Target] to avoid overflows of the // associated [proxytime.Time.Rate]. Values above this are silently clamped. const MaxTarget = gas.Gas(math.MaxUint64 / TargetToRate) @@ -138,21 +103,21 @@ func (tm *Time) Excess() gas.Gas { } // Price returns the price of a unit of gas, i.e. the "base fee", determined by -// [gas.CalculatePrice]. However, when [hook.GasPriceConfig.StaticPricing] is -// true, Price always returns [hook.GasPriceConfig.MinPrice]. +// [gas.CalculatePrice]. However, when [GasPriceConfig.StaticPricing] is +// true, Price always returns [GasPriceConfig.MinPrice]. func (tm *Time) Price() gas.Price { - if tm.config.staticPricing { - return tm.config.minPrice + if tm.config.StaticPricing { + return tm.config.MinPrice } // TODO (ceyonur): Consider omitting `MinPrice` in favor of `MinExcess`. // https://github.com/ava-labs/strevm/issues/267 - return gas.CalculatePrice(tm.config.minPrice, tm.excess, tm.excessScalingFactor()) + return gas.CalculatePrice(tm.config.MinPrice, tm.excess, tm.excessScalingFactor()) } // excessScalingFactor returns the K variable of ACP-103/176, i.e. -// [config.targetToExcessScaling] * T, capped at [math.MaxUint64]. +// [GasPriceConfig.TargetToExcessScaling] * T, capped at [math.MaxUint64]. func (tm *Time) excessScalingFactor() gas.Gas { - return intmath.BoundedMultiply(tm.config.targetToExcessScaling, tm.target, math.MaxUint64) + return intmath.BoundedMultiply(tm.config.TargetToExcessScaling, tm.target, math.MaxUint64) } // BaseFee is equivalent to [Time.Price], returning the result as a uint256 for diff --git a/vms/saevm/gastime/gastime_test.go b/vms/saevm/gastime/gastime_test.go index 2aa74a0a1d55..21be2caacc7a 100644 --- a/vms/saevm/gastime/gastime_test.go +++ b/vms/saevm/gastime/gastime_test.go @@ -16,12 +16,11 @@ import ( "github.com/stretchr/testify/require" "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/avalanchego/vms/saevm/hook" "github.com/ava-labs/avalanchego/vms/saevm/intmath" "github.com/ava-labs/avalanchego/vms/saevm/proxytime" ) -func mustNew(tb testing.TB, at time.Time, target, startingExcess gas.Gas, gasPriceConfig hook.GasPriceConfig) *Time { +func mustNew(tb testing.TB, at time.Time, target, startingExcess gas.Gas, gasPriceConfig GasPriceConfig) *Time { tb.Helper() tm, err := New(at, target, startingExcess, gasPriceConfig) require.NoError(tb, err, "New(%v, %d, %d, %v)", at, target, startingExcess, gasPriceConfig) @@ -36,7 +35,7 @@ func (tm *Time) cloneViaCanotoRoundTrip(tb testing.TB) *Time { } func TestClone(t *testing.T) { - tm := mustNew(t, time.Unix(42, 1), 1e6, 1e5, hook.GasPriceConfig{TargetToExcessScaling: 100, MinPrice: 200}) + tm := mustNew(t, time.Unix(42, 1), 1e6, 1e5, GasPriceConfig{TargetToExcessScaling: 100, MinPrice: 200}) if diff := cmp.Diff(tm, tm.Clone(), CmpOpt()); diff != "" { t.Errorf("%T.Clone() diff (-want +got):\n%s", tm, diff) @@ -363,7 +362,7 @@ func TestMinAndStaticPrice(t *testing.T) { { name: "zero_min_errors", minPrice: 0, - wantErr: testerr.Is(errInvalidGasPriceConfig), + wantErr: testerr.Is(errMinPriceZero), }, { name: "static_pricing_returns_min", diff --git a/vms/saevm/hook/BUILD.bazel b/vms/saevm/hook/BUILD.bazel index 4ee090752dda..ed5c417a3515 100644 --- a/vms/saevm/hook/BUILD.bazel +++ b/vms/saevm/hook/BUILD.bazel @@ -10,6 +10,7 @@ go_library( "//ids", "//snow/engine/snowman/block", "//vms/components/gas", + "//vms/saevm/gastime", "//vms/saevm/intmath", "//vms/saevm/params", "//vms/saevm/types", diff --git a/vms/saevm/hook/hook.go b/vms/saevm/hook/hook.go index 5078b5cd1802..21399fd82b9b 100644 --- a/vms/saevm/hook/hook.go +++ b/vms/saevm/hook/hook.go @@ -26,6 +26,7 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow/engine/snowman/block" "github.com/ava-labs/avalanchego/vms/components/gas" + "github.com/ava-labs/avalanchego/vms/saevm/gastime" "github.com/ava-labs/avalanchego/vms/saevm/intmath" saeparams "github.com/ava-labs/avalanchego/vms/saevm/params" @@ -56,14 +57,14 @@ type Points interface { // will be closed by the VM when no longer needed. It MAY use the provided // directory for persistence and MUST NOT write data outside of it. ExecutionResultsDB(dataDir string) (saetypes.ExecutionResults, error) - // GasConfigAfter returns the gas target and configuration that should go // into effect immediately after the provided block. - GasConfigAfter(*types.Header) (target gas.Gas, c GasPriceConfig) - // SubSecondBlockTime returns the sub-second portion of the block time, - // which MUST be non-negative and strictly shorter than a second; i.e. a - // value d such that 0 <= d < [time.Second]. - SubSecondBlockTime(h *types.Header) time.Duration + GasConfigAfter(*types.Header) (target gas.Gas, c gastime.GasPriceConfig) + // BlockTime returns the exact block time for the given header, as recorded + // in [BlockBuilder.BuildHeader]. The returned time MUST match the header + // ([time.Time.Unix] == [types.Header.Time]) and MAY include a sub-second + // component. + BlockTime(h *types.Header) time.Time // SettledHeight returns the block height which [types.Header.Root] corresponds // with as the post-execution state root. It MUST match the value passed to // [BlockBuilder.BuildBlock], from which the [types.Header] will be sourced. @@ -194,40 +195,6 @@ func (o *Op) ApplyTo(stateDB *state.StateDB) error { return nil } -// GasPriceConfig contains gas-related parameters that can be configured via hooks. -type GasPriceConfig struct { - // TargetToExcessScaling is the ratio between the gas target and the - // reciprocal of the excess coefficient used in price calculation - // (K variable in ACP-176, where K = TargetToExcessScaling * T). - // MUST be non-zero. - TargetToExcessScaling gas.Gas - // MinPrice is the minimum gas price / base fee (M parameter in ACP-176). - // MUST be non-zero. - MinPrice gas.Price - // StaticPricing is a flag indicating whether the gas price should be static - // at the minimum price. - StaticPricing bool -} - -var ( - errTargetToExcessScalingZero = errors.New("targetToExcessScaling must be non-zero") - errMinPriceZero = errors.New("minPrice must be non-zero") -) - -// Validate checks that the GasPriceConfig fields are valid. -func (c *GasPriceConfig) Validate() error { - if c.TargetToExcessScaling == 0 { - return errTargetToExcessScalingZero - } - // TODO (ceyonur): Decide whether we want to allow zero min price exclusive for static pricing, - // to support fee-less networks. - // https://github.com/ava-labs/strevm/issues/266 - if c.MinPrice == 0 { - return errMinPriceZero - } - return nil -} - // MinimumGasConsumption MUST be used as the implementation for the respective // method on [params.RulesHooks]. The concrete type implementing the hooks MUST // propagate incoming and return arguments unchanged. diff --git a/vms/saevm/hook/hookstest/BUILD.bazel b/vms/saevm/hook/hookstest/BUILD.bazel index 85c1e7edf611..1d6a5311cfa8 100644 --- a/vms/saevm/hook/hookstest/BUILD.bazel +++ b/vms/saevm/hook/hookstest/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "//snow/engine/snowman/block", "//utils/set", "//vms/components/gas", + "//vms/saevm/gastime", "//vms/saevm/hook", "//vms/saevm/saetest", "//vms/saevm/types", diff --git a/vms/saevm/hook/hookstest/stub.go b/vms/saevm/hook/hookstest/stub.go index a71b658a5dfe..6b19822538b8 100644 --- a/vms/saevm/hook/hookstest/stub.go +++ b/vms/saevm/hook/hookstest/stub.go @@ -22,6 +22,7 @@ import ( "github.com/ava-labs/avalanchego/snow/engine/snowman/block" "github.com/ava-labs/avalanchego/utils/set" "github.com/ava-labs/avalanchego/vms/components/gas" + "github.com/ava-labs/avalanchego/vms/saevm/gastime" "github.com/ava-labs/avalanchego/vms/saevm/hook" "github.com/ava-labs/avalanchego/vms/saevm/saetest" @@ -36,7 +37,7 @@ type Stub struct { Ops []Op ExecutionResultsDBFn func(string) (saetypes.ExecutionResults, error) CanExecuteTransactionFn func(common.Address, *common.Address, libevm.StateReader) error - GasPriceConfig hook.GasPriceConfig + GasPriceConfig gastime.GasPriceConfig } var _ hook.PointsG[Op] = (*Stub)(nil) @@ -45,7 +46,7 @@ var _ hook.PointsG[Op] = (*Stub)(nil) type HookOption = options.Option[Stub] // WithGasPriceConfig overrides the default gas config. -func WithGasPriceConfig(cfg hook.GasPriceConfig) HookOption { +func WithGasPriceConfig(cfg gastime.GasPriceConfig) HookOption { return options.Func[Stub](func(s *Stub) { s.GasPriceConfig = cfg }) @@ -82,16 +83,9 @@ func WithExecutionResultsDBFn(fn func(string) (saetypes.ExecutionResults, error) // NewStub returns a stub with defaults applied. // It uses [gastime.DefaultGasPriceConfig] unless overridden by [WithGasPriceConfig]. func NewStub(target gas.Gas, opts ...HookOption) *Stub { - // defaultGasPriceConfig is the same as [gastime.DefaultGasPriceConfig]. It is defined - // here to avoid a circular dependency between [gastime] and [hookstest]. - defaultGasPriceConfig := hook.GasPriceConfig{ - TargetToExcessScaling: 87, - MinPrice: 1, - StaticPricing: false, - } return options.ApplyTo(&Stub{ Target: target, - GasPriceConfig: defaultGasPriceConfig, + GasPriceConfig: gastime.DefaultGasPriceConfig(), }, opts...) } @@ -199,15 +193,16 @@ func (s *Stub) BlockRebuilderFrom(b *types.Block) (hook.BlockBuilder[Op], error) } // GasConfigAfter ignores its argument and always returns [Stub.Target] and [Stub.GasPriceConfig]. -func (s *Stub) GasConfigAfter(*types.Header) (gas.Gas, hook.GasPriceConfig) { +func (s *Stub) GasConfigAfter(*types.Header) (gas.Gas, gastime.GasPriceConfig) { return s.Target, s.GasPriceConfig } -// SubSecondBlockTime returns the sub-second time encoded and stored by -// [Stub.BuildHeader] in the header's `Extra` field. If said field is empty, -// SubSecondBlockTime returns 0. -func (*Stub) SubSecondBlockTime(hdr *types.Header) time.Duration { - return getHeaderExtra(hdr).subSec +// BlockTime returns exact block time from [Stub.BuildHeader] by combining the +// stored seconds in [types.Header.Time] and the sub-second component from +// [types.Header.Extra]. +func (*Stub) BlockTime(hdr *types.Header) time.Time { + subSec := getHeaderExtra(hdr).subSec //nolint:staticcheck // subSec intentionally communicates that the value is < time.Second + return time.Unix(int64(hdr.Time), int64(subSec)) //#nosec G115 -- Won't overflow for a few millennia } // SettledHeight returns the height encoded in the Header by [Stub.BuildBlock] diff --git a/vms/saevm/sae/block_builder.go b/vms/saevm/sae/block_builder.go index ffd744fd3151..34d00f1d06b3 100644 --- a/vms/saevm/sae/block_builder.go +++ b/vms/saevm/sae/block_builder.go @@ -337,8 +337,8 @@ func lastToSettle( now time.Time, log logging.Logger, ) (*blocks.Block, error) { - bTime := blocks.PreciseTime(hooks, hdr) - pTime := blocks.PreciseTime(hooks, parent.Header()) + bTime := hooks.BlockTime(hdr) + pTime := hooks.BlockTime(parent.Header()) // It is allowed for [hook.BlockBuilder] to further constrain the allowed // block times. However, every block MUST at least satisfy these basic diff --git a/vms/saevm/sae/recovery.go b/vms/saevm/sae/recovery.go index 272ce3edb9e0..019b780574e2 100644 --- a/vms/saevm/sae/recovery.go +++ b/vms/saevm/sae/recovery.go @@ -121,7 +121,7 @@ func (rec *recovery) consensusCriticalBlocks(exec *saexec.Executor) (_ *syncMap[ // extend appends to the chain all the blocks in settler's ancestry up to // and including the block that it settled. extend := func(settler *blocks.Block) error { - settleAt := blocks.PreciseTime(rec.hooks, settler.Header()).Add(-saeparams.Tau) + settleAt := rec.hooks.BlockTime(settler.Header()).Add(-saeparams.Tau) tm := proxytime.Of[gas.Gas](settleAt) for { diff --git a/vms/saevm/saexec/execution.go b/vms/saevm/saexec/execution.go index 0c8547dfea98..368b32d7fb15 100644 --- a/vms/saevm/saexec/execution.go +++ b/vms/saevm/saexec/execution.go @@ -159,9 +159,10 @@ func Execute( log.Debug("Executing block") parent := b.ParentBlock() + header := b.Header() gasClock := parent.ExecutedByGasTime().Clone() - gasClock.BeforeBlock(hooks, b.Header()) + gasClock.BeforeBlock(hooks.BlockTime(header)) perTxClock := gasClock.Time.Clone() stateDB, err := sdbo.StateDB(parent.PostExecutionStateRoot()) @@ -176,7 +177,6 @@ func Execute( baseFee := gasClock.BaseFee() b.CheckBaseFeeBound(baseFee) - header := types.CopyHeader(b.Header()) header.BaseFee = baseFee.ToBig() signer := b.Signer(config) @@ -253,7 +253,8 @@ func Execute( } endTime := time.Now() - if err := gasClock.AfterBlock(blockGasConsumed, hooks, b.Header()); err != nil { + target, gasCfg := hooks.GasConfigAfter(b.Header()) + if err := gasClock.AfterBlock(blockGasConsumed, target, gasCfg); err != nil { return nil, fmt.Errorf("after-block gas time update: %w", err) } diff --git a/vms/saevm/worstcase/state.go b/vms/saevm/worstcase/state.go index 35297210c772..f22c99ffe4c5 100644 --- a/vms/saevm/worstcase/state.go +++ b/vms/saevm/worstcase/state.go @@ -117,7 +117,7 @@ func (s *State) StartBlock(h *types.Header) error { ) } - s.clock.BeforeBlock(s.hooks, h) + s.clock.BeforeBlock(s.hooks.BlockTime(h)) s.blockSize = 0 s.maxBlockSize = safeMaxBlockSize(s.clock) @@ -332,7 +332,8 @@ func (s *State) GasUsed() uint64 { // resulted in said transaction being included, which is reflected in the // indexing of tx-sender balances. func (s *State) FinishBlock() (*blocks.WorstCaseBounds, error) { - if err := s.clock.AfterBlock(s.blockSize, s.hooks, s.curr); err != nil { + target, gasCfg := s.hooks.GasConfigAfter(s.curr) + if err := s.clock.AfterBlock(s.blockSize, target, gasCfg); err != nil { return nil, fmt.Errorf("finishing block gas time update: %w", err) } s.qSize += s.blockSize diff --git a/vms/saevm/worstcase/state_test.go b/vms/saevm/worstcase/state_test.go index 85a6c2f1f842..3ab1f37e7973 100644 --- a/vms/saevm/worstcase/state_test.go +++ b/vms/saevm/worstcase/state_test.go @@ -277,7 +277,7 @@ func TestMultipleBlocks(t *testing.T) { Time: block.time, } - wantLatestEndTime.BeforeBlock(sut.hooks, header) + wantLatestEndTime.BeforeBlock(sut.hooks.BlockTime(header)) require.NoErrorf(t, state.StartBlock(header), "StartBlock(%d)", i) require.Equalf(t, block.wantBaseFee, state.BaseFee(), "base fee after StartBlock(%d)", i) require.Equalf(t, block.wantGasLimit, state.GasLimit(), "gas limit after StartBlock(%d)", i) @@ -292,7 +292,8 @@ func TestMultipleBlocks(t *testing.T) { got, err := state.FinishBlock() require.NoError(t, err, "FinishBlock()") - require.NoError(t, wantLatestEndTime.AfterBlock(gas.Gas(state.GasUsed()), sut.hooks, header), "AfterBlock()") + target, c := sut.hooks.GasConfigAfter(header) + require.NoError(t, wantLatestEndTime.AfterBlock(gas.Gas(state.GasUsed()), target, c), "AfterBlock()") want := &blocks.WorstCaseBounds{ MaxBaseFee: block.wantBaseFee,