diff --git a/consensus/src/lib.rs b/consensus/src/lib.rs index 0926c0cfd72..4f656a4a309 100644 --- a/consensus/src/lib.rs +++ b/consensus/src/lib.rs @@ -141,16 +141,9 @@ stability_scope!(BETA, cfg(not(target_arch = "wasm32")) { /// 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 diff --git a/consensus/src/marshal/core/actor.rs b/consensus/src/marshal/core/actor.rs index 7d1fa31f1d8..74169bd6f21 100644 --- a/consensus/src/marshal/core/actor.rs +++ b/consensus/src/marshal/core/actor.rs @@ -543,10 +543,18 @@ where buffer.send(round, block, Recipients::Some(peers)).await; } Message::Verified { round, block, ack } => { + // If the round has already been pruned by tip advancement, + // `cache_verified` is a no-op because the round is below + // the retention floor (and no longer is required by consensus + // to make progress). self.cache_verified(round, block.digest(), block).await; ack.send_lossy(()); } Message::Certified { round, block, ack } => { + // If the round has already been pruned by tip advancement, + // `cache_block` is a no-op because the round is below + // the retention floor (and no longer is required by consensus + // to make progress). self.cache_block(round, block.digest(), block).await; ack.send_lossy(()); } diff --git a/consensus/src/marshal/mocks/harness.rs b/consensus/src/marshal/mocks/harness.rs index 5480677bc42..ac1c60aa6b9 100644 --- a/consensus/src/marshal/mocks/harness.rs +++ b/consensus/src/marshal/mocks/harness.rs @@ -1011,7 +1011,7 @@ pub fn certify_at_later_view_survives_earlier_view_pruning() { // 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); diff --git a/consensus/src/marshal/standard/inline.rs b/consensus/src/marshal/standard/inline.rs index e7df6194e41..e0c934cf961 100644 --- a/consensus/src/marshal/standard/inline.rs +++ b/consensus/src/marshal/standard/inline.rs @@ -442,7 +442,17 @@ where ES: Epocher, { async fn certify(&mut self, round: Round, digest: Self::Digest) -> oneshot::Receiver { - // If block was already seen, return immediately. + // 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); @@ -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})" ); }); } @@ -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})" ); }); } diff --git a/storage/src/archive/mod.rs b/storage/src/archive/mod.rs index dc290b1ffb3..03e4e7caea0 100644 --- a/storage/src/archive/mod.rs +++ b/storage/src/archive/mod.rs @@ -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(