-
Notifications
You must be signed in to change notification settings - Fork 131
fix(l1,l2): temporary fix for risc0 ecmul precompile #5240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Lines of code reportTotal lines added: Detailed view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes RISC0-specific support from the bn254_g1_mul precompile function, consolidating it to only support the SP1 feature flag for the substrate-bn implementation.
- Simplified feature flag conditionals for
bn254_g1_multo only check forsp1instead of bothsp1andrisc0 - Removed RISC0-specific workaround code that handled differences in substrate-bn's multiplication API
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
**Motivation** <!-- Why does this pull request exist? What are its goals? --> **Description** <!-- A clear and concise general description of the changes this PR introduces --> - Go back to using `ark_bn254` for risc0 Created issue to fix it in another moment as it is not a priority right now, it should be low hanging though #5241
Motivation
Description
ark_bn254for risc0Created issue to fix it in another moment as it is not a priority right now, it should be low hanging though #5241