Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion consensus/late_signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,34 @@ func (consensus *Consensus) checkOwnCommitInclusion(blockNum uint64, blockHash c
localPubs = append(localPubs, key.Pub.Bytes)
}

for _, pub := range excludedLocalCommitKeys(mask, localPubs) {
reportLocalCommitInclusions(mask, localPubs, func(pub bls.SerializedPublicKey) {
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")
})
}

// reportLocalCommitInclusions counts each local committee key present in the
// COMMITTED participant set toward signature_total, and late_signature when
// the key is disabled in the bitmap.
func reportLocalCommitInclusions(mask *bls.Mask, localPubs []bls.SerializedPublicKey, onMissing func(bls.SerializedPublicKey)) {
for _, pub := range localPubs {
ok, err := mask.KeyEnabled(pub)
if err != nil {
continue
}
consensusSignatureTotalCounterVec.With(prometheus.Labels{
"role": "validator",
"phase": "committed",
}).Inc()
if ok {
continue
}
if onMissing != nil {
onMissing(pub)
}
consensusLateSignatureCounterVec.With(prometheus.Labels{
"role": "validator",
"phase": "committed",
Expand All @@ -71,6 +93,14 @@ func excludedLocalCommitKeys(mask *bls.Mask, localPubs []bls.SerializedPublicKey
return excluded
}

// reportAcceptedVote counts a prepare/commit vote accepted on time by the leader.
func (consensus *Consensus) reportAcceptedVote(phase string) {
consensusSignatureTotalCounterVec.With(prometheus.Labels{
"role": "leader",
"phase": phase,
}).Inc()
}

// 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) {
Expand All @@ -82,6 +112,10 @@ func (consensus *Consensus) reportLateVoteIfPastFinalized(recvMsg *FBFTMessage,
"role": "leader",
"phase": phase,
}).Inc()
consensusSignatureTotalCounterVec.With(prometheus.Labels{
"role": "leader",
"phase": phase,
}).Inc()

consensus.getLogger().Info().
Uint64("msgBlockNum", recvMsg.BlockNum).
Expand Down
62 changes: 57 additions & 5 deletions consensus/late_signature_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/ethereum/go-ethereum/common"
msg_pb "github.com/harmony-one/harmony/api/proto/message"
"github.com/harmony-one/harmony/crypto/bls"
"github.com/prometheus/client_golang/prometheus"
dto "github.com/prometheus/client_model/go"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -36,7 +37,7 @@ func TestExcludedLocalCommitKeys(t *testing.T) {
require.Empty(t, excluded)
}

// TestReportLateVoteIfPastFinalized increments the metric only for the prior block.
// TestReportLateVoteIfPastFinalized increments late and total only for the prior block.
func TestReportLateVoteIfPastFinalized(t *testing.T) {
c := &Consensus{current: NewState(Normal, 0)}
initMetrics()
Expand All @@ -50,10 +51,51 @@ func TestReportLateVoteIfPastFinalized(t *testing.T) {
c.reportLateVoteIfPastFinalized(recvMsg, 12)
c.reportLateVoteIfPastFinalized(nil, 11)

before := lateSignatureCount(t, "leader", msg_pb.MessageType_COMMIT.String())
phase := msg_pb.MessageType_COMMIT.String()
beforeLate := lateSignatureCount(t, "leader", phase)
beforeTotal := signatureTotalCount(t, "leader", phase)
c.reportLateVoteIfPastFinalized(recvMsg, 11)
after := lateSignatureCount(t, "leader", msg_pb.MessageType_COMMIT.String())
require.Equal(t, before+1, after)
require.Equal(t, beforeLate+1, lateSignatureCount(t, "leader", phase))
require.Equal(t, beforeTotal+1, signatureTotalCount(t, "leader", phase))
}

// TestReportAcceptedVote increments the total counter for on-time votes.
func TestReportAcceptedVote(t *testing.T) {
c := &Consensus{current: NewState(Normal, 0)}
initMetrics()

phase := msg_pb.MessageType_PREPARE.String()
beforeLate := lateSignatureCount(t, "leader", phase)
beforeTotal := signatureTotalCount(t, "leader", phase)
c.reportAcceptedVote(phase)
require.Equal(t, beforeLate, lateSignatureCount(t, "leader", phase))
require.Equal(t, beforeTotal+1, signatureTotalCount(t, "leader", phase))
}

// TestReportLocalCommitInclusionsCountsTotal increments total for included and excluded local keys.
func TestReportLocalCommitInclusionsCountsTotal(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})
require.NoError(t, mask.SetKey(pub1.Bytes, true)) // pub2 excluded

initMetrics()
beforeLate := lateSignatureCount(t, "validator", "committed")
beforeTotal := signatureTotalCount(t, "validator", "committed")

var missing []bls.SerializedPublicKey
reportLocalCommitInclusions(mask, []bls.SerializedPublicKey{pub1.Bytes, pub2.Bytes, pub3.Bytes}, func(pub bls.SerializedPublicKey) {
missing = append(missing, pub)
})

require.Equal(t, []bls.SerializedPublicKey{pub2.Bytes}, missing)
require.Equal(t, beforeLate+1, lateSignatureCount(t, "validator", "committed"))
require.Equal(t, beforeTotal+2, signatureTotalCount(t, "validator", "committed"))
}

// TestRecordLastCommitSentGatesInclusionCheck skips checks without a matching sent COMMIT.
Expand Down Expand Up @@ -84,7 +126,17 @@ func TestRecordLastCommitSentGatesInclusionCheck(t *testing.T) {

func lateSignatureCount(t *testing.T, role, phase string) float64 {
t.Helper()
metric, err := consensusLateSignatureCounterVec.GetMetricWithLabelValues(role, phase)
return counterValue(t, consensusLateSignatureCounterVec, role, phase)
}

func signatureTotalCount(t *testing.T, role, phase string) float64 {
t.Helper()
return counterValue(t, consensusSignatureTotalCounterVec, role, phase)
}

func counterValue(t *testing.T, vec *prometheus.CounterVec, role, phase string) float64 {
t.Helper()
metric, err := vec.GetMetricWithLabelValues(role, phase)
require.NoError(t, err)
var m dto.Metric
require.NoError(t, metric.Write(&m))
Expand Down
2 changes: 2 additions & 0 deletions consensus/leader.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ func (consensus *Consensus) onPrepare(recvMsg *FBFTMessage) {
consensus.getLogger().Warn().Err(err).Msg("[OnPrepare] prepareBitmap.SetKey failed")
return
}
consensus.reportAcceptedVote(recvMsg.MessageType.String())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify the intended behavior: if a validator sends one aggregated vote containing 10 BLS keys, signature_total is incremented by 1 rather than 10 here. Is that the expected unit for this metric?

//// Write - End

//// Read - Start
Expand Down Expand Up @@ -309,6 +310,7 @@ func (consensus *Consensus) onCommit(recvMsg *FBFTMessage) {
Msg("[OnCommit] commitBitmap.SetKey failed")
return
}
consensus.reportAcceptedVote(recvMsg.MessageType.String())
//// Write - End

//// Read - Start
Expand Down
16 changes: 16 additions & 0 deletions consensus/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ var (
"phase",
},
)
// consensusSignatureTotalCounterVec counts all prepare/commit votes that are
// either accepted on time or classified as late, and all local commit
// inclusion checks. Pair with late_signature to derive on-time vs late rates.
consensusSignatureTotalCounterVec = prometheus.NewCounterVec(
prometheus.CounterOpts{
Namespace: "hmy",
Subsystem: "consensus",
Name: "signature_total",
Help: "total prepare/commit votes (accepted or late) and local commit inclusion checks",
},
[]string{
"role",
"phase",
},
)

onceMetrics sync.Once

Expand Down Expand Up @@ -203,6 +218,7 @@ func initMetrics() {
consensusPubkeyVec,
consensusFinalityHistogram,
consensusLateSignatureCounterVec,
consensusSignatureTotalCounterVec,
lastPreimageImportGauge,
preimageEndGauge,
preimageStartGauge,
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ require (
github.com/multiformats/go-multihash v0.2.3
github.com/olekukonko/tablewriter v0.0.5
github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58
github.com/prometheus/client_model v0.6.1
golang.org/x/exp v0.0.0-20250128182459-e0ece0dbea4c
golang.org/x/term v0.28.0
golang.org/x/text v0.21.0
Expand Down Expand Up @@ -250,7 +251,6 @@ require (
github.com/pion/webrtc/v3 v3.3.5 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/polydawn/refmt v0.89.0 // indirect
github.com/prometheus/client_model v0.6.1 // indirect
github.com/prometheus/common v0.62.0 // indirect
github.com/prometheus/procfs v0.15.1 // indirect
github.com/prometheus/tsdb v0.7.1 // indirect
Expand Down