Skip to content
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

check for non-canonical field element representations #11

Closed
xrchz opened this issue Nov 25, 2022 · 5 comments
Closed

check for non-canonical field element representations #11

xrchz opened this issue Nov 25, 2022 · 5 comments

Comments

@xrchz
Copy link
Contributor

xrchz commented Nov 25, 2022

Considering the conversation here ethereum/consensus-specs#3057 and ongoing discussion, just want to record that the C library may need to detect and fail on non-canonical inputs (i.e., greater than the modulus).

@sauliusgrigaitis
Copy link

Key question - is there a valid case that MODULUS <= value <= MAX_DATA_TYPE_VALUE at the application level. Seems not. So the crypto library should just throw an informative error that it got a value that doesn't fit into MODULUS. This would also help to debug the application.

@sauliusgrigaitis
Copy link

I would vote for adding an informative error at the crypto library just because at the application level most of the devs are not that familiar with cryptography. By application level, I mean client level.

@xrchz
Copy link
Contributor Author

xrchz commented Dec 1, 2022

Does this need to happen for the arguments of any other API functions than verify_kzg_proof?
Or is it all the functions that take a blob or blobs too?

@xrchz
Copy link
Contributor Author

xrchz commented Dec 2, 2022

Following the latest specs (after ethereum/consensus-specs@23d3aee) I think the way to do it is put the check in bytes_to_bls_field (and have an unchecked version hash_to_bls_field for compute_challenges)

@xrchz
Copy link
Contributor Author

xrchz commented Dec 13, 2022

Closed by #20

@xrchz xrchz closed this as completed Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants