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

In batch cell verification, check if there are zero cells #3835

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

jtraglia
Copy link
Member

@jtraglia jtraglia commented Jul 5, 2024

Given zero cells, this will throw an exception. We can check for this & return true.

Traceback (most recent call last):
  File "/Users/jtraglia/Projects/jtraglia/consensus-specs/tests/generators/kzg_7594/main.py", line 812, in <module>
    gen_runner.run_generator("kzg_7594", [
  File "/Users/jtraglia/.asdf/installs/python/3.10.4/lib/python3.10/site-packages/eth2spec/gen_helpers/gen_base/gen_runner.py", line 238, in run_generator
    for test_case in tprov.make_cases():
  File "/Users/jtraglia/Projects/jtraglia/consensus-specs/tests/generators/kzg_7594/main.py", line 795, in cases_fn
    for data in test_case_fn():
  File "/Users/jtraglia/Projects/jtraglia/consensus-specs/tests/generators/kzg_7594/main.py", line 241, in case_verify_cell_kzg_proof_batch
    assert spec.verify_cell_kzg_proof_batch(row_commitments, row_indices, column_indices, cells, proofs)
  File "/Users/jtraglia/.asdf/installs/python/3.10.4/lib/python3.10/site-packages/eth2spec/eip7594/mainnet.py", line 5405, in verify_cell_kzg_proof_batch
    return verify_cell_kzg_proof_batch_impl(row_commitments, row_indices, column_indices, cosets_evals, proofs)
  File "/Users/jtraglia/.asdf/installs/python/3.10.4/lib/python3.10/site-packages/eth2spec/eip7594/mainnet.py", line 5269, in verify_cell_kzg_proof_batch_impl
    rli = bls.bytes48_to_G1(g1_lincomb(KZG_SETUP_G1_MONOMIAL[:n], sum_interp_polys_coeff))
  File "/Users/jtraglia/.asdf/installs/python/3.10.4/lib/python3.10/site-packages/eth2spec/eip7594/mainnet.py", line 4395, in g1_lincomb
    assert len(points) == len(scalars)

@jtraglia jtraglia added the EIP-7594 PeerDAS label Jul 5, 2024
@kevaundray
Copy link
Contributor

kevaundray commented Jul 8, 2024

Wouldn't it be better to make the verification algorithm "complete" ? -- ie not have this edge case

@jtraglia
Copy link
Member Author

jtraglia commented Jul 8, 2024

Wouldn't it be better to make the verification algorithm "complete" ? -- ie not have this edge case

Hmm yes good idea! I've pushed a commit which does this + a new test.

@asn-d6
Copy link
Contributor

asn-d6 commented Jul 8, 2024

Code LGTM. Should this also come with a test vector? Or the test vector was there already?

@jtraglia
Copy link
Member Author

jtraglia commented Jul 8, 2024

Code LGTM. Should this also come with a test vector? Or the test vector was there already?

Yes, there's already a test vector for this. That's how I noticed the problem.

# Valid: zero cells
cells, row_commitments, row_indices, column_indices, proofs = [], [], [], [], []
assert spec.verify_cell_kzg_proof_batch(row_commitments, row_indices, column_indices, cells, proofs)
identifier = make_id(row_commitments, row_indices, column_indices, cells, proofs)
yield f'verify_cell_kzg_proof_batch_case_valid_zero_cells_{identifier}', {
'input': {
'row_commitments': encode_hex_list(row_commitments),
'row_indices': row_indices,
'column_indices': column_indices,
'cells': encode_hex_list(cells),
'proofs': encode_hex_list(proofs),
},
'output': True
}

@asn-d6 asn-d6 merged commit 1642610 into ethereum:dev Jul 8, 2024
26 checks passed
@jtraglia jtraglia deleted the early-exit-zero-cells branch July 8, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EIP-7594 PeerDAS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants