Skip to content

Fix SHIFT chip deviations from spec#430

Merged
MauroToscano merged 5 commits into
mainfrom
fix/shift-deviations
Mar 16, 2026
Merged

Fix SHIFT chip deviations from spec#430
MauroToscano merged 5 commits into
mainfrom
fix/shift-deviations

Conversation

@nicole-graus
Copy link
Copy Markdown
Collaborator

Fix SHIFT chip deviations from spec

Description

PR #400 updated the SHIFT spec, so here we update the code to match. This PR fixes two deviations:

  • DEV-1 — Merge HWSL and HWSLC into a single paired bus interaction:
    HWSL now returns [(x << y) mod 2^16, x >> (16 - y)] as a pair per spec. The separate HWSLC bus, multiplicity column, and BitwiseOperationType variant are removed.

  • DEV-2 — Make limb_shift[3] virtual:
    Only limb_shift_raw[0..2] are stored as auxiliary columns. limb_shift[3] is derived as 1 - limb_shift_raw[0] - limb_shift_raw[1] - limb_shift_raw[2] wherever needed, reducing the column count from 27 to 26.

@github-actions
Copy link
Copy Markdown

Codex Code Review

  1. Medium – Incorrect BusId numeric decoding mapping
    In prover/src/tables/types.rs:157, TryFrom<u64> was shifted after removing Hwslc, but it now:

    • maps 12 to Shift (should be Dvrm),
    • shifts all subsequent mappings by one,
    • omits valid discriminant 21 (CommitNextByte), and
    • has no mapping for Dvrm at all.

    Because BusId is #[repr(u64)] and From<BusId> for u64 uses raw discriminants, TryFrom no longer round-trips correctly with From, which can misdecode bus IDs or reject valid ones.

Security findings: No direct security vulnerability identified in this diff.

Assumptions / gaps

  • I couldn’t run tests due sandbox/rustup permission issues (could not create temp file /home/runner/.rustup/tmp/...), so this review is static-analysis only.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 16, 2026

Review: Fix SHIFT chip deviations from spec

No security vulnerabilities, bugs, or significant performance issues found. The implementation is correct and clean.

Verified:

  • DEV-1 (paired HWSL bus): The BITWISE table now correctly returns [SLL, SLLC] as a 4-value bus interaction. The SHIFT table senders include both X[i] and Y[i] per limb, matching the receiver arity. The collect_bitwise_from_shift correctly emits a single Hwsl op per limb instead of separate Hwsl + Hwslc.

  • DEV-2 (virtual limb_shift[3]): The coefficient substitution is correct — (1 - ls[0]) + 15*ls[1] + 31*ls[2] + 47*ls[3] with ls[3] = 1 - raw[0] - raw[1] - raw[2] reduces to 48 - 48*raw[0] - 32*raw[1] - 16*raw[2] ✓. The virtual derivation is consistently applied in get_ls, LimbShiftIsBit(3), and the AND_BYTE bus interaction.

  • LinearTerm coefficients: -1 and 65535 are both valid for i64-typed coefficients; the Goldilocks field conversion handles negative values correctly as p - 1.

  • Bus ID renumbering: The TryFrom<u64> mapping is updated consistently (Hwslc gap at 10 removed, subsequent IDs shifted down by 1). If serialized proofs ever embed bus IDs numerically this would be a compatibility concern, but for an in-process prover this is fine.

  • Column counts and test assertions: 27→26 (shift), 22→21 (bitwise), bus interaction counts (15→11 in SHIFT, 11→10 in BITWISE) all match the updated logic.

@MauroToscano MauroToscano added this pull request to the merge queue Mar 16, 2026
@MauroToscano MauroToscano removed this pull request from the merge queue due to a manual request Mar 16, 2026
@MauroToscano MauroToscano added this pull request to the merge queue Mar 16, 2026
@MauroToscano MauroToscano removed this pull request from the merge queue due to a manual request Mar 16, 2026
@MauroToscano MauroToscano added this pull request to the merge queue Mar 16, 2026
Merged via the queue into main with commit e040925 Mar 16, 2026
9 checks passed
@MauroToscano MauroToscano deleted the fix/shift-deviations branch March 16, 2026 19:46
erik-3milabs pushed a commit that referenced this pull request Apr 10, 2026
* Merge HWSL and HWSLC into a single paired bus interaction

* change hwsl in types

* make limb_shift[3] virtual

* fix comments with hwslc references

* Fix TryFrom<u64> for BusId
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.

4 participants