Upstream Goldilocks compatibility #2499
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # miden-vm/tests/integration/operations/crypto_ops.rs # processor/src/operations/crypto_ops.rs # processor/src/operations/ext2_ops.rs # processor/src/operations/field_ops.rs # processor/src/operations/fri_ops.rs # processor/src/operations/horner_ops.rs # processor/src/operations/u32_ops.rs # processor/src/parallel/core_trace_fragment/tests.rs # prover/src/lib.rs
Reduce diff against `next` by aligning API patterns. Some changes are
temporary simplifications that should be addressed in follow-up PRs.
## Changes Made
### Matching `next` branch patterns
- Remove `Copy` from `StackInputs`/`StackOutputs` (add `.clone()` calls)
- Rename field `precompile_requests` → `pc_requests` in ExecutionProof
- Remove `Default` derive from `HashFunction`
- Reorder `HashFunction` variants: Poseidon2=0x04, Keccak=0x05
- Use `.inverse()` instead of `.try_inverse().unwrap()` where appropriate
- Delete unused `air/src/utils.rs` and `processor/src/transpose.rs`
### Temporary simplifications
- Replace `.expect("...")` with `.unwrap()` on `QuadFelt::from_basis_coefficients_slice`
- Various formatting fixes (indentation, parentheses removal)
## Follow-up Issues to Create
### 1. Rename `pc_requests` back to `precompile_requests`
The abbreviated name reduces readability. The full name should be
restored after the migration is complete.
**Files**: `air/src/proof.rs`
### 2. Migrate to `QuadFelt::new(a, b)` API
The `next` branch uses `QuadFelt::new(a, b)` instead of
`QuadFelt::from_basis_coefficients_slice(&[a, b]).unwrap()`.
This is cleaner and avoids the unwrap entirely.
**Files**: ~33 occurrences across processor/, miden-vm/, crates/
### 3. Audit inverse method semantics
One test changed from `delta.inverse()` to `delta.try_inverse().unwrap_or(ZERO)`
which silently returns ZERO on zero input. Verify this matches production
behavior or restore panic semantics to match `next` which uses `.inv()`.
**Files**: `processor/src/chiplets/memory/tests.rs`
### 4. Restore helpful `.expect()` messages where appropriate
Replacing `.expect("context")` with `.unwrap()` loses error context.
Review and restore messages for non-obvious failure points.
### 5. Verify Keccak discriminant change is intentional
`HashFunction::Keccak` changed from 0x04 to 0x05. This is a serialization
breaking change for any existing proofs using Keccak. Confirm this is
acceptable or document the migration path.
**Files**: `air/src/proof.rs`
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
adr1anh
changed the base branch from
adr1anh/replace-goldilocks
to
tracking-p3-0.4.2
adr1anh
added
the
no changelog
Transitive dependencies from p3-miden were pulling Plonky3 crates from crates.io while the workspace used git versions. This caused type mismatches like . Add [patch.crates-io] section to ensure all Plonky3 dependencies resolve to the same git versions.
Replace usages of removed From<u8/u16/u32> trait impls with PrimeCharacteristicRing trait methods (from_u8, from_u16, from_u32).
adr1anh
force-pushed
the
al-goldilocks-compat
branch
from
91c2e2d to
9b97702
Compare
adr1anh
force-pushed
the
tracking-p3-0.4.2
branch
2 times, most recently
from
b68cf4c to
c938c0a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Checklist before requesting a review
nextaccording to naming convention.