fix(dot/sync): fix "block with unknown header is ready" error (#2191)

dot/sync/chain_sync.go

@@ -13,10 +13,11 @@ import (
 	"sync"
 	"time"
 
-	"github.com/ChainSafe/gossamer/dot/peerset"
+	"github.com/ChainSafe/chaindb"
 	"github.com/libp2p/go-libp2p-core/peer"
 
 	"github.com/ChainSafe/gossamer/dot/network"
+	"github.com/ChainSafe/gossamer/dot/peerset"
 	"github.com/ChainSafe/gossamer/dot/types"
 	"github.com/ChainSafe/gossamer/lib/common"
 	"github.com/ChainSafe/gossamer/lib/common/variadic"
@@ -522,7 +523,7 @@ func (cs *chainSync) setMode(mode chainSyncState) {
 	case bootstrap:
 		cs.handler = newBootstrapSyncer(cs.blockState)
 	case tip:
-		cs.handler = newTipSyncer(cs.blockState, cs.pendingBlocks, cs.readyBlocks)
+		cs.handler = newTipSyncer(cs.blockState, cs.pendingBlocks, cs.readyBlocks, cs.handleReadyBlock)
 	}
 
 	cs.state = mode
@@ -692,41 +693,59 @@ func (cs *chainSync) doSync(req *network.BlockRequestMessage, peersTried map[pee
 	// response was validated! place into ready block queue
 	for _, bd := range resp.BlockData {
 		// block is ready to be processed!
-		handleReadyBlock(bd, cs.pendingBlocks, cs.readyBlocks)
+		cs.handleReadyBlock(bd)
 	}
 
 	return nil
 }
 
-func handleReadyBlock(bd *types.BlockData, pendingBlocks DisjointBlockSet, readyBlocks *blockQueue) {
-	// see if there are any descendents in the pending queue that are now ready to be processed,
-	// as we have just become aware of their parent block
+func (cs *chainSync) handleReadyBlock(bd *types.BlockData) {
+	if cs.readyBlocks.has(bd.Hash) {
+		logger.Tracef("ignoring block %s in response, already in ready queue", bd.Hash)
+		return
+	}
 
 	// if header was not requested, get it from the pending set
 	// if we're expecting headers, validate should ensure we have a header
 	if bd.Header == nil {
-		block := pendingBlocks.getBlock(bd.Hash)
+		block := cs.pendingBlocks.getBlock(bd.Hash)
 		if block == nil {
-			logger.Criticalf("block with unknown header is ready: hash=%s", bd.Hash)
+			// block wasn't in the pending set!
+			// let's check the db as maybe we already processed it
+			has, err := cs.blockState.HasHeader(bd.Hash)
+			if err != nil && !errors.Is(err, chaindb.ErrKeyNotFound) {
+				logger.Debugf("failed to check if header is known for hash %s: %s", bd.Hash, err)
+				return
+			}
+
+			if has {
+				logger.Tracef("ignoring block we've already processed, hash=%s", bd.Hash)
+				return
+			}
+
+			// this is bad and shouldn't happen
+			logger.Errorf("block with unknown header is ready: hash=%s", bd.Hash)
 			return
 		}
 
 		bd.Header = block.header
 	}
 
 	if bd.Header == nil {
-		logger.Criticalf("new ready block number (unknown) with hash %s", bd.Hash)
+		logger.Errorf("new ready block number (unknown) with hash %s", bd.Hash)
 		return
 	}
 
 	logger.Tracef("new ready block number %s with hash %s", bd.Header.Number, bd.Hash)
 
+	// see if there are any descendents in the pending queue that are now ready to be processed,
+	// as we have just become aware of their parent block
 	ready := []*types.BlockData{bd}
-	ready = pendingBlocks.getReadyDescendants(bd.Hash, ready)
+	ready = cs.pendingBlocks.getReadyDescendants(bd.Hash, ready)
 
 	for _, rb := range ready {
-		pendingBlocks.removeBlock(rb.Hash)
-		readyBlocks.push(rb)
+		cs.pendingBlocks.removeBlock(rb.Hash)
+		cs.readyBlocks.push(rb)
 	}
 }
 

dot/sync/chain_sync_test.go

@@ -662,6 +662,7 @@ func TestChainSync_doSync(t *testing.T) {
 	resp := &network.BlockResponseMessage{
 		BlockData: []*types.BlockData{
 			{
+				Hash: common.Hash{0x1},
 				Header: &types.Header{
 					Number: big.NewInt(1),
 				},
@@ -687,13 +688,15 @@ func TestChainSync_doSync(t *testing.T) {
 	resp = &network.BlockResponseMessage{
 		BlockData: []*types.BlockData{
 			{
+				Hash: common.Hash{0x3},
 				Header: &types.Header{
 					ParentHash: parent,
 					Number:     big.NewInt(3),
 				},
 				Body: &types.Body{},
 			},
 			{
+				Hash: common.Hash{0x2},
 				Header: &types.Header{
 					Number: big.NewInt(2),
 				},
@@ -762,7 +765,7 @@ func TestHandleReadyBlock(t *testing.T) {
 	}
 	cs.pendingBlocks.addBlock(block2NotDescendant)
 
-	handleReadyBlock(block1.ToBlockData(), cs.pendingBlocks, cs.readyBlocks)
+	cs.handleReadyBlock(block1.ToBlockData())
 
 	require.False(t, cs.pendingBlocks.hasBlock(header1.Hash()))
 	require.False(t, cs.pendingBlocks.hasBlock(header2.Hash()))

dot/sync/tip_syncer.go

@@ -8,23 +8,29 @@ import (
 	"math/big"
 
 	"github.com/ChainSafe/gossamer/dot/network"
+	"github.com/ChainSafe/gossamer/dot/types"
 	"github.com/ChainSafe/gossamer/lib/common"
 )
 
 var _ workHandler = &tipSyncer{}
 
+type handleReadyBlockFunc func(*types.BlockData)
+
 // tipSyncer handles workers when syncing at the tip of the chain
 type tipSyncer struct {
-	blockState    BlockState
-	pendingBlocks DisjointBlockSet
-	readyBlocks   *blockQueue
+	blockState       BlockState
+	pendingBlocks    DisjointBlockSet
+	readyBlocks      *blockQueue
+	handleReadyBlock handleReadyBlockFunc
 }
 
-func newTipSyncer(blockState BlockState, pendingBlocks DisjointBlockSet, readyBlocks *blockQueue) *tipSyncer {
+func newTipSyncer(blockState BlockState, pendingBlocks DisjointBlockSet, readyBlocks *blockQueue,
+	handleReadyBlock handleReadyBlockFunc) *tipSyncer {
 	return &tipSyncer{
-		blockState:    blockState,
-		pendingBlocks: pendingBlocks,
-		readyBlocks:   readyBlocks,
+		blockState:       blockState,
+		pendingBlocks:    pendingBlocks,
+		readyBlocks:      readyBlocks,
+		handleReadyBlock: handleReadyBlock,
 	}
 }
 
@@ -210,7 +216,7 @@ func (s *tipSyncer) handleTick() ([]*worker, error) {
 		if has || s.readyBlocks.has(block.header.ParentHash) {
 			// block is ready, as parent is known!
 			// also, move any pendingBlocks that are descendants of this block to the ready blocks queue
-			handleReadyBlock(block.toBlockData(), s.pendingBlocks, s.readyBlocks)
+			s.handleReadyBlock(block.toBlockData())
 			continue
 		}
 

dot/sync/tip_syncer_test.go

@@ -28,7 +28,13 @@ func newTestTipSyncer(t *testing.T) *tipSyncer {
 
 	readyBlocks := newBlockQueue(maxResponseSize)
 	pendingBlocks := newDisjointBlockSet(pendingBlocksLimit)
-	return newTipSyncer(bs, pendingBlocks, readyBlocks)
+	cs := &chainSync{
+		blockState:    bs,
+		readyBlocks:   readyBlocks,
+		pendingBlocks: pendingBlocks,
+	}
+
+	return newTipSyncer(bs, pendingBlocks, readyBlocks, cs.handleReadyBlock)
 }
 
 func TestTipSyncer_handleNewPeerState(t *testing.T) {