review 1

beacon_chain/consensus_object_pools/data_column_quarantine.nim

@@ -23,7 +23,7 @@ const
 type
   DataColumnQuarantine* = object
     data_columns*:
-      OrderedTable[(Eth2Digest, ColumnIndex), ref DataColumnSidecar]
+      OrderedTable[DataColumnIdentifier, ref DataColumnSidecar]
     onDataColumnSidecarCallback*: OnDataColumnSidecarCallback
   
   DataColumnFetchRecord* = object
@@ -32,15 +32,13 @@ type
 
   OnDataColumnSidecarCallback = proc(data: DataColumnSidecar) {.gcsafe, raises: [].}
 
-func shortLog*(x: seq[ColumnIndex]): string =
-  "<" & x.mapIt($it).join(", ") & ">"
 
 func shortLog*(x: seq[DataColumnFetchRecord]): string =
   "[" & x.mapIt(shortLog(it.block_root) & shortLog(it.indices)).join(", ") & "]"
 
 func put*(quarantine: var DataColumnQuarantine,
           dataColumnSidecar: ref DataColumnSidecar) =
-  if quarantine.data_columns.lenu64 >= MaxDataColumns:
+  if quarantine.data_columns.len >= static(MaxDataColumns.int):
     # FIFO if full. For example, sync manager and request manager can race
     # to put data columns in at the same time, so one gets data column
     # insert -> block resolve -> data column insert, which leaves
@@ -49,7 +47,7 @@ func put*(quarantine: var DataColumnQuarantine,
     # This also therefore automatically garbage-collects otherwise valid 
     # data columns that are correctly signed, point to either correct block
     # root which isn't ever seen, and then for any reason simply never used.
-    var oldest_column_key: (Eth2Digest, ColumnIndex)
+    var oldest_column_key: DataColumnIdentifier
     for k in quarantine.data_columns.keys:
       oldest_column_key = k
       break
@@ -57,7 +55,9 @@ func put*(quarantine: var DataColumnQuarantine,
   let block_root = 
     hash_tree_root(dataColumnSidecar.signed_block_header.message)
   discard quarantine.data_columns.hasKeyOrPut(
-    (block_root, dataColumnSidecar.index), dataColumnSidecar)
+    DataColumnIdentifier(block_root: block_root,
+                         index: dataColumnSidecar.index),
+                         dataColumnSidecar)
 
 func hasDataColumn*(
     quarantine: DataColumnQuarantine,
@@ -75,29 +75,40 @@ func hasDataColumn*(
 
 func popDataColumns*(
     quarantine: var DataColumnQuarantine, digest: Eth2Digest,
-    blck: deneb.SignedBeaconBlock | electra.SignedBeaconBlock):
+    blck: electra.SignedBeaconBlock):
     seq[ref DataColumnSidecar] =
   var r: seq[ref DataColumnSidecar]
   for idx in 0..<len(blck.message.body.blob_kzg_commitments):
     var c: ref DataColumnSidecar
-    if quarantine.data_columns.pop((digest, ColumnIndex idx), c):
+    if quarantine.data_columns.pop(
+        DataColumnIdentifier(block_root: digest,
+                             index: ColumnIndex idx),
+                             c):
       r.add(c)
   r
 
 func hasDataColumns*(quarantine: DataColumnQuarantine,
-    blck: deneb.SignedBeaconBlock | electra.SignedBeaconBlock): bool =
+    blck: electra.SignedBeaconBlock): bool =
   for idx in 0..<len(blck.message.body.blob_kzg_commitments):
-    if (blck.root, ColumnIndex idx) notin quarantine.data_columns:
+    let dc_id = DataColumnIdentifier(
+      block_root: blck.root,
+      index: ColumnIndex idx)
+    if dc_id notin quarantine.data_columns:
       return false
   true
 
 func dataColumnFetchRecord*(quarantine: DataColumnQuarantine,
-    blck: deneb.SignedBeaconBlock | electra.SignedBeaconBlock): DataColumnFetchRecord =
+                            blck: electra.SignedBeaconBlock):
+                            DataColumnFetchRecord =
   var indices: seq[ColumnIndex]
   for i in 0..<len(blck.message.body.blob_kzg_commitments):
-    let idx = ColumnIndex(i)
+    let
+      idx = ColumnIndex(i)
+      dc_id = DataColumnIdentifier(
+        block_root: blck.root,
+        index: idx)
     if not quarantine.data_columns.hasKey(
-        (blck.root, idx)):
+        dc_id):
       indices.add(idx)
   DataColumnFetchRecord(block_root: blck.root, indices: indices)
 

beacon_chain/gossip_processing/gossip_validation.nim

@@ -495,8 +495,10 @@ proc validateBlobSidecar*(
 # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.6/specs/_features/eip7594/p2p-interface.md#data_column_sidecar_subnet_id
 proc validateDataColumnSidecar*(
     dag: ChainDAGRef, quarantine: ref Quarantine,
-    dataColumnQuarantine: ref DataColumnQuarantine, data_column_sidecar: DataColumnSidecar,
-    wallTime: BeaconTime, subnet_id: uint64): Result[void, ValidationError] =
+    dataColumnQuarantine: ref DataColumnQuarantine, 
+    data_column_sidecar: DataColumnSidecar,
+    wallTime: BeaconTime, subnet_id: uint64):
+    Result[void, ValidationError] =
 
   template block_header: untyped = data_column_sidecar.signed_block_header.message
 
@@ -510,8 +512,9 @@ proc validateDataColumnSidecar*(
   if not (compute_subnet_for_data_column_sidecar(data_column_sidecar.index) == subnet_id):
     return dag.checkedReject("DataColumnSidecar: The sidecar is not for the correct subnet")
 
-  # [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 
+  # [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).
   if not (block_header.slot <=
       (wallTime + MAXIMUM_GOSSIP_CLOCK_DISPARITY).slotOrZero):

beacon_chain/spec/datatypes/eip7594.nim

@@ -109,3 +109,6 @@ func shortLog*(v: seq[DataColumnSidecar]): auto =
 
 func shortLog*(x: seq[DataColumnIdentifier]): string =
   "[" & x.mapIt(shortLog(it.block_root) & "/" & $it.index).join(", ") & "]"
+
+func shortLog*(x: seq[ColumnIndex]): string =
+  "<" & x.mapIt($it).join(", ") & ">"

beacon_chain/spec/network.nim

@@ -109,7 +109,7 @@ func getBlobSidecarTopic*(forkDigest: ForkDigest,
 func compute_subnet_for_blob_sidecar*(blob_index: BlobIndex): BlobId =
   BlobId(blob_index mod BLOB_SIDECAR_SUBNET_COUNT)
 
-# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.6/specs/_features/eip7594/p2p-interface.md#data_column_sidecar_subnet_id
+# https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.6/specs/_features/eip7594/p2p-interface.md#compute_subnet_for_data_column_sidecar
 func compute_subnet_for_data_column_sidecar*(column_index: ColumnIndex): uint64 =
     uint64(column_index mod DATA_COLUMN_SIDECAR_SUBNET_COUNT)