docs: simplify comments

This commit is contained in:
moudyellaz
2026-08-07 18:30:14 +02:00
parent 69a5aa7899
commit bb54bd0581
16 changed files with 163 additions and 248 deletions
@@ -27,8 +27,7 @@ use ping_core::{ReceiverInstruction, ping_record_pda};
const INITIAL_BALANCE: u128 = 100;
const LOCK_AMOUNT: u128 = 30;
const RECIPIENT: [u8; 32] = [9; 32];
/// The source block a delivery names. These tests drive the guest directly, so
/// there is no peer block to hash and any fixed value does.
/// These tests drive the guest directly, so any fixed source-block hash does.
const SRC_BLOCK_HASH: [u8; 32] = [7; 32];
/// State registering the cross-zone builtins these tests exercise.
@@ -540,14 +539,11 @@ fn mint_replay_rejected() {
}
}
/// A peer that publishes two different blocks claiming one block id gets at most
/// one of them delivered from.
/// A peer publishing two blocks at one block id gets at most one delivered from.
///
/// The shard's address covers the zone and the block id but not which block
/// claimed them, so both resolve to the same account. The first delivery binds
/// it to its own source block and the second cannot execute against it. Failing
/// is the point: were the second merely no-op'd as a replay, a peer could pick
/// which of two messages at one coordinate the target program ever sees.
/// Both resolve to the same shard account; the first binds it. Failing rather
/// than no-opping is the point: a replay no-op would let a peer choose which of
/// two messages at one coordinate the target program ever sees.
#[test]
fn a_delivery_from_a_second_block_at_the_same_id_is_refused() {
let inbox_id = programs::cross_zone_inbox().id();
@@ -561,8 +557,7 @@ fn a_delivery_from_a_second_block_at_the_same_id_is_refused() {
let mut state = base_state();
seed_inbox_config(&mut state, self_zone, src_zone, [9_u32; 8], receiver_id);
// The shard as the first delivery left it: bound to SRC_BLOCK_HASH, holding
// that block's transaction 0.
// The shard as the first delivery left it: bound, holding transaction 0.
let seen_id = inbox_seen_shard_account_id(inbox_id, &src_zone, src_block_id);
let mut shard = SeenShard::default();
shard.insert(SRC_BLOCK_HASH, 0);
@@ -613,9 +608,8 @@ fn a_delivery_from_a_second_block_at_the_same_id_is_refused() {
"a delivery from a block the shard is not bound to must not execute"
);
// The control, so the refusal above is the binding and not the shape of the
// transaction: the same second delivery, differing only in naming the block
// the shard is bound to, executes and is recorded alongside the first.
// Control: the same delivery naming the bound block executes, so the refusal
// above is the binding and not the transaction's shape.
let control_words = risc0_zkvm::serde::to_vec(&ReceiverInstruction::Record {
payload: b"from-the-bound-block".to_vec(),
})
+4 -5
View File
@@ -33,12 +33,11 @@ pub struct Emission {
/// Where a delivery came from on the peer chain.
///
/// One struct so the watcher and the verifier fill the same field list. They
/// must produce byte-identical dispatch transactions for the same emission, and
/// a field one side sets differently is exactly how that breaks.
/// One struct so the watcher and the verifier fill the same field list: their
/// dispatch transactions for one emission must be byte-identical.
///
/// `src_block_hash` is the block's recomputed hash on both sides, never the
/// `header.hash` it declares, which its signature does not cover.
/// `src_block_hash` is the recomputed hash on both sides, never the declared
/// `header.hash`, which the signature does not cover.
pub struct EmissionSource {
pub src_zone: ZoneId,
pub src_block_id: u64,
+17 -29
View File
@@ -63,14 +63,9 @@ pub enum CrossZoneVerifyError {
},
}
/// What the verifier treats as one delivery for the purpose of skipping
/// re-derivation.
///
/// The replay key and the source block it came from. The inbox no-ops a replay
/// only when the shard it lands in is bound to that same block, so skipping on
/// the key alone would wave through a dispatch the guest will refuse, and the
/// block would then park and hold ingestion. Both sides have to agree on what a
/// replay is.
/// The replay key plus the source block, which is what the inbox treats as one
/// delivery. Skipping re-derivation on the key alone would wave through a
/// dispatch the guest refuses, parking the block and holding ingestion.
type SeenKey = (MessageKey, [u8; 32]);
/// One peer zone's cached blocks, plus how far this reader has read them as an
@@ -462,10 +457,8 @@ impl CrossZoneVerifier {
)));
}
// Recomputed here rather than read from `msg`, which would make the
// field attest to itself. `accept_peer_block` already proved this equals
// the `header.hash` of every cached block, so it costs one hash and the
// fact stays local.
// Recomputed rather than read from `msg`, which would make the field
// attest to itself.
Ok(build_dispatch_from_emission(
&EmissionSource {
src_zone: msg.src_zone,
@@ -542,6 +535,13 @@ struct PeerPass {
stalled_at: Option<Slot>,
}
fn seen_key(msg: &CrossZoneMessage) -> SeenKey {
(
message_key(&msg.src_zone, msg.src_block_id, msg.src_tx_index),
msg.src_block_hash,
)
}
/// Whether a block read off a peer's channel may enter the cache. The channel
/// authorizes who may write, not what they may claim.
///
@@ -550,14 +550,6 @@ struct PeerPass {
/// without recomputing it a peer can assert links it never built. The key check
/// applies only when one is pinned, mirroring the watcher; it subsumes the hash
/// check, but a peer with no pinned key still gets that one.
/// The delivery `msg` identifies, for the seen set.
fn seen_key(msg: &CrossZoneMessage) -> SeenKey {
(
message_key(&msg.src_zone, msg.src_block_id, msg.src_tx_index),
msg.src_block_hash,
)
}
fn accept_peer_block(
block: &Block,
peer_zone: ZoneId,
@@ -876,10 +868,8 @@ mod tests {
let verifier = verifier();
cache_chain(&verifier, peer_chain(b"hi")).await;
// Everything else matches the peer's block, so only the claimed source
// hash is wrong. The verifier recomputes it from the block it resolved
// rather than reading the field, which is what makes this detectable at
// all; trusting the message would make the field attest to itself.
// Only the claimed source hash is wrong. Detectable because the verifier
// recomputes it from the resolved block instead of reading the field.
let block =
produce_dummy_block(9, None, vec![dispatch_naming_block_hash(b"hi", [0xab; 32])]);
assert!(
@@ -980,11 +970,9 @@ mod tests {
let keys = verifier.verify_block(&first).await.expect("first verifies");
verifier.record_seen(keys).await;
// Same zone, block id and transaction index as the delivery just seen,
// but naming a different source block. The inbox refuses this rather
// than no-opping it, since the shard is bound to the other block, so
// skipping re-derivation here would wave through a dispatch that then
// parks the block and holds ingestion.
// Same coordinates as the delivery just seen, different source block.
// The inbox refuses rather than no-ops it, so skipping re-derivation
// would wave through a dispatch that parks the block.
let other = produce_dummy_block(
10,
None,
+3 -4
View File
@@ -44,10 +44,9 @@ fn main() {
mint_amount, amount,
"locked amount must equal the wrapped mint amount"
);
// Refused here rather than on the destination, where the mint would fail
// after this side had already escrowed the balance. Nothing releases an
// escrow, so an amount the destination will not mint has to fail before the
// debit, in the submitter's own transaction where they can see it.
// Before the debit, not on the destination: nothing releases an escrow, so
// an amount the destination will not mint has to fail in the submitter's own
// transaction.
assert!(
amount <= MAX_MINT_AMOUNT,
"locked amount exceeds what the wrapped token will mint"
+22 -33
View File
@@ -9,11 +9,9 @@ use serde::{Deserialize, Serialize};
const MESSAGE_KEY_DOMAIN: [u8; 32] = *b"/LEZ/v0.3/CrossZoneMsgKey/00000/";
const INBOX_CONFIG_SEED: [u8; 32] = *b"/LEZ/v0.3/CrossZoneInboxCfg/000/";
/// Not `/00/`, which keyed a shard by `src_block_id / 10_000`: an epoch and a
/// block id collide under one domain, so a shard carrying the older layout would
/// land at a new address and decode as nonsense. Belt and braces, since the
/// image id is what actually relocates every PDA here, and it moves with any
/// change to this crate.
/// `/01/` because `/00/` keyed shards by epoch: an epoch and a block id are
/// indistinguishable under one domain. Belt and braces, since the image id
/// already relocates every PDA in this crate whenever the crate changes.
const INBOX_SEEN_SEED_DOMAIN: [u8; 32] = *b"/LEZ/v0.3/CrossZoneInboxSeen/01/";
/// Raw 32-byte zone (channel) id; the host maps it to the zone-sdk `ChannelId`.
@@ -126,18 +124,13 @@ impl InboxConfig {
/// What one peer block has already delivered.
///
/// One shard per peer block, holding transaction indices rather than message
/// keys. The shard's own address binds `(src_zone, src_block_id)`, so hashing
/// those into a 32-byte key and storing it back inside that account records
/// nothing the address does not.
/// Indices, not message keys: the shard's address already binds
/// `(src_zone, src_block_id)`, so a key stored inside it adds nothing.
///
/// Indices rather than keys are what make a per-block shard affordable, not
/// free. A shard costs a new account plus a 36-byte header, so at one delivery
/// per peer block it holds more state per message than one shard shared across
/// ten thousand blocks did, and it breaks even around five. What it buys is that
/// a shard cannot saturate: at 32 bytes per delivery, one peer block's own
/// messages could overflow the account, and the guest's only way to say so is a
/// panic that costs the message.
/// A shard costs an account plus a 36-byte header and breaks even against a
/// shared shard at about five deliveries. What that buys is saturation
/// resistance: at 32 bytes per delivery one peer block could overflow the
/// account, and the guest's only answer is a panic that costs the message.
#[derive(Clone, Debug, Default, PartialEq, Eq, BorshSerialize, BorshDeserialize)]
pub struct SeenShard {
/// Recomputed hash of the peer block this shard records deliveries from.
@@ -153,11 +146,10 @@ impl SeenShard {
/// Borsh is 32 bytes of hash, a 4-byte count, then 4 bytes per index, so
/// this is exactly the 100 KiB an account may carry.
///
/// What keeps it out of reach is the L1 inscription limit rather than
/// anything a peer configures: a whole block is inscribed as one op, capped
/// near 1.75 MiB, and a minimal emitting transaction is about 257 bytes, so
/// one peer block tops out around 7,100 deliveries. Raising that L1 cap past
/// roughly 6.3 MiB would put this back in reach.
/// Out of reach only because of the L1 inscription cap: a block inscribes as
/// one op near 1.75 MiB and a minimal emitting transaction is about 257
/// bytes, capping a peer block near 7,100 deliveries. Raising that L1 cap
/// past roughly 6.3 MiB puts this back in reach.
pub const MAX_DELIVERIES: usize = 25_591;
/// Decodes a shard from account data; empty data is an unclaimed shard.
@@ -175,10 +167,9 @@ impl SeenShard {
/// Whether a delivery from the block with this hash may be recorded here.
///
/// An unclaimed shard binds to whoever claims it first. Unclaimed is the
/// whole value being default rather than the hash being all zero, so a shard
/// that has recorded anything can never read as unclaimed even if a hash
/// somehow were.
/// An unclaimed shard binds to its first claimant. Unclaimed is the whole
/// value being default, not the hash being zero, so a shard holding any
/// delivery can never read as unclaimed.
#[must_use]
pub fn binds(&self, src_block_hash: &[u8; 32]) -> bool {
*self == Self::default() || self.src_block_hash == *src_block_hash
@@ -189,13 +180,11 @@ impl SeenShard {
self.delivered.contains(&src_tx_index)
}
/// Binds the shard if unclaimed and records the delivery; returns true if it
/// was newly recorded.
/// Binds the shard if unclaimed and records the delivery; true if new.
///
/// A hash the shard does not bind records nothing. The guest asserts
/// [`Self::binds`] before reaching this, so that refusal is a backstop
/// against a later caller rebinding a claimed shard and quietly erasing
/// which peer block delivered what.
/// A non-binding hash records nothing. The guest already asserts
/// [`Self::binds`], so this is a backstop against a future caller rebinding
/// a claimed shard and erasing which peer block delivered what.
pub fn insert(&mut self, src_block_hash: [u8; 32], src_tx_index: u32) -> bool {
if !self.binds(&src_block_hash) {
return false;
@@ -280,8 +269,8 @@ pub fn inbox_seen_shard_account_id(
/// Seed of the seen-shard PDA, exposed so the guest can claim the account.
///
/// One shard per peer block, so what it can hold is bounded by that block, and a
/// peer cannot accumulate deliveries into one account across many of them.
/// One shard per peer block, so a peer cannot accumulate deliveries from many
/// blocks into one account.
#[must_use]
pub fn inbox_seen_shard_seed(src_zone: &ZoneId, src_block_id: u64) -> PdaSeed {
use risc0_zkvm::sha::{Impl, Sha256 as _};
+5 -8
View File
@@ -97,15 +97,12 @@ fn dispatch(
let mut shard =
SeenShard::from_bytes(&seen.account.data.clone().into_inner()).expect("seen shard decodes");
// One block id, one delivering block. The shard's address binds the zone and
// the block id but not which block claimed them, so an equivocating peer's
// two blocks at one id resolve to this same account. The first to deliver
// binds it to its own hash and the second aborts here, rather than the two
// sharing a replay set while being different blocks.
// One block id, one delivering block. The address binds the zone and block
// id but not which block claimed them, so an equivocating peer's two blocks
// at one id land here; the first binds the shard and the second aborts.
//
// Before the replay check, not after. A mismatched hash has to fail the
// transaction; reaching the replay branch first would turn a delivery from
// the wrong block into a silent no-op, which is what the indexer's own
// Before the replay check, not after: reaching the replay branch first would
// turn a wrong-block delivery into a silent no-op, which the indexer's
// already-seen short circuit would then wave through.
assert!(
shard.binds(&msg.src_block_hash),
+7 -13
View File
@@ -10,20 +10,14 @@ use serde::{Deserialize, Serialize};
/// The most one mint may credit.
///
/// The amount is chosen on the peer zone and the balance is a `u128`, so without
/// a bound a single delivery can push a holding to within a hair of the maximum.
/// Every honest mint to that recipient then overflows, and an overflow is a guest
/// panic, so each one fails execution and is eventually given up on. The holding
/// is unusable for inbound transfers for good, at a cost to the attacker of one
/// message.
/// The peer zone chooses the amount and the balance is a `u128`, so unbounded
/// one delivery pins a holding near the maximum, every later honest mint
/// overflows into a guest panic, and the holding is bricked for inbound
/// transfers at a cost of one message. The cap does not remove that ceiling, it
/// makes reaching it cost 2^64 deliveries instead of one.
///
/// A cap does not put the maximum out of reach, it makes reaching it cost 2^64
/// deliveries rather than one.
///
/// `u64::MAX` is a bound the bridge imposes rather than one native balances
/// already obey: `Balance` is a `u128` and the faucet is seeded at its maximum,
/// so a larger amount is representable. `bridge_lock` refuses one at the source
/// so it fails in the submitter's transaction rather than after escrowing.
/// `u64::MAX` is the bridge's bound, not one native balances obey. `bridge_lock`
/// refuses a larger amount at the source so it fails before escrowing.
pub const MAX_MINT_AMOUNT: u128 = 0xFFFF_FFFF_FFFF_FFFF;
const CONFIG_SEED_DOMAIN: [u8; 32] = *b"/LEZ/v0.3/WrappedTokenConfig/00/";
+2 -2
View File
@@ -180,8 +180,8 @@ pub fn increment_cross_zone_dispatches_retired_total() {
cross_zone_dispatches_retired_total_counter().increment(1);
}
/// Retained dead letters, which both evict at their cap and drop when a
/// delivery turns out to settle, so this falls as well as rises.
/// Retained dead letters. A gauge, not a counter: eviction and reconciliation
/// make this fall as well as rise.
pub fn record_cross_zone_dead_letter_dispatches(count: usize) {
gauge!(
description: "Given-up-on cross-zone deliveries currently retained for inspection",
+2 -3
View File
@@ -1183,9 +1183,8 @@ mod tests {
panic!("the recorded transaction is an inbox dispatch");
};
// The block this delivery came from, which is what the indexer
// recomputes independently when it re-derives the same transaction.
// Naming a different block is what makes the two disagree.
// The indexer recomputes this independently when it re-derives the same
// transaction; a different block here is what makes the two disagree.
assert_eq!(
msg.src_block_hash,
chain_block(1).recompute_hash().0,
+29 -36
View File
@@ -1082,9 +1082,8 @@ impl<BP: BlockPublisherTrait> SequencerCore<BP> {
/// A delivery's payload and target accounts are chosen on the peer zone and
/// validated by nobody in between, so one can fail for good; but a failure
/// can equally be a property of the moment, so give up only after several.
/// Giving up moves the record to the dead letter, which is what keeps a peer
/// from growing the pending list with deliveries that can never execute
/// while still leaving the delivery somewhere an operator can find it.
/// Giving up moves the record to the dead letter: a peer cannot grow the
/// pending list with deliveries that never execute, and it stays findable.
fn count_dispatch_failure(&self, tx: &LeeTransaction) {
let Some(message) = extract_cross_zone_dispatch(tx) else {
return;
@@ -1134,12 +1133,11 @@ impl<BP: BlockPublisherTrait> SequencerCore<BP> {
}
}
/// The deliveries this node has given up on, with how many times it has done
/// so, which is the larger number once entries evict or reconcile away.
/// The deliveries this node has given up on, and how many times it has.
///
/// Retained is read first so the pair can only skew towards a total that
/// leads its list, which is an ordinary evicted or settled state. The other
/// order would report entries against a total of zero.
/// leads its list, an ordinary evicted or settled state. The other order
/// would report entries against a total of zero.
pub fn cross_zone_dead_letters(&self) -> Result<(u64, Vec<DeadLetterDispatchRecord>), DbError> {
let dbio = self.store.dbio();
let retained = dbio.get_dead_letter_cross_zone_dispatches()?;
@@ -1230,16 +1228,14 @@ fn deposit_already_minted(state: &lee::V03State, deposit_op_id: HashType) -> boo
/// Whether a cross-zone delivery is already on the chain we are building on.
///
/// The inbox records each peer block's delivered transaction indices in that
/// block's seen shard and no-ops a replay, so the shard is the same kind of
/// answer the deposit receipt gives: state, not bookkeeping. An orphan reverts
/// the entry with the block, so the next turn re-delivers with nothing of ours
/// to unwind.
/// The inbox records each peer block's delivered indices in that block's seen
/// shard and no-ops a replay, so the shard is the same kind of answer the
/// deposit receipt gives: state, not bookkeeping. An orphan reverts the entry
/// with the block, so the next turn re-delivers with nothing to unwind.
///
/// Both halves matter. A shard bound to a different peer block is not this
/// delivery's replay record; it is what will make this delivery abort, and
/// calling that delivered would drop the record instead of retrying it and
/// dead-lettering it where an operator can see it.
/// delivery's replay record, it is what will make it abort, and calling that
/// delivered would drop the record instead of dead-lettering it.
fn dispatch_already_delivered(state: &lee::V03State, message: &CrossZoneMessage) -> bool {
let shard_id = cross_zone_inbox_core::inbox_seen_shard_account_id(
programs::cross_zone_inbox().id(),
@@ -1253,6 +1249,22 @@ fn dispatch_already_delivered(state: &lee::V03State, message: &CrossZoneMessage)
})
}
/// Publishes how many given-up-on deliveries are retained.
///
/// Read from the store because the list falls as well as rises (eviction, and
/// reconciliation when a delivery settles elsewhere). Costs a read and a decode,
/// so call it only where one of those can have happened.
fn record_dead_letter_gauge(dbio: &RocksDBIO) {
match dbio.get_dead_letter_cross_zone_dispatches() {
Ok(records) => {
sequencer_core_metrics::record_cross_zone_dead_letter_dispatches(records.len());
}
Err(err) => {
warn!("Failed to read the cross-zone dead letter for its gauge: {err:#}");
}
}
}
/// Feed one channel delta into the follow state and mirror it to the store:
/// revert orphaned, then apply and persist adopted and finalized blocks.
/// Production builds on this same head. Wired to the publisher via
@@ -1265,24 +1277,6 @@ fn dispatch_already_delivered(state: &lee::V03State, message: &CrossZoneMessage)
/// relies on a valid successor or a restart. `ChainState` never emits
/// `AcceptOutcome::RetryableFailure` yet; adding retry parity here is a
/// follow-up.
/// Publishes how many given-up-on deliveries are retained.
///
/// Read from the store rather than tracked in memory because the list falls as
/// well as rises: it evicts at its cap, and an entry is dropped when its
/// delivery turns out to settle on a block another sequencer produced. Called
/// only where one of those can have happened, since it costs a read and a
/// decode.
fn record_dead_letter_gauge(dbio: &RocksDBIO) {
match dbio.get_dead_letter_cross_zone_dispatches() {
Ok(records) => {
sequencer_core_metrics::record_cross_zone_dead_letter_dispatches(records.len());
}
Err(err) => {
warn!("Failed to read the cross-zone dead letter for its gauge: {err:#}");
}
}
}
fn apply_follow_update(
dbio: &RocksDBIO,
chain: &Mutex<ChainState>,
@@ -1461,9 +1455,8 @@ fn apply_follow_update(
};
sequencer_core_metrics::record_chain_height(head_height);
// This is the runtime path that reconciles: a delivery this node gave up on
// reaches a block another sequencer produced, and finalizing that block
// drops its dead letter.
// The runtime reconcile path: finalizing another sequencer's block drops the
// dead letter of a delivery this node gave up on.
record_dead_letter_gauge(dbio);
if outcome.accepted_deposits > 0 {
+4 -8
View File
@@ -221,10 +221,8 @@ fn dispatch_tx(src_block_id: u64, payload: Vec<u8>) -> LeeTransaction {
))
}
/// A stand-in for the peer block's recomputed hash, distinct per block id.
///
/// These records are seeded straight into the store rather than read off a peer
/// channel, so no real block exists to hash. Only its consistency matters.
/// A stand-in for the peer block's recomputed hash, distinct per block id. These
/// records are seeded into the store, so no real block exists to hash.
fn peer_block_hash(src_block_id: u64) -> [u8; 32] {
let mut hash = [0_u8; 32];
hash[..8].copy_from_slice(&src_block_id.to_le_bytes());
@@ -720,10 +718,8 @@ async fn a_dispatch_that_never_executes_is_given_up_on_after_repeated_failures()
"giving up on a delivery must take its record out of the pending list"
);
// A dispatch that fails execution is left out of the block, so the dead
// letter is the only place recording that this happened at all. The origin
// is the point of the record: it is what identifies which message stopped
// being attempted, and this is the only place that builds one.
// The dead letter is the only record that this happened, and the origin is
// what identifies which message stopped being attempted.
let dbio = sequencer.store.dbio();
let dead_letters = dbio.get_dead_letter_cross_zone_dispatches().unwrap();
assert_eq!(dead_letters.len(), 1);
+5 -7
View File
@@ -11,11 +11,10 @@ use serde_with::{DeserializeFromStr, SerializeDisplay};
#[derive(Debug, Clone, PartialEq, Eq, Hash, SerializeDisplay, DeserializeFromStr)]
pub struct ChannelId(pub [u8; 32]);
/// A cross-zone delivery a sequencer gave up on after repeated execution
/// failures.
/// A cross-zone delivery a sequencer gave up on after repeated failures.
///
/// Identifies the message rather than carrying it: the peer zone, block id and
/// transaction index are what locate it on the peer's channel.
/// Identifies the message rather than carrying it: zone, block id and tx index
/// locate it on the peer's channel.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct CrossZoneDeadLetter {
pub message_key: HashType,
@@ -28,9 +27,8 @@ pub struct CrossZoneDeadLetter {
/// What a sequencer has given up delivering.
///
/// `total_retired` counts every give-up; `retained` holds the ones still kept.
/// The two differ once entries evict at the cap, or once a delivery this node
/// abandoned settles on a block another sequencer produced.
/// `total_retired` counts every give-up, `retained` only the ones still kept;
/// they diverge on eviction and on reconciliation.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct CrossZoneDeadLetterReport {
pub total_retired: u64,
+33 -51
View File
@@ -83,16 +83,13 @@ pub const MAX_PENDING_CROSS_ZONE_DISPATCHES: usize = 4096;
/// How many given-up-on cross-zone deliveries are kept for inspection.
///
/// Retaining them is the point, but a peer chooses how many deliveries fail, so
/// this list cannot be unbounded any more than the pending one can. At the cap
/// the oldest is dropped, which keeps the entries an operator reaching for this
/// after an alert actually wants. Nothing is concealed by that: every
/// retirement is counted separately and that count does not evict.
/// A peer chooses how many deliveries fail, so this cannot be unbounded. The
/// oldest evicts at the cap, and nothing is concealed by that: retirements are
/// counted separately and the count does not evict.
///
/// A count is a real bound here only because a record identifies a delivery
/// instead of carrying it. Each is a fixed 84 bytes, so the whole list is 21 KB
/// at the cap, bounded in bytes as well as in entries. That matters because it
/// is one value rewritten under the lock that block production needs.
/// An entry count bounds bytes only because a record identifies a delivery
/// rather than carrying it. At a fixed 84 bytes each the list is 21 KB, which
/// matters because it is one value rewritten under the block-production lock.
pub const MAX_DEAD_LETTER_CROSS_ZONE_DISPATCHES: usize = 256;
/// Key base for storing the LEE state.
@@ -107,10 +104,8 @@ pub const CF_LEE_STATE_NAME: &str = "cf_lee_state";
/// What counting a failed production attempt did to a delivery's record.
///
/// Three outcomes rather than a bool because the caller reports each
/// differently and only one of them means this node stopped trying. A delivery
/// that has already settled has no pending record, so it is [`Self::Absent`]
/// rather than a give-up.
/// Three outcomes rather than a bool: only one means this node stopped trying,
/// and a settled delivery has no record, so it is [`Self::Absent`].
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum DispatchFailure {
/// Counted; the delivery is still pending and will be attempted again.
@@ -783,18 +778,14 @@ impl RocksDBIO {
/// Counts a failed production attempt against a delivery, retiring it once
/// it reaches `retire_at`.
///
/// Retiring moves the record out of the pending list and into the dead
/// letter. The pending list has to lose it, or the drain would re-feed a
/// transaction that never executes for ever; the dead letter is what keeps
/// the delivery identifiable, since a dispatch that fails execution is left
/// out of the block and so leaves no trace anywhere else. It is bounded on
/// its own terms, so a peer that can make deliveries fail still cannot grow
/// the store without limit.
/// The pending list has to lose the record, or the drain re-feeds a
/// transaction that never executes for ever. The dead letter keeps the
/// delivery identifiable, since a dispatch that fails execution is left out
/// of the block and leaves no trace elsewhere, and it is bounded separately.
///
/// A delivery with no pending record is reported as [`DispatchFailure::Absent`]
/// rather than as a retirement: there is nothing to count against, and the
/// two are different events. It is the ordinary shape of a delivery that
/// settled and then failed to execute on a later attempt.
/// No pending record gives [`DispatchFailure::Absent`], not a retirement:
/// the ordinary shape of a delivery that settled and then failed a later
/// attempt.
pub fn record_dispatch_failure(
&self,
message_key: [u8; 32],
@@ -828,12 +819,10 @@ impl RocksDBIO {
transaction_bytes: u32::try_from(retired.transaction.len()).unwrap_or(u32::MAX),
};
// One entry per delivery, not per retirement. The same delivery can be
// recorded again after its record is gone, since a watcher rebuilding a
// peer tip re-reads that channel from the peer's genesis and a delivery
// that never executes never reaches the inbox seen-set to be recognised
// as delivered. Without this, one message that always fails would fill
// the list with copies of itself and evict every other one.
// One entry per delivery, not per retirement. A watcher rebuilding a
// peer tip re-reads from the peer's genesis, and a never-executing
// delivery never reaches the seen-set, so the same one retires again;
// undeduped it would evict every other entry with copies of itself.
let mut dead_letters = self.get_dead_letter_cross_zone_dispatches()?;
if !dead_letters
.iter()
@@ -844,17 +833,15 @@ impl RocksDBIO {
dead_letters.remove(0);
}
}
// Counted per retirement even so: it measures how often this node gave
// up, which the retained list cannot, since that both evicts and drops
// entries whose delivery later settles.
// Counted per retirement even so: the retained list evicts and drops
// settled entries, so its length is not how often this node gave up.
let count = self
.get_dead_letter_cross_zone_dispatch_count()?
.saturating_add(1);
// One batch: the record leaving the pending list and arriving in the
// dead letter is one event, and a crash between the two halves would
// either lose the message silently or leave the drain retrying a
// delivery already recorded as given up on.
// One batch: a crash between the two halves either loses the message
// silently or leaves the drain retrying a delivery already recorded as
// given up on.
let mut batch = WriteBatch::default();
self.put_pending_cross_zone_dispatches_batch(&records, &mut batch)?;
self.put_batch(
@@ -907,10 +894,8 @@ impl RocksDBIO {
records.retain(|record| !to_remove.contains(&record.message_key));
let removed = before.saturating_sub(records.len());
// Both lists in one batch, for the same reason the retire path batches:
// a crash between them leaves the pending record gone and a dead letter
// behind saying the delivery was abandoned, and nothing recomputes these
// keys on a later pass to correct it.
// Both lists in one batch, as in `record_dispatch_failure`: nothing
// recomputes these keys on a later pass to fix a torn write.
let mut batch = WriteBatch::default();
if removed > 0 {
self.put_pending_cross_zone_dispatches_batch(&records, &mut batch)?;
@@ -929,14 +914,12 @@ impl RocksDBIO {
/// Stages the removal of dead letters whose delivery turned out to settle.
///
/// Giving up is this node's decision and every sequencer makes it alone,
/// against its own head and its own mempool ordering, so a delivery this one
/// stopped attempting can still reach a block another one produced. Left
/// alone, the entry would report that delivery as abandoned for as long as
/// the store lives, and nothing else removes one.
/// Every sequencer gives up alone, against its own head, so a delivery this
/// one abandoned can still reach another's block. Nothing else removes an
/// entry, so without this it reports as abandoned for the store's lifetime.
///
/// The count is deliberately not decremented. It records how often this node
/// gave up, which stays true whatever happened next.
/// The count is deliberately not decremented: it records how often this node
/// gave up, which stays true.
fn stage_reconciled_dead_letters(
&self,
settled: &std::collections::HashSet<&[u8; 32]>,
@@ -983,9 +966,8 @@ impl RocksDBIO {
self.put_pending_cross_zone_dispatches_batch(&records, batch)?;
}
// A settled delivery this node had given up on is not one it abandoned,
// and this is the path that catches the ordinary case: another sequencer
// carries it into a block that then becomes irreversible.
// The ordinary case: another sequencer carried a delivery this node gave
// up on into a block that just became irreversible.
self.stage_reconciled_dead_letters(&to_remove, batch)?;
Ok(removed)
}
+17 -23
View File
@@ -297,9 +297,8 @@ pub struct PendingCrossZoneDispatchRecord {
/// validated by nobody in between, so one can fail for good. A failure can
/// equally be a property of the moment, so a single one is not enough to
/// give up on a delivery. Once too many accumulate the record leaves this
/// list, which the drain re-feeds every turn, and a
/// [`DeadLetterDispatchRecord`] is kept in its place so the delivery this
/// node stopped attempting is still identifiable.
/// list (the drain re-feeds it every turn) for a
/// [`DeadLetterDispatchRecord`], which keeps the delivery identifiable.
pub failed_attempts: u32,
}
@@ -361,29 +360,25 @@ pub struct DispatchOrigin {
/// A cross-zone delivery this node has given up on.
///
/// A dispatch that fails execution is left out of the block, so unlike every
/// other failure in the pipeline this one leaves no on-chain trace that it was
/// ever attempted. This record is what makes it observable rather than a log
/// line that scrolls away.
/// A dispatch that fails execution is left out of the block, so nothing on chain
/// records that it was attempted; this is the only durable trace.
///
/// It identifies the message rather than carrying it. The peer block and
/// transaction index are enough to read the message back off the peer channel,
/// and the encoded transaction is chosen by the peer zone and can exceed this
/// node's whole block size limit, so retaining it would bound the list in
/// entries while leaving it unbounded in bytes.
/// It identifies the message rather than carrying it: the peer block and index
/// are enough to read it back off the channel, and the encoded transaction is
/// peer-chosen and can exceed a whole block, which would leave the list bounded
/// in entries but unbounded in bytes.
///
/// Giving up is this node's decision, not the network's: another sequencer may
/// carry the same delivery successfully, and a record here is dropped again if
/// that happens.
/// Giving up is this node's decision, not the network's, so an entry is dropped
/// again if another sequencer carries the same delivery.
#[derive(Debug, Clone, PartialEq, Eq, BorshSerialize, BorshDeserialize)]
pub struct DeadLetterDispatchRecord {
pub message_key: [u8; 32],
pub origin: DispatchOrigin,
/// Attempts made before giving up, recording the policy that was in force
/// rather than distinguishing one retirement from another.
/// Attempts made before giving up, so the record carries the policy that was
/// in force at the time.
pub failed_attempts: u32,
/// Size of the delivery transaction that would not execute, which is the
/// diagnostic for the one failure mode that is about size.
/// Size of the delivery transaction that would not execute, the diagnostic
/// for size-related failures.
pub transaction_bytes: u32,
}
@@ -422,10 +417,9 @@ impl SimpleWritableCell for DeadLetterCrossZoneDispatchesCellRef<'_> {
/// Deliveries given up on since this store was created.
///
/// Counted separately from the retained list because that list both evicts at
/// its cap and drops entries whose delivery later settles, so its length is not
/// how many times this node has given up. A node that gave up hundreds of times
/// would otherwise look like one that gave up at the cap.
/// Separate from the retained list, which evicts at its cap and drops settled
/// entries: a node that gave up hundreds of times would otherwise look like one
/// that gave up at the cap.
#[derive(BorshSerialize, BorshDeserialize)]
pub struct DeadLetterCrossZoneDispatchCountCell(pub u64);
+3 -9
View File
@@ -719,10 +719,7 @@ fn a_dead_letter_is_dropped_once_its_delivery_settles_elsewhere() {
1
);
// Every sequencer decides to give up alone, against its own head, so a
// delivery this node stopped attempting can still reach a block another one
// produced. Reporting it as abandoned for ever afterwards is the failure
// this guards against.
// A delivery this node gave up on can still reach another sequencer's block.
dbio.drop_settled_cross_zone_dispatches(&[key]).unwrap();
assert!(
dbio.get_dead_letter_cross_zone_dispatches()
@@ -775,11 +772,8 @@ fn one_delivery_that_always_fails_takes_one_dead_letter_slot() {
let temp_dir = tempdir().unwrap();
let (dbio, _genesis) = dbio_with_genesis(temp_dir.path());
// A watcher rebuilding a peer tip re-reads that channel from the peer's
// genesis, and a delivery that never executes never reaches the inbox
// seen-set to be recognised as delivered, so the same one is recorded and
// retired again. Without dedupe it would fill the list with copies of itself
// and evict every other message that was given up on.
// A watcher rebuilding a peer tip re-reads from genesis, so the same
// never-executing delivery retires repeatedly (see `record_dispatch_failure`).
let key = key_from_index(1);
let other = key_from_index(2);
dbio.add_pending_cross_zone_dispatches(vec![PendingCrossZoneDispatchRecord::recorded(
@@ -195,9 +195,8 @@ pub fn dashboard() -> Dashboard {
.row(
7,
[
// A dispatch that fails execution is left out of the block, so
// nothing on chain records that a delivery was abandoned. These
// are the only signal that it happened.
// A failed dispatch is left out of the block, so nothing on chain
// records it. These panels are the only signal.
Panel::stat("Cross-zone deliveries given up on since startup")
.width(6)
.unit(Unit::Short)