From be631ce0ead3c7f30ff519a1154e2ec951f014a3 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Mon, 31 Aug 2026 15:29:01 +0800 Subject: [PATCH 01/17] fix(state): allow bucket capacity to reach 2^40 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `shi_rehash` bounded a new capacity with `BUCKET_SLOT_ID_MASK` (2^40 - 1), the largest slot *index*, but capacity is a slot *count*. Since capacities only ever double up from `MIN_BUCKET_SIZE`, the largest one the assert admitted was 2^39 — half the 2^40 a bucket subtree can actually address. Introduce `MAX_BUCKET_SIZE`, derived from the subtree structure (2^32 segments of 256 slots), and bound the capacity with it. Also correct the capacity-to-level table above `subtree_root_level`, which was inverted. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013Spbs54VBmctgmRydZqNfj --- salt/src/constant.rs | 40 +++++++++++++++++++++++++++++++++- salt/src/state/state.rs | 43 ++++++++++++++++++++++++++++++++++--- salt/src/trie/node_utils.rs | 30 +++++++++++++++++++++----- 3 files changed, 104 insertions(+), 9 deletions(-) diff --git a/salt/src/constant.rs b/salt/src/constant.rs index 8f47c24b..70422e64 100644 --- a/salt/src/constant.rs +++ b/salt/src/constant.rs @@ -35,6 +35,22 @@ pub const MIN_BUCKET_SIZE: usize = 1 << MIN_BUCKET_SIZE_BITS; /// Set equal to MIN_BUCKET_SIZE since metadata buckets don't need to resize /// and maintaining uniform size simplifies the implementation. pub const META_BUCKET_SIZE: usize = MIN_BUCKET_SIZE; +/// Maximum capacity of a SALT bucket (2^40 = 1,099,511,627,776 slots). +/// +/// A bucket keeps its slots in the deepest level of its subtree, which holds +/// `TRIE_WIDTH^(MAX_SUBTREE_LEVELS - 1)` = 256^4 = 2^32 segments of `MIN_BUCKET_SIZE` +/// = 256 slots each. So a bucket can address 2^32 * 2^8 = 2^40 slots, and +/// `subtree_root_level(MAX_BUCKET_SIZE)` is 0, the topmost subtree level. +/// +/// This equals `1 << BUCKET_SLOT_BITS`: slot IDs run over `0..MAX_BUCKET_SIZE`, whose +/// largest member is `BUCKET_SLOT_ID_MASK`, so every slot ID still fits in the low +/// `BUCKET_SLOT_BITS` bits of a `SaltKey`. +/// +/// **Note**: `MAX_BUCKET_SIZE` is a slot *count*, whereas `BUCKET_SLOT_ID_MASK` is the +/// largest slot *index*. Bounding a capacity by the mask caps it one doubling short, +/// at 2^39, because capacities only ever double up from `MIN_BUCKET_SIZE`. +pub const MAX_BUCKET_SIZE: u64 = + 1 << ((MAX_SUBTREE_LEVELS - 1) * TRIE_WIDTH_BITS + MIN_BUCKET_SIZE_BITS); // ============================================================================ // Trie Structure Constants @@ -129,7 +145,8 @@ pub const STARTING_NODE_ID: [usize; MAX_SUBTREE_LEVELS] = [ pub const BUCKET_ID_BITS: usize = 24; /// Maximum number of bits to represent a slot index in a bucket. -/// 40 bits supports up to ~1 trillion slots per bucket, providing ample room for growth. +/// 40 bits holds every slot index of a maximally expanded bucket, which has +/// `MAX_BUCKET_SIZE` = 2^40 (~1.1 trillion) slots indexed `0..=BUCKET_SLOT_ID_MASK`. pub const BUCKET_SLOT_BITS: usize = 40; /// Mask to extract the slot ID from a NodeId or SaltKey. @@ -269,6 +286,7 @@ mod tests { assert_eq!(MIN_BUCKET_SIZE_BITS, 8); assert_eq!(MIN_BUCKET_SIZE, 256); assert_eq!(META_BUCKET_SIZE, 256); + assert_eq!(MAX_BUCKET_SIZE, 1_099_511_627_776); assert_eq!(MAIN_TRIE_LEVELS, 4); assert_eq!(MAX_SUBTREE_LEVELS, 5); assert_eq!(TRIE_WIDTH_BITS, 8); @@ -287,6 +305,26 @@ mod tests { assert_eq!(STARTING_NODE_ID, [0, 1, 257, 65_793, 16_843_009]); } + /// `MAX_BUCKET_SIZE` must be the slot *count* a full subtree addresses, not the + /// largest slot *index*. Confusing the two caps buckets one doubling short. + #[test] + fn test_max_bucket_size_matches_subtree_capacity() { + // The deepest subtree level has TRIE_WIDTH^(MAX_SUBTREE_LEVELS - 1) segments, + // each holding MIN_BUCKET_SIZE slots. + let segments = (TRIE_WIDTH as u64).pow((MAX_SUBTREE_LEVELS - 1) as u32); + assert_eq!(segments, 1u64 << 32); + assert_eq!(MAX_BUCKET_SIZE, segments * MIN_BUCKET_SIZE as u64); + + // Every slot index of a maximally expanded bucket fits the 40-bit slot field, + // and the largest one is exactly the mask. + assert_eq!(MAX_BUCKET_SIZE, 1u64 << BUCKET_SLOT_BITS); + assert_eq!(MAX_BUCKET_SIZE - 1, BUCKET_SLOT_ID_MASK); + + // Capacities only ever double up from MIN_BUCKET_SIZE, so bounding one by + // BUCKET_SLOT_ID_MASK would have stopped at 2^39 instead of 2^40. + assert_eq!(MAX_BUCKET_SIZE / BUCKET_RESIZE_MULTIPLIER, 1u64 << 39); + } + #[test] fn test_default_commitment_boundary_selection() { assert_eq!(default_commitment(ROOT_NODE_ID), default_commitment(0)); diff --git a/salt/src/state/state.rs b/salt/src/state/state.rs index 94fc0a42..bd9e14c2 100644 --- a/salt/src/state/state.rs +++ b/salt/src/state/state.rs @@ -41,7 +41,7 @@ use super::{hasher, updates::StateUpdates}; use crate::{ - constant::{BUCKET_RESIZE_MULTIPLIER, BUCKET_SLOT_ID_MASK}, + constant::{BUCKET_RESIZE_MULTIPLIER, MAX_BUCKET_SIZE}, traits::StateReader, types::*, }; @@ -666,6 +666,11 @@ impl<'a, Store: StateReader> EphemeralSaltState<'a, Store> { /// /// If the new capacity is smaller than the number of existing entries in the bucket, /// the function returns early without making any changes to prevent data loss. + /// + /// ## Panics + /// + /// Panics if `new_capacity` exceeds [`MAX_BUCKET_SIZE`], the largest capacity a + /// bucket subtree can address (2^40 slots). pub fn shi_rehash( &mut self, bucket_id: BucketId, @@ -674,8 +679,8 @@ impl<'a, Store: StateReader> EphemeralSaltState<'a, Store> { out_updates: &mut StateUpdates, ) -> Result<(), Store::Error> { assert!( - new_capacity <= BUCKET_SLOT_ID_MASK, - "Exceeds max bucket capacity: {new_capacity} > {BUCKET_SLOT_ID_MASK}" + new_capacity <= MAX_BUCKET_SIZE, + "Exceeds max bucket capacity: {new_capacity} > {MAX_BUCKET_SIZE}" ); // Step 1: Extract all existing entries (but do not clear the cache yet) @@ -1057,6 +1062,7 @@ fn compute_resize_capacity(capacity: u64, used: u64) -> u64 { #[cfg(test)] mod tests { use super::*; + use crate::constant::BUCKET_SLOT_ID_MASK; use std::{collections::BTreeMap, vec, vec::Vec}; use crate::{ @@ -2729,6 +2735,37 @@ mod tests { } } + /// A bucket subtree addresses exactly `MAX_BUCKET_SIZE` = 2^40 slots, so that + /// capacity must be accepted. The bound used to be `BUCKET_SLOT_ID_MASK` (2^40 - 1), + /// which — since capacities only ever double from 256 — capped buckets at 2^39. + #[test] + fn test_max_bucket_capacity_is_a_reachable_power_of_two() { + // Capacities only ever double up from MIN_BUCKET_SIZE, so 2^40 is reachable: + // it is 2^39 resized once more. + assert_eq!( + compute_resize_capacity(1u64 << 39, 1u64 << 39), + MAX_BUCKET_SIZE + ); + // The superseded bound was BUCKET_SLOT_ID_MASK, one slot short of 2^40, so the + // largest power-of-two capacity it admitted was only 2^39. + assert_eq!(MAX_BUCKET_SIZE, BUCKET_SLOT_ID_MASK + 1); + } + + #[test] + #[should_panic(expected = "Exceeds max bucket capacity")] + fn test_shi_rehash_rejects_capacity_above_max() { + let reader = EmptySalt; + let mut state = EphemeralSaltState::new(&reader); + let mut updates = StateUpdates::default(); + // One doubling past the largest addressable capacity. + let _ = state.shi_rehash( + TEST_BUCKET, + 0, + MAX_BUCKET_SIZE * BUCKET_RESIZE_MULTIPLIER, + &mut updates, + ); + } + #[test] fn test_shi_rehash_too_small_capacity_has_no_side_effects() { let reader = EmptySalt; diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index f85f2a8f..18f7534a 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -366,19 +366,33 @@ pub(crate) fn subtree_leaf_start_key(node_id: &NodeId) -> SaltKey { /// to accommodate the given capacity. /// /// # Subtree Level Mapping +/// +/// The root climbs *upward* as capacity grows: a small bucket is a lone leaf at the +/// deepest level, while a maximal one is rooted at level 0 over a full 5-level subtree. +/// /// ```text -/// Level 0: Root (capacity = 1) [root:0] -/// Level 1: Small (capacity ≤ 256) [0] [1] ... [256] -/// Level 2: Medium (capacity ≤ 65,536) [0]...[256] [257]...[65536] -/// Level 3: Large (capacity ≤ 16,777,216) [0]...[65536] [65537]...[16777216] -/// Level 4: Extra Large (capacity ≤ 2^32) [0]...[16777216] [16777217]...[2^32] +/// capacity ≤ 2^8 (256) → level 4 a lone leaf, no internal nodes +/// capacity ≤ 2^16 (65,536) → level 3 over up to 256 leaves +/// capacity ≤ 2^24 (16,777,216) → level 2 over up to 65,536 leaves +/// capacity ≤ 2^32 (4,294,967,296) → level 1 over up to 16,777,216 leaves +/// capacity ≤ 2^40 (1,099,511,627,776) → level 0 over up to 4,294,967,296 leaves /// ``` /// +/// Each leaf ("segment") holds `MIN_BUCKET_SIZE` = 256 consecutive slots, so the +/// deepest level's 2^32 segments cap a bucket at 2^32 * 256 = 2^40 slots, which is +/// [`MAX_BUCKET_SIZE`](crate::constant::MAX_BUCKET_SIZE). +/// /// # Arguments /// * `capacity` - The total number of slots the bucket needs to accommodate /// /// # Returns /// The subtree level (0-4) that should serve as the root for this capacity +/// +/// # Panics +/// +/// Panics in debug builds (and wraps `level` in release builds) if `capacity` exceeds +/// `MAX_BUCKET_SIZE`, since no subtree level can hold it. Callers must bound capacity +/// first; `EphemeralSaltState::shi_rehash` asserts this. pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { // Start from the deepest possible level let mut level = MAX_SUBTREE_LEVELS - 1; @@ -396,6 +410,7 @@ pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { #[cfg(test)] mod tests { use super::*; + use crate::constant::MAX_BUCKET_SIZE; /// Tests the vc_position_in_parent function for various node types and positions. /// @@ -739,6 +754,11 @@ mod tests { (16777217, 1), // Just over 256^3 → level 1 (4294967296, 1), // 256^4 slots → still level 1 (4294967297, 0), // Just over 256^4 → root level + // The top of the range: a maximally expanded bucket is rooted at level 0 + // over 2^32 segments of 256 slots. Anything larger has nowhere to go. + (1 << 39, 0), // largest capacity the old bound allowed + (MAX_BUCKET_SIZE - 1, 0), // BUCKET_SLOT_ID_MASK + (MAX_BUCKET_SIZE, 0), // 2^40 — still fits the full subtree ]; for (capacity, expected) in cases { From 80d22c0d96825a41e3bbd1cdb0fcb4b65a8e5e6f Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Mon, 31 Aug 2026 15:33:40 +0800 Subject: [PATCH 02/17] docs(salt): state the 2^40 bucket-capacity ceiling consistently The capacity->subtree-level ladders in `MAX_SUBTREE_LEVELS`, the `node_utils` module doc and `get_subtree_levels` all trailed off before the top of the range ("65537+ slots: Root at higher levels as needed", "Capacity > 4,294,967,296", "And so on"), so none of them stated the ceiling this fix is about. The `MAX_SUBTREE_LEVELS` ladder also mapped 768-65,536 slots to level 2; it is level 3. The README said a bucket resizes "to a multiple of 256" when it in fact doubles - the very property that made the old bound bite at 2^39. Also pin the level-0 node-id arithmetic at max capacity in `test_subtrie_change_info_capacity_boundaries`: the deepest local node number at 2^40 is exactly `leftmost_node(5) - 1`, so the ceiling has zero slack. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013Spbs54VBmctgmRydZqNfj --- README.md | 2 +- salt/src/constant.rs | 12 ++++++++---- salt/src/traits.rs | 3 ++- salt/src/trie/node_utils.rs | 6 +++++- salt/src/trie/trie.rs | 23 +++++++++++++++++++++++ 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 73cc6caf..c450beba 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ The main trie's internal nodes also use homomorphic commitments. After bucket co While each step up the trie costs one ECMul, updates from multiple distinct child nodes are batched into a single update for their common parent. This optimization is extremely effective because the trie's width shrinks dramatically at higher levels, consolidating many changes at the leaf level into a small number of updates near the root. For example, updating 200,000 random keys in SALT requires a total of approximately 460,000 ECMul operations, or an amortized cost of about **2.3 ECMuls** per key. ### Bucket Growth -While the main SALT tree is static, the buckets are not. A bucket is initialized with 256 slots. When it fills up, it can be resized to a multiple of 256. If a bucket grows beyond 256 slots, it is partitioned into 256-slot segments. A new complete 256-ary **bucket tree** is built on top of these segments, and the root of this new tree becomes the bucket's new commitment (also the new leaf in the main trie). The diagram below shows a bucket tree of 768 slots. +While the main SALT tree is static, the buckets are not. A bucket is initialized with 256 slots. When it fills up, its capacity is doubled, so every capacity is a power of two from 256 up to 2^40 slots (the most a 5-level bucket tree can address). If a bucket grows beyond 256 slots, it is partitioned into 256-slot segments. A new complete 256-ary **bucket tree** is built on top of these segments, and the root of this new tree becomes the bucket's new commitment (also the new leaf in the main trie). The diagram below shows a bucket tree of 768 slots. ``` (In Main SALT Trie) diff --git a/salt/src/constant.rs b/salt/src/constant.rs index 70422e64..c313b974 100644 --- a/salt/src/constant.rs +++ b/salt/src/constant.rs @@ -70,11 +70,15 @@ pub const MAIN_TRIE_LEVELS: usize = 4; /// leaf nodes at the deepest level (level 4) of the MAXIMAL subtree structure. As /// bucket capacity increases, the subtree root moves UP to accommodate more leaves. /// -/// Structure evolution by capacity: +/// Structure evolution by capacity (see `subtree_root_level`): /// - 256 slots (1 segment): Single-node subtree, root at level 4 -/// - 512 slots (2 segments): Root at level 3, 2 leaf nodes at level 4 -/// - 768-65536 slots: Root at level 2, internal nodes at level 3, leaves at level 4 -/// - 65537+ slots: Root at higher levels as needed +/// - 512-65,536 slots: Root at level 3, leaves at level 4 +/// - 65,537-16,777,216 slots: Root at level 2 +/// - 16,777,217-4,294,967,296 slots: Root at level 1 +/// - 4,294,967,297-1,099,511,627,776 slots: Root at level 0, the full 5-level subtree +/// +/// The last row is the ceiling: the deepest level holds 256^4 = 2^32 segments, so a +/// bucket tops out at `MAX_BUCKET_SIZE` = 2^32 * 256 = 2^40 slots. /// /// Example for 512-slot bucket (2 segments): /// ```text diff --git a/salt/src/traits.rs b/salt/src/traits.rs index 2c57f630..25f0b8d2 100644 --- a/salt/src/traits.rs +++ b/salt/src/traits.rs @@ -167,7 +167,8 @@ pub trait StateReader: Debug + Send + Sync { /// - Capacity ≤ 256: 1 level (no internal nodes) /// - Capacity ≤ 65,536: 2 levels (1 internal + 1 leaf) /// - Capacity ≤ 16,777,216: 3 levels (2 internal + 1 leaf) - /// - And so on, up to 5 levels maximum + /// - Capacity ≤ 4,294,967,296: 4 levels (3 internal + 1 leaf) + /// - Capacity ≤ 1,099,511,627,776 (`MAX_BUCKET_SIZE`): 5 levels, the maximum /// /// # Arguments /// diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index 18f7534a..7c1e8065 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -102,12 +102,16 @@ //! ├─ Up to 65,536 Level 3 nodes //! └─ Up to 16,777,216 Level 4 leaf nodes //! -//! Capacity > 4,294,967,296: +//! Capacity ≤ 1,099,511,627,776 (2^40 = MAX_BUCKET_SIZE): //! Root at Level 0 //! NodeId = (bucket_id << 40) | 0 //! Full 5-level subtree structure +//! ├─ Up to 256 Level 1 nodes +//! └─ Up to 4,294,967,296 Level 4 leaf nodes //! ``` //! +//! 2^40 is the ceiling: the deepest level holds 256^4 = 2^32 segments of 256 slots. +//! //! ### Key Insights //! //! - The root node is always the **first node** at its level within the bucket's subtree namespace diff --git a/salt/src/trie/trie.rs b/salt/src/trie/trie.rs index e0ed3c65..9113bfc5 100644 --- a/salt/src/trie/trie.rs +++ b/salt/src/trie/trie.rs @@ -1152,6 +1152,10 @@ impl SubtrieChangeInfo { #[cfg(test)] mod tests { use super::*; + use crate::{ + constant::{BUCKET_SLOT_ID_MASK, MAX_BUCKET_SIZE, MIN_BUCKET_SIZE}, + types::leftmost_node, + }; use crate::{ mem_store::MemStore, state::{state::EphemeralSaltState, updates::StateUpdates}, @@ -1529,6 +1533,25 @@ mod tests { assert_change(512, 256, 3, 4); assert_change(65_536, 65_792, 3, 2); assert_change(65_537, 65_536, 2, 3); + + // Top of the range. Every capacity above 2^32 is rooted at level 0, so the + // 2^39 -> 2^40 expansion the old `BUCKET_SLOT_ID_MASK` bound rejected is an + // ordinary level-0 transition, and STARTING_NODE_ID[0] == 0 makes the top id + // exactly `bucket_id << BUCKET_SLOT_BITS`. + assert_change(1 << 32, (1 << 32) + 1, 1, 0); + assert_change(1 << 39, MAX_BUCKET_SIZE, 0, 0); + assert_change(MAX_BUCKET_SIZE, 1 << 39, 0, 0); + + // At maximum capacity the deepest node's local number must stay inside the + // 40-bit slot field, or it would bleed into the bucket-id bits. + let segments = MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64; + let deepest_local = STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + segments - 1; + assert_eq!(deepest_local, 4_311_810_304); + assert_eq!( + deepest_local, + leftmost_node(MAX_SUBTREE_LEVELS as u32).unwrap() - 1 + ); + assert!(deepest_local <= BUCKET_SLOT_ID_MASK); } /// Rebuilds a main trie node commitment from storage for testing purposes. From 282d385b288a077ffcd86a845c7e1493b894eedc Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Mon, 31 Aug 2026 15:36:01 +0800 Subject: [PATCH 03/17] docs(trie): don't overstate what bounds subtree_root_level's input The `# Panics` note added in the previous commit told callers that `shi_rehash` bounds capacity for them. It bounds only the `new_capacity` it is asked to resize to; a capacity read back from a stored `BucketMeta` is validated nowhere, and `BucketMeta::try_from` accepts any u64. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013Spbs54VBmctgmRydZqNfj --- salt/src/trie/node_utils.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index 7c1e8065..4e9deb04 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -396,7 +396,9 @@ pub(crate) fn subtree_leaf_start_key(node_id: &NodeId) -> SaltKey { /// /// Panics in debug builds (and wraps `level` in release builds) if `capacity` exceeds /// `MAX_BUCKET_SIZE`, since no subtree level can hold it. Callers must bound capacity -/// first; `EphemeralSaltState::shi_rehash` asserts this. +/// themselves. `EphemeralSaltState::shi_rehash` asserts it for the capacity it is +/// asked to resize *to*, but a capacity read back from a stored `BucketMeta` is not +/// validated anywhere, so callers that source one from a `StateReader` are on their own. pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { // Start from the deepest possible level let mut level = MAX_SUBTREE_LEVELS - 1; From 4b5270a9d0a2578ad972db13ff69a9aa294a23da Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Mon, 31 Aug 2026 15:40:28 +0800 Subject: [PATCH 04/17] test(state): decouple the capacity test from the load-factor threshold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `test_max_bucket_capacity_is_a_reachable_power_of_two` asserted that `compute_resize_capacity(2^39, 2^39)` returns 2^40, which holds only at the default 80% load factor. The `test-bucket-resize` feature drops the threshold to 1%, so the same call doubles to 2^46 and the test failed in the CI job that enables it. State the claim through `BUCKET_RESIZE_MULTIPLIER` instead — one doubling past 2^39 is 2^40, whatever the threshold — and keep `compute_resize_capacity` covered with a threshold-independent assertion that a resize lands on a power of two. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013Spbs54VBmctgmRydZqNfj --- salt/src/state/state.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/salt/src/state/state.rs b/salt/src/state/state.rs index bd9e14c2..d8be06fe 100644 --- a/salt/src/state/state.rs +++ b/salt/src/state/state.rs @@ -2741,11 +2741,16 @@ mod tests { #[test] fn test_max_bucket_capacity_is_a_reachable_power_of_two() { // Capacities only ever double up from MIN_BUCKET_SIZE, so 2^40 is reachable: - // it is 2^39 resized once more. - assert_eq!( - compute_resize_capacity(1u64 << 39, 1u64 << 39), - MAX_BUCKET_SIZE - ); + // it is 2^39 doubled once more. (Stated via the multiplier rather than a + // `compute_resize_capacity` call, because how many doublings a given `used` + // triggers depends on the load-factor threshold, which `test-bucket-resize` + // overrides.) + assert_eq!((1u64 << 39) * BUCKET_RESIZE_MULTIPLIER, MAX_BUCKET_SIZE); + assert!(MAX_BUCKET_SIZE.is_power_of_two()); + // Whatever the threshold, a resize lands on a power of two. + let resized = compute_resize_capacity(MIN_BUCKET_SIZE as u64, MIN_BUCKET_SIZE as u64); + assert!(resized.is_power_of_two()); + assert!(resized > MIN_BUCKET_SIZE as u64); // The superseded bound was BUCKET_SLOT_ID_MASK, one slot short of 2^40, so the // largest power-of-two capacity it admitted was only 2^39. assert_eq!(MAX_BUCKET_SIZE, BUCKET_SLOT_ID_MASK + 1); From 802e30d36e291c2b1899130373db6e6e81613a20 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 15:14:32 +0800 Subject: [PATCH 05/17] fix(proof): reject out-of-range subtree levels on decode `fx_hashmap_serde::deserialize` accepted any u8 as a bucket's subtree level, but a prover only ever emits 1 (a bucket at MIN_BUCKET_SIZE, including every metadata bucket) through MAX_SUBTREE_LEVELS (a bucket at MAX_BUCKET_SIZE), and nothing downstream is defined outside that range. `parents_and_points` embeds any level >= 2 into an encoded parent id, and `logic_parent_id` decodes it back with `STARTING_NODE_ID[MAX_SUBTREE_LEVELS - level]`, which underflows for 6 and 7; the trie's `update_bucket_subtrees` turns a witnessed level into a capacity of `TRIE_WIDTH^level`, which for 6 or more sends `subtree_root_level` below level 0; and level 0 makes `update_bucket_subtrees` treat an expanded bucket as unexpanded while `parents_and_points` derives an inconsistent node set for it. A proof or witness from an untrusted source could therefore panic a verifier (levels 6 and 7) or drive it through a shape it was never meant to handle (level 0), instead of being rejected at the decode boundary. Reject a level outside `1..=MAX_SUBTREE_LEVELS` in the visitor, next to the existing duplicate-key check. The wire format is unchanged. The check also lands in `stateless-core::LightWitness`, which reuses the module through `#[serde(with = "salt::fx_hashmap_serde")]`; its bincode round-trip test currently feeds levels 0, 7 and 255 and will need in-range values once it moves past salt v1.0.5. `round_trip_preserves_entries` used 0 and 255 as levels and now uses in-range ones; new tests cover 0, MAX_SUBTREE_LEVELS + 1 and both boundaries. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/proof/prover.rs | 69 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 65 insertions(+), 4 deletions(-) diff --git a/salt/src/proof/prover.rs b/salt/src/proof/prover.rs index 14b92db8..0dbaed26 100644 --- a/salt/src/proof/prover.rs +++ b/salt/src/proof/prover.rs @@ -1,6 +1,6 @@ //! Prover for the Salt proof use crate::{ - constant::{BUCKET_SLOT_ID_MASK, DOMAIN_SIZE, STARTING_NODE_ID}, + constant::{BUCKET_SLOT_ID_MASK, DOMAIN_SIZE, MAX_SUBTREE_LEVELS, STARTING_NODE_ID}, proof::{ shape::{connect_parent_id, logic_parent_id, parents_and_points}, subtrie::create_sub_trie, @@ -152,6 +152,14 @@ pub struct SaltProof { /// and breaks downstream alloy-tx-macros 1.0.23). Entries are emitted in /// ascending key order to keep proof bytes deterministic across provers. /// Also reused downstream (e.g. `stateless-core::LightWitness`) via `#[serde(with = "salt::fx_hashmap_serde")]`. +/// +/// Deserialization rejects a duplicate `BucketId` and any level outside +/// `1..=MAX_SUBTREE_LEVELS`. A bucket at `MIN_BUCKET_SIZE` capacity has one +/// level (its single segment) and a bucket at `MAX_BUCKET_SIZE` has +/// `MAX_SUBTREE_LEVELS`, so no prover emits anything else, and the code that +/// turns a level back into a subtree shape (`parents_and_points`, and the +/// trie's `update_bucket_subtrees` via `get_subtree_levels`) is only defined on +/// that range. pub mod fx_hashmap_serde { use super::*; @@ -177,12 +185,20 @@ pub mod fx_hashmap_serde { type Value = FxHashMap; fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("a map of BucketId to u8") + write!( + f, + "a map of BucketId to a subtree level in 1..={MAX_SUBTREE_LEVELS}" + ) } fn visit_map>(self, mut access: A) -> Result { let mut map: FxHashMap = FxHashMap::default(); while let Some((k, v)) = access.next_entry::()? { + if !(1..=MAX_SUBTREE_LEVELS as u8).contains(&v) { + return Err(A::Error::custom(format!( + "level {v} for BucketId {k} is outside 1..={MAX_SUBTREE_LEVELS}" + ))); + } if map.insert(k, v).is_some() { return Err(A::Error::custom("duplicate BucketId in levels")); } @@ -1515,8 +1531,12 @@ mod tests { /// Serializing then deserializing must yield an equivalent map. #[test] fn round_trip_preserves_entries() { - let original = - levels_wrapper([(0u32, 0u8), (42, 3), (1_000_000, 7), (BucketId::MAX, 255)]); + let original = levels_wrapper([ + (0u32, 1u8), + (42, 3), + (1_000_000, 4), + (BucketId::MAX, MAX_SUBTREE_LEVELS as u8), + ]); let bytes = bincode::serde::encode_to_vec(&original, bincode::config::legacy()).unwrap(); @@ -1586,6 +1606,47 @@ mod tests { bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()); assert!(result.is_err(), "duplicate BucketId must be rejected"); } + + /// Bincode (legacy config) bytes of a one-entry `levels` map, built by + /// hand so a test can present a level no serializer would emit. + fn single_entry_bytes(bucket_id: BucketId, level: u8) -> Vec { + let mut bytes = Vec::new(); + bytes.extend_from_slice(&1u64.to_le_bytes()); + bytes.extend_from_slice(&bucket_id.to_le_bytes()); + bytes.push(level); + bytes + } + + #[test] + fn rejects_zero_level() { + let bytes = single_entry_bytes(7, 0); + let result: Result<(LevelsWrapper, _), _> = + bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()); + assert!(result.is_err(), "level 0 must be rejected"); + } + + #[test] + fn rejects_level_above_max() { + let bytes = single_entry_bytes(7, MAX_SUBTREE_LEVELS as u8 + 1); + let result: Result<(LevelsWrapper, _), _> = + bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()); + assert!( + result.is_err(), + "level above MAX_SUBTREE_LEVELS must be rejected" + ); + } + + /// Both ends of the valid range decode; only values outside it are refused. + #[test] + fn accepts_boundary_levels() { + for level in [1u8, MAX_SUBTREE_LEVELS as u8] { + let bytes = single_entry_bytes(7, level); + let (decoded, _): (LevelsWrapper, _) = + bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()) + .unwrap_or_else(|e| panic!("level {level} must decode: {e}")); + assert_eq!(decoded, levels_wrapper([(7, level)])); + } + } } /// Tests create_leaf_node_queries with missing key-value data. From 0ab6a66db326cf8df870e82e216b38b685b486fa Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 15:14:33 +0800 Subject: [PATCH 06/17] fix(types): bound SaltValue's declared lengths on decode `SaltValue` derived `Deserialize`, so any 94-byte payload decoded, including one whose `key_len + value_len` exceeds the 92 bytes that follow the two length prefixes. `key()`, `value()` and `data_len()` trust the prefixes and slice `data` with them, so a witness or proof from an untrusted source could make a consumer panic on an out-of-range slice rather than reject the value. Hand-write `Deserialize` around the same one-field struct shape the derive produced (bincode bytes and self-describing encodings are unchanged) and validate through a new `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>` that refuses `2 + key_len + value_len > MAX_SALT_VALUE_BYTES`. The zero padding past `data_len()` is not checked here; it never enters `kv_hash`, which hashes only the trimmed key and value bytes. Also correct the `MAX_SALT_VALUE_BYTES` doc: the 40- versus 72-byte account value is selected by whether the code hash is the empty-code hash, not by EOA versus contract; an EIP-7702-delegated EOA carries its delegation designator's code hash and takes the 72-byte form. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/types.rs | 92 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 90 insertions(+), 2 deletions(-) diff --git a/salt/src/types.rs b/salt/src/types.rs index d6c833e4..20396a84 100644 --- a/salt/src/types.rs +++ b/salt/src/types.rs @@ -251,7 +251,9 @@ impl From for SaltKey { /// There are 3 types of [`SaltValue`]: `Account`, `Storage`, and `BucketMeta`. /// /// For `Account`, the key length is 20 bytes, and the value length is either 40 -/// (for EOA's) or 72 bytes (for smart contracts). So, encoding an `Account` requires: +/// (when the account's code hash is the empty-code hash) or 72 bytes (any other +/// code hash: a contract, or an EIP-7702-delegated EOA, whose code hash covers +/// its delegation designator). So, encoding an `Account` requires: /// `key_len`(1) + `value_len`(1) + `key`(20) + `value`(40 or 72) = 62 or 94 bytes. /// /// For `Storage`, the key length is 52 bytes, and the value length is 32 bytes. @@ -270,7 +272,13 @@ pub const MAX_SALT_VALUE_BYTES: usize = 94; /// /// Format: `key_len` (1 byte) | `value_len` (1 byte) | `key` | `value` /// Supports Account, Storage, and BucketMeta types. -#[derive(Clone, Debug, Deref, DerefMut, PartialEq, Eq, Serialize, Deserialize)] +/// +/// The encoded bytes are followed by zero padding out to `MAX_SALT_VALUE_BYTES`, so +/// `2 + key_len + value_len` never exceeds `MAX_SALT_VALUE_BYTES`. Deserialization +/// enforces that bound (through `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>`) so that a +/// decoded value can be sliced by [`SaltValue::key`] and [`SaltValue::value`] +/// without running past the buffer. +#[derive(Clone, Debug, Deref, DerefMut, PartialEq, Eq, Serialize)] pub struct SaltValue { /// Fixed-size array accommodating the largest possible encoded data (94 bytes). #[deref] @@ -279,6 +287,41 @@ pub struct SaltValue { pub data: [u8; MAX_SALT_VALUE_BYTES], } +impl<'de> Deserialize<'de> for SaltValue { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + // Same wire shape as the derived impl (a `SaltValue` struct with a single + // `data` field), so serialized bytes are unchanged; only the bound on the + // declared lengths is added. + #[derive(Deserialize)] + #[serde(rename = "SaltValue")] + struct Unchecked { + #[serde(with = "serde_arrays")] + data: [u8; MAX_SALT_VALUE_BYTES], + } + + let Unchecked { data } = Unchecked::deserialize(deserializer)?; + SaltValue::try_from(data).map_err(serde::de::Error::custom) + } +} + +impl TryFrom<[u8; MAX_SALT_VALUE_BYTES]> for SaltValue { + type Error = SaltError; + + /// Wrap an already-encoded buffer, rejecting one whose declared lengths overrun it. + fn try_from(data: [u8; MAX_SALT_VALUE_BYTES]) -> Result { + let declared = 2 + data[0] as usize + data[1] as usize; + if declared > MAX_SALT_VALUE_BYTES { + return Err(SaltError::InvalidFormat { + message: "SaltValue key_len + value_len overruns MAX_SALT_VALUE_BYTES", + }); + } + Ok(Self { data }) + } +} + impl SaltValue { /// Create a new encoded value from separate key and value byte slices. /// @@ -678,6 +721,51 @@ mod tests { assert!(metadata.value().is_empty()); } + /// Tests that SaltValue deserialization keeps the derived wire shape (94 raw + /// bytes under bincode, a one-field struct under a self-describing format) and + /// rejects a payload whose declared lengths overrun the buffer. + #[test] + fn salt_value_deserialize_bounds_declared_lengths() { + let value = SaltValue::new(&[0x11; 20], &[0x22; 72]); + assert_eq!(value.data_len(), MAX_SALT_VALUE_BYTES); + + let bytes = bincode::serde::encode_to_vec(&value, bincode::config::legacy()).unwrap(); + assert_eq!(bytes, value.data); + let (decoded, _): (SaltValue, _) = + bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()).unwrap(); + assert_eq!(decoded, value); + + let json = serde_json::to_string(&value).unwrap(); + let decoded: SaltValue = serde_json::from_str(&json).unwrap(); + assert_eq!(decoded, value); + + // One byte past the largest legal layout (20 + 72) is rejected... + let mut overrun = value.data; + overrun[1] = 73; + assert!(SaltValue::try_from(overrun).is_err()); + let result: Result<(SaltValue, _), _> = + bincode::serde::decode_from_slice(&overrun, bincode::config::legacy()); + assert!( + result.is_err(), + "key_len + value_len overrun must be rejected" + ); + + // ...and so is a key_len that overruns on its own. + let mut overrun = [0u8; MAX_SALT_VALUE_BYTES]; + overrun[0] = u8::MAX; + let result: Result<(SaltValue, _), _> = + bincode::serde::decode_from_slice(&overrun, bincode::config::legacy()); + assert!(result.is_err(), "key_len overrun must be rejected"); + + // An all-zero buffer (key_len = value_len = 0) is still a valid encoding. + let (decoded, _): (SaltValue, _) = bincode::serde::decode_from_slice( + &[0u8; MAX_SALT_VALUE_BYTES], + bincode::config::legacy(), + ) + .unwrap(); + assert_eq!(decoded, SaltValue::new(&[], &[])); + } + /// Tests conversion between BucketMeta and SaltValue. For metadata, the key /// contains the 12-byte serialized BucketMeta and the value is empty. Verifies /// bidirectional conversion preserves nonce and capacity correctly. From b331179f1f6bf3af406f335a12aec352d4103368 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 15:14:33 +0800 Subject: [PATCH 07/17] docs(state): describe what a skipped load-factor resize costs The comment on the count-unavailable branch of `shi_upsert` said the witness "may omit" the usage count as a size optimization, and that skipping the resize "can only delay" it as a "temporary deviation" the next honest insertion self-heals. Neither matches the protocol. A witness builder includes every slot of any bucket the block's insertions resize, so the count is absent exactly when no resize is due. And a resize a sequencer skips is not corrected on the delta path at all: canonicalization rebuilds only buckets actually resized in the block, and delta consumers reproduce the transmitted layout verbatim, so only stateless validation against a complete witness from a builder independent of the sequencer exposes it. The exhaustion-case resize and the next counted insertion remain as structural backstops, and the comment now says so in those terms. Also fix the seed note in `hasher.rs`: the four seeds are the low 128 bits of keccak256("Make Ethereum Great Again") read as big-endian 32-bit words, not "the lower 32 bytes" of a digest that is 32 bytes long. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/state/hasher.rs | 4 ++- salt/src/state/state.rs | 55 ++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/salt/src/state/hasher.rs b/salt/src/state/hasher.rs index ca28a700..d268b581 100644 --- a/salt/src/state/hasher.rs +++ b/salt/src/state/hasher.rs @@ -11,7 +11,9 @@ use crate::constant::NUM_META_BUCKETS; use crate::types::BucketId; use core::hash::{BuildHasher, Hasher}; -/// Fixed seeds derived from the lower 32 bytes of keccak256("Make Ethereum Great Again"). +/// Fixed seeds: the low 128 bits of keccak256("Make Ethereum Great Again") +/// (`0xfd3d34b57e26ebb766fefcc2225e73fc921321f42ccb667e60d68842077ada9d`), +/// read as four big-endian 32-bit words. const HASHER_SEEDS: [u64; 4] = [0x921321f4, 0x2ccb667e, 0x60d68842, 0x077ada9d]; /// Computes a deterministic 64-bit hash of the input bytes. diff --git a/salt/src/state/state.rs b/salt/src/state/state.rs index d8be06fe..c0042ed0 100644 --- a/salt/src/state/state.rs +++ b/salt/src/state/state.rs @@ -553,34 +553,39 @@ impl<'a, Store: StateReader> EphemeralSaltState<'a, Store> { } else { // Bucket usage count is unavailable (metadata.used is None). // - // This can only occur during stateless validation when replaying blocks - // with execution witnesses. The witness may omit the bucket usage count - // to optimize witness size. + // This only occurs during stateless validation, where a bucket's + // count is known only if the witness covers every slot of it. The + // load-factor check is then skipped: neither approximated from the + // slots in view nor treated as an error, leaving the exhaustion + // case below as the only resize trigger. // - // ## Witness Size Trade-off - // Including the usage count would require revealing ALL slots in the - // bucket (to prove the count is correct), significantly increasing - // witness size for every insertion operation. + // ## Why skipping is correct against a complete witness + // A witness builder includes every slot of any bucket the block's + // insertions resize, so the count is available exactly where a + // resize is due and this replay reaches the same decision the + // builder's replay did. An absent count means the builder's replay + // resized nothing here; witnessing the whole bucket for every other + // insertion would only inflate the witness. // - // ## Security Model - // We accept this optimization because: - // 1. Omitting usage count cannot create invalid key-value pairs - // 2. It can only delay bucket resizing (temporary deviation from the - // canonical state) - // 3. The worst case: a malicious sequencer causes the bucket to exceed - // its ideal load factor, degrading performance but not correctness + // ## What a skipped resize costs + // Skipping cannot misplace an entry or admit a value that does not + // belong: every placement still follows the probe sequence, and the + // committed root still authenticates the bucket's true contents. + // What it breaks is the layout bound. A sequencer that skips a due + // resize commits the bucket at a capacity the load factor would have + // rejected, and nothing on the delta path corrects it: + // canonicalization rebuilds only buckets actually resized in the + // block, and nodes applying the block's deltas reproduce the + // transmitted layout verbatim without re-running SHI logic. Only + // stateless validation exposes it, and only against a complete + // witness from a builder independent of the sequencer: a witness + // that leaves the bucket's remaining slots uncovered makes the + // validator skip the same check and accept the over-loaded bucket. // - // ## Self-Healing Mechanism - // When a legitimate sequencer performs the next insertion, it will: - // 1. Compute the actual usage count from the full state - // 2. Trigger resize if needed (restoring optimal bucket structure) - // 3. Continue normal operations with proper load factor tracking - // Even a malicious sequencer will be forced to resize the bucket when - // no empty slot can be found for insertion because it has no choice - // but to reveal all slots at this point. - // - // This approach prioritizes witness compactness while maintaining - // eventual consistency of bucket structure. + // The backstops are structural rather than checked: an insertion + // into a completely full bucket forces the exhaustion-case resize + // below, and any later insertion applied with the count available + // triggers the postponed resize at that point. } return Ok(()); } From 881118878c172b6c59a53ff7d997187b7483f7aa Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 17:07:14 +0800 Subject: [PATCH 08/17] fix(types): bound BucketMeta capacity on decode `BucketMeta::try_from` accepted any u64 as a capacity. A decoded capacity of 0 runs `shi_upsert`'s probe loop zero times, so the exhaustion path computes a new capacity of `compute_resize_capacity(0, 0) = 0`, passes the `<= MAX_BUCKET_SIZE` assert, rehashes to 0 and recurses into `shi_upsert` without bound; a capacity above `MAX_BUCKET_SIZE` drives `subtree_root_level` below level 0 and the trie indexes `STARTING_NODE_ID` out of range. Both enter through the same decoder, which is the entry point for witness-supplied metadata. Reject `capacity == 0` and `capacity > MAX_BUCKET_SIZE` there. This is the range the trie can represent, not the protocol's capacity domain: the protocol only produces `MIN_BUCKET_SIZE` doubled some number of times, but tests exercise the SHI logic at capacities such as 3, 5, 8 and 4, so that stricter check is left for a separate change. The `# Panics` note on `subtree_root_level` no longer says a stored capacity is validated nowhere. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/trie/node_utils.rs | 5 ++- salt/src/types.rs | 83 ++++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index 4e9deb04..cfbafe23 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -397,8 +397,9 @@ pub(crate) fn subtree_leaf_start_key(node_id: &NodeId) -> SaltKey { /// Panics in debug builds (and wraps `level` in release builds) if `capacity` exceeds /// `MAX_BUCKET_SIZE`, since no subtree level can hold it. Callers must bound capacity /// themselves. `EphemeralSaltState::shi_rehash` asserts it for the capacity it is -/// asked to resize *to*, but a capacity read back from a stored `BucketMeta` is not -/// validated anywhere, so callers that source one from a `StateReader` are on their own. +/// asked to resize *to*, and `BucketMeta::try_from` rejects a decoded capacity outside +/// `1..=MAX_BUCKET_SIZE`, so a capacity read back through a `StateReader` is bounded; +/// a `BucketMeta` built in-process with a larger `capacity` field is not. pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { // Start from the deepest possible level let mut level = MAX_SUBTREE_LEVELS - 1; diff --git a/salt/src/types.rs b/salt/src/types.rs index 20396a84..a3d7760b 100644 --- a/salt/src/types.rs +++ b/salt/src/types.rs @@ -10,8 +10,8 @@ use core::ops::RangeInclusive; use crate::constant::{ - BUCKET_SLOT_BITS, BUCKET_SLOT_ID_MASK, MIN_BUCKET_SIZE, MIN_BUCKET_SIZE_BITS, NUM_BUCKETS, - NUM_META_BUCKETS, TRIE_WIDTH, + BUCKET_SLOT_BITS, BUCKET_SLOT_ID_MASK, MAX_BUCKET_SIZE, MIN_BUCKET_SIZE, MIN_BUCKET_SIZE_BITS, + NUM_BUCKETS, NUM_META_BUCKETS, TRIE_WIDTH, }; use derive_more::{Deref, DerefMut}; @@ -100,17 +100,36 @@ impl TryFrom<&[u8]> for BucketMeta { message: "BucketMeta requires exactly 12 bytes", }); } + let nonce = + u32::from_le_bytes( + bytes[0..4] + .try_into() + .map_err(|_| SaltError::InvalidFormat { + message: "Failed to parse nonce from bytes", + })?, + ); + let capacity = + u64::from_le_bytes( + bytes[4..12] + .try_into() + .map_err(|_| SaltError::InvalidFormat { + message: "Failed to parse capacity from bytes", + })?, + ); + // A decoded capacity feeds `probe` (which divides by it) and + // `subtree_root_level` (which has no level below 0), so bound it here + // rather than at every consumer. This is only the range the trie can + // represent: capacities the protocol produces are `MIN_BUCKET_SIZE` + // doubled some number of times, which is not enforced at this boundary + // because tests exercise the SHI logic at smaller capacities. + if capacity == 0 || capacity > MAX_BUCKET_SIZE { + return Err(SaltError::InvalidFormat { + message: "BucketMeta capacity must be in 1..=MAX_BUCKET_SIZE", + }); + } Ok(Self { - nonce: u32::from_le_bytes(bytes[0..4].try_into().map_err(|_| { - SaltError::InvalidFormat { - message: "Failed to parse nonce from bytes", - } - })?), - capacity: u64::from_le_bytes(bytes[4..12].try_into().map_err(|_| { - SaltError::InvalidFormat { - message: "Failed to parse capacity from bytes", - } - })?), + nonce, + capacity, used: None, }) } @@ -654,8 +673,8 @@ mod tests { fn bucket_meta_serialization() { let meta = BucketMeta { nonce: 0x12345678, - capacity: 0x123456789ABCDEF0, - used: Some(100), // This value is not serialized + capacity: 0x123456789A, // arbitrary, within 1..=MAX_BUCKET_SIZE + used: Some(100), // This value is not serialized }; let bytes = meta.to_bytes(); let recovered = BucketMeta::try_from(&bytes[..]).unwrap(); @@ -678,6 +697,38 @@ mod tests { assert_eq!(bincode_recovered, recovered); } + /// Tests that decoding bounds the capacity to what the trie can represent: + /// 0 would divide by zero in `probe` and recurse without bound in `shi_upsert`, + /// and anything above `MAX_BUCKET_SIZE` has no subtree level to hold it. + #[test] + fn bucket_meta_decode_bounds_capacity() { + let encode = |capacity: u64| { + BucketMeta { + nonce: 7, + capacity, + used: None, + } + .to_bytes() + }; + + for capacity in [1, MIN_BUCKET_SIZE as u64, MAX_BUCKET_SIZE] { + let meta = BucketMeta::try_from(&encode(capacity)[..]) + .unwrap_or_else(|e| panic!("capacity {capacity} must decode: {e}")); + assert_eq!(meta.capacity, capacity); + } + + for capacity in [0, MAX_BUCKET_SIZE + 1, u64::MAX] { + assert!( + BucketMeta::try_from(&encode(capacity)[..]).is_err(), + "capacity {capacity} must be rejected" + ); + } + + // The same bound applies when the metadata arrives inside a SaltValue. + let value = SaltValue::new(&encode(0), &[]); + assert!(BucketMeta::try_from(value).is_err()); + } + /// Tests BucketMeta default constructor. Verifies that default values match /// expected initialization: nonce=0, capacity=MIN_BUCKET_SIZE (256), used=None. #[test] @@ -877,8 +928,8 @@ mod tests { let long_bytes = [1u8; 20]; assert!(BucketMeta::try_from(&long_bytes[..]).is_err()); - // Test exactly 12 bytes (required) - should succeed - let valid_bytes = [0u8; 12]; + // Test exactly 12 bytes (required) with an in-range capacity - should succeed + let valid_bytes = BucketMeta::default().to_bytes(); assert!(BucketMeta::try_from(&valid_bytes[..]).is_ok()); } From adb61d2c69ed03b81210de0a24bf1d5a2e3b729c Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 17:10:04 +0800 Subject: [PATCH 09/17] fix(trie): keep subtree-local node arithmetic in u64 `vc_position_in_parent` and `get_parent_node` cast a subtree-local node number to `usize` before subtracting the level's starting id. Every local number fit in 32 bits while capacity topped out at 2^39, but at `MAX_BUCKET_SIZE` the deepest one is 4,311,810,304, past `u32::MAX`, so on a 32-bit target such as wasm32 (which `state::ahash::convert` names as a deployment target) the cast truncates and both functions return the wrong node. Do the arithmetic in u64 and cast only the sub-256 result. Adds a test pinning both functions at that deepest node. On 64-bit hosts it passed before as well, so it documents the boundary rather than reproducing the truncation. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/trie/node_utils.rs | 44 ++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index cfbafe23..043bcbe2 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -218,9 +218,11 @@ pub(crate) fn vc_position_in_parent(node_id: &NodeId) -> usize { let local_number = get_local_number(*node_id); let trie_level = get_bfs_level(local_number); - // Calculate relative position from the start of this level - let relative_pos = local_number as usize - STARTING_NODE_ID[trie_level]; - relative_pos % TRIE_WIDTH + // Calculate relative position from the start of this level. Stay in u64: at + // `MAX_BUCKET_SIZE` a subtree-local number exceeds u32, so a 32-bit `usize` + // cannot hold it. + let relative_pos = local_number - STARTING_NODE_ID[trie_level] as u64; + (relative_pos % TRIE_WIDTH as u64) as usize } /// Computes the NodeId of a specific child given its parent and child index. @@ -280,8 +282,10 @@ pub(crate) fn get_parent_node(node_id: &NodeId) -> NodeId { // Determine which level this node is on let level = get_bfs_level(local_node_id); - // Calculate relative position from the start of current level - let relative_position = local_node_id as usize - STARTING_NODE_ID[level]; + // Calculate relative position from the start of current level. Stay in u64: + // at `MAX_BUCKET_SIZE` a subtree-local number exceeds u32, so a 32-bit + // `usize` cannot hold it. + let relative_position = local_node_id - STARTING_NODE_ID[level] as u64; // Divide by 256 (right-shift by 8) to get parent's relative position // Each parent has 256 children, so child positions 0-255 → parent 0, @@ -290,7 +294,7 @@ pub(crate) fn get_parent_node(node_id: &NodeId) -> NodeId { // Add parent level's starting position to get absolute parent ID // and preserve the bucket ID for subtree nodes - bucket_id + (parent_relative_position + STARTING_NODE_ID[level - 1]) as NodeId + bucket_id + parent_relative_position + STARTING_NODE_ID[level - 1] as NodeId } /// Maps a bucket ID to its subtree root node in the main trie. @@ -419,6 +423,34 @@ mod tests { use super::*; use crate::constant::MAX_BUCKET_SIZE; + /// The deepest subtree-local node number at `MAX_BUCKET_SIZE` exceeds u32, so + /// the level-relative arithmetic has to stay in u64 rather than `usize` to be + /// right on 32-bit targets. Pins the values that arithmetic produces there. + #[test] + fn parent_and_position_at_max_capacity_depth() { + use crate::constant::{ + BUCKET_SLOT_BITS, MAX_SUBTREE_LEVELS, MIN_BUCKET_SIZE, NUM_META_BUCKETS, + STARTING_NODE_ID, TRIE_WIDTH, + }; + use crate::types::{get_local_number, NodeId}; + + let segments = MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64; + let deepest_local = STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + segments - 1; + assert!(deepest_local > u32::MAX as u64); + + let bucket_id = NUM_META_BUCKETS as u64; + let node: NodeId = (bucket_id << BUCKET_SLOT_BITS) | deepest_local; + + assert_eq!(vc_position_in_parent(&node), TRIE_WIDTH - 1); + + let parent = get_parent_node(&node); + assert_eq!(parent >> BUCKET_SLOT_BITS, bucket_id); + assert_eq!( + get_local_number(parent), + STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 2] as u64 + segments / TRIE_WIDTH as u64 - 1 + ); + } + /// Tests the vc_position_in_parent function for various node types and positions. /// /// Verifies that the function correctly calculates vector commitment positions (0-255) From cdad45b3d35147a9a89ee75988fd9c0f05fb428f Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 17:10:05 +0800 Subject: [PATCH 10/17] refactor(types): validate SaltValue through a field-level deserializer The previous commit hand-wrote `Deserialize` for `SaltValue` around a private mirror struct to keep the derived wire shape while adding the length bound. A field-level `deserialize_with` does the same with the derive kept: the derive still emits the one-field struct, `serde_arrays` still decodes the array, and `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>` applies the bound. `TryFrom` now reads the declared length through `data_len()`, the quantity `key()` and `value()` slice by, instead of recomputing it. Also say in the struct doc that the zero padding past `data_len()` is not checked on decode; the earlier wording could be read as promising that. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/types.rs | 49 ++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 26 deletions(-) diff --git a/salt/src/types.rs b/salt/src/types.rs index a3d7760b..095fa1c4 100644 --- a/salt/src/types.rs +++ b/salt/src/types.rs @@ -294,36 +294,33 @@ pub const MAX_SALT_VALUE_BYTES: usize = 94; /// /// The encoded bytes are followed by zero padding out to `MAX_SALT_VALUE_BYTES`, so /// `2 + key_len + value_len` never exceeds `MAX_SALT_VALUE_BYTES`. Deserialization -/// enforces that bound (through `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>`) so that a -/// decoded value can be sliced by [`SaltValue::key`] and [`SaltValue::value`] -/// without running past the buffer. -#[derive(Clone, Debug, Deref, DerefMut, PartialEq, Eq, Serialize)] +/// enforces that length bound (see `deserialize_checked_data`) so that a decoded +/// value can be sliced by [`SaltValue::key`] and [`SaltValue::value`] without running +/// past the buffer. The padding itself is not checked on decode: it never enters the +/// slot hash, which covers only the trimmed key and value bytes. +#[derive(Clone, Debug, Deref, DerefMut, PartialEq, Eq, Serialize, Deserialize)] pub struct SaltValue { /// Fixed-size array accommodating the largest possible encoded data (94 bytes). #[deref] #[deref_mut] - #[serde(with = "serde_arrays")] + #[serde( + serialize_with = "serde_arrays::serialize", + deserialize_with = "deserialize_checked_data" + )] pub data: [u8; MAX_SALT_VALUE_BYTES], } -impl<'de> Deserialize<'de> for SaltValue { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - // Same wire shape as the derived impl (a `SaltValue` struct with a single - // `data` field), so serialized bytes are unchanged; only the bound on the - // declared lengths is added. - #[derive(Deserialize)] - #[serde(rename = "SaltValue")] - struct Unchecked { - #[serde(with = "serde_arrays")] - data: [u8; MAX_SALT_VALUE_BYTES], - } - - let Unchecked { data } = Unchecked::deserialize(deserializer)?; - SaltValue::try_from(data).map_err(serde::de::Error::custom) - } +/// Deserializes [`SaltValue::data`] through `serde_arrays`, the same wire shape a +/// plain `#[serde(with = "serde_arrays")]` field has, and applies the +/// `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>` bound on the declared lengths. +fn deserialize_checked_data<'de, D>(deserializer: D) -> Result<[u8; MAX_SALT_VALUE_BYTES], D::Error> +where + D: serde::Deserializer<'de>, +{ + let data: [u8; MAX_SALT_VALUE_BYTES] = serde_arrays::deserialize(deserializer)?; + SaltValue::try_from(data) + .map(|value| value.data) + .map_err(serde::de::Error::custom) } impl TryFrom<[u8; MAX_SALT_VALUE_BYTES]> for SaltValue { @@ -331,13 +328,13 @@ impl TryFrom<[u8; MAX_SALT_VALUE_BYTES]> for SaltValue { /// Wrap an already-encoded buffer, rejecting one whose declared lengths overrun it. fn try_from(data: [u8; MAX_SALT_VALUE_BYTES]) -> Result { - let declared = 2 + data[0] as usize + data[1] as usize; - if declared > MAX_SALT_VALUE_BYTES { + let value = Self { data }; + if value.data_len() > MAX_SALT_VALUE_BYTES { return Err(SaltError::InvalidFormat { message: "SaltValue key_len + value_len overruns MAX_SALT_VALUE_BYTES", }); } - Ok(Self { data }) + Ok(value) } } From 088def6d824addc66219364aa34d14fb19728567 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 17:10:05 +0800 Subject: [PATCH 11/17] docs(salt): qualify the resize backstop and the README growth example The `shi_upsert` comment called the exhaustion-case resize a structural backstop without saying where it stops: at `MAX_BUCKET_SIZE` there is no larger capacity to resize to, `compute_resize_capacity` still doubles, and `shi_rehash` asserts. Say so. The README's growth paragraph states that every capacity is a power of two and then draws a 768-slot bucket tree; mark the diagram as an illustration of the shape rather than a capacity the protocol produces. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- README.md | 2 +- salt/src/state/state.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c450beba..bd8f33ed 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ The main trie's internal nodes also use homomorphic commitments. After bucket co While each step up the trie costs one ECMul, updates from multiple distinct child nodes are batched into a single update for their common parent. This optimization is extremely effective because the trie's width shrinks dramatically at higher levels, consolidating many changes at the leaf level into a small number of updates near the root. For example, updating 200,000 random keys in SALT requires a total of approximately 460,000 ECMul operations, or an amortized cost of about **2.3 ECMuls** per key. ### Bucket Growth -While the main SALT tree is static, the buckets are not. A bucket is initialized with 256 slots. When it fills up, its capacity is doubled, so every capacity is a power of two from 256 up to 2^40 slots (the most a 5-level bucket tree can address). If a bucket grows beyond 256 slots, it is partitioned into 256-slot segments. A new complete 256-ary **bucket tree** is built on top of these segments, and the root of this new tree becomes the bucket's new commitment (also the new leaf in the main trie). The diagram below shows a bucket tree of 768 slots. +While the main SALT tree is static, the buckets are not. A bucket is initialized with 256 slots. When it fills up, its capacity is doubled, so every capacity is a power of two from 256 up to 2^40 slots (the most a 5-level bucket tree can address). If a bucket grows beyond 256 slots, it is partitioned into 256-slot segments. A new complete 256-ary **bucket tree** is built on top of these segments, and the root of this new tree becomes the bucket's new commitment (also the new leaf in the main trie). The diagram below shows a bucket tree of 768 slots (three segments) purely to illustrate the shape; a bucket the protocol has grown holds 512, 1024, ... slots. ``` (In Main SALT Trie) diff --git a/salt/src/state/state.rs b/salt/src/state/state.rs index c0042ed0..63b9d83f 100644 --- a/salt/src/state/state.rs +++ b/salt/src/state/state.rs @@ -582,10 +582,12 @@ impl<'a, Store: StateReader> EphemeralSaltState<'a, Store> { // that leaves the bucket's remaining slots uncovered makes the // validator skip the same check and accept the over-loaded bucket. // - // The backstops are structural rather than checked: an insertion - // into a completely full bucket forces the exhaustion-case resize - // below, and any later insertion applied with the count available - // triggers the postponed resize at that point. + // The backstops are structural rather than checked, and hold + // below the capacity ceiling: an insertion into a completely full + // bucket forces the exhaustion-case resize below, and any later + // insertion applied with the count available triggers the + // postponed resize at that point. At `MAX_BUCKET_SIZE` there is no + // larger capacity to resize to, and `shi_rehash` asserts instead. } return Ok(()); } From e44090c248ad3216552e9ac4a145ba1fad090bb4 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 17:10:05 +0800 Subject: [PATCH 12/17] test(proof): keep levels fixtures inside the valid range `serialization_is_order_independent` still carried a level of 6, which the deserializer now rejects; the test only serializes, so it passed, but the fixture contradicted the range the module documents. The `single_entry_bytes` helper's comment also claimed a level "no serializer would emit", which is false: `fx_hashmap_serde::serialize` writes any u8 unchanged, and it is the prover that never produces such a level. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- salt/src/proof/prover.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/salt/src/proof/prover.rs b/salt/src/proof/prover.rs index 0dbaed26..269d3b03 100644 --- a/salt/src/proof/prover.rs +++ b/salt/src/proof/prover.rs @@ -1570,7 +1570,7 @@ mod tests { (4, 3), (1_000_000, 4), (5, 5), - (BucketId::MAX, 6), + (BucketId::MAX, 1), ]; let forward = levels_wrapper(entries); @@ -1608,7 +1608,8 @@ mod tests { } /// Bincode (legacy config) bytes of a one-entry `levels` map, built by - /// hand so a test can present a level no serializer would emit. + /// hand so a test can present a level no prover produces (the serializer + /// itself writes any u8 unchanged; only deserialization validates). fn single_entry_bytes(bucket_id: BucketId, level: u8) -> Vec { let mut bytes = Vec::new(); bytes.extend_from_slice(&1u64.to_le_bytes()); From a1ddd441e3fbe85a46dcaf45937da11318b1158b Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 17:17:41 +0800 Subject: [PATCH 13/17] chore(mutants): re-pin line-scoped suppressions after line drift The `suppression hygiene` job failed on this branch with seven stale line-pinned entries in `mutants/suppressions.toml`: the comment rewrite at the top of `hasher.rs` moved every line below it down by two, and the doc additions in `constant.rs` moved `DEFAULT_COMMITMENT_AT_LEVEL` down by 21. Each entry is re-pinned to the same site it was reviewed for, identified by content rather than offset: the level-0 `STARTING_NODE_ID[0] + 1` boundary (221 -> 242), the `key_len + 4 <= STACK_BUFFER_SIZE` branch of `hash_with_nonce` (71 -> 73), and the `bucket_id` sites (51 -> 53, 58 -> 60). No justification changes. Verified locally with a `cargo mutants --list --package salt` universe and `scripts/mutation_gate.py orphans`: 0/38 stale. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb --- mutants/suppressions.toml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/mutants/suppressions.toml b/mutants/suppressions.toml index e64c3b58..267308a6 100644 --- a/mutants/suppressions.toml +++ b/mutants/suppressions.toml @@ -53,7 +53,7 @@ reviewer = "krabat/feat/mutation-testing review 2026-07-05" kind = "line" category = "equivalent" file = "salt/src/constant.rs" -line = 221 +line = 242 mutant = "replace + with * in default_commitment" justification = "Turns the level-0 boundary from STARTING_NODE_ID[0]+1 into STARTING_NODE_ID[0]*1 = 0, but the level-0 tuple carries identical left/right commitments, so the selected value is unchanged for every node id. Pinned to the level-0 site; the level 1-3 boundary additions are killed by test_default_commitment_boundary_selection." reviewer = "full-run triage 2026-07-08 (issue #144)" @@ -151,7 +151,7 @@ reviewer = "full-run triage 2026-07-08 (issue #144)" kind = "line" category = "equivalent" file = "salt/src/state/hasher.rs" -line = 71 +line = 73 mutant = "replace + with * in hash_with_nonce" justification = "Pinned to the buffer-selection guard (line 71): key_len + 4 <= 64 vs key_len * 4 <= 64 only moves the stack/heap buffer split; both paths hash the identical byte sequence. The `key_len + 4` slice-length sites (lines 74-75) are non-equivalent and are killed by tests (verified by a cargo-mutants run: only line 71 survives), so the pin cannot mask them." reviewer = "full-run triage 2026-07-08 (issue #144)" @@ -200,7 +200,7 @@ reviewer = "full-run triage 2026-07-08 (issue #144)" kind = "line" category = "dead" file = "salt/src/state/hasher.rs" -line = 51 +line = 53 mutant = "replace bucket_id -> BucketId with Default::default()" justification = "The test-bucket-resize cfg variant is not compiled under the canonical mutation feature set. Pinned to that variant's site so the production bucket_id, whose function-replacement mutant is killed by the pinned bucket-id tests, can never be covered by this entry." reviewer = "full-run triage 2026-07-08 (issue #144)" @@ -209,7 +209,7 @@ reviewer = "full-run triage 2026-07-08 (issue #144)" kind = "line" category = "dead" file = "salt/src/state/hasher.rs" -line = 58 +line = 60 mutant = "replace % with + in bucket_id" justification = "test-bucket-resize cfg variant, not compiled under the canonical mutation feature set; line-pinned so the production arithmetic is never covered." reviewer = "full-run triage 2026-07-08 (issue #144)" @@ -218,7 +218,7 @@ reviewer = "full-run triage 2026-07-08 (issue #144)" kind = "line" category = "dead" file = "salt/src/state/hasher.rs" -line = 58 +line = 60 mutant = "replace % with / in bucket_id" justification = "test-bucket-resize cfg variant, not compiled under the canonical mutation feature set; line-pinned so the production arithmetic is never covered." reviewer = "full-run triage 2026-07-08 (issue #144)" @@ -227,7 +227,7 @@ reviewer = "full-run triage 2026-07-08 (issue #144)" kind = "line" category = "dead" file = "salt/src/state/hasher.rs" -line = 58 +line = 60 mutant = "replace + with * in bucket_id" justification = "test-bucket-resize cfg variant, not compiled under the canonical mutation feature set; line-pinned so the production arithmetic is never covered." reviewer = "full-run triage 2026-07-08 (issue #144)" @@ -236,7 +236,7 @@ reviewer = "full-run triage 2026-07-08 (issue #144)" kind = "line" category = "dead" file = "salt/src/state/hasher.rs" -line = 58 +line = 60 mutant = "replace + with - in bucket_id" justification = "test-bucket-resize cfg variant, not compiled under the canonical mutation feature set; line-pinned so the production arithmetic is never covered." reviewer = "full-run triage 2026-07-08 (issue #144)" From a93e2122122e5d48cf8032d4a1284d4843c8ac25 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 18:16:56 +0800 Subject: [PATCH 14/17] fix(proof): subtract child offsets in u64 before narrowing to usize `multi_commitments_to_scalars` computed a child's index within its parent's 256-slot commitment as `absolute_node_id as usize - child_idx as usize`. Both ids carry the bucket in their high bits and, at the top of a level-4 subtree, a local number past u32::MAX: the 256-child range that begins at local node 0xffffff01 ends at 0x100000000. On a 32-bit target each cast drops the high bits first, so that last child reads as 0, the subtraction underflows, and proof generation panics under overflow checks (or wraps to the right answer by accident without them). Subtract in u64 and narrow the sub-256 result, as `vc_position_in_parent` and `get_parent_node` now do. Answers the Codex review thread anchored at `constant.rs:53`. Co-Authored-By: Claude Fable 5.1 --- salt/src/proof/subtrie.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/salt/src/proof/subtrie.rs b/salt/src/proof/subtrie.rs index 608be8d6..2575964e 100644 --- a/salt/src/proof/subtrie.rs +++ b/salt/src/proof/subtrie.rs @@ -120,7 +120,10 @@ where // Replace defaults with actual commitments where they exist for (absolute_node_id, commitment_bytes) in children { - let relative_index = absolute_node_id as usize - child_idx as usize; + // Subtract in u64 before narrowing: a 256-child range near the top of + // a level-4 subtree straddles 2^32, so casting each id to a 32-bit + // `usize` first would underflow. + let relative_index = (absolute_node_id - child_idx) as usize; child_commitments[relative_index] = to_element(commitment_bytes); } From 8a8d39848399cf2184daa19e2d8677e06def5683 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 2 Sep 2026 18:16:56 +0800 Subject: [PATCH 15/17] fix(trie): fail fast when get_parent_node lands on the wrong level The spec-gate's trie operator pack rewrites `STARTING_NODE_ID[level - 1]` to `STARTING_NODE_ID[level]` in `get_parent_node`, which returns a node on the child's own level. Every walk to the root then spins forever, so the mutant was reported as a timeout, which the gate treats as inconclusive rather than killed. It surfaced now because the u64 rewrite put that line into the PR's diff scope; the same expression was there before, untested by the diff gate. Check the returned node's level with a `debug_assert_eq!` (debug builds only, one `get_bfs_level` per call). Under the mutant the first `get_parent_node` call now panics and nextest finishes with a failure instead of hanging, verified by applying the mutation by hand: exit 100 after 7 seconds. Co-Authored-By: Claude Fable 5.1 --- salt/src/trie/node_utils.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index 043bcbe2..b243ce68 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -294,7 +294,13 @@ pub(crate) fn get_parent_node(node_id: &NodeId) -> NodeId { // Add parent level's starting position to get absolute parent ID // and preserve the bucket ID for subtree nodes - bucket_id + parent_relative_position + STARTING_NODE_ID[level - 1] as NodeId + let parent = bucket_id + parent_relative_position + STARTING_NODE_ID[level - 1] as NodeId; + debug_assert_eq!( + get_bfs_level(get_local_number(parent)), + level - 1, + "parent of a level-{level} node must sit one level up" + ); + parent } /// Maps a bucket ID to its subtree root node in the main trie. From ebe10399a269ccfb48b96c68290df1cfbf549047 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Tue, 8 Sep 2026 15:00:07 +0800 Subject: [PATCH 16/17] refactor(salt): pin the capacity ceiling at compile time and drop the review scaffolding - `MAX_BUCKET_SIZE == 1 << BUCKET_SLOT_BITS`, its segment-count derivation, and the deepest-subtree-node-fits-the-slot-field bound are now `const` assertions next to the definition; the three tests that restated them are removed. - `parents_and_points` walks the fixed-depth main trie for exactly `MAIN_TRIE_LEVELS - 1` steps instead of until it reaches node 0, so a corrupt parent id (or the trie pack's `STARTING_NODE_ID[level - 1] -> [level]` mutant) fails fast instead of spinning. The `debug_assert` in `get_parent_node` that stood in for that is removed; the hand-applied mutant now ends the suite in 18 s with exit 100. - `SaltValue` deserialization checks `data_len` inline instead of through a `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>` layer, and `BucketMeta::try_from` drops two error arms the length guard already makes unreachable. - Tests reuse `subtree_leaf_for_key`, `get_child_node`, `From` and a single `levels_bytes` builder instead of restating their arithmetic or byte layout; the three near-identical level-range tests collapse into one. - The ceiling derivation lives in the `MAX_BUCKET_SIZE` doc alone; the other docs link to it, and `subtree_root_level` no longer enumerates its callers. - Re-pin the `default_commitment` suppression after the constant.rs line shift. Co-Authored-By: Claude Fable 5.1 --- mutants/suppressions.toml | 2 +- salt/src/constant.rs | 43 +++++++++++---------- salt/src/proof/prover.rs | 66 ++++++++++++--------------------- salt/src/proof/shape.rs | 14 ++++--- salt/src/state/state.rs | 22 ----------- salt/src/trie/node_utils.rs | 52 +++++++------------------- salt/src/trie/trie.rs | 26 +++---------- salt/src/types.rs | 74 ++++++++++++------------------------- 8 files changed, 97 insertions(+), 202 deletions(-) diff --git a/mutants/suppressions.toml b/mutants/suppressions.toml index 267308a6..79ef15fc 100644 --- a/mutants/suppressions.toml +++ b/mutants/suppressions.toml @@ -53,7 +53,7 @@ reviewer = "krabat/feat/mutation-testing review 2026-07-05" kind = "line" category = "equivalent" file = "salt/src/constant.rs" -line = 242 +line = 261 mutant = "replace + with * in default_commitment" justification = "Turns the level-0 boundary from STARTING_NODE_ID[0]+1 into STARTING_NODE_ID[0]*1 = 0, but the level-0 tuple carries identical left/right commitments, so the selected value is unchanged for every node id. Pinned to the level-0 site; the level 1-3 boundary additions are killed by test_default_commitment_boundary_selection." reviewer = "full-run triage 2026-07-08 (issue #144)" diff --git a/salt/src/constant.rs b/salt/src/constant.rs index c313b974..adbb7e8a 100644 --- a/salt/src/constant.rs +++ b/salt/src/constant.rs @@ -52,6 +52,26 @@ pub const META_BUCKET_SIZE: usize = MIN_BUCKET_SIZE; pub const MAX_BUCKET_SIZE: u64 = 1 << ((MAX_SUBTREE_LEVELS - 1) * TRIE_WIDTH_BITS + MIN_BUCKET_SIZE_BITS); +// `MAX_BUCKET_SIZE` is derived from the subtree shape while `BUCKET_SLOT_BITS` is the +// `SaltKey` layout; tie them together at compile time so neither can drift. +const _: () = { + // The shift form above is the segment count times the segment size. + assert!( + MAX_BUCKET_SIZE + == (TRIE_WIDTH as u64).pow((MAX_SUBTREE_LEVELS - 1) as u32) * MIN_BUCKET_SIZE as u64 + ); + // Every slot index of a maximally expanded bucket fits the slot field, and the + // largest one is exactly `BUCKET_SLOT_ID_MASK`. + assert!(MAX_BUCKET_SIZE == 1 << BUCKET_SLOT_BITS); + // So does that bucket's deepest subtree node, which must not bleed into the + // bucket-id bits of a `NodeId`. + assert!( + STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64 + - 1 + <= BUCKET_SLOT_ID_MASK + ); +}; + // ============================================================================ // Trie Structure Constants // ============================================================================ @@ -77,8 +97,7 @@ pub const MAIN_TRIE_LEVELS: usize = 4; /// - 16,777,217-4,294,967,296 slots: Root at level 1 /// - 4,294,967,297-1,099,511,627,776 slots: Root at level 0, the full 5-level subtree /// -/// The last row is the ceiling: the deepest level holds 256^4 = 2^32 segments, so a -/// bucket tops out at `MAX_BUCKET_SIZE` = 2^32 * 256 = 2^40 slots. +/// The last row ends at [`MAX_BUCKET_SIZE`], whose doc derives that ceiling. /// /// Example for 512-slot bucket (2 segments): /// ```text @@ -309,26 +328,6 @@ mod tests { assert_eq!(STARTING_NODE_ID, [0, 1, 257, 65_793, 16_843_009]); } - /// `MAX_BUCKET_SIZE` must be the slot *count* a full subtree addresses, not the - /// largest slot *index*. Confusing the two caps buckets one doubling short. - #[test] - fn test_max_bucket_size_matches_subtree_capacity() { - // The deepest subtree level has TRIE_WIDTH^(MAX_SUBTREE_LEVELS - 1) segments, - // each holding MIN_BUCKET_SIZE slots. - let segments = (TRIE_WIDTH as u64).pow((MAX_SUBTREE_LEVELS - 1) as u32); - assert_eq!(segments, 1u64 << 32); - assert_eq!(MAX_BUCKET_SIZE, segments * MIN_BUCKET_SIZE as u64); - - // Every slot index of a maximally expanded bucket fits the 40-bit slot field, - // and the largest one is exactly the mask. - assert_eq!(MAX_BUCKET_SIZE, 1u64 << BUCKET_SLOT_BITS); - assert_eq!(MAX_BUCKET_SIZE - 1, BUCKET_SLOT_ID_MASK); - - // Capacities only ever double up from MIN_BUCKET_SIZE, so bounding one by - // BUCKET_SLOT_ID_MASK would have stopped at 2^39 instead of 2^40. - assert_eq!(MAX_BUCKET_SIZE / BUCKET_RESIZE_MULTIPLIER, 1u64 << 39); - } - #[test] fn test_default_commitment_boundary_selection() { assert_eq!(default_commitment(ROOT_NODE_ID), default_commitment(0)); diff --git a/salt/src/proof/prover.rs b/salt/src/proof/prover.rs index 269d3b03..29359902 100644 --- a/salt/src/proof/prover.rs +++ b/salt/src/proof/prover.rs @@ -1593,59 +1593,41 @@ mod tests { assert_eq!(forward_bytes, reverse_bytes); } - #[test] - fn rejects_duplicate_bucket_id() { - let mut bytes = Vec::new(); - bytes.extend_from_slice(&2u64.to_le_bytes()); - bytes.extend_from_slice(&7u32.to_le_bytes()); - bytes.push(1u8); - bytes.extend_from_slice(&7u32.to_le_bytes()); - bytes.push(2u8); - - let result: Result<(LevelsWrapper, _), _> = - bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()); - assert!(result.is_err(), "duplicate BucketId must be rejected"); - } - - /// Bincode (legacy config) bytes of a one-entry `levels` map, built by - /// hand so a test can present a level no prover produces (the serializer - /// itself writes any u8 unchanged; only deserialization validates). - fn single_entry_bytes(bucket_id: BucketId, level: u8) -> Vec { - let mut bytes = Vec::new(); - bytes.extend_from_slice(&1u64.to_le_bytes()); - bytes.extend_from_slice(&bucket_id.to_le_bytes()); - bytes.push(level); + /// Bincode (legacy config) bytes of a `levels` map, built by hand so a test + /// can present entries no prover produces: a duplicate bucket, or a level + /// outside the valid range (the serializer writes any u8 unchanged; only + /// deserialization validates). + fn levels_bytes(entries: &[(BucketId, u8)]) -> Vec { + let mut bytes = (entries.len() as u64).to_le_bytes().to_vec(); + for (bucket_id, level) in entries { + bytes.extend_from_slice(&bucket_id.to_le_bytes()); + bytes.push(*level); + } bytes } - #[test] - fn rejects_zero_level() { - let bytes = single_entry_bytes(7, 0); - let result: Result<(LevelsWrapper, _), _> = - bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()); - assert!(result.is_err(), "level 0 must be rejected"); + fn decode(bytes: &[u8]) -> Result { + bincode::serde::decode_from_slice(bytes, bincode::config::legacy()) + .map(|(decoded, _)| decoded) } #[test] - fn rejects_level_above_max() { - let bytes = single_entry_bytes(7, MAX_SUBTREE_LEVELS as u8 + 1); - let result: Result<(LevelsWrapper, _), _> = - bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()); + fn rejects_duplicate_bucket_id() { assert!( - result.is_err(), - "level above MAX_SUBTREE_LEVELS must be rejected" + decode(&levels_bytes(&[(7, 1), (7, 2)])).is_err(), + "duplicate BucketId must be rejected" ); } - /// Both ends of the valid range decode; only values outside it are refused. + /// Levels outside `1..=MAX_SUBTREE_LEVELS` are refused; both ends of the + /// range decode in `round_trip_preserves_entries`. #[test] - fn accepts_boundary_levels() { - for level in [1u8, MAX_SUBTREE_LEVELS as u8] { - let bytes = single_entry_bytes(7, level); - let (decoded, _): (LevelsWrapper, _) = - bincode::serde::decode_from_slice(&bytes, bincode::config::legacy()) - .unwrap_or_else(|e| panic!("level {level} must decode: {e}")); - assert_eq!(decoded, levels_wrapper([(7, level)])); + fn rejects_out_of_range_levels() { + for level in [0, MAX_SUBTREE_LEVELS as u8 + 1] { + assert!( + decode(&levels_bytes(&[(7, level)])).is_err(), + "level {level} must be rejected" + ); } } } diff --git a/salt/src/proof/shape.rs b/salt/src/proof/shape.rs index e38e051b..0cfb4119 100644 --- a/salt/src/proof/shape.rs +++ b/salt/src/proof/shape.rs @@ -19,7 +19,9 @@ use rustc_hash::FxBuildHasher; type FxHashMap = HashMap; use crate::{ - constant::{BUCKET_SLOT_BITS, MAX_SUBTREE_LEVELS, STARTING_NODE_ID}, + constant::{ + BUCKET_SLOT_BITS, MAIN_TRIE_LEVELS, MAX_SUBTREE_LEVELS, ROOT_NODE_ID, STARTING_NODE_ID, + }, trie::node_utils::{ bucket_root_node_id, get_parent_node, subtree_leaf_for_key, vc_position_in_parent, }, @@ -71,11 +73,12 @@ pub(crate) fn parents_and_points( // ============================================================================ // Phase 1: Main Trie Traversal // ============================================================================ - // Walk from the bucket root up to the main trie root (node 0), recording - // each parent-child relationship. This captures the path through the fixed - // 4-level main trie structure that leads to this bucket. + // Walk from the bucket root up to the main trie root, recording each + // parent-child relationship. The main trie has a fixed depth, so the walk + // is exactly `MAIN_TRIE_LEVELS - 1` steps; bounding it by that count rather + // than by reaching the root keeps a corrupt parent id from spinning forever. let mut node = bucket_root_node_id(salt_key.bucket_id()); - while node != 0 { + for _ in 0..MAIN_TRIE_LEVELS - 1 { let parent_node = get_parent_node(&node); // Record that this parent needs to prove the child at this position internal_nodes @@ -85,6 +88,7 @@ pub(crate) fn parents_and_points( node = parent_node; } + debug_assert_eq!(node, ROOT_NODE_ID); // ============================================================================ // Phase 2: Bucket Tree Traversal diff --git a/salt/src/state/state.rs b/salt/src/state/state.rs index 63b9d83f..d5e77907 100644 --- a/salt/src/state/state.rs +++ b/salt/src/state/state.rs @@ -1069,7 +1069,6 @@ fn compute_resize_capacity(capacity: u64, used: u64) -> u64 { #[cfg(test)] mod tests { use super::*; - use crate::constant::BUCKET_SLOT_ID_MASK; use std::{collections::BTreeMap, vec, vec::Vec}; use crate::{ @@ -2742,27 +2741,6 @@ mod tests { } } - /// A bucket subtree addresses exactly `MAX_BUCKET_SIZE` = 2^40 slots, so that - /// capacity must be accepted. The bound used to be `BUCKET_SLOT_ID_MASK` (2^40 - 1), - /// which — since capacities only ever double from 256 — capped buckets at 2^39. - #[test] - fn test_max_bucket_capacity_is_a_reachable_power_of_two() { - // Capacities only ever double up from MIN_BUCKET_SIZE, so 2^40 is reachable: - // it is 2^39 doubled once more. (Stated via the multiplier rather than a - // `compute_resize_capacity` call, because how many doublings a given `used` - // triggers depends on the load-factor threshold, which `test-bucket-resize` - // overrides.) - assert_eq!((1u64 << 39) * BUCKET_RESIZE_MULTIPLIER, MAX_BUCKET_SIZE); - assert!(MAX_BUCKET_SIZE.is_power_of_two()); - // Whatever the threshold, a resize lands on a power of two. - let resized = compute_resize_capacity(MIN_BUCKET_SIZE as u64, MIN_BUCKET_SIZE as u64); - assert!(resized.is_power_of_two()); - assert!(resized > MIN_BUCKET_SIZE as u64); - // The superseded bound was BUCKET_SLOT_ID_MASK, one slot short of 2^40, so the - // largest power-of-two capacity it admitted was only 2^39. - assert_eq!(MAX_BUCKET_SIZE, BUCKET_SLOT_ID_MASK + 1); - } - #[test] #[should_panic(expected = "Exceeds max bucket capacity")] fn test_shi_rehash_rejects_capacity_above_max() { diff --git a/salt/src/trie/node_utils.rs b/salt/src/trie/node_utils.rs index b243ce68..7d8a4518 100644 --- a/salt/src/trie/node_utils.rs +++ b/salt/src/trie/node_utils.rs @@ -110,7 +110,7 @@ //! └─ Up to 4,294,967,296 Level 4 leaf nodes //! ``` //! -//! 2^40 is the ceiling: the deepest level holds 256^4 = 2^32 segments of 256 slots. +//! 2^40 is [`MAX_BUCKET_SIZE`](crate::constant::MAX_BUCKET_SIZE), whose doc derives the ceiling. //! //! ### Key Insights //! @@ -294,13 +294,7 @@ pub(crate) fn get_parent_node(node_id: &NodeId) -> NodeId { // Add parent level's starting position to get absolute parent ID // and preserve the bucket ID for subtree nodes - let parent = bucket_id + parent_relative_position + STARTING_NODE_ID[level - 1] as NodeId; - debug_assert_eq!( - get_bfs_level(get_local_number(parent)), - level - 1, - "parent of a level-{level} node must sit one level up" - ); - parent + bucket_id + parent_relative_position + STARTING_NODE_ID[level - 1] as NodeId } /// Maps a bucket ID to its subtree root node in the main trie. @@ -392,24 +386,15 @@ pub(crate) fn subtree_leaf_start_key(node_id: &NodeId) -> SaltKey { /// capacity ≤ 2^40 (1,099,511,627,776) → level 0 over up to 4,294,967,296 leaves /// ``` /// -/// Each leaf ("segment") holds `MIN_BUCKET_SIZE` = 256 consecutive slots, so the -/// deepest level's 2^32 segments cap a bucket at 2^32 * 256 = 2^40 slots, which is -/// [`MAX_BUCKET_SIZE`](crate::constant::MAX_BUCKET_SIZE). +/// The last row is the ceiling, [`MAX_BUCKET_SIZE`](crate::constant::MAX_BUCKET_SIZE). /// /// # Arguments -/// * `capacity` - The total number of slots the bucket needs to accommodate +/// * `capacity` - The total number of slots the bucket needs to accommodate. Callers +/// must keep it at or below `MAX_BUCKET_SIZE`; past that `level` underflows (a +/// panic in debug builds, a wrap in release builds). /// /// # Returns /// The subtree level (0-4) that should serve as the root for this capacity -/// -/// # Panics -/// -/// Panics in debug builds (and wraps `level` in release builds) if `capacity` exceeds -/// `MAX_BUCKET_SIZE`, since no subtree level can hold it. Callers must bound capacity -/// themselves. `EphemeralSaltState::shi_rehash` asserts it for the capacity it is -/// asked to resize *to*, and `BucketMeta::try_from` rejects a decoded capacity outside -/// `1..=MAX_BUCKET_SIZE`, so a capacity read back through a `StateReader` is bounded; -/// a `BucketMeta` built in-process with a larger `capacity` field is not. pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { // Start from the deepest possible level let mut level = MAX_SUBTREE_LEVELS - 1; @@ -427,33 +412,24 @@ pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { #[cfg(test)] mod tests { use super::*; - use crate::constant::MAX_BUCKET_SIZE; + use crate::constant::{MAX_BUCKET_SIZE, NUM_META_BUCKETS}; /// The deepest subtree-local node number at `MAX_BUCKET_SIZE` exceeds u32, so /// the level-relative arithmetic has to stay in u64 rather than `usize` to be - /// right on 32-bit targets. Pins the values that arithmetic produces there. + /// right on 32-bit targets. Pins what that arithmetic produces there. #[test] fn parent_and_position_at_max_capacity_depth() { - use crate::constant::{ - BUCKET_SLOT_BITS, MAX_SUBTREE_LEVELS, MIN_BUCKET_SIZE, NUM_META_BUCKETS, - STARTING_NODE_ID, TRIE_WIDTH, - }; - use crate::types::{get_local_number, NodeId}; - - let segments = MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64; - let deepest_local = STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + segments - 1; - assert!(deepest_local > u32::MAX as u64); - - let bucket_id = NUM_META_BUCKETS as u64; - let node: NodeId = (bucket_id << BUCKET_SLOT_BITS) | deepest_local; + let last_slot = SaltKey::from((NUM_META_BUCKETS as BucketId, MAX_BUCKET_SIZE - 1)); + let node = subtree_leaf_for_key(&last_slot); + assert!(get_local_number(node) > u32::MAX as u64); + // The last segment is the last child of the last level-3 node. assert_eq!(vc_position_in_parent(&node), TRIE_WIDTH - 1); - let parent = get_parent_node(&node); - assert_eq!(parent >> BUCKET_SLOT_BITS, bucket_id); + assert_eq!(get_child_node(&parent, TRIE_WIDTH - 1), node); assert_eq!( get_local_number(parent), - STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 2] as u64 + segments / TRIE_WIDTH as u64 - 1 + STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 - 1 ); } diff --git a/salt/src/trie/trie.rs b/salt/src/trie/trie.rs index 9113bfc5..3867726e 100644 --- a/salt/src/trie/trie.rs +++ b/salt/src/trie/trie.rs @@ -1152,10 +1152,6 @@ impl SubtrieChangeInfo { #[cfg(test)] mod tests { use super::*; - use crate::{ - constant::{BUCKET_SLOT_ID_MASK, MAX_BUCKET_SIZE, MIN_BUCKET_SIZE}, - types::leftmost_node, - }; use crate::{ mem_store::MemStore, state::{state::EphemeralSaltState, updates::StateUpdates}, @@ -1165,8 +1161,8 @@ mod tests { use crate::{ constant::{ - default_commitment, EMPTY_SLOT_HASH, MAIN_TRIE_LEVELS, MIN_BUCKET_SIZE_BITS, - STARTING_NODE_ID, + default_commitment, EMPTY_SLOT_HASH, MAIN_TRIE_LEVELS, MAX_BUCKET_SIZE, + MIN_BUCKET_SIZE_BITS, STARTING_NODE_ID, }, empty_salt::EmptySalt, }; @@ -1534,24 +1530,12 @@ mod tests { assert_change(65_536, 65_792, 3, 2); assert_change(65_537, 65_536, 2, 3); - // Top of the range. Every capacity above 2^32 is rooted at level 0, so the - // 2^39 -> 2^40 expansion the old `BUCKET_SLOT_ID_MASK` bound rejected is an - // ordinary level-0 transition, and STARTING_NODE_ID[0] == 0 makes the top id - // exactly `bucket_id << BUCKET_SLOT_BITS`. + // Top of the range: every capacity above 2^32 is rooted at level 0, so the + // 2^39 <-> 2^40 doubling is an ordinary level-0 transition, and + // STARTING_NODE_ID[0] == 0 makes the top id exactly `bucket_id << BUCKET_SLOT_BITS`. assert_change(1 << 32, (1 << 32) + 1, 1, 0); assert_change(1 << 39, MAX_BUCKET_SIZE, 0, 0); assert_change(MAX_BUCKET_SIZE, 1 << 39, 0, 0); - - // At maximum capacity the deepest node's local number must stay inside the - // 40-bit slot field, or it would bleed into the bucket-id bits. - let segments = MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64; - let deepest_local = STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + segments - 1; - assert_eq!(deepest_local, 4_311_810_304); - assert_eq!( - deepest_local, - leftmost_node(MAX_SUBTREE_LEVELS as u32).unwrap() - 1 - ); - assert!(deepest_local <= BUCKET_SLOT_ID_MASK); } /// Rebuilds a main trie node commitment from storage for testing purposes. diff --git a/salt/src/types.rs b/salt/src/types.rs index 095fa1c4..edb71f0e 100644 --- a/salt/src/types.rs +++ b/salt/src/types.rs @@ -100,28 +100,15 @@ impl TryFrom<&[u8]> for BucketMeta { message: "BucketMeta requires exactly 12 bytes", }); } - let nonce = - u32::from_le_bytes( - bytes[0..4] - .try_into() - .map_err(|_| SaltError::InvalidFormat { - message: "Failed to parse nonce from bytes", - })?, - ); + let (nonce, capacity) = bytes.split_at(4); + let nonce = u32::from_le_bytes(nonce.try_into().expect("4 bytes after the length check")); let capacity = - u64::from_le_bytes( - bytes[4..12] - .try_into() - .map_err(|_| SaltError::InvalidFormat { - message: "Failed to parse capacity from bytes", - })?, - ); + u64::from_le_bytes(capacity.try_into().expect("8 bytes after the length check")); // A decoded capacity feeds `probe` (which divides by it) and // `subtree_root_level` (which has no level below 0), so bound it here // rather than at every consumer. This is only the range the trie can - // represent: capacities the protocol produces are `MIN_BUCKET_SIZE` - // doubled some number of times, which is not enforced at this boundary - // because tests exercise the SHI logic at smaller capacities. + // represent; that the protocol only ever produces `MIN_BUCKET_SIZE` + // doubled some number of times is not enforced here. if capacity == 0 || capacity > MAX_BUCKET_SIZE { return Err(SaltError::InvalidFormat { message: "BucketMeta capacity must be in 1..=MAX_BUCKET_SIZE", @@ -311,31 +298,21 @@ pub struct SaltValue { } /// Deserializes [`SaltValue::data`] through `serde_arrays`, the same wire shape a -/// plain `#[serde(with = "serde_arrays")]` field has, and applies the -/// `TryFrom<[u8; MAX_SALT_VALUE_BYTES]>` bound on the declared lengths. +/// plain `#[serde(with = "serde_arrays")]` field has, and rejects a buffer whose +/// declared lengths overrun it. fn deserialize_checked_data<'de, D>(deserializer: D) -> Result<[u8; MAX_SALT_VALUE_BYTES], D::Error> where D: serde::Deserializer<'de>, { - let data: [u8; MAX_SALT_VALUE_BYTES] = serde_arrays::deserialize(deserializer)?; - SaltValue::try_from(data) - .map(|value| value.data) - .map_err(serde::de::Error::custom) -} - -impl TryFrom<[u8; MAX_SALT_VALUE_BYTES]> for SaltValue { - type Error = SaltError; - - /// Wrap an already-encoded buffer, rejecting one whose declared lengths overrun it. - fn try_from(data: [u8; MAX_SALT_VALUE_BYTES]) -> Result { - let value = Self { data }; - if value.data_len() > MAX_SALT_VALUE_BYTES { - return Err(SaltError::InvalidFormat { - message: "SaltValue key_len + value_len overruns MAX_SALT_VALUE_BYTES", - }); - } - Ok(value) + let value = SaltValue { + data: serde_arrays::deserialize(deserializer)?, + }; + if value.data_len() > MAX_SALT_VALUE_BYTES { + return Err(serde::de::Error::custom( + "SaltValue key_len + value_len overruns MAX_SALT_VALUE_BYTES", + )); } + Ok(value.data) } impl SaltValue { @@ -699,31 +676,27 @@ mod tests { /// and anything above `MAX_BUCKET_SIZE` has no subtree level to hold it. #[test] fn bucket_meta_decode_bounds_capacity() { - let encode = |capacity: u64| { - BucketMeta { - nonce: 7, - capacity, - used: None, - } - .to_bytes() + let meta = |capacity: u64| BucketMeta { + nonce: 7, + capacity, + used: None, }; for capacity in [1, MIN_BUCKET_SIZE as u64, MAX_BUCKET_SIZE] { - let meta = BucketMeta::try_from(&encode(capacity)[..]) + let decoded = BucketMeta::try_from(&meta(capacity).to_bytes()[..]) .unwrap_or_else(|e| panic!("capacity {capacity} must decode: {e}")); - assert_eq!(meta.capacity, capacity); + assert_eq!(decoded.capacity, capacity); } for capacity in [0, MAX_BUCKET_SIZE + 1, u64::MAX] { assert!( - BucketMeta::try_from(&encode(capacity)[..]).is_err(), + BucketMeta::try_from(&meta(capacity).to_bytes()[..]).is_err(), "capacity {capacity} must be rejected" ); } // The same bound applies when the metadata arrives inside a SaltValue. - let value = SaltValue::new(&encode(0), &[]); - assert!(BucketMeta::try_from(value).is_err()); + assert!(BucketMeta::try_from(SaltValue::from(meta(0))).is_err()); } /// Tests BucketMeta default constructor. Verifies that default values match @@ -790,7 +763,6 @@ mod tests { // One byte past the largest legal layout (20 + 72) is rejected... let mut overrun = value.data; overrun[1] = 73; - assert!(SaltValue::try_from(overrun).is_err()); let result: Result<(SaltValue, _), _> = bincode::serde::decode_from_slice(&overrun, bincode::config::legacy()); assert!( From f93abed2de256478d5903493e1c2d1e6d9049916 Mon Sep 17 00:00:00 2001 From: "liquan.eth" Date: Wed, 9 Sep 2026 08:05:33 +0800 Subject: [PATCH 17/17] fix(constant): pin the deepest subtree node exactly so the spec-gate mutant is unviable The spec-gate's trie pack rewrites `STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1]` to `[MAX_SUBTREE_LEVELS - 2]` inside the `MAX_BUCKET_SIZE` const block. That only lowered the node checked against `BUCKET_SLOT_ID_MASK` from the level-4 base to the level-3 base, so the `<=` bound still held, the crate still compiled, and no runtime test can observe a weaker compile-time guard: the mutant survived (PR #150 spec-gate). Assert the zero-slack fact instead: the last node of a maximally expanded bucket is the one just before a sixth subtree level would begin (`leftmost_node(MAX_SUBTREE_LEVELS)`), then bound that node by the mask as before. With the wrong level base the equality fails at compile time (E0080), so the mutant is now unviable, the same outcome the sibling asserts in this block already produce under the same operator. `umutate.py run --diff origin/main` reports 5 caught, 0 missed, 5 unviable. The rewrite adds two lines above `default_commitment`, so the line-pinned `+ with *` suppression moves from 261 to 263; `mutation_gate.py orphans` reports 0/38 stale against the combined universe. Co-Authored-By: Claude Fable 5.1 --- mutants/suppressions.toml | 2 +- salt/src/constant.rs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/mutants/suppressions.toml b/mutants/suppressions.toml index 79ef15fc..3ec78746 100644 --- a/mutants/suppressions.toml +++ b/mutants/suppressions.toml @@ -53,7 +53,7 @@ reviewer = "krabat/feat/mutation-testing review 2026-07-05" kind = "line" category = "equivalent" file = "salt/src/constant.rs" -line = 261 +line = 263 mutant = "replace + with * in default_commitment" justification = "Turns the level-0 boundary from STARTING_NODE_ID[0]+1 into STARTING_NODE_ID[0]*1 = 0, but the level-0 tuple carries identical left/right commitments, so the selected value is unchanged for every node id. Pinned to the level-0 site; the level 1-3 boundary additions are killed by test_default_commitment_boundary_selection." reviewer = "full-run triage 2026-07-08 (issue #144)" diff --git a/salt/src/constant.rs b/salt/src/constant.rs index adbb7e8a..6eff785b 100644 --- a/salt/src/constant.rs +++ b/salt/src/constant.rs @@ -63,13 +63,15 @@ const _: () = { // Every slot index of a maximally expanded bucket fits the slot field, and the // largest one is exactly `BUCKET_SLOT_ID_MASK`. assert!(MAX_BUCKET_SIZE == 1 << BUCKET_SLOT_BITS); - // So does that bucket's deepest subtree node, which must not bleed into the - // bucket-id bits of a `NodeId`. - assert!( - STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64 - - 1 - <= BUCKET_SLOT_ID_MASK - ); + // That bucket's segments fill the deepest subtree level exactly: its last node + // is the one just before a sixth level would begin, so the ceiling has no slack + // and a wrong level base fails to compile here. + const MAX_SUBTREE_NODE_ID: u64 = STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 + + MAX_BUCKET_SIZE / MIN_BUCKET_SIZE as u64 + - 1; + assert!(MAX_SUBTREE_NODE_ID + 1 == leftmost_node(MAX_SUBTREE_LEVELS as u32).unwrap()); + // And that node must not bleed into the bucket-id bits of a `NodeId`. + assert!(MAX_SUBTREE_NODE_ID <= BUCKET_SLOT_ID_MASK); }; // ============================================================================