fix(salt): allow 2^40 bucket capacity and validate what proofs and witnesses decode

[flyq]

Summary

A bucket's capacity should be able to reach 2^40 slots, the most a bucket subtree can address. It topped out at 2^39 instead.
The review rounds on this PR widened it: it now also rejects, at the decode boundary, a subtree level, a SaltValue length pair, or a BucketMeta capacity that the trie cannot represent, and keeps subtree node arithmetic in u64 so a 32-bit target is correct at the new ceiling. See "Also in this PR" and "Review follow-ups" below.

Root cause

shi_rehash bounded the new capacity with BUCKET_SLOT_ID_MASK:

assert!(new_capacity <= BUCKET_SLOT_ID_MASK, ...)  // 2^40 - 1

BUCKET_SLOT_ID_MASK is the largest slot index; capacity is a slot count. The two differ by one, and that one matters here: capacities start at MIN_BUCKET_SIZE and only ever double (compute_resize_capacity, salt/src/state/state.rs:1054), so every reachable capacity is a power of two. 2^40 > 2^40 - 1 fails the assert, which leaves 2^39 as the largest capacity a bucket can reach — half the addressable space.

2^40 is the real ceiling: a bucket's slots live in the deepest level of its subtree, which holds 256^(MAX_SUBTREE_LEVELS - 1) = 2^32 segments of MIN_BUCKET_SIZE = 256 slots each, so 2^32 × 256 = 2^40. Slot IDs then run 0..2^40, whose largest member is exactly BUCKET_SLOT_ID_MASK — so they still fit the 40-bit slot field of a SaltKey.

Fix

Add MAX_BUCKET_SIZE (salt/src/constant.rs:52), derived from the subtree structure rather than restated as a literal, and bound the capacity with it (salt/src/state/state.rs:682).

Verification

The ceiling has zero slack — 2^40 is exactly right, not merely larger:

• At capacity 2^40 the deepest node's local BFS number is 16,843,009 + 2^32 - 1 = 4,311,810,304, which is precisely leftmost_node(5) - 1, the last valid level-4 id. One more segment would classify as BFS level 5 and index SUBTREE_DEFAULT_COMMITMENT[5] out of bounds.
• It is also where subtree_root_level stops: 2^41 drives level -= 1 at level 0. So the assert sits exactly at the structural limit.

Raising the cap opens no new code path. subtree_root_level already returns 0 for every capacity above 2^32, so the level-0 subtree root and its (bucket_id << 40) | 0 logical address have been live since 2^33 — seven doublings below the old cap. Expansion/contraction (trie.rs), proof shape (shape.rs), and sub-trie/prover addressing (subtrie.rs, prover.rs) were each traced at level 0 and are unaffected; create_sub_trie was exercised at capacities 2^32/2^33/2^39/2^40 and produced identical shapes.

Docs

Three capacity→level ladders 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 stated the ceiling. The MAX_SUBTREE_LEVELS ladder also mapped 768–65,536 slots to level 2; it is level 3. The table above subtree_root_level was inverted on all five rows — it read as depth-from-root while the function returns the root's level. The README said a bucket resizes "to a multiple of 256"; it doubles, which is the property that made the old bound bite.

Testing

Green on all four feature combinations CI runs — default (210 passed), --features test-bucket-resize (199), --no-default-features (209), and --no-default-features --features test-bucket-resize (198) — with clippy and fmt clean. New coverage:

• test_max_bucket_size_matches_subtree_capacity — derives 2^40 from the subtree structure and pins it against the slot field.
• test_max_bucket_capacity_is_a_reachable_power_of_two — 2^40 is one doubling past 2^39, and the old bound was one slot short. Stated through BUCKET_RESIZE_MULTIPLIER rather than a compute_resize_capacity result, since test-bucket-resize overrides the load-factor threshold.
• test_shi_rehash_rejects_capacity_above_max — the assert still fires past the ceiling.
• test_subtree_root_level — adds 2^39, 2^40 - 1, 2^40.
• test_subtrie_change_info_capacity_boundaries — level-0 transitions incl. 2^39 → 2^40, plus the zero-slack check above.

There is deliberately no positive test that rehashes at 2^40: shi_rehash iterates 0..old_capacity, so it would never finish.

Also in this PR: validate what a proof or witness supplies

Two decode-time bounds in the same family as the capacity assert, and two comment fixes, from the SALT spec review:

