consensus-specs icon indicating copy to clipboard operation
consensus-specs copied to clipboard

EIP4844: Update cryptography API and Fiat-Shamir logic

Open asn-d6 opened this issue 2 years ago • 3 comments

This PR does a few things with the cryptography of EIP-4844.

Most importantly, it changes the public API of the KZG library to the following high-level API:

- compute_aggregate_kzg_proof()
- verify_aggregate_kzg_proof()
- blob_to_kzg_commitment()

compared to the old much more low-level API:

- compute_powers()
- matrix_lincomb()
- lincomb()
- bytes_to_bls_field()
- evaluate_polynomial_in_evaluation_form()
- verify_kzg_proof()
- compute_kzg_proof()

This means that all the cryptographic logic (including Fiat-Shamir) is now isolated and hidden in the KZG library and the validator.md file ends up being significantly simplified, only calling high-level KZG functions.

Some additional things that this PR does:

  • Moves all EIP4844 cryptography into polynomial-commitments.md
  • Improves the Fiat-Shamir stack by removing the need for SSZ and by introducing domain separators.

Future TODO tasks:

  • Add unittests in test_validator.py that tests the high-level API

asn-d6 avatar Oct 14 '22 21:10 asn-d6

Closes #3026

kevaundray avatar Oct 16 '22 04:10 kevaundray

Good separation of concerns :+1:

can you give more on the debate over ssz vs domain separators vs neither?

Currently we all agree to remove ssz from the Fiat-Shamir function as this allows for client teams to not implement any of the cryptography.

The debate is on whether these separator labels that we append just before we add a serialised point or field element or polynomial into the bytes array, add any extra security vs having no label at all.

The argument for having them is that you want a precise mapping from your higher level objects to the bytes array. For example, if I was adding a point into the byte array, I could simply serialise the point as 48 bytes then append onto the byte array, or I could first append a label "POINT" and then add the 48 bytes. In this way, the byte array also encodes the types.

Although this seems like a reasonable thing to do, there is no strong rationale as to whether it improves security, and seems to lean on the side of being "extra safe"

kevaundray avatar Oct 16 '22 21:10 kevaundray

As of two minutes ago, I think we now have enough information to come to a conclusion on how to proceed with this

kevaundray avatar Oct 16 '22 22:10 kevaundray

Where is ENDIANNESS defined?

kevaundray avatar Nov 02 '22 14:11 kevaundray

Where is ENDIANNESS defined?

https://github.com/ethereum/annotated-spec/blob/master/phase0/beacon-chain.md#constants

asn-d6 avatar Nov 02 '22 17:11 asn-d6

Merged! Thanks everyone!

asn-d6 avatar Nov 03 '22 15:11 asn-d6