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

EIP4844: Update cryptography API and Fiat-Shamir logic #3038

Merged
merged 35 commits into from
Nov 3, 2022
Merged
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
429e597
4844: Start moving cryptography functions to polynomial-commitments
asn-d6 Oct 14, 2022
22a4dcd
4844: hash_to_bls_field() doesn't use SSZ and does domain separation
asn-d6 Oct 14, 2022
b5959a1
4844: Refactor aggregation and move to polynomial-commitments
asn-d6 Oct 14, 2022
642f138
4844: Fix ToC
asn-d6 Oct 14, 2022
ff528a2
Merge branch 'dev' into pr3038
hwwhww Oct 15, 2022
91476fe
Fix linter error
hwwhww Oct 15, 2022
090dc7e
Blobs as flat ByteVector
dankrad Oct 20, 2022
30d19a3
Move is_data_available from validator to beacon_chain
dankrad Oct 21, 2022
0eb82cf
Set domain separators to empty string
dankrad Oct 21, 2022
7631c18
Add placeholder domain separators for two protocols; add old transcri…
dankrad Oct 21, 2022
fe7af4b
Remove duplicate hash in
dankrad Oct 22, 2022
46b6b24
Fix typo
dankrad Oct 22, 2022
889deff
Fix doctoc
dankrad Oct 22, 2022
83ca385
Fix typo
dankrad Oct 22, 2022
cbc170b
Fix tests
dankrad Oct 22, 2022
89d4ae0
Fix doctoc
dankrad Oct 22, 2022
b9dfdaf
Happy linter
dankrad Oct 22, 2022
033567b
Use Polynomial type consistently, vector_lincomb to poly_lincomb
dankrad Oct 22, 2022
e81d54c
Small fixes
dankrad Oct 22, 2022
463948e
Fix get_sample_blob
dankrad Oct 22, 2022
a33a423
Make `blob_to_field_elements` return `Polynomial` and fix tests
hwwhww Oct 23, 2022
31ad8a5
Rearrange presets
hwwhww Oct 24, 2022
0174521
Proofread
hwwhww Oct 24, 2022
dfcf33c
By kev: Removes domain separators as constants, Removes second call t…
dankrad Oct 27, 2022
d98c103
Add name for Fiat-Shamir protocol
dankrad Oct 30, 2022
80d4d09
Improve comments in polynomial-commitments.md
asn-d6 Nov 1, 2022
c8b8b53
Fix description of BLS_MODULUS
asn-d6 Nov 1, 2022
5354a96
blob_to_field_elements() -> blob_to_polynomial()
asn-d6 Nov 1, 2022
0e2e477
Use modular multiplication in compute_aggregated_poly_and_commitment
asn-d6 Nov 1, 2022
db619e2
Satisfy executable spec tests
asn-d6 Nov 1, 2022
cb46b11
Improve code that deals with the evaluation challenge
asn-d6 Nov 2, 2022
af48987
Minor spec improvements (after review by hww)
asn-d6 Nov 2, 2022
1c9a8db
Remove extra space
dankrad Nov 3, 2022
186a2eb
Move `BYTES_PER_FIELD_ELEMENT` from presets to constants
hwwhww Nov 3, 2022
c130995
Clarify BYTES_PER_FIELD_ELEMENT comment a bit more
asn-d6 Nov 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions specs/eip4844/polynomial-commitments.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ Should we determine that they are required at a later point we can set
| - | - | - |
| `BLS_MODULUS` | `52435875175126190479447740508185965837690552500527637822603658699938581184513` | Scalar field modulus of BLS12-381 |
| `ROOTS_OF_UNITY` | `Vector[BLSFieldElement, FIELD_ELEMENTS_PER_BLOB]` | Roots of unity of order FIELD_ELEMENTS_PER_BLOB over the BLS12-381 field |
| `DOMAIN_SEPARATOR_BLOB_PROTOCOL` | `b""` | Fiat-Shamir domain separator blob verification |
| `DOMAIN_SEPARATOR_AGGREGATE_PROTOCOL` | `b""` | Fiat-Shamir domain separator random aggregation |
| `DOMAIN_SEPARATOR_EVAL_PROTOCOL` | `b""` | Fiat-Shamir domain separator random evaluation |
| `DOMAIN_SEPARATOR_FIELD_ELEMENT` | `b""` | Fiat-Shamir domain separator for field elements |
| `DOMAIN_SEPARATOR_POINT` | `b""` | Fiat-Shamir domain separator for G1 points |
| `DOMAIN_SEPARATOR_SQUEEZE` | `b""` | Fiat-Shamir domain separator before hashing |
Expand Down Expand Up @@ -153,13 +154,13 @@ def blob_to_field_elements(blob: Blob) -> Vector[BLSFieldElement, FIELD_ELEMENTS
#### `hash_to_bls_field`

```python
def hash_to_bls_field(polys: Sequence[Polynomial], comms: Sequence[KZGCommitment]) -> BLSFieldElement:
def hash_to_bls_field(initializer: bytes, polys: Sequence[Polynomial], comms: Sequence[KZGCommitment]) -> Tuple[BLSFieldElement, bytes]:
"""
Compute 32-byte hash of serialised polynomials and commitments concatenated.
This hash is then converted to a BLS field element.
The output is not uniform over the BLS field.
"""
data = DOMAIN_SEPARATOR_BLOB_PROTOCOL
data = initializer

# Append each polynomial which is composed by field elements
for poly in polys:
Expand All @@ -173,7 +174,9 @@ def hash_to_bls_field(polys: Sequence[Polynomial], comms: Sequence[KZGCommitment
data += commitment

data += DOMAIN_SEPARATOR_SQUEEZE
return bytes_to_bls_field(hash(data))
h = hash(data)

return bytes_to_bls_field(hash(data)), h
asn-d6 marked this conversation as resolved.
Show resolved Hide resolved
```

#### `bls_modular_inverse`
Expand Down Expand Up @@ -300,12 +303,12 @@ def compute_kzg_proof(polynomial: Sequence[BLSFieldElement], z: BLSFieldElement)
```python
def compute_aggregated_poly_and_commitment(
blobs: Sequence[Blob],
kzg_commitments: Sequence[KZGCommitment]) -> Tuple[Polynomial, KZGCommitment]:
kzg_commitments: Sequence[KZGCommitment]) -> Tuple[Polynomial, KZGCommitment, bytes]:
"""
Return the aggregated polynomial and aggregated KZG commitment.
"""
# Generate random linear combination challenges
r = hash_to_bls_field(blobs, kzg_commitments)
r, h = hash_to_bls_field(DOMAIN_SEPARATOR_AGGREGATE_PROTOCOL, blobs, kzg_commitments)
r_powers = compute_powers(r, len(kzg_commitments))

# Create aggregated polynomial in evaluation form
Expand All @@ -314,16 +317,16 @@ def compute_aggregated_poly_and_commitment(
# Compute commitment to aggregated polynomial
aggregated_poly_commitment = KZGCommitment(g1_lincomb(kzg_commitments, r_powers))

return aggregated_poly, aggregated_poly_commitment
return aggregated_poly, aggregated_poly_commitment, h
```

#### `compute_aggregate_kzg_proof`

```python
def compute_aggregate_kzg_proof(blobs: Sequence[Blob]) -> KZGProof:
commitments = [blob_to_kzg_commitment(blob) for blob in blobs]
aggregated_poly, aggregated_poly_commitment = compute_aggregated_poly_and_commitment(blobs, commitments)
x = hash_to_bls_field([aggregated_poly], [aggregated_poly_commitment])
aggregated_poly, aggregated_poly_commitment, h = compute_aggregated_poly_and_commitment(blobs, commitments)
x, _ = hash_to_bls_field(DOMAIN_SEPARATOR_EVAL_PROTOCOL + h, [aggregated_poly], [aggregated_poly_commitment])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit:

h + DOMAIN_SEPARATOR_EVAL_PROTOCOL is more natural than DOMAIN_SEPARATOR_EVAL_PROTOCOL + h because the previous transcript state h may already be saved in the hash buffer and so starting a new protocol would just mean we append DOMAIN_SEPARATOR_EVAL_PROTOCOL to the buffer

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be the other way around:

  1. I think the domain separator should be at the very start of the hash function, otherwise it does not prevent random oracle values from colliding.
  2. h is not the full transcript, but only the output of the hash, so this is trivial for efficiency purposes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching h to be appended first instead of second, should not change random oracle collisions I think, unless the hash function is bad.

Instead of thinking of putting it at the beginning of the hash function, I'm thinking of putting it at the beginning of the sub protocol. Ie each new protocol if given a unique domain separator will effectively have a "different" hash function or RO

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not believe that what you suggest functions as a domain separator. If your hashed string is
"DOMAIN_SEPARATOR_PROTOCOL1" + TRANSCRIPT_PROTOCOL1 + "DOMAIN_SEPARATOR_PROTOCOL2" + TRANSCRIPT_PROTOCOL2,
then what guarantees you that "DOMAIN_SEPARATOR_PROTOCOL2" does not appear somewhere in TRANSCRIPT_PROTOCOL1?

return compute_kzg_proof(aggregated_poly, x)
```

Expand All @@ -333,13 +336,13 @@ def compute_aggregate_kzg_proof(blobs: Sequence[Blob]) -> KZGProof:
def verify_aggregate_kzg_proof(blobs: Sequence[Blob],
expected_kzg_commitments: Sequence[KZGCommitment],
kzg_aggregated_proof: KZGCommitment) -> bool:
aggregated_poly, aggregated_poly_commitment = compute_aggregated_poly_and_commitment(
aggregated_poly, aggregated_poly_commitment, h = compute_aggregated_poly_and_commitment(
blobs,
expected_kzg_commitments,
)

# Generate challenge `x` and evaluate the aggregated polynomial at `x`
x = hash_to_bls_field([aggregated_poly], [aggregated_poly_commitment])
x, _ = hash_to_bls_field(DOMAIN_SEPARATOR_EVAL_PROTOCOL + h, [aggregated_poly], [aggregated_poly_commitment])
# Evaluate aggregated polynomial at `x` (evaluation function checks for div-by-zero)
y = evaluate_polynomial_in_evaluation_form(aggregated_poly, x)

Expand Down