Commitment vote signature verification (#1250)

* Commitment vote signature verification added

consensus/polybft/state.go

@@ -39,8 +39,8 @@ type TransportMessage struct {
 	Hash []byte
 	// Message signature
 	Signature []byte
-	// Node identifier
-	NodeID string
+	// From is the address of the message signer
+	From string
 	// Number of epoch
 	EpochNumber uint64
 }

consensus/polybft/state_sync_manager.go

@@ -171,17 +171,17 @@ func (s *stateSyncManager) saveVote(msg *TransportMessage) error {
 	valSet := s.validatorSet
 	s.lock.RUnlock()
 
-	if valSet == nil || msg.EpochNumber < epoch {
-		// Epoch metadata is undefined or received message for some of the older epochs
+	if valSet == nil || msg.EpochNumber != epoch {
+		// Epoch metadata is undefined or received a message for the irrelevant epoch
 		return nil
 	}
 
-	if !valSet.Includes(types.StringToAddress(msg.NodeID)) {
-		return fmt.Errorf("validator is not among the active validator set")
+	if err := s.verifyVoteSignature(valSet, types.StringToAddress(msg.From), msg.Signature, msg.Hash); err != nil {
+		return fmt.Errorf("error verifying vote signature: %w", err)
 	}
 
 	msgVote := &MessageSignature{
-		From:      msg.NodeID,
+		From:      msg.From,
 		Signature: msg.Signature,
 	}
 
@@ -193,13 +193,33 @@ func (s *stateSyncManager) saveVote(msg *TransportMessage) error {
 	s.logger.Info(
 		"deliver message",
 		"hash", hex.EncodeToString(msg.Hash),
-		"sender", msg.NodeID,
+		"sender", msg.From,
 		"signatures", numSignatures,
 	)
 
 	return nil
 }
 
+// Verifies signature of the message against the public key of the signer and checks if the signer is a validator
+func (s *stateSyncManager) verifyVoteSignature(valSet ValidatorSet, signer types.Address, signature []byte,
+	hash []byte) error {
+	validator := valSet.Accounts().GetValidatorMetadata(signer)
+	if validator == nil {
+		return fmt.Errorf("unable to resolve validator %s", signer)
+	}
+
+	unmarshaledSignature, err := bls.UnmarshalSignature(signature)
+	if err != nil {
+		return fmt.Errorf("failed to unmarshal signature from signer %s, %w", signer.String(), err)
+	}
+
+	if !unmarshaledSignature.Verify(validator.BlsKey, hash, bls.DomainCheckpointManager) {
+		return fmt.Errorf("incorrect signature from %s", signer)
+	}
+
+	return nil
+}
+
 // AddLog saves the received log from event tracker if it matches a state sync event ABI
 func (s *stateSyncManager) AddLog(eventLog *ethgo.Log) {
 	if !stateTransferEventABI.Match(eventLog) {
@@ -523,7 +543,7 @@ func (s *stateSyncManager) buildCommitment() error {
 	s.multicast(&TransportMessage{
 		Hash:        hashBytes,
 		Signature:   signature,
-		NodeID:      s.config.key.String(),
+		From:        s.config.key.String(),
 		EpochNumber: epoch,
 	})
 

consensus/polybft/state_sync_manager_test.go

@@ -124,7 +124,7 @@ func (m *mockMsg) sign(val *testValidator) *TransportMessage {
 	return &TransportMessage{
 		Hash:        m.hash,
 		Signature:   signature,
-		NodeID:      val.Address().String(),
+		From:        val.Address().String(),
 		EpochNumber: m.epoch,
 	}
 }
@@ -142,6 +142,54 @@ func TestStateSyncManager_MessagePool_SenderIsNoValidator(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestStateSyncManager_MessagePool_InvalidEpoch(t *testing.T) {
+	t.Parallel()
+
+	vals := newTestValidators(5)
+
+	s := newTestStateSyncManager(t, vals.getValidator("0"))
+	s.validatorSet = vals.toValidatorSet()
+
+	val := newMockMsg()
+	msg := val.sign(vals.getValidator("0"))
+	msg.EpochNumber = 1
+
+	err := s.saveVote(msg)
+	require.NoError(t, err)
+
+	// no votes for the current epoch
+	votes, err := s.state.StateSyncStore.getMessageVotes(0, msg.Hash)
+	require.NoError(t, err)
+	require.Len(t, votes, 0)
+
+	// returns an error for the invalid epoch
+	_, err = s.state.StateSyncStore.getMessageVotes(1, msg.Hash)
+	require.Error(t, err)
+}
+
+func TestStateSyncManager_MessagePool_SenderAndSignatureMissmatch(t *testing.T) {
+	t.Parallel()
+
+	vals := newTestValidators(5)
+
+	s := newTestStateSyncManager(t, vals.getValidator("0"))
+	s.validatorSet = vals.toValidatorSet()
+
+	// validator signs the msg in behalf of another validator
+	val := newMockMsg()
+	msg := val.sign(vals.getValidator("0"))
+	msg.From = vals.getValidator("1").Address().String()
+	err := s.saveVote(msg)
+	require.Error(t, err)
+
+	// non validator signs the msg in behalf of a validator
+	badVal := newTestValidator("a", 0)
+	msg = newMockMsg().sign(badVal)
+	msg.From = vals.getValidator("1").Address().String()
+	err = s.saveVote(msg)
+	require.Error(t, err)
+}
+
 func TestStateSyncManager_MessagePool_SenderVotes(t *testing.T) {
 	vals := newTestValidators(5)