Skip to content

[Doc] Document an invariant on current round. #3614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vicsn merged 1 commit into from
May 9, 2025
Merged

[Doc] Document an invariant on current round. #3614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions node/bft/src/helpers/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ pub struct StorageInner<N: Network> {
current_height: AtomicU32,
/* Once per round */
/// The current round.
///
/// Invariant: current_round > 0.
/// This is established in [`Storage::new`], which sets it to at least 1 via [`Storage::update_current_round`].
/// The only callers of [`Storage::update_current_round`] are
/// [`Storage::increment_to_next_round`] and [`Storage::sync_round_with_block`],
/// both of which set it to at least 1.
current_round: AtomicU64,
/// The `round` for which garbage collection has occurred **up to** (inclusive).
gc_round: AtomicU64,
Expand Down
10 changes: 6 additions & 4 deletions node/bft/src/primary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ impl<N: Network> Primary<N> {
self.workers.iter().map(|worker| worker.num_ratifications()).sum()
}

/// Returns the number of solutions.
/// Returns the number of unconfirmed solutions.
pub fn num_unconfirmed_solutions(&self) -> usize {
self.workers.iter().map(|worker| worker.num_solutions()).sum()
}
Expand Down Expand Up @@ -365,6 +365,8 @@ impl<N: Network> Primary<N> {
let previous_round = round.saturating_sub(1);

// If the current round is 0, return early.
// This can actually never happen, because of the invariant that the current round is never 0
// (see [`StorageInner::current_round`]).
ensure!(round > 0, "Round 0 cannot have transaction batches");

// If the current storage round is below the latest proposal round, then return early.
Expand Down Expand Up @@ -737,7 +739,7 @@ impl<N: Network> Primary<N> {
self.signed_proposals.read().get(&batch_author).copied()
{
// If the signed round is ahead of the peer's batch round, do not sign the proposal.
// Note: while this may be valid behaviour, additional formal analysis and testing will need to be done before allowing it.
// Note: while this may be valid behavior, additional formal analysis and testing will need to be done before allowing it.
if signed_round > batch_header.round() {
bail!(
"Peer ({batch_author}) proposed a batch for a previous round ({}), latest signed round: {signed_round}",
Expand Down Expand Up @@ -1270,7 +1272,7 @@ impl<N: Network> Primary<N> {
};
// If there is no proposed batch, attempt to propose a batch.
// Note: Do NOT spawn a task around this function call. Proposing a batch is a critical path,
// and only one batch needs be proposed at a time.
// and only one batch needs to be proposed at a time.
if let Err(e) = self_.propose_batch().await {
warn!("Cannot propose a batch - {e}");
}
Expand Down Expand Up @@ -1683,7 +1685,7 @@ impl<N: Network> Primary<N> {
bail!("Round {batch_round} is too far in the past")
}

// If node is not in sync mode and the node is not synced. Then return an error.
// If node is not in sync mode and the node is not synced, then return an error.
if !IS_SYNCING && !self.is_synced() {
bail!(
"Failed to process batch header `{}` at round {batch_round} from '{peer_ip}' (node is syncing)",
Expand Down