From d42783ff07738b308a302557c719af1ca347e43e Mon Sep 17 00:00:00 2001 From: Richard Holzeis Date: Mon, 17 Jun 2024 16:37:25 +0200 Subject: [PATCH 1/3] refactor: Remove unnecessary pool argument --- coordinator/src/bin/coordinator.rs | 3 +-- coordinator/src/node/rollover.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/coordinator/src/bin/coordinator.rs b/coordinator/src/bin/coordinator.rs index 61c22d027..97c42a970 100644 --- a/coordinator/src/bin/coordinator.rs +++ b/coordinator/src/bin/coordinator.rs @@ -263,11 +263,10 @@ async fn main() -> Result<()> { node.inner.oracle_pubkey, ); let _handle = rollover::monitor( - pool.clone(), + node.clone(), node_event_handler.subscribe(), notification_service.get_sender(), network, - node.clone(), ); let _handle = collaborative_revert::monitor( pool.clone(), diff --git a/coordinator/src/node/rollover.rs b/coordinator/src/node/rollover.rs index ad3c4b3c7..71bf687ce 100644 --- a/coordinator/src/node/rollover.rs +++ b/coordinator/src/node/rollover.rs @@ -18,7 +18,6 @@ use anyhow::Result; use bitcoin::secp256k1::PublicKey; use bitcoin::Network; use diesel::r2d2::ConnectionManager; -use diesel::r2d2::Pool; use diesel::r2d2::PooledConnection; use diesel::PgConnection; use dlc_manager::contract::contract_input::ContractInput; @@ -39,11 +38,10 @@ use xxi_node::node::event::NodeEvent; use xxi_node::node::ProtocolId; pub fn monitor( - pool: Pool>, + node: Node, mut receiver: broadcast::Receiver, notifier: mpsc::Sender, network: Network, - node: Node, ) -> RemoteHandle<()> { let (fut, remote_handle) = async move { loop { @@ -52,10 +50,9 @@ pub fn monitor( tokio::spawn({ let notifier = notifier.clone(); let node = node.clone(); - let pool = pool.clone(); async move { if let Err(e) = node - .check_if_eligible_for_rollover(pool, notifier, peer, network) + .check_if_eligible_for_rollover(notifier, peer, network) .await { tracing::error!( @@ -87,14 +84,16 @@ pub fn monitor( impl Node { async fn check_if_eligible_for_rollover( &self, - pool: Pool>, notifier: mpsc::Sender, trader_id: PublicKey, network: Network, ) -> Result<()> { - let mut conn = spawn_blocking(move || pool.get()) - .await - .expect("task to complete")?; + let mut conn = spawn_blocking({ + let pool = self.pool.clone(); + move || pool.get() + }) + .await + .expect("task to complete")?; tracing::debug!(%trader_id, "Checking if the user's position is eligible for rollover"); From a78ea65d86c3cf754d6a33d2da2332bd7ecf2acd Mon Sep 17 00:00:00 2001 From: Richard Holzeis Date: Mon, 17 Jun 2024 16:40:40 +0200 Subject: [PATCH 2/3] feat: Sync channel eagerly when proposing a rollover Analogue to what we do when opening a position. We check if the contract is confirmed. If not, we sync eagerly - thus increasing the chances that the rollover proposal will succeed. This covers a small edge case where the user opened a position right before the rollover window and the coordinator hasn't synced yet. --- coordinator/src/node/rollover.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/coordinator/src/node/rollover.rs b/coordinator/src/node/rollover.rs index 71bf687ce..7f022421a 100644 --- a/coordinator/src/node/rollover.rs +++ b/coordinator/src/node/rollover.rs @@ -180,6 +180,14 @@ impl Node { ) -> Result<()> { let trader_pubkey = position.trader; + if !self + .inner + .check_if_signed_channel_is_confirmed(trader_pubkey) + .await? + { + bail!("Cannot rollover a contract that is not confirmed"); + } + let next_expiry = commons::calculate_next_expiry(OffsetDateTime::now_utc(), network); let (oracle_pk, contract_tx_fee_rate) = { From 9635806fc4c065520d50588befbf30a4577fef53 Mon Sep 17 00:00:00 2001 From: Richard Holzeis Date: Mon, 17 Jun 2024 16:42:28 +0200 Subject: [PATCH 3/3] fix: Only accept a rollover offer if the contract has been confirmed. Syncs the wallet eagerly if the channel is not yet confirmed. If the contract is not confirmed, the rollover offer gets rejected. This fixes an issue where the app otherwise risk ending up in an intermediate state as it can't accept the `RenewConfirm` message. see https://github.com/get10101/10101/issues/2630 --- mobile/native/src/dlc/node.rs | 91 +++++++++++++++++++++-------------- 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/mobile/native/src/dlc/node.rs b/mobile/native/src/dlc/node.rs index 9a4836464..5d32d2c07 100644 --- a/mobile/native/src/dlc/node.rs +++ b/mobile/native/src/dlc/node.rs @@ -1,4 +1,5 @@ use crate::db; +use crate::dlc::check_if_signed_channel_is_confirmed; use crate::event; use crate::event::BackgroundTask; use crate::event::EventInternal; @@ -26,6 +27,7 @@ use dlc_messages::channel::OfferChannel; use dlc_messages::channel::Reject; use dlc_messages::channel::RenewOffer; use dlc_messages::channel::SettleOffer; +use futures::executor::block_on; use itertools::Itertools; use lightning::chain::transaction::OutPoint; use lightning::sign::DelayedPaymentOutputDescriptor; @@ -744,46 +746,61 @@ impl Node { )?; let channel_id = offer.renew_offer.channel_id; + if !block_on(check_if_signed_channel_is_confirmed())? { + tracing::warn!( + "Rejecting rollover offer as the DLC channel has not been confirmed yet." + ); - match self.inner.dlc_manager.accept_renew_offer(&channel_id) { - Ok((renew_accept, node_id)) => { - let positions = get_positions()?; - let position = positions.first().context("No position to roll over")?; - - let new_unpaid_funding_fee_events = handle_unpaid_funding_fee_events( - &offer - .funding_fee_events - .iter() - .map(|e| { - FundingFeeEvent::unpaid( - position.contract_symbol, - Decimal::try_from(position.quantity).expect("to fit"), - position.direction, - e.price, - e.funding_fee, - e.due_date, - ) - }) - .collect_vec(), - )?; - - handle_rollover_offer(expiry_timestamp, &new_unpaid_funding_fee_events)?; - - self.send_dlc_message( - to_secp_pk_30(node_id), - TenTenOneMessage::RolloverAccept(TenTenOneRolloverAccept { renew_accept }), - )?; - } - Err(e) => { - tracing::error!("Failed to accept DLC channel rollover offer: {e}"); + event::publish(&EventInternal::BackgroundNotification( + BackgroundTask::Rollover(TaskStatus::Failed( + "Failed to rollover with unconfirmed DLC channel.".to_string(), + )), + )); + + self.reject_rollover_offer(&channel_id)?; + } else { + tracing::warn!("DLC channel is confirmed. Accepting rollover offer."); + + match self.inner.dlc_manager.accept_renew_offer(&channel_id) { + Ok((renew_accept, node_id)) => { + let positions = get_positions()?; + let position = positions.first().context("No position to roll over")?; + + let new_unpaid_funding_fee_events = handle_unpaid_funding_fee_events( + &offer + .funding_fee_events + .iter() + .map(|e| { + FundingFeeEvent::unpaid( + position.contract_symbol, + Decimal::try_from(position.quantity).expect("to fit"), + position.direction, + e.price, + e.funding_fee, + e.due_date, + ) + }) + .collect_vec(), + )?; + + handle_rollover_offer(expiry_timestamp, &new_unpaid_funding_fee_events)?; + + self.send_dlc_message( + to_secp_pk_30(node_id), + TenTenOneMessage::RolloverAccept(TenTenOneRolloverAccept { renew_accept }), + )?; + } + Err(e) => { + tracing::error!("Failed to accept DLC channel rollover offer: {e}"); - event::publish(&EventInternal::BackgroundNotification( - BackgroundTask::Rollover(TaskStatus::Failed(format!("{e}"))), - )); + event::publish(&EventInternal::BackgroundNotification( + BackgroundTask::Rollover(TaskStatus::Failed(format!("{e}"))), + )); - self.reject_rollover_offer(&channel_id)?; - } - }; + self.reject_rollover_offer(&channel_id)?; + } + }; + } Ok(()) }