From ef7d263fed4c7bce624ec3aaff5872db13a2a985 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 28 Jul 2026 18:01:05 -0400 Subject: [PATCH] Amend Wave B storage contracts after adversarial review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The B2 review of the first B1/B3 slice found five defects whose fixes were not available to the packages that had to make them: each needed a change to a surface those packages do not own. Rather than let them restate a frozen fact locally, the lead amends the surfaces and the packages consume them. Contract review 2026-07-28-A records all five with the weaker alternative that was rejected for each. - CommitEvidenceSigner::sign_event's parameter becomes signing_digest. The message is SignedCommittedTransactionV1::signing_digest, which binds key epoch and durability result around the canonical event; the bare event digest is chain identity, not signature material. A doc line was not enough: misuse is undetectable until after durability, and is first observed by a mirror on another instance. - TransactionEvidenceV1::actor() is new, exhaustive over all six variants. A destination event's actor must restate the evidence's source instance, and deriving it a second way store-side is what produced the defect it closes. - format::object_type_code becomes pub(crate) and recovery.rs's private twin is deleted, so one table exists where three did. - RefRecord gains from_target/target() in terms of RefTarget, with the code table stated once per direction and an unknown kind refused by value as CheckpointError::RefKind. Defaulting an unknown kind would launder it into the next checkpoint within one interval. - max_projection_objects is capped at MAX_CANONICAL_ITEMS and its default lowered to it; max_projection_chunks likewise, and max_projection_bytes against the transitive per-chunk allowance. The old default described a projection no manifest could encode. Supporting a hundred million objects requires a versioned chunked or indexed manifest design, not a larger hostile-decode ceiling. Both governing documents are updated where they now misstate a frozen fact, including §6.4's claim that signing covers the event digest. §6.5 records the two carry-forwards from B3's first slice — production staging use is incomplete, and expire() has no scheduler — and records that the requested StoreEngine staging accessor is a pending amendment which must not be a bare Arc, since a clone could outlive EngineShared, survive release of the root LOCK, and keep serving a root this process no longer holds. No package file is touched: engine.rs, transaction.rs, and staging.rs consume these in their own commits. levcs-protocol 6 consumer tests, levcs-store 109 library tests. Co-Authored-By: Claude Opus 5 (1M context) --- crates/levcs-protocol/src/v2.rs | 29 ++++ .../levcs-protocol/tests/phase0_consumers.rs | 90 ++++++++++--- crates/levcs-store/src/checkpoint.rs | 110 ++++++++++++++-- crates/levcs-store/src/format.rs | 11 +- crates/levcs-store/src/options.rs | 107 ++++++++++++++- crates/levcs-store/src/recovery.rs | 47 ++----- crates/levcs-store/src/types.rs | 18 ++- doc/instance-throughput-rewrite-plan.md | 124 ++++++++++++++++++ doc/phase1-storage-spine-scope.md | 65 ++++++++- 9 files changed, 529 insertions(+), 72 deletions(-) diff --git a/crates/levcs-protocol/src/v2.rs b/crates/levcs-protocol/src/v2.rs index 7c1b4d3..1d5cc9f 100644 --- a/crates/levcs-protocol/src/v2.rs +++ b/crates/levcs-protocol/src/v2.rs @@ -2361,6 +2361,35 @@ impl TransactionEvidenceV1 { } } + /// The principal a committed event's `actor` field must carry. + /// + /// This is not a convenience projection. `validate_mirror_application` + /// requires `destination_event.actor == source_instance` for all four + /// mirror kinds, and the administrative signing digests bind `actor` + /// directly, so `actor` is a property *of the evidence* that the + /// destination event restates. A sequencer that fills it from its own + /// signing key instead produces an event that signs, fences, and becomes + /// durable, and only then fails evidence verification — at the mirror, on + /// another instance, with no way to withdraw it. Reading it off the + /// evidence is the only construction that cannot drift. + pub fn actor(&self) -> PublicKeyBytes { + match self { + Self::ClientV2 { signed_envelope } => match signed_envelope { + SignedClientOperationV2::Init(signed) => signed.signer, + SignedClientOperationV2::Push(signed) => signed.signer, + }, + Self::MirrorEventV1 { + source_instance, .. + } => *source_instance, + Self::MirrorSnapshotV1 { + source_instance, .. + } => *source_instance, + Self::LegacyMigrationV1 { actor, .. } => *actor, + Self::ProjectionAdminV1 { actor, .. } => *actor, + Self::AdministrativeV1 { actor, .. } => *actor, + } + } + fn validate_structure(&self) -> CodecResult<()> { match self { Self::MirrorEventV1 { diff --git a/crates/levcs-protocol/tests/phase0_consumers.rs b/crates/levcs-protocol/tests/phase0_consumers.rs index 82c6dc4..238fc6c 100644 --- a/crates/levcs-protocol/tests/phase0_consumers.rs +++ b/crates/levcs-protocol/tests/phase0_consumers.rs @@ -6,18 +6,20 @@ use support::*; fn destination_event_for(evidence: &TransactionEvidenceV1) -> CommittedTransactionV1 { let repo_id = id(3); - let (actor, operation_id) = match evidence { + // The actor is read off the evidence rather than recomputed here. A helper + // that derived it a second way could agree with `validate_mirror_application` + // by coincidence while every production sequencer disagreed, which is the + // defect this accessor exists to remove. + let actor = evidence.actor(); + let operation_id = match evidence { TransactionEvidenceV1::MirrorEventV1 { source_instance, source_event, .. - } => ( + } => mirror_event_operation_id( *source_instance, - mirror_event_operation_id( - *source_instance, - repo_id, - source_event.transaction.repo_sequence, - ), + repo_id, + source_event.transaction.repo_sequence, ), TransactionEvidenceV1::MirrorSnapshotV1 { source_instance, @@ -25,17 +27,21 @@ fn destination_event_for(evidence: &TransactionEvidenceV1) -> CommittedTransacti destination_projection, projected_manifest_digest, .. - } => ( + } => mirror_snapshot_operation_id( *source_instance, - mirror_snapshot_operation_id( - *source_instance, - repo_id, - source_snapshot.snapshot.generation_digest().unwrap(), - *destination_projection, - *projected_manifest_digest, - ), + repo_id, + source_snapshot.snapshot.generation_digest().unwrap(), + *destination_projection, + *projected_manifest_digest, ), - _ => (key(8).public().0, [0x71; 16]), + // Named individually: these four are not mirror applications, so they + // have no derived operation ID. A catch-all here would silently give a + // newly added mirror variant a constant operation ID and a passing + // test. + TransactionEvidenceV1::ClientV2 { .. } + | TransactionEvidenceV1::LegacyMigrationV1 { .. } + | TransactionEvidenceV1::ProjectionAdminV1 { .. } + | TransactionEvidenceV1::AdministrativeV1 { .. } => [0x71; 16], }; CommittedTransactionV1 { repo_id, @@ -162,6 +168,58 @@ fn mirror_event_consumer_distinguishes_projected_from_cursor_only() { .is_err()); } +#[test] +fn evidence_actor_is_the_binding_a_destination_event_must_restate() { + // One expected principal per `all_evidence()` entry, in order: the two + // client pushes are signed by keys 1 and 2, both mirror variants name + // instance key 7, and the three administrative variants are signed by the + // admin key 8. Written out rather than derived, so a variant that starts + // reporting a different principal fails here instead of agreeing with a + // second copy of the same mistake. + let expected = [ + key(1).public().0, + key(2).public().0, + key(7).public().0, + key(7).public().0, + key(8).public().0, + key(8).public().0, + key(8).public().0, + ]; + let evidence = all_evidence(); + assert_eq!(evidence.len(), expected.len()); + for (value, expected_actor) in evidence.iter().zip(expected) { + assert_eq!(value.actor(), expected_actor); + } + + // The failure this accessor exists to prevent. Filling `actor` from the + // destination instance's own signing key produces an event that signs, + // fences, and becomes durable, and only then fails mirror verification — + // at which point no caller can withdraw it. + let snapshot_evidence = evidence + .iter() + .find(|value| matches!(value, TransactionEvidenceV1::MirrorSnapshotV1 { .. })) + .unwrap(); + let event = destination_event_for(snapshot_evidence); + validate_mirror_application( + MirrorApplicationKindV1::SnapshotInline, + snapshot_evidence, + &event, + None, + ) + .unwrap(); + let destination_signed_actor = CommittedTransactionV1 { + actor: key(9).public().0, + ..event + }; + assert!(validate_mirror_application( + MirrorApplicationKindV1::SnapshotInline, + snapshot_evidence, + &destination_signed_actor, + None + ) + .is_err()); +} + #[test] fn consumer_dispatch_is_total_for_every_transaction_source_kind() { let mut seen = [false; 6]; diff --git a/crates/levcs-store/src/checkpoint.rs b/crates/levcs-store/src/checkpoint.rs index f9942e5..3a1854f 100644 --- a/crates/levcs-store/src/checkpoint.rs +++ b/crates/levcs-store/src/checkpoint.rs @@ -99,6 +99,12 @@ pub enum CheckpointError { CountCeiling(&'static str), #[error("checkpoint body is malformed: {0}")] Body(&'static str), + /// Named separately from [`CheckpointError::Body`] because the offending + /// value is the whole diagnosis: a ref kind is one byte, and "which byte" + /// is the difference between a torn write and a reader that never learned + /// about a kind a newer writer emits. + #[error("checkpoint carries unknown ref kind {0}")] + RefKind(u8), #[error("checkpoint has trailing bytes after its declared body")] TrailingBytes, #[error("checkpoint file name is not .checkpoint")] @@ -118,6 +124,14 @@ impl From for StoreError { // --------------------------------------------------------------------------- /// One typed ref binding, as of the checkpoint's `shard_committed_sequence`. +/// +/// The physical `(ref_kind, name)` pair stays public because the checkpoint +/// body encodes it directly, but nothing outside this module should ever +/// interpret it: use [`RefRecord::from_target`] and [`RefRecord::target`]. +/// Every consumer that decodes `ref_kind` itself is restating a table this +/// module owns, and the copy is only ever discovered when the two disagree — +/// which, for a derived-but-authoritative ref table, means a reopened store +/// silently resolving a branch as a release. #[derive(Clone, Debug, PartialEq, Eq)] pub struct RefRecord { pub namespace: NamespaceId, @@ -126,6 +140,33 @@ pub struct RefRecord { pub target: ObjectId, } +impl RefRecord { + /// Builds the physical record for a typed ref binding. + pub fn from_target(namespace: NamespaceId, target: &RefTarget, object: ObjectId) -> Self { + let (ref_kind, name) = ref_target_parts(target); + Self { + namespace, + ref_kind, + name: name.as_bytes().to_vec(), + target: object, + } + } + + /// The typed binding this record encodes. + /// + /// Fallible on purpose. A checkpoint is derived state read back from disk, + /// so `ref_kind` and `name` are attacker-adjacent in exactly the sense the + /// rest of this module's readers are: an unknown kind is refused by name, + /// never defaulted to `Branch` and never dropped. Defaulting would turn a + /// corrupt byte into a plausible ref that the next checkpoint would then + /// write back as if it had always been there. + pub fn target(&self) -> Result { + let name = std::str::from_utf8(&self.name) + .map_err(|_| CheckpointError::Body("ref record name utf-8"))?; + ref_target_from_kind_code(self.ref_kind, name) + } +} + /// A retained operation/receipt row. /// /// `first_receipt_visibility_micros` is the field recovery step 10 promotes. @@ -330,6 +371,14 @@ fn read_optional_object_id( } } +/// The checkpoint body's ref-kind codes, encode side. +/// +/// Paired with [`ref_target_from_kind_code`], and the only place in the crate +/// that assigns these two codes. `format.rs` has its own table for the *frame* +/// encoding; that is not a duplicate, because the two physical formats are +/// versioned independently and each is self-consistent — a frame code is only +/// ever compared to a frame code. The copies that mattered were the ones that +/// decoded a `RefRecord` this module produced, and those are now gone. fn ref_target_parts(target: &RefTarget) -> (u8, &str) { match target { RefTarget::Branch(name) => (1, name), @@ -337,6 +386,26 @@ fn ref_target_parts(target: &RefTarget) -> (u8, &str) { } } +/// The checkpoint body's ref-kind codes, decode side. +/// +/// Rejects an unknown code by value rather than defaulting: a ref table is +/// derived state, and a silently reclassified ref would be written back into +/// the next checkpoint as though it were authoritative. +fn ref_target_from_kind_code(kind: u8, name: &str) -> Result { + if name.is_empty() + || name.len() > MAX_REF_NAME_LEN as usize + || name.as_bytes().contains(&0) + || levcs_core::refs::validate_ref_name(name).is_err() + { + return Err(CheckpointError::Body("ref target name")); + } + match kind { + 1 => Ok(RefTarget::Branch(name.to_owned())), + 2 => Ok(RefTarget::Release(name.to_owned())), + unknown => Err(CheckpointError::RefKind(unknown)), + } +} + fn validate_ref_target(name: &str) -> Result<(), StoreError> { if name.len() > MAX_REF_NAME_LEN as usize { return Err(StoreError::LimitExceeded { @@ -411,16 +480,8 @@ fn read_applied_refs(r: &mut Reader<'_>) -> Result, CheckpointEr return Err(CheckpointError::Body("receipt ref target name length")); } let name = std::str::from_utf8(r.take(name_len as usize, "receipt ref target name")?) - .map_err(|_| CheckpointError::Body("receipt ref target name utf-8"))? - .to_owned(); - if name.as_bytes().contains(&0) || levcs_core::refs::validate_ref_name(&name).is_err() { - return Err(CheckpointError::Body("receipt ref target name")); - } - let target = match kind { - 1 => RefTarget::Branch(name), - 2 => RefTarget::Release(name), - _ => return Err(CheckpointError::Body("receipt ref target kind")), - }; + .map_err(|_| CheckpointError::Body("receipt ref target name utf-8"))?; + let target = ref_target_from_kind_code(kind, name)?; if !targets.insert(target.clone()) { return Err(CheckpointError::Body( "duplicate receipt applied-ref target", @@ -1058,6 +1119,35 @@ mod tests { ); } + #[test] + fn a_ref_record_round_trips_through_its_typed_target() { + for target in [ + RefTarget::Branch("refs/heads/main".into()), + RefTarget::Release("refs/releases/v1".into()), + ] { + let record = RefRecord::from_target(ns(1), &target, oid(0x40)); + assert_eq!(record.target().expect("a record we built decodes"), target); + } + } + + #[test] + fn an_unknown_ref_kind_is_refused_by_value_and_never_defaulted() { + let mut record = RefRecord::from_target( + ns(1), + &RefTarget::Branch("refs/heads/main".into()), + oid(0x40), + ); + for kind in [0u8, 3, 255] { + record.ref_kind = kind; + assert_eq!( + record.target().unwrap_err(), + CheckpointError::RefKind(kind), + "an unreadable ref must not resolve as a branch: the next checkpoint would \ + write the guess back as though it were recovered state" + ); + } + } + #[test] fn a_legacy_checkpoint_with_receipts_requires_offline_rebuild() { let legacy = sample() diff --git a/crates/levcs-store/src/format.rs b/crates/levcs-store/src/format.rs index fb2c10e..6c93542 100644 --- a/crates/levcs-store/src/format.rs +++ b/crates/levcs-store/src/format.rs @@ -1227,7 +1227,16 @@ mod wire { use wire::{R, W}; -fn object_type_code(value: ObjectType) -> u8 { +/// The single in-crate statement of the store's object-type codes. +/// +/// `pub(crate)` rather than private because `recovery.rs` and the engine both +/// have to write the same code into an index entry, and a private mapping does +/// not prevent a second table — it only guarantees the second table is written +/// somewhere else and compared to this one by review. The recovered index entry +/// and the submitted object are compared end to end by reopening a store, and +/// that comparison is only meaningful while there is one table to disagree +/// with. +pub(crate) fn object_type_code(value: ObjectType) -> u8 { // Exhaustive on purpose: a new object type must break this build rather // than acquire an undocumented on-disk code. match value { diff --git a/crates/levcs-store/src/options.rs b/crates/levcs-store/src/options.rs index a57ac09..80026ce 100644 --- a/crates/levcs-store/src/options.rs +++ b/crates/levcs-store/src/options.rs @@ -132,7 +132,9 @@ impl Default for StoreOptions { max_replay_bytes: 8 * 1024 * 1024 * 1024, max_objects_per_transaction: 65_536, max_refs_per_transaction: 4_096, - max_projection_objects: 100_000_000, + // The manifest is a flat canonical vector, so this is the codec's + // item ceiling and not a tuning choice. See `validate`. + max_projection_objects: 1_000_000, max_projection_bytes: 1024 * 1024 * 1024 * 1024, max_projection_chunks: 1_000_000, max_status_entries: 1_000_000, @@ -227,17 +229,62 @@ impl StoreOptions { self.max_objects_per_transaction >= 1, "max_objects_per_transaction must be nonzero" ); + // The projection ceilings are bounded by what a manifest can actually + // represent, not only by what an operator would like to allow. + // + // `ProjectionStageManifestV1` carries `objects` and `chunk_digests` as + // flat canonical vectors, each capped at `MAX_CANONICAL_ITEMS`, and the + // whole encoding is capped at `MAX_CANONICAL_BYTES`. A configuration + // above those caps does not fail at seal after a long transfer — it + // *cannot* succeed, and every session admitted under it burns a + // principal's whole staging budget on a transfer guaranteed to be + // refused when its manifest is encoded. Refusing at startup is the only + // point where the operator learns this from the configuration rather + // than from a stuck mirror. + // + // The old 100,000,000 default asserted a capability the format does not + // have. Restoring it is not a matter of raising a decode ceiling: + // supporting a hundred million objects requires a versioned + // chunked/indexed manifest, so that a reader can bound its work without + // materializing the whole membership set. A larger hostile-decode + // ceiling would buy the object count by giving up the property that + // makes the ceiling worth having. + let canonical_items = levcs_protocol::codec::MAX_CANONICAL_ITEMS as u64; + let canonical_bytes = levcs_protocol::codec::MAX_CANONICAL_BYTES as u64; require!( - self.max_projection_objects >= 1, - "max_projection_objects must be nonzero" + self.max_projection_objects >= 1 && self.max_projection_objects <= canonical_items, + "max_projection_objects must be in 1..={canonical_items} while the staging \ + manifest is one flat canonical vector, got {}", + self.max_projection_objects ); require!( - self.max_projection_bytes >= 1, - "max_projection_bytes must be nonzero" + self.max_projection_chunks >= 1 + && u64::from(self.max_projection_chunks) <= canonical_items, + "max_projection_chunks must be in 1..={canonical_items}: the manifest's \ + chunk_digests vector is bounded by the same item ceiling, got {}", + self.max_projection_chunks ); + // Bytes are bounded transitively rather than directly: object bytes + // travel in `ProjectionStageChunkV1`s, one canonical encoding each, so + // no projection can carry more than its chunk allowance times the + // canonical byte ceiling. This is a necessary condition and not a + // sufficient one — a chunk also pays framing and descriptor overhead, + // so a configuration passing here can still refuse an individual chunk. + // A configuration failing here is unsatisfiable for every projection at + // the declared maximum, which is the case worth refusing at startup. + let representable_projection_bytes = u64::from(self.max_projection_chunks) + .checked_mul(canonical_bytes) + .ok_or_else(|| { + StoreError::InvalidConfiguration( + "max_projection_chunks times the canonical byte ceiling overflows u64".into(), + ) + })?; require!( - self.max_projection_chunks >= 1, - "max_projection_chunks must be nonzero" + self.max_projection_bytes >= 1 + && self.max_projection_bytes <= representable_projection_bytes, + "max_projection_bytes must be in 1..={representable_projection_bytes} \ + (max_projection_chunks * {canonical_bytes}), got {}", + self.max_projection_bytes ); require!( self.max_status_entries >= 1, @@ -472,6 +519,52 @@ mod tests { ); } + #[test] + fn projection_ceilings_above_what_a_manifest_can_encode_are_refused() { + let items = levcs_protocol::codec::MAX_CANONICAL_ITEMS as u64; + + let mut o = valid(); + o.max_projection_objects = items; + o.validate() + .expect("exactly the canonical item ceiling is representable"); + o.max_projection_objects = items + 1; + assert!( + o.validate().is_err(), + "a projection larger than one canonical vector can never seal a manifest" + ); + + let mut o = valid(); + o.max_projection_chunks = u32::try_from(items + 1).expect("ceiling fits u32"); + o.staging_max_files_per_session = u64::from(o.max_projection_chunks) + 2; + o.staging_max_files_per_principal = o.staging_max_files_per_session * 2; + o.staging_max_files_global = o.staging_max_files_per_session * 8; + assert!( + o.validate().is_err(), + "chunk_digests is bounded by the same item ceiling as the object vector" + ); + + // Bytes are refused only where the chunk allowance cannot carry them, + // so this asserts the transitive bound rather than a fixed number. + let mut o = valid(); + o.max_projection_chunks = 1; + o.max_projection_bytes = levcs_protocol::codec::MAX_CANONICAL_BYTES as u64 + 1; + assert!( + o.validate().is_err(), + "one chunk cannot carry more than one canonical encoding's worth of bytes" + ); + } + + #[test] + fn the_default_projection_ceilings_are_the_ones_the_format_can_represent() { + let o = valid(); + assert_eq!( + o.max_projection_objects, + levcs_protocol::codec::MAX_CANONICAL_ITEMS as u64, + "the default is the manifest ceiling; raising it needs a versioned \ + chunked manifest, not a larger decode ceiling" + ); + } + #[test] fn staging_nested_bounds_must_be_monotonic() { let mut o = valid(); diff --git a/crates/levcs-store/src/recovery.rs b/crates/levcs-store/src/recovery.rs index 07a95bc..9c2739e 100644 --- a/crates/levcs-store/src/recovery.rs +++ b/crates/levcs-store/src/recovery.rs @@ -81,14 +81,14 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use im::Vector; -use levcs_core::{ObjectId, ObjectType}; +use levcs_core::ObjectId; use levcs_protocol::oracle::{self, RecoveredTailFact, RecoveryOutcome}; use levcs_protocol::v2::{RefMutation, RefTarget, StagedProjectionInstallV1, TypedRefCas}; use crate::checkpoint::{Checkpoint, CheckpointError, CheckpointLoad, ReceiptRecord, RefRecord}; use crate::format::{ - CurrentPointer, Frame, FrameError, FrameHeader, FrameObjectsV1, JournalHeader, Manifest, - TailRange, TransactionFramePayloadV1, JOURNAL_HEADER_LEN, + object_type_code, CurrentPointer, Frame, FrameError, FrameHeader, FrameObjectsV1, + JournalHeader, Manifest, TailRange, TransactionFramePayloadV1, JOURNAL_HEADER_LEN, }; use crate::index::{IndexDelta, IndexKey, IndexLocation, IndexRun, NamespaceCatalog}; use crate::journal::{Journal, QuarantinedTail, ScannedFrame, TailScan, TailStop}; @@ -1082,18 +1082,6 @@ impl PayloadFacts for CanonicalPayloadFacts { } } -fn object_type_code(value: ObjectType) -> u8 { - // Exhaustive so a new logical object type cannot acquire an accidental - // recovered-index representation. - match value { - ObjectType::Blob => 1, - ObjectType::Tree => 2, - ObjectType::Commit => 3, - ObjectType::Release => 4, - ObjectType::Authority => 5, - } -} - /// A shard-sequence fault. /// /// **A distinct type from [`RepoSequenceFault`].** Scope 4-A2 deliverable 4 @@ -2530,24 +2518,20 @@ fn apply_recovered_refs( refs: &mut Vec, replayed: &[ReplayedFrame], ) -> Result<(), StoreError> { - let mut state: BTreeMap<(NamespaceId, u8, Vec), ObjectId> = refs + // Keyed by the typed target, not by the physical `(ref_kind, name)` pair. + // Replay compares a checkpointed record against a frame's `RefTarget`, so + // whichever side is converted, one of them is being interpreted — and doing + // it here meant restating the checkpoint's code table in a module that has + // no way to notice when the two drift. + let mut state: BTreeMap<(NamespaceId, RefTarget), ObjectId> = refs .iter() - .map(|record| { - ( - (record.namespace, record.ref_kind, record.name.clone()), - record.target, - ) - }) - .collect(); + .map(|record| Ok(((record.namespace, record.target()?), record.target))) + .collect::>()?; for frame in replayed { let facts = &frame.facts; for update in &frame.payload.ref_updates { - let (kind, name) = match &update.target { - RefTarget::Branch(name) => (1u8, name.as_bytes().to_vec()), - RefTarget::Release(name) => (2u8, name.as_bytes().to_vec()), - }; - let key = (facts.namespace, kind, name); + let key = (facts.namespace, update.target.clone()); let observed = state.get(&key).copied(); if observed != update.expected { return Err(StoreError::Corruption(format!( @@ -2568,12 +2552,7 @@ fn apply_recovered_refs( *refs = state .into_iter() - .map(|((namespace, ref_kind, name), target)| RefRecord { - namespace, - ref_kind, - name, - target, - }) + .map(|((namespace, target), object)| RefRecord::from_target(namespace, &target, object)) .collect(); Ok(()) } diff --git a/crates/levcs-store/src/types.rs b/crates/levcs-store/src/types.rs index ba584f6..3c4dc3f 100644 --- a/crates/levcs-store/src/types.rs +++ b/crates/levcs-store/src/types.rs @@ -220,7 +220,7 @@ impl From for StoreError { } } -/// Signs `CommittedTransactionV1` event digests on behalf of the instance. +/// Signs committed-transaction evidence on behalf of the instance. /// /// Registered by instance composition (plan §5.2). The store calls it before /// append and never decides who may sign — that is a federation trust @@ -228,7 +228,21 @@ impl From for StoreError { pub trait CommitEvidenceSigner: Send + Sync { fn key_epoch(&self) -> u64; fn public_key(&self) -> [u8; 32]; - fn sign_event(&self, event_digest: &ObjectId) -> Result<[u8; 64], SignerError>; + + /// Signs `SignedCommittedTransactionV1::signing_digest`, **not** the bare + /// event digest. + /// + /// The parameter is named for the value a correct caller must pass because + /// nothing downstream can detect the substitution until verification + /// fails. The frozen `SignedCommittedTransactionV1::verify` recomputes + /// `signing_digest(transaction, source_key_epoch, durability_result)` and + /// checks `source_signature` against it, so a signature over the event + /// digest alone would omit the key epoch and the durability result and + /// would be rejected by every verifier — including the mirror path in + /// another instance, after the transaction is already durable. The event + /// digest is the chain identity that `previous_event_digest` links; it is + /// not the signature message. + fn sign_event(&self, signing_digest: &ObjectId) -> Result<[u8; 64], SignerError>; } /// Capability token for constructing a `ValidatedTransaction`. diff --git a/doc/instance-throughput-rewrite-plan.md b/doc/instance-throughput-rewrite-plan.md index 5dbb425..2b8f077 100644 --- a/doc/instance-throughput-rewrite-plan.md +++ b/doc/instance-throughput-rewrite-plan.md @@ -978,6 +978,130 @@ captured read cannot race reclamation. `GATE_EXIT=0`. B1 NamespaceTxn, B3 StagingSessions, and B4 StoreHarnessB may now dispatch against that surface; changes to it require another contract review here. +##### Contract review 2026-07-28-A + +B2's adversarial review of the B1/B3 slice returned five interface requests against frozen +surfaces. All five are granted, one with a tightened bound, and they land as a single +reviewed change before either package's fix pass — so that B1 and B3 consume a contract +rather than negotiate with it. Four of the five exist to delete a restated frozen table, +which is the Wave A finding in its interface form: a package that cannot reach a mapping +does not stop needing it, it writes a second copy that agrees today. + +**1. `CommitEvidenceSigner::sign_event` signs the signing digest, and now says so.** The +parameter was named `event_digest`. What a correct implementation must sign is +`SignedCommittedTransactionV1::signing_digest(transaction, source_key_epoch, +durability_result)`, because that is the value the frozen `verify` recomputes. The two are +not interchangeable: the signing digest commits to the key epoch and the durability result +in addition to the canonical transaction, while the event digest is the chain identity that +`previous_event_digest` links. B1 inferred this correctly and passes the signing digest; +the interface, read literally, told it to do something else. + +The weaker fix — a doc line on the trait, leaving the parameter named `event_digest` — was +rejected because of where the error surfaces. A signature over the wrong message is +produced by the signer, accepted by the store, fenced, and made durable, and is first +detected by a *verifier*, which may be a mirror on another instance. There is no point +between those two events at which anything can decline the transaction. An interface whose +misuse is undetectable until after durability has to state the requirement in the name a +caller types, not in prose beside it. + +Amended: `types.rs`, and scope §2.2 where the frozen signature is printed. No +implementation changed; `engine.rs` already passed the correct value. + +**2. `TransactionEvidenceV1::actor()`, which closes a P1 rather than adding a +convenience.** `validate_mirror_application` requires `destination_event.actor == +source_instance` for all four mirror kinds, and the administrative signing digests bind +`actor` directly, so `actor` is a property of the evidence that the committed event +restates. B1 filled it from the destination signer's public key — the only value it could +reach — which is a durable event that signs, fences, and publishes and *then* fails +evidence verification at the mirror. That is the worst available failure ordering: the +transaction is committed and unwithdrawable before anything says it is malformed. + +The accessor is exhaustive over the six variants and reads the client principal off the +signed envelope's signer. Deriving it in the store instead was rejected on the grounds that +a second derivation is precisely what produced the defect: whatever the store computes, +`validate_mirror_application` is the authority, and only the protocol crate can be wrong +about it in one place. + +Amended: `v2.rs`. `phase0_consumers.rs`'s `destination_event_for` helper now takes the +actor from the accessor instead of its own match, and its `_` catch-all — which had been +supplying both the actor and a constant operation ID to four named variants — is replaced +with those four named arms. The same edit adds a test asserting the actor of every evidence +variant and proving that substituting the destination instance's own key makes mirror +validation fail. Charter item 8: the accessor's first consumer is a test that would have +caught the P1, not a helper beside it. + +**3. One object-type table in the crate.** `format::object_type_code` and an identical +private copy in `recovery.rs` both existed, so `engine.rs` wrote a third. `format`'s is now +`pub(crate)` and `recovery.rs`'s twin is deleted; `recovery.rs` calls the shared one. The +half-measure — exposing `format`'s and leaving recovery's in place — would have left the +crate with two tables and a new way to reach a third, which is worse than either, because +the exposure makes it look resolved. + +**4. Typed `RefRecord` conversion.** `checkpoint::RefRecord` carried `ref_kind: u8` with no +way to interpret it, so `engine.rs` restated the branch/release codes to read a record +`checkpoint.rs` had written. `RefRecord::from_target` and `RefRecord::target` now own that +conversion, with the code table stated once per direction in `checkpoint.rs` and +`read_applied_refs` rewired onto the decode half. `recovery.rs`'s replay map is keyed by +`RefTarget` rather than by the physical `(kind, name)` pair, which removes its copy too. + +An unknown code is refused as `CheckpointError::RefKind(u8)`, carrying the byte. Neither a +default to `Branch` nor a skip is acceptable: a ref table is derived state that the next +checkpoint writes back, so a silently reclassified ref is laundered into the record within +one checkpoint interval and is indistinguishable from a ref that was always there. Naming +the byte is the difference between a torn write and a reader that predates a kind a newer +writer emits. + +The refusal is at interpretation, not at checkpoint decode. Rejecting an unknown kind while +decoding the body would be stronger — it would let checkpoint fallback try another +generation instead of failing at first use — but it changes which generations are loadable, +which is a durability-visible change to a frozen reader and beyond what these five rulings +grant. It deserves its own ruling; it does not deserve to ride along beside four table +deletions. + +`format.rs` keeps its own ref-kind table for the *frame* encoding, deliberately. That is +not the duplicate that mattered: the frame and the checkpoint are independently versioned +physical formats, each self-consistent, and a frame code is only ever compared to a frame +code. The copies worth removing were the ones interpreting a `RefRecord` produced +elsewhere. + +**5. Projection ceilings, approved with a tightened bound.** `max_projection_objects` +defaulted to 100,000,000 while `ProjectionStageManifestV1.objects` is a flat canonical +vector capped at `MAX_CANONICAL_ITEMS` (1,000,000). The ceiling is now that cap and the +default is lowered to it; `max_projection_chunks` is capped identically against +`chunk_digests`; and `max_projection_bytes` is capped at `max_projection_chunks * +MAX_CANONICAL_BYTES`, since object bytes travel in per-chunk canonical encodings. The byte +bound is necessary and not sufficient — a chunk also pays framing — and it is stated that +way in the code, because a bound advertised as exact and enforced as approximate is worse +than one that admits what it is. + +**The reasoning that belongs on the record: supporting a hundred million objects requires a +versioned chunked/indexed manifest design, not a larger hostile-decode ceiling.** Raising +`MAX_CANONICAL_ITEMS` would buy the object count by giving up the property the ceiling +exists for — that a reader can bound its work before it materializes an attacker-declared +vector. The right shape is a manifest a reader can traverse in pieces, and that is a +Phase 2+ format with its own version, not a constant edit. + +Refusing at startup rather than at seal is the other half. The old configuration did not +fail early; it admitted sessions, pinned a principal's whole staging budget, accepted hours +of transfer at the supported floor, and refused at manifest encode. `options.rs` now +refuses the configuration, which is the only point where the operator learns this from the +configuration instead of from a stuck mirror. + +Amended: `options.rs`, with tests asserting the boundary in both directions, that the +default *is* the format ceiling, and that the byte bound tracks the chunk allowance rather +than a fixed number. `staging.rs`'s per-session `MAX_CANONICAL_ITEMS` checks are now +redundant with startup validation; they are B3's to remove or to keep as a belt, and either +is defensible now that no admitted configuration can reach them. + +**Expected collateral.** The tightened ceilings break exactly one test, +`staging_sessions.rs::a_declared_projection_over_a_configured_ceiling_is_refused_before_pinning`, +which sets `max_projection_chunks = 1` while leaving `max_projection_bytes` at the 1 TiB +default — now an unsatisfiable configuration that `StoreOptions::validate` refuses before +the store opens. The test's intent survives; its fixture has to vary the two together. It +is B3's file and folds into B3's fix pass. The gate is transiently red between the two +passes, which is accepted: landing the contract first is what keeps both packages from +implementing against a surface that is about to move. + ### Phase 1 — storage engine spine Lead first defines sealed transaction/frame/snapshot interfaces and file ownership. That deliverable (D0) landed on 2026-07-24 as `crates/levcs-store`: the frozen public API compiling against `StoreError::NotImplemented`, the file-ownership split, strict configuration validation, the single durability syscall funnel with its counters and fault hooks, the failpoint registry in enforced one-to-one correspondence with `oracle::AppendFailpoint`, and the journal-level drive seam that lets the crash harness run in Wave A. The enforced gate is `scripts/check-phase1.sh`, which runs `check-phase0.sh` first so the Phase 0 freeze stays enforced. That work is scoped in `doc/phase1-storage-spine-scope.md`, which realizes this section as a file-ownership matrix, a frozen `levcs-store` API, a physical format and durability/recovery specification, per-package deliverables and acceptance criteria, the Wave A adversarial review charter, and the capacity analysis for P2 on the frozen reference hardware. This plan remains authoritative; that document is the Phase 1 realization of it and lists the decisions that must be resolved before Wave A starts. diff --git a/doc/phase1-storage-spine-scope.md b/doc/phase1-storage-spine-scope.md index 864257f..2bad7a6 100644 --- a/doc/phase1-storage-spine-scope.md +++ b/doc/phase1-storage-spine-scope.md @@ -238,7 +238,8 @@ Also frozen in D0: pub trait CommitEvidenceSigner: Send + Sync { fn key_epoch(&self) -> u64; fn public_key(&self) -> [u8; 32]; - fn sign_event(&self, event_digest: &ObjectId) -> Result<[u8; 64], SignerError>; + /// `SignedCommittedTransactionV1::signing_digest`, not the event digest. + fn sign_event(&self, signing_digest: &ObjectId) -> Result<[u8; 64], SignerError>; } /// Privileged construction. Sealed so instance validation, recovery, and @@ -250,6 +251,15 @@ impl ValidatedTransaction { pub struct PrivilegedConstruction(()); ``` +The parameter name is normative, and contract review 2026-07-28-A amended it from +`event_digest`. What the signature must cover is +`SignedCommittedTransactionV1::signing_digest(transaction, source_key_epoch, +durability_result)`, because that is what the frozen `verify` recomputes. A signature over +the bare event digest omits the key epoch and the durability result and verifies nowhere — +including at a mirror on another instance, after the transaction is already durable. The +event digest is the chain identity `previous_event_digest` links; it is not the signature +message. + `PrivilegedConstruction` must **not** be obtainable from a `&StoreEngine`. A `StoreEngine::privileged()` method would make the seal decorative: anything holding an engine — which is everything that can call `submit` — could mint one. D0 gates it instead @@ -1193,6 +1203,16 @@ contract review in `doc/instance-throughput-rewrite-plan.md`. D0-B already exerc nine of its eleven items amend a frozen or signature-frozen file, and each is a recorded amendment rather than an edit. +**Contract review 2026-07-28-A** is the second exercise of that rule, this time driven by +B2's findings against the B1/B3 slice rather than by the lead's own integration. Five +interface requests were granted and landed on the frozen surfaces ahead of the packages' +fix passes: `CommitEvidenceSigner::sign_event`'s parameter renamed to `signing_digest` +(§2.2); a protocol-owned `TransactionEvidenceV1::actor()`; `format::object_type_code` made +`pub(crate)` with `recovery.rs`'s twin removed; typed `RefRecord::target()`/`from_target()` +on the checkpoint record; and the projection ceilings above. Four of the five exist to +delete a restated frozen table — B1 filed each of them instead of quietly restating it a +third time, which is exactly what §6.0 asks for and what Wave A did not get. + The frozen surface is the **library**. Wave A's harness — `src/bin/store-crash-driver.rs`, `src/bin/store-bench.rs`, `tests/crash_matrix.rs`, and `scripts/verify-store-recovery.sh` — is not frozen; ownership transfers to B4, whose whole purpose is to extend it (§6.6). Test @@ -1541,7 +1561,9 @@ criteria. middle, and last position of a forming group, in each case with later signer results already available.** Prove the failed transaction appends nothing, and that every retained suffix transaction is re-sequenced, re-chained, and re-signed as necessary, leaving no - sequence gap. Signing covers the event digest, which chains `previous_event_digest`, so + sequence gap. Signing covers `SignedCommittedTransactionV1::signing_digest`, which commits + to the whole committed transaction and therefore to the chained + `previous_event_digest`, so dropping a member from the middle of a group invalidates every signature after it — the already-returned results for the suffix are now signatures over a chain that no longer exists. A partial repair here produces a durable, correctly-fenced frame carrying a @@ -1600,6 +1622,15 @@ B3's deliverables: minimum supported transfer rate must make one complete transfer possible, or creation rejects *before* pinning anything. A session that cannot finish is a session that only consumes budget. + + Contract review 2026-07-28-A adds the representability half of the same argument. + `max_projection_objects` and `max_projection_chunks` are capped at + `codec::MAX_CANONICAL_ITEMS`, and `max_projection_bytes` at + `max_projection_chunks * codec::MAX_CANONICAL_BYTES`, because the sealed manifest is one + flat canonical vector and the chunks are one canonical encoding each. Startup refuses a + configuration above those caps rather than admitting sessions whose manifest could never + encode. `max_projection_objects` now defaults to `MAX_CANONICAL_ITEMS`; the previous + default of 100,000,000 advertised a capacity the format does not have. 5. **Artifacts are written by maintenance workers, synced, uniquely named, and unreferenced.** They never enter namespace membership, object-existence answers, snapshots, refs, receipts, event feeds, dedupe state, or `CURRENT`. @@ -1684,6 +1715,36 @@ a silently dropped pin is a leaked session that no expiry will collect. Both packages write a test driving the full lifecycle — pin, adopt, publish, unpin — and B2 checks the two agree. A seam with tests on only one side is the Wave A finding restated. +#### Carry-forwards after the first B3 slice — recorded, not closed + +Two properties are implemented and not operated. Both are recorded here rather than only in +an ignored test, because a carry-forward that lives in a test attribute is invisible to +anyone reading the scope to decide whether a deliverable is met, which is the mechanism by +which Wave A's `GroupBuilder` gap nearly shipped as satisfied. + +1. **Production staging use is incomplete.** The *ownership* seam is closed — `StoreEngine::open` + constructs the single `ProjectionStaging` under the held `RecoverySession` and passes it as + recovery's `ProjectionRecoveryResolver` — but the *use* seam is not. No production path + calls `begin`, `finalize`, or `adopt_projection`; they are reachable only from B3's own + tests. Deliverable 5's security acceptance — sealed objects invisible to + `RepoSnapshot::locate` until a `submit` adopts them — therefore remains unasserted, and its + test stays ignored with both blockers named. Having a production caller for construction is + not the same as having one for the mechanism, and only the second discharges charter item 8. + +2. **`ProjectionStaging::expire` has no scheduler.** Expiry is implemented and unit-tested, and + nothing in production drives it. Session age is therefore a bound that is enforced when + asked and never asked — a limit no deployment currently applies. The bound is not met until + something operates it. + +**The `StoreEngine` staging accessor B3 requested is a pending interface amendment, and it must +not be a bare `Arc`.** A cloned `Arc` can outlive `EngineShared`, survive the +release of the root `LOCK`, and keep serving staging operations against a root this process no +longer holds — which reintroduces the exact defect the root-lock-proof constructor was added to +make inexpressible, and reintroduces it from the reader side where the constructor cannot see +it. The eventual façade or handle must retain engine and root-lock authority for the lifetime +of every session it creates. Until that is designed and frozen, the accessor does not exist and +the acceptance test above stays blocked. + ### 6.6 B4 — StoreHarnessB Owns the crash driver, the benchmark, the matrix, and the recovery script.