supernodes to da on NUMBER_OF_COLUMNS div 2

beacon_chain/consensus_object_pools/blob_quarantine.nim

@@ -469,7 +469,7 @@ func hasSidecars*(
     supernode = (len(quarantine.custodyColumns) == NUMBER_OF_COLUMNS)
     columnsCount =
       if supernode:
-        (NUMBER_OF_COLUMNS div 2 + 1)
+        NUMBER_OF_COLUMNS div 2
       else:
         len(quarantine.custodyColumns)
 
@@ -555,7 +555,7 @@ proc popSidecars*(
     supernode = (len(quarantine.custodyColumns) == NUMBER_OF_COLUMNS)
     columnsCount =
       if supernode:
-        (NUMBER_OF_COLUMNS div 2 + 1)
+        NUMBER_OF_COLUMNS div 2
       else:
         len(quarantine.custodyColumns)
 
@@ -576,10 +576,10 @@ proc popSidecars*(
           "Record should only have loaded values, but it is `" &
             $sidecar.kind & "`")
         sidecars.add(sidecar.data)
-      if len(sidecars) >= (NUMBER_OF_COLUMNS div 2 + 1):
+      if len(sidecars) >= NUMBER_OF_COLUMNS div 2:
         break
 
-    doAssert(len(sidecars) >= (NUMBER_OF_COLUMNS div 2 + 1),
+    doAssert(len(sidecars) >= NUMBER_OF_COLUMNS div 2,
       "Incorrect amount of sidecars in record for supernode - " &
         $len(sidecars))
   else:
@@ -689,21 +689,21 @@ func fetchMissingSidecars*(
   if supernode:
     if len(record.sidecars) == 0:
       for column in peerMap.items():
-        if len(res) > columnsCount:
+        if len(res) >= columnsCount:
           # We don't need to request more than (NUMBER_OF_COLUMNS div 2)
           # columns.
           break
         res.incl(column)
     else:
-      if record.count > columnsCount:
+      if record.count >= columnsCount:
         # We already have enough columns for reconstruction.
         return
           DataColumnsByRootIdentifier(
             block_root: blockRoot,
             indices: DataColumnIndices(default(seq[ColumnIndex])))
 
       for column in peerMap.items():
-        if record.count + len(res) > columnsCount:
+        if record.count + len(res) >= columnsCount:
           # We don't need to request more than (NUMBER_OF_COLUMNS div 2)
           # columns.
           break

beacon_chain/spec/column_map.nim

@@ -97,6 +97,6 @@ func `$`*(a: ColumnMap): string =
   "[" & a.items().toSeq().mapIt($it).join(", ") & "]"
 
 func shortLog*(a: ColumnMap): string =
-  if len(a) > NUMBER_OF_COLUMNS div 2:
+  if len(a) >= NUMBER_OF_COLUMNS div 2:
     return "[supernode]"
   $a

tests/test_column_map.nim

@@ -103,7 +103,7 @@ suite "ColumnMap test suite":
           vector[2]
 
   test "supernode test":
-    for max in ((NUMBER_OF_COLUMNS div 2) + 1) ..< NUMBER_OF_COLUMNS:
+    for max in (NUMBER_OF_COLUMNS div 2) ..< NUMBER_OF_COLUMNS:
       var columns: seq[ColumnIndex]
       for i in 0 ..< max:
         columns.add(ColumnIndex(i))

tests/test_quarantine.nim

@@ -1315,14 +1315,14 @@ suite "ColumnQuarantine data structure test suite " & preset():
       sidecars1 =
         block:
           var res: seq[ref fulu.DataColumnSidecar]
-          for i in 0 ..< (len(custodyColumns) div 2 + 1):
+          for i in 0 ..< (len(custodyColumns) div 2):
             res.add(newClone(genDataColumnSidecar(
               index = int(custodyColumns[i]), slot = 1, proposer_index = 5)))
           res
       sidecars2 =
         block:
           var res: seq[ref fulu.DataColumnSidecar]
-          for i in 0 ..< (len(custodyColumns) div 2 + 1):
+          for i in 0 ..< (len(custodyColumns) div 2):
             res.add(newClone(genDataColumnSidecar(
               index = int(custodyColumns[i]), slot = 1, proposer_index = 6)))
           res
@@ -1493,14 +1493,14 @@ suite "ColumnQuarantine data structure test suite " & preset():
       sidecars1 =
         block:
           var res: seq[ref fulu.DataColumnSidecar]
-          for i in 0 ..< (len(custodyColumns) div 2 + 1):
+          for i in 0 ..< (len(custodyColumns) div 2):
             res.add(newClone(genDataColumnSidecar(
               index = int(custodyColumns[i]), slot = 1, proposer_index = 5)))
           res
       sidecars2 =
         block:
           var res: seq[ref fulu.DataColumnSidecar]
-          for i in 0 ..< (len(custodyColumns) div 2 + 1):
+          for i in 0 ..< (len(custodyColumns) div 2):
             res.add(newClone(genDataColumnSidecar(
               index = int(custodyColumns[i]), slot = 2, proposer_index = 50)))
           res
@@ -1513,59 +1513,44 @@ suite "ColumnQuarantine data structure test suite " & preset():
       fuluBlock1 = genFuluSignedBeaconBlock(broot1, commitments1)
       fuluBlock2 = genFuluSignedBeaconBlock(broot2, commitments2)
 
-    func checkSupernodeExpected(
-      root: Eth2Digest,
-      index: int,
-      missing: DataColumnsByRootIdentifier
-    ): bool =
-      const ExpectedVectors = [
-        (@[63, 64, 65, 66, 95, 96, 97, 98], 0 .. 57),
-        (@[63, 64, 65, 66, 95, 96, 97], 58 .. 58),
-        (@[63, 64, 65, 66, 95, 96], 59 .. 59),
-        (@[63, 64, 65, 66, 95], 60 .. 60),
-        (@[63, 64, 65, 66], 61 .. 61),
-        (@[63, 64, 65], 62 .. 62),
-        (@[63, 64], 63 .. 63),
-        (@[64], 64 .. 64),
-        (@[], 65 .. 65)
-      ]
-
-      doAssert(index in 0 .. 65)
-      for expect in ExpectedVectors:
-        if index in expect[1]:
-          if len(expect[0]) != len(missing.indices):
-            return false
-          for i in 0 ..< len(missing.indices):
-            if missing.block_root != root:
-              return false
-            if (int(missing.indices[i]) != expect[0][i]):
-              return false
-          return true
-      false
+    func expectedFor(i: int): seq[int] =
+      let peer = @[63,64,65,66,95,96,97,98]
+      if i < 56:
+        return peer
+      let want = 64 - i
+      result = peer[0 ..< min(want, peer.len)]
+
+    func checkSupernodeExpected(root: Eth2Digest, i: int, missing: DataColumnsByRootIdentifier): bool =
+      let exp = expectedFor(i)
+      if exp.len != missing.indices.len: return false
+      if missing.block_root != root: return false
+      for j in 0 ..< exp.len:
+        if int(missing.indices[j]) != exp[j]: return false
+      return true
 
-    for i in 0 ..< len(sidecars1) + 1:
+    for i in 0 ..< len(sidecars1):
       let
         missing1 = bq.fetchMissingSidecars(broot1, fuluBlock1)
         missing2 = bq.fetchMissingSidecars(broot2, fuluBlock2)
-        missing3 =
-          bq.fetchMissingSidecars(broot1, fuluBlock1, peerCustodyColumns1)
+        missing3 = bq.fetchMissingSidecars(broot1, fuluBlock1, peerCustodyColumns1)
+
       check:
         compareSidecars(
           broot1,
-          sidecars1.toOpenArray(i, len(sidecars1) - 1), missing1) == true
+          sidecars1.toOpenArray(i, len(sidecars1) - 1),
+          missing1
+        ) == true
+
         compareSidecars(
           broot2,
-          sidecars2.toOpenArray(i, len(sidecars2) - 1), missing2) == true
-        checkSupernodeExpected(
-          broot1,
-          i, missing3) == true
+          sidecars2.toOpenArray(i, len(sidecars2) - 1),
+          missing2
+        ) == true
 
-      if i >= len(sidecars1):
-        break
+        checkSupernodeExpected(broot1, i, missing3) == true
 
       bq.put(broot1, sidecars1[i])
       bq.put(broot2, sidecars2[i])
-
     bq.remove(broot1)
     bq.remove(broot2)
     check len(bq) == 0