From 2d5d70d8563534ff4cc3a25c8eba2bd397d854a8 Mon Sep 17 00:00:00 2001
From: Daniel Karzel <daniel@comit.network>
Date: Mon, 1 Feb 2021 18:14:53 +1100
Subject: [PATCH] Timeout for Alice waiting for ack for sending transfer proof

If dialing Bob fails Alice waits for the acknowledgement of the transfer proof indefinitely.
The timout prevents her execution from hanging.
Added a ToDo to re-visit the ack receivers. They don't add value at the moment and should be removed.
---
 swap/src/protocol/alice/event_loop.rs         | 30 +++++++++++++++----
 swap/src/protocol/alice/steps.rs              | 11 +++++--
 swap/src/protocol/alice/swap.rs               |  1 +
 ...refunds_using_cancel_and_refund_command.rs |  8 ++---
 4 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/swap/src/protocol/alice/event_loop.rs b/swap/src/protocol/alice/event_loop.rs
index 26efd22b..b2b3873c 100644
--- a/swap/src/protocol/alice/event_loop.rs
+++ b/swap/src/protocol/alice/event_loop.rs
@@ -1,4 +1,5 @@
 use crate::{
+    execution_params::ExecutionParams,
     network::{transport::SwapTransport, TokioExecutor},
     protocol::{
         alice::{Behaviour, OutEvent, State0, State3, SwapResponse, TransferProof},
@@ -9,7 +10,10 @@ use anyhow::{anyhow, Context, Result};
 use libp2p::{
     core::Multiaddr, futures::FutureExt, request_response::ResponseChannel, PeerId, Swarm,
 };
-use tokio::sync::mpsc::{Receiver, Sender};
+use tokio::{
+    sync::mpsc::{Receiver, Sender},
+    time::timeout,
+};
 use tracing::{error, trace};
 
 #[allow(missing_debug_implementations)]
@@ -91,13 +95,27 @@ impl EventLoopHandle {
         Ok(())
     }
 
-    pub async fn send_transfer_proof(&mut self, bob: PeerId, msg: TransferProof) -> Result<()> {
+    pub async fn send_transfer_proof(
+        &mut self,
+        bob: PeerId,
+        msg: TransferProof,
+        execution_params: ExecutionParams,
+    ) -> Result<()> {
         let _ = self.send_transfer_proof.send((bob, msg)).await?;
 
-        self.recv_transfer_proof_ack
-            .recv()
-            .await
-            .ok_or_else(|| anyhow!("Failed to receive transfer proof ack from Bob"))?;
+        // TODO: Re-evaluate if these acknowledges are necessary at all.
+        // If we don't use a timeout here and Alice fails to dial Bob she will wait
+        // indefinitely for this acknowledge.
+        if timeout(
+            execution_params.bob_time_to_act,
+            self.recv_transfer_proof_ack.recv(),
+        )
+        .await
+        .is_err()
+        {
+            error!("Failed to receive transfer proof ack from Bob")
+        }
+
         Ok(())
     }
 }
diff --git a/swap/src/protocol/alice/steps.rs b/swap/src/protocol/alice/steps.rs
index d07b72d7..8cff4b5e 100644
--- a/swap/src/protocol/alice/steps.rs
+++ b/swap/src/protocol/alice/steps.rs
@@ -96,6 +96,7 @@ pub async fn lock_xmr<W>(
     state3: alice::State3,
     event_loop_handle: &mut EventLoopHandle,
     monero_wallet: Arc<W>,
+    execution_params: ExecutionParams,
 ) -> Result<()>
 where
     W: Transfer,
@@ -117,9 +118,13 @@ where
     //  Otherwise Alice might publish the lock tx twice!
 
     event_loop_handle
-        .send_transfer_proof(bob_peer_id, TransferProof {
-            tx_lock_proof: transfer_proof,
-        })
+        .send_transfer_proof(
+            bob_peer_id,
+            TransferProof {
+                tx_lock_proof: transfer_proof,
+            },
+            execution_params,
+        )
         .await?;
 
     Ok(())
diff --git a/swap/src/protocol/alice/swap.rs b/swap/src/protocol/alice/swap.rs
index beb17e91..1d7d5a79 100644
--- a/swap/src/protocol/alice/swap.rs
+++ b/swap/src/protocol/alice/swap.rs
@@ -165,6 +165,7 @@ async fn run_until_internal(
                     *state3.clone(),
                     &mut event_loop_handle,
                     monero_wallet.clone(),
+                    execution_params,
                 )
                 .await?;
 
diff --git a/swap/tests/bob_refunds_using_cancel_and_refund_command.rs b/swap/tests/bob_refunds_using_cancel_and_refund_command.rs
index 04b33df1..854fbe33 100644
--- a/swap/tests/bob_refunds_using_cancel_and_refund_command.rs
+++ b/swap/tests/bob_refunds_using_cancel_and_refund_command.rs
@@ -10,7 +10,7 @@ async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() {
         let (bob_swap, bob_join_handle) = ctx.new_swap_as_bob().await;
 
         let alice_handle = alice::run(alice_swap);
-        tokio::spawn(alice_handle);
+        let alice_swap_handle = tokio::spawn(alice_handle);
 
         let bob_state = bob::run_until(bob_swap, is_btc_locked).await.unwrap();
 
@@ -57,10 +57,8 @@ async fn given_bob_manually_refunds_after_btc_locked_bob_refunds() {
 
         ctx.assert_bob_refunded(bob_state).await;
 
-        // TODO: Alice hangs indefinitely waiting for Blob's acknowledge on
-        //  sending the transfer proof
-        // let alice_state = alice_swap_handle.await.unwrap().unwrap();
-        // ctx.assert_alice_refunded(alice_state).await;
+        let alice_state = alice_swap_handle.await.unwrap().unwrap();
+        ctx.assert_alice_refunded(alice_state).await;
     })
     .await;
 }