From 578407a4c3ed5e21e818dec1722c8327d25e8ed9 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 12:56:47 -0500
Subject: [PATCH 01/12] Reject invalid DataColumnSidecar for zero blobs

---
 specs/_features/eip7594/p2p-interface.md      |   7 +
 .../test/eip7594/unittests/test_networking.py | 147 +++++++++++++++++-
 2 files changed, 153 insertions(+), 1 deletion(-)

diff --git a/specs/_features/eip7594/p2p-interface.md b/specs/_features/eip7594/p2p-interface.md
index 9087a8210c..301d5c3dd4 100644
--- a/specs/_features/eip7594/p2p-interface.md
+++ b/specs/_features/eip7594/p2p-interface.md
@@ -71,7 +71,11 @@ def verify_data_column_sidecar_kzg_proofs(sidecar: DataColumnSidecar) -> bool:
     """
     Verify if the proofs are correct.
     """
+    # A sidecar for zero blobs is invalid
+    assert len(sidecar.kzg_commitments) > 0
+    # The sidecar index must be within the valid range
     assert sidecar.index < NUMBER_OF_COLUMNS
+    # There should be an equal number of cells/commitments/proofs
     assert len(sidecar.column) == len(sidecar.kzg_commitments) == len(sidecar.kzg_proofs)
 
     # The column index also represents the cell index
@@ -93,6 +97,9 @@ def verify_data_column_sidecar_inclusion_proof(sidecar: DataColumnSidecar) -> bo
     """
     Verify if the given KZG commitments included in the given beacon block.
     """
