Skip to content
3 changes: 1 addition & 2 deletions vms/saevm/blocks/execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()")
Expand Down
2 changes: 1 addition & 1 deletion vms/saevm/blocks/time.go
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirming my understanding: PreciseTime() is replaced by hook.Points.BlockTime() and GasTime() is just proxytime.Of(hook.Points.BlockTime())?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PreciseTime is replaced by the hook 👍.
GasTime was never used.

Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ func GasTime(hooks hook.Points, hdr, parent *types.Header) *proxytime.Time[gas.G
target, _ := hooks.GasConfigAfter(parent)
rate := gastime.SafeRateOfTarget(target)
tm := proxytime.New(hdr.Time, rate)
tm.Tick(gastime.SubSecond(hooks, hdr, rate))
tm.Tick(gastime.SubSecond(hooks.SubSecondBlockTime(hdr), rate))
Comment thread
alarso16 marked this conversation as resolved.
Outdated
Comment thread
alarso16 marked this conversation as resolved.
Outdated
return tm
}
5 changes: 0 additions & 5 deletions vms/saevm/gastime/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
28 changes: 12 additions & 16 deletions vms/saevm/gastime/acp176.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,24 @@ 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"
)

// 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) {
// timestamp portions provided. `subSec` must be in [0, second).
func (tm *Time) BeforeBlock(sec uint64, subSec time.Duration) { //nolint:staticcheck // subSec intentionally communicates that the value is < time.Second
Comment thread
alarso16 marked this conversation as resolved.
Outdated
tm.FastForwardTo(
h.Time,
SubSecond(hooks, h, tm.Rate()),
sec,
SubSecond(subSec, tm.Rate()),
)
}

// 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
Expand All @@ -35,8 +32,7 @@ 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 {
if err := cfg.Validate(); err != nil {
return fmt.Errorf("%T.newConfig() after block: %w", tm, err)
Comment thread
alarso16 marked this conversation as resolved.
Outdated
}
if cfg.equals(tm.config) {
Expand All @@ -49,8 +45,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
Expand All @@ -61,7 +57,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
}

Expand All @@ -71,7 +67,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
Expand Down
95 changes: 36 additions & 59 deletions vms/saevm/gastime/acp176_test.go
Comment thread
alarso16 marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@
"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"
"golang.org/x/text/language"
"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
Expand All @@ -29,34 +26,33 @@

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,
},
}

for _, tt := range tests {
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")

Check warning on line 54 in vms/saevm/gastime/acp176_test.go

View workflow job for this annotation

GitHub Actions / Lint

`assert.Equal`" continues on failure; consider require for fail-fast
assert.Equal(t, initialMinPrice, tm.config.MinPrice, "minPrice changed")

Check warning on line 55 in vms/saevm/gastime/acp176_test.go

View workflow job for this annotation

GitHub Actions / Lint

`assert.Equal`" continues on failure; consider require for fail-fast
})
}
}
Expand All @@ -76,13 +72,9 @@
newTime uint64 = initialTime + 1
newTarget = initialTarget + 100_000
)
hook := hookstest.NewStub(newTarget)
header := &types.Header{
Time: newTime,
}

initialPrice := tm.Price()
tm.BeforeBlock(hook, header)
tm.BeforeBlock(newTime, 0)
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
Expand All @@ -98,7 +90,7 @@
expectedEndTime = newTime + secondsOfGasUsed
)
used := initialRate * secondsOfGasUsed
require.NoError(t, tm.AfterBlock(used, hook, header), "AfterBlock()")
require.NoError(t, tm.AfterBlock(used, newTarget, DefaultGasPriceConfig()), "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
Expand All @@ -116,6 +108,7 @@
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())
Expand Down Expand Up @@ -161,24 +154,14 @@
block.limit = max(block.used, block.limit)
block.target = clampTarget(max(block.target, 1))

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)
worstcase.BeforeBlock(block.time, block.nanos)
actual.BeforeBlock(block.time, block.nanos)

// 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()")
}
})
}
Expand Down Expand Up @@ -339,7 +322,7 @@
t.Skip("New target too low")
}

initConfig := hook.GasPriceConfig{
initConfig := GasPriceConfig{
TargetToExcessScaling: gas.Gas(initScaling),
MinPrice: gas.Price(initMinPrice),
StaticPricing: initStaticPricing,
Expand All @@ -353,8 +336,11 @@

{
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 {
Expand All @@ -366,24 +352,24 @@
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
}
Expand Down Expand Up @@ -411,7 +397,7 @@
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()
Expand Down Expand Up @@ -440,15 +426,6 @@
})
}

// 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
Expand Down
4 changes: 2 additions & 2 deletions vms/saevm/gastime/cmpopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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](),
}
}
Loading
Loading