Zz fixes simplified #39
Conversation
adr1anh
left a comment
adr1anh
left a comment
There was a problem hiding this comment.
Looks good, I realized that we're overwriting uni-stark and still taking in some upstream changes. I don't think this is a problem, as eventually we should be able to isolate all of our own changes to only a couple of crates.
| type EF: ExtensionField<Self::F>; | ||
|
|
||
| /// Expression type over extension field elements. | ||
| type ExprEF: Algebra<Self::Expr> + Algebra<Self::EF>; |
There was a problem hiding this comment.
Upstream Plonky3 changed from ExprEF: From<Self::Expr> + many individual trait bounds to ExprEF: Algebra<Self::Expr> + Algebra<Self::EF> in PR Plonky3#640.
The 0xMiden fork inherited this API from upstream. However, @zhenfeizhang original zz/merge-remote branch temporarily reverted these bounds back to From<Self::Expr> + Algebra<Self::EF> in commit d9a9a14 ("revert changes in uni-stark") to work around API compatibility issues while implementing the multi-trace proving logic.
This branch restores the modern Algebra<Self::Expr> + Algebra<Self::EF> bounds and implements them by adding Algebra<SymbolicExpression<F>> implementations for SymbolicExpression<BinomialExtensionField<F, D>>, ensuring the multi-trace code works correctly with the more modern API rather than relying on the temporary From-based workaround.
There was a problem hiding this comment.
Are we fine with just copying the original now-upstream uni-stark code without attribution?
There was a problem hiding this comment.
Prob should ACK original uni-stark... on it
…gin/main The 'revert changes in uni-stark' commit incorrectly removed the BaseAirWithAuxTrace trait, and the cherry-pick conflict resolution incorrectly modified batch-stark files. Restoring these from origin/main to fix compilation errors.
This is needed for the ExtensionBuilder trait bound after restoring air.rs from origin/main.
…ld and update bounds Added specific Algebra implementation for SymbolicExpression<BinomialExtensionField<F, D>> to satisfy the ExtensionBuilder trait requirements after restoring air.rs from origin/main. Updated where clauses in ExtensionBuilder and PermutationAirBuilder impls to include the Algebra bound.
- Added constraints() method as alias for base_constraints() - Added get_log_quotient_degree() function for backward compatibility - Added empty BaseAirWithAuxTrace implementations for Blake3Air, KeccakAir, Poseidon2Air, and VectorizedPoseidon2Air - Fixed trace reference types in examples/src/proofs.rs
Restored batch-stark directory from backup branch to get the multi-trace implementation. Fixed missing trait imports in poseidon2-air files after checkout.
- Changed &trace to trace in prove() calls - Added log_folding_factor to FriParameters in tests - Changed Rc to Arc in lookup tests to match SymbolicExpression
- Fixed miden-prover test signatures to use &trace - Ran cargo fmt to fix formatting issues - All CI checks now passing (except batch-stark which is excluded)
The backup branch had multi-trace lookup implementation that doesn't work with origin/main APIs. Since no zzhang commits modified batch-stark/src/, using origin/main version.
Update batch-stark prover and verifier to work with the multi-trace API: - Pass aux_width=0 and num_randomness=0 to get_log_quotient_degree (batch-stark doesn't support aux traces yet) - Remove aux_trace_local and aux_trace_next from OpenedValues construction - Remove unused BasedVectorSpace import - Update test to not reference aux_trace fields batch-stark now compiles and all tests pass.
huitseeker
force-pushed
the
zz-fixes-simplified
branch
from
47ddfbf to
01a9db6
Compare
…ancements Remove code from upstream Plonky3 PRs that was accidentally included: - Remove uni-stark/src/sub_builder.rs (from upstream PR Plonky3#1172 by Robin Salen) - Remove uni-stark/tests/rc_sub_builder.rs (test for SubAirBuilder) - Revert matrix/src/horizontally_truncated.rs to origin/main (removes new_with_range from PR Plonky3#1170) These features are not part of zzhang's multi-trace work and were only included because they were in the 'revert changes in uni-stark' commit that was cherry-picked. All tests still pass after removal.
|
Closing in favor of https://github.com/0xmiden/p3-miden |
This is #32 without the upstream fixes. Closes #21 #23 #27 #29