feat: Remaining ChainVM methods (#183)

Add GetBlock and GetBlockIDByHeight with basic tests. The refactoring
necessary to expose the settling action is really gross, so we might
just need to simplify some of that eventually (very panic-prone).
Additionally, whether a block is synchronous or not is not persisted, so
it cannot be loaded.

---------

Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>

Author: 3 people

blocks/block.go

@@ -17,9 +17,12 @@ import (
 	"github.com/ava-labs/avalanchego/vms/components/gas"
 	"github.com/ava-labs/libevm/common"
 	"github.com/ava-labs/libevm/core/types"
+	"github.com/ava-labs/libevm/ethdb"
+	"github.com/ava-labs/libevm/params"
 	"go.uber.org/zap"
 
 	"github.com/ava-labs/strevm/proxytime"
+	"github.com/ava-labs/strevm/saedb"
 )
 
 // A Block extends a [types.Block] to track SAE-defined concepts of async
@@ -96,6 +99,26 @@ func New(eth *types.Block, parent, lastSettled *Block, log logging.Logger) (*Blo
 	return b, nil
 }
 
+// RestoreSettledBlock constructs a new block with [New] and restores it to an
+// settled state before returning it. By definition of being settled, the
+// returned block also includes post-execution artefacts.
+func RestoreSettledBlock(eth *types.Block, log logging.Logger, db ethdb.Database, xdb saedb.ExecutionResults, config *params.ChainConfig) (*Block, error) {
+	b, err := New(eth, nil, nil, log)
+	if err != nil {
+		return nil, err
+	}
+	if err := b.RestoreExecutionArtefacts(db, xdb, config); err != nil {
+		return nil, fmt.Errorf("restoring to executed state: %v", err)
+	}
+	if err := b.markSettled(nil); err != nil {
+		return nil, fmt.Errorf("restoring to settled state: %v", err)
+	}
+	if err := b.CheckInvariants(Settled); err != nil {
+		return nil, err
+	}
+	return b, nil
+}
+
 var (
 	errParentHashMismatch         = errors.New("block-parent hash mismatch")
 	errBlockHeightNotIncrementing = errors.New("block height not incrementing")

blocks/settlement.go

@@ -83,6 +83,8 @@ func (b *Block) markSettled(lastSettled *atomic.Pointer[Block]) error {
 // Unlike [Block.MarkExecuted], MarkSynchronous does not call
 // [Block.SetAsHeadBlock], which MUST be done by the caller, i.f.f. the chain
 // has not yet commenced asynchronous execution.
+//
+// TODO(arr4n) refactor to avoid requiring DB writes.
 func (b *Block) MarkSynchronous(hooks hook.Points, db ethdb.Database, xdb saedb.ExecutionResults, excessAfter gas.Gas) error {
 	ethB := b.EthBlock()
 	// Receipts of a synchronous block have already been "settled" by the block

sae/blocks.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/ava-labs/avalanchego/database"
 	"github.com/ava-labs/avalanchego/ids"
+	"github.com/ava-labs/avalanchego/snow/consensus/snowman"
 	"github.com/ava-labs/avalanchego/snow/engine/snowman/block"
 	"github.com/ava-labs/libevm/common"
 	"github.com/ava-labs/libevm/core"
@@ -364,23 +365,82 @@ func canonicalBlock(db ethdb.Database, num uint64) (*types.Block, error) {
 }
 
 // GetBlock returns the block with the given ID, or [database.ErrNotFound].
+//
+// It is expected that blocks that have been successfully verified should be
+// returned correctly. It is also expected that blocks that have been
+// accepted by the consensus engine should be able to be fetched. It is not
+// required for blocks that have been rejected by the consensus engine to be
+// able to be fetched.
 func (vm *VM) GetBlock(ctx context.Context, id ids.ID) (*blocks.Block, error) {
-	b, ok := vm.blocks.Load(common.Hash(id))
-	if !ok {
-		return nil, database.ErrNotFound
-	}
-	return b, nil
+	var _ snowman.Block // protect the input to allow comment linking
+
+	return readByHash(
+		vm,
+		common.Hash(id),
+		func(b *blocks.Block) *blocks.Block {
+			return b
+		},
+		func(db ethdb.Reader, hash common.Hash, num uint64) (*blocks.Block, error) {
+			// A block that's not in memory has either been rejected, not yet
+			// verified, or settled. Of these, only the latter would be in the
+			// database.
+			//
+			// There is, however, a negligible (read: near impossible) but
+			// non-zero chance that [VM.VerifyBlock] and [VM.AcceptBlock] were
+			// *both* called between [readByHash] checking the in-memory block
+			// store and loading the canonical number from the database. That
+			// could result in an unexecuted block, which would cause an error
+			// when restoring it.
+			//
+			// TODO(arr4n) I think [readHash] should be providing this guarantee
+			// as it has access to the [syncMap] and its lock.
+			if vm.last.settled.Load().Height() < num {
+				return nil, database.ErrNotFound
+			}
+
+			ethB := rawdb.ReadBlock(db, hash, num)
+			if num > vm.last.synchronous {
+				return blocks.RestoreSettledBlock(
+					ethB,
+					vm.log(),
+					vm.db,
+					vm.xdb,
+					vm.exec.ChainConfig(),
+				)
+			}
+
+			b, err := vm.newBlock(ethB, nil, nil)
+			if err != nil {
+				return nil, err
+			}
+			// Excess is only used for executing the next block, which can never
+			// be the case if `b` isn't actually the last synchronous block, so
+			// passing the same value for all is OK.
+			if err := b.MarkSynchronous(vm.hooks(), vm.db, vm.xdb, vm.config.ExcessAfterLastSynchronous); err != nil {
+				return nil, err
+			}
+			return b, nil
+		},
+		database.ErrNotFound,
+	)
 }
 
 // GetBlockIDAtHeight returns the accepted block at the given height, or
 // [database.ErrNotFound].
-func (vm *VM) GetBlockIDAtHeight(context.Context, uint64) (ids.ID, error) {
-	return ids.Empty, errUnimplemented
+func (vm *VM) GetBlockIDAtHeight(ctx context.Context, height uint64) (ids.ID, error) {
+	id := ids.ID(rawdb.ReadCanonicalHash(vm.db, height))
+	if id == ids.Empty {
+		return id, database.ErrNotFound
+	}
+	return id, nil
 }
 
 var _ blocks.Source = (*VM)(nil).blockSource
 
 func (vm *VM) blockSource(hash common.Hash, num uint64) (*blocks.Block, bool) {
+	// TODO(arr4n) this MUST be updated to support all blocks (256) necessary to
+	// back a [core.ChainContext] in [saexec.Executor] otherwise op codes like
+	// BLOCKHASH will panic.
 	b, ok := vm.blocks.Load(hash)
 	if !ok || b.NumberU64() != num {
 		return nil, false

sae/rpc.go

@@ -337,27 +337,27 @@ func (b *ethAPIBackend) SyncProgress() ethereum.SyncProgress {
 }
 
 func (b *ethAPIBackend) HeaderByNumber(ctx context.Context, n rpc.BlockNumber) (*types.Header, error) {
-	return readByNumber(b, n, rawdb.ReadHeader)
+	return readByNumber(b, n, neverErrs(rawdb.ReadHeader))
 }
 
 func (b *ethAPIBackend) BlockByNumber(ctx context.Context, n rpc.BlockNumber) (*types.Block, error) {
-	return readByNumber(b, n, rawdb.ReadBlock)
+	return readByNumber(b, n, neverErrs(rawdb.ReadBlock))
 }
 
 func (b *ethAPIBackend) HeaderByHash(ctx context.Context, hash common.Hash) (*types.Header, error) {
-	return readByHash(b, hash, (*blocks.Block).Header, rawdb.ReadHeader), nil
+	return readByHash(b.vm, hash, (*blocks.Block).Header, neverErrs(rawdb.ReadHeader), nil /* errWhenNotFound */)
 }
 
 func (b *ethAPIBackend) BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error) {
-	return readByHash(b, hash, (*blocks.Block).EthBlock, rawdb.ReadBlock), nil
+	return readByHash(b.vm, hash, (*blocks.Block).EthBlock, neverErrs(rawdb.ReadBlock), nil /* errWhenNotFound */)
 }
 
 func (b *ethAPIBackend) HeaderByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Header, error) {
-	return readByNumberOrHash(b, blockNrOrHash, (*blocks.Block).Header, rawdb.ReadHeader)
+	return readByNumberOrHash(b, blockNrOrHash, (*blocks.Block).Header, neverErrs(rawdb.ReadHeader))
 }
 
 func (b *ethAPIBackend) BlockByNumberOrHash(ctx context.Context, blockNrOrHash rpc.BlockNumberOrHash) (*types.Block, error) {
-	return readByNumberOrHash(b, blockNrOrHash, (*blocks.Block).EthBlock, rawdb.ReadBlock)
+	return readByNumberOrHash(b, blockNrOrHash, (*blocks.Block).EthBlock, neverErrs(rawdb.ReadBlock))
 }
 
 func (b *ethAPIBackend) GetTransaction(ctx context.Context, txHash common.Hash) (exists bool, tx *types.Transaction, blockHash common.Hash, blockNumber uint64, index uint64, err error) {
@@ -415,29 +415,43 @@ func (b *ethAPIBackend) GetPoolTransactions() (types.Transactions, error) {
 }
 
 type (
-	canonicalReader[T any] func(ethdb.Reader, common.Hash, uint64) *T
+	canonicalReader[T any] func(ethdb.Reader, common.Hash, uint64) (*T, error)
 	blockAccessor[T any]   func(*blocks.Block) *T
 )
 
+func neverErrs[T any](fn func(ethdb.Reader, common.Hash, uint64) *T) canonicalReader[T] {
+	return func(r ethdb.Reader, h common.Hash, n uint64) (*T, error) {
+		return fn(r, h, n), nil
+	}
+}
+
 func readByNumber[T any](b *ethAPIBackend, n rpc.BlockNumber, read canonicalReader[T]) (*T, error) {
 	num, err := b.resolveBlockNumber(n)
 	if errors.Is(err, errFutureBlockNotResolved) {
 		return nil, nil
 	} else if err != nil {
 		return nil, err
 	}
-	return read(b.vm.db, rawdb.ReadCanonicalHash(b.vm.db, num), num), nil
+	return read(b.vm.db, rawdb.ReadCanonicalHash(b.vm.db, num), num)
 }
 
-func readByHash[T any](b *ethAPIBackend, hash common.Hash, fromMem blockAccessor[T], fromDB canonicalReader[T]) *T {
-	if blk, ok := b.vm.blocks.Load(hash); ok {
-		return fromMem(blk)
+// readByHash returns `fromMem(b)` if a block with the specified hash is in the
+// VM's memory, otherwise it returns `fromDB()` i.f.f. the block was previously
+// accepted. If `fromDB()` is called then the block is guaranteed to exist if
+// read with [rawdb] functions.
+//
+// A hash that is in neither of the VM's memory nor the database will result in
+// a return of `(nil, errWhenNotFound)` to allow for usage with the [rawdb]
+// pattern of returning `(nil, nil)`.
+func readByHash[T any](vm *VM, hash common.Hash, fromMem blockAccessor[T], fromDB canonicalReader[T], errWhenNotFound error) (*T, error) {
+	if blk, ok := vm.blocks.Load(hash); ok {
+		return fromMem(blk), nil
 	}
-	num := rawdb.ReadHeaderNumber(b.vm.db, hash)
+	num := rawdb.ReadHeaderNumber(vm.db, hash)
 	if num == nil {
-		return nil
+		return nil, errWhenNotFound
 	}
-	return fromDB(b.vm.db, hash, *num)
+	return fromDB(vm.db, hash, *num)
 }
 
 // TODO(arr4n) DRY [readByHash] and [readByNumberOrHash]
@@ -450,7 +464,7 @@ func readByNumberOrHash[T any](b *ethAPIBackend, blockNrOrHash rpc.BlockNumberOr
 	if blk, ok := b.vm.blocks.Load(hash); ok {
 		return fromMem(blk), nil
 	}
-	return fromDB(b.vm.db, hash, n), nil
+	return fromDB(b.vm.db, hash, n)
 }
 
 var (

sae/rpc_stateful_test.go

@@ -43,7 +43,7 @@ func TestEthCall(t *testing.T) {
 	}
 
 	sign := sut.wallet.SetNonceAndSign
-	b := sut.createAndAcceptBlock(t, sign(t, 0, deploy), sign(t, 0, deposit))
+	b := sut.runConsensusLoopFromLastAccepted(t, sign(t, 0, deploy), sign(t, 0, deposit))
 	require.Len(t, b.Transactions(), 2, "tx count")
 	require.NoErrorf(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 	for _, r := range b.Receipts() {

sae/rpc_test.go

@@ -436,15 +436,15 @@ func TestEthGetters(t *testing.T) {
 
 	// Once a block is settled, its ancestors are only accessible from the
 	// database.
-	onDisk := sut.createAndAcceptBlock(t, createTx(t, zeroAddr))
+	onDisk := sut.runConsensusLoopFromLastAccepted(t, createTx(t, zeroAddr))
 
-	settled := sut.createAndAcceptBlock(t, createTx(t, zeroAddr))
+	settled := sut.runConsensusLoopFromLastAccepted(t, createTx(t, zeroAddr))
 	vmTime.advanceToSettle(ctx, t, settled)
 
-	executed := sut.createAndAcceptBlock(t, createTx(t, zeroAddr))
+	executed := sut.runConsensusLoopFromLastAccepted(t, createTx(t, zeroAddr))
 	require.NoErrorf(t, executed.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", executed)
 
-	pending := sut.createAndAcceptBlock(t, createTx(t, blockingPrecompile))
+	pending := sut.runConsensusLoopFromLastAccepted(t, createTx(t, blockingPrecompile))
 
 	for _, b := range []*blocks.Block{genesis, onDisk, settled, executed, pending} {
 		t.Run(fmt.Sprintf("block_num_%d", b.Height()), func(t *testing.T) {
@@ -532,15 +532,15 @@ func TestGetLogs(t *testing.T) {
 	// and therefore moved to disk.
 	indexed := make([]*blocks.Block, bloomSectionSize)
 	for i := range indexed {
-		indexed[i] = sut.createAndAcceptBlock(t, txWithLog(t))
+		indexed[i] = sut.runConsensusLoopFromLastAccepted(t, txWithLog(t))
 	}
 
-	settled := sut.createAndAcceptBlock(t, txWithLog(t))
+	settled := sut.runConsensusLoopFromLastAccepted(t, txWithLog(t))
 	vmTime.advanceToSettle(ctx, t, settled)
 
-	noLogs := sut.createAndAcceptBlock(t, txWithoutLog(t))
+	noLogs := sut.runConsensusLoopFromLastAccepted(t, txWithoutLog(t))
 
-	executed := sut.createAndAcceptBlock(t, txWithLog(t))
+	executed := sut.runConsensusLoopFromLastAccepted(t, txWithLog(t))
 	require.NoErrorf(t, executed.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", executed)
 
 	// Although the FiltersAPI will work without any blocks indexed, such a
@@ -698,7 +698,7 @@ func TestGetReceipts(t *testing.T) {
 
 	slice := func(t *testing.T, from, to int) (*blocks.Block, []*types.Receipt) {
 		t.Helper()
-		b := sut.createAndAcceptBlock(t, txs[from:to]...)
+		b := sut.runConsensusLoopFromLastAccepted(t, txs[from:to]...)
 		rs := want[from:to]
 
 		var totalGas uint64
@@ -725,7 +725,7 @@ func TestGetReceipts(t *testing.T) {
 		Gas:      params.TxGas,
 		GasPrice: big.NewInt(1),
 	})
-	pending := sut.createAndAcceptBlock(t, pendingTx)
+	pending := sut.runConsensusLoopFromLastAccepted(t, pendingTx)
 
 	var tests []rpcTest
 	for _, tc := range []struct {
@@ -913,7 +913,7 @@ func TestDebugGetRawTransaction(t *testing.T) {
 		Gas:       params.TxGas,
 		GasFeeCap: big.NewInt(1),
 	})
-	b := sut.createAndAcceptBlock(t, tx)
+	b := sut.runConsensusLoopFromLastAccepted(t, tx)
 	require.NoErrorf(t, b.WaitUntilExecuted(ctx), "%T.WaitUntilExecuted()", b)
 
 	marshaled, err := tx.MarshalBinary()

sae/vm.go

@@ -60,6 +60,7 @@ type VM struct {
 	preference     atomic.Pointer[blocks.Block]
 	last           struct {
 		accepted, settled atomic.Pointer[blocks.Block]
+		synchronous       uint64
 	}
 
 	exec       *saexec.Executor
@@ -142,9 +143,9 @@ func NewVM(
 	if err != nil {
 		return nil, fmt.Errorf("blocks.New([last synchronous], ...): %v", err)
 	}
+	vm.last.synchronous = lastSync.Height()
 
 	{ // ==========  Sync -> Async  ==========
-		// TODO(arr4n) refactor to avoid DB writes on every startup.
 		if err := lastSync.MarkSynchronous(cfg.Hooks, db, xdb, cfg.ExcessAfterLastSynchronous); err != nil {
 			return nil, fmt.Errorf("%T{genesis}.MarkSynchronous(): %v", lastSync, err)
 		}