Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
  • Loading branch information
yacovm committed Aug 1, 2024
1 parent 31eaeed commit 9474e9e
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 35 deletions.
13 changes: 2 additions & 11 deletions chains/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
4 changes: 0 additions & 4 deletions snow/engine/snowman/block/test_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions snow/engine/snowman/block/vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 4 additions & 4 deletions snow/engine/snowman/bootstrap/bootstrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion snow/engine/snowman/bootstrap/bootstrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion snow/engine/snowman/bootstrap/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ type Config struct {

VM block.ChainVM

Parser block.Parser
// NonVerifyingParse parses blocks without verifying them.
NonVerifyingParse block.ParseFunc

Bootstrapped func()
}
8 changes: 4 additions & 4 deletions snow/engine/snowman/bootstrap/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
10 changes: 1 addition & 9 deletions vms/platformvm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion vms/proposervm/vm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}{
Expand Down

0 comments on commit 9474e9e

Please sign in to comment.