From d3de65064382e1c0b4566f3641747ff3e6414773 Mon Sep 17 00:00:00 2001 From: frozen <355847+Frozen@users.noreply.github.com> Date: Wed, 8 Nov 2023 15:20:55 -0400 Subject: [PATCH] Refactored verify block. --- consensus/consensus.go | 12 +----- consensus/consensus_service.go | 60 +++++++++++++++--------------- consensus/consensus_v2.go | 7 ---- consensus/validator.go | 5 --- consensus/view_change.go | 15 +++++--- consensus/view_change_construct.go | 11 +++--- node/node_handler_test.go | 2 +- node/node_newblock_test.go | 2 +- 8 files changed, 48 insertions(+), 66 deletions(-) diff --git a/consensus/consensus.go b/consensus/consensus.go index b396f6eadd..61fd37f2b8 100644 --- a/consensus/consensus.go +++ b/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 // will trigger state syncing when blockNum is low @@ -170,7 +165,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 { @@ -305,11 +300,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() diff --git a/consensus/consensus_service.go b/consensus/consensus_service.go index cd15333a01..c3b0a5ce45 100644 --- a/consensus/consensus_service.go +++ b/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 } diff --git a/consensus/consensus_v2.go b/consensus/consensus_v2.go index b3c94a77fc..c963f6d149 100644 --- a/consensus/consensus_v2.go +++ b/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) diff --git a/consensus/validator.go b/consensus/validator.go index 0506f4359d..c148a61890 100644 --- a/consensus/validator.go +++ b/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()) diff --git a/consensus/view_change.go b/consensus/view_change.go index efc1760e84..d03ae5a13a 100644 --- a/consensus/view_change.go +++ b/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 diff --git a/consensus/view_change_construct.go b/consensus/view_change_construct.go index 061d2a795c..fcf025e74d 100644 --- a/consensus/view_change_construct.go +++ b/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 { diff --git a/node/node_handler_test.go b/node/node_handler_test.go index a5085652b0..867a9616dc 100644 --- a/node/node_handler_test.go +++ b/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) } } diff --git a/node/node_newblock_test.go b/node/node_newblock_test.go index 86dd1e6c7e..5780b7cda0 100644 --- a/node/node_newblock_test.go +++ b/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) }