EIP4844 precompile: Strict checks when parsing scalar field elements from the wire #3138
Conversation
bytes_to_bls_field() will be used in the precompile and hence it should error out when provided with malicious inputs.
The previous commit made bytes_to_bls_field() be strict about its inputs. However in compute_challenges() we are dealing with Fiat-Shamir and hash outputs that could be innocuously higher than the modulus. For this reason we add the hash_to_bls_field() helper for use in compute_challenges().
|
I think you can also value = int.from_bytes(blob[i * BYTES_PER_FIELD_ELEMENT: (i + 1) * BYTES_PER_FIELD_ELEMENT], ENDIANNESS)
assert value < BLS_MODULUS |
| The output is not uniform over the BLS field. | ||
| """ | ||
| hashed_data = hash(data) | ||
| return int.from_bytes(hashed_data, ENDIANNESS) % BLS_MODULUS |
There was a problem hiding this comment.
Can you make this:
BLSFieldElement(int.from_bytes(hashed_data, ENDIANNESS) % BLS_MODULUS)
| return int.from_bytes(b, ENDIANNESS) % BLS_MODULUS | ||
| field_element = int.from_bytes(b, ENDIANNESS) | ||
| assert field_element < BLS_MODULUS | ||
| return field_element |
There was a problem hiding this comment.
Same here, can you make this BLSFieldElement(field_element)
|
Thanks for the review. I pushed fixes to your suggestions. |
Done in 69bafc0 |
|
Minor note: In For reference: the reason we do the initial hash first is to reduce the amount of memory needed to compute the subsequent hashes. Without it we would need to copy the whole |
asn-d6
force-pushed
the
strict_modulus_check
branch
from
69bafc0
to
e4df4ba
Compare
I agree this is useful info but not sure if the spec code is the best place to document it. There are so many design decisions taken during writing this spec that are not documented in the spec, and that makes a comment with this particular piece of info feel out of place. I don't have a very strong opinion on this, so feel free to take a stab at it and push to this PR. If not, let's move forward with the current state of this PR. |
verify_kzg_proof()currently reduces inputs to be below the modulus. This can end up in surprising side effects.In this PR we make
bytes_to_bls_field()assert out of its inputs are not canonical.For the purposes of Fiat-Shamir and hash-to-field, we introduce a new function
hash_to_bls_field()which does the modular reduction on its own.Commit messages include explanation on what's happening.