From 1e42ed7fd7513e505fa58911d956b7eee4bfa529 Mon Sep 17 00:00:00 2001
From: Daniel Karzel <daniel@comit.network>
Date: Thu, 11 Nov 2021 16:51:16 +1100
Subject: [PATCH] Remove order before updating state to `IncomingOrderRequest`

We should remove the order as soon as we know that we will create a cfd out of it.
The (automated) maker reacts on the state change on the feed and may trigger creating a new order.
If we only remove the order at the end the `None` might arrive at the taker after we send the new order.
Furthermore, we should always remove the order independent of accept/reject, so this is done only once upon handling the take request now.

Note that due to this async nature of the application it can still happen that the taker receives `None` after the new `Some(order)`, but this behaviour becomes less likely and the code is generally more correct.
We could also make sending the message to the taker sync, but that might have unwanted, long-blocking side effects.
---
 daemon/src/maker_cfd.rs | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/daemon/src/maker_cfd.rs b/daemon/src/maker_cfd.rs
index 9b82a6e..9f0cf01 100644
--- a/daemon/src/maker_cfd.rs
+++ b/daemon/src/maker_cfd.rs
@@ -442,7 +442,17 @@ where
             }
         };
 
-        // 2. Insert CFD in DB
+        // 2. Remove current order
+        // The order is removed before we update the state, because the maker might react on the
+        // state change. Once we know that we go for either an accept/reject scenario we
+        // have to remove the current order.
+        self.current_order_id = None;
+        self.takers
+            .do_send_async(maker_inc_connections::BroadcastOrder(None))
+            .await?;
+        self.order_feed_sender.send(None)?;
+
+        // 3. Insert CFD in DB
         let cfd = Cfd::new(
             current_order.clone(),
             quantity,
@@ -455,7 +465,9 @@ where
         );
         insert_cfd_and_send_to_feed(&cfd, &mut conn, &self.cfd_feed_actor_inbox).await?;
 
-        // 3. check if order has acceptable amounts
+        // 4. check if order has acceptable amounts and if not reject the cfd
+        // Since rejection is tied to the cfd state at the moment we can only do this after creating
+        // a cfd.
         if quantity < current_order.min_quantity || quantity > current_order.max_quantity {
             tracing::warn!(
                 "Order rejected because quantity {} was out of bounds. It was either <{} or >{}",
@@ -465,17 +477,8 @@ where
             );
 
             self.reject_order(taker_id, cfd, conn).await?;
-
-            return Ok(());
         }
 
-        // 4. Remove current order
-        self.current_order_id = None;
-        self.takers
-            .do_send_async(maker_inc_connections::BroadcastOrder(None))
-            .await?;
-        self.order_feed_sender.send(None)?;
-
         Ok(())
     }
 
@@ -500,11 +503,6 @@ where
         Ok(())
     }
 
-    /// Reject an order
-    ///
-    /// Rejection includes removing the order and saving in the db that it was rejected.
-    /// In the current model it is essential to remove the order because a taker
-    /// that received a rejection cannot communicate with the maker until a new order is published.
     async fn reject_order(
         &mut self,
         taker_id: TakerId,
@@ -522,13 +520,6 @@ where
             })
             .await?;
 
-        // Remove order for all
-        self.current_order_id = None;
-        self.takers
-            .do_send_async(maker_inc_connections::BroadcastOrder(None))
-            .await?;
-        self.order_feed_sender.send(None)?;
-
         Ok(())
     }
 }