• fix(proof): fx_hashmap_serde::deserialize now rejects a subtree level outside 1..=MAX_SUBTREE_LEVELS (salt/src/proof/prover.rs:197). Previously any byte decoded; parents_and_points embeds a level ≥ 2 into an encoded parent id and logic_parent_id decodes it with STARTING_NODE_ID[MAX_SUBTREE_LEVELS - level] (salt/src/proof/shape.rs:298), which underflows for 6 and 7, and the trie's update_bucket_subtrees turns a witnessed level into TRIE_WIDTH^level (salt/src/trie/trie.rs:513), which for ≥ 6 drives subtree_root_level below level 0 (level 0 makes it treat an expanded bucket as unexpanded). Wire format unchanged; the check also reaches stateless-core::LightWitness, whose round-trip test uses levels 0/7/255 and will need in-range values when its salt pin moves past v1.0.5.
• fix(types): SaltValue deserialization now rejects 2 + key_len + value_len > MAX_SALT_VALUE_BYTES; key()/value() slice data by those prefixes, so an overrun was a panic, not a rejection. The bound lives in TryFrom<[u8; 94]> (salt/src/types.rs:326) and is applied by a field-level deserialize_with (salt/src/types.rs:316) that keeps the derive and the wire shape (a later commit in this PR moved it there from a hand-written Deserialize). Zero padding past data_len() is deliberately not checked (it never enters kv_hash). Also fixes the 40-vs-72-byte account-value doc: the branch is empty code hash vs any other, not EOA vs contract (EIP-7702-delegated EOAs take the 72-byte form).
• docs(state): the count-unavailable branch of shi_upsert (salt/src/state/state.rs:554) no longer claims the witness "may omit" the count as an optimization or that a skipped resize "can only delay" it; a builder witnesses every slot of a bucket the block resizes, and a skipped resize persists on the delta path and is exposed only by stateless validation against a complete witness from an independent builder. hasher.rs:14 now states the seeds are the low 128 bits of the keccak digest as big-endian 32-bit words, not its "lower 32 bytes".

New tests: rejects_zero_level, rejects_level_above_max, accepts_boundary_levels (prover.rs) and salt_value_deserialize_bounds_declared_lengths (types.rs, also pins the bincode bytes to the raw 94-byte array and a JSON round-trip). Gates re-run on the full branch: default 214 passed, --features test-bucket-resize 203, --no-default-features 213, --no-default-features --features test-bucket-resize 202 (each the earlier count plus the 4 new tests); clippy, fmt, cargo-sort and the riscv64 no_std check clean.

Review follow-ups

From the /code-review high pass on the branch:

• fix(types): BucketMeta::try_from (salt/src/types.rs:95) now rejects capacity == 0 and capacity > MAX_BUCKET_SIZE (:125). Capacity 0 ran shi_upsert's probe loop zero times, then compute_resize_capacity(0, 0) = 0 passed the assert, rehashed to 0 and recursed without bound; above 2^40 subtree_root_level has no level to return. This is the range the trie can represent, not the protocol's domain (256 doubled n times): tests drive the SHI logic at capacities 3/5/8/4 through the same decoder, so the stricter check stays separate. Two fixtures moved into range.
• fix(trie): vc_position_in_parent and get_parent_node (salt/src/trie/node_utils.rs:224, :288) did their level-relative subtraction after an as usize cast. At 2^40 the deepest subtree-local number is 4,311,810,304, past u32::MAX, so a 32-bit target (wasm32 is named as one in state::ahash::convert) truncated. The arithmetic now stays in u64; a test pins both functions at that node.
• refactor(types): the SaltValue length bound moved from a hand-written Deserialize plus mirror struct to a field-level deserialize_with (salt/src/types.rs:316), keeping the derive and the wire shape; TryFrom reads the bound through data_len(). The struct doc now says the zero padding is not checked on decode.
• docs(salt): the shi_upsert backstop comment is qualified to below the capacity ceiling (at 2^40 shi_rehash asserts rather than resizing; the spec has no rule for that case either), and the README marks the 768-slot diagram as an illustration rather than a protocol-produced capacity.
• test(proof): the order-independence fixture no longer carries a level of 6, and the single_entry_bytes comment no longer claims the serializer filters levels.
• chore(mutants): the suppression hygiene job failed on the previous head because seven line-pinned entries in mutants/suppressions.toml drifted (the hasher.rs comment rewrite moved its lines down by two, the constant.rs doc additions moved DEFAULT_COMMITMENT_AT_LEVEL down by 21). Each is re-pinned to the site it was reviewed for, identified by content; verified locally against a cargo mutants --list --package salt universe with scripts/mutation_gate.py orphans, 0/38 stale.
• fix(proof): Codex's inline finding, the last 32-bit-narrowing site: multi_commitments_to_scalars computed a child's slot as absolute_node_id as usize - child_idx as usize (salt/src/proof/subtrie.rs:126). The 256-child range that begins at local node 0xffffff01 ends at 0x100000000, so on a 32-bit target the casts drop the high bits and the subtraction underflows. It now subtracts in u64 and narrows the sub-256 result.
• fix(trie): spec-gate failed on the previous head with one timed-out mutant, the trie pack's STARTING_NODE_ID[level - 1] -> [level] rewrite in get_parent_node, which returns a same-level node and makes every walk to the root spin. A debug_assert_eq! on the returned node's level in get_parent_node (salt/src/trie/node_utils.rs, debug builds only) turns that into an immediate failure; applying the mutation by hand now ends the nextest run in 7 s with exit 100 instead of hanging. The expression predates this PR and only entered the diff gate's scope through the u64 rewrite.

