Reject invalid DataColumnSidecar for zero blobs

specs/_features/eip7594/fork-choice.md

@@ -31,7 +31,8 @@ def is_data_available(beacon_block_root: Root) -> bool:
  # `MIN_EPOCHS_FOR_DATA_COLUMN_SIDECARS_REQUESTS` epochs. 
  column_sidecars = retrieve_column_sidecars(beacon_block_root)
  return all(
- verify_data_column_sidecar_kzg_proofs(column_sidecar)
+ verify_data_column_sidecar(column_sidecar)
+ and verify_data_column_sidecar_kzg_proofs(column_sidecar)
  for column_sidecar in column_sidecars
  )
 ```

specs/_features/eip7594/p2p-interface.md

@@ -14,6 +14,7 @@
  - [Containers](#containers)
  - [`DataColumnIdentifier`](#datacolumnidentifier)
  - [Helpers](#helpers)
+ - [`verify_data_column_sidecar`](#verify_data_column_sidecar)
  - [`verify_data_column_sidecar_kzg_proofs`](#verify_data_column_sidecar_kzg_proofs)
  - [`verify_data_column_sidecar_inclusion_proof`](#verify_data_column_sidecar_inclusion_proof)
  - [`compute_subnet_for_data_column_sidecar`](#compute_subnet_for_data_column_sidecar)
@@ -64,16 +65,35 @@ class DataColumnIdentifier(Container):
 
 ### Helpers
 
+##### `verify_data_column_sidecar`
+
+```python
+def verify_data_column_sidecar(sidecar: DataColumnSidecar) -> bool:
+ """
+ Verify if the data column sidecar is valid.
+ """
+ # The sidecar index must be within the valid range
+ if sidecar.index >= NUMBER_OF_COLUMNS:
+ return False
+
+ # A sidecar for zero blobs is invalid
+ if len(sidecar.kzg_commitments) == 0:
+ return False
+
+ # The column length must be equal to the number of commitments/proofs
+ if len(sidecar.column) != len(sidecar.kzg_commitments) or len(sidecar.column) != len(sidecar.kzg_proofs):
+ return False
+
+ return True
+```
+
 ##### `verify_data_column_sidecar_kzg_proofs`
 
 ```python
 def verify_data_column_sidecar_kzg_proofs(sidecar: DataColumnSidecar) -> bool:
  """
- Verify if the proofs are correct.
+ Verify if the KZG proofs are correct.
  """
- assert sidecar.index < NUMBER_OF_COLUMNS
- assert len(sidecar.column) == len(sidecar.kzg_commitments) == len(sidecar.kzg_proofs)
-
  # The column index also represents the cell index
  cell_indices = [CellIndex(sidecar.index)] * len(sidecar.column)
 
@@ -148,7 +168,7 @@ The *type* of the payload of this topic is `DataColumnSidecar`.
 
 The following validations MUST pass before forwarding the `sidecar: DataColumnSidecar` on the network, assuming the alias `block_header = sidecar.signed_block_header.message`:
 
-- _[REJECT]_ The sidecar's index is consistent with `NUMBER_OF_COLUMNS` -- i.e. `sidecar.index < NUMBER_OF_COLUMNS`.
+- _[REJECT]_ The sidecar is valid as verified by `verify_data_column_sidecar(sidecar)`.
 - _[REJECT]_ The sidecar is for the correct subnet -- i.e. `compute_subnet_for_data_column_sidecar(sidecar.index) == subnet_id`.
 - _[IGNORE]_ The sidecar is not from a future slot (with a `MAXIMUM_GOSSIP_CLOCK_DISPARITY` allowance) -- i.e. validate that `block_header.slot <= current_slot` (a client MAY queue future sidecars for processing at the appropriate slot).
 - _[IGNORE]_ The sidecar is from a slot greater than the latest finalized slot -- i.e. validate that `block_header.slot > compute_start_slot_at_epoch(state.finalized_checkpoint.epoch)`

tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py

@@ -1,8 +1,173 @@
+import random
 from eth2spec.test.context import (
- spec_test,
  single_phase,
+ spec_state_test,
+ spec_test,
  with_eip7594_and_later,
 )
+from eth2spec.debug.random_value import (
+ RandomizationMode,
+ get_random_ssz_object,
+)
+from eth2spec.test.helpers.block import (
+ sign_block,
+)
+from eth2spec.test.helpers.execution_payload import (
+ compute_el_block_hash,
+)
+from eth2spec.test.helpers.sharding import (
+ get_sample_opaque_tx,
+)
+
+
+# Helper functions
+
+
+def compute_data_column_sidecar(spec, state):
+ rng = random.Random(5566)
+ opaque_tx, blobs, blob_kzg_commitments, _ = get_sample_opaque_tx(spec, blob_count=2)
+ block = get_random_ssz_object(
+ rng,
+ spec.BeaconBlock,
+ max_bytes_length=2000,
+ max_list_length=2000,
+ mode=RandomizationMode,
+ chaos=True,
+ )
+ block.body.blob_kzg_commitments = blob_kzg_commitments
+ block.body.execution_payload.transactions = [opaque_tx]
+ block.body.execution_payload.block_hash = compute_el_block_hash(spec, block.body.execution_payload, state)
+ signed_block = sign_block(spec, state, block, proposer_index=0)
+ cells_and_kzg_proofs = [spec.compute_cells_and_kzg_proofs(blob) for blob in blobs]
+ return spec.get_data_column_sidecars(signed_block, cells_and_kzg_proofs)[0]
+
+
+# Tests for verify_data_column_sidecar
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar__valid(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ assert spec.verify_data_column_sidecar(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar__invalid_zero_blobs(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.column = []
+ sidecar.kzg_commitments = []
+ sidecar.kzg_proofs = []
+ assert not spec.verify_data_column_sidecar(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar__invalid_index(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.index = 128
+ assert not spec.verify_data_column_sidecar(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar__invalid_mismatch_len_column(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.column = sidecar.column[1:]
+ assert not spec.verify_data_column_sidecar(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar__invalid_mismatch_len_kzg_commitments(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
+ assert not spec.verify_data_column_sidecar(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecars__invalid_mismatch_len_kzg_proofs(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.kzg_proofs = sidecar.kzg_proofs[1:]
+ assert not spec.verify_data_column_sidecar(sidecar)
+
+
+# Tests for verify_data_column_sidecar_kzg_proofs
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ assert spec.verify_data_column_sidecar_kzg_proofs(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_kzg_proofs__invalid_wrong_column(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.column[0] = sidecar.column[1]
+ assert not spec.verify_data_column_sidecar_kzg_proofs(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_kzg_proofs__invalid_wrong_commitment(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.kzg_commitments[0] = sidecar.kzg_commitments[1]
+ assert not spec.verify_data_column_sidecar_kzg_proofs(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_kzg_proofs__invalid_wrong_proof(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.kzg_proofs[0] = sidecar.kzg_proofs[1]
+ assert not spec.verify_data_column_sidecar_kzg_proofs(sidecar)
+
+
+# Tests for verify_data_column_sidecar_inclusion_proof
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ assert spec.verify_data_column_sidecar_inclusion_proof(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_inclusion_proof__invalid_missing_commitment(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
+ assert not spec.verify_data_column_sidecar_inclusion_proof(sidecar)
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_inclusion_proof__invalid_duplicate_commitment(spec, state):
+ sidecar = compute_data_column_sidecar(spec, state)
+ sidecar.kzg_commitments = sidecar.kzg_commitments + [sidecar.kzg_commitments[0]]
+ assert not spec.verify_data_column_sidecar_inclusion_proof(sidecar)
+
+
+# Tests for compute_subnet_for_data_column_sidecar
 
 
 @with_eip7594_and_later