update engine tests; add BlockAddress hashing tests

codex/blockexchange/engine/engine.nim

@@ -302,26 +302,42 @@ proc blockPresenceHandler*(
   if peerCtx.isNil:
     return
 
+  trace "Build presence list"
+
   for blk in blocks:
     if presence =? Presence.init(blk):
       peerCtx.setPresence(presence)
 
+  trace "Built presence list"
+
+  trace "Remove dont want cids"
+
   let
     peerHave = peerCtx.peerHave
     dontWantCids = peerHave.filterIt(it notin ourWantList)
 
+  trace "Removed dont want cids"
+
   if dontWantCids.len > 0:
     peerCtx.cleanPresence(dontWantCids)
 
+  trace "Remove want cids"
+
   let ourWantCids = ourWantList.filterIt(
     it in peerHave and not self.pendingBlocks.retriesExhausted(it) and
       not self.pendingBlocks.isInFlight(it)
   )
 
+  trace "Removed want cids"
+
+  trace "Update pending blocks"
+
   for address in ourWantCids:
     self.pendingBlocks.setInFlight(address, true)
     self.pendingBlocks.decRetries(address)
 
+  trace "Updated pending blocks"
+
   if ourWantCids.len > 0:
     trace "Peer has blocks in our wantList", peer, wants = ourWantCids
     # FIXME: this will result in duplicate requests for blocks
@@ -694,6 +710,9 @@ proc taskHandler*(
           continue
         blockDeliveries.add(blockDelivery)
 
+      if blockDeliveries.len == 0:
+        continue
+
       await self.network.request.sendBlocksDelivery(peerCtx.id, blockDeliveries)
       codex_block_exchange_blocks_sent.inc(blockDeliveries.len.int64)
       # Drops the batch from want list. Note that the send might still fail down the line

codex/blockexchange/protobuf/blockexc.nim

@@ -9,7 +9,6 @@
 
 import std/hashes
 import std/sequtils
-import pkg/stew/endians2
 
 import message
 
@@ -20,13 +19,6 @@ export Wantlist, WantType, WantListEntry
 export BlockDelivery, BlockPresenceType, BlockPresence
 export AccountMessage, StateChannelUpdate
 
-proc hash*(a: BlockAddress): Hash =
-  if a.leaf:
-    let data = a.treeCid.data.buffer & @(a.index.uint64.toBytesBE)
-    hash(data)
-  else:
-    hash(a.cid.data.buffer)
-
 proc hash*(e: WantListEntry): Hash =
   hash(e.address)
 

codex/blocktype.nim

@@ -9,6 +9,7 @@
 
 import std/tables
 import std/sugar
+import std/hashes
 
 export tables
 
@@ -18,7 +19,7 @@ push:
   {.upraises: [].}
 
 import pkg/libp2p/[cid, multicodec, multihash]
-import pkg/stew/byteutils
+import pkg/stew/[byteutils, endians2]
 import pkg/questionable
 import pkg/questionable/results
 
@@ -67,6 +68,13 @@ proc `$`*(a: BlockAddress): string =
   else:
     "cid: " & $a.cid
 
+proc hash*(a: BlockAddress): Hash =
+  if a.leaf:
+    let data = a.treeCid.data.buffer & @(a.index.uint64.toBytesBE)
+    hash(data)
+  else:
+    hash(a.cid.data.buffer)
+
 proc cidOrTreeCid*(a: BlockAddress): Cid =
   if a.leaf: a.treeCid else: a.cid
 

tests/codex/blockexchange/engine/testengine.nim

@@ -620,7 +620,8 @@ asyncchecksuite "Task Handler":
     proc sendBlocksDelivery(
         id: PeerId, blocksDelivery: seq[BlockDelivery]
     ) {.async: (raises: [CancelledError]).} =
-      check peersCtx[0].peerWants[0].inFlight
+      let blockAddress = peersCtx[0].peerWants[0].address
+      check peersCtx[0].isInFlight(blockAddress)
 
     for blk in blocks:
       (await engine.localStore.putBlock(blk)).tryGet()
@@ -633,7 +634,6 @@ asyncchecksuite "Task Handler":
         cancel: false,
         wantType: WantType.WantBlock,
         sendDontHave: false,
-        inFlight: false,
       )
     )
     await engine.taskHandler(peersCtx[0])
@@ -646,12 +646,12 @@ asyncchecksuite "Task Handler":
         cancel: false,
         wantType: WantType.WantBlock,
         sendDontHave: false,
-        inFlight: false,
       )
     )
     await engine.taskHandler(peersCtx[0])
 
-    check not peersCtx[0].peerWants[0].inFlight
+    let blockAddress = peersCtx[0].peerWants[0].address
+    check not peersCtx[0].isInFlight(blockAddress)
 
   test "Should send presence":
     let present = blocks

tests/codex/examples.nim

@@ -38,8 +38,7 @@ proc example*(_: type Pricing): Pricing =
   Pricing(address: EthAddress.example, price: uint32.rand.u256)
 
 proc example*(_: type bt.Block, size: int = 4096): bt.Block =
-  let length = rand(size)
-  let bytes = newSeqWith(length, rand(uint8))
+  let bytes = newSeqWith(size, rand(uint8))
   bt.Block.new(bytes).tryGet()
 
 proc example*(_: type PeerId): PeerId =

tests/codex/testblocktype.nim

@@ -0,0 +1,44 @@
+import pkg/unittest2
+import pkg/libp2p/cid
+
+import pkg/codex/blocktype
+
+import ./examples
+
+suite "blocktype":
+  test "should hash equal non-leaf block addresses onto the same hash":
+    let
+      cid1 = Cid.example
+      nonLeaf1 = BlockAddress.init(cid1)
+      nonLeaf2 = BlockAddress.init(cid1)
+
+    check nonLeaf1 == nonLeaf2
+    check nonLeaf1.hash == nonLeaf2.hash
+
+  test "should hash equal leaf block addresses onto the same hash":
+    let
+      cid1 = Cid.example
+      leaf1 = BlockAddress.init(cid1, 0)
+      leaf2 = BlockAddress.init(cid1, 0)
+
+    check leaf1 == leaf2
+    check leaf1.hash == leaf2.hash
+
+  test "should hash different non-leaf block addresses onto different hashes":
+    let
+      cid1 = Cid.example
+      cid2 = Cid.example
+      nonLeaf1 = BlockAddress.init(cid1)
+      nonLeaf2 = BlockAddress.init(cid2)
+
+    check nonLeaf1 != nonLeaf2
+    check nonLeaf1.hash != nonLeaf2.hash
+
+  test "should hash different leaf block addresses onto different hashes":
+    let
+      cid1 = Cid.example
+      leaf1 = BlockAddress.init(cid1, 0)
+      leaf2 = BlockAddress.init(cid1, 1)
+
+    check leaf1 != leaf2
+    check leaf1.hash != leaf2.hash