Skip to content

Commit

Permalink
BFT-486: Batch vote metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Jul 23, 2024
1 parent 99df300 commit 928cf60
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 7 deletions.
52 changes: 46 additions & 6 deletions node/actors/network/src/gossip/batch_votes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@ use std::{collections::HashSet, fmt, sync::Arc};
use zksync_concurrency::sync;
use zksync_consensus_roles::attester;

use super::metrics;

#[derive(Debug, Default)]
pub(super) struct BatchUpdateStats {
num_added: usize,
last_added: Option<attester::BatchNumber>,
}

impl BatchUpdateStats {
fn added(&mut self, number: attester::BatchNumber) {
self.num_added += 1;
self.last_added = Some(number);
}
}

/// Represents the current state of node's knowledge about the attester votes.
///
/// Eventually this data structure will have to track voting potentially happening
Expand Down Expand Up @@ -57,13 +72,14 @@ impl BatchVotes {
/// It exits as soon as an invalid entry is found.
/// `self` might get modified even if an error is returned
/// (all entries verified so far are added).
///
/// Returns true iff some new entry was added.
pub(super) fn update(
&mut self,
attesters: &attester::Committee,
data: &[Arc<attester::Signed<attester::Batch>>],
) -> anyhow::Result<bool> {
let mut changed = false;
) -> anyhow::Result<BatchUpdateStats> {
let mut stats = BatchUpdateStats::default();

let mut done = HashSet::new();
for d in data {
Expand Down Expand Up @@ -97,10 +113,10 @@ impl BatchVotes {
d.verify()?;

self.add(d.clone(), weight);

changed = true;
stats.added(d.msg.number);
}
Ok(changed)

Ok(stats)
}

/// Check if we have achieved quorum for any of the batch hashes.
Expand Down Expand Up @@ -216,16 +232,35 @@ impl BatchVotesWatch {
) -> anyhow::Result<()> {
let this = self.0.lock().await;
let mut votes = this.borrow().clone();
if votes.update(attesters, data)? {
let stats = votes.update(attesters, data)?;

if let Some(last_added) = stats.last_added {
this.send_replace(votes);

metrics::BATCH_VOTES_METRICS
.last_added_vote_batch_number
.set(last_added.0);

metrics::BATCH_VOTES_METRICS
.votes_added
.inc_by(stats.num_added as u64);
}

metrics::BATCH_VOTES_METRICS
.committee_size
.set(attesters.len());

Ok(())
}

/// Set the minimum batch number on the votes and discard old data.
pub(crate) async fn set_min_batch_number(&self, min_batch_number: attester::BatchNumber) {
let this = self.0.lock().await;
this.send_modify(|votes| votes.set_min_batch_number(min_batch_number));

metrics::BATCH_VOTES_METRICS
.min_batch_number
.set(min_batch_number.0);
}
}

Expand All @@ -251,6 +286,11 @@ impl BatchVotesPublisher {
return Ok(());
}
let attestation = attester.sign_msg(batch);

metrics::BATCH_VOTES_METRICS
.last_signed_batch_number
.set(attestation.msg.number.0);

self.0.update(attesters, &[Arc::new(attestation)]).await
}
}
31 changes: 31 additions & 0 deletions node/actors/network/src/gossip/metrics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/// Metrics realted to the gossiping of L1 batch votes.

Check warning on line 1 in node/actors/network/src/gossip/metrics.rs

View workflow job for this annotation

GitHub Actions / typos

"realted" should be "related".
#[derive(Debug, vise::Metrics)]
#[metrics(prefix = "network_gossip_batch_votes")]
pub(crate) struct BatchVotesMetrics {
/// Number of members in the attester committee.
pub(crate) committee_size: vise::Gauge<usize>,

/// Number of votes added to the tally.
///
/// Its rate of change should correlate with the attester committe size,

Check warning on line 10 in node/actors/network/src/gossip/metrics.rs

View workflow job for this annotation

GitHub Actions / typos

"committe" should be "committee".
/// save for any new joiner casting their historic votes in a burst.
pub(crate) votes_added: vise::Counter,

/// The minimum batch number we still expect votes for.
///
/// This should go up as the main node indicates the finalisation of batches,
/// or as soon as batch QCs are found and persisted.
pub(crate) min_batch_number: vise::Gauge<u64>,

/// Batch number in the last vote added to the register.
///
/// This should go up as L1 batches are created, save for any temporary
/// outlier from lagging attesters or ones sending votes far in the future.
pub(crate) last_added_vote_batch_number: vise::Gauge<u64>,

/// Batch number of the last batch signed by this attester.
pub(crate) last_signed_batch_number: vise::Gauge<u64>,
}

#[vise::register]
pub(super) static BATCH_VOTES_METRICS: vise::Global<BatchVotesMetrics> = vise::Global::new();
3 changes: 2 additions & 1 deletion node/actors/network/src/gossip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ mod batch_votes;
mod fetch;
mod handshake;
pub mod loadtest;
mod metrics;
mod runner;
#[cfg(test)]
mod testonly;
Expand Down Expand Up @@ -171,7 +172,7 @@ impl Network {
self.batch_store
.persist_batch_qc(ctx, qc)
.await
.wrap("queue_batch_qc")?;
.wrap("persist_batch_qc")?;

self.batch_votes
.set_min_batch_number(next_batch_number)
Expand Down

0 comments on commit 928cf60

Please sign in to comment.