Skip to content

Commit 2ee117b

Browse files
cootcoot
committed
tx-submission: verify tx sizes
1 parent 33424d9 commit 2ee117b

File tree

8 files changed

+170
-69
lines changed

8 files changed

+170
-69
lines changed

‎ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/Diffusion/Node/MiniProtocols.hs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ import Ouroboros.Network.TxSubmission.Inbound.Types (TraceTxLogic,
109109
TraceTxSubmissionInbound)
110110
import Ouroboros.Network.TxSubmission.Outbound (txSubmissionOutbound)
111111
import Test.Ouroboros.Network.Diffusion.Node.NodeKernel
112-
import Test.Ouroboros.Network.TxSubmission.Types (Mempool, Tx, getMempoolReader,
113-
getMempoolWriter, txSubmissionCodec2)
112+
import Test.Ouroboros.Network.TxSubmission.Types (Mempool, Tx (..),
113+
getMempoolReader, getMempoolWriter, txSubmissionCodec2)
114114

115115

116116
-- | Protocol codecs.
@@ -684,6 +684,7 @@ applications debugTracer txSubmissionInboundTracer txSubmissionInboundDebug node
684684
txChannelsVar
685685
sharedTxStateVar
686686
(getMempoolReader mempool)
687+
getTxSize
687688
them $ \api -> do
688689
let server = txSubmissionInboundV2
689690
txSubmissionInboundTracer

‎ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/TxSubmission/AppV2.hs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,7 @@ runTxSubmission tracer tracerTxLogic state txDecisionPolicy = do
214214
txChannelsVar
215215
sharedTxStateVar
216216
(getMempoolReader inboundMempool)
217+
getTxSize
217218
addr $ \api -> do
218219
let server = txSubmissionInboundV2 verboseTracer
219220
(getMempoolWriter inboundMempool)

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

Lines changed: 63 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import Data.Monoid (Sum (..))
3535
import Data.Sequence.Strict qualified as StrictSeq
3636
import Data.Set (Set)
3737
import Data.Set qualified as Set
38+
import Data.Typeable
3839

3940
import NoThunks.Class
4041

@@ -46,6 +47,7 @@ import Ouroboros.Network.TxSubmission.Inbound.Policy
4647
import Ouroboros.Network.TxSubmission.Inbound.State (PeerTxState (..),
4748
SharedTxState (..))
4849
import Ouroboros.Network.TxSubmission.Inbound.State qualified as TXS
50+
import Ouroboros.Network.TxSubmission.Inbound.Types qualified as TXS
4951

5052
import Test.Ouroboros.Network.BlockFetch (PeerGSVT (..))
5153
import Test.Ouroboros.Network.TxSubmission.Types
@@ -306,7 +308,7 @@ mkArbPeerTxState mempoolHasTxFun txIdsInflight unacked txMaskMap =
306308
where
307309
mempoolHasTx = apply mempoolHasTxFun
308310
availableTxIds = Map.fromList
309-
[ (txid, getTxSize tx) | (txid, TxAvailable tx _) <- Map.assocs txMaskMap
311+
[ (txid, getTxAdvSize tx) | (txid, TxAvailable tx _) <- Map.assocs txMaskMap
310312
, not (mempoolHasTx txid)
311313
]
312314
unknownTxs = Set.fromList
@@ -315,7 +317,7 @@ mkArbPeerTxState mempoolHasTxFun txIdsInflight unacked txMaskMap =
315317
]
316318

317319
requestedTxIdsInflight = fromIntegral txIdsInflight
318-
requestedTxsInflightSize = foldMap getTxSize inflightMap
320+
requestedTxsInflightSize = foldMap getTxAdvSize inflightMap
319321
requestedTxsInflight = Map.keysSet inflightMap
320322

321323
-- exclude `txid`s which are already in the mempool, we never request such
@@ -759,12 +761,20 @@ instance Arbitrary ArbCollectTxs where
759761

760762
receivedTx <- sublistOf requestedTxIds'
761763
>>= traverse (\txid -> do
764+
-- real size, which might be different from
765+
-- the advertised size
766+
size <- frequency [ (9, pure (availableTxIds Map.! txid))
767+
, (1, chooseEnum (0, maxTxSize))
768+
]
769+
762770
valid <- frequency [(4, pure True), (1, pure False)]
763-
pure $ Tx { getTxId = txid,
764-
getTxSize = availableTxIds Map.! txid,
765-
getTxValid = valid })
771+
pure $ Tx { getTxId = txid,
772+
getTxSize = size,
773+
-- `availableTxIds` contains advertised sizes
774+
getTxAdvSize = availableTxIds Map.! txid,
775+
getTxValid = valid })
766776