+    # A sidecar for zero blobs is invalid
+    assert len(sidecar.kzg_commitments) > 0
+
     gindex = get_subtree_index(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
     return is_valid_merkle_branch(
         leaf=hash_tree_root(sidecar.kzg_commitments),
diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index 2ab52be6c5..cad046ec53 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -1,8 +1,26 @@
+import functools
+import random
+import threading
 from eth2spec.test.context import (
-    spec_test,
+    expect_assertion_error,
     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,
+)
 
 
 @with_eip7594_and_later
@@ -17,3 +35,130 @@ def test_compute_subnet_for_data_column_sidecar(spec):
     # next one should be duplicate
     next_subnet = spec.compute_subnet_for_data_column_sidecar(spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT)
     assert next_subnet == subnet_results[0]
+
+
+@functools.cache
+def compute_data_column_sidecars(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)
+
+
+# Necessary to cache the result of compute_data_column_sidecars().
+# So multiple tests do not attempt to compute the sidecars at the same time.
+compute_data_column_sidecars_lock = threading.Lock()
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        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_zero_blobs(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.column = []
+        sidecar.kzg_commitments = []
+        sidecar.kzg_proofs = []
+        expect_assertion_error(lambda: 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__invalid_index(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.index = 128
+        expect_assertion_error(lambda: 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__invalid_mismatch_len_column(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.column = sidecar.column[1:]
+        expect_assertion_error(lambda: 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__invalid_mismatch_len_kzg_commitments(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
+        expect_assertion_error(lambda: 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__invalid_mismatch_len_kzg_proofs(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.kzg_proofs = sidecar.kzg_proofs[1:]
+        expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+
+
+@with_eip7594_and_later
+@spec_state_test
+@single_phase
+def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        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__valid_but_missing_commitment(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        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__valid_but_duplicate_commitment(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.kzg_commitments = sidecar.kzg_commitments + [sidecar.kzg_commitments[0]]
+        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_zero_blobs(spec, state):
+    with compute_data_column_sidecars_lock:
+        sidecar = compute_data_column_sidecars(spec, state)[0]
+        sidecar.column = []
+        sidecar.kzg_commitments = []
+        sidecar.kzg_proofs = []
+        expect_assertion_error(lambda: spec.verify_data_column_sidecar_inclusion_proof(sidecar))

From 3f49e6c1557467377c56d11d25a7f94514b954e8 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 14:02:18 -0500
Subject: [PATCH 02/12] Deepcopy sidecar before modifying it

---
 .../test/eip7594/unittests/test_networking.py | 49 ++++++++++---------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index cad046ec53..585508d2da 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -1,3 +1,4 @@
+import copy
 import functools
 import random
 import threading
@@ -38,7 +39,7 @@ def test_compute_subnet_for_data_column_sidecar(spec):
 
 
 @functools.cache
-def compute_data_column_sidecars(spec, state):
+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(
@@ -54,20 +55,20 @@ def compute_data_column_sidecars(spec, state):
     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)
+    return spec.get_data_column_sidecars(signed_block, cells_and_kzg_proofs)[0]
 
 
-# Necessary to cache the result of compute_data_column_sidecars().
+# Necessary to cache the result of compute_data_column_sidecar().
 # So multiple tests do not attempt to compute the sidecars at the same time.
-compute_data_column_sidecars_lock = threading.Lock()
+compute_data_column_sidecar_lock = threading.Lock()
 
 
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         assert spec.verify_data_column_sidecar_kzg_proofs(sidecar)
 
 
@@ -75,8 +76,8 @@ def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs__invalid_zero_blobs(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.column = []
         sidecar.kzg_commitments = []
         sidecar.kzg_proofs = []
@@ -87,8 +88,8 @@ def test_verify_data_column_sidecar_kzg_proofs__invalid_zero_blobs(spec, state):
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_index(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.index = 128
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -97,8 +98,8 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_index(spec, stat
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_column(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.column = sidecar.column[1:]
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -107,8 +108,8 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_col
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg_commitments(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -117,8 +118,8 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg_proofs(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.kzg_proofs = sidecar.kzg_proofs[1:]
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -127,8 +128,8 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         assert spec.verify_data_column_sidecar_inclusion_proof(sidecar)
 
 
@@ -136,8 +137,8 @@ def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__valid_but_missing_commitment(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
         assert not spec.verify_data_column_sidecar_inclusion_proof(sidecar)
 
@@ -146,8 +147,8 @@ def test_verify_data_column_sidecar_inclusion_proof__valid_but_missing_commitmen
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__valid_but_duplicate_commitment(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(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)
 
@@ -156,8 +157,8 @@ def test_verify_data_column_sidecar_inclusion_proof__valid_but_duplicate_commitm
 @spec_state_test
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__invalid_zero_blobs(spec, state):
-    with compute_data_column_sidecars_lock:
-        sidecar = compute_data_column_sidecars(spec, state)[0]
+    with compute_data_column_sidecar_lock:
+        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
         sidecar.column = []
         sidecar.kzg_commitments = []
         sidecar.kzg_proofs = []

From 4578b59ab7334fd10ff548e0c774d7159b6c319f Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 14:39:09 -0500
Subject: [PATCH 03/12] Use wrapper function which does the deepcopy

---
 .../test/eip7594/unittests/test_networking.py | 26 +++++++++++--------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index 585508d2da..e91364c648 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -39,7 +39,7 @@ def test_compute_subnet_for_data_column_sidecar(spec):
 
 
 @functools.cache
-def compute_data_column_sidecar(spec, state):
+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(
@@ -57,6 +57,10 @@ def compute_data_column_sidecar(spec, state):
     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]
 
+def compute_data_column_sidecar(spec, state):
+    """This function returns a copy of a cached data column sidecar."""
+    return copy.deepcopy(_compute_data_column_sidecar(spec, state))
+
 
 # Necessary to cache the result of compute_data_column_sidecar().
 # So multiple tests do not attempt to compute the sidecars at the same time.
@@ -68,7 +72,7 @@ def compute_data_column_sidecar(spec, state):
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         assert spec.verify_data_column_sidecar_kzg_proofs(sidecar)
 
 
@@ -77,7 +81,7 @@ def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs__invalid_zero_blobs(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         sidecar.column = []
         sidecar.kzg_commitments = []
         sidecar.kzg_proofs = []
@@ -89,7 +93,7 @@ def test_verify_data_column_sidecar_kzg_proofs__invalid_zero_blobs(spec, state):
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_index(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         sidecar.index = 128
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -99,7 +103,7 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_index(spec, stat
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_column(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         sidecar.column = sidecar.column[1:]
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -109,7 +113,7 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_col
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg_commitments(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -119,7 +123,7 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg
 @single_phase
 def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg_proofs(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         sidecar.kzg_proofs = sidecar.kzg_proofs[1:]
         expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
 
@@ -129,7 +133,7 @@ def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_kzg
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         assert spec.verify_data_column_sidecar_inclusion_proof(sidecar)
 
 
@@ -138,7 +142,7 @@ def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__valid_but_missing_commitment(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(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)
 
@@ -148,7 +152,7 @@ def test_verify_data_column_sidecar_inclusion_proof__valid_but_missing_commitmen
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__valid_but_duplicate_commitment(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(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)
 
@@ -158,7 +162,7 @@ def test_verify_data_column_sidecar_inclusion_proof__valid_but_duplicate_commitm
 @single_phase
 def test_verify_data_column_sidecar_inclusion_proof__invalid_zero_blobs(spec, state):
     with compute_data_column_sidecar_lock:
-        sidecar = copy.deepcopy(compute_data_column_sidecar(spec, state))
+        sidecar = compute_data_column_sidecar(spec, state)
         sidecar.column = []
         sidecar.kzg_commitments = []
         sidecar.kzg_proofs = []

From df987b5e50960969718a2ca1d67d29c1f36ee709 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 15:47:42 -0500
Subject: [PATCH 04/12] Create new verify_data_column_sidecar() function

---
 specs/_features/eip7594/p2p-interface.md      |  22 ++-
 .../test/eip7594/unittests/test_networking.py | 161 ++++++++++--------
 2 files changed, 103 insertions(+), 80 deletions(-)

diff --git a/specs/_features/eip7594/p2p-interface.md b/specs/_features/eip7594/p2p-interface.md
index 301d5c3dd4..a73a955420 100644
--- a/specs/_features/eip7594/p2p-interface.md
+++ b/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,12 +65,12 @@ class DataColumnIdentifier(Container):
 
 ### Helpers
 
-##### `verify_data_column_sidecar_kzg_proofs`
+##### `verify_data_column_sidecar`
 
 ```python
-def verify_data_column_sidecar_kzg_proofs(sidecar: DataColumnSidecar) -> bool:
+def verify_data_column_sidecar(sidecar: DataColumnSidecar) -> bool:
     """
-    Verify if the proofs are correct.
+    Verify if the data column sidecar is valid.
     """
     # A sidecar for zero blobs is invalid
     assert len(sidecar.kzg_commitments) > 0
@@ -78,6 +79,16 @@ def verify_data_column_sidecar_kzg_proofs(sidecar: DataColumnSidecar) -> bool:
     # There should be an equal number of cells/commitments/proofs
     assert len(sidecar.column) == len(sidecar.kzg_commitments) == len(sidecar.kzg_proofs)
 
+    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.
+    """
     # The column index also represents the cell index
     cell_indices = [CellIndex(sidecar.index)] * len(sidecar.column)
 
@@ -97,9 +108,6 @@ def verify_data_column_sidecar_inclusion_proof(sidecar: DataColumnSidecar) -> bo
     """
     Verify if the given KZG commitments included in the given beacon block.
     """
-    # A sidecar for zero blobs is invalid
-    assert len(sidecar.kzg_commitments) > 0
-
     gindex = get_subtree_index(get_generalized_index(BeaconBlockBody, 'blob_kzg_commitments'))
     return is_valid_merkle_branch(
         leaf=hash_tree_root(sidecar.kzg_commitments),
@@ -155,7 +163,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)`
diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index e91364c648..3c3b40013e 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -1,7 +1,4 @@
-import copy
-import functools
 import random
-import threading
 from eth2spec.test.context import (
     expect_assertion_error,
     single_phase,
@@ -24,22 +21,10 @@
 )
 
 
-@with_eip7594_and_later
-@spec_test
-@single_phase
-def test_compute_subnet_for_data_column_sidecar(spec):
-    subnet_results = []
-    for column_index in range(spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT):
-        subnet_results.append(spec.compute_subnet_for_data_column_sidecar(column_index))
-    # no duplicates
-    assert len(subnet_results) == len(set(subnet_results))
-    # next one should be duplicate
-    next_subnet = spec.compute_subnet_for_data_column_sidecar(spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT)
-    assert next_subnet == subnet_results[0]
+# Helper functions
 
 
-@functools.cache
-def _compute_data_column_sidecar(spec, state):
+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(
@@ -57,113 +42,143 @@ def _compute_data_column_sidecar(spec, state):
     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]
 
-def compute_data_column_sidecar(spec, state):
-    """This function returns a copy of a cached data column sidecar."""
-    return copy.deepcopy(_compute_data_column_sidecar(spec, state))
 
+# Tests for verify_data_column_sidecar
 
-# Necessary to cache the result of compute_data_column_sidecar().
-# So multiple tests do not attempt to compute the sidecars at the same time.
-compute_data_column_sidecar_lock = threading.Lock()
+@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_kzg_proofs__valid(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        assert spec.verify_data_column_sidecar_kzg_proofs(sidecar)
+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 = []
+    expect_assertion_error(lambda: spec.verify_data_column_sidecar(sidecar))
 
 
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_kzg_proofs__invalid_zero_blobs(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.column = []
-        sidecar.kzg_commitments = []
-        sidecar.kzg_proofs = []
-        expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+def test_verify_data_column_sidecar__invalid_index(spec, state):
+    sidecar = compute_data_column_sidecar(spec, state)
+    sidecar.index = 128
+    expect_assertion_error(lambda: spec.verify_data_column_sidecar(sidecar))
 
 
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_index(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.index = 128
-        expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+def test_verify_data_column_sidecar__invalid_mismatch_len_column(spec, state):
+    sidecar = compute_data_column_sidecar(spec, state)
+    sidecar.column = sidecar.column[1:]
+    expect_assertion_error(lambda: spec.verify_data_column_sidecar(sidecar))
 
 
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_kzg_proofs_invalid__invalid_mismatch_len_column(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.column = sidecar.column[1:]
-        expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+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:]
+    expect_assertion_error(lambda: 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__invalid_mismatch_len_kzg_commitments(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
-        expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+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:]
+    expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(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__invalid_mismatch_len_kzg_proofs(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.kzg_proofs = sidecar.kzg_proofs[1:]
-        expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+def test_verify_data_column_sidecar_kzg_proofs__valid_but_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__valid_but_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__valid_but_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):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        assert spec.verify_data_column_sidecar_inclusion_proof(sidecar)
+    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__valid_but_missing_commitment(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.kzg_commitments = sidecar.kzg_commitments[1:]
-        assert not spec.verify_data_column_sidecar_inclusion_proof(sidecar)
+    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__valid_but_duplicate_commitment(spec, state):
-    with compute_data_column_sidecar_lock:
-        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)
+    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
-@spec_state_test
+@spec_test
 @single_phase
-def test_verify_data_column_sidecar_inclusion_proof__invalid_zero_blobs(spec, state):
-    with compute_data_column_sidecar_lock:
-        sidecar = compute_data_column_sidecar(spec, state)
-        sidecar.column = []
-        sidecar.kzg_commitments = []
-        sidecar.kzg_proofs = []
-        expect_assertion_error(lambda: spec.verify_data_column_sidecar_inclusion_proof(sidecar))
+def test_compute_subnet_for_data_column_sidecar(spec):
+    subnet_results = []
+    for column_index in range(spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT):
+        subnet_results.append(spec.compute_subnet_for_data_column_sidecar(column_index))
+    # no duplicates
+    assert len(subnet_results) == len(set(subnet_results))
+    # next one should be duplicate
+    next_subnet = spec.compute_subnet_for_data_column_sidecar(spec.config.DATA_COLUMN_SIDECAR_SUBNET_COUNT)
+    assert next_subnet == subnet_results[0]

From 09cc20459be20df3658b56147bced663022c78e0 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 15:53:42 -0500
Subject: [PATCH 05/12] Add an extra blank line for consistency

---
 .../pyspec/eth2spec/test/eip7594/unittests/test_networking.py    | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index 3c3b40013e..3797a4ecdb 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -45,6 +45,7 @@ def compute_data_column_sidecar(spec, state):
 
 # Tests for verify_data_column_sidecar
 
+
 @with_eip7594_and_later
 @spec_state_test
 @single_phase

From e02cbab59f007a7541671c497bd8a226f3fb4a4f Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 15:55:16 -0500
Subject: [PATCH 06/12] Move index check to top

---
 specs/_features/eip7594/p2p-interface.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/specs/_features/eip7594/p2p-interface.md b/specs/_features/eip7594/p2p-interface.md
index a73a955420..712845c56f 100644
--- a/specs/_features/eip7594/p2p-interface.md
+++ b/specs/_features/eip7594/p2p-interface.md
@@ -72,10 +72,10 @@ def verify_data_column_sidecar(sidecar: DataColumnSidecar) -> bool:
     """
     Verify if the data column sidecar is valid.
     """
-    # A sidecar for zero blobs is invalid
-    assert len(sidecar.kzg_commitments) > 0
     # The sidecar index must be within the valid range
     assert sidecar.index < NUMBER_OF_COLUMNS
+    # A sidecar for zero blobs is invalid
+    assert len(sidecar.kzg_commitments) > 0
     # There should be an equal number of cells/commitments/proofs
     assert len(sidecar.column) == len(sidecar.kzg_commitments) == len(sidecar.kzg_proofs)
 

From 3984bd36041bb974ad22cc624b82d88b6c30e601 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 16:02:39 -0500
Subject: [PATCH 07/12] Convert assert to return False

---
 specs/_features/eip7594/p2p-interface.md              | 11 ++++++++---
 .../test/eip7594/unittests/test_networking.py         | 11 +++++------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/specs/_features/eip7594/p2p-interface.md b/specs/_features/eip7594/p2p-interface.md
index 712845c56f..3dd1994255 100644
--- a/specs/_features/eip7594/p2p-interface.md
+++ b/specs/_features/eip7594/p2p-interface.md
@@ -73,11 +73,16 @@ 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
-    assert sidecar.index < NUMBER_OF_COLUMNS
+    if sidecar.index >= NUMBER_OF_COLUMNS
+        return False
+
     # A sidecar for zero blobs is invalid
-    assert len(sidecar.kzg_commitments) > 0
+    if len(sidecar.kzg_commitments) == 0:
+        return False
+
     # There should be an equal number of cells/commitments/proofs
-    assert len(sidecar.column) == len(sidecar.kzg_commitments) == len(sidecar.kzg_proofs)
+    if len(sidecar.column) != len(sidecar.kzg_commitments) or len(sidecar.column) != len(sidecar.kzg_proofs):
+        return False
 
     return True
 ```
diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index 3797a4ecdb..a818c746fd 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -1,6 +1,5 @@
 import random
 from eth2spec.test.context import (
-    expect_assertion_error,
     single_phase,
     spec_state_test,
     spec_test,
@@ -62,7 +61,7 @@ def test_verify_data_column_sidecar__invalid_zero_blobs(spec, state):
     sidecar.column = []
     sidecar.kzg_commitments = []
     sidecar.kzg_proofs = []
-    expect_assertion_error(lambda: spec.verify_data_column_sidecar(sidecar))
+    assert not spec.verify_data_column_sidecar(sidecar)
 
 
 @with_eip7594_and_later
@@ -71,7 +70,7 @@ def test_verify_data_column_sidecar__invalid_zero_blobs(spec, state):
 def test_verify_data_column_sidecar__invalid_index(spec, state):
     sidecar = compute_data_column_sidecar(spec, state)
     sidecar.index = 128
-    expect_assertion_error(lambda: spec.verify_data_column_sidecar(sidecar))
+    assert not spec.verify_data_column_sidecar(sidecar)
 
 
 @with_eip7594_and_later
@@ -80,7 +79,7 @@ def test_verify_data_column_sidecar__invalid_index(spec, state):
 def test_verify_data_column_sidecar__invalid_mismatch_len_column(spec, state):
     sidecar = compute_data_column_sidecar(spec, state)
     sidecar.column = sidecar.column[1:]
-    expect_assertion_error(lambda: spec.verify_data_column_sidecar(sidecar))
+    assert not spec.verify_data_column_sidecar(sidecar)
 
 
 @with_eip7594_and_later
@@ -89,7 +88,7 @@ def test_verify_data_column_sidecar__invalid_mismatch_len_column(spec, state):
 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:]
-    expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+    assert not spec.verify_data_column_sidecar(sidecar)
 
 
 @with_eip7594_and_later
@@ -98,7 +97,7 @@ def test_verify_data_column_sidecar__invalid_mismatch_len_kzg_commitments(spec,
 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:]
-    expect_assertion_error(lambda: spec.verify_data_column_sidecar_kzg_proofs(sidecar))
+    assert not spec.verify_data_column_sidecar(sidecar)
 
 
 # Tests for verify_data_column_sidecar_kzg_proofs

From 3196487a689056d7d26ef1ed6d7daf4414d70658 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 16:05:10 -0500
Subject: [PATCH 08/12] Add missing semicolon

---
 specs/_features/eip7594/p2p-interface.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/specs/_features/eip7594/p2p-interface.md b/specs/_features/eip7594/p2p-interface.md
index 3dd1994255..710b9f482c 100644
--- a/specs/_features/eip7594/p2p-interface.md
+++ b/specs/_features/eip7594/p2p-interface.md
@@ -73,7 +73,7 @@ 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
+    if sidecar.index >= NUMBER_OF_COLUMNS:
         return False
 
     # A sidecar for zero blobs is invalid

From 2ceb0fd74da30f081caedd6c136646ef2d367030 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Wed, 2 Oct 2024 16:51:46 -0500
Subject: [PATCH 09/12] Fix some nits

---
 specs/_features/eip7594/p2p-interface.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/specs/_features/eip7594/p2p-interface.md b/specs/_features/eip7594/p2p-interface.md
index 710b9f482c..4c4ae83efb 100644
--- a/specs/_features/eip7594/p2p-interface.md
+++ b/specs/_features/eip7594/p2p-interface.md
@@ -80,7 +80,7 @@ def verify_data_column_sidecar(sidecar: DataColumnSidecar) -> bool:
     if len(sidecar.kzg_commitments) == 0:
         return False
 
-    # There should be an equal number of cells/commitments/proofs
+    # 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
 
@@ -92,7 +92,7 @@ def verify_data_column_sidecar(sidecar: DataColumnSidecar) -> bool:
 ```python
 def verify_data_column_sidecar_kzg_proofs(sidecar: DataColumnSidecar) -> bool:
     """
-    Verify if the proofs are correct.
+    Verify if the KZG proofs are correct.
     """
     # The column index also represents the cell index
     cell_indices = [CellIndex(sidecar.index)] * len(sidecar.column)

From 3e80dc8ed02000ab9f3faee036d1fe565461d5eb Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Thu, 3 Oct 2024 07:42:45 -0500
Subject: [PATCH 10/12] Add new verification to is_data_available

---
 specs/_features/eip7594/fork-choice.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/specs/_features/eip7594/fork-choice.md b/specs/_features/eip7594/fork-choice.md
index a6ddf05087..d40d134192 100644
--- a/specs/_features/eip7594/fork-choice.md
+++ b/specs/_features/eip7594/fork-choice.md
@@ -31,6 +31,7 @@ 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(column_sidecar) and
         verify_data_column_sidecar_kzg_proofs(column_sidecar)
         for column_sidecar in column_sidecars
     )

From 30f6aba593572c84c3bc8d21d4256402e5993e06 Mon Sep 17 00:00:00 2001
From: Justin Traglia <95511699+jtraglia@users.noreply.github.com>
Date: Thu, 3 Oct 2024 11:04:33 -0500
Subject: [PATCH 11/12] Put "and" at the beginning of the next line

Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
---
 specs/_features/eip7594/fork-choice.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/specs/_features/eip7594/fork-choice.md b/specs/_features/eip7594/fork-choice.md
index d40d134192..f406b7472a 100644
--- a/specs/_features/eip7594/fork-choice.md
+++ b/specs/_features/eip7594/fork-choice.md
@@ -31,8 +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(column_sidecar) and
-        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
     )
 ```

From 62c32da22bec597b05e5796e10515a78345f6589 Mon Sep 17 00:00:00 2001
From: Justin Traglia <jtraglia@pm.me>
Date: Thu, 3 Oct 2024 11:20:38 -0500
Subject: [PATCH 12/12] Be more consistent with test names

---
 .../eth2spec/test/eip7594/unittests/test_networking.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
index a818c746fd..633508f9a2 100644
--- a/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
+++ b/tests/core/pyspec/eth2spec/test/eip7594/unittests/test_networking.py
@@ -114,7 +114,7 @@ def test_verify_data_column_sidecar_kzg_proofs__valid(spec, state):
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_kzg_proofs__valid_but_wrong_column(spec, state):
+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)
@@ -123,7 +123,7 @@ def test_verify_data_column_sidecar_kzg_proofs__valid_but_wrong_column(spec, sta
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_kzg_proofs__valid_but_wrong_commitment(spec, state):
+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)
@@ -132,7 +132,7 @@ def test_verify_data_column_sidecar_kzg_proofs__valid_but_wrong_commitment(spec,
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_kzg_proofs__valid_but_wrong_proof(spec, state):
+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)
@@ -152,7 +152,7 @@ def test_verify_data_column_sidecar_inclusion_proof__valid(spec, state):
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_inclusion_proof__valid_but_missing_commitment(spec, state):
+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)
@@ -161,7 +161,7 @@ def test_verify_data_column_sidecar_inclusion_proof__valid_but_missing_commitmen
 @with_eip7594_and_later
 @spec_state_test
 @single_phase
-def test_verify_data_column_sidecar_inclusion_proof__valid_but_duplicate_commitment(spec, state):
+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)