feat: introduce callable oracle for ecrecover#489
Open
antoniolocascio wants to merge 9 commits intodevfrom
Open
feat: introduce callable oracle for ecrecover#489antoniolocascio wants to merge 9 commits intodevfrom
antoniolocascio wants to merge 9 commits intodevfrom
Conversation
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
from
048cd1e to
bacdbaf
Compare
antoniolocascio
commented
Jan 15, 2026
| "ecrecover", | ||
| resources, | ||
| { ecrecover_as_system_function_inner(input, output, resources) } | ||
| // TODO: reconsider if we actually want to use the oracle based version here |
Contributor
Author
There was a problem hiding this comment.
Opening thread to think if we want to use the oracle-based version in prod
Contributor
There was a problem hiding this comment.
Note: we should not forget about it in v0.4.0 @AntonD3
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
2 times, most recently
from
7a1fb22 to
c84ce90
Compare
shamatar
reviewed
Jan 23, 2026
| assert!(el.normalizes_to_zero() == false); | ||
| let mut candidate = el; | ||
| // sqrt_in_place_inner returns true if the input is a quadratic residue (has a square root) | ||
| let is_quadratic_residue = candidate.sqrt_in_place_inner(); |
Member
There was a problem hiding this comment.
Did you check it on negative cases by the way? I wonder if there are state tests with Ecrecover that have an input that is not square root. Here we need candidate to be some exact value and not just random one
Contributor
Author
There was a problem hiding this comment.
Yes, I added negative tests both in unit/prop and fuzzing. Regarding fuzzing, I ran it for several hours and it didn't find any issues
shamatar
reviewed
Jan 23, 2026
Member
shamatar
left a comment
shamatar
left a comment
There was a problem hiding this comment.
Looks fine for me in general. Did you get any fuzzer results already?
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
from
c84ce90 to
8441f21
Compare
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
2 times, most recently
from
9cc035a to
c6a17fa
Compare
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
from
c6a17fa to
c0118ed
Compare
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
3 times, most recently
from
daa7d73 to
8b94f23
Compare
antoniolocascio
force-pushed
the
alocascio-field-oracle
branch
from
8b94f23 to
7e53692
Compare
Contributor
Benchmark report
|
shamatar
approved these changes
Feb 23, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
What ❔
Reimplement the hints for ecrecover from ethproofs.
Instead of having a new full implementation of ecrecover, this PR introduces "hooks" for 3 secp256k1 field operations. These hooks have two implmentations: default, where the implementation is straightforward (same implementation as before this PR) and oracle-based, where we use the new callable oracles.
This way, most of the logic for ecrecover is reused.
Given that this is a critical part of the system, the PR includes:
Why ❔
Is this a breaking change?
Checklist