Skip to content

refactor(prover): align byte range-checks with ARE_BYTES spec#600

Merged
MauroToscano merged 5 commits into
mainfrom
feat/are-bytes-alignment
May 22, 2026
Merged

refactor(prover): align byte range-checks with ARE_BYTES spec#600
MauroToscano merged 5 commits into
mainfrom
feat/are-bytes-alignment

Conversation

@ColoCarletti
Copy link
Copy Markdown
Collaborator

Description:

Spec #532 splits the old IS_BYTE lookup into the IS_BYTE template and the ARE_BYTES[X, Y] paired interaction. Two changes:

  1. Rename IS_BYTE → ARE_BYTES across the prover (BusId, multiplicity column, BitwiseOperationType, doc comments, tests).

  2. Fix KECCAK and KECCAK_RND, which were sending single-element values on the (now 2-element) ARE_BYTES bus LogUp fingerprints couldn't balance. Pair the sends:

    • keccak: addr bytes → 4 (addr[2i], addr[2i+1]) pairs
    • keccak_rnd theta: cxz_left → 20 adjacent-byte pairs per round
    • keccak_rnd rho: (rot_left[b], rot_right[b]) → 200 pairs per round

Trace builder emits matching paired ops; bus/op count assertions updated.

@github-actions
Copy link
Copy Markdown

Codex Code Review

No issues found in the PR diff.

I checked the IsByte to AreBytes rename and the changed Keccak byte-pairing against the corresponding BITWISE receiver signatures and multiplicity collection. The updated sender signatures appear matched by collect_bitwise_from_* changes.

Verification note: I attempted cargo check -p prover, but the sandbox blocked rustup/cargo writes under /home/runner/.rustup / .cargo and network access for dependencies, so I could not complete a compile check locally.

@claude
Copy link
Copy Markdown

claude Bot commented May 21, 2026

Review: refactor(prover): align byte range-checks with ARE_BYTES spec

Overall: Clean, well-scoped refactor. The rename is consistent across all files, the pairing logic is semantically correct, and the count assertions check out arithmetically (440→220 per round for keccak_rnd, 8→4 per call for keccak addr). No security or correctness issues found.

Low: Two stale counts in summary comments

In collect_bitwise_from_keccak (trace_builder.rs), two inline comments were updated from IS_BYTE to ARE_BYTES but their numeric counts were not halved to reflect the pairing:

  • Line 1748: ARE_BYTES on Cxz_left (40) → should be (20) (5 x-cols × 4 pairs)
  • Line 1827: ARE_BYTES (400) → should be (200) (5×5×8 pairs)

The section-header comments and test assertion comments in the same function already use the correct updated counts (20 and 200), so this is just a minor inconsistency.

Everything else looks correct

  • LogUp fingerprint semantics: ARE_BYTES[X, Y] checks both X and Y are in [0, 256); pairing (rot_left[b], rot_right[b]) and (cxz_left[2i], cxz_left[2i+1]) is semantically equivalent to two separate single-byte checks. ✓
  • Trace builder pairing matches bus interaction pairing in all three keccak sites (addr, theta cxz_left, rho rot_left+rot_right). ✓
  • single_byte() / byte_op(..., 0) pattern for IS_BYTE-style single-byte checks preserved everywhere. ✓
  • Bus interaction counts: 138→134 (keccak), 1371→1151 (keccak_rnd), math verified. ✓
  • Test assertion for total bitwise ops updated correctly (109+24*1368105+24*1148). ✓

Comment thread prover/src/tables/trace_builder.rs Outdated
Comment thread prover/src/tables/trace_builder.rs Outdated
Addresses review feedback on PR #600: the two comments in
`collect_bitwise_from_keccak` were renamed IS_BYTE -> ARE_BYTES but kept
the pre-pairing counts. After pairing adjacent bytes into ARE_BYTES
interactions the per-round counts halve:

- theta: ARE_BYTES on Cxz_left is 20 pairs, not 40.
- rho: ARE_BYTES is 200 pairs, not 400.

These now match the `are_bytes = 24 * 220 + 4` test assertion (220/round
= 20 theta + 200 rho). Comment-only; no behavior change.
@MauroToscano MauroToscano enabled auto-merge May 22, 2026 14:24
@MauroToscano MauroToscano added this pull request to the merge queue May 22, 2026
Merged via the queue into main with commit c67bbfb May 22, 2026
12 checks passed
@MauroToscano MauroToscano deleted the feat/are-bytes-alignment branch May 22, 2026 15:22
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