From b527ecb08e3d786ebf59d7988c86b934425b8582 Mon Sep 17 00:00:00 2001 From: kourin Date: Wed, 21 Dec 2022 20:51:51 +0900 Subject: [PATCH] Refactor hash calculation with round --- consensus/ibft/consensus_backend.go | 25 ++++++-- consensus/ibft/ibft.go | 45 ++------------ consensus/ibft/messages.go | 27 ++++----- consensus/ibft/signer/signer.go | 10 ++-- consensus/ibft/verifier.go | 91 ++++++++++++++++++++--------- 5 files changed, 102 insertions(+), 96 deletions(-) diff --git a/consensus/ibft/consensus_backend.go b/consensus/ibft/consensus_backend.go index e45cda2592..5258d98447 100644 --- a/consensus/ibft/consensus_backend.go +++ b/consensus/ibft/consensus_backend.go @@ -144,7 +144,7 @@ func (i *backendIBFT) MaximumFaultyNodes() uint64 { func (i *backendIBFT) HasQuorum( blockNumber uint64, - messages []*proto.Message, + msgs []*proto.Message, msgType proto.MessageType, ) bool { validators, err := i.forkManager.GetValidators(blockNumber) @@ -158,13 +158,28 @@ func (i *backendIBFT) HasQuorum( return false } - if msgType == proto.MessageType_PREPREPARE { - return len(messages) > 0 + var ( + numMsgs = len(msgs) + quorum = i.quorumSize(blockNumber)(validators) + ) + + switch msgType { + case proto.MessageType_PREPREPARE: + return numMsgs >= 1 + case proto.MessageType_PREPARE: + return numMsgs >= quorum-1 + case proto.MessageType_COMMIT, proto.MessageType_ROUND_CHANGE: + return numMsgs >= quorum } - quorum := i.quorumSize(blockNumber)(validators) + // should not reach here + i.logger.Warn( + "invalid message type when calculation quorum", + "height", blockNumber, + "messageType", msgType, + ) - return len(messages) >= quorum + return false } // buildBlock builds the block, based on the passed in snapshot and parent header diff --git a/consensus/ibft/ibft.go b/consensus/ibft/ibft.go index b48af04c00..5d1250b323 100644 --- a/consensus/ibft/ibft.go +++ b/consensus/ibft/ibft.go @@ -5,13 +5,11 @@ import ( "fmt" "time" - protoIBFT "github.com/0xPolygon/go-ibft/messages/proto" "github.com/0xPolygon/polygon-edge/blockchain" "github.com/0xPolygon/polygon-edge/consensus" "github.com/0xPolygon/polygon-edge/consensus/ibft/fork" "github.com/0xPolygon/polygon-edge/consensus/ibft/proto" "github.com/0xPolygon/polygon-edge/consensus/ibft/signer" - "github.com/0xPolygon/polygon-edge/crypto" "github.com/0xPolygon/polygon-edge/helper/progress" "github.com/0xPolygon/polygon-edge/network" "github.com/0xPolygon/polygon-edge/secrets" @@ -22,7 +20,6 @@ import ( "github.com/armon/go-metrics" "github.com/hashicorp/go-hclog" "google.golang.org/grpc" - protoBuf "google.golang.org/protobuf/proto" ) const ( @@ -435,10 +432,10 @@ func (i *backendIBFT) VerifyBlock(block *types.Block) error { return err } - hashForCS, err := calcHashForCS( + hashForCS, err := i.calculateProposalHash( headerSigner, block, - extra, + extra.RoundNumber, ) if err != nil { return err @@ -458,40 +455,6 @@ func (i *backendIBFT) VerifyBlock(block *types.Block) error { return nil } -func calcHashForCS( - signer signer.Signer, - block *types.Block, - extra *signer.IstanbulExtra, -) ([]byte, error) { - if extra.RoundNumber == nil { - return block.Hash().Bytes(), nil - } - - // need to remove some fields from finalized header for hash calculation - originalHeader := block.Header - - filteredHeader, err := signer.FilterHeaderForHash(block.Header) - if err != nil { - return nil, err - } - - block.Header = filteredHeader.ComputeHash() - - proposedBlock := &protoIBFT.ProposedBlock{ - EthereumBlock: block.MarshalRLP(), - Round: *extra.RoundNumber, - } - - proposedBlockRaw, err := protoBuf.Marshal(proposedBlock) - if err != nil { - return nil, err - } - - block.Header = originalHeader - - return crypto.Keccak256(proposedBlockRaw), nil -} - // quorumSize returns a callback that when executed on a Validators computes // number of votes required to reach quorum based on the size of the set. // The blockNumber argument indicates which formula was used to calculate the result (see PRs #513, #549) @@ -638,10 +601,10 @@ func (i *backendIBFT) verifyParentCommittedSeals( return err } - parentHash, err := calcHashForCS( + parentHash, err := i.calculateProposalHash( parentSigner, parentBlock, - parentExtra, + parentExtra.RoundNumber, ) if err != nil { return err diff --git a/consensus/ibft/messages.go b/consensus/ibft/messages.go index e014286cf8..8502216862 100644 --- a/consensus/ibft/messages.go +++ b/consensus/ibft/messages.go @@ -24,34 +24,27 @@ func (i *backendIBFT) BuildPrePrepareMessage( certificate *protoIBFT.RoundChangeCertificate, view *protoIBFT.View, ) *protoIBFT.Message { - proposedBlock := &protoIBFT.ProposedBlock{ - EthereumBlock: ethereumBlock, - Round: view.Round, - } - // hash calculation begins - proposalHash, err := i.getProposalHashFromBlock(ethereumBlock, view.Round) + proposalHash, err := i.calculateProposalHashFromBlockBytes( + i.currentSigner, + ethereumBlock, + &view.Round, + ) if err != nil { return nil } - // hash calculation ends - - // TODO: Double-check that even without this we are correctly validating the block elsewhere. - //block := &types.Block{} - //if err := block.UnmarshalRLP(proposal); err != nil { - //return nil - //} - //proposalHash := block.Hash().Bytes() - msg := &protoIBFT.Message{ View: view, From: i.ID(), Type: protoIBFT.MessageType_PREPREPARE, Payload: &protoIBFT.Message_PreprepareData{ PreprepareData: &protoIBFT.PrePrepareMessage{ - Proposal: proposedBlock, - ProposalHash: proposalHash, + Proposal: &protoIBFT.ProposedBlock{ + EthereumBlock: ethereumBlock, + Round: view.Round, + }, + ProposalHash: proposalHash.Bytes(), Certificate: certificate, }, }, diff --git a/consensus/ibft/signer/signer.go b/consensus/ibft/signer/signer.go index 8d5692ae9b..77d76ecb4c 100644 --- a/consensus/ibft/signer/signer.go +++ b/consensus/ibft/signer/signer.go @@ -48,7 +48,7 @@ type Signer interface { sealMap map[types.Address][]byte, ) (*types.Header, error) VerifyCommittedSeals( - hash []byte, + hash types.Hash, committedSeals Seals, validators validators.Validators, quorumSize int, @@ -56,7 +56,7 @@ type Signer interface { // ParentCommittedSeals VerifyParentCommittedSeals( - parentHash []byte, + parentHash types.Hash, header *types.Header, parentValidators validators.Validators, quorum int, @@ -229,7 +229,7 @@ func (s *SignerImpl) WriteCommittedSeals( // VerifyCommittedSeals verifies CommittedSeals in IBFT Extra of the header func (s *SignerImpl) VerifyCommittedSeals( - hash []byte, + hash types.Hash, committedSeals Seals, validators validators.Validators, quorumSize int, @@ -256,7 +256,7 @@ func (s *SignerImpl) VerifyCommittedSeals( // VerifyParentCommittedSeals verifies ParentCommittedSeals in IBFT Extra of the header func (s *SignerImpl) VerifyParentCommittedSeals( - parentHash []byte, + parentHash types.Hash, header *types.Header, parentValidators validators.Validators, quorum int, @@ -279,7 +279,7 @@ func (s *SignerImpl) VerifyParentCommittedSeals( } rawMsg := crypto.Keccak256( - wrapCommitHash(parentHash), + wrapCommitHash(parentHash[:]), ) numSeals, err := s.keyManager.VerifyCommittedSeals( diff --git a/consensus/ibft/verifier.go b/consensus/ibft/verifier.go index aa4a3dd8bf..a9e818e140 100644 --- a/consensus/ibft/verifier.go +++ b/consensus/ibft/verifier.go @@ -6,12 +6,69 @@ import ( "github.com/0xPolygon/go-ibft/messages" protoIBFT "github.com/0xPolygon/go-ibft/messages/proto" + "github.com/0xPolygon/polygon-edge/consensus/ibft/signer" "github.com/0xPolygon/polygon-edge/crypto" "github.com/0xPolygon/polygon-edge/types" "google.golang.org/protobuf/proto" ) // Verifier impl for go-ibft +// calculateProposalHashFromBlockBytes is a helper method to marshal ethereum block in bytes +// and pass to calculateProposalHash +func (i *backendIBFT) calculateProposalHashFromBlockBytes( + signer signer.Signer, + ethereumBlockBytes []byte, + round *uint64, +) (types.Hash, error) { + ethereumBlock := &types.Block{} + if err := ethereumBlock.UnmarshalRLP(ethereumBlockBytes); err != nil { + return types.ZeroHash, err + } + + return i.calculateProposalHash( + signer, + ethereumBlock, + round, + ) +} + +// calculateProposalHash is new hash calculation for proposal in go-ibft, +// which includes round number block is finalized at +func (i *backendIBFT) calculateProposalHash( + signer signer.Signer, + block *types.Block, + round *uint64, +) (types.Hash, error) { + if round == nil { + // legacy hash calculation + return block.Hash(), nil + } + + filteredHeader, err := signer.FilterHeaderForHash(block.Header) + if err != nil { + return types.ZeroHash, err + } + + blockForHash := &types.Block{ + Header: filteredHeader.ComputeHash(), + Transactions: block.Transactions, + Uncles: block.Uncles, + } + + proposedBlock := &protoIBFT.ProposedBlock{ + EthereumBlock: blockForHash.MarshalRLP(), + Round: *round, + } + + proposedBlockBytes, err := proto.Marshal(proposedBlock) + if err != nil { + return types.ZeroHash, err + } + + return types.BytesToHash( + crypto.Keccak256(proposedBlockBytes), + ), nil +} func (i *backendIBFT) IsValidBlock(proposal []byte) bool { var ( @@ -137,38 +194,16 @@ func (i *backendIBFT) IsProposer(id []byte, height, round uint64) bool { } func (i *backendIBFT) IsValidProposalHash(proposal *protoIBFT.ProposedBlock, hash []byte) bool { - proposalHash, err := i.getProposalHashFromBlock(proposal.EthereumBlock, proposal.Round) + proposalHash, err := i.calculateProposalHashFromBlockBytes( + i.currentSigner, + proposal.EthereumBlock, + &proposal.Round, + ) if err != nil { return false } - return bytes.Equal(proposalHash, hash) -} - -func (i *backendIBFT) getProposalHashFromBlock(ethereumBlock []byte, round uint64) ([]byte, error) { - block := &types.Block{} - if err := block.UnmarshalRLP(ethereumBlock); err != nil { - return nil, err - } - - filteredHeader, err := i.currentSigner.FilterHeaderForHash(block.Header) - if err != nil { - return nil, err - } - - block.Header = filteredHeader.ComputeHash() - - proposedBlockForHash := &protoIBFT.ProposedBlock{ - EthereumBlock: block.MarshalRLP(), - Round: round, - } - - proposedBlockRaw, err := proto.Marshal(proposedBlockForHash) - if err != nil { - return nil, err - } - - return crypto.Keccak256(proposedBlockRaw), nil + return bytes.Equal(proposalHash.Bytes(), hash) } func (i *backendIBFT) IsValidCommittedSeal(