Don't wait forever in TokNext TokMustReply

In the (TokNext TokMustReply) state the responder has indicated that it currently doesn't have a new tip for us and we should wait. This patch limits the time we wait to 60s which means that we don't end up waiting forever incase of network problems. This is implemented in the form of a parameter to timeLimitsChainSync in order to get around troublesome testcases in ouroboros-consensus-mock which don't handle any reasonable timeouts.
IntersectMBO · Mar 31, 2020 · c7e7c52 · c7e7c52
1 parent 2118c19
commit c7e7c52
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 6 deletions.
diff --git a/ouroboros-consensus/ouroboros-consensus-test-infra/src/Test/ThreadNet/Network.hs b/ouroboros-consensus/ouroboros-consensus-test-infra/src/Test/ThreadNet/Network.hs
@@ -759,6 +759,7 @@ runThreadNetwork ThreadNetworkArgs
                   -- node
                   nullDebugProtocolTracers
                   (customProtocolCodecs pInfoConfig)
+                  Nothing
                   (protocolHandlers nodeArgs nodeKernel)
 
       -- In practice, a robust wallet/user can persistently add a transaction

diff --git a/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs b/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs
@@ -209,6 +209,7 @@ run tracers protocolTracers chainDbTracer diffusionTracers diffusionArguments
       nodeKernel
       protocolTracers
       (protocolCodecs (getTopLevelConfig nodeKernel) version)
+      (Just 60)
       (protocolHandlers nodeArgs nodeKernel)
 
     mkDiffusionApplications

diff --git a/ouroboros-consensus/src/Ouroboros/Consensus/NodeNetwork.hs b/ouroboros-consensus/src/Ouroboros/Consensus/NodeNetwork.hs
@@ -497,9 +497,10 @@ consensusNetworkApps
     => NodeKernel m peer blk
     -> ProtocolTracers m peer localPeer blk failure
     -> ProtocolCodecs blk failure m bytesCS bytesCS bytesBF bytesBF bytesTX bytesLCS bytesLTX bytesLSQ
+    -> Maybe DiffTime -- Workaround for ouroboros-consensus-mock test cases that can't cope with a timeouts
     -> ProtocolHandlers m peer blk
     -> NetworkApplication m peer localPeer bytesCS bytesBF bytesTX bytesLCS bytesLTX bytesLSQ ()
-consensusNetworkApps kernel ProtocolTracers {..} ProtocolCodecs {..} ProtocolHandlers {..} =
+consensusNetworkApps kernel ProtocolTracers {..} ProtocolCodecs {..} chainSyncTimeout ProtocolHandlers {..} =
     NetworkApplication {
       naChainSyncClient,
       naChainSyncServer,
@@ -534,7 +535,7 @@ consensusNetworkApps kernel ProtocolTracers {..} ProtocolCodecs {..} ProtocolHan
           (contramap (TraceLabelPeer them) ptChainSyncTracer)
           pcChainSyncCodec
           (byteLimitsChainSync (const 0)) -- TODO: Real Bytelimits, see #1727
-          timeLimitsChainSync
+          (timeLimitsChainSync chainSyncTimeout)
           channel
           $ chainSyncClientPeerPipelined
           $ phChainSyncClient varCandidate
@@ -548,7 +549,7 @@ consensusNetworkApps kernel ProtocolTracers {..} ProtocolCodecs {..} ProtocolHan
         (contramap (TraceLabelPeer them) ptChainSyncSerialisedTracer)
         pcChainSyncCodecSerialised
         (byteLimitsChainSync (const 0)) -- TODO: Real Bytelimits, see #1727
-        timeLimitsChainSync
+        (timeLimitsChainSync chainSyncTimeout)
         channel
         $ chainSyncServerPeer
         $ phChainSyncServer registry

diff --git a/ouroboros-network/src/Ouroboros/Network/Protocol/ChainSync/Codec.hs b/ouroboros-network/src/Ouroboros/Network/Protocol/ChainSync/Codec.hs
@@ -52,14 +52,14 @@ byteLimitsChainSync = ProtocolSizeLimits stateToLimit
 -- `TokNext TokCanAwait` `longWait` timeout
 -- `TokNext TokMustReply` No timeout
 -- `TokIntersect` `longWait timeout
-timeLimitsChainSync :: ProtocolTimeLimits (ChainSync header tip)
-timeLimitsChainSync = ProtocolTimeLimits stateToLimit
+timeLimitsChainSync :: Maybe DiffTime -> ProtocolTimeLimits (ChainSync header tip)
+timeLimitsChainSync consensusTimeout = ProtocolTimeLimits stateToLimit
   where
     stateToLimit :: forall (pr :: PeerRole) (st :: ChainSync header tip).
                     PeerHasAgency pr st -> Maybe DiffTime
     stateToLimit (ClientAgency TokIdle)                = waitForever
     stateToLimit (ServerAgency (TokNext TokCanAwait))  = shortWait
-    stateToLimit (ServerAgency (TokNext TokMustReply)) = waitForever
+    stateToLimit (ServerAgency (TokNext TokMustReply)) = consensusTimeout
     stateToLimit (ServerAgency TokIntersect)           = shortWait
 
 -- | The main CBOR 'Codec' for the 'ChainSync' protocol.