diff --git a/README.md b/README.md index 73cc6caf..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, 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 (three segments) purely to illustrate the shape; a bucket the protocol has grown holds 512, 1024, ... slots. ``` (In Main SALT Trie) diff --git a/mutants/suppressions.toml b/mutants/suppressions.toml index e64c3b58..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 = 221 +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)" @@ -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)" diff --git a/salt/src/constant.rs b/salt/src/constant.rs index 8f47c24b..6eff785b 100644 --- a/salt/src/constant.rs +++ b/salt/src/constant.rs @@ -35,6 +35,44 @@ 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); + +// `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); + // 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); +}; // ============================================================================ // Trie Structure Constants @@ -54,11 +92,14 @@ 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 ends at [`MAX_BUCKET_SIZE`], whose doc derives that ceiling. /// /// Example for 512-slot bucket (2 segments): /// ```text @@ -129,7 +170,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 +311,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); diff --git a/salt/src/proof/prover.rs b/salt/src/proof/prover.rs index 14b92db8..29359902 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(); @@ -1550,7 +1570,7 @@ mod tests { (4, 3), (1_000_000, 4), (5, 5), - (BucketId::MAX, 6), + (BucketId::MAX, 1), ]; let forward = levels_wrapper(entries); @@ -1573,18 +1593,42 @@ mod tests { assert_eq!(forward_bytes, reverse_bytes); } + /// 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 + } + + fn decode(bytes: &[u8]) -> Result { + bincode::serde::decode_from_slice(bytes, bincode::config::legacy()) + .map(|(decoded, _)| decoded) + } + #[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"); + assert!( + decode(&levels_bytes(&[(7, 1), (7, 2)])).is_err(), + "duplicate BucketId must be rejected" + ); + } + + /// Levels outside `1..=MAX_SUBTREE_LEVELS` are refused; both ends of the + /// range decode in `round_trip_preserves_entries`. + #[test] + 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/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); } 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 94fc0a42..d5e77907 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::*, }; @@ -553,34 +553,41 @@ 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, 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(()); } @@ -666,6 +673,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 +686,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) @@ -2729,6 +2741,21 @@ mod tests { } } + #[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/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 f85f2a8f..7d8a4518 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 [`MAX_BUCKET_SIZE`](crate::constant::MAX_BUCKET_SIZE), whose doc derives the ceiling. +//! //! ### Key Insights //! //! - The root node is always the **first node** at its level within the bucket's subtree namespace @@ -214,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. @@ -276,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, @@ -286,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. @@ -366,16 +374,24 @@ 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 /// ``` /// +/// 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 @@ -396,6 +412,26 @@ pub(crate) fn subtree_root_level(mut capacity: u64) -> usize { #[cfg(test)] mod tests { use super::*; + 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 what that arithmetic produces there. + #[test] + fn parent_and_position_at_max_capacity_depth() { + 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!(get_child_node(&parent, TRIE_WIDTH - 1), node); + assert_eq!( + get_local_number(parent), + STARTING_NODE_ID[MAX_SUBTREE_LEVELS - 1] as u64 - 1 + ); + } /// Tests the vc_position_in_parent function for various node types and positions. /// @@ -739,6 +775,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 { diff --git a/salt/src/trie/trie.rs b/salt/src/trie/trie.rs index e0ed3c65..3867726e 100644 --- a/salt/src/trie/trie.rs +++ b/salt/src/trie/trie.rs @@ -1161,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, }; @@ -1529,6 +1529,13 @@ 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 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); } /// Rebuilds a main trie node commitment from storage for testing purposes. diff --git a/salt/src/types.rs b/salt/src/types.rs index d6c833e4..edb71f0e 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,23 @@ impl TryFrom<&[u8]> for BucketMeta { message: "BucketMeta requires exactly 12 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(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; 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", + }); + } 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, }) } @@ -251,7 +257,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,15 +278,43 @@ 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. +/// +/// 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 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], } +/// Deserializes [`SaltValue::data`] through `serde_arrays`, the same wire shape a +/// 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 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 { /// Create a new encoded value from separate key and value byte slices. /// @@ -611,8 +647,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(); @@ -635,6 +671,34 @@ 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 meta = |capacity: u64| BucketMeta { + nonce: 7, + capacity, + used: None, + }; + + for capacity in [1, MIN_BUCKET_SIZE as u64, MAX_BUCKET_SIZE] { + let decoded = BucketMeta::try_from(&meta(capacity).to_bytes()[..]) + .unwrap_or_else(|e| panic!("capacity {capacity} must decode: {e}")); + assert_eq!(decoded.capacity, capacity); + } + + for capacity in [0, MAX_BUCKET_SIZE + 1, u64::MAX] { + assert!( + 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. + assert!(BucketMeta::try_from(SaltValue::from(meta(0))).is_err()); + } + /// Tests BucketMeta default constructor. Verifies that default values match /// expected initialization: nonce=0, capacity=MIN_BUCKET_SIZE (256), used=None. #[test] @@ -678,6 +742,50 @@ 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; + 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. @@ -789,8 +897,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()); }