Use Bytes48 for commitments/proofs#86
Use Bytes48 for commitments/proofs#86xrchz merged 22 commits intoethereum:mainfrom jtraglia:use-bytes48
Conversation
| Ok(Self { bytes: new_bytes }) | ||
| } | ||
|
|
||
| pub fn to_proof(self) -> Result<KZGProof, Error> { |
There was a problem hiding this comment.
Are we using these helper functions? If not, maybe we can remove them? If yes, i think they should do the validate_kzg_G1 logic since we consider KZGProof and KZGCommitment to be trusted types.
There was a problem hiding this comment.
Good call. They weren't actually being used. Removed.
|
LGTM! |
| static C_KZG_RET validate_kzg_g1(g1_t *out, const Bytes48 *b) { | ||
| /* Fast check without needing to uncompress */ | ||
| if (memcmp(G1_POINT_AT_INFINITY.bytes, b->bytes, sizeof(b)) == 0) | ||
| return C_KZG_OK; |
There was a problem hiding this comment.
Don't we still need to fill out in this case?
There was a problem hiding this comment.
Yes, thank you. I'm just going to remove that check, since there's an infinity check below.
| typedef struct { uint8_t bytes[48]; } Bytes48; | ||
| typedef struct { uint8_t bytes[BYTES_PER_BLOB]; } Blob; | ||
|
|
||
| typedef Bytes48 KZGCommitment; |
There was a problem hiding this comment.
I probably missed it, but are these used for something?
There was a problem hiding this comment.
They are used as inputs/outputs in internal functions as in the spec. They signify a KZG proof or commitment which has been deserialized and normalized.
|
Does this address #84 too? |
It does yes. Altho we might want to leave that ticket open to figure out how to add a small subgroup unittest. |
3 participants
This PR implements the changes defined here:
Notable:
KZGProof/KZGCommitmentare considered trusted types.Bytes48is untrusted.z_bytesinstead ofzfor consistency, this will be renamed later anyway.bytes_to_g1function (bytes_from_g1is still required).validate_kzg_g1. Not sure which is better.