Merge pull request #3953 from jtraglia/reject-invalid-dcs

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