WIP: Use the same TX fetch preference as V1

Author: karknu

ouroboros-network/lib/Ouroboros/Network/TxSubmission/Inbound/V2/State.hs

@@ -18,6 +18,7 @@ module Ouroboros.Network.TxSubmission.Inbound.V2.State
 
 import Control.Monad.Class.MonadTime.SI (DiffTime, Time, addTime, diffTime)
 import Data.Foldable (foldl', toList)
+import Data.List (sortOn)
 import Data.IntMap.Strict qualified as IntMap
 import Data.IntSet (IntSet)
 import Data.IntSet qualified as IntSet
@@ -373,13 +374,14 @@ pickRequestTxsAction ctx@PeerActionContext { pacNow, pacPolicy, pacPeerState, pa
 
         leaseUntil = addTime (interTxSpace pacPolicy) pacNow
 
-        -- Iterate the peer's unacknowledged queue, which preserves the peer's
-        -- advertisement order. Peers are expected to advertise in chain-
-        -- topological order (parents before children), so walking in that
-        -- order aligns fetch order with submission-validity order and a
-        -- child is never requested ahead of its parent when the same peer
-        -- carries both.
+        -- Order candidates by raw txid bytes so every node in the network
+        -- prioritises the same fetches, regardless of which peer
+        -- advertised them first locally. This mirrors V1's
+        -- 'Map.splitAt' on 'Map txid SizeInBytes' priority and produces
+        -- convergent cross-node propagation. Submission still happens
+        -- in advertisement order via 'pickSubmitAction'.
         candidates =
+          sortOn (\(k, _) -> IntMap.lookup k (sharedKeyToRawTxId pacSharedState))
           [ (k, txSize)
           | TxKey k <- toList (peerUnacknowledgedTxIds pacPeerState)
           , IntSet.notMember k (peerRequestedTxs pacPeerState)
@@ -771,14 +773,15 @@ dropTxKeys :: HasRawTxId txid
            -> SharedTxState peeraddr txid
            -> SharedTxState peeraddr txid
 dropTxKeys keys st@SharedTxState { sharedTxTable, sharedRetainedTxs, sharedTxIdToKey
-                                 , sharedKeyToTxId }
+                                 , sharedKeyToTxId, sharedKeyToRawTxId }
   | IntSet.null keys = st
   | otherwise =
       st {
-        sharedTxTable = IntMap.withoutKeys sharedTxTable keys,
-        sharedRetainedTxs = retainedDeleteKeys keys sharedRetainedTxs,
-        sharedTxIdToKey = IntSet.foldl' deleteTxId sharedTxIdToKey keys,
-        sharedKeyToTxId = IntMap.withoutKeys sharedKeyToTxId keys
+        sharedTxTable      = IntMap.withoutKeys sharedTxTable keys,
+        sharedRetainedTxs  = retainedDeleteKeys keys sharedRetainedTxs,
+        sharedTxIdToKey    = IntSet.foldl' deleteTxId sharedTxIdToKey keys,
+        sharedKeyToTxId    = IntMap.withoutKeys sharedKeyToTxId keys,
+        sharedKeyToRawTxId = IntMap.withoutKeys sharedKeyToRawTxId keys
       }
   where
     deleteTxId txIdToKey k =
@@ -794,12 +797,13 @@ dropLookupOnly :: HasRawTxId txid
                => IntSet.IntSet
                -> SharedTxState peeraddr txid
                -> SharedTxState peeraddr txid
-dropLookupOnly keys st@SharedTxState { sharedTxIdToKey, sharedKeyToTxId }
+dropLookupOnly keys st@SharedTxState { sharedTxIdToKey, sharedKeyToTxId, sharedKeyToRawTxId }
   | IntSet.null keys = st
   | otherwise =
       st {
-        sharedTxIdToKey = IntSet.foldl' deleteTxId sharedTxIdToKey keys,
-        sharedKeyToTxId = IntMap.withoutKeys sharedKeyToTxId keys
+        sharedTxIdToKey    = IntSet.foldl' deleteTxId sharedTxIdToKey keys,
+        sharedKeyToTxId    = IntMap.withoutKeys sharedKeyToTxId keys,
+        sharedKeyToRawTxId = IntMap.withoutKeys sharedKeyToRawTxId keys
       }
   where
     deleteTxId txIdToKey k =
@@ -1308,16 +1312,18 @@ handleReceivedTxIds mempoolHasTx now policy requestedTxIds txidsAndSizes
         sharedChangedAcc' = sharedChangedAcc || txKeyWasNew
         (txKey@(TxKey k), txKeyWasNew, sharedAcc') = lookupOrInternTxId txid sharedAcc
 
-    lookupOrInternTxId txid st@SharedTxState { sharedTxIdToKey, sharedKeyToTxId, sharedNextTxKey }
+    lookupOrInternTxId txid st@SharedTxState { sharedTxIdToKey, sharedKeyToTxId
+                                              , sharedKeyToRawTxId, sharedNextTxKey }
       | Just key <- Map.lookup rawId sharedTxIdToKey = (key, False, st)
       | otherwise =
           let key = TxKey sharedNextTxKey
           in ( key
              , True
              , st {
-                 sharedTxIdToKey = Map.insert rawId key sharedTxIdToKey,
-                 sharedKeyToTxId = IntMap.insert sharedNextTxKey txid sharedKeyToTxId,
-                 sharedNextTxKey = sharedNextTxKey + 1
+                 sharedTxIdToKey    = Map.insert rawId key sharedTxIdToKey,
+                 sharedKeyToTxId    = IntMap.insert sharedNextTxKey txid sharedKeyToTxId,
+                 sharedKeyToRawTxId = IntMap.insert sharedNextTxKey rawId sharedKeyToRawTxId,
+                 sharedNextTxKey    = sharedNextTxKey + 1
                }
              )
       where rawId = getRawTxId txid

ouroboros-network/lib/Ouroboros/Network/TxSubmission/Inbound/V2/Types.hs

@@ -489,10 +489,11 @@ data SharedTxState peeraddr txid = SharedTxState {
     -- | Accepted txs retained locally for a bounded time so later txid
     -- advertisements can be acked without re-requesting the body.
     sharedRetainedTxs :: !RetainedTxs,
-    sharedTxIdToKey   :: !(Map RawTxId TxKey),
-    sharedKeyToTxId   :: !(IntMap txid),
-    sharedNextTxKey   :: !Int,
-    sharedGeneration  :: !Word64
+    sharedTxIdToKey    :: !(Map RawTxId TxKey),
+    sharedKeyToTxId    :: !(IntMap txid),
+    sharedKeyToRawTxId :: !(IntMap RawTxId),
+    sharedNextTxKey    :: !Int,
+    sharedGeneration   :: !Word64
   }
   deriving stock (Eq, Show, Generic)
   deriving anyclass (NFData, NoThunks)
@@ -517,6 +518,7 @@ emptySharedTxState = SharedTxState {
     sharedRetainedTxs      = retainedEmpty,
     sharedTxIdToKey        = Map.empty,
     sharedKeyToTxId        = IntMap.empty,
+    sharedKeyToRawTxId     = IntMap.empty,
     sharedNextTxKey        = 0,
     sharedGeneration       = 0
   }
@@ -635,15 +637,17 @@ internTxId :: HasRawTxId txid
            => txid
            -> SharedTxState peeraddr txid
            -> (RawTxId, TxKey, SharedTxState peeraddr txid)
-internTxId txid st@SharedTxState { sharedTxIdToKey, sharedKeyToTxId, sharedNextTxKey }
+internTxId txid st@SharedTxState { sharedTxIdToKey, sharedKeyToTxId
+                                  , sharedKeyToRawTxId, sharedNextTxKey }
   | Just key <- Map.lookup rawId sharedTxIdToKey = (rawId, key, st)
   | otherwise =
       let key = TxKey sharedNextTxKey in
       ( rawId
          , key
-         , st { sharedTxIdToKey = Map.insert rawId key sharedTxIdToKey
-              , sharedKeyToTxId = IntMap.insert sharedNextTxKey txid sharedKeyToTxId
-              , sharedNextTxKey = sharedNextTxKey + 1
+         , st { sharedTxIdToKey    = Map.insert rawId key sharedTxIdToKey
+              , sharedKeyToTxId    = IntMap.insert sharedNextTxKey txid sharedKeyToTxId
+              , sharedKeyToRawTxId = IntMap.insert sharedNextTxKey rawId sharedKeyToRawTxId
+              , sharedNextTxKey    = sharedNextTxKey + 1
               }
          )
   where

ouroboros-network/tests/lib/Test/Ouroboros/Network/TxSubmission/TxLogic.hs

@@ -32,7 +32,7 @@ import Data.Foldable (foldl', toList)
 import Data.Function (on)
 import Data.IntMap.Strict qualified as IntMap
 import Data.IntSet qualified as IntSet
-import Data.List (elemIndex, mapAccumL, nub, nubBy, sortBy)
+import Data.List (elemIndex, mapAccumL, nub, nubBy, sortBy, sortOn)
 import Data.Map.Strict qualified as Map
 import Data.Maybe (listToMaybe)
 import Data.Sequence.Strict qualified as StrictSeq
@@ -990,9 +990,7 @@ prop_nextPeerAction_picksTxsRespectingBudget (ArbTxDecisionPolicy policy0)
         (IntMap.lookup k (peerAvailableTxIds peerState1))
 
     budget = getSizeInBytes (txsSizeInflightPerPeer policy)
-    isPrefix _ [] = True
-    isPrefix [] _ = False
-    isPrefix (x:xs) (y:ys) = x == y && isPrefix xs ys
+    rawIdOf k = IntMap.lookup k (sharedKeyToRawTxId sharedState1)
   in
     case action of
       PeerRequestTxs picked ->
@@ -1003,8 +1001,11 @@ prop_nextPeerAction_picksTxsRespectingBudget (ArbTxDecisionPolicy policy0)
             -- total at or below the budget.
             tailSize = pickedSize - maybe 0 sizeOf (listToMaybe pickedKeys)
         in conjoin
-             [ counterexample "picked is not a prefix of the unacked queue"
-                 (property (pickedKeys `isPrefix` keyOrder))
+             [ counterexample "picked is not a subset of the unacked queue"
+                 (property (IntSet.fromList pickedKeys
+                              `IntSet.isSubsetOf` IntSet.fromList keyOrder))
+             , counterexample "picked is not in raw-txid order"
+                 (property (pickedKeys == sortOn rawIdOf pickedKeys))
              , counterexample
                  ("tail of picked exceeds budget: " ++ show pickedSize)
                  (property (tailSize <= budget))