Skip to content

Feat/is whh - #1022

Merged
MauroToscano merged 32 commits into
mainfrom
feat/is-whh
Oct 23, 2026
Merged

MauroToscano merged 32 commits into
mainfrom
feat/is-whh

Conversation

@jotabulacios

@jotabulacios jotabulacios commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Merge after #1016.

Implements the spec changes from #1003 and #1015, plus the IS_WH multiplicity bound from #1017.

  • New IS_WH table: one row per distinct (Word, Half) pair, range-checks its three halves with multiplicity μ and bounds μ with IS_B20[μ]. Pairs sent more than 2^20 − 1 times are split over several rows.
  • is_whh() helper for the template: sends IS_WH[word_hi, half_mid] and IS_HALF[half_lo]. The caller's cond must be a bit.
  • KECCAK state_ptr moves from DWordHL to DWordWHH: 486 columns and 84 interactions instead of 511 and 134.
  • TableCounts gets an is_wh field (at most one table), so RECURSION_INPUT_VERSION goes to 5 (main's v4 is the layout without commit) and the statement/epoch tags to V5.
  • The constant-multiplicity guard for optional tables also accepts IS_B20 sends, same reasoning as ARE_BYTES.

@jotabulacios
jotabulacios added this pull request to stack #1023 October 15, 2026 13:39
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

No actionable issues found in the PR diff. Reviewed statically for safety, correctness, significant performance issues, and dead code; no builds or tests run.

Comment thread prover/src/tables/trace_builder.rs Outdated
@claude

claude Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Static review of the IS_WH table, the IS_WHH template and the KECCAK state_ptr move to DWordWHH. I found no correctness or security problems.

What I checked:

  • IS_WH bus: the receiver's word, value[1] + 2^16·value[2], has both halves IS_HALF-checked. So the KECCAK word_hi column is range-checked to 32 bits through the bus, and the ADD constraint ties the limbs to addr + 8·lane.
  • μ bound: the IS_B20[μ] send has multiplicity 1 and the same packing as the other IS_B20 senders. All-zero padding rows send IS_B20[0], which the collector counts. The MU_MAX split is correct and keeps rows sorted, so the trace is deterministic.
  • KECCAK: cond = MU is constrained to a bit (idx 51). Padding rows (μ = 0) gate every send.
  • Wiring and versions: TableCounts.is_wh is included in the statement absorb, the total, the at-most-one guard and the AIR/trace pairing. RECURSION_INPUT_VERSION and both domain tags are bumped. Bus ID 32 is new and does not collide.

One non-blocking comment, inline: is_wh_keys builds one key per lane for every call, and dedup_is_wh_rows runs twice over that list (once in the collector, once in trace generation). Deduplicating once and sharing the rows would cut memory and time on keccak-heavy runs, and is simpler.

Nit: lib.rs now reads "none of the nineteen optional chips carries…" — fine as-is.

Not covered: the PR branch wasn't checked out in the sandbox, so I reviewed from gh pr diff alone and could not read the surrounding code. I did not build or run the tests.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

AI Review

PR #1022 · 28 changed files

Findings

No non-rejected structured findings were reported.

Reviewer Lanes

Lane Model Prompt Status Findings

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.

@nicole-graus

Copy link
Copy Markdown
Collaborator

/bench

@github-actions

Copy link
Copy Markdown

Benchmark — real block (ethrex_mainnet_25368371.bin) (median of 3)

continuations · epoch 2^22 · 8 epochs

Metric main PR Δ
Peak heap 46399 MB 47079 MB +680 MB (+1.5%) ⚪
Prove time 101.718s 102.980s +1.262s (+1.2%) ⚪

✅ No significant change.

Prove-time spread 1.6% (102.980s / 104.448s / 102.766s)

Commit: 0a78672 · Baseline: built from main · Runner: self-hosted bench

(lo = two halves, hi = the word). Needed for the KECCAK state_ptr.
and use them for the KECCAK state_ptr.

IS_WH range checks the top 48 bits of an address, with one row per
distinct (word, half) pair. Its multiplicity is bounded with IS_B20, as in
down from 511 and 134.

TableCounts gets an is_wh field, so RECURSION_INPUT_VERSION goes to 4 and
the statement tags to V5.
… both

the trace and the BITWISE collector, instead of deduplicating the full key
list twice. Also fix two comments: the at-most-one check is not only for
accelerators, and bus ID 29 was the EC_SCALAR ServeK bus, not an ECSM one.
… Word

in an IS_WH row, and a KECCAK state_ptr resplit that keeps the ADD and the
memory address but puts half_mid out of range. Each one fails if its
IS_HALF send is removed.

Also add IS_WH to the IR, OOD and GPU differential test lists, rename its
MU_MAX to IS_WH_MU_MAX and pin it to the IS_B20 range, and fix the
at_most_one comment: for IS_WH the count is what bounds the μ sum.
…a 64 KiB

boundary (new test_keccak_64k program), a continuation run where only the
keccak epoch ships IS_WH, and a recursion blob whose inner proof has IS_WH.

The two IS_WH range forgeries now check that the bus balance is what rejects
them, and the Word one gets its own control. Also add IS_WH to the GPU test
list and fix a few stale bits: the TableCounts doc, KECCAK's interaction
capacity, the debug bus legend (stopped at ID 21) and the no-accelerators test.
…ecutor tests,

as its comment says, and have the two IS_WH forgery tests run the real verifier
again next to the bus check.
@jotabulacios

Copy link
Copy Markdown
Collaborator Author

/ai-review

@github-actions

Copy link
Copy Markdown

Codex Code Review

No actionable issues found in the PR diff. Reviewed statically; no builds or tests were run.

@claude

claude Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Reviewed statically. I found no blocking issues.

Checked and OK:

  • Bus payloads match. is_whh sends [word_hi, half_mid], and IS_WH receives [v1 + 2^16·v2, v0]. All three halves are range-checked with multiplicity μ, so word_hi < 2^32 holds. That is the assumption the from_dword_whh ADD carry needs for hi.
  • μ bound. IS_B20[μ] is sent with multiplicity 1, IS_WH_MU_MAX is the top BITWISE row (there is a test for this), and at_most_one caps the table count. Together these keep the per-key sum far below p. A row with μ = 0 receives nothing, so its unchecked halves can't be used.
  • Relaxed constant-multiplicity guard. The IS_B20 send uses Multiplicity::One, so padding rows do contribute. collect_bitwise_from_is_wh counts them with the same padded_rows as generate_is_wh_trace, and the table is never shipped present-but-empty. Both builders read the same is_wh_rows vector.
  • New table is wired in everywhere. AIR order matches trace order (air_trace_pairs and the refs list both put is_wh after hints). TableCounts, the statement absorb, the version/tag bumps and the exhaustive Traces destructures all include it.

One low-priority readability nit: state_addr.checked_add(lane * 8).expect("keccak state address range must be validated…") now appears three times: generate_keccak_trace, collect_bitwise_from_keccak and the new keccak::is_wh_keys. A small fn state_ptr(op, lane) -> u64 in keccak.rs would keep the trace, the BITWISE counts and the IS_WH keys from drifting apart.

Resolve the two conflicts with #984 (COPY replaces COMMIT):
- trace_builder.rs BITWISE collectors: keep COPY's collector from main and this
  branch's KECCAK_RND padding count (keccak_rnd::padding_are_bytes).
- skip_empty_tables_tests.rs: COMMIT left the optional-table list (17 tables);
  keep this branch's ARE_BYTES exemption imports.
- dvrm.rs / trace_builder.rs: DVRM's MSB16 and NEG ZERO lookups are counted once
  per row, so an op split at MU_MAX sends them once per row.
- trace_builder.rs: KECCAK_RND no longer sends HWSL.
- continuations_design.md: LT and MUL bound their own mu with a constant-1
  ARE_BYTES send.
Brings in main's #984 (COPY replaces COMMIT) through #1016's branch.
Conflicts resolved by keeping both sides:
- TableCounts / statement / VmAirs / Traces: drop commit, keep is_wh and copy.
- BusId: IS_WH = 32 next to COPY's CopyNext = 33. 32 was one of the two IDs
  main's comment listed as unused; 29 and 34 stay unused.
- RECURSION_INPUT_VERSION 5 with a v5 note: main's v4 already means the
  layout without commit, so is_wh needs its own number.
- Tests: counts 20 / 18 / 6, the accelerator test renamed to
  a_run_without_accelerators_omits_all_five_and_is_wh, COMMIT's omission
  tests dropped as on main, and main's committed-output test kept with this
  branch's "dropping ..." message style.
- lib.rs: eighteen optional chips, not nineteen (COMMIT left, IS_WH joined).
- copy.rs: the prover now has an IS_WH table; COPY keeps four IS_HALF lookups
  where the spec uses IS_WHH, and moving them is a follow-up.
- skip_empty_tables_tests.rs: the cap covers the five accelerators and IS_WH;
  COPY is among the always-present tables.
- gpu_constraint_interp_real.rs: 27 tables.
- continuations_design.md: validate() also caps IS_WH at one table, which is
  what bounds its multiplicity sum.
Base automatically changed from fix/multiplicity-soundness to main October 23, 2026 19:01
#1016 landed on main as 8e413a3, whose tree is identical to the branch head
647c6b8 already merged here, so this merge only joins the histories; the tree
is unchanged.
@MauroToscano
MauroToscano added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit e70bde4 Oct 23, 2026
15 checks passed
@MauroToscano
MauroToscano deleted the feat/is-whh branch October 23, 2026 19:58
MauroToscano added a commit that referenced this pull request Oct 23, 2026
Conflict: lib.rs, VmAirs::new builds both the IS_WH AIRs and the pinned
REGISTER AIR.

Changes the merge needs without a textual conflict:
- cpu_start counts the IS_WH sub-proofs, which air_refs places after the
  accelerators and before the CPU tables; without it the verifier would read
  CPU heights one sub-proof early in every proof that ships IS_WH. The
  air_refs debug check and cpu_proof_start_matches_air_refs_layout (64 masks,
  is_wh included) follow.
- is_wh_tests: VmAirs::new takes the CPU heights and the REGISTER commitment;
  the prover commits to its own trace.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants