diff --git a/.envrc b/.envrc index 565032a..6083ec6 100644 --- a/.envrc +++ b/.envrc @@ -1,2 +1,2 @@ -export GOROOT="$(go1.23.7 env GOROOT)" -PATH_add "$(go1.23.7 env GOROOT)/bin" \ No newline at end of file +export GOROOT="$(go1.24.7 env GOROOT)" +PATH_add "$(go1.24.7 env GOROOT)/bin" diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 473c709..f0b5a69 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -36,7 +36,7 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: - version: v1.63.3 + version: v1.64.8 yamllint: runs-on: ubuntu-latest diff --git a/.golangci.yml b/.golangci.yml index 8694165..e221766 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -82,6 +82,11 @@ linters-settings: severity: warning disabled: false + testifylint: + enable-all: true + disable: + - require-error # Blanket usage of require over assert is an anti-pattern + issues: include: # Many of the default exclusions are because, verbatim "Annoying issue", diff --git a/blocks/block.go b/blocks/block.go index 8ecf5b7..0df5941 100644 --- a/blocks/block.go +++ b/blocks/block.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + // Package blocks defines [Streaming Asynchronous Execution] (SAE) blocks. // // [Streaming Asynchronous Execution]: https://github.com/avalanche-foundation/ACPs/tree/main/ACPs/194-streaming-asynchronous-execution @@ -6,12 +9,10 @@ package blocks import ( "errors" "fmt" - "math" "runtime" "sync/atomic" "github.com/ava-labs/avalanchego/utils/logging" - "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/types" "go.uber.org/zap" ) @@ -27,12 +28,21 @@ type Block struct { // Rationale: the ancestral pointers form a linked list that would prevent // garbage collection if not severed. Once a block is settled there is no // need to inspect its history so we sacrifice the ancestors to the GC - // Overlord as a sign of our unwavering fealty. - ancestry atomic.Pointer[ancestry] + // Overlord as a sign of our unwavering fealty. See [InMemoryBlockCount] for + // observability. + ancestry atomic.Pointer[ancestry] + // Only the genesis block or the last pre-SAE block is synchronous. These + // are self-settling by definition so their `ancestry` MUST be nil. + synchronous bool + // Non-nil i.f.f. [Block.MarkExecuted] or [Block.ResotrePostExecutionState] + // have returned without error. execution atomic.Pointer[executionResults] - // See [Block.SetInterimExecutionTime for setting and [LastToSettleAt] for - // usage. The pointer MAY be nil if execution is yet to commence. + // Allows this block to be ruled out as able to be settled at a particular + // time (i.e. if this field is >= said time). The pointer MAY be nil if + // execution is yet to commence. For more details, see + // [Block.SetInterimExecutionTime for setting and [LastToSettleAt] for + // usage. executionExceededSecond atomic.Pointer[uint64] executed chan struct{} // closed after `execution` is set @@ -41,11 +51,11 @@ type Block struct { log logging.Logger } -var inMemoryBlockCount atomic.Uint64 +var inMemoryBlockCount atomic.Int64 // InMemoryBlockCount returns the number of blocks created with [New] that are // yet to have their GC finalizers run. -func InMemoryBlockCount() uint64 { +func InMemoryBlockCount() int64 { return inMemoryBlockCount.Load() } @@ -56,12 +66,11 @@ func New(eth *types.Block, parent, lastSettled *Block, log logging.Logger) (*Blo executed: make(chan struct{}), settled: make(chan struct{}), } - // TODO(arr4n) change to runtime.AddCleanup after the Go version has been - // bumped to >=1.24.0. + inMemoryBlockCount.Add(1) - runtime.SetFinalizer(b, func(*Block) { - inMemoryBlockCount.Add(math.MaxUint64) // -1 - }) + runtime.AddCleanup(b, func(struct{}) { + inMemoryBlockCount.Add(-1) + }, struct{}{}) if err := b.setAncestors(parent, lastSettled); err != nil { return nil, err @@ -94,6 +103,10 @@ func (b *Block) setAncestors(parent, lastSettled *Block) error { // CopyAncestorsFrom populates the [Block.ParentBlock] and [Block.LastSettled] // values, typically only required during database recovery. The source block // MUST have the same hash as b. +// +// Although the individual ancestral blocks are shallow copied, calling +// [Block.MarkSettled] on either the source or destination will NOT clear the +// pointers of the other. func (b *Block) CopyAncestorsFrom(c *Block) error { if from, to := c.Hash(), b.Hash(); from != to { return fmt.Errorf("%w: copying internals from block %#x to %#x", errHashMismatch, from, to) @@ -106,10 +119,3 @@ func (b *Block) CopyAncestorsFrom(c *Block) error { // embedded in the [Block]. Use [Block.PostExecutionStateRoot] or // [Block.SettledStateRoot] instead. func (b *Block) Root() {} - -// SettledStateRoot returns the state root after execution of the last block -// settled by b. It is a convenience wrapper for calling [types.Block.Root] on -// the embedded [types.Block]. -func (b *Block) SettledStateRoot() common.Hash { - return b.Block.Root() -} diff --git a/blocks/block_test.go b/blocks/block_test.go index 98ac78d..7053882 100644 --- a/blocks/block_test.go +++ b/blocks/block_test.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package blocks import ( @@ -9,6 +12,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/ava-labs/strevm/saetest" ) func newEthBlock(num, time uint64, parent *types.Block) *types.Block { @@ -24,7 +29,7 @@ func newEthBlock(num, time uint64, parent *types.Block) *types.Block { func newBlock(tb testing.TB, eth *types.Block, parent, lastSettled *Block) *Block { tb.Helper() - b, err := New(eth, parent, lastSettled, logging.NoLog{}) + b, err := New(eth, parent, lastSettled, saetest.NewTBLogger(tb, logging.Warn)) require.NoError(tb, err, "New()") return b } @@ -39,34 +44,41 @@ func newChain(tb testing.TB, startHeight, total uint64, lastSettledAtHeight map[ ) byNum := make(map[uint64]*Block) - if lastSettledAtHeight == nil { - lastSettledAtHeight = make(map[uint64]uint64) - } - for i := range total { n := startHeight + i - var settle *Block + var ( + settle *Block + synchronous bool + ) if s, ok := lastSettledAtHeight[n]; ok { - settle = byNum[s] + if s == n { + require.Zero(tb, s, "Only genesis block is self-settling") + synchronous = true + } else { + require.Less(tb, s, n, "Last-settled height MUST be <= current height") + settle = byNum[s] + } } - byNum[n] = newBlock(tb, newEthBlock(n, n /*time*/, ethParent), parent, settle) - blocks = append(blocks, byNum[n]) + b := newBlock(tb, newEthBlock(n, n /*time*/, ethParent), parent, settle) + byNum[n] = b + blocks = append(blocks, b) + if synchronous { + require.NoError(tb, b.MarkSynchronous(), "MarkSynchronous()") + } parent = byNum[n] - ethParent = parent.Block + ethParent = parent.EthBlock() } return blocks } func TestSetAncestors(t *testing.T) { - t.Parallel() - parent := newBlock(t, newEthBlock(5, 5, nil), nil, nil) lastSettled := newBlock(t, newEthBlock(3, 0, nil), nil, nil) - child := newEthBlock(6, 6, parent.Block) + child := newEthBlock(6, 6, parent.EthBlock()) t.Run("incorrect_parent", func(t *testing.T) { // Note that the arguments to [New] are inverted. @@ -78,8 +90,8 @@ func TestSetAncestors(t *testing.T) { dest := newBlock(t, child, nil, nil) t.Run("destination_before_copy", func(t *testing.T) { - assert.Nilf(t, dest.ParentBlock(), "%T.ParentBlock()") - assert.Nilf(t, dest.LastSettled(), "%T.LastSettled()") + assert.Nilf(t, dest.ParentBlock(), "%T.ParentBlock()", dest) + assert.Nilf(t, dest.LastSettled(), "%T.LastSettled()", dest) }) if t.Failed() { t.FailNow() @@ -91,7 +103,7 @@ func TestSetAncestors(t *testing.T) { } t.Run("incompatible_destination_block", func(t *testing.T) { - ethB := newEthBlock(dest.Height()+1 /*mismatch*/, dest.Time(), parent.Block) + ethB := newEthBlock(dest.Height()+1 /*mismatch*/, dest.BuildTime(), parent.EthBlock()) dest := newBlock(t, ethB, nil, nil) require.ErrorIs(t, dest.CopyAncestorsFrom(source), errHashMismatch) }) diff --git a/blocks/cmpopt.go b/blocks/cmpopt.go index da888fd..1ab9faf 100644 --- a/blocks/cmpopt.go +++ b/blocks/cmpopt.go @@ -1,60 +1,49 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + //go:build !prod && !nocmpopts package blocks import ( - "sync/atomic" + "github.com/google/go-cmp/cmp" - "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/strevm/cmputils" "github.com/ava-labs/strevm/saetest" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" ) // CmpOpt returns a configuration for [cmp.Diff] to compare [Block] instances in // tests. func CmpOpt() cmp.Option { - return cmp.Options{ - cmp.Comparer((*Block).equalForTests), - saetest.CmpBigInts(), - saetest.CmpTimes(), - saetest.CmpReceiptsByMerkleRoot(), - saetest.ComparerOptWithNilCheck(func(a, b *types.Block) bool { - return a.Hash() == b.Hash() - }), - cmpopts.EquateComparable( - // We're not running tests concurrently with anything that will - // modify [Block.accepted] nor [Block.executed] so this is safe. - // Using a [cmp.Transformer] would make the linter complain about - // copying. - atomic.Bool{}, - ), - } + return cmp.Comparer((*Block).equalForTests) } func (b *Block) equalForTests(c *Block) bool { - fn := saetest.ComparerWithNilCheck(func(b, c *Block) bool { - if b.Hash() != c.Hash() { - return false - } - - fn := saetest.ComparerWithNilCheck(func(b, c *ancestry) bool { - return b.parent.equalForTests(c.parent) && b.lastSettled.equalForTests(c.lastSettled) - }) - if !fn(b.ancestry.Load(), c.ancestry.Load()) { - return false - } - - return b.execution.Load().equalForTests(c.execution.Load()) + fn := cmputils.WithNilCheck(func(b, c *Block) bool { + return true && + b.Hash() == c.Hash() && + b.ancestry.Load().equalForTests(c.ancestry.Load()) && + b.execution.Load().equalForTests(c.execution.Load()) }) return fn(b, c) } +func (a *ancestry) equalForTests(b *ancestry) bool { + fn := cmputils.WithNilCheck(func(a, b *ancestry) bool { + return true && + a.parent.equalForTests(b.parent) && + a.lastSettled.equalForTests(b.lastSettled) + }) + return fn(a, b) +} + func (e *executionResults) equalForTests(f *executionResults) bool { - fn := saetest.ComparerWithNilCheck(func(e, f *executionResults) bool { - return e.byGas.Compare(f.byGas.Time) == 0 && - e.gasUsed == f.gasUsed && + fn := cmputils.WithNilCheck(func(e, f *executionResults) bool { + return true && + e.byGas.Rate() == f.byGas.Rate() && + e.byGas.Compare(f.byGas.Time) == 0 && // N.B. Compare is only valid if rates are equal e.receiptRoot == f.receiptRoot && + saetest.MerkleRootsEqual(e.receipts, f.receipts) && e.stateRootPost == f.stateRootPost }) return fn(e, f) diff --git a/blocks/execution.go b/blocks/execution.go index bd46c19..fa6b08b 100644 --- a/blocks/execution.go +++ b/blocks/execution.go @@ -1,8 +1,13 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package blocks import ( "context" + "errors" "fmt" + "math/big" "slices" "time" @@ -15,6 +20,7 @@ import ( "github.com/ava-labs/strevm/gastime" "github.com/ava-labs/strevm/hook" "github.com/ava-labs/strevm/proxytime" + "go.uber.org/zap" ) //go:generate go run github.com/StephenButtolph/canoto/canoto $GOFILE @@ -23,21 +29,22 @@ import ( // transactions, with the highest-known gas time. This MAY be at any resolution // but MUST be monotonic. func (b *Block) SetInterimExecutionTime(t *proxytime.Time[gas.Gas]) { - p := t.Unix() + sec := t.Unix() if t.Fraction().Numerator == 0 { - p-- + sec-- } - b.executionExceededSecond.Store(&p) + b.executionExceededSecond.Store(&sec) } type executionResults struct { byGas gastime.Time `canoto:"value,1"` byWall time.Time // For metrics only; allowed to be incorrect. + baseFee *big.Int // Receipts are deliberately not stored by the canoto representation as they - // are already in the database. Only [Block.RestorePostExecutionState] reads - // the stored canoto, also accepting a [types.Receipts] argument that it - // checks against `receiptRoot`. + // are already in the database. All methods that read the stored canoto + // either accept a [types.Receipts] for comparison against the + // `receiptRoot`, or don't care about receipts at all. receipts types.Receipts receiptRoot common.Hash `canoto:"fixed bytes,2"` gasUsed gas.Gas `canoto:"uint,3"` @@ -46,20 +53,22 @@ type executionResults struct { canotoData canotoData_executionResults } -// MarkExecuted marks the block as having being executed at the specified -// time(s) and with the specified results. It also sets the chain's head block -// to b. +// MarkExecuted marks the block as having been executed at the specified time(s) +// and with the specified results. It also sets the chain's head block to b. // // MarkExecuted guarantees that state is persisted to the database before // in-memory indicators of execution are updated. [Block.Executed] returning -// true is therefore indicative of a successful database write by MarkExecuted. +// true and [Block.WaitUntilExecuted] returning cleanly are both therefore +// indicative of a successful database write by MarkExecuted. // -// This function MUST NOT be called more than once. The wall-clock [time.Time] -// is for metrics only. +// This method MUST NOT be called more than once and its usage is mutually +// exclusive of [Block.RestorePostExecutionState]. The wall-clock [time.Time] is +// for metrics only. func (b *Block) MarkExecuted( db ethdb.Database, byGas *gastime.Time, byWall time.Time, + baseFee *big.Int, receipts types.Receipts, stateRootPost common.Hash, hooks hook.Points, @@ -72,6 +81,7 @@ func (b *Block) MarkExecuted( e := &executionResults{ byGas: *byGas.Clone(), byWall: byWall, + baseFee: new(big.Int).Set(baseFee), receipts: slices.Clone(receipts), gasUsed: used, receiptRoot: types.DeriveSha(receipts, trie.NewStackTrie(nil)), @@ -79,9 +89,10 @@ func (b *Block) MarkExecuted( } batch := db.NewBatch() - rawdb.WriteHeadBlockHash(batch, b.Hash()) - rawdb.WriteHeadHeaderHash(batch, b.Hash()) - rawdb.WriteReceipts(batch, b.Hash(), b.NumberU64(), receipts) + hash := b.Hash() + rawdb.WriteHeadBlockHash(batch, hash) + rawdb.WriteHeadHeaderHash(batch, hash) + rawdb.WriteReceipts(batch, hash, b.NumberU64(), receipts) if err := b.writePostExecutionState(batch, e); err != nil { return err } @@ -96,16 +107,21 @@ func (b *Block) MarkExecuted( return b.markExecuted(e) } +var errMarkBlockExecutedAgain = errors.New("block re-marked as executed") + func (b *Block) markExecuted(e *executionResults) error { if !b.execution.CompareAndSwap(nil, e) { - b.log.Error("Block re-marked as executed") - return fmt.Errorf("block %d re-marked as executed", b.Height()) + // This is fatal because we corrupted the database's head block if we + // got here by [Block.MarkExecuted] being called twice (an invalid use + // of the API). + b.log.Fatal("Block re-marked as executed") + return fmt.Errorf("%w: height %d", errMarkBlockExecutedAgain, b.Height()) } close(b.executed) return nil } -// WaitUntilExecuted blocks until either [Block.MarkExecuted] is called or the +// WaitUntilExecuted blocks until [Block.MarkExecuted] is called or the // [context.Context] is cancelled. func (b *Block) WaitUntilExecuted(ctx context.Context) error { select { @@ -116,50 +132,60 @@ func (b *Block) WaitUntilExecuted(ctx context.Context) error { } } -// Executed reports whether [Block.MarkExecuted] has been called and returned -// without error. +// Executed reports whether [Block.MarkExecuted] has been called without +// resulting in an error. func (b *Block) Executed() bool { return b.execution.Load() != nil } -func zero[T any]() (z T) { return } +func executionArtefact[T any](b *Block, desc string, get func(*executionResults) T) T { + e := b.execution.Load() + if e == nil { + b.log.Error("execution artefact requested before execution", + zap.String("artefact", desc), + ) + var zero T + return zero + } + return get(e) +} // ExecutedByGasTime returns a clone of the gas time passed to // [Block.MarkExecuted] or nil if no such successful call has been made. func (b *Block) ExecutedByGasTime() *gastime.Time { - if e := b.execution.Load(); e != nil { + return executionArtefact(b, "execution (gas) time", func(e *executionResults) *gastime.Time { return e.byGas.Clone() - } - b.log.Error("Get block execution (gas) time before execution") - return nil + }) } // ExecutedByWallTime returns the wall time passed to [Block.MarkExecuted] or // the zero time if no such successful call has been made. func (b *Block) ExecutedByWallTime() time.Time { - if e := b.execution.Load(); e != nil { + return executionArtefact(b, "execution (wall) time", func(e *executionResults) time.Time { return e.byWall - } - b.log.Error("Get block execution (wall) time before execution") - return zero[time.Time]() + }) +} + +// BaseFee returns the base gas price passed to [Block.MarkExecuted] or nil if +// no such successful call has been made. +func (b *Block) BaseFee() *big.Int { + return executionArtefact(b, "receipts", func(e *executionResults) *big.Int { + return new(big.Int).Set(e.baseFee) + }) } // Receipts returns the receipts passed to [Block.MarkExecuted] or nil if no // such successful call has been made. func (b *Block) Receipts() types.Receipts { - if e := b.execution.Load(); e != nil { + return executionArtefact(b, "receipts", func(e *executionResults) types.Receipts { return slices.Clone(e.receipts) - } - b.log.Error("Get block receipts before execution") - return nil + }) } // PostExecutionStateRoot returns the state root passed to [Block.MarkExecuted] // or the zero hash if no such successful call has been made. func (b *Block) PostExecutionStateRoot() common.Hash { - if e := b.execution.Load(); e != nil { + return executionArtefact(b, "state root", func(e *executionResults) common.Hash { return e.stateRootPost - } - b.log.Error("Get block state root before execution") - return zero[common.Hash]() + }) } diff --git a/blocks/execution_test.go b/blocks/execution_test.go new file mode 100644 index 0000000..7bb019a --- /dev/null +++ b/blocks/execution_test.go @@ -0,0 +1,146 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package blocks + +import ( + "context" + "fmt" + "math/big" + "testing" + "time" + + "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/rawdb" + "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/libevm/ethdb" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/ava-labs/strevm/gastime" + "github.com/ava-labs/strevm/hook/hooktest" + "github.com/ava-labs/strevm/saetest" +) + +// markExecutedForTests calls [Block.MarkExecuted] with zero-value +// post-execution artefacts (other than the gas time). +func (b *Block) markExecutedForTests(tb testing.TB, db ethdb.Database, tm *gastime.Time) { + tb.Helper() + require.NoError(tb, b.MarkExecuted(db, tm, time.Time{}, new(big.Int), nil, common.Hash{}, hooktest.Simple{}), "MarkExecuted()") +} + +func TestMarkExecuted(t *testing.T) { + txs := make(types.Transactions, 10) + for i := range txs { + txs[i] = types.NewTx(&types.LegacyTx{ + Nonce: uint64(i), //nolint:gosec // Won't overflow + }) + } + + ethB := types.NewBlock( + &types.Header{ + Number: big.NewInt(1), + Time: 42, + }, + txs, + nil, nil, // uncles, receipts + saetest.TrieHasher(), + ) + db := rawdb.NewMemoryDatabase() + rawdb.WriteBlock(db, ethB) + + settles := newBlock(t, newEthBlock(0, 0, nil), nil, nil) + settles.markExecutedForTests(t, db, gastime.New(0, 1, 0)) + b := newBlock(t, ethB, nil, settles) + + t.Run("before_MarkExecuted", func(t *testing.T) { + require.False(t, b.Executed(), "Executed()") + require.NoError(t, b.CheckInvariants(NotExecuted), "CheckInvariants(NotExecuted)") + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + require.ErrorIs(t, context.DeadlineExceeded, b.WaitUntilExecuted(ctx), "WaitUntilExecuted()") + + rec := saetest.NewLogRecorder(logging.Warn) + b.log = rec + assertNumErrorLogs := func(t *testing.T, want int) { + t.Helper() + assert.Len(t, rec.At(logging.Error), want, "Number of ERROR logs") + } + + tests := []struct { + method string + call func() any + }{ + {"ExecutedByGasTime()", func() any { return b.ExecutedByGasTime() }}, + {"BaseFee()", func() any { return b.BaseFee() }}, + {"Receipts()", func() any { return b.Receipts() }}, + {"PostExecutionStateRoot()", func() any { return b.PostExecutionStateRoot() }}, + } + for i, tt := range tests { + assert.Zero(t, tt.call(), tt.method) + assertNumErrorLogs(t, i+1) + } + }) + + gasTime := gastime.New(42, 1e6, 42) + wallTime := time.Unix(42, 100) + stateRoot := common.Hash{'s', 't', 'a', 't', 'e'} + baseFee := big.NewInt(314159) + var receipts types.Receipts + for _, tx := range txs { + receipts = append(receipts, &types.Receipt{ + TxHash: tx.Hash(), + }) + } + require.NoError(t, b.MarkExecuted(db, gasTime, wallTime, baseFee, receipts, stateRoot, hooktest.Simple{}), "MarkExecuted()") + + t.Run("after_MarkExecuted", func(t *testing.T) { + require.True(t, b.Executed(), "Executed()") + assert.NoError(t, b.CheckInvariants(Executed), "CheckInvariants(Executed)") + + require.NoError(t, b.WaitUntilExecuted(context.Background()), "WaitUntilExecuted()") + + assert.Zero(t, b.ExecutedByGasTime().Compare(gasTime.Time), "ExecutedByGasTime().Compare([original input])") + assert.Zero(t, b.BaseFee().Cmp(baseFee), "BaseFee().Cmp([original input])") + assert.Empty(t, cmp.Diff(receipts, b.Receipts(), saetest.CmpByMerkleRoots[types.Receipts]()), "Receipts()") + + assert.Equal(t, stateRoot, b.PostExecutionStateRoot(), "PostExecutionStateRoot()") // i.e. this block + // Although not directly relevant to MarkExecuted, demonstrate that the + // two notion's of a state root are in fact different. + assert.Equal(t, settles.EthBlock().Root(), b.SettledStateRoot(), "SettledStateRoot()") // i.e. the block this block settles + assert.NotEqual(t, b.SettledStateRoot(), b.PostExecutionStateRoot(), "PostExecutionStateRoot() != SettledStateRoot()") + + t.Run("MarkExecuted_again", func(t *testing.T) { + rec := saetest.NewLogRecorder(logging.Warn) + b.log = rec + assert.ErrorIs(t, b.MarkExecuted(db, gasTime, wallTime, baseFee, receipts, stateRoot, hooktest.Simple{}), errMarkBlockExecutedAgain) + // The database's head block might have been corrupted so this MUST + // be a fatal action. + assert.Len(t, rec.At(logging.Fatal), 1, "FATAL logs") + }) + }) + + t.Run("database", func(t *testing.T) { + t.Run("head_block", func(t *testing.T) { + for fn, got := range map[string]interface{ Hash() common.Hash }{ + "ReadHeadBlockHash": selfAsHasher(rawdb.ReadHeadBlockHash(db)), + "ReadHeadHeaderHash": selfAsHasher(rawdb.ReadHeadHeaderHash(db)), + "ReadHeadBlock": rawdb.ReadHeadBlock(db), + "ReadHeadHeader": rawdb.ReadHeadHeader(db), + } { + t.Run(fmt.Sprintf("rawdb.%s", fn), func(t *testing.T) { + require.NotNil(t, got) + assert.Equalf(t, b.Hash(), got.Hash(), "rawdb.%s()", fn) + }) + } + }) + }) +} + +// selfAsHasher adds a Hash() method to a common.Hash, returning itself. +type selfAsHasher common.Hash + +func (h selfAsHasher) Hash() common.Hash { return common.Hash(h) } diff --git a/blocks/export.go b/blocks/export.go new file mode 100644 index 0000000..5f552f6 --- /dev/null +++ b/blocks/export.go @@ -0,0 +1,47 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package blocks + +import ( + "math/big" + + "github.com/ava-labs/libevm/common" + "github.com/ava-labs/libevm/core/types" +) + +// While an argument can be made for embedding the [types.Block] in [Block], +// instead of aliasing methods, that risks incorrect usage of subtle differences +// under SAE. These methods are direct aliases i.f.f. the interpretation is +// unambiguous, otherwise their names are clarified (e.g. +// [Block.SettledStateRoot]). + +// EthBlock returns the raw EVM block wrapped by b. Prefer accessing its +// properties via the methods aliased on [Block] as some (e.g. +// [types.Block.Root]) have ambiguous interpretation under SAE. +func (b *Block) EthBlock() *types.Block { return b.Block } + +// SettledStateRoot returns the state root after execution of the last block +// settled by b. It is a convenience wrapper for calling [types.Block.Root] on +// the wrapped [types.Block]. +func (b *Block) SettledStateRoot() common.Hash { + return b.Block.Root() +} + +// BuildTime returns the Unix timestamp of the block, which is the canonical +// inclusion time of its transactions; see [Block.ExecutedByGasTime] for their +// execution timestamp. BuildTime is a convenience wrapper for calling +// [types.Block.Time] on the wrapped [types.Block]. +func (b *Block) BuildTime() uint64 { return b.Block.Time() } + +// Hash returns [types.Block.Hash] from the wrapped [types.Block]. +func (b *Block) Hash() common.Hash { return b.Block.Hash() } + +// ParentHash returns [types.Block.ParentHash] from the wrapped [types.Block]. +func (b *Block) ParentHash() common.Hash { return b.Block.ParentHash() } + +// NumberU64 returns [types.Block.NumberU64] from the wrapped [types.Block]. +func (b *Block) NumberU64() uint64 { return b.Block.NumberU64() } + +// Number returns [types.Block.Number] from the wrapped [types.Block]. +func (b *Block) Number() *big.Int { return b.Block.Number() } diff --git a/blocks/invariants.go b/blocks/invariants.go index 917356e..29494ea 100644 --- a/blocks/invariants.go +++ b/blocks/invariants.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package blocks import ( @@ -7,25 +10,34 @@ import ( "github.com/ava-labs/libevm/trie" ) -type brokenInvariant struct { - b *Block - msg string -} +// A LifeCycleStage defines the progression of a block from acceptance through +// to settlement. +type LifeCycleStage int + +// Valid [LifeCycleStage] values. Blocks proceed in increasing stage numbers, +// but specific values MUST NOT be relied upon to be stable. +const ( + NotExecuted LifeCycleStage = iota + Executed + Settled + + Accepted = NotExecuted +) -func (err brokenInvariant) Error() string { - return fmt.Sprintf("block %d: %s", err.b.Height(), err.msg) +func (b *Block) brokenInvariantErr(msg string) error { + return fmt.Errorf("block %d: %s", b.Height(), msg) } -// CheckInvariants checks internal invariants against expected state, typically -// only necessary during database recovery. -func (b *Block) CheckInvariants(expectExecuted, expectSettled bool) error { +// CheckInvariants checks internal invariants against expected stage, typically +// only used during database recovery. +func (b *Block) CheckInvariants(expect LifeCycleStage) error { switch e := b.execution.Load(); e { case nil: // not executed - if expectExecuted { + if expect >= Executed { return b.brokenInvariantErr("expected to be executed") } default: // executed - if !expectExecuted { + if expect < Executed { return b.brokenInvariantErr("unexpectedly executed") } if e.receiptRoot != types.DeriveSha(e.receipts, trie.NewStackTrie(nil)) { @@ -35,11 +47,11 @@ func (b *Block) CheckInvariants(expectExecuted, expectSettled bool) error { switch a := b.ancestry.Load(); a { case nil: // settled - if !expectSettled { + if expect < Settled { return b.brokenInvariantErr("unexpectedly settled") } default: // not settled - if expectSettled { + if expect >= Settled { return b.brokenInvariantErr("expected to be settled") } if b.SettledStateRoot() != b.LastSettled().PostExecutionStateRoot() { @@ -49,7 +61,3 @@ func (b *Block) CheckInvariants(expectExecuted, expectSettled bool) error { return nil } - -func (b *Block) brokenInvariantErr(msg string) error { - return brokenInvariant{b: b, msg: msg} -} diff --git a/blocks/settlement.go b/blocks/settlement.go index 5e70e34..786d84e 100644 --- a/blocks/settlement.go +++ b/blocks/settlement.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package blocks import ( @@ -5,17 +8,18 @@ import ( "errors" "fmt" "slices" - - "go.uber.org/zap" ) type ancestry struct { parent, lastSettled *Block } -var errBlockResettled = errors.New("block re-settled") +var ( + errBlockResettled = errors.New("block re-settled") + errBlockAncestryChanged = errors.New("block ancestry changed during settlement") +) -// MarkSettled marks the block as having being settled. This function MUST NOT +// MarkSettled marks the block as having been settled. This function MUST NOT // be called more than once. // // After a call to MarkSettled, future calls to [Block.ParentBlock] and @@ -26,14 +30,37 @@ func (b *Block) MarkSettled() error { b.log.Error(errBlockResettled.Error()) return fmt.Errorf("%w: block height %d", errBlockResettled, b.Height()) } - if b.ancestry.CompareAndSwap(a, nil) { - close(b.settled) - return nil + if !b.ancestry.CompareAndSwap(a, nil) { // almost certainly means concurrent calls to this method + b.log.Fatal("Block ancestry changed during settlement") + // We have to return something to keen the compiler happy, even though we + // expect the Fatal to be, well, fatal. + return errBlockAncestryChanged } - b.log.Fatal("Block ancestry changed") - // We have to return something to keen the compiler happy, even though we - // expect the Fatal to be, well, fatal. - return errors.New("block ancestry changed") + close(b.settled) + return nil +} + +// IsSettled returns true if the block has been settled. +func (b *Block) IsSettled() bool { + return b.ancestry.Load() == nil +} + +// MarkSynchronous is a special case of [Block.MarkSettled], reserved for the +// last pre-SAE block, which MAY be the genesis block. These are, by definition, +// self-settling so require special treatment as such behaviour is impossible +// under SAE rules. +// +// Wherever MarkSynchronous results in different behaviour to +// [Block.MarkSettled], the respective methods are documented as such. They can +// otherwise be considered identical. +func (b *Block) MarkSynchronous() error { + b.synchronous = true + return b.MarkSettled() +} + +// IsSynchronous returns true if the block was settled synchronously. +func (b *Block) IsSynchronous() bool { + return b.synchronous } // WaitUntilSettled blocks until either [Block.MarkSettled] is called or the @@ -47,54 +74,78 @@ func (b *Block) WaitUntilSettled(ctx context.Context) error { } } +func (b *Block) ancestor(ifSettledErrMsg string, get func(*ancestry) *Block) *Block { + a := b.ancestry.Load() + if a == nil { + b.log.Error(ifSettledErrMsg) + return nil + } + return get(a) +} + const ( - getParentOfSettledMsg = "Get parent of settled block" - getSettledOfSettledMsg = "Get last-settled of settled block" + getParentOfSettledErrMsg = "Get parent of settled block" + getSettledOfSettledErrMsg = "Get last-settled of settled block" ) // ParentBlock returns the block's parent unless [Block.MarkSettled] has been -// called, in which case it returns nil. +// called, in which case it returns nil and logs an error. func (b *Block) ParentBlock() *Block { - if a := b.ancestry.Load(); a != nil { + return b.ancestor(getParentOfSettledErrMsg, func(a *ancestry) *Block { return a.parent - } - b.log.Debug(getParentOfSettledMsg) - return nil + }) } // LastSettled returns the last-settled block at the time of b's acceptance, -// unless [Block.MarkSettled] has been called, in which case it returns nil. +// unless [Block.MarkSettled] has been called, in which case it returns nil and +// logs an error. If [Block.MarkSynchronous] was called instead, LastSettled +// always returns `b` itself, without logging. Note that this value might not be +// distinct between contiguous blocks. func (b *Block) LastSettled() *Block { - if a := b.ancestry.Load(); a != nil { - return a.lastSettled + if b.synchronous { + return b } - b.log.Error( - getSettledOfSettledMsg, - zap.Stack("stacktrace"), - ) - return nil + return b.ancestor(getSettledOfSettledErrMsg, func(a *ancestry) *Block { + return a.lastSettled + }) } // Settles returns the executed blocks that b settles if it is accepted by // consensus. If `x` is the block height of the `b.ParentBlock().LastSettled()` // and `y` is the height of the `b.LastSettled()`, then Settles returns the -// contiguous, half-open range (x,y] or an empty slice i.f.f. x==y. +// contiguous, half-open range (x,y] or an empty slice i.f.f. x==y. Every block +// therefore returns a disjoint (and possibly empty) set of historical blocks. // // It is not valid to call Settles after a call to [Block.MarkSettled] on either -// b or its parent. +// b or its parent. If [Block.MarkSynchronous] was called instead, Settles +// always returns a single-element slice of `b` itself. func (b *Block) Settles() []*Block { - return b.ParentBlock().IfChildSettles(b.LastSettled()) + if b.synchronous { + return []*Block{b} + } + return settling(b.ParentBlock().LastSettled(), b.LastSettled()) } -// IfChildSettles is similar to [Block.Settles] but with different definitions +// WhenChildSettles returns the blocks that would be settled by a child of `b`, +// given the last-settled block at that child's block time. Note that the +// last-settled block at the child's time MAY be equal to the last-settled of +// `b` (its parent), in which case WhenChildSettles returns an empty slice. +// +// The argument is typically the return value of [LastToSettleAt], where that +// function receives `b` as the parent. See the Example. +// +// WhenChildSettles MUST only be called before the call to [Block.MarkSettled] +// on `b`. The intention is that this method is called on the VM's preferred +// block, which always meets this criterion. This is by definition of +// settlement, which requires that at least one descendant block has already +// been accepted, which the preference never has. +// +// WhenChildSettles is similar to [Block.Settles] but with different definitions // of `x` and `y` (as described in [Block.Settles]). It is intended for use // during block building and defines `x` as the block height of // `b.LastSettled()` while `y` as the height of the argument passed to this // method. -// -// The argument is typically the return value of [LastToSettleAt], where that -// function receives b as the parent. See the Example. -func (b *Block) IfChildSettles(lastSettledOfChild *Block) []*Block { +func (b *Block) WhenChildSettles(lastSettledOfChild *Block) []*Block { return settling(b.LastSettled(), lastSettledOfChild) } @@ -104,58 +155,81 @@ func (b *Block) IfChildSettles(lastSettledOfChild *Block) []*Block { // arguments have the same block hash. func settling(lastOfParent, lastOfCurr *Block) []*Block { var settling []*Block - for s := lastOfCurr; s.ParentBlock() != nil && s.Hash() != lastOfParent.Hash(); s = s.ParentBlock() { + // TODO(arr4n) abstract this to combine functionality with iterators + // introduced by @StephenButtolph. + for s := lastOfCurr; s.Hash() != lastOfParent.Hash(); s = s.ParentBlock() { settling = append(settling, s) } slices.Reverse(settling) return settling } -// LastToSettleAt returns the last block to be settled at time `settleAt` if -// building on the specified parent block, and a boolean to indicate if +// LastToSettleAt returns (a) the last block to be settled at time `settleAt` if +// building on the specified parent block, and (b) a boolean to indicate if // settlement is currently possible. If the returned boolean is false, the // execution stream is lagging and LastToSettleAt can be called again after some // indeterminate delay. // -// See the Example for [Block.IfChildSettles] for one usage of the returned +// See the Example for [Block.WhenChildSettles] for one usage of the returned // block. -func LastToSettleAt(settleAt uint64, parent *Block) (*Block, bool) { - // These variables are only abstracted for clarity; they are not needed - // beyond the scope of the `for` loop. - var block, child *Block - block = parent // therefore `child` remains nil - - // The only way [Block.parent] can be nil is if it was already settled (see - // invariant in [Block]). If a block was already settled then only that or a - // later (i.e. unsettled) block can be returned by this loop, therefore we - // have a guarantee that the loop update will never result in `block==nil`. - // Framed differently, because `settleAt` is >= what it was when this - // function was used to build `parent`, if `block.parent==nil` then it will - // have already been settled in `parent` and therefore executed by - // `<=settleAt` so will be returned here. - for ; ; block, child = block.ParentBlock(), block { - if block.Time() > settleAt { +func LastToSettleAt(settleAt uint64, parent *Block) (b *Block, ok bool) { + defer func() { + // Avoids having to perform this check at every return. + if !ok { + b = nil + } + }() + + // A block can be the last to settle at some time i.f.f. two criteria are + // met: + // + // 1. The block has finished execution by said time and; + // + // 2. The block's child is known to have *not* finished execution or be + // unable to finish by that time. + // + // The block currently being built can never finish in time, so we start + // with criterion (2) being met. + known := true + + // The only way [Block.ParentBlock] can be nil is if `block` was already + // settled (see invariant in [Block]). If a block was already settled then + // only that or a later (i.e. unsettled) block can be returned by this loop, + // therefore we have a guarantee that the loop update will never result in + // `block==nil`. + for block := parent; ; block = block.ParentBlock() { + // Guarantees that the loop will always exit as the last pre-SAE block + // (perhaps the genesis) is always settled, by definition. + if settled := block.ancestry.Load() == nil; settled { + return block, known + } + + if startsNoEarlierThan := block.BuildTime(); startsNoEarlierThan > settleAt { + known = true continue } + // TODO(arr4n) more fine-grained checks are possible by computing the + // minimum possible gas consumption of blocks. For example, + // `block.BuildTime()+block.intrinsicGasSum()` can be compared against + // `settleAt`, as can the sum of a chain of blocks. if t := block.executionExceededSecond.Load(); t != nil && *t >= settleAt { + known = true continue } if e := block.execution.Load(); e != nil { if e.byGas.CompareUnix(settleAt) > 0 { - // Although this check is redundant because of the similar one - // just above, it's fast so there's no harm in double-checking. + // There may have been a race between this check and the + // execution-exceeded one above, so we have to check again. + known = true continue } - return block, true + return block, known } - // TODO(arr4n) more fine-grained checks are possible for scenarios where - // (a) `block` could never execute before `settleAt` so we would - // `continue`; and (b) `block` will definitely execute in time and - // `child` could never, in which case return `nil, false`. - _ = child - - return nil, false + // Note that a grandchild block having unknown execution completion time + // does not rule out knowing a child's completion time, so this could be + // set to true in a future loop iteration. + known = false } } diff --git a/blocks/settlement_test.go b/blocks/settlement_test.go index e3ada4b..3232a59 100644 --- a/blocks/settlement_test.go +++ b/blocks/settlement_test.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package blocks import ( @@ -8,71 +11,43 @@ import ( "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/vms/components/gas" - "github.com/ava-labs/libevm/common" "github.com/ava-labs/libevm/core/rawdb" - "github.com/ava-labs/libevm/core/types" - "github.com/ava-labs/libevm/ethdb" - "github.com/ava-labs/strevm/gastime" - "github.com/ava-labs/strevm/hook/hooktest" - "github.com/ava-labs/strevm/proxytime" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/zap" + + "github.com/ava-labs/strevm/gastime" + "github.com/ava-labs/strevm/proxytime" + "github.com/ava-labs/strevm/saetest" ) -func ExampleBlock_IfChildSettles() { +//nolint:testableexamples // Output is meaningless +func ExampleBlock_WhenChildSettles() { parent := blockBuildingPreference() - settle, ok := LastToSettleAt(uint64(time.Now().Unix()), parent) + settle, ok := LastToSettleAt(uint64(time.Now().Unix()), parent) //nolint:gosec // Time won't overflow for quite a while if !ok { return // execution is lagging; please come back soon } - // Returns the (possibly empty) slice of blocks that may be settled by the + // Returns the (possibly empty) slice of blocks that would be settled by the // block being built. - _ = parent.IfChildSettles(settle) -} - -func blockBuildingPreference() *Block { - b, _ := New(&types.Block{}, nil, nil, nil) - return b + _ = parent.WhenChildSettles(settle) } -type logRecorder struct { - logging.NoLog - records []logRecord -} - -type logRecord struct { - level logging.Level - msg string - fields []zap.Field -} - -func (l *logRecorder) Error(msg string, fields ...zap.Field) { - l.records = append(l.records, logRecord{ - level: logging.Error, - msg: msg, - fields: fields, - }) -} - -func (l *logRecorder) Fatal(msg string, fields ...zap.Field) { - l.records = append(l.records, logRecord{ - level: logging.Fatal, - msg: msg, - fields: fields, - }) -} +// blockBuildingPreference exists only to allow examples to build. +func blockBuildingPreference() *Block { return nil } func TestSettlementInvariants(t *testing.T) { - t.Parallel() - parent := newBlock(t, newEthBlock(5, 5, nil), nil, nil) lastSettled := newBlock(t, newEthBlock(3, 3, nil), nil, nil) - b := newBlock(t, newEthBlock(0, 0, parent.Block), parent, lastSettled) + b := newBlock(t, newEthBlock(6, 10, parent.EthBlock()), parent, lastSettled) + + db := rawdb.NewMemoryDatabase() + for _, b := range []*Block{b, parent, lastSettled} { + b.markExecutedForTests(t, db, gastime.New(b.BuildTime(), 1, 0)) + } t.Run("before_MarkSettled", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) @@ -81,6 +56,7 @@ func TestSettlementInvariants(t *testing.T) { assert.True(t, b.ParentBlock().equalForTests(parent), "ParentBlock().equalForTests([constructor arg])") assert.True(t, b.LastSettled().equalForTests(lastSettled), "LastSettled().equalForTests([constructor arg])") + assert.NoError(t, b.CheckInvariants(Executed), "CheckInvariants(Executed)") }) if t.Failed() { t.FailNow() @@ -90,35 +66,40 @@ func TestSettlementInvariants(t *testing.T) { t.Run("after_MarkSettled", func(t *testing.T) { assert.NoError(t, b.WaitUntilSettled(context.Background()), "WaitUntilSettled()") + assert.NoError(t, b.CheckInvariants(Settled), "CheckInvariants(Settled)") - var rec logRecorder - b.log = &rec + rec := saetest.NewLogRecorder(logging.Warn) + b.log = rec + assertNumErrorLogs := func(t *testing.T, want int) { + t.Helper() + assert.Len(t, rec.At(logging.Error), want, "Number of ERROR") + } assert.Nil(t, b.ParentBlock(), "ParentBlock()") - assert.Len(t, rec.records, 1, "Number of ERROR or FATAL logs") + assertNumErrorLogs(t, 1) assert.Nil(t, b.LastSettled(), "LastSettled()") - assert.Len(t, rec.records, 2, "Number of ERROR or FATAL logs") + assertNumErrorLogs(t, 2) assert.ErrorIs(t, b.MarkSettled(), errBlockResettled, "second call to MarkSettled()") - assert.Len(t, rec.records, 3, "Number of ERROR or FATAL logs") + assertNumErrorLogs(t, 3) if t.Failed() { t.FailNow() } - want := []logRecord{ + want := []*saetest.LogRecord{ { - level: logging.Error, - msg: getParentOfSettledMsg, + Level: logging.Error, + Msg: getParentOfSettledErrMsg, }, { - level: logging.Error, - msg: getSettledOfSettledMsg, + Level: logging.Error, + Msg: getSettledOfSettledErrMsg, }, { - level: logging.Error, - msg: errBlockResettled.Error(), + Level: logging.Error, + Msg: errBlockResettled.Error(), }, } - if diff := cmp.Diff(want, rec.records, cmp.AllowUnexported(logRecord{})); diff != "" { + if diff := cmp.Diff(want, rec.AtLeast(logging.Error)); diff != "" { t.Errorf("ERROR + FATAL logs diff (-want +got):\n%s", diff) } }) @@ -138,7 +119,7 @@ func TestSettles(t *testing.T) { 9: 7, } wantSettles := map[uint64][]uint64{ - // It is not valid to call Settles() on the genesis block + 0: {0}, 1: nil, 2: nil, 3: nil, @@ -175,27 +156,27 @@ func TestSettles(t *testing.T) { for _, b := range blocks[1:] { tests = append(tests, testCase{ - name: fmt.Sprintf("Block(%d).IfChildSettles([same as parent])", b.Height()), - got: b.IfChildSettles(b.LastSettled()), + name: fmt.Sprintf("Block(%d).WhenChildSettles([same as parent])", b.Height()), + got: b.WhenChildSettles(b.LastSettled()), want: nil, }) } tests = append(tests, []testCase{ { - got: blocks[7].IfChildSettles(blocks[3]), + got: blocks[7].WhenChildSettles(blocks[3]), want: nil, }, { - got: blocks[7].IfChildSettles(blocks[4]), + got: blocks[7].WhenChildSettles(blocks[4]), want: numsToBlocks(4), }, { - got: blocks[7].IfChildSettles(blocks[5]), + got: blocks[7].WhenChildSettles(blocks[5]), want: numsToBlocks(4, 5), }, { - got: blocks[7].IfChildSettles(blocks[6]), + got: blocks[7].WhenChildSettles(blocks[6]), want: numsToBlocks(4, 5, 6), }, }...) @@ -203,23 +184,18 @@ func TestSettles(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if diff := cmp.Diff(tt.want, tt.got, cmpopts.EquateEmpty(), CmpOpt()); diff != "" { - t.Errorf("diff (-want +got):\n%s", diff) + t.Errorf("Settles() diff (-want +got):\n%s", diff) } }) } } -func (b *Block) markExecutedForTests(tb testing.TB, db ethdb.Database, tm *gastime.Time) { - tb.Helper() - require.NoError(tb, b.MarkExecuted(db, tm, time.Time{}, nil, common.Hash{}, hooktest.Simple{}), "MarkExecuted()") -} - func TestLastToSettleAt(t *testing.T) { blocks := newChain(t, 0, 30, nil) t.Run("helper_invariants", func(t *testing.T) { for i, b := range blocks { - require.Equal(t, uint64(i), b.Height()) - require.Equal(t, b.Time(), b.Height()) + require.Equal(t, uint64(i), b.Height()) //nolint:gosec // Slice index won't overflow + require.Equal(t, b.BuildTime(), b.Height()) } }) @@ -275,15 +251,21 @@ func TestLastToSettleAt(t *testing.T) { requireTime(t, 13, 1) blocks[8].markExecutedForTests(t, db, tm) - for _, b := range blocks { - if !b.Executed() { - continue - } + require.False( + t, blocks[9].Executed(), + "Block 9 MUST remain unexecuted", // exercises lagging-execution logic when building on 9 + ) + + for i, b := range blocks { // Setting interim execution time isn't required for the algorithm to // work as it just allows [LastToSettleAt] to return definitive results // earlier in execution. It does, however, risk an edge-case error for - // blocks that complete execution on an exact second boundary; see the - // [Block.SetInterimExecutionTime] implementation for details. + // blocks that complete execution on an exact second boundary so needs + // to be tested; see the [Block.SetInterimExecutionTime] implementation + // for details. + if i%2 == 0 || !b.Executed() { + continue + } b.SetInterimExecutionTime(b.ExecutedByGasTime().Time) } @@ -329,14 +311,8 @@ func TestLastToSettleAt(t *testing.T) { { settleAt: 9, parent: blocks[9], - // The current implementation is very coarse-grained and MAY return - // false negatives that would simply require a retry after some - // indeterminate period of time. Even though the execution time of - // `blocks[8]` guarantees that `blocks[9]` MUST finish execution - // after the settlement time, our current implementation doesn't - // check this. It is expected that this specific test case will one - // day fail, at which point it MUST be updated to want `blocks[7]`. - wantOK: false, + wantOK: true, + want: blocks[7], }, { settleAt: 15, diff --git a/blocks/snow.go b/blocks/snow.go index cc77df8..1c7bc55 100644 --- a/blocks/snow.go +++ b/blocks/snow.go @@ -1,3 +1,6 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + package blocks import ( @@ -5,9 +8,10 @@ import ( "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/libevm/rlp" - "github.com/ava-labs/strevm/adaptor" "go.uber.org/zap" + "github.com/ava-labs/strevm/adaptor" + // Imported to allow IDE resolution of comments like [types.Block]. The // package is imported in other files so this is a no-op beyond devex. _ "github.com/ava-labs/libevm/core/types" @@ -15,35 +19,35 @@ import ( var _ adaptor.BlockProperties = (*Block)(nil) -// ID returns [types.Block.Hash] from the embedded [types.Block]. +// ID returns [types.Block.Hash] from the wrapped [types.Block]. func (b *Block) ID() ids.ID { return ids.ID(b.Hash()) } -// Parent returns [types.Block.ParentHash] from the embedded [types.Block]. +// Parent returns [types.Block.ParentHash] from the wrapped [types.Block]. func (b *Block) Parent() ids.ID { return ids.ID(b.ParentHash()) } -// Bytes returns the RLP encoding of the embedded [types.Block]. If encoding -// returns an error, it is logged at the WARNING level and a nil slice is +// Bytes returns the RLP encoding of the wrapped [types.Block]. If encoding +// returns an error, it is logged at the ERROR level and a nil slice is // returned. func (b *Block) Bytes() []byte { buf, err := rlp.EncodeToBytes(b) if err != nil { - b.log.Warn("RLP encoding error", zap.Error(err)) + b.log.Error("RLP encoding error", zap.Error(err)) return nil } return buf } -// Height returns [types.Block.NumberU64] from the embedded [types.Block]. +// Height returns [types.Block.NumberU64] from the wrapped [types.Block]. func (b *Block) Height() uint64 { return b.NumberU64() } -// Timestamp returns the timestamp of the embedded [types.Block], at +// Timestamp returns the timestamp of the wrapped [types.Block], at // [time.Second] resolution. func (b *Block) Timestamp() time.Time { - return time.Unix(int64(b.Time()), 0) + return time.Unix(int64(b.BuildTime()), 0) //nolint:gosec // Won't be a problem for a few millennia } diff --git a/builder.go b/builder.go index 97fcdb7..d76c807 100644 --- a/builder.go +++ b/builder.go @@ -207,14 +207,14 @@ func (vm *VM) buildBlockOnHistory( receipts []types.Receipts gasUsed uint64 ) - // We can never concurrently build and accept a block on the same parent, - // which guarantees that `parent` won't be settled, so the [Block] invariant - // means that `parent.lastSettled != nil`. - for _, b := range parent.IfChildSettles(lastSettled) { - brs := b.Receipts() - receipts = append(receipts, brs) - for _, r := range brs { - gasUsed += r.GasUsed + if !parent.IsSettled() { + // The parent can only be settled here if it was settled synchronously. + for _, b := range parent.WhenChildSettles(lastSettled) { + brs := b.Receipts() + receipts = append(receipts, brs) + for _, r := range brs { + gasUsed += r.GasUsed + } } } diff --git a/cmputils/cmputils.go b/cmputils/cmputils.go index 6ecedc5..1b3f35a 100644 --- a/cmputils/cmputils.go +++ b/cmputils/cmputils.go @@ -28,3 +28,26 @@ func pathIncludes[T any](p cmp.Path) bool { } return false } + +// WithNilCheck returns a function that returns: +// +// true if both a and b are nil +// false if exactly one of a or b is nil +// fn(a,b) if neither a nor b are nil +func WithNilCheck[T any](fn func(*T, *T) bool) func(*T, *T) bool { + return func(a, b *T) bool { + switch an, bn := a == nil, b == nil; { + case an && bn: + return true + case an || bn: + return false + } + return fn(a, b) + } +} + +// ComparerWithNilCheck is a convenience wrapper, returning a [cmp.Comparer] +// after wrapping `fn` in [WithNilCheck]. +func ComparerWithNilCheck[T any](fn func(*T, *T) bool) cmp.Option { + return cmp.Comparer(WithNilCheck(fn)) +} diff --git a/db.go b/db.go index 7f07038..d4c06eb 100644 --- a/db.go +++ b/db.go @@ -56,17 +56,21 @@ func (vm *VM) upgradeLastSynchronousBlock(lastSync LastSynchronousBlock) error { clock := gastime.New(block.Time(), lastSync.Target, lastSync.ExcessAfter) receipts := rawdb.ReadRawReceipts(vm.db, lastSync.Hash, block.Height()) - if err := block.MarkExecuted(vm.db, clock, block.Timestamp(), receipts, block.Block.Root(), vm.hooks); err != nil { + baseFee := block.BaseFee() + if baseFee == nil { + baseFee = big.NewInt(0) + } + if err := block.MarkExecuted(vm.db, clock, block.Timestamp(), baseFee, receipts, block.Block.Root(), vm.hooks); err != nil { return err } if err := block.WriteLastSettledNumber(vm.db); err != nil { return err } - if err := block.MarkSettled(); err != nil { + if err := block.MarkSynchronous(); err != nil { return err } - if err := block.CheckInvariants(true, true); err != nil { + if err := block.CheckInvariants(blocks.Settled); err != nil { return fmt.Errorf("upgrading last synchronous block: %v", err) } vm.logger().Info( diff --git a/go.mod b/go.mod index 9484758..c80e74e 100644 --- a/go.mod +++ b/go.mod @@ -21,16 +21,8 @@ require ( ) require ( - github.com/BurntSushi/toml v1.5.0 // indirect - github.com/DataDog/zstd v1.5.2 // indirect - github.com/Microsoft/go-winio v0.6.1 // indirect - github.com/VictoriaMetrics/fastcache v1.12.1 // indirect - github.com/beorn7/perks v1.0.1 // indirect github.com/bits-and-blooms/bitset v1.10.0 // indirect github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect - github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0 // indirect - github.com/cenkalti/backoff/v4 v4.2.1 // indirect - github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/cockroachdb/errors v1.9.1 // indirect github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b // indirect github.com/cockroachdb/pebble v0.0.0-20230928194634-aa077af62593 // indirect @@ -40,21 +32,48 @@ require ( github.com/consensys/gnark-crypto v0.12.1 // indirect github.com/crate-crypto/go-ipa v0.0.0-20231025140028-3c0104f4b233 // indirect github.com/crate-crypto/go-kzg-4844 v1.0.0 // indirect - github.com/davecgh/go-spew v1.1.1 // indirect - github.com/deckarep/golang-set/v2 v2.1.0 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 // indirect github.com/ethereum/c-kzg-4844 v1.0.0 // indirect - github.com/fatih/structtag v1.2.0 // indirect - github.com/fsnotify/fsnotify v1.6.0 // indirect - github.com/gballet/go-libpcsclite v0.0.0-20191108122812-4678299bea08 // indirect github.com/gballet/go-verkle v0.1.1-0.20231031103413-a67434b50f46 // indirect github.com/getsentry/sentry-go v0.18.0 // indirect - github.com/go-logr/logr v1.4.1 // indirect - github.com/go-logr/stdr v1.2.2 // indirect github.com/go-ole/go-ole v1.3.0 // indirect github.com/gofrs/flock v0.8.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/snappy v0.0.5-0.20220116011046-fa5810519dcb // indirect + github.com/klauspost/compress v1.18.0 // indirect + github.com/kr/pretty v0.3.1 // indirect + github.com/kr/text v0.2.0 // indirect + github.com/mattn/go-runewidth v0.0.13 // indirect + github.com/mmcloughlin/addchain v0.4.0 // indirect + github.com/olekukonko/tablewriter v0.0.5 // indirect + github.com/pkg/errors v0.9.1 // indirect + github.com/rivo/uniseg v0.2.0 // indirect + github.com/rogpeppe/go-internal v1.12.0 // indirect + github.com/shirou/gopsutil v3.21.11+incompatible // indirect + github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a // indirect + github.com/tklauser/go-sysconf v0.3.12 // indirect + github.com/tklauser/numcpus v0.6.1 // indirect + github.com/yusufpapurcu/wmi v1.2.2 // indirect + golang.org/x/sync v0.17.0 // indirect + rsc.io/tmplfunc v0.0.3 // indirect +) + +require ( + github.com/BurntSushi/toml v1.5.0 // indirect + github.com/DataDog/zstd v1.5.2 // indirect + github.com/Microsoft/go-winio v0.6.1 // indirect + github.com/VictoriaMetrics/fastcache v1.12.1 // indirect + github.com/beorn7/perks v1.0.1 // indirect + github.com/btcsuite/btcd/chaincfg/chainhash v1.1.0 // indirect + github.com/cenkalti/backoff/v4 v4.2.1 // indirect + github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/deckarep/golang-set/v2 v2.1.0 // indirect + github.com/fatih/structtag v1.2.0 // indirect + github.com/fsnotify/fsnotify v1.6.0 // indirect + github.com/gballet/go-libpcsclite v0.0.0-20191108122812-4678299bea08 // indirect + github.com/go-logr/logr v1.4.1 // indirect + github.com/go-logr/stdr v1.2.2 // indirect github.com/google/renameio/v2 v2.0.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/gorilla/rpc v1.2.0 // indirect @@ -65,31 +84,17 @@ require ( github.com/huin/goupnp v1.3.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/jackpal/go-nat-pmp v1.0.2 // indirect - github.com/klauspost/compress v1.18.0 // indirect - github.com/kr/pretty v0.3.1 // indirect - github.com/kr/text v0.2.0 // indirect - github.com/mattn/go-runewidth v0.0.13 // indirect - github.com/mmcloughlin/addchain v0.4.0 // indirect github.com/mr-tron/base58 v1.2.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect - github.com/olekukonko/tablewriter v0.0.5 // indirect - github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.62.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect - github.com/rivo/uniseg v0.2.0 // indirect - github.com/rogpeppe/go-internal v1.12.0 // indirect - github.com/shirou/gopsutil v3.21.11+incompatible // indirect github.com/spf13/cobra v1.8.1 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/status-im/keycard-go v0.2.0 // indirect github.com/supranational/blst v0.3.14 // indirect - github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a // indirect - github.com/tklauser/go-sysconf v0.3.12 // indirect - github.com/tklauser/numcpus v0.6.1 // indirect github.com/tyler-smith/go-bip39 v1.1.0 // indirect - github.com/yusufpapurcu/wmi v1.2.2 // indirect go.opentelemetry.io/otel v1.22.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.22.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.22.0 // indirect @@ -102,7 +107,6 @@ require ( golang.org/x/crypto v0.43.0 // indirect golang.org/x/mod v0.29.0 // indirect golang.org/x/net v0.46.0 // indirect - golang.org/x/sync v0.17.0 // indirect golang.org/x/sys v0.37.0 // indirect golang.org/x/text v0.30.0 // indirect golang.org/x/tools v0.38.0 // indirect @@ -113,5 +117,4 @@ require ( google.golang.org/protobuf v1.36.5 // indirect gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - rsc.io/tmplfunc v0.0.3 // indirect ) diff --git a/saetest/logging.go b/saetest/logging.go new file mode 100644 index 0000000..ca57e8b --- /dev/null +++ b/saetest/logging.go @@ -0,0 +1,141 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package saetest + +import ( + "runtime" + "slices" + "testing" + + "github.com/ava-labs/avalanchego/utils/logging" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" +) + +// logger is the common wrapper around [LogRecorder] and [tbLogger] handlers, +// plumbing all levels into the handler. +type logger struct { + level logging.Level + handler interface { + log(logging.Level, string, ...zap.Field) + } + with []zap.Field + // Some methods will panic, in which case they need to be implemented. This + // is better than embedding a [logging.NoLog], which could silently drop + // important entries. + logging.Logger +} + +var _ logging.Logger = (*logger)(nil) + +func (l *logger) With(fields ...zap.Field) logging.Logger { + return &logger{ + level: l.level, + handler: l.handler, + with: slices.Concat(l.with, fields), + } +} + +func (l *logger) log(lvl logging.Level, msg string, fields ...zap.Field) { + if lvl < l.level { + return + } + l.handler.log(lvl, msg, slices.Concat(l.with, fields)...) +} + +func (l *logger) Debug(msg string, fs ...zap.Field) { l.log(logging.Debug, msg, fs...) } +func (l *logger) Trace(msg string, fs ...zap.Field) { l.log(logging.Trace, msg, fs...) } +func (l *logger) Info(msg string, fs ...zap.Field) { l.log(logging.Info, msg, fs...) } +func (l *logger) Warn(msg string, fs ...zap.Field) { l.log(logging.Warn, msg, fs...) } +func (l *logger) Error(msg string, fs ...zap.Field) { l.log(logging.Error, msg, fs...) } +func (l *logger) Fatal(msg string, fs ...zap.Field) { l.log(logging.Fatal, msg, fs...) } + +// NewLogRecorder constructs a new [LogRecorder] at the specified level. +func NewLogRecorder(level logging.Level) *LogRecorder { + r := new(LogRecorder) + r.logger = &logger{ + handler: r, // yes, the recursion is gross, but that's composition for you ¯\_(ツ)_/¯ + level: level, + } + return r +} + +// A LogRecorder is a [logging.Logger] that stores all logs as [LogRecord] +// entries for inspection. +type LogRecorder struct { + *logger + Records []*LogRecord +} + +// A LogRecord is a single entry in a [LogRecorder]. +type LogRecord struct { + Level logging.Level + Msg string + Fields []zap.Field +} + +func (l *LogRecorder) log(lvl logging.Level, msg string, fields ...zap.Field) { + l.Records = append(l.Records, &LogRecord{ + Level: lvl, + Msg: msg, + Fields: fields, + }) +} + +// Filter returns the recorded logs for which `fn` returns true. +func (l *LogRecorder) Filter(fn func(*LogRecord) bool) []*LogRecord { + var out []*LogRecord + for _, r := range l.Records { + if fn(r) { + out = append(out, r) + } + } + return out +} + +// At returns all recorded logs at the specified [logging.Level]. +func (l *LogRecorder) At(lvl logging.Level) []*LogRecord { + return l.Filter(func(r *LogRecord) bool { return r.Level == lvl }) +} + +// AtLeast returns all recorded logs at or above the specified [logging.Level]. +func (l *LogRecorder) AtLeast(lvl logging.Level) []*LogRecord { + return l.Filter(func(r *LogRecord) bool { return r.Level >= lvl }) +} + +// NewTBLogger constructs a logger that propagates logs to the [testing.TB]. +// WARNING and ERROR logs are sent to [testing.TB.Errorf] while FATAL is sent to +// [testing.TB.Fatalf]. All other logs are sent to [testing.TB.Logf]. Although +// the level can be configured, it is silently capped at [logging.Warn]. +// +//nolint:thelper // The outputs include the logging site while the TB site is most useful if here +func NewTBLogger(tb testing.TB, level logging.Level) logging.Logger { + return &logger{ + level: min(level, logging.Warn), + handler: &tbLogger{tb: tb}, + } +} + +type tbLogger struct { + tb testing.TB +} + +func (l *tbLogger) log(lvl logging.Level, msg string, fields ...zap.Field) { + var to func(string, ...any) + switch { + case lvl == logging.Warn || lvl == logging.Error: // because @ARR4N says warnings in tests are errors + to = l.tb.Errorf + case lvl >= logging.Fatal: + to = l.tb.Fatalf + default: + to = l.tb.Logf + } + + enc := zapcore.NewMapObjectEncoder() + for _, f := range fields { + f.AddTo(enc) + } + _, file, line, _ := runtime.Caller(3) + to("[Log@%s] %s %v - %s:%d", lvl, msg, enc.Fields, file, line) +} diff --git a/saetest/saetest.go b/saetest/saetest.go new file mode 100644 index 0000000..b0cc7d6 --- /dev/null +++ b/saetest/saetest.go @@ -0,0 +1,29 @@ +// Copyright (C) 2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +// Package saetest provides testing helpers for [Streaming Asynchronous +// Execution] (SAE). +// +// [Streaming Asynchronous Execution]: https://github.com/avalanche-foundation/ACPs/tree/main/ACPs/194-streaming-asynchronous-execution +package saetest + +import ( + "github.com/ava-labs/libevm/core/types" + "github.com/ava-labs/libevm/trie" + "github.com/google/go-cmp/cmp" +) + +// TrieHasher returns an arbitrary trie hasher. +func TrieHasher() types.TrieHasher { + return trie.NewStackTrie(nil) +} + +// MerkleRootsEqual returns whether the two arguments have the same Merkle root. +func MerkleRootsEqual[T types.DerivableList](a, b T) bool { + return types.DeriveSha(a, TrieHasher()) == types.DeriveSha(b, TrieHasher()) +} + +// CmpByMerkleRoots returns a [cmp.Comparer] using [MerkleRootsEqual]. +func CmpByMerkleRoots[T types.DerivableList]() cmp.Option { + return cmp.Comparer(MerkleRootsEqual[T]) +} diff --git a/saexec/execution.go b/saexec/execution.go index 1b568bd..6ce0407 100644 --- a/saexec/execution.go +++ b/saexec/execution.go @@ -170,7 +170,7 @@ func (e *Executor) execute(ctx context.Context, b *blocks.Block) error { // 1. [blocks.Block.MarkExecuted] guarantees disk then in-memory changes. // 2. Internal indicator of last executed MUST follow in-memory change. // 3. External indicator of last executed MUST follow internal indicator. - if err := b.MarkExecuted(e.db, e.gasClock.Clone(), endTime, receipts, root, e.hooks); err != nil { + if err := b.MarkExecuted(e.db, e.gasClock.Clone(), endTime, header.BaseFee, receipts, root, e.hooks); err != nil { return err } e.lastExecuted.Store(b) // (2)