767-
pure $ assert (foldMap getTxSize receivedTx <= requestedTxsInflightSize)
777+
pure $ assert (foldMap getTxAdvSize receivedTx <= requestedTxsInflightSize)
768778
$ ArbCollectTxs mempoolHasTxFun
769779
(Set.fromList requestedTxIds')
770780
(Map.fromList [ (getTxId tx, tx) | tx <- receivedTx ])
@@ -856,24 +866,49 @@ prop_collectTxsImpl (ArbCollectTxs _mempoolHasTxFun txidsRequested txsReceived p
856866
label ("number of txids inflight " ++ labelInt 25 5 (Map.size $ inflightTxs st)) $
857867
label ("number of txids requested " ++ labelInt 25 5 (Set.size txidsRequested)) $
858868
label ("number of txids received " ++ labelInt 10 2 (Map.size txsReceived)) $
859-
860-
-- InboundState invariant
861-
counterexample
862-
( "InboundState invariant violation:\n" ++ show st' ++ "\n"
863-
++ show ps'
864-
)
865-
(sharedTxStateInvariant st')
866-
867-
.&&.
868-
-- `collectTxsImpl` doesn't modify unacknowledged TxId's
869-
counterexample "acknowledged property violation"
870-
( let unacked = toList $ unacknowledgedTxIds ps
871-
unacked' = toList $ unacknowledgedTxIds ps'
872-
in unacked === unacked'
873-
)
869+
label ("hasTxSizeError " ++ show hasTxSizeErr) $
870+
871+
case TXS.collectTxsImpl getTxSize peeraddr txidsRequested txsReceived st of
872+
Right st' | not hasTxSizeErr ->
873+
let ps' = peerTxStates st' Map.! peeraddr in
874+
-- InboundState invariant
875+
counterexample
876+
( "InboundState invariant violation:\n" ++ show st' ++ "\n"
877+
++ show ps'
878+
)
879+
(sharedTxStateInvariant st')
880+
881+
.&&.
882+
-- `collectTxsImpl` doesn't modify unacknowledged TxId's
883+
counterexample "acknowledged property violation"
884+
( let unacked = toList $ unacknowledgedTxIds ps
885+
unacked' = toList $ unacknowledgedTxIds ps'
886+
in unacked === unacked'
887+
)
888+
889+
Right _ ->
890+
counterexample "collectTxsImpl should return Left"
891+
. counterexample (show txsReceived)
892+
$ False
893+
Left _ | not hasTxSizeErr ->
894+
counterexample "collectTxsImpl should return Right" False
895+
896+
Left (TXS.ProtocolErrorTxSizeError as) ->
897+
counterexample (show as)
898+
$ Set.fromList ((\(txid, _, _) -> coerceTxId txid) `map` as)
899+
===
900+
Map.keysSet (Map.filter (\tx -> getTxSize tx /= getTxAdvSize tx) txsReceived)
901+
Left e ->
902+
counterexample ("unexpected error: " ++ show e) False
874903
where
875-
st' = TXS.collectTxsImpl peeraddr txidsRequested txsReceived st
876-
ps' = peerTxStates st' Map.! peeraddr
904+
hasTxSizeErr = any (\tx -> getTxSize tx /= getTxAdvSize tx) txsReceived
905+
906+
-- The `ProtocolErrorTxSizeError` type is an existential type. We know that
907+
-- the type of `txid` is `TxId`, we just don't have evidence for it.
908+
coerceTxId :: Typeable txid => txid -> TxId
909+
coerceTxId txid = case cast txid of
910+
Just a -> a
911+
Nothing -> error "impossible happened! Is the test still using `TxId` for `txid`?"
877912

878913

879914
-- | Verify that `SharedTxState` returned by `collectTxsImpl` if evaluated to
@@ -883,11 +918,11 @@ prop_collectTxsImpl_nothunks
883918
:: ArbCollectTxs
884919
-> Property
885920
prop_collectTxsImpl_nothunks (ArbCollectTxs _mempoolHasTxFun txidsRequested txsReceived peeraddr _ st) =
886-
case unsafeNoThunks $! st' of
887-
Nothing -> property True
888-
Just ctx -> counterexample (show ctx) False
889-
where
890-
st' = TXS.collectTxsImpl peeraddr txidsRequested txsReceived st
921+
case TXS.collectTxsImpl getTxSize peeraddr txidsRequested txsReceived st of
922+
Right st' -> case unsafeNoThunks $! st' of
923+
Nothing -> property True
924+
Just ctx -> counterexample (show ctx) False
925+
Left _ -> property True
891926

892927

893928
newtype ArbTxDecisionPolicy = ArbTxDecisionPolicy TxDecisionPolicy

‎ouroboros-network/sim-tests-lib/Test/Ouroboros/Network/TxSubmission/Types.hs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,13 @@ import Text.Printf
5555

5656

5757
data Tx txid = Tx {
58-
getTxId :: !txid,
59-
getTxSize :: !SizeInBytes,
58+
getTxId :: !txid,
59+
getTxSize :: !SizeInBytes,
60+
getTxAdvSize :: !SizeInBytes,
6061
-- | If false this means that when this tx will be submitted to a remote
6162
-- mempool it will not be valid. The outbound mempool might contain
6263
-- invalid tx's in this sense.
63-
getTxValid :: !Bool
64+
getTxValid :: !Bool
6465
}
6566
deriving (Eq, Ord, Show, Generic)
6667

@@ -69,13 +70,17 @@ instance ShowProxy txid => ShowProxy (Tx txid) where
6970
showProxy _ = "Tx " ++ showProxy (Proxy :: Proxy txid)
7071

7172
instance Arbitrary txid => Arbitrary (Tx txid) where
72-
arbitrary =
73+
arbitrary = do
74+
-- note:
75+
-- generating small tx sizes avoids overflow error when semigroup
76+
-- instance of `SizeInBytes` is used (summing up all inflight tx
77+
-- sizes).
78+
(size, advSize) <- frequency [ (9, (\a -> (a,a)) <$> chooseEnum (0, maxTxSize))
79+
, (1, (,) <$> chooseEnum (0, maxTxSize) <*> chooseEnum (0, maxTxSize))
80+
]
7381
Tx <$> arbitrary
74-
<*> chooseEnum (0, maxTxSize)
75-
-- note:
76-
-- generating small tx sizes avoids overflow error when semigroup
77-
-- instance of `SizeInBytes` is used (summing up all inflight tx
78-
-- sizes).
82+
<*> pure size
83+
<*> pure advSize
7984
<*> frequency [ (3, pure True)
8085
, (1, pure False)
8186
]
@@ -167,15 +172,17 @@ txSubmissionCodec2 =
167172
codecTxSubmission2 CBOR.encodeInt CBOR.decodeInt
168173
encodeTx decodeTx
169174
where
170-
encodeTx Tx {getTxId, getTxSize, getTxValid} =
171-
CBOR.encodeListLen 3
175+
encodeTx Tx {getTxId, getTxSize, getTxAdvSize, getTxValid} =
176+
CBOR.encodeListLen 4
172177
<> CBOR.encodeInt getTxId
173178
<> CBOR.encodeWord32 (getSizeInBytes getTxSize)
179+
<> CBOR.encodeWord32 (getSizeInBytes getTxAdvSize)
174180
<> CBOR.encodeBool getTxValid
175181

176182
decodeTx = do
177183
_ <- CBOR.decodeListLen
178184
Tx <$> CBOR.decodeInt
185+
<*> (SizeInBytes <$> CBOR.decodeWord32)
179186
<*> (SizeInBytes <$> CBOR.decodeWord32)
180187
<*> CBOR.decodeBool
181188

‎ouroboros-network/src/Ouroboros/Network/TxSubmission/Inbound/Registry.hs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import Data.Foldable (traverse_
2727
, foldl'
2828
#endif
2929
)
30+
import Data.Typeable (Typeable)
3031
import Data.Map.Strict (Map)
3132
import Data.Map.Strict qualified as Map
3233
import Data.Maybe (fromMaybe)
@@ -75,7 +76,7 @@ data PeerTxAPI m txid tx = PeerTxAPI {
7576
-- ^ requested txids
7677
-> Map txid tx
7778
-- ^ received txs
78-
-> m ()
79+
-> m (Maybe TxSubmissionProtocolError)
7980
-- ^ handle received txs
8081
}
8182

@@ -90,13 +91,16 @@ withPeer
9091
, MonadMVar m
9192
, MonadSTM m
9293
, Ord txid
94+
, Typeable txid
95+
, Show txid
9396
, Ord peeraddr
9497
, Show peeraddr
9598
)
9699
=> Tracer m (TraceTxLogic peeraddr txid tx)
97100
-> TxChannelsVar m peeraddr txid tx
98101
-> SharedTxStateVar m peeraddr txid tx
99102
-> TxSubmissionMempoolReader txid tx idx m
103+
-> (tx -> SizeInBytes)
100104
-> peeraddr
101105
-- ^ new peer
102106
-> (PeerTxAPI m txid tx -> m a)
@@ -106,6 +110,7 @@ withPeer tracer
106110
channelsVar
107111
sharedStateVar
108112
TxSubmissionMempoolReader { mempoolGetSnapshot }
113+
txSize
109114
peeraddr io =
110115
bracket
111116
(do -- create a communication channel
@@ -209,9 +214,9 @@ withPeer tracer
209214
-- ^ requested txids
210215
-> Map txid tx
211216
-- ^ received txs
212-
-> m ()
217+
-> m (Maybe TxSubmissionProtocolError)
213218
handleReceivedTxs txids txs =
214-
collectTxs tracer sharedStateVar peeraddr txids txs
219+
collectTxs tracer txSize sharedStateVar peeraddr txids txs
215220

216221

217222
decisionLogicThread

‎ouroboros-network/src/Ouroboros/Network/TxSubmission/Inbound/Server.hs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,9 @@ txSubmissionInboundV2
182182

183183
unless (Map.keysSet received `Set.isSubsetOf` requested) $
184184
throwIO ProtocolErrorTxNotRequested
185-
-- TODO: all sizes of txs which were announced earlier with
186-
-- `MsgReplyTxIds` must be verified.
187185

188-
handleReceivedTxs requested received
189-
k
186+
mbe <- handleReceivedTxs requested received
187+
case mbe of
188+
-- one of `tx`s had a wrong size
189+
Just e -> throwIO e
190+
Nothing -> k

0 commit comments

Comments
 (0)