From 0d9cf7df8510d04a6af6f1693237951ea894ee84 Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 15 Oct 2021 13:32:14 +1100 Subject: [PATCH 1/2] Always cleanup order upon rejection Upon rejection, we always have to remove the order on the maker side, otherwise a taker cannot re-try taking (with e.g. a different amount) because there is already a rejected cfd in the taker database. --- daemon/src/maker_cfd.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/daemon/src/maker_cfd.rs b/daemon/src/maker_cfd.rs index d2644cd..b74bec2 100644 --- a/daemon/src/maker_cfd.rs +++ b/daemon/src/maker_cfd.rs @@ -16,6 +16,8 @@ use bdk::bitcoin::secp256k1::schnorrsig; use cfd_protocol::secp256k1_zkp::Signature; use futures::channel::mpsc; use futures::{future, SinkExt}; +use rocket_db_pools::sqlx::Sqlite; +use sqlx::pool::PoolConnection; use std::collections::HashMap; use std::time::SystemTime; use tokio::sync::watch; @@ -513,12 +515,8 @@ impl Actor { current_order.min_quantity, current_order.max_quantity ); - self.takers - .do_send_async(maker_inc_connections::TakerMessage { - taker_id, - command: TakerCommand::NotifyOrderRejected { id: order_id }, - }) - .await?; + + self.reject_order(taker_id, order_id, conn).await?; return Ok(()); } @@ -654,6 +652,22 @@ impl Actor { } }; + self.reject_order(taker_id, order_id, conn).await?; + + 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, + order_id: OrderId, + mut conn: PoolConnection, + ) -> Result<()> { // Update order in db insert_new_cfd_state_by_order_id( order_id, From 7355c339a69361b6ba4478f3cabc06c8e02be97d Mon Sep 17 00:00:00 2001 From: Daniel Karzel Date: Fri, 15 Oct 2021 13:33:27 +1100 Subject: [PATCH 2/2] Remove todo in favour of warning for awareness Better log a warning, the assumption is that we rarely see this. Let's see if that is true. --- daemon/src/maker_cfd.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/daemon/src/maker_cfd.rs b/daemon/src/maker_cfd.rs index b74bec2..a6260ef 100644 --- a/daemon/src/maker_cfd.rs +++ b/daemon/src/maker_cfd.rs @@ -502,7 +502,13 @@ impl Actor { command: TakerCommand::NotifyInvalidOrderId { id: order_id }, }) .await?; - // TODO: Return an error here? + + // An outdated order on the taker side does not require any state change on the + // maker. notifying the taker with a specific message should be sufficient. + // Since this is a scenario that we should rarely see we log + // a warning to be sure we don't trigger this code path frequently. + tracing::warn!("Taker tried to take order with outdated id {}", order_id); + return Ok(()); } };