Skip to content

Merge main into shrink-cpu-byte-alu (resolve #652 conflicts)#665

Merged
MauroToscano merged 2 commits into
feat/shrink-cpu-byte-alufrom
merge/main-into-shrink-cpu
Jun 11, 2026
Merged

Merge main into shrink-cpu-byte-alu (resolve #652 conflicts)#665
MauroToscano merged 2 commits into
feat/shrink-cpu-byte-alufrom
merge/main-into-shrink-cpu

Conversation

@MauroToscano

Copy link
Copy Markdown
Contributor

What

Merges latest main into feat/shrink-cpu-byte-alu to clear the merge conflicts on #644.

Since this branch diverged, main advanced only by #652 (soundness: underconstrained chips). #652 added IS_HALFWORD range-checks and wired LT/MUL transition constraints on the same chips this branch reworks — hence the conflicts.

Conflicts resolved (3)

File Resolution
test_utils.rs Union of imports (store from this branch + BusId from #652, used by is_halfword_sender_columns).
tables/shift.rs (collect_bitwise_from_shift) Union — kept both this branch's shift-amount range-checks and #652's input-half (VM-3) checks. The merged bus_interactions carries both sender sets, so both lookup sets are required for the IsHalfword bus to balance. Taking only one side would break bus balance and silently drop a soundness check.
tests/mul_tests.rs Bus-interaction count 24 (#652) with the ALU receiver label (this branch); wiring-test constraint count 6 → 8, because this branch adds the two SignedIsBit(LHS/RHS) constraints that #652's count predated.

Auto-merge handled the rest correctly: #652's MUL input-half senders + trace lookups (→ 24 interactions) and DVRM n/d senders + lookups (→ 20 IS_HALF) both survived intact alongside the ALU-bus rework.

Note: this branch already adds SignedIsBit(LHS_SIGNED/RHS_SIGNED) to MUL — the exact gap an earlier review of #652 had flagged as missing.

Verification

  • cargo build (prover test binary) ✓
  • mul / shift / dvrm / lt unit + bus-count + wiring + busless-soundness tests ✓
  • End-to-end prove+verify: test_shift_8, sllw, srli_one_zero, mul_8 ✓ — confirms the IsHalfword bus balances with the union resolution.

🤖 Generated with Claude Code

jotabulacios and others added 2 commits June 11, 2026 19:32
* register missing in-chip soundness constraints

* add underconstrained-chip soundness guard

* add underconstrained-chip soundness regression tests

* harden in_chip_constraint_count against
  unsigned-subtraction underflow

* Revert beyond-spec SHIFT/DVRM constraints

* Update stale comments and capacity estimate

* Fix stale bus-interaction comments left by soundness fixes

The IS_HALF/IS_HALFWORD senders added in this PR made several doc
comments and one capacity hint inaccurate; update them to match.

- dvrm.rs: module doc IS_HALF count ×16 -> ×20 (matches the function
  doc, which already said ×20 after n/d were added)
- mul.rs: module + bus_interactions docs IS_HALF ×8 -> ×16 and note the
  lhs/rhs input range checks, not just lo/hi outputs
- shift.rs: module doc "11 total" -> "15 total", add IS_HALFWORD (×4) to
  the sender list, and bump Vec::with_capacity(11) -> 15
- test_utils.rs: create_lt_air / create_mul_air now wire transition
  constraints, so their doc comments say "constraints and bus
  interactions" (matching create_shift_air / create_dvrm_air)

---------

Co-authored-by: Diego K <[email protected]>
Co-authored-by: MauroFab <[email protected]>
main advanced by PR #652 (soundness: underconstrained chips), which added
IS_HALFWORD range-checks and wired LT/MUL transition constraints on the same
chips this branch reworks. Conflicts resolved:

- test_utils.rs: union of imports (store + BusId).
- tables/shift.rs collect_bitwise_from_shift: keep BOTH #644's shift-amount
  range-checks AND #652's input-half (VM-3) checks. The merged
  bus_interactions carries both sender sets, so both lookup sets are required
  for the IsHalfword bus to balance.
- tests/mul_tests.rs: bus-interaction count 24 (#652) with the ALU receiver
  label (#644); wiring-test constraint count 6 -> 8, because this branch adds
  the two SignedIsBit(LHS/RHS) constraints that #652's count predated.

Verified: cargo build; mul/shift/dvrm/lt unit + bus-count + wiring + busless
soundness tests pass; end-to-end prove+verify passes for shift_8, sllw,
srli_one_zero, mul_8.
@github-actions

Copy link
Copy Markdown

Codex Code Review

No issues found in the PR diff.

I could not complete a local test run: cargo/rustup attempted to write under read-only /home/runner/.rustup or /home/runner/.cargo paths. I did verify the changed wiring and lookup multiplicities by inspection against the diff and surrounding code.

Comment thread prover/src/test_utils.rs
/// Used to assert input/operand half-limbs are range-checked. Scope: only
/// single-column `Packed` senders (which is how every current IS_HALFWORD sender is
/// declared); it does not inspect `Linear` senders or sender multiplicities.
pub fn is_halfword_sender_columns(interactions: &[BusInteraction]) -> Vec<usize> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Low — silent false confidence if Linear senders are ever used for IS_HALFWORD

This helper silently drops BusValue::Linear senders (the None arm on line 195). Any future IS_HALFWORD sender written as a Linear value will be invisible here, causing presence-check tests to pass while the range check is actually absent.

The doc comment acknowledges this limitation, but the function name doesn't surface it at call sites — callers read is_halfword_sender_columns(…).contains(&c) as "is this column IS_HALF range-checked" without noticing the Packed-only scope. Consider renaming to is_halfword_packed_sender_columns so the restriction is visible everywhere it's called.

Comment thread prover/src/tests/shift_tests.rs
@claude

claude Bot commented Jun 11, 2026

Copy link
Copy Markdown

Code Review

Overall: This PR correctly closes the VM-3 soundness gap (non-canonical half-decomposition attacks) across MUL, DVRM, and SHIFT, and fixes the silent dead-constraint issue in create_lt_air/create_mul_air. The conflict resolution is well-explained and the new regression tests are a good pattern. Two items to flag:


Medium — VM-4: is_negative unconstrained on unsigned-shift rows (deferred, but needs a tracking issue)

SHIFT-C14 sends MSB16[in[3]] → is_negative | signed with Multiplicity::Column(SIGNED), which means the bus provides no binding on is_negative when signed = 0. A malicious prover can set is_negative = 1 on an unsigned SRL/SLL row; the extension = 65535 * is_negative term (SHIFT-C3) then forces arithmetic-right-shift semantics for what should be a logical shift. Any unsigned shift of a value with MSB=1 would produce wrong output that the verifier would accept.

The PR acknowledges this and explicitly defers it (shift.toml omits the constraint). Please ensure there is an open tracking issue for VM-4 before this merges so it doesn't get lost.


Low — is_halfword_sender_columns name doesn't surface its Packed-only scope

Flagged inline. The function silently drops BusValue::Linear senders; if a future IS_HALFWORD check is written as a Linear value the presence tests pass while the check is missing. See inline comment for a rename suggestion.


Correctness notes (no issues found)

  • New n/d IS_HALF senders in DVRM use Multiplicity::Sum(MU_Q, MU_R) — consistent with existing r/n_sub_r/q senders; correct.
  • New lhs/rhs IS_HALF senders in MUL use Multiplicity::Sum(MU_LO, MU_HI) — consistent with existing lo/hi senders; correct.
  • collect_bitwise_from_mul: per-op count is now 8+4+4+4 = 20; capacity hint matches exactly (MSB16 entries live in a separate second loop and over-fill without triggering reallocation).
  • collect_bitwise_from_dvrm: per-op count is 8+4+4+4+1+1 = 22; capacity * 24 is a safe over-estimate.
  • Fixing create_lt_air/create_mul_air to wire real constraints (instead of vec![]) is an important correctness fix — the new wiring tests guard against regression.

@MauroToscano MauroToscano merged commit 732f5bb into feat/shrink-cpu-byte-alu Jun 11, 2026
14 checks passed
@MauroToscano MauroToscano deleted the merge/main-into-shrink-cpu branch June 11, 2026 20:57
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.

2 participants