From 3786275181c96065cec2054d51d8d8d22b720260 Mon Sep 17 00:00:00 2001
From: Karl Knutsson <karl.knutsson@iohk.io>
Date: Mon, 30 Mar 2020 09:48:11 +0200
Subject: [PATCH] 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.
---
 .../src/Test/ThreadNet/Network.hs                      |  1 +
 ouroboros-consensus/src/Ouroboros/Consensus/Node.hs    |  2 ++
 .../src/Ouroboros/Consensus/NodeNetwork.hs             |  7 ++++---
 .../src/Ouroboros/Network/Protocol/ChainSync/Codec.hs  | 10 +++++-----
 4 files changed, 12 insertions(+), 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
index 6fcefcb63e7..4c55e787546 100644
--- 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
index 9501059620c..cee3a615033 100644
--- 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
index 44854085f23..6103a8e79fd 100644
--- 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
index f05ec722d9a..7ddd3170545 100644
--- 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.