Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
25 changes: 9 additions & 16 deletions consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,23 +134,15 @@ stability_scope!(BETA, cfg(not(target_arch = "wasm32")) {

/// CertifiableAutomaton extends [Automaton] with the ability to certify payloads before finalization.
///
/// This trait is required by consensus implementations (like Simplex) that support a certification
/// phase between notarization and finalization. Applications that do not need custom certification
/// logic can use the default implementation which always certifies.
/// Applications that do not need custom certification logic can use the default
/// implementation which always certifies.
pub trait CertifiableAutomaton: Automaton {
/// Determine whether a verified payload is safe to commit.
///
/// The round parameter identifies which consensus round is being certified, allowing
/// applications to associate certification with the correct verification context.
///
/// Note: In applications where payloads incorporate the round number (recommended),
/// each round will have a unique payload digest. However, the same payload may appear
/// in multiple rounds when re-proposing notarized blocks at epoch boundaries or in
/// integrations where payloads are round-agnostic.
///
/// This is particularly useful for applications that employ erasure coding, which
/// can override this method to delay or prevent finalization until they have
/// reconstructed and validated the full block (e.g., after receiving enough shards).
/// applications to associate certification with the correct verification context. The
/// same payload may appear in multiple rounds, so implementations must key any state
/// on `(round, payload)` rather than `payload` alone.
///
/// Like [`Automaton::verify`], payloads produced by [`Automaton::propose`] are certifiable-by-construction.
/// Also like [`Automaton::verify`], certification is single-shot for the given
Expand Down Expand Up @@ -203,9 +195,10 @@ stability_scope!(BETA, cfg(not(target_arch = "wasm32")) {

/// Broadcast a payload to the given recipients.
///
/// Returns `true` when the relay accepted the payload for the requested
/// broadcast plan. Returns `false` when the relay could not complete the
/// handoff.
/// Returns `true` when the relay has durably handled the payload (per
/// the plan's requirements, e.g. local persistence for `Propose`).
/// Returns `false` when the relay could not complete the handoff,
/// which consensus treats as a fatal condition for the round.
fn broadcast(
&mut self,
payload: Self::Digest,
Expand Down
15 changes: 12 additions & 3 deletions consensus/src/marshal/core/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use std::{
pin::Pin,
sync::Arc,
};
use tracing::{debug, error, warn};
use tracing::{debug, error, info, warn};

/// The key used to store the last processed height in the metadata store.
const LATEST_KEY: U64 = U64::new(0xFF);
Expand Down Expand Up @@ -491,7 +491,7 @@ where
},
// Handle consensus inputs before backfill or resolver traffic
Some(message) = self.mailbox.recv() else {
debug!("mailbox closed, shutting down");
info!("mailbox closed, shutting down");
Comment thread
patrick-ogrady marked this conversation as resolved.
Outdated
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
break;
} => {
match message {
Expand Down Expand Up @@ -542,6 +542,15 @@ where
};
buffer.send(round, block, Recipients::Some(peers)).await;
}
// Both handlers ack unconditionally even when the round
Comment thread
patrick-ogrady marked this conversation as resolved.
Outdated
// has already been pruned by tip advancement. In that case
// `cache_verified`/`cache_block` is a no-op because the
// round is below the retention floor, but pruning past a
// round implies consensus has already advanced past it, so
// any downstream notarize/finalize vote a caller casts on
// the strength of this ack is stale and will be ignored by
// peers. Returning false here would stall a background
// task waiting on an outcome that no longer matters.
Message::Verified { round, block, ack } => {
self.cache_verified(round, block.digest(), block).await;
ack.send_lossy(());
Expand Down Expand Up @@ -736,7 +745,7 @@ where
},
// Handle resolver messages last (batched up to max_repair, sync once)
Some(message) = resolver_rx.recv() else {
debug!("handler closed, shutting down");
info!("handler closed, shutting down");
return;
} => {
// Drain up to max_repair messages: blocks handled immediately,
Expand Down
2 changes: 1 addition & 1 deletion consensus/src/marshal/mocks/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ pub fn certify_at_later_view_survives_earlier_view_pruning<H: TestHarness>() {
// past V=1's retention boundary but not past V=25's. With
// view_retention_timeout=10 and prunable_items_per_section=10,
// processing views 1..=21 leaves `oldest_allowed=10` in both prunable
// archives V=1 is dropped, V=25 is retained.
// archives. V=1 is dropped, V=25 is retained.
const CHAIN_LEN: u64 = 21;
let mut parent = Sha256::hash(b"");
let mut parent_commitment = H::genesis_parent_commitment(NUM_VALIDATORS as u16);
Expand Down
16 changes: 13 additions & 3 deletions consensus/src/marshal/standard/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,17 @@ where
ES: Epocher,
{
async fn certify(&mut self, round: Round, digest: Self::Digest) -> oneshot::Receiver<bool> {
// If block was already seen, return immediately.
// Fast path: verify has already run for this (round, digest) and its
// success was recorded in `available_blocks`. `verify` does not mark a
// round available until `marshal.verified(round, block)` has returned,
// and that call blocks on `put_sync` of the block into the round's
// verified cache. Because the verified and notarized caches share the
// same pruning schedule (both advance together to `min_view`), the
// block is already durable for this round and re-persisting it into
// the notarized cache would be a redundant `put_sync`. The slow path
// below persists through the notarized cache because in that case
// verify has not run locally and the block may be held only in the
// broadcast buffer, which is not durable.
if self.available_blocks.lock().contains(&(round, digest)) {
let (tx, rx) = oneshot::channel();
tx.send_lossy(true);
Expand Down Expand Up @@ -938,7 +948,7 @@ mod tests {
let post_restart = marshal2.get_block(&child_digest).await;
assert!(
post_restart.is_some(),
"verify resolved true block must be durably persisted (seed={seed})"
"verify resolved true so block must be durably persisted (seed={seed})"
);
});
}
Expand Down Expand Up @@ -1040,7 +1050,7 @@ mod tests {
let post_restart = marshal2.get_block(&child_digest).await;
assert!(
post_restart.is_some(),
"certify resolved true block must be durably persisted (seed={seed})"
"certify resolved true so block must be durably persisted (seed={seed})"
);
});
}
Expand Down
2 changes: 1 addition & 1 deletion storage/src/archive/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub trait Archive: Send {
/// Store an item in [Archive].
///
/// Indices are unique: if the index already exists, put does nothing and returns. Duplicate
/// indices can be stored via [MultiArchive::put_multi]. Keys need not be unique the same key
/// indices can be stored via [MultiArchive::put_multi]. Keys need not be unique: the same key
/// may be stored at multiple indices, and a subsequent [Archive::get] or [Archive::has] call
/// with an [Identifier::Key] identifier may return any of the values associated with that key.
fn put(
Expand Down
Loading