From 52b57be54ac4262e3239e65c480b665b733d8a62 Mon Sep 17 00:00:00 2001 From: Richard Holzeis Date: Tue, 21 May 2024 14:45:30 +0200 Subject: [PATCH 1/2] fix: Ignore missing order id when rejecting a rollover offer In case of a rollover we do not have an order id or and order in filling. As a result this logic failed when trying to reject a rollover offer. The solution is still hacky as with the correct context information we shouldn't even try to set the order to failed, but that context is missing when we are reconnecting in the `on_connect` function and finding a dlc channel in state `RenewOffered`. --- mobile/native/src/dlc/dlc_handler.rs | 5 +++++ mobile/native/src/trade/order/handler.rs | 22 +++++++++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/mobile/native/src/dlc/dlc_handler.rs b/mobile/native/src/dlc/dlc_handler.rs index 1e33930e6..f847c01f9 100644 --- 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 index b8bd5dad6..cbda52730 100644 --- 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) } }; From ffd6b56957c793279fcf2e985572c3848b4d01f1 Mon Sep 17 00:00:00 2001 From: Richard Holzeis Date: Tue, 21 May 2024 14:46:17 +0200 Subject: [PATCH 2/2] fix: Set position to open if a rollover is rejected Otherwise the position is stuck in `Rollover` and won't get recovered. --- coordinator/src/db/positions.rs | 23 ----------------------- coordinator/src/node.rs | 7 ++++++- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/coordinator/src/db/positions.rs b/coordinator/src/db/positions.rs index c50d510dc..a60547752 100644 --- 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 index fc22040aa..70479e4e2 100644 --- 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, )?; }