Return FullBlock from syncer (#1110)

* Return FullBlock from syncer

* Comments fix

* Comments fix

* Extract common parts of WriteBlock and WriteFullBlock

* Revert "Extract common parts of WriteBlock and WriteFullBlock"

This reverts commit 62166e6.

Co-authored-by: Stefan Negovanović <[email protected]>

archive/restore.go

@@ -23,7 +23,7 @@ type blockchainInterface interface {
 	GetBlockByNumber(uint64, bool) (*types.Block, bool)
 	GetHashByNumber(uint64) types.Hash
 	WriteBlock(*types.Block, string) error
-	VerifyFinalizedBlock(*types.Block) error
+	VerifyFinalizedBlock(*types.Block) (*types.FullBlock, error)
 }
 
 // RestoreChain reads blocks from the archive and write to the chain
@@ -78,7 +78,7 @@ func importBlocks(chain blockchainInterface, blockStream *blockStream, progressi
 	nextBlock := firstBlock
 
 	for {
-		if err := chain.VerifyFinalizedBlock(nextBlock); err != nil {
+		if _, err := chain.VerifyFinalizedBlock(nextBlock); err != nil {
 			return err
 		}
 

archive/restore_test.go

@@ -55,8 +55,8 @@ func (m *mockChain) WriteBlock(block *types.Block, _ string) error {
 	return nil
 }
 
-func (m *mockChain) VerifyFinalizedBlock(block *types.Block) error {
-	return nil
+func (m *mockChain) VerifyFinalizedBlock(block *types.Block) (*types.FullBlock, error) {
+	return &types.FullBlock{Block: block}, nil
 }
 
 func (m *mockChain) SubscribeEvents() blockchain.Subscription {

blockchain/blockchain.go

@@ -679,44 +679,48 @@ func (b *Blockchain) WriteHeadersWithBodies(headers []*types.Header) error {
 // outside the method call
 func (b *Blockchain) VerifyPotentialBlock(block *types.Block) error {
 	// Do just the initial block verification
-	return b.verifyBlock(block)
+	_, err := b.verifyBlock(block)
+
+	return err
 }
 
 // VerifyFinalizedBlock verifies that the block is valid by performing a series of checks.
 // It is assumed that the block status is sealed (committed)
-func (b *Blockchain) VerifyFinalizedBlock(block *types.Block) error {
+func (b *Blockchain) VerifyFinalizedBlock(block *types.Block) (*types.FullBlock, error) {
 	// Make sure the consensus layer verifies this block header
 	if err := b.consensus.VerifyHeader(block.Header); err != nil {
-		return fmt.Errorf("failed to verify the header: %w", err)
+		return nil, fmt.Errorf("failed to verify the header: %w", err)
 	}
 
 	// Do the initial block verification
-	if err := b.verifyBlock(block); err != nil {
-		return err
+	receipts, err := b.verifyBlock(block)
+	if err != nil {
+		return nil, err
 	}
 
-	return nil
+	return &types.FullBlock{Block: block, Receipts: receipts}, nil
 }
 
 // verifyBlock does the base (common) block verification steps by
 // verifying the block body as well as the parent information
-func (b *Blockchain) verifyBlock(block *types.Block) error {
+func (b *Blockchain) verifyBlock(block *types.Block) ([]*types.Receipt, error) {
 	// Make sure the block is present
 	if block == nil {
-		return ErrNoBlock
+		return nil, ErrNoBlock
 	}
 
 	// Make sure the block is in line with the parent block
 	if err := b.verifyBlockParent(block); err != nil {
-		return err
+		return nil, err
 	}
 
 	// Make sure the block body data is valid
-	if err := b.verifyBlockBody(block); err != nil {
-		return err
+	receipts, err := b.verifyBlockBody(block)
+	if err != nil {
+		return nil, err
 	}
 
-	return nil
+	return receipts, nil
 }
 
 // verifyBlockParent makes sure that the child block is in line
@@ -774,7 +778,7 @@ func (b *Blockchain) verifyBlockParent(childBlock *types.Block) error {
 // - The trie roots match up (state, transactions, receipts, uncles)
 // - The receipts match up
 // - The execution result matches up
-func (b *Blockchain) verifyBlockBody(block *types.Block) error {
+func (b *Blockchain) verifyBlockBody(block *types.Block) ([]*types.Receipt, error) {
 	// Make sure the Uncles root matches up
 	if hash := buildroot.CalculateUncleRoot(block.Uncles); hash != block.Header.Sha3Uncles {
 		b.logger.Error(fmt.Sprintf(
@@ -783,7 +787,7 @@ func (b *Blockchain) verifyBlockBody(block *types.Block) error {
 			block.Header.Sha3Uncles,
 		))
 
-		return ErrInvalidSha3Uncles
+		return nil, ErrInvalidSha3Uncles
 	}
 
 	// Make sure the transactions root matches up
@@ -794,21 +798,21 @@ func (b *Blockchain) verifyBlockBody(block *types.Block) error {
 			block.Header.TxRoot,
 		))
 
-		return ErrInvalidTxRoot
+		return nil, ErrInvalidTxRoot
 	}
 
 	// Execute the transactions in the block and grab the result
 	blockResult, executeErr := b.executeBlockTransactions(block)
 	if executeErr != nil {
-		return fmt.Errorf("unable to execute block transactions, %w", executeErr)
+		return nil, fmt.Errorf("unable to execute block transactions, %w", executeErr)
 	}
 
 	// Verify the local execution result with the proposed block data
 	if err := blockResult.verifyBlockResult(block); err != nil {
-		return fmt.Errorf("unable to verify block execution result, %w", err)
+		return nil, fmt.Errorf("unable to verify block execution result, %w", err)
 	}
 
-	return nil
+	return blockResult.Receipts, nil
 }
 
 // verifyBlockResult verifies that the block transaction execution result

blockchain/blockchain_test.go

@@ -1202,7 +1202,8 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 			},
 		}
 
-		assert.ErrorIs(t, blockchain.verifyBlockBody(block), ErrInvalidSha3Uncles)
+		_, err = blockchain.verifyBlockBody(block)
+		assert.ErrorIs(t, err, ErrInvalidSha3Uncles)
 	})
 
 	t.Run("Invalid Transactions root", func(t *testing.T) {
@@ -1219,7 +1220,8 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 			},
 		}
 
-		assert.ErrorIs(t, blockchain.verifyBlockBody(block), ErrInvalidTxRoot)
+		_, err = blockchain.verifyBlockBody(block)
+		assert.ErrorIs(t, err, ErrInvalidTxRoot)
 	})
 
 	t.Run("Invalid execution result - missing parent", func(t *testing.T) {
@@ -1246,7 +1248,8 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 			},
 		}
 
-		assert.ErrorIs(t, blockchain.verifyBlockBody(block), ErrParentNotFound)
+		_, err = blockchain.verifyBlockBody(block)
+		assert.ErrorIs(t, err, ErrParentNotFound)
 	})
 
 	t.Run("Invalid execution result - unable to fetch block creator", func(t *testing.T) {
@@ -1285,7 +1288,8 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 			},
 		}
 
-		assert.ErrorIs(t, blockchain.verifyBlockBody(block), errBlockCreatorNotFound)
+		_, err = blockchain.verifyBlockBody(block)
+		assert.ErrorIs(t, err, errBlockCreatorNotFound)
 	})
 
 	t.Run("Invalid execution result - unable to execute transactions", func(t *testing.T) {
@@ -1327,6 +1331,7 @@ func TestBlockchain_VerifyBlockBody(t *testing.T) {
 			},
 		}
 
-		assert.ErrorIs(t, blockchain.verifyBlockBody(block), errUnableToExecute)
+		_, err = blockchain.verifyBlockBody(block)
+		assert.ErrorIs(t, err, errUnableToExecute)
 	})
 }

consensus/dev/dev.go

@@ -197,7 +197,7 @@ func (d *Dev) writeNewBlock(parent *types.Header) error {
 		Receipts: transition.Receipts(),
 	})
 
-	if err := d.blockchain.VerifyFinalizedBlock(block); err != nil {
+	if _, err := d.blockchain.VerifyFinalizedBlock(block); err != nil {
 		return err
 	}
 

consensus/ibft/ibft.go

@@ -208,16 +208,16 @@ func (i *backendIBFT) Initialize() error {
 
 // sync runs the syncer in the background to receive blocks from advanced peers
 func (i *backendIBFT) startSyncing() {
-	callInsertBlockHook := func(block *types.Block) bool {
-		if err := i.currentHooks.PostInsertBlock(block); err != nil {
-			i.logger.Error("failed to call PostInsertBlock", "height", block.Header.Number, "error", err)
+	callInsertBlockHook := func(fullBlock *types.FullBlock) bool {
+		if err := i.currentHooks.PostInsertBlock(fullBlock.Block); err != nil {
+			i.logger.Error("failed to call PostInsertBlock", "height", fullBlock.Block.Header.Number, "error", err)
 		}
 
-		if err := i.updateCurrentModules(block.Number() + 1); err != nil {
-			i.logger.Error("failed to update sub modules", "height", block.Number()+1, "err", err)
+		if err := i.updateCurrentModules(fullBlock.Block.Number() + 1); err != nil {
+			i.logger.Error("failed to update sub modules", "height", fullBlock.Block.Number()+1, "err", err)
 		}
 
-		i.txpool.ResetWithHeaders(block.Header)
+		i.txpool.ResetWithHeaders(fullBlock.Block.Header)
 
 		return false
 	}

consensus/polybft/consensus_runtime.go

@@ -225,52 +225,52 @@ func (c *consensusRuntime) IsBridgeEnabled() bool {
 }
 
 // OnBlockInserted is called whenever fsm or syncer inserts new block
-func (c *consensusRuntime) OnBlockInserted(block *types.Block) {
+func (c *consensusRuntime) OnBlockInserted(fullBlock *types.FullBlock) {
 	c.lock.Lock()
 	defer c.lock.Unlock()
 
-	if c.lastBuiltBlock != nil && c.lastBuiltBlock.Number >= block.Number() {
+	if c.lastBuiltBlock != nil && c.lastBuiltBlock.Number >= fullBlock.Block.Number() {
 		c.logger.Debug("on block inserted already handled",
-			"current", c.lastBuiltBlock.Number, "block", block.Number())
+			"current", c.lastBuiltBlock.Number, "block", fullBlock.Block.Number())
 
 		return
 	}
 
-	if err := updateBlockMetrics(block, c.lastBuiltBlock); err != nil {
+	if err := updateBlockMetrics(fullBlock.Block, c.lastBuiltBlock); err != nil {
 		c.logger.Error("failed to update block metrics", "error", err)
 	}
 
 	// after the block has been written we reset the txpool so that the old transactions are removed
-	c.config.txPool.ResetWithHeaders(block.Header)
+	c.config.txPool.ResetWithHeaders(fullBlock.Block.Header)
 
 	var (
 		epoch = c.epoch
 		err   error
 	)
 
 	// handle commitment and proofs creation
-	if err := c.stateSyncManager.PostBlock(&PostBlockRequest{Block: block}); err != nil {
+	if err := c.stateSyncManager.PostBlock(&PostBlockRequest{Block: fullBlock.Block}); err != nil {
 		c.logger.Error("failed to post block state sync", "err", err)
 	}
 
 	// TODO - this condition will need to be changed to recognize that either slashing happened
 	// or epoch reached its fixed size
-	if c.isFixedSizeOfEpochMet(block.Header.Number, epoch) {
-		if epoch, err = c.restartEpoch(block.Header); err != nil {
+	if c.isFixedSizeOfEpochMet(fullBlock.Block.Header.Number, epoch) {
+		if epoch, err = c.restartEpoch(fullBlock.Block.Header); err != nil {
 			c.logger.Error("failed to restart epoch after block inserted", "error", err)
 
 			return
 		}
 	}
 
-	if err := c.proposerCalculator.Update(block.Number()); err != nil {
+	if err := c.proposerCalculator.Update(fullBlock.Block.Number()); err != nil {
 		// do not return if proposer snapshot hasn't been inserted, next call of OnBlockInserted will catch-up
 		c.logger.Warn("Could not update proposer calculator", "err", err)
 	}
 
 	// finally update runtime state (lastBuiltBlock, epoch, proposerSnapshot)
 	c.epoch = epoch
-	c.lastBuiltBlock = block.Header
+	c.lastBuiltBlock = fullBlock.Block.Header
 }
 
 // FSM creates a new instance of fsm
@@ -731,30 +731,30 @@ func (c *consensusRuntime) BuildProposal(view *proto.View) []byte {
 func (c *consensusRuntime) InsertBlock(proposal []byte, committedSeals []*messages.CommittedSeal) {
 	fsm := c.fsm
 
-	block, err := fsm.Insert(proposal, committedSeals)
+	fullBlock, err := fsm.Insert(proposal, committedSeals)
 	if err != nil {
 		c.logger.Error("cannot insert proposal", "error", err)
 
 		return
 	}
 
 	if c.IsBridgeEnabled() {
-		if (fsm.isEndOfEpoch || c.checkpointManager.isCheckpointBlock(block.Header.Number)) &&
-			bytes.Equal(c.config.Key.Address().Bytes(), block.Header.Miner) {
+		if (fsm.isEndOfEpoch || c.checkpointManager.isCheckpointBlock(fullBlock.Block.Header.Number)) &&
+			bytes.Equal(c.config.Key.Address().Bytes(), fullBlock.Block.Header.Miner) {
 			go func(header types.Header, epochNumber uint64) {
 				if err := c.checkpointManager.submitCheckpoint(header, fsm.isEndOfEpoch); err != nil {
 					c.logger.Warn("failed to submit checkpoint",
 						"checkpoint block", header.Number,
 						"epoch number", epochNumber,
 						"error", err)
 				}
-			}(*block.Header, fsm.epochNumber)
+			}(*fullBlock.Block.Header, fsm.epochNumber)
 
-			c.checkpointManager.latestCheckpointID = block.Number()
+			c.checkpointManager.latestCheckpointID = fullBlock.Block.Number()
 		}
 	}
 
-	c.OnBlockInserted(block)
+	c.OnBlockInserted(fullBlock)
 }
 
 // ID return ID (address actually) of the current node

consensus/polybft/consensus_runtime_test.go

@@ -227,7 +227,7 @@ func TestConsensusRuntime_OnBlockInserted_EndOfEpoch(t *testing.T) {
 		lastBuiltBlock:   &types.Header{Number: header.Number - 1},
 		stateSyncManager: &dummyStateSyncManager{},
 	}
-	runtime.OnBlockInserted(builtBlock)
+	runtime.OnBlockInserted(&types.FullBlock{Block: builtBlock})
 
 	require.True(t, runtime.state.isEpochInserted(currentEpochNumber+1))
 	require.Equal(t, newEpochNumber, runtime.epoch.Number)
@@ -281,7 +281,7 @@ func TestConsensusRuntime_OnBlockInserted_MiddleOfEpoch(t *testing.T) {
 		logger:             hclog.NewNullLogger(),
 		proposerCalculator: NewProposerCalculatorFromSnapshot(snapshot, config, hclog.NewNullLogger()),
 	}
-	runtime.OnBlockInserted(builtBlock)
+	runtime.OnBlockInserted(&types.FullBlock{Block: builtBlock})
 
 	require.Equal(t, header.Number, runtime.lastBuiltBlock.Number)
 }

consensus/polybft/fsm.go

@@ -406,14 +406,13 @@ func (f *fsm) VerifyStateTransactions(transactions []*types.Transaction) error {
 }
 
 // Insert inserts the sealed proposal
-func (f *fsm) Insert(proposal []byte, committedSeals []*messages.CommittedSeal) (*types.Block, error) {
-	newBlock := f.target.Block
-	receipts := f.target.Receipts
+func (f *fsm) Insert(proposal []byte, committedSeals []*messages.CommittedSeal) (*types.FullBlock, error) {
+	newBlock := f.target
 
 	// In this function we should try to return little to no errors since
 	// at this point everything we have to do is just commit something that
 	// we should have already computed beforehand.
-	extra, _ := GetIbftExtra(newBlock.Header.ExtraData)
+	extra, _ := GetIbftExtra(newBlock.Block.Header.ExtraData)
 
 	// create map for faster access to indexes
 	nodeIDIndexMap := make(map[types.Address]int, f.validators.Len())
@@ -457,9 +456,9 @@ func (f *fsm) Insert(proposal []byte, committedSeals []*messages.CommittedSeal)
 	}
 
 	// Write extar data to header
-	newBlock.Header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...)
+	newBlock.Block.Header.ExtraData = append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...)
 
-	if err := f.backend.CommitBlock(&types.FullBlock{Block: newBlock, Receipts: receipts}); err != nil {
+	if err := f.backend.CommitBlock(newBlock); err != nil {
 		return nil, err
 	}
 
@@ -471,7 +470,7 @@ func (f *fsm) Insert(proposal []byte, committedSeals []*messages.CommittedSeal)
 	}
 
 	// commit exit events only when we finalize a block
-	events, err := getExitEventsFromReceipts(epoch, newBlock.Number(), receipts)
+	events, err := getExitEventsFromReceipts(epoch, newBlock.Block.Number(), newBlock.Receipts)
 	if err != nil {
 		return newBlock, err
 	}