From 9474e9e082cb1503ff4c61c7b540d637904b787b Mon Sep 17 00:00:00 2001 From: Yacov Manevich Date: Thu, 1 Aug 2024 22:11:27 +0200 Subject: [PATCH] Address code review comments Signed-off-by: Yacov Manevich --- chains/manager.go | 13 ++----------- snow/engine/snowman/block/test_vm.go | 4 ---- snow/engine/snowman/block/vm.go | 8 ++++++++ snow/engine/snowman/bootstrap/bootstrapper.go | 8 ++++---- snow/engine/snowman/bootstrap/bootstrapper_test.go | 2 +- snow/engine/snowman/bootstrap/config.go | 3 ++- snow/engine/snowman/bootstrap/storage.go | 8 ++++---- vms/platformvm/vm_test.go | 10 +--------- vms/proposervm/vm_test.go | 2 +- 9 files changed, 23 insertions(+), 35 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 06c9607986b5..f3418a48cefd 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -954,7 +954,7 @@ func (m *manager) createAvalancheChain( // create bootstrap gear bootstrapCfg := smbootstrap.Config{ - Parser: &localParser{VM: proposerVM}, + NonVerifyingParse: block.ParseFunc(proposerVM.ParseLocalBlock), AllGetsServer: snowGetHandler, Ctx: ctx, Beacons: vdrs, @@ -1356,7 +1356,7 @@ func (m *manager) createSnowmanChain( // create bootstrap gear bootstrapCfg := smbootstrap.Config{ - Parser: &localParser{VM: proposerVM}, + NonVerifyingParse: block.ParseFunc(proposerVM.ParseLocalBlock), AllGetsServer: snowGetHandler, Ctx: ctx, Beacons: beacons, @@ -1588,12 +1588,3 @@ func (m *manager) getOrMakeVMRegisterer(vmID ids.ID, chainAlias string) (metrics ) return chainReg, err } - -// localParser intercepts invocations to ParseBlock and re-routes them to ParseLocalBlock -type localParser struct { - *proposervm.VM -} - -func (lp *localParser) ParseBlock(ctx context.Context, blockBytes []byte) (smcon.Block, error) { - return lp.ParseLocalBlock(ctx, blockBytes) -} diff --git a/snow/engine/snowman/block/test_vm.go b/snow/engine/snowman/block/test_vm.go index af77a0a924ca..cdbeabacc4f1 100644 --- a/snow/engine/snowman/block/test_vm.go +++ b/snow/engine/snowman/block/test_vm.go @@ -75,10 +75,6 @@ func (vm *TestVM) ParseBlock(ctx context.Context, b []byte) (snowman.Block, erro return nil, errParseBlock } -func (vm *TestVM) ParseLocalBlock(ctx context.Context, b []byte) (snowman.Block, error) { - return vm.ParseBlock(ctx, b) -} - func (vm *TestVM) GetBlock(ctx context.Context, id ids.ID) (snowman.Block, error) { if vm.GetBlockF != nil { return vm.GetBlockF(ctx, id) diff --git a/snow/engine/snowman/block/vm.go b/snow/engine/snowman/block/vm.go index d28dc7ef7a6d..792eda28511d 100644 --- a/snow/engine/snowman/block/vm.go +++ b/snow/engine/snowman/block/vm.go @@ -83,3 +83,11 @@ type Parser interface { // It is expected for all historical blocks to be parseable. ParseBlock(ctx context.Context, blockBytes []byte) (snowman.Block, error) } + +// ParseFunc defines a function that parses raw bytes into a block. +type ParseFunc func(context.Context, []byte) (snowman.Block, error) + +// ParseBlock wraps a ParseFunc into a ParseBlock function, to be used by a Parser interface +func (f ParseFunc) ParseBlock(ctx context.Context, blockBytes []byte) (snowman.Block, error) { + return f(ctx, blockBytes) +} diff --git a/snow/engine/snowman/bootstrap/bootstrapper.go b/snow/engine/snowman/bootstrap/bootstrapper.go index c76b83a4f432..966bdcf67d11 100644 --- a/snow/engine/snowman/bootstrap/bootstrapper.go +++ b/snow/engine/snowman/bootstrap/bootstrapper.go @@ -114,13 +114,13 @@ type Bootstrapper struct { // Called when bootstrapping is done on a specific chain onFinished func(ctx context.Context, lastReqID uint32) error - parser block.Parser + nonVerifyingParser block.Parser } func New(config Config, onFinished func(ctx context.Context, lastReqID uint32) error) (*Bootstrapper, error) { metrics, err := newMetrics(config.Ctx.Registerer) return &Bootstrapper{ - parser: config.Parser, + nonVerifyingParser: config.NonVerifyingParse, Config: config, metrics: metrics, StateSummaryFrontierHandler: common.NewNoOpStateSummaryFrontierHandler(config.Ctx.Log), @@ -181,7 +181,7 @@ func (b *Bootstrapper) Start(ctx context.Context, startReqID uint32) error { return fmt.Errorf("failed to initialize interval tree: %w", err) } - b.missingBlockIDs, err = getMissingBlockIDs(ctx, b.DB, b.parser, b.tree, b.startingHeight) + b.missingBlockIDs, err = getMissingBlockIDs(ctx, b.DB, b.nonVerifyingParser, b.tree, b.startingHeight) if err != nil { return fmt.Errorf("failed to initialize missing block IDs: %w", err) } @@ -653,7 +653,7 @@ func (b *Bootstrapper) tryStartExecuting(ctx context.Context) error { log, b.DB, &parseAcceptor{ - parser: b.parser, + parser: b.nonVerifyingParser, ctx: b.Ctx, numAccepted: b.numAccepted, }, diff --git a/snow/engine/snowman/bootstrap/bootstrapper_test.go b/snow/engine/snowman/bootstrap/bootstrapper_test.go index b3d9f3e4a888..6bccfdb58f95 100644 --- a/snow/engine/snowman/bootstrap/bootstrapper_test.go +++ b/snow/engine/snowman/bootstrap/bootstrapper_test.go @@ -90,7 +90,7 @@ func newConfig(t *testing.T) (Config, ids.NodeID, *common.SenderTest, *block.Tes peerTracker.Connected(peer, version.CurrentApp) return Config{ - Parser: vm, + NonVerifyingParse: vm.ParseBlock, AllGetsServer: snowGetHandler, Ctx: ctx, Beacons: vdrs, diff --git a/snow/engine/snowman/bootstrap/config.go b/snow/engine/snowman/bootstrap/config.go index 6f37c457a535..d501e37f5499 100644 --- a/snow/engine/snowman/bootstrap/config.go +++ b/snow/engine/snowman/bootstrap/config.go @@ -38,7 +38,8 @@ type Config struct { VM block.ChainVM - Parser block.Parser + // NonVerifyingParse parses blocks without verifying them. + NonVerifyingParse block.ParseFunc Bootstrapped func() } diff --git a/snow/engine/snowman/bootstrap/storage.go b/snow/engine/snowman/bootstrap/storage.go index 7dafc3a40225..bc5488e3870e 100644 --- a/snow/engine/snowman/bootstrap/storage.go +++ b/snow/engine/snowman/bootstrap/storage.go @@ -38,7 +38,7 @@ const ( func getMissingBlockIDs( ctx context.Context, db database.KeyValueReader, - parser block.Parser, + nonVerifyingParser block.Parser, tree *interval.Tree, lastAcceptedHeight uint64, ) (set.Set[ids.ID], error) { @@ -57,7 +57,7 @@ func getMissingBlockIDs( return nil, err } - blk, err := parser.ParseBlock(ctx, blkBytes) + blk, err := nonVerifyingParser.ParseBlock(ctx, blkBytes) if err != nil { return nil, err } @@ -130,7 +130,7 @@ func execute( haltable common.Haltable, log logging.Func, db database.Database, - parser block.Parser, + nonVerifyingParser block.Parser, tree *interval.Tree, lastAcceptedHeight uint64, ) error { @@ -198,7 +198,7 @@ func execute( for !haltable.Halted() && iterator.Next() { blkBytes := iterator.Value() - blk, err := parser.ParseBlock(ctx, blkBytes) + blk, err := nonVerifyingParser.ParseBlock(ctx, blkBytes) if err != nil { return err } diff --git a/vms/platformvm/vm_test.go b/vms/platformvm/vm_test.go index 1160500ed523..7c44e18c72ff 100644 --- a/vms/platformvm/vm_test.go +++ b/vms/platformvm/vm_test.go @@ -1513,7 +1513,7 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { require.NoError(err) bootstrapConfig := bootstrap.Config{ - Parser: &parser{VM: vm}, + NonVerifyingParse: vm.ParseBlock, AllGetsServer: snowGetHandler, Ctx: consensusCtx, Beacons: beacons, @@ -2525,11 +2525,3 @@ func TestPruneMempool(t *testing.T) { _, ok = vm.Builder.Get(baseTxID) require.True(ok) } - -type parser struct { - *VM -} - -func (a *parser) ParseLocalBlock(ctx context.Context, blockBytes []byte) (smcon.Block, error) { - return a.VM.ParseBlock(ctx, blockBytes) -} diff --git a/vms/proposervm/vm_test.go b/vms/proposervm/vm_test.go index 159ed970ab00..1fe2c225a5e1 100644 --- a/vms/proposervm/vm_test.go +++ b/vms/proposervm/vm_test.go @@ -2513,7 +2513,7 @@ func TestLocalParse(t *testing.T) { tests := []struct { name string - f func(ctx context.Context, b []byte) (snowman.Block, error) + f block.ParseFunc block []byte resultingBlock interface{} }{