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 70s 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 · 3786275 · 3786275
1 parent 2118c19
commit 3786275
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 8 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,8 @@ run tracers protocolTracers chainDbTracer diffusionTracers diffusionArguments
       nodeKernel
       protocolTracers
       (protocolCodecs (getTopLevelConfig nodeKernel) version)
+      (Just 70) -- timeout after waiting this long for the next header
+                -- 70s allows for 3 slots (3 * 20s)
       (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 #1882, test cases that can't cope with 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
@@ -50,16 +50,16 @@ byteLimitsChainSync = ProtocolSizeLimits stateToLimit
 --
 -- `TokIdle`  No timeout
 -- `TokNext TokCanAwait` `longWait` timeout
--- `TokNext TokMustReply` No timeout
--- `TokIntersect` `longWait timeout
-timeLimitsChainSync :: ProtocolTimeLimits (ChainSync header tip)
-timeLimitsChainSync = ProtocolTimeLimits stateToLimit
+-- `TokNext TokMustReply` consensusTimeout timeout
+-- `TokIntersect` `longWait` timeout
+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.