-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
Remove invalid signature check on v
in ECDSA
#3457
Comments
The check is useful for two reasons:
(More context in this tweet). |
@axic the fundamental question I think here is whether the better error reporting should come from a library or the client implementation directly. I see your point re EIP-1271 tho. |
Checking a condition twice (library & pre-compile implementation) is still not right IMHO. |
We want to generalize the use of custom errors. So, I'd keep the check like we have it today, and just replace the revert reason wit a custom error. |
If we do this we should add a small natspec section that informs the dev that the client implementations actually make this check as well. It's important to be informative about that IMO. We could link to the now merged Yello Paper PR #860 from @axic as well. |
Motivation
All client implementations of the precompile
ecrecover
apparently check if the valuev
is 27 or 28. The references for the different client implementations can be found here.There are two ways to deal with this IMHO:
The text was updated successfully, but these errors were encountered: