From 9ed32000ab181f3dfdca4658b915eb2e50ad1d7a Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 11 Nov 2021 16:43:44 +1100 Subject: [PATCH 1/2] Rename `insert_cfd` to `insert_cfd_and_send_to_feed` `insert_cfd` gives the wrong impression, this function does more than that and the name should show it. --- daemon/src/cfd_actors.rs | 2 +- daemon/src/db.rs | 4 ++-- daemon/src/maker_cfd.rs | 4 ++-- daemon/src/taker_cfd.rs | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/daemon/src/cfd_actors.rs b/daemon/src/cfd_actors.rs index 111c938..820504a 100644 --- a/daemon/src/cfd_actors.rs +++ b/daemon/src/cfd_actors.rs @@ -6,7 +6,7 @@ use sqlx::pool::PoolConnection; use sqlx::Sqlite; use tokio::sync::watch; -pub async fn insert_cfd( +pub async fn insert_cfd_and_send_to_feed( cfd: &Cfd, conn: &mut PoolConnection, update_sender: &watch::Sender>, diff --git a/daemon/src/db.rs b/daemon/src/db.rs index 4b6033c..b31ded7 100644 --- a/daemon/src/db.rs +++ b/daemon/src/db.rs @@ -569,7 +569,7 @@ mod tests { let cfd_1 = Cfd::dummy(); db::insert_order(&cfd_1.order, &mut conn).await.unwrap(); - cfd_actors::insert_cfd(&cfd_1, &mut conn, &cfd_feed_sender) + cfd_actors::insert_cfd_and_send_to_feed(&cfd_1, &mut conn, &cfd_feed_sender) .await .unwrap(); @@ -577,7 +577,7 @@ mod tests { let cfd_2 = Cfd::dummy(); db::insert_order(&cfd_2.order, &mut conn).await.unwrap(); - cfd_actors::insert_cfd(&cfd_2, &mut conn, &cfd_feed_sender) + cfd_actors::insert_cfd_and_send_to_feed(&cfd_2, &mut conn, &cfd_feed_sender) .await .unwrap(); assert_eq!(cfd_feed_receiver.borrow().clone(), vec![cfd_1, cfd_2]); diff --git a/daemon/src/maker_cfd.rs b/daemon/src/maker_cfd.rs index cf463c6..9b82a6e 100644 --- a/daemon/src/maker_cfd.rs +++ b/daemon/src/maker_cfd.rs @@ -1,4 +1,4 @@ -use crate::cfd_actors::{self, append_cfd_state, insert_cfd}; +use crate::cfd_actors::{self, append_cfd_state, insert_cfd_and_send_to_feed}; use crate::db::{insert_order, load_cfd_by_order_id, load_order_by_id}; use crate::maker_inc_connections::TakerCommand; use crate::model::cfd::{ @@ -453,7 +453,7 @@ where taker_id, }, ); - insert_cfd(&cfd, &mut conn, &self.cfd_feed_actor_inbox).await?; + insert_cfd_and_send_to_feed(&cfd, &mut conn, &self.cfd_feed_actor_inbox).await?; // 3. check if order has acceptable amounts if quantity < current_order.min_quantity || quantity > current_order.max_quantity { diff --git a/daemon/src/taker_cfd.rs b/daemon/src/taker_cfd.rs index fc846fd..6aa20cb 100644 --- a/daemon/src/taker_cfd.rs +++ b/daemon/src/taker_cfd.rs @@ -1,4 +1,4 @@ -use crate::cfd_actors::{self, append_cfd_state, insert_cfd}; +use crate::cfd_actors::{self, append_cfd_state, insert_cfd_and_send_to_feed}; use crate::db::{insert_order, load_cfd_by_order_id, load_order_by_id}; use crate::model::cfd::{ Cfd, CfdState, CfdStateChangeEvent, CfdStateCommon, CollaborativeSettlement, Dlc, Order, @@ -157,7 +157,7 @@ impl Actor { CfdState::outgoing_order_request(), ); - insert_cfd(&cfd, &mut conn, &self.cfd_feed_actor_inbox).await?; + insert_cfd_and_send_to_feed(&cfd, &mut conn, &self.cfd_feed_actor_inbox).await?; // Cleanup own order feed, after inserting the cfd. // Due to the 1:1 relationship between order and cfd we can never create another cfd for the From 1e42ed7fd7513e505fa58911d956b7eee4bfa529 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Thu, 11 Nov 2021 16:51:16 +1100 Subject: [PATCH 2/2] 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(()) } }