From ed3e91cbde620640e240b7f2aa830e23478e2378 Mon Sep 17 00:00:00 2001
From: benbierens <thatbenbierens@gmail.com>
Date: Thu, 5 Dec 2024 10:00:20 +0100
Subject: [PATCH] Review comments by Dmitriy

---
 codex/blockexchange/engine/engine.nim      | 64 ++++++++++++----------
 codex/blockexchange/peers/peerctxstore.nim |  2 +-
 2 files changed, 36 insertions(+), 30 deletions(-)

diff --git a/codex/blockexchange/engine/engine.nim b/codex/blockexchange/engine/engine.nim
index 16b2169b..63fb2dca 100644
--- a/codex/blockexchange/engine/engine.nim
+++ b/codex/blockexchange/engine/engine.nim
@@ -194,7 +194,7 @@ proc requestBlock*(
       asyncSpawn b.monitorBlockHandle(blockFuture, address, selected.id)
       b.pendingBlocks.setInFlight(address)
       await b.sendWantBlock(@[address], selected)
-      
+
     await b.sendWantHave(@[address], peers.without)
 
   # Don't let timeouts bubble up. We can't be too broad here or we break
@@ -399,33 +399,15 @@ proc wantListHandler*(
       address   = e.address
       wantType  = $e.wantType
 
+    # Update metrics
+    if e.wantType == WantType.WantHave:
+      codex_block_exchange_want_have_lists_received.inc()
+    elif e.wantType == WantType.WantBlock:
+      codex_block_exchange_want_block_lists_received.inc()
+
+    # Update peerCtx
     if idx < 0: # new entry
       peerCtx.peerWants.add(e)
-
-      let
-        have = await e.address in b.localStore
-        price = @(
-          b.pricing.get(Pricing(price: 0.u256))
-          .price.toBytesBE)
-
-      if e.wantType == WantType.WantHave:
-        codex_block_exchange_want_have_lists_received.inc()
-      elif e.wantType == WantType.WantBlock:
-        codex_block_exchange_want_block_lists_received.inc()
-
-      if not have and e.sendDontHave:
-        presence.add(
-          BlockPresence(
-          address: e.address,
-          `type`: BlockPresenceType.DontHave,
-          price: price))
-      elif have and e.wantType == WantType.WantHave:
-        presence.add(
-          BlockPresence(
-          address: e.address,
-          `type`: BlockPresenceType.Have,
-          price: price))
-      
     else: # update existing entry
       # peer doesn't want this block anymore
       if e.cancel:
@@ -435,13 +417,37 @@ proc wantListHandler*(
         # different want params
         peerCtx.peerWants[idx] = e # update entry
 
+    if not e.cancel:
+      # Respond to wantHaves immediately
+      if e.wantType == WantType.WantHave:
+        let
+          have = await e.address in b.localStore
+          price = @(
+            b.pricing.get(Pricing(price: 0.u256))
+            .price.toBytesBE)
+
+        if not have and e.sendDontHave:
+          presence.add(
+            BlockPresence(
+            address: e.address,
+            `type`: BlockPresenceType.DontHave,
+            price: price))
+        elif have:
+          presence.add(
+            BlockPresence(
+            address: e.address,
+            `type`: BlockPresenceType.Have,
+            price: price))
+
+      # Schedule handling of wantBlocks
+      if e.wantType == WantType.WantBlock:
+        if not b.scheduleTask(peerCtx):
+          warn "Unable to schedule task for peer", peer
+
   if presence.len > 0:
     trace "Sending presence to remote", items = presence.mapIt($it).join(",")
     await b.network.request.sendPresence(peer, presence)
 
-  if not b.scheduleTask(peerCtx):
-    warn "Unable to schedule task for peer", peer
-
 proc accountHandler*(
   engine: BlockExcEngine,
   peer: PeerId,
diff --git a/codex/blockexchange/peers/peerctxstore.nim b/codex/blockexchange/peers/peerctxstore.nim
index 14f332e7..4b65d849 100644
--- a/codex/blockexchange/peers/peerctxstore.nim
+++ b/codex/blockexchange/peers/peerctxstore.nim
@@ -32,7 +32,7 @@ logScope:
 type
   PeerCtxStore* = ref object of RootObj
     peers*: OrderedTable[PeerId, BlockExcPeerCtx]
-  PeersForBlock* = ref object of RootObj
+  PeersForBlock* = object of RootObj
     with*: seq[BlockExcPeerCtx]
     without*: seq[BlockExcPeerCtx]