Not taken: the stateless-validator round_trip_through_bincode fixture (other repo, pinned at v1.0.5), the spec-faithful capacity-domain check, and a rule for resizing at the ceiling, which needs a spec decision first. Gates after these commits (re-run unchanged after the two Codex/spec-gate follow-ups): default 216 passed, --features test-bucket-resize 205, --no-default-features 215, --no-default-features --features test-bucket-resize 204 (two new tests each); clippy, fmt, cargo-sort and the riscv64 no_std check clean.

Scope

stateless-validator needs nothing: it has no mirrored capacity bound anywhere, overrides get_subtree_levels to read a witnessed levels byte instead of calling subtree_root_level, and pins salt at tag v1.0.5 — so this change does not reach it until that pin moves.

Notes (pre-existing, not addressed here)

• BucketMeta::try_from bounded capacity nowhere when this PR was opened; the review follow-ups above close the crash cases (0 and above 2^40). What remains separate is the protocol's exact capacity domain, 256 doubled n times, which the decoder still does not enforce because tests drive the SHI logic at smaller capacities.
• salt/src/types.rs says the (bucket_id << 40) | 0 form is forbidden for bucket roots, while the node_utils module doc prescribes it as the level-0 subtree root. Both are defensible — the former governs persistent addressing, the latter a transient navigation alias — but the wording contradicts itself.
• subtrie.rs:343 and prover.rs:533 discriminate main-trie from subtree nodes with < BUCKET_SLOT_ID_MASK where < (1 << BUCKET_SLOT_BITS) is meant. Behaviourally identical (no reachable node id is 2^40 - 1), so a readability wart only.

🤖 Generated with Claude Code

https://claude.ai/code/session_013Spbs54VBmctgmRydZqNfj
https://claude.ai/code/session_013xfA5h7MHVfiKE6kPbVJGb

[github-actions]

Performance Benchmark Comparison

Compared 5 benchmark(s) against the latest main baseline.

Detailed Comparison

Benchmark	Baseline Throughput (Kelem/s)	New Throughput (Kelem/s)	Change
update 10000 KVs/1 threads	63.01	73.42	+16.52%
update 10000 KVs/2 threads	121.50	139.67	+14.95%
update 10000 KVs/4 threads	224.12	257.71	+14.99%
update 10000 KVs/8 threads	387.53	431.35	+11.31%
update 10000 KVs/16 threads	536.66	580.05	+8.09%

[github-actions]

Mutation testing - PASS

Mutation score: 100.0% (5/5 viable mutants killed)

• caught: 5
• survived, real gaps: 0
• timed out, inconclusive: 0
• suppressed: 0
• unviable: 5
• timeout total: 0

No unsuppressed survivors or timeouts remain.

[github-actions]

Mutation testing - PASS

Mutation score: 100.0% (23/23 viable mutants killed)

• caught: 23
• survived, real gaps: 0
• timed out, inconclusive: 0
• suppressed: 0
• unviable: 49
• timeout total: 0

No unsuppressed survivors or timeouts remain.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1ddd441e3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".