chore(l2): bump ZisK to 0.15.0 & add modexp syscall #5694
Merged
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
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
|
ilitteri
reviewed
Dec 22, 2025
Lines of code reportTotal lines added: Detailed view |
xqft
changed the title
xqft
requested review from
a team,
ManuelBilbao and
avilagaston9
as code owners
ilitteri
approved these changes
Dec 22, 2025
Oppen
reviewed
Dec 22, 2025
Comment on lines
-678
to
694
| let base_mod = base % &modulus; // malachite requires base to be reduced to modulus first | ||
| base_mod.mod_pow(&exponent, &modulus) | ||
| #[cfg(not(feature = "zisk"))] | ||
| { | ||
| let base_mod = base % &modulus; // malachite requires base to be reduced to modulus first | ||
| base_mod.mod_pow(&exponent, &modulus) | ||
| } | ||
|
|
||
| #[cfg(feature = "zisk")] | ||
| { | ||
| use ziskos::zisklib::modexp_u64; | ||
| let (mut base, mut exponent, mut modulus) = (base, exponent, modulus); | ||
| let base_limbs = base.to_limbs_asc(); | ||
| let exponent_limbs = exponent.to_limbs_asc(); | ||
| let modulus_limbs = modulus.to_limbs_asc(); | ||
| let result_limbs = modexp_u64(&base_limbs, &exponent_limbs, &modulus_limbs); | ||
| Natural::from_owned_limbs_asc(result_limbs) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Two things:
- Try not to put these implementations ad-hoc, as it quickly grows with different provers;
- I think we were using
target_vendorrather than feature flags for this.
Contributor
Author
There was a problem hiding this comment.
Yes, at some point we would need to create a module, maybe in ethrex-crypto, to refactor the syscalls and patched crates.
Initially I tried using
#[cfg(all(target_os = "zkvm", target_vendor = "zisk"))]
which I copied from a ZisK patched crate here, but when compiling the guest program it seems to not enable the ZisK path of the code because I had a snippet that didn't compile but the guest compiled anyways. We also lose linting (CI workflows use feature flags).
Considering that currently we are not using target_vendor anywhere in the codebase for zkVMs precompiles, we could leave for a future PR the task of:
- Changing all precompile conditional compilation cases from feature flags to
target_vendor - Have some kind of feedback to make sure we are compiling the right path for each case
- Change the CI workflows to have linting in those cases
Because I think these are out of scope of this PR
Contributor
There was a problem hiding this comment.
I agree with this but I'd rather prefer to address it all in one single PR later.
tomip01
approved these changes
Dec 23, 2025
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.
0.15.0 introduces a new modexp syscall and fixes for the existing patches, apart from other optimizations (see https://github.com/0xPolygonHermez/zisk/releases/tag/v0.15.0)
we found a regression some time ago in main, probably related to a ZisK patch, in which some blocks failed to execute with a gas mismatch error. We tested those blocks again and the regression was gone. So the new patches' versions contain fixes too.