Conversation
In compact mode, ensure_allocated was a no-op for Elt::Allocated but allocated a new variable for Elt::Num. Whether an element is Num vs Allocated depends on the MDS matrix multiplication results, which vary by input data. This caused different variable counts for different inputs, breaking IVC schemes that require a fixed R1CS shape. Fix: always allocate a fresh variable and enforce equality, regardless of the Elt variant. Remove the unused enforce parameter since it must always be true for soundness. Bump version to 0.65.1.
There was a problem hiding this comment.
Pull request overview
Fixes Poseidon compact mode to always allocate a fresh R1CS variable in ensure_allocated, regardless of whether the Elt is already Allocated or a Num. Previously, Allocated variants were returned as-is, causing inconsistent variable counts across different inputs and breaking IVC schemes that rely on a fixed R1CS shape.
Changes:
- Unified
ensure_allocatedto always allocate + constrain, ensuring deterministic R1CS variable count in compact mode - Bumped crate version from 0.65.0 to 0.66
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/frontend/gadgets/poseidon/circuit2.rs |
Always allocate a fresh variable in ensure_allocated, removing the Allocated short-circuit path |
Cargo.toml |
Version bump to 0.66 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
srinathsetty
force-pushed
the
poseidon-compact-fix
branch
from
82098b1 to
383206b
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes Poseidon's compact mode by ensuring ensure_allocated always allocates a fresh variable (with an equality constraint), regardless of whether the Elt is already Allocated or a Num. Previously, the Allocated variant short-circuited without allocating, causing inconsistent R1CS variable counts between compact and non-compact modes.
Changes:
- Rewrote
Elt::ensure_allocatedto always allocate a new variable and constrain it, using the unifiedlc()method - Removed the now-unnecessary
enforceparameter from all call sites - Bumped crate version to 0.65.1
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/frontend/gadgets/poseidon/circuit2.rs | Rewrote ensure_allocated to always allocate + constrain; removed enforce param |
| src/provider/poseidon.rs | Updated two call sites to drop the enforce argument |
| Cargo.toml | Version bump 0.65.0 → 0.65.1 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
In the updated version, when compact mode is enabled, we allocate a variable regardless of whether the input is Num or already allocated