From 74ee8bb74d107dacb1edb487ed45c7275beed8bf Mon Sep 17 00:00:00 2001
From: Agnish Ghosh <iamagnix@gmail.com>
Date: Sat, 6 Jul 2024 13:24:51 +0530
Subject: [PATCH] refactor cells and proofs logic + fix edge cases

---
 beacon_chain/spec/eip7594_helpers.nim      | 33 +++++++++++++---------
 beacon_chain/sync/request_manager.nim      |  4 +--
 beacon_chain/validators/message_router.nim |  5 +++-
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/beacon_chain/spec/eip7594_helpers.nim b/beacon_chain/spec/eip7594_helpers.nim
index 483b6443e..79ad86281 100644
--- a/beacon_chain/spec/eip7594_helpers.nim
+++ b/beacon_chain/spec/eip7594_helpers.nim
@@ -214,43 +214,48 @@ proc compute_signed_block_header(signed_block: deneb.SignedBeaconBlock |
 proc get_data_column_sidecars*(signed_block: deneb.SignedBeaconBlock |
                                electra.SignedBeaconBlock, 
                                blobs: seq[KzgBlob]): 
-                               seq[DataColumnSidecar] =
+                               Result[seq[DataColumnSidecar], string] =
   var
     blck = signed_block.message
     signed_beacon_block_header = compute_signed_block_header(signed_block)
     cellsAndProofs: seq[KzgCellsAndKzgProofs]
     kzg_incl_proof: array[4, Eth2Digest]
   
+  if blobs.len == 0:
+    return err ("Blob sequence can't be empty")
+
   for blob in blobs:
     let
-      computed_cell = computeCellsAndProofs(blob)
+      cell_and_proof = computeCellsAndProofs(blob)
 
-    if computed_cell.isErr():
+    if cell_and_proof.isErr():
       return err("EIP7549: Could not compute cells")
 
-    cellsAndProofs.add(computed_cell.get())
+    if cell_and_proof.isOk:
+      cellsAndProofs.add(cell_and_proof.get())
 
   let blobCount = blobs.len
   var
-    cells: seq[seq[Cell]]
-    proofs: seq[seq[KzgProof]]
+    cells = newSeqOfCap[seq[Cell]](blobs.len)
+    proofs = newSeqOfCap[seq[KzgProof]](blobs.len)
 
   for i in 0..<blobCount:
     for j in 0..<int(CELLS_PER_EXT_BLOB):
-      cells[i].add(cellsAndProofs[i].cells[j])
+      cells[i][j] = (cellsAndProofs[i].cells[j])
 
   for i in 0..<blobCount:
-    for j in 0..<int(MAX_BLOB_COMMITMENTS_PER_BLOCK):
-      proofs[i].add(cellsAndProofs[i].proofs[j])
+    for j in 0..<int(CELLS_PER_EXT_BLOB):
+      proofs[i][j] = (cellsAndProofs[i].proofs[j])
 
-  var sidecars = newSeqOfCap[DataColumnSidecar](NUMBER_OF_COLUMNS)
+  var sidecars = newSeqOfCap[DataColumnSidecar](CELLS_PER_EXT_BLOB)
 
-  for columnIndex in 0..<NUMBER_OF_COLUMNS:
+  for columnIndex in 0..<CELLS_PER_EXT_BLOB:
     var column: DataColumn
+    var kzgProofOfColumn: KzgProofs
+    
     for rowIndex in 0..<blobCount:
       column[rowIndex] = cells[rowIndex][columnIndex]
-
-    var kzgProofOfColumn: KzgProofs
+    
     for rowIndex in 0..<blobCount:
       kzgProofOfColumn[rowIndex] = proofs[rowIndex][columnIndex]
 
@@ -264,7 +269,7 @@ proc get_data_column_sidecars*(signed_block: deneb.SignedBeaconBlock |
       27.GeneralizedIndex,
       sidecar.kzg_commitments_inclusion_proof).expect("Valid gindex")
     sidecars.add(sidecar)
-  sidecars
+  ok(sidecars)
 
 # Helper function to `verifyCellKzgProofBatch` at https://github.com/ethereum/c-kzg-4844/blob/das/bindings/nim/kzg_ex.nim#L170
 proc validate_data_column_sidecar*(
diff --git a/beacon_chain/sync/request_manager.nim b/beacon_chain/sync/request_manager.nim
index 8398bf357..9becf5483 100644
--- a/beacon_chain/sync/request_manager.nim
+++ b/beacon_chain/sync/request_manager.nim
@@ -29,10 +29,10 @@ const
   SYNC_MAX_REQUESTED_BLOCKS* = 32 # Spec allows up to MAX_REQUEST_BLOCKS.
     ## Maximum number of blocks which will be requested in each
     ## `beaconBlocksByRoot` invocation.
-  PARALLEL_REQUESTS* = 8
+  PARALLEL_REQUESTS* = 2
     ## Number of peers we using to resolve our request.
   
-  PARALLEL_REQUESTS_DATA_COLUMNS* = 20
+  PARALLEL_REQUESTS_DATA_COLUMNS* = 8
 
   BLOB_GOSSIP_WAIT_TIME_NS* = 2 * 1_000_000_000
     ## How long to wait for blobs to arrive over gossip before fetching.
diff --git a/beacon_chain/validators/message_router.nim b/beacon_chain/validators/message_router.nim
index a970bedfd..9cb9216f5 100644
--- a/beacon_chain/validators/message_router.nim
+++ b/beacon_chain/validators/message_router.nim
@@ -167,7 +167,10 @@ proc routeSignedBeaconBlock*(
   when typeof(blck).kind >= ConsensusFork.Deneb:   
     if blobsOpt.isSome():
       let blobs = blobsOpt.get()
-      let data_columns = get_data_column_sidecars(blck, blobs.mapIt(it.blob))
+      let dataColumnsOpt = get_data_column_sidecars(blck, blobs.mapIt(it.blob))
+      if not dataColumnsOpt.isOk:
+        debug "Issue with computing data column from blob bundle"
+      let data_columns = dataColumnsOpt.get()
       var das_workers = newSeq[Future[SendResult]](len(data_columns))
       for i in 0..<data_columns.lenu64:
         let subnet_id = compute_subnet_for_data_column_sidecar(i)