Skip to content

Commit

Permalink
fix(dot/sync): remove block announcement in bootstrap sync mode (#2906
Browse files Browse the repository at this point in the history
)
  • Loading branch information
EclesioMeloJunior committed Nov 3, 2022
1 parent c331361 commit 2b4c257
Show file tree
Hide file tree
Showing 10 changed files with 237 additions and 47 deletions.
6 changes: 5 additions & 1 deletion dot/core/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,16 @@ func (s *Service) StorageRoot() (common.Hash, error) {
}

// HandleBlockImport handles a block that was imported via the network
func (s *Service) HandleBlockImport(block *types.Block, state *rtstorage.TrieState) error {
func (s *Service) HandleBlockImport(block *types.Block, state *rtstorage.TrieState, announce bool) error {
err := s.handleBlock(block, state)
if err != nil {
return fmt.Errorf("handling block: %w", err)
}

if !announce {
return nil
}

bestBlockHash := s.blockState.BestBlockHash()
isBestBlock := bestBlockHash.Equal(block.Header.Hash())

Expand Down
6 changes: 6 additions & 0 deletions dot/network/notifications_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package network

import (
"errors"
"fmt"
"reflect"
"testing"
"time"
Expand Down Expand Up @@ -196,6 +197,11 @@ func TestCreateNotificationsMessageHandler_BlockAnnounceHandshake(t *testing.T)

err = handler(stream, testHandshake)
require.ErrorIs(t, err, errCannotValidateHandshake)

expectedErrorMessage := fmt.Sprintf("handling handshake: %s from peer %s using protocol %s: genesis hash mismatch",
errCannotValidateHandshake, testPeerID, info.protocolID)
require.EqualError(t, err, expectedErrorMessage)

data := info.peersData.getInboundHandshakeData(testPeerID)
require.NotNil(t, data)
require.True(t, data.received)
Expand Down
49 changes: 32 additions & 17 deletions dot/sync/chain_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ type chainProcessor struct {
ctx context.Context
cancel context.CancelFunc

chainSync ChainSync

// blocks that are ready for processing. ie. their parent is known, or their parent is ahead
// of them within this channel and thus will be processed first
readyBlocks *blockQueue
Expand All @@ -42,24 +44,35 @@ type chainProcessor struct {
telemetry telemetry.Client
}

func newChainProcessor(readyBlocks *blockQueue, pendingBlocks DisjointBlockSet,
blockState BlockState, storageState StorageState,
transactionState TransactionState, babeVerifier BabeVerifier,
finalityGadget FinalityGadget, blockImportHandler BlockImportHandler, telemetry telemetry.Client) *chainProcessor {
type chainProcessorConfig struct {
readyBlocks *blockQueue
pendingBlocks DisjointBlockSet
syncer ChainSync
blockState BlockState
storageState StorageState
transactionState TransactionState
babeVerifier BabeVerifier
finalityGadget FinalityGadget
blockImportHandler BlockImportHandler
telemetry telemetry.Client
}

func newChainProcessor(cfg chainProcessorConfig) *chainProcessor {
ctx, cancel := context.WithCancel(context.Background())

return &chainProcessor{
ctx: ctx,
cancel: cancel,
readyBlocks: readyBlocks,
pendingBlocks: pendingBlocks,
blockState: blockState,
storageState: storageState,
transactionState: transactionState,
babeVerifier: babeVerifier,
finalityGadget: finalityGadget,
blockImportHandler: blockImportHandler,
telemetry: telemetry,
readyBlocks: cfg.readyBlocks,
pendingBlocks: cfg.pendingBlocks,
chainSync: cfg.syncer,
blockState: cfg.blockState,
storageState: cfg.storageState,
transactionState: cfg.transactionState,
babeVerifier: cfg.babeVerifier,
finalityGadget: cfg.finalityGadget,
blockImportHandler: cfg.blockImportHandler,
telemetry: cfg.telemetry,
}
}

Expand Down Expand Up @@ -109,6 +122,8 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
return fmt.Errorf("failed to check block state has body for hash %s: %w", bd.Hash, err)
}

// while in bootstrap mode we don't need to broadcast block announcements
announceImportedBlock := s.chainSync.syncState() == tip
if hasHeader && hasBody {
// TODO: fix this; sometimes when the node shuts down the "best block" isn't stored properly,
// so when the node restarts it has blocks higher than what it thinks is the best, causing it not to sync
Expand Down Expand Up @@ -149,7 +164,7 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
return err
}

if err := s.blockImportHandler.HandleBlockImport(block, state); err != nil {
if err := s.blockImportHandler.HandleBlockImport(block, state, announceImportedBlock); err != nil {
logger.Warnf("failed to handle block import: %s", err)
}

Expand All @@ -170,7 +185,7 @@ func (s *chainProcessor) processBlockData(bd *types.BlockData) error {
Body: *bd.Body,
}

if err := s.handleBlock(block); err != nil {
if err := s.handleBlock(block, announceImportedBlock); err != nil {
logger.Debugf("failed to handle block number %d: %s", block.Header.Number, err)
return err
}
Expand Down Expand Up @@ -201,7 +216,7 @@ func (s *chainProcessor) handleBody(body *types.Body) {
}

// handleHeader handles blocks (header+body) included in BlockResponses
func (s *chainProcessor) handleBlock(block *types.Block) error {
func (s *chainProcessor) handleBlock(block *types.Block, announceImportedBlock bool) error {
parent, err := s.blockState.GetHeader(block.Header.ParentHash)
if err != nil {
return fmt.Errorf("%w: %s", errFailedToGetParent, err)
Expand Down Expand Up @@ -233,7 +248,7 @@ func (s *chainProcessor) handleBlock(block *types.Block) error {
return fmt.Errorf("failed to execute block %d: %w", block.Header.Number, err)
}

if err = s.blockImportHandler.HandleBlockImport(block, ts); err != nil {
if err = s.blockImportHandler.HandleBlockImport(block, ts, announceImportedBlock); err != nil {
return err
}

Expand Down
Loading

0 comments on commit 2b4c257

Please sign in to comment.