diff --git a/codex/blockexchange/engine/engine.nim b/codex/blockexchange/engine/engine.nim index 270f6688..73b5442c 100644 --- a/codex/blockexchange/engine/engine.nim +++ b/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 diff --git a/codex/blockexchange/protobuf/blockexc.nim b/codex/blockexchange/protobuf/blockexc.nim index 69868681..a0512cd3 100644 --- a/codex/blockexchange/protobuf/blockexc.nim +++ b/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) diff --git a/codex/blocktype.nim b/codex/blocktype.nim index 7e13493d..233d4e8f 100644 --- a/codex/blocktype.nim +++ b/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 diff --git a/tests/codex/blockexchange/engine/testengine.nim b/tests/codex/blockexchange/engine/testengine.nim index 0541c119..68550735 100644 --- a/tests/codex/blockexchange/engine/testengine.nim +++ b/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 diff --git a/tests/codex/examples.nim b/tests/codex/examples.nim index 52b8a0b8..260adbfc 100644 --- a/tests/codex/examples.nim +++ b/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 = diff --git a/tests/codex/testblocktype.nim b/tests/codex/testblocktype.nim new file mode 100644 index 00000000..b0ea2732 --- /dev/null +++ b/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