IBFT: Ensure that Committed Seal is not zero value (#1118)

* Ensure there is no zero addr Committed Seal When a commit message is processed, if the Proposal is missing due to some reason, we should not generate a Zero address Committed Seal. Such an incorrect seal causes `BAD BLOCK` failure with an `invalid signature` error during operations such as Block import * run gofmt * Remove stale comment * Add unit test * Revert "Remove stale comment" This reverts commit 279be71. * Move comment * Update test with error checking Co-authored-by: baptiste-b-pegasys <[email protected]> Co-authored-by: baptiste-b-pegasys <[email protected]>
Consensys · Sep 30, 2021 · 743e4a1 · 743e4a1
1 parent b57775d
commit 743e4a1
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 2 deletions.
diff --git a/consensus/istanbul/ibft/core/core.go b/consensus/istanbul/ibft/core/core.go
@@ -97,11 +97,11 @@ func (c *core) finalizeMessage(msg *ibfttypes.Message) ([]byte, error) {
 	// Add sender address
 	msg.Address = c.Address()
 
-	// Add proof of consensus
-	msg.CommittedSeal = []byte{}
 	// Assign the CommittedSeal if it's a COMMIT message and proposal is not nil
 	if msg.Code == ibfttypes.MsgCommit && c.current.Proposal() != nil {
+		msg.CommittedSeal = []byte{}
 		seal := PrepareCommittedSeal(c.current.Proposal().Hash())
+		// Add proof of consensus
 		msg.CommittedSeal, err = c.backend.Sign(seal)
 		if err != nil {
 			return nil, err

diff --git a/consensus/istanbul/ibft/core/core_test.go b/consensus/istanbul/ibft/core/core_test.go
@@ -25,6 +25,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
+	ibfttypes "github.com/ethereum/go-ethereum/consensus/istanbul/ibft/types"
 	"github.com/ethereum/go-ethereum/core/types"
 	elog "github.com/ethereum/go-ethereum/log"
 )
@@ -95,3 +96,37 @@ func TestQuorumSize(t *testing.T) {
 		}
 	}
 }
+
+func TestNilCommittedSealWithEmptyProposal(t *testing.T) {
+	N := uint64(4)
+	F := uint64(1)
+
+	sys := NewTestSystemWithBackend(N, F)
+	backend := sys.backends[0]
+	c := backend.engine
+	// Set the current round state with an empty proposal
+	preprepare := &istanbul.Preprepare{
+		View: c.currentView(),
+	}
+	c.current.SetPreprepare(preprepare)
+
+	// Create a Commit message
+	subject := &istanbul.Subject{
+		View:   c.currentView(),
+		Digest: common.StringToHash("1234567890"),
+	}
+	subjectPayload, err := ibfttypes.Encode(subject)
+	if err != nil {
+		t.Errorf("problem with encoding: %v", err)
+	}
+	msg := &ibfttypes.Message{
+		Code: ibfttypes.MsgCommit,
+		Msg:  subjectPayload,
+	}
+
+	c.finalizeMessage(msg)
+
+	if msg.CommittedSeal != nil {
+		t.Errorf("Unexpected committed seal: %s", msg.CommittedSeal)
+	}
+}