Refactored verify block. (#4554)

consensus/consensus.go

@@ -39,9 +39,6 @@ const (
 	AsyncProposal
 )
 
-// VerifyBlockFunc is a function used to verify the block and keep trace of verified blocks
-type VerifyBlockFunc func(*types.Block) error
-
 // Consensus is the main struct with all states and data related to consensus process.
 type Consensus struct {
 	Decider quorum.Decider
@@ -94,8 +91,6 @@ type Consensus struct {
 	// The post-consensus job func passed from Node object
 	// Called when consensus on a new block is done
 	PostConsensusJob func(*types.Block) error
-	// The verifier func passed from Node object
-	BlockVerifier VerifyBlockFunc
 	// verified block to state sync broadcast
 	VerifiedNewBlock chan *types.Block
 	// Channel for DRG protocol to send pRnd (preimage of randomness resulting from combined vrf
@@ -168,7 +163,7 @@ func (consensus *Consensus) Beaconchain() core.BlockChain {
 	return consensus.registry.GetBeaconchain()
 }
 
-// VerifyBlock is a function used to verify the block and keep trace of verified blocks.
+// verifyBlock is a function used to verify the block and keep trace of verified blocks.
 func (consensus *Consensus) verifyBlock(block *types.Block) error {
 	if !consensus.fBFTLog.IsBlockVerified(block.Hash()) {
 		if err := consensus.BlockVerifier(block); err != nil {
@@ -302,11 +297,6 @@ func New(
 	consensus.IgnoreViewIDCheck = abool.NewBool(false)
 	// Make Sure Verifier is not null
 	consensus.vc = newViewChange()
-	// TODO: reference to blockchain/beaconchain should be removed.
-	verifier := VerifyNewBlock(registry.GetWebHooks(), consensus.Blockchain(), consensus.Beaconchain())
-	consensus.BlockVerifier = verifier
-	consensus.vc.verifyBlock = consensus.verifyBlock
-
 	// init prometheus metrics
 	initMetrics()
 	consensus.AddPubkeyMetrics()

consensus/consensus_service.go

@@ -660,40 +660,38 @@ func (consensus *Consensus) getLogger() *zerolog.Logger {
 	return &logger
 }
 
-// VerifyNewBlock is called by consensus participants to verify the block (account model) they are
+// BlockVerifier is called by consensus participants to verify the block (account model) they are
 // running consensus on.
-func VerifyNewBlock(hooks *webhooks.Hooks, blockChain core.BlockChain, beaconChain core.BlockChain) func(*types.Block) error {
-	return func(newBlock *types.Block) error {
-		if err := blockChain.ValidateNewBlock(newBlock, beaconChain); err != nil {
-			switch {
-			case errors.Is(err, core.ErrKnownBlock):
-				return nil
-			default:
-			}
+func (consensus *Consensus) BlockVerifier(newBlock *types.Block) error {
+	if err := consensus.Blockchain().ValidateNewBlock(newBlock, consensus.Beaconchain()); err != nil {
+		switch {
+		case errors.Is(err, core.ErrKnownBlock):
+			return nil
+		default:
+		}
 
-			if hooks := hooks; hooks != nil {
-				if p := hooks.ProtocolIssues; p != nil {
-					url := p.OnCannotCommit
-					go func() {
-						webhooks.DoPost(url, map[string]interface{}{
-							"bad-header": newBlock.Header(),
-							"reason":     err.Error(),
-						})
-					}()
-				}
+		if hooks := consensus.registry.GetWebHooks(); hooks != nil {
+			if p := hooks.ProtocolIssues; p != nil {
+				url := p.OnCannotCommit
+				go func() {
+					webhooks.DoPost(url, map[string]interface{}{
+						"bad-header": newBlock.Header(),
+						"reason":     err.Error(),
+					})
+				}()
 			}
-			utils.Logger().Error().
-				Str("blockHash", newBlock.Hash().Hex()).
-				Int("numTx", len(newBlock.Transactions())).
-				Int("numStakingTx", len(newBlock.StakingTransactions())).
-				Err(err).
-				Msgf("[VerifyNewBlock] Cannot Verify New Block!!!, blockHeight %d, myHeight %d", newBlock.NumberU64(), blockChain.CurrentHeader().NumberU64())
-			return errors.WithMessagef(err,
-				"[VerifyNewBlock] Cannot Verify New Block!!! block-hash %s txn-count %d",
-				newBlock.Hash().Hex(),
-				len(newBlock.Transactions()),
-			)
 		}
-		return nil
+		utils.Logger().Error().
+			Str("blockHash", newBlock.Hash().Hex()).
+			Int("numTx", len(newBlock.Transactions())).
+			Int("numStakingTx", len(newBlock.StakingTransactions())).
+			Err(err).
+			Msgf("[VerifyNewBlock] Cannot Verify New Block!!!, blockHeight %d, myHeight %d", newBlock.NumberU64(), consensus.Blockchain().CurrentHeader().NumberU64())
+		return errors.WithMessagef(err,
+			"[VerifyNewBlock] Cannot Verify New Block!!! block-hash %s txn-count %d",
+			newBlock.Hash().Hex(),
+			len(newBlock.Transactions()),
+		)
 	}
+	return nil
 }

consensus/consensus_v2.go

@@ -469,9 +469,6 @@ func (consensus *Consensus) GetLastMileBlockIter(bnStart uint64, cb func(iter *L
 
 // GetLastMileBlockIter get the iterator of the last mile blocks starting from number bnStart
 func (consensus *Consensus) getLastMileBlockIter(bnStart uint64, cb func(iter *LastMileBlockIter) error) error {
-	if consensus.BlockVerifier == nil {
-		return errors.New("consensus haven't initialized yet")
-	}
 	blocks, _, err := consensus.getLastMileBlocksAndMsg(bnStart)
 	if err != nil {
 		return err
@@ -620,10 +617,6 @@ func (consensus *Consensus) verifyLastCommitSig(lastCommitSig []byte, blk *types
 
 // tryCatchup add the last mile block in PBFT log memory cache to blockchain.
 func (consensus *Consensus) tryCatchup() error {
-	// TODO: change this to a more systematic symbol
-	if consensus.BlockVerifier == nil {
-		return errors.New("consensus haven't finished initialization")
-	}
 	initBN := consensus.getBlockNum()
 	defer consensus.postCatchup(initBN)
 

consensus/validator.go

@@ -125,11 +125,6 @@ func (consensus *Consensus) validateNewBlock(recvMsg *FBFTMessage) (*types.Block
 		Hex("blockHash", recvMsg.BlockHash[:]).
 		Msg("[validateNewBlock] Prepared message and block added")
 
-	if consensus.BlockVerifier == nil {
-		consensus.getLogger().Debug().Msg("[validateNewBlock] consensus received message before init. Ignoring")
-		return nil, errors.New("nil block verifier")
-	}
-
 	if err := consensus.verifyBlock(&blockObj); err != nil {
 		consensus.getLogger().Error().Err(err).Msg("[validateNewBlock] Block verification failed")
 		return nil, errors.Errorf("Block verification failed: %s", err.Error())

consensus/view_change.go

@@ -290,7 +290,9 @@ func (consensus *Consensus) startViewChange() {
 		nextViewID,
 		consensus.getBlockNum(),
 		consensus.priKey,
-		members); err != nil {
+		members,
+		consensus.verifyBlock,
+	); err != nil {
 		consensus.getLogger().Error().Err(err).Msg("[startViewChange] Init Payload Error")
 	}
 
@@ -406,16 +408,19 @@ func (consensus *Consensus) onViewChange(recvMsg *FBFTMessage) {
 	consensus.vc.AddViewIDKeyIfNotExist(recvMsg.ViewID, members)
 
 	// do it once only per viewID/Leader
-	if err := consensus.vc.InitPayload(consensus.fBFTLog,
+	if err := consensus.vc.InitPayload(
+		consensus.fBFTLog,
 		recvMsg.ViewID,
 		recvMsg.BlockNum,
 		consensus.priKey,
-		members); err != nil {
+		members,
+		consensus.verifyBlock,
+	); err != nil {
 		consensus.getLogger().Error().Err(err).Msg("[onViewChange] Init Payload Error")
 		return
 	}
 
-	err = consensus.vc.ProcessViewChangeMsg(consensus.fBFTLog, consensus.Decider, recvMsg)
+	err = consensus.vc.ProcessViewChangeMsg(consensus.fBFTLog, consensus.Decider, recvMsg, consensus.verifyBlock)
 	if err != nil {
 		consensus.getLogger().Error().Err(err).
 			Uint64("viewID", recvMsg.ViewID).
@@ -483,7 +488,7 @@ func (consensus *Consensus) onNewView(recvMsg *FBFTMessage) {
 		return
 	}
 
-	preparedBlock, err := consensus.vc.VerifyNewViewMsg(recvMsg)
+	preparedBlock, err := consensus.vc.VerifyNewViewMsg(recvMsg, consensus.verifyBlock)
 	if err != nil {
 		consensus.getLogger().Warn().Err(err).Msg("[onNewView] Verify New View Msg Failed")
 		return

consensus/view_change_construct.go

@@ -46,7 +46,6 @@ type viewChange struct {
 
 	m1Payload []byte // message payload for type m1 := |vcBlockHash|prepared_agg_sigs|prepared_bitmap|, new leader only need one
 
-	verifyBlock        VerifyBlockFunc
 	viewChangeDuration time.Duration
 }
 
@@ -152,7 +151,7 @@ func (vc *viewChange) GetM3Bitmap(viewID uint64) ([]byte, []byte) {
 }
 
 // VerifyNewViewMsg returns true if the new view message is valid
-func (vc *viewChange) VerifyNewViewMsg(recvMsg *FBFTMessage) (*types.Block, error) {
+func (vc *viewChange) VerifyNewViewMsg(recvMsg *FBFTMessage, verifyBlock func(block *types.Block) error) (*types.Block, error) {
 	if recvMsg.M3AggSig == nil || recvMsg.M3Bitmap == nil {
 		return nil, errors.New("[VerifyNewViewMsg] M3AggSig or M3Bitmap is nil")
 	}
@@ -215,7 +214,7 @@ func (vc *viewChange) VerifyNewViewMsg(recvMsg *FBFTMessage) (*types.Block, erro
 		if !bytes.Equal(preparedBlockHash[:], blockHash) {
 			return nil, errors.New("[VerifyNewViewMsg] Prepared block hash doesn't match msg block hash")
 		}
-		if err := vc.verifyBlock(preparedBlock); err != nil {
+		if err := verifyBlock(preparedBlock); err != nil {
 			return nil, err
 		}
 		return preparedBlock, nil
@@ -239,6 +238,7 @@ func (vc *viewChange) ProcessViewChangeMsg(
 	fbftlog *FBFTLog,
 	decider quorum.Decider,
 	recvMsg *FBFTMessage,
+	verifyBlock func(block *types.Block) error,
 ) error {
 	preparedBlock := &types.Block{}
 	if !recvMsg.HasSingleSender() {
@@ -256,7 +256,7 @@ func (vc *viewChange) ProcessViewChangeMsg(
 		if err := rlp.DecodeBytes(recvMsg.Block, preparedBlock); err != nil {
 			return err
 		}
-		if err := vc.verifyBlock(preparedBlock); err != nil {
+		if err := verifyBlock(preparedBlock); err != nil {
 			return err
 		}
 		_, ok := vc.bhpSigs[recvMsg.ViewID][senderKeyStr]
@@ -381,6 +381,7 @@ func (vc *viewChange) InitPayload(
 	blockNum uint64,
 	privKeys multibls.PrivateKeys,
 	members multibls.PublicKeys,
+	verifyBlock func(block *types.Block) error,
 ) error {
 	// m1 or m2 init once per viewID/key.
 	// m1 and m2 are mutually exclusive.
@@ -405,7 +406,7 @@ func (vc *viewChange) InitPayload(
 		hasBlock := false
 		if preparedMsg != nil {
 			if preparedBlock := fbftlog.GetBlockByHash(preparedMsg.BlockHash); preparedBlock != nil {
-				if err := vc.verifyBlock(preparedBlock); err == nil {
+				if err := verifyBlock(preparedBlock); err == nil {
 					vc.getLogger().Info().Uint64("viewID", viewID).Uint64("blockNum", blockNum).Int("size", binary.Size(preparedBlock)).Msg("[InitPayload] add my M1 (prepared) type messaage")
 					msgToSign := append(preparedMsg.BlockHash[:], preparedMsg.Payload...)
 					for _, key := range privKeys {

node/node_handler_test.go

@@ -134,7 +134,7 @@ func TestVerifyNewBlock(t *testing.T) {
 
 	// work around vrf verification as it's tested in another test.
 	node.Blockchain().Config().VRFEpoch = big.NewInt(2)
-	if err := consensus.VerifyNewBlock(nil, node.Blockchain(), node.Beaconchain())(block); err != nil {
+	if err := node.Blockchain().ValidateNewBlock(block, node.Beaconchain()); err != nil {
 		t.Error("New block is not verified successfully:", err)
 	}
 }

node/node_newblock_test.go

@@ -74,7 +74,7 @@ func TestFinalizeNewBlockAsync(t *testing.T) {
 		commitSigs, func() uint64 { return 0 }, common.Address{}, nil, nil,
 	)
 
-	if err := consensus.VerifyNewBlock(nil, blockchain, nil)(block); err != nil {
+	if err := blockchain.ValidateNewBlock(block, blockchain); err != nil {
 		t.Error("New block is not verified successfully:", err)
 	}