From 5fdbb5dafbfde9f6ece281de5e557d656c83d452 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Thu, 14 Dec 2023 13:08:35 +1100 Subject: [PATCH 1/2] fix(maker): Trigger on-chain and off-chain sync at the same time Otherwise we can run into problems like this one[1]. [1]: https://github.com/get10101/10101/actions/runs/7196185313/job/19600541210#step:12:2970. --- crates/tests-e2e/examples/fund.rs | 4 ++-- crates/tests-e2e/src/maker.rs | 5 +++-- crates/tests-e2e/tests/maker.rs | 4 ++-- maker/src/routes.rs | 18 ++++++++++++------ 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/tests-e2e/examples/fund.rs b/crates/tests-e2e/examples/fund.rs index 2b13a6926..ba1b6c18f 100644 --- a/crates/tests-e2e/examples/fund.rs +++ b/crates/tests-e2e/examples/fund.rs @@ -61,7 +61,7 @@ async fn fund_everything(faucet: &str, coordinator: &str, maker: &str) -> Result bitcoind.fund(&maker_addr, Amount::ONE_BTC).await?; bitcoind.mine(10).await?; maker - .sync_on_chain() + .sync() .await .context("Failed to sync maker's on-chain wallet")?; @@ -155,7 +155,7 @@ async fn fund_everything(faucet: &str, coordinator: &str, maker: &str) -> Result bitcoind.mine(10).await?; maker - .sync_on_chain() + .sync() .await .expect("to be able to sync on-chain wallet for maker"); let maker_balance = maker.get_balance().await?; diff --git a/crates/tests-e2e/src/maker.rs b/crates/tests-e2e/src/maker.rs index f24485237..8e02b4284 100644 --- a/crates/tests-e2e/src/maker.rs +++ b/crates/tests-e2e/src/maker.rs @@ -37,9 +37,10 @@ impl Maker { .is_ok_and(|r| r.status().is_success()) } - pub async fn sync_on_chain(&self) -> Result<()> { + /// Sync the on-chain and Lightning wallets. + pub async fn sync(&self) -> Result<()> { let no_json: Option<()> = None; - self.post("/api/sync-on-chain", no_json).await?; + self.post("/api/sync", no_json).await?; Ok(()) } diff --git a/crates/tests-e2e/tests/maker.rs b/crates/tests-e2e/tests/maker.rs index 6f4fadc5e..cc3d802e4 100644 --- a/crates/tests-e2e/tests/maker.rs +++ b/crates/tests-e2e/tests/maker.rs @@ -32,7 +32,7 @@ async fn maker_can_open_channel_to_coordinator_and_send_payment() -> Result<()> .await .unwrap(); bitcoind.mine(1).await.unwrap(); - maker.sync_on_chain().await.unwrap(); + maker.sync().await.unwrap(); let balance_maker_before_channel = maker.get_balance().await?.offchain; @@ -47,7 +47,7 @@ async fn maker_can_open_channel_to_coordinator_and_send_payment() -> Result<()> // Mine one block to render the public channel is usable. bitcoind.mine(1).await.unwrap(); coordinator.sync_wallet().await.unwrap(); - maker.sync_on_chain().await.unwrap(); + maker.sync().await.unwrap(); let balance_maker_after_channel = maker.get_balance().await?.offchain; diff --git a/maker/src/routes.rs b/maker/src/routes.rs index ac357b0ef..6b7dd5a43 100644 --- a/maker/src/routes.rs +++ b/maker/src/routes.rs @@ -69,7 +69,7 @@ pub fn router( .route("/api/channels", get(list_channels).post(create_channel)) .route("/api/connect", post(connect_to_peer)) .route("/api/pay-invoice/:invoice", post(pay_invoice)) - .route("/api/sync-on-chain", post(sync_on_chain)) + .route("/api/sync", post(sync)) .route("/api/position", get(get_position)) .route("/api/node", get(get_node_info)) .route("/metrics", get(get_metrics)) @@ -260,11 +260,17 @@ pub async fn pay_invoice( Ok(()) } -pub async fn sync_on_chain(State(state): State>) -> Result<(), AppError> { - spawn_blocking(move || state.node.ldk_wallet().sync()) - .await - .expect("task to complete") - .map_err(|e| AppError::InternalServerError(format!("Could not sync wallet: {e:#}")))?; +/// Internal API for syncing the on-chain and Lightning wallets. +pub async fn sync(State(state): State>) -> Result<(), AppError> { + spawn_blocking(move || { + state.node.sync_on_chain_wallet()?; + state.node.sync_lightning_wallet()?; + + anyhow::Ok(()) + }) + .await + .expect("task to complete") + .map_err(|e| AppError::InternalServerError(format!("Could not sync wallet: {e:#}")))?; Ok(()) } From ac7cfe44b80a67befd07eb1cb0dd82b35ed8523e Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Thu, 14 Dec 2023 13:19:26 +1100 Subject: [PATCH 2/2] fix(ln-dlc-node): Sync both wallets back to back in tests Whilst we don't usually run into errors in `ln-dlc-node` tests, it's still better to make sure that both wallets agree on the latest state of the blockchain before continuing. --- .../tests/dlc/non_collaborative_settlement.rs | 20 ++++++++-------- .../just_in_time_channel/channel_close.rs | 8 +++---- .../src/tests/just_in_time_channel/create.rs | 24 +++++++++---------- crates/ln-dlc-node/src/tests/mod.rs | 17 ++++++++----- .../src/tests/multi_hop_payment.rs | 12 +++++----- crates/ln-dlc-node/src/tests/probe.rs | 12 +++++----- .../src/tests/single_hop_payment.rs | 4 ++-- 7 files changed, 51 insertions(+), 46 deletions(-) diff --git a/crates/ln-dlc-node/src/tests/dlc/non_collaborative_settlement.rs b/crates/ln-dlc-node/src/tests/dlc/non_collaborative_settlement.rs index fda7437c5..b14aaa19e 100644 --- a/crates/ln-dlc-node/src/tests/dlc/non_collaborative_settlement.rs +++ b/crates/ln-dlc-node/src/tests/dlc/non_collaborative_settlement.rs @@ -44,8 +44,8 @@ async fn force_close_ln_dlc_channel() { .await .unwrap(); - coordinator.sync_on_chain().await.unwrap(); - app.sync_on_chain().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); + app.sync_wallets().await.unwrap(); // Act @@ -55,8 +55,8 @@ async fn force_close_ln_dlc_channel() { // transactions mine(288).await.unwrap(); - coordinator.sync_on_chain().await.unwrap(); - app.sync_on_chain().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); + app.sync_wallets().await.unwrap(); // Ensure publication of the glue and buffer transactions (otherwise we need to wait for the // periodic task) @@ -70,8 +70,8 @@ async fn force_close_ln_dlc_channel() { // Assert - coordinator.sync_on_chain().await.unwrap(); - app.sync_on_chain().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); + app.sync_wallets().await.unwrap(); // Mining 288 blocks ensures that we get: // - 144 required confirmations for the delayed output on the LN commitment transaction to be @@ -79,8 +79,8 @@ async fn force_close_ln_dlc_channel() { // - 288 required confirmations for the CET to be published. mine(288).await.unwrap(); - coordinator.sync_on_chain().await.unwrap(); - app.sync_on_chain().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); + app.sync_wallets().await.unwrap(); // Ensure publication of CET (otherwise we need to wait for the periodic task) sub_channel_manager_periodic_check( @@ -95,9 +95,9 @@ async fn force_close_ln_dlc_channel() { mine(1).await.unwrap(); tracing::info!("Mined 1 block"); - coordinator.sync_on_chain().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); tracing::info!("Coordinator synced on-chain"); - app.sync_on_chain().await.unwrap(); + app.sync_wallets().await.unwrap(); tracing::info!("App synced on-chain"); let coordinator_on_chain_balance_after_force_close = diff --git a/crates/ln-dlc-node/src/tests/just_in_time_channel/channel_close.rs b/crates/ln-dlc-node/src/tests/just_in_time_channel/channel_close.rs index 4e10ebc44..96a4f2cfb 100644 --- a/crates/ln-dlc-node/src/tests/just_in_time_channel/channel_close.rs +++ b/crates/ln-dlc-node/src/tests/just_in_time_channel/channel_close.rs @@ -69,7 +69,7 @@ async fn ln_collab_close() { // Mine one block to confirm the close transaction bitcoind::mine(1).await.unwrap(); - payee.sync_on_chain().await.unwrap(); + payee.sync_wallets().await.unwrap(); // Assert @@ -134,7 +134,7 @@ async fn ln_force_close() { .force_close_broadcasting_latest_txn(&channel_id, &coordinator.info.pubkey) .unwrap(); - payee.sync_on_chain().await.unwrap(); + payee.sync_wallets().await.unwrap(); assert_eq!(payee.get_on_chain_balance().unwrap().confirmed, 0); assert_eq!(payee.get_ldk_balance().available(), 0); @@ -155,12 +155,12 @@ async fn ln_force_close() { // Syncing the payee's wallet should now trigger a `SpendableOutputs` event // corresponding to their revocable output in the commitment transaction, which they // will subsequently spend in a new transaction paying to their on-chain wallet - payee.sync_on_chain().await.unwrap(); + payee.sync_wallets().await.unwrap(); // Mine one more block to confirm the transaction spending the payee's revocable output // in the commitment transaction bitcoind::mine(1).await.unwrap(); - payee.sync_on_chain().await.unwrap(); + payee.sync_wallets().await.unwrap(); // Assert diff --git a/crates/ln-dlc-node/src/tests/just_in_time_channel/create.rs b/crates/ln-dlc-node/src/tests/just_in_time_channel/create.rs index ebb95fd65..4fd8304de 100644 --- a/crates/ln-dlc-node/src/tests/just_in_time_channel/create.rs +++ b/crates/ln-dlc-node/src/tests/just_in_time_channel/create.rs @@ -250,9 +250,9 @@ pub(crate) async fn send_interceptable_payment( let user_channel_id = liquidity_request.user_channel_id; let jit_channel_fee = liquidity_request.fee_sats; - payer.ldk_wallet().sync()?; - coordinator.ldk_wallet().sync()?; - payee.ldk_wallet().sync()?; + payer.sync_wallets().await?; + coordinator.sync_wallets().await?; + payee.sync_wallets().await?; let payer_balance_before = payer.get_ldk_balance(); let coordinator_balance_before = coordinator.get_ldk_balance(); @@ -310,9 +310,9 @@ pub(crate) async fn send_interceptable_payment( // Assert // Sync LN wallet after payment is claimed to update the balances - payer.ldk_wallet().sync()?; - coordinator.ldk_wallet().sync()?; - payee.ldk_wallet().sync()?; + payer.sync_wallets().await?; + coordinator.sync_wallets().await?; + payee.sync_wallets().await?; let payer_balance_after = payer.get_ldk_balance(); let coordinator_balance_after = coordinator.get_ldk_balance(); @@ -346,9 +346,9 @@ pub(crate) async fn send_payment( invoice_amount_sat: u64, coordinator_just_in_time_channel_creation_outbound_liquidity: Option, ) -> Result<()> { - payer.ldk_wallet().sync()?; - coordinator.ldk_wallet().sync()?; - payee.ldk_wallet().sync()?; + payer.sync_wallets().await?; + coordinator.sync_wallets().await?; + payee.sync_wallets().await?; let payer_balance_before = payer.get_ldk_balance(); let coordinator_balance_before = coordinator.get_ldk_balance(); @@ -394,9 +394,9 @@ pub(crate) async fn send_payment( // Assert // Sync LN wallet after payment is claimed to update the balances - payer.ldk_wallet().sync()?; - coordinator.ldk_wallet().sync()?; - payee.ldk_wallet().sync()?; + payer.sync_wallets().await?; + coordinator.sync_wallets().await?; + payee.sync_wallets().await?; let payer_balance_after = payer.get_ldk_balance(); let coordinator_balance_after = coordinator.get_ldk_balance(); diff --git a/crates/ln-dlc-node/src/tests/mod.rs b/crates/ln-dlc-node/src/tests/mod.rs index 16dbc084f..9b519e6f5 100644 --- a/crates/ln-dlc-node/src/tests/mod.rs +++ b/crates/ln-dlc-node/src/tests/mod.rs @@ -210,15 +210,20 @@ impl Node { Ok((node, running)) } - /// Trigger on-chain wallet sync. + /// Trigger on-chain and off-chain wallet syncs. /// /// We wrap the wallet sync with a `block_in_place` to avoid blocking the async task in /// `tokio::test`s. /// /// Because we use `block_in_place`, we must configure the `tokio::test`s with `flavor = /// "multi_thread"`. - async fn sync_on_chain(&self) -> Result<()> { - block_in_place(|| self.ldk_wallet().sync()) + async fn sync_wallets(&self) -> Result<()> { + block_in_place(|| { + self.sync_on_chain_wallet()?; + self.sync_lightning_wallet()?; + + Ok(()) + }) } async fn fund(&self, amount: Amount) -> Result<()> { @@ -238,7 +243,7 @@ impl Node { while self.get_confirmed_balance().await.unwrap() < expected_balance { let interval = Duration::from_millis(200); - self.sync_on_chain().await.unwrap(); + self.sync_wallets().await.unwrap(); tokio::time::sleep(interval).await; tracing::debug!( @@ -325,8 +330,8 @@ impl Node { // We need to sync both parties, even if // `trust_own_funding_0conf` is true for the creator // of the channel (`self`) - self.sync_on_chain().await.unwrap(); - peer.sync_on_chain().await.unwrap(); + self.sync_wallets().await.unwrap(); + peer.sync_wallets().await.unwrap(); } tracing::debug!( diff --git a/crates/ln-dlc-node/src/tests/multi_hop_payment.rs b/crates/ln-dlc-node/src/tests/multi_hop_payment.rs index 73b9293c3..b65c5d624 100644 --- a/crates/ln-dlc-node/src/tests/multi_hop_payment.rs +++ b/crates/ln-dlc-node/src/tests/multi_hop_payment.rs @@ -47,9 +47,9 @@ async fn multi_hop_payment() { let coordinator_balance_before = coordinator.get_ldk_balance(); let payee_balance_before = payee.get_ldk_balance(); - payer.sync_on_chain().await.unwrap(); - coordinator.sync_on_chain().await.unwrap(); - payee.sync_on_chain().await.unwrap(); + payer.sync_wallets().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); + payee.sync_wallets().await.unwrap(); // Act @@ -74,9 +74,9 @@ async fn multi_hop_payment() { // Assert // Sync LN wallet after payment is claimed to update the balances - payer.sync_on_chain().await.unwrap(); - coordinator.sync_on_chain().await.unwrap(); - payee.sync_on_chain().await.unwrap(); + payer.sync_wallets().await.unwrap(); + coordinator.sync_wallets().await.unwrap(); + payee.sync_wallets().await.unwrap(); let payer_balance_after = payer.get_ldk_balance(); let coordinator_balance_after = coordinator.get_ldk_balance(); diff --git a/crates/ln-dlc-node/src/tests/probe.rs b/crates/ln-dlc-node/src/tests/probe.rs index e8b71735c..97821d82a 100644 --- a/crates/ln-dlc-node/src/tests/probe.rs +++ b/crates/ln-dlc-node/src/tests/probe.rs @@ -44,12 +44,12 @@ async fn estimate_payment_fee() { tracing::info!("Waiting for channels to be discovered"); for _ in 0..5 { - payer.sync_on_chain().await.unwrap(); - a.sync_on_chain().await.unwrap(); - b.sync_on_chain().await.unwrap(); - c.sync_on_chain().await.unwrap(); - d.sync_on_chain().await.unwrap(); - payee.sync_on_chain().await.unwrap(); + payer.sync_wallets().await.unwrap(); + a.sync_wallets().await.unwrap(); + b.sync_wallets().await.unwrap(); + c.sync_wallets().await.unwrap(); + d.sync_wallets().await.unwrap(); + payee.sync_wallets().await.unwrap(); payer.broadcast_node_announcement(); a.broadcast_node_announcement(); diff --git a/crates/ln-dlc-node/src/tests/single_hop_payment.rs b/crates/ln-dlc-node/src/tests/single_hop_payment.rs index 3251c7387..fa97325d2 100644 --- a/crates/ln-dlc-node/src/tests/single_hop_payment.rs +++ b/crates/ln-dlc-node/src/tests/single_hop_payment.rs @@ -46,8 +46,8 @@ async fn single_hop_payment() { // Assert // Sync LN wallet after payment is claimed to update the balances - payer.sync_on_chain().await.unwrap(); - payee.sync_on_chain().await.unwrap(); + payer.sync_wallets().await.unwrap(); + payee.sync_wallets().await.unwrap(); let payer_balance_after = payer.get_ldk_balance(); let payee_balance_after = payee.get_ldk_balance();