Add KZG tests for invalid length inputs #3282
Conversation
tests/generators/kzg_4844/main.py
Outdated
| blob, z = VALID_BLOBS[1], VALID_ZS[1] | ||
| proof = spec.compute_kzg_proof(blob, z) | ||
| for y in INVALID_ZS: | ||
| blob, z = VALID_BLOBS[4], VALID_ZS[1] |
There was a problem hiding this comment.
I made this change (testing all of the invalid field element values) because I wanted to test invalid length field elements for the y field too. Previously, this was only testing one invalid y value.
| def verify_kzg_proof(commitment_bytes: Bytes48, | ||
| z: Bytes32, | ||
| y: Bytes32, | ||
| z_bytes: Bytes32, | ||
| y_bytes: Bytes32, | ||
| proof_bytes: Bytes48) -> bool: | ||
| """ | ||
| Verify KZG proof that ``p(z) == y`` where ``p(z)`` is the polynomial represented by ``polynomial_kzg``. | ||
| Receives inputs as bytes. | ||
| Public method. | ||
| """ | ||
| assert len(commitment_bytes) == BYTES_PER_COMMITMENT | ||
| assert len(z_bytes) == BYTES_PER_FIELD_ELEMENT | ||
| assert len(y_bytes) == BYTES_PER_FIELD_ELEMENT | ||
| assert len(proof_bytes) == BYTES_PER_PROOF |
There was a problem hiding this comment.
You're right, typing hint doesn't mandatorily verify the length. You can also assert it is checking with SSZ remerkleable:
- You can cast:
Bytes48(commitment_bytes), it will raise an error if the length is wrong. - You can use
assert isinstance(commitment_bytes, Bytes48)to force the input to beremerkleable-defined Bytes48. That said, Python's built-in 48-byte object will be an invalid input.
However, I think the assertions you added are more explicit and clear for client devs. 👍
There was a problem hiding this comment.
Interesting, thanks! Sounds good 👍
|
I'd also like to merge this before the next release to have the maximum coverage in test vectors |
|
Great. The latest changes have been merged into this PR & I have tested that all of KZG tests generate successfully. |
jtraglia
changed the title
We would like to test invalid length inputs (blobs/commitments/proofs). This PR adds these tests and updates the spec to accommodate these changes. This required adding explicit length checks; Python's type-hinting does not actually enforce byte lengths. @hwwhww is there a better way to do this? Also, I fixed a couple of nits with parameter names.