Use p3-miden cleanup-and-simplify branch and patched Plonky3 #2492
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]>
…d Plonky3" This reverts commit 256f292.
huitseeker
force-pushed
the
al-migrate-p3-ver2
branch
13 times, most recently
from
02468c7 to
116efcf
Compare
Contributor
|
@Al-Kindi-0 - is this PR still relevant? If so, let's merge the latest |
Contributor
Author
Yes, this is still relevant but I am thinking after the |
Contributor
Contributor
Author
Contributor
|
I can take care of this and then close this PR |
Contributor
Author
That would be great, thanks! |
Contributor
Author
|
Closing this one as we can open a new PR once support for periodic columns is integrated in upstream P3. |
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.