Merge pull request #2557 from get10101/fix/handle-pending-rollover-offer

fix: Ignore missing order id when rejecting a rollover offer
get10101 · May 23, 2024 · 9a39e25 · 9a39e25
2 parents 10aff81 + ffd6b56
commit 9a39e25
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 31 deletions.
diff --git a/coordinator/src/db/positions.rs b/coordinator/src/db/positions.rs
@@ -166,29 +166,6 @@ impl Position {
         Ok(crate::position::models::Position::from(position))
     }
 
-    /// sets the status of the position in state `Closing` to a new state
-    pub fn update_closing_position(
-        conn: &mut PgConnection,
-        trader_pubkey: String,
-        state: crate::position::models::PositionState,
-    ) -> Result<()> {
-        let state = PositionState::from(state);
-        let affected_rows = diesel::update(positions::table)
-            .filter(positions::trader_pubkey.eq(trader_pubkey.clone()))
-            .filter(positions::position_state.eq(PositionState::Closing))
-            .set((
-                positions::position_state.eq(state),
-                positions::update_timestamp.eq(OffsetDateTime::now_utc()),
-            ))
-            .execute(conn)?;
-
-        if affected_rows == 0 {
-            bail!("Could not update position to {state:?} for {trader_pubkey}")
-        }
-
-        Ok(())
-    }
-
     /// sets the status of all open position to closing (note, we expect that number to be always
     /// exactly 1)
     pub fn set_open_position_to_closing(

diff --git a/coordinator/src/node.rs b/coordinator/src/node.rs
@@ -481,9 +481,14 @@ impl Node {
                                     "DLC Channel settle offer has been rejected. Setting position to back to open."
                                 );
 
-                        db::positions::Position::update_closing_position(
+                        db::positions::Position::update_position_state(
                             &mut connection,
                             node_id.to_string(),
+                            vec![
+                                // the closing price doesn't matter here.
+                                PositionState::Closing { closing_price: 0.0 },
+                                PositionState::Rollover,
+                            ],
                             PositionState::Open,
                         )?;
                     }

diff --git a/mobile/native/src/dlc/dlc_handler.rs b/mobile/native/src/dlc/dlc_handler.rs
@@ -206,6 +206,11 @@ impl DlcHandler {
                         BackgroundTask::RecoverDlc(TaskStatus::Pending),
                     ));
 
+                    // FIXME(holzeis): We need to be able to differentiate between a
+                    // SignedChannelState::RenewOffered and a RolloverOffer. This differentiation
+                    // currently only lives in 10101 and in the dlc messages, but not on the channel
+                    // state. Hence at the moment we also reject pending `RolloverOffers` the same
+                    // way we reject `RenewOffers`
                     self.node
                         .reject_renew_offer(None, channel_id)
                         .context("Failed to reject pending renew offer")?;

diff --git a/mobile/native/src/trade/order/handler.rs b/mobile/native/src/trade/order/handler.rs
@@ -322,20 +322,28 @@ pub(crate) fn order_failed(
     let order = match order_id {
         None => get_order_in_filling(),
         Some(order_id) => db::get_order(order_id),
-    }?
-    .with_context(|| format!("Could not find order. order_id = {order_id:?}"))
+    }
     .inspect_err(|e| {
+        // it doesn't matter that we send here an async trade failed even though we do not exactly
+        // know what kind of background task failed, because the error screen looks always the same.
         event::publish(&EventInternal::BackgroundNotification(
             BackgroundTask::AsyncTrade(TaskStatus::Failed(format!("{e:#}"))),
         ))
     })?;
 
     let task_status = TaskStatus::Failed(format!("{error:#}"));
-    let task = match order.reason {
-        OrderReason::Manual => BackgroundTask::AsyncTrade(task_status),
-        OrderReason::Expired => BackgroundTask::Expire(task_status),
-        OrderReason::CoordinatorLiquidated | OrderReason::TraderLiquidated => {
-            BackgroundTask::Expire(task_status)
+    let task = match order {
+        Some(order) => match order.reason {
+            OrderReason::Manual => BackgroundTask::AsyncTrade(task_status),
+            OrderReason::Expired => BackgroundTask::Expire(task_status),
+            OrderReason::CoordinatorLiquidated | OrderReason::TraderLiquidated => {
+                BackgroundTask::Expire(task_status)
+            }
+        },
+        None => {
+            // if we can't find a filling order it must have been a rollover. Note this is not very
+            // nice, but we are missing the required context information at the moment.
+            BackgroundTask::Rollover(task_status)
         }
     };