consensus: surface late and excluded commit signatures to operators

consensus/checks.go

@@ -58,6 +58,7 @@ func (consensus *Consensus) senderKeySanityChecks(msg *msg_pb.Message, senderKey
 func (consensus *Consensus) isRightBlockNumAndViewID(recvMsg *FBFTMessage) bool {
 	blockNum := consensus.getBlockNum()
 	if recvMsg.ViewID != consensus.getCurBlockViewID() || recvMsg.BlockNum != blockNum {
+		consensus.reportLateVoteIfPastFinalized(recvMsg, blockNum)
 		consensus.getLogger().Debug().
 			Uint64("blockNum", blockNum).
 			Str("recvMsg", recvMsg.String()).

consensus/consensus.go

@@ -136,6 +136,10 @@ type Consensus struct {
 	lastKnownSignPower int64
 	lastKnowViewChange int64
 
+	// last COMMIT this node successfully broadcast
+	lastCommitSentBlockNum uint64
+	lastCommitSentHash     atomic.Value // common.Hash
+
 	transitions struct {
 		finalCommit bool
 	}

consensus/late_signature.go

@@ -0,0 +1,93 @@
+package consensus
+
+import (
+	"sync/atomic"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/harmony-one/harmony/crypto/bls"
+	"github.com/prometheus/client_golang/prometheus"
+)
+
+// recordLastCommitSent stores the block number and hash of the last COMMIT
+// this node broadcast successfully.
+func (consensus *Consensus) recordLastCommitSent(blockNum uint64, blockHash common.Hash) {
+	atomic.StoreUint64(&consensus.lastCommitSentBlockNum, blockNum)
+	consensus.lastCommitSentHash.Store(blockHash)
+}
+
+// checkOwnCommitInclusion logs and counts when a local COMMIT key is absent
+// from the final COMMITTED bitmap for a block this node previously signed.
+func (consensus *Consensus) checkOwnCommitInclusion(blockNum uint64, blockHash common.Hash, mask *bls.Mask) {
+	if mask == nil {
+		return
+	}
+	if atomic.LoadUint64(&consensus.lastCommitSentBlockNum) != blockNum {
+		return
+	}
+	v := consensus.lastCommitSentHash.Load()
+	if v == nil {
+		return
+	}
+	sentHash, ok := v.(common.Hash)
+	if !ok || sentHash != blockHash {
+		return
+	}
+
+	priKeys, err := consensus.getPriKeysInCommittee()
+	if err != nil || len(priKeys) == 0 {
+		return
+	}
+	localPubs := make([]bls.SerializedPublicKey, 0, len(priKeys))
+	for _, key := range priKeys {
+		localPubs = append(localPubs, key.Pub.Bytes)
+	}
+
+	for _, pub := range excludedLocalCommitKeys(mask, localPubs) {
+		consensus.getLogger().Warn().
+			Uint64("blockNum", blockNum).
+			Str("blockHash", blockHash.Hex()).
+			Str("blsPubKey", pub.Hex()).
+			Msg("[OnCommitted] local commit signature not included in final commit bitmap")
+		consensusLateSignatureCounterVec.With(prometheus.Labels{
+			"role":  "validator",
+			"phase": "committed",
+		}).Inc()
+	}
+}
+
+// excludedLocalCommitKeys returns local committee keys that are present in the
+// participant set but disabled in the commit bitmap.
+func excludedLocalCommitKeys(mask *bls.Mask, localPubs []bls.SerializedPublicKey) []bls.SerializedPublicKey {
+	excluded := make([]bls.SerializedPublicKey, 0)
+	for _, pub := range localPubs {
+		ok, err := mask.KeyEnabled(pub)
+		if err != nil {
+			continue
+		}
+		if !ok {
+			excluded = append(excluded, pub)
+		}
+	}
+	return excluded
+}
+
+// reportLateVoteIfPastFinalized logs and counts a prepare/commit vote whose
+// block number is exactly one behind the leader's current block number.
+func (consensus *Consensus) reportLateVoteIfPastFinalized(recvMsg *FBFTMessage, myBlockNum uint64) {
+	if recvMsg == nil || recvMsg.BlockNum+1 != myBlockNum {
+		return
+	}
+	phase := recvMsg.MessageType.String()
+	consensusLateSignatureCounterVec.With(prometheus.Labels{
+		"role":  "leader",
+		"phase": phase,
+	}).Inc()
+
+	consensus.getLogger().Info().
+		Uint64("msgBlockNum", recvMsg.BlockNum).
+		Uint64("myBlockNum", myBlockNum).
+		Uint64("msgViewID", recvMsg.ViewID).
+		Str("phase", phase).
+		Str("recvMsg", recvMsg.String()).
+		Msg("[Consensus] late vote received after block finalized")
+}

consensus/late_signature_test.go

@@ -0,0 +1,92 @@
+package consensus
+
+import (
+	"testing"
+
+	"github.com/ethereum/go-ethereum/common"
+	msg_pb "github.com/harmony-one/harmony/api/proto/message"
+	"github.com/harmony-one/harmony/crypto/bls"
+	dto "github.com/prometheus/client_model/go"
+	"github.com/stretchr/testify/require"
+)
+
+// TestExcludedLocalCommitKeys checks bitmap exclusion for local committee keys.
+func TestExcludedLocalCommitKeys(t *testing.T) {
+	pub1 := bls.PublicKeyWrapper{Object: bls.RandPrivateKey().GetPublicKey()}
+	pub2 := bls.PublicKeyWrapper{Object: bls.RandPrivateKey().GetPublicKey()}
+	pub3 := bls.PublicKeyWrapper{Object: bls.RandPrivateKey().GetPublicKey()}
+	pub1.Bytes.FromLibBLSPublicKey(pub1.Object)
+	pub2.Bytes.FromLibBLSPublicKey(pub2.Object)
+	pub3.Bytes.FromLibBLSPublicKey(pub3.Object)
+
+	mask := bls.NewMask([]bls.PublicKeyWrapper{pub1, pub2, pub3})
+	require.NoError(t, mask.SetKey(pub1.Bytes, true))
+	require.NoError(t, mask.SetKey(pub3.Bytes, true))
+
+	excluded := excludedLocalCommitKeys(mask, []bls.SerializedPublicKey{pub1.Bytes, pub2.Bytes})
+	require.Equal(t, []bls.SerializedPublicKey{pub2.Bytes}, excluded)
+
+	excluded = excludedLocalCommitKeys(mask, []bls.SerializedPublicKey{pub1.Bytes})
+	require.Empty(t, excluded)
+
+	// Key not in participants is ignored (err from KeyEnabled).
+	outsider := bls.PublicKeyWrapper{Object: bls.RandPrivateKey().GetPublicKey()}
+	outsider.Bytes.FromLibBLSPublicKey(outsider.Object)
+	excluded = excludedLocalCommitKeys(mask, []bls.SerializedPublicKey{outsider.Bytes})
+	require.Empty(t, excluded)
+}
+
+// TestReportLateVoteIfPastFinalized increments the metric only for the prior block.
+func TestReportLateVoteIfPastFinalized(t *testing.T) {
+	c := &Consensus{current: NewState(Normal, 0)}
+	initMetrics()
+
+	recvMsg := &FBFTMessage{
+		MessageType: msg_pb.MessageType_COMMIT,
+		BlockNum:    10,
+		ViewID:      1,
+	}
+	// Not the immediately previous block — no-op.
+	c.reportLateVoteIfPastFinalized(recvMsg, 12)
+	c.reportLateVoteIfPastFinalized(nil, 11)
+
+	before := lateSignatureCount(t, "leader", msg_pb.MessageType_COMMIT.String())
+	c.reportLateVoteIfPastFinalized(recvMsg, 11)
+	after := lateSignatureCount(t, "leader", msg_pb.MessageType_COMMIT.String())
+	require.Equal(t, before+1, after)
+}
+
+// TestRecordLastCommitSentGatesInclusionCheck skips checks without a matching sent COMMIT.
+func TestRecordLastCommitSentGatesInclusionCheck(t *testing.T) {
+	pub1 := bls.PublicKeyWrapper{Object: bls.RandPrivateKey().GetPublicKey()}
+	pub2 := bls.PublicKeyWrapper{Object: bls.RandPrivateKey().GetPublicKey()}
+	pub1.Bytes.FromLibBLSPublicKey(pub1.Object)
+	pub2.Bytes.FromLibBLSPublicKey(pub2.Object)
+
+	mask := bls.NewMask([]bls.PublicKeyWrapper{pub1, pub2})
+	require.NoError(t, mask.SetKey(pub2.Bytes, true)) // pub1 excluded
+
+	hash := common.HexToHash("0xabc")
+	c := &Consensus{current: NewState(Normal, 0)}
+	initMetrics()
+
+	// Without a matching last-sent commit, check is a no-op even if key excluded.
+	c.checkOwnCommitInclusion(5, hash, mask)
+
+	c.recordLastCommitSent(5, hash)
+	// Still no-op: getPriKeysInCommittee fails with empty priKey.
+	c.checkOwnCommitInclusion(5, hash, mask)
+
+	// Wrong hash — no-op.
+	c.recordLastCommitSent(5, common.HexToHash("0xdef"))
+	c.checkOwnCommitInclusion(5, hash, mask)
+}
+
+func lateSignatureCount(t *testing.T, role, phase string) float64 {
+	t.Helper()
+	metric, err := consensusLateSignatureCounterVec.GetMetricWithLabelValues(role, phase)
+	require.NoError(t, err)
+	var m dto.Metric
+	require.NoError(t, metric.Write(&m))
+	return m.GetCounter().GetValue()
+}

consensus/metrics.go

@@ -127,6 +127,20 @@ var (
 			Buckets:   prometheus.ExponentialBuckets(800, 1.25, 10),
 		},
 	)
+	// consensusLateSignatureCounterVec counts late prepare/commit votes and
+	// local COMMIT keys missing from the COMMITTED bitmap.
+	consensusLateSignatureCounterVec = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: "hmy",
+			Subsystem: "consensus",
+			Name:      "late_signature",
+			Help:      "votes arriving after finalize, or local commits missing from the COMMITTED bitmap",
+		},
+		[]string{
+			"role",
+			"phase",
+		},
+	)
 
 	onceMetrics sync.Once
 
@@ -188,6 +202,7 @@ func initMetrics() {
 			consensusGaugeVec,
 			consensusPubkeyVec,
 			consensusFinalityHistogram,
+			consensusLateSignatureCounterVec,
 			lastPreimageImportGauge,
 			preimageEndGauge,
 			preimageStartGauge,

consensus/validator.go

@@ -184,6 +184,7 @@ func (consensus *Consensus) sendCommitMessages(blockObj *types.Block) {
 	if err := consensus.broadcastConsensusP2pMessages(p2pMsgs); err != nil {
 		consensus.getLogger().Warn().Err(err).Msg("[sendCommitMessages] Cannot send commit message!!")
 	} else {
+		consensus.recordLastCommitSent(blockObj.NumberU64(), blockObj.Hash())
 		consensus.getLogger().Info().
 			Uint64("blockNum", consensus.BlockNum()).
 			Hex("blockHash", consensus.current.blockHash[:]).
@@ -357,6 +358,8 @@ func (consensus *Consensus) onCommitted(recvMsg *FBFTMessage) {
 		consensus.getLogger().Error().Err(err).Msg("[OnCommitted] readSignatureBitmapPayload failed")
 		return
 	}
+	// Compare against the COMMITTED bitmap before any later SetMask mutation.
+	consensus.checkOwnCommitInclusion(recvMsg.BlockNum, recvMsg.BlockHash, mask)
 	consensus.fBFTLog.AddVerifiedMessage(recvMsg)
 	consensus.aggregatedCommitSig = aggSig
 	consensus.commitBitmap = mask