Refactor hash calculation with round

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

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

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,
 			},
 		},

consensus/ibft/signer/signer.go

@@ -48,15 +48,15 @@ type Signer interface {
 		sealMap map[types.Address][]byte,
 	) (*types.Header, error)
 	VerifyCommittedSeals(
-		hash []byte,
+		hash types.Hash,
 		committedSeals Seals,
 		validators validators.Validators,
 		quorumSize int,
 	) error
 
 	// 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(

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(