From 51357c646224492edba12dcf96916815c886835c Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Mon, 19 Feb 2024 14:23:27 +1100 Subject: [PATCH 1/7] chore(ln-dlc-node): Rewrite funding transaction test Soon `rust-dlc` will enforce including at least one change output per party in the funding transaction, so that either party can fee-bump it. The original `can_open_channel_with_min_inputs` test actually forced `rust-dlc` to generate a change-output-less funding transaction. This is valid for now, but it doesn't help to build on assumptions that are going to change soon. The new test simply checks that the funding transaction pays fees at the expected fee rate. If we can trust `rust-dlc` to respect the fee rate we give it, we can confidently display this information to a user who is about to open a channel, so that they know the transaction fee cost of their action. --- crates/ln-dlc-node/src/tests/dlc_channel.rs | 51 ++++++++++++++------- 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/crates/ln-dlc-node/src/tests/dlc_channel.rs b/crates/ln-dlc-node/src/tests/dlc_channel.rs index b575e24c8..664238566 100644 --- a/crates/ln-dlc-node/src/tests/dlc_channel.rs +++ b/crates/ln-dlc-node/src/tests/dlc_channel.rs @@ -260,39 +260,56 @@ async fn can_open_and_force_close_channel() { #[tokio::test(flavor = "multi_thread")] #[ignore] -async fn can_open_channel_with_min_inputs() { +async fn funding_transaction_pays_expected_fees() { init_tracing(); + // Arrange + let app_dlc_collateral = Amount::from_sat(10_000); let coordinator_dlc_collateral = Amount::from_sat(10_000); - // We must fix the fee rate so that we can predict how many sats `rust-dlc` will allocate - // for transaction fees. - let fee_rate_sats_per_vbyte = 2; - let expected_fund_tx_fee = 252 * fee_rate_sats_per_vbyte; - - // This also depends on the fee rate, but the formula is a bit more involved. - let fee_reserve = 880; - - // Fee costs are evenly split. - let fee_cost_per_party = (expected_fund_tx_fee + fee_reserve) / 2; - let fee_cost_per_party = Amount::from_sat(fee_cost_per_party); + let fee_rate_sats_per_vb = 2; - let (app, _running_app) = start_and_fund_app(app_dlc_collateral + fee_cost_per_party, 1).await; + // Give enough funds to app and coordinator so that each party can have their own change output. + // This is not currently enforced by `rust-dlc`, but it will be in the near future: + // https://github.com/p2pderivatives/rust-dlc/pull/152. + let (app, _running_app) = start_and_fund_app(app_dlc_collateral * 2, 1).await; let (coordinator, _running_coordinator) = - start_and_fund_coordinator(coordinator_dlc_collateral + fee_cost_per_party, 1).await; + start_and_fund_coordinator(app_dlc_collateral * 2, 1).await; + + // Act let (app_signed_channel, _) = open_channel_and_position( app.clone(), coordinator.clone(), app_dlc_collateral, coordinator_dlc_collateral, - Some(fee_rate_sats_per_vbyte), + Some(fee_rate_sats_per_vb), ) .await; - // No change output means that the inputs were spent in full by the fund output. - assert!(app_signed_channel.fund_tx.output.len() == 1); + // Assert + + let fund_tx_outputs_amount = app_signed_channel + .fund_tx + .output + .iter() + .fold(Amount::ZERO, |acc, output| { + acc + Amount::from_sat(output.value) + }); + + let fund_tx_inputs_amount = Amount::from_sat( + app_signed_channel.own_params.input_amount + app_signed_channel.counter_params.input_amount, + ); + + let fund_tx_fee = fund_tx_inputs_amount - fund_tx_outputs_amount; + + let fund_tx_weight_wu = app_signed_channel.fund_tx.weight(); + let fund_tx_weight_vb = (fund_tx_weight_wu / 4) as u64; + + let fund_tx_fee_rate_sats_per_vb = fund_tx_fee.to_sat() / fund_tx_weight_vb; + + assert_eq!(fund_tx_fee_rate_sats_per_vb, fee_rate_sats_per_vb); } async fn start_and_fund_app( From d0349ef43426a7591a691cc907c53ecd9b7462a7 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Mon, 19 Feb 2024 18:07:01 +1100 Subject: [PATCH 2/7] chore(ln-dlc-node): Reproduce bug when calculating DLC channel fees --- Cargo.lock | 10 +-- Cargo.toml | 10 +-- coordinator/src/collaborative_revert.rs | 4 +- crates/ln-dlc-node/src/tests/dlc_channel.rs | 69 +++++++++++++++++++++ 4 files changed, 81 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4abbc8af2..77f8bbcea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1366,7 +1366,7 @@ dependencies = [ [[package]] name = "dlc" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=bc31c6167e304d7886c328da91e8c17dad1afce5#bc31c6167e304d7886c328da91e8c17dad1afce5" +source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" dependencies = [ "bitcoin 0.29.2", "miniscript 8.0.2", @@ -1378,7 +1378,7 @@ dependencies = [ [[package]] name = "dlc-manager" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=bc31c6167e304d7886c328da91e8c17dad1afce5#bc31c6167e304d7886c328da91e8c17dad1afce5" +source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" dependencies = [ "async-trait", "bitcoin 0.29.2", @@ -1394,7 +1394,7 @@ dependencies = [ [[package]] name = "dlc-messages" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=bc31c6167e304d7886c328da91e8c17dad1afce5#bc31c6167e304d7886c328da91e8c17dad1afce5" +source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" dependencies = [ "bitcoin 0.29.2", "dlc", @@ -1407,7 +1407,7 @@ dependencies = [ [[package]] name = "dlc-trie" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=bc31c6167e304d7886c328da91e8c17dad1afce5#bc31c6167e304d7886c328da91e8c17dad1afce5" +source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" dependencies = [ "bitcoin 0.29.2", "dlc", @@ -2928,7 +2928,7 @@ checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" [[package]] name = "p2pd-oracle-client" version = "0.1.0" -source = "git+https://github.com/get10101/rust-dlc?rev=bc31c6167e304d7886c328da91e8c17dad1afce5#bc31c6167e304d7886c328da91e8c17dad1afce5" +source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" dependencies = [ "chrono", "dlc-manager", diff --git a/Cargo.toml b/Cargo.toml index 2dab36f1c..0314512f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,11 +20,11 @@ resolver = "2" # We are using our own fork of `rust-dlc` at least until we can drop all the LN-DLC features. Also, # `p2pderivatives/rust-dlc#master` is missing certain patches that can only be found in the LN-DLC # branch. -dlc-manager = { git = "https://github.com/get10101/rust-dlc", rev = "bc31c6167e304d7886c328da91e8c17dad1afce5" } -dlc-messages = { git = "https://github.com/get10101/rust-dlc", rev = "bc31c6167e304d7886c328da91e8c17dad1afce5" } -dlc = { git = "https://github.com/get10101/rust-dlc", rev = "bc31c6167e304d7886c328da91e8c17dad1afce5" } -p2pd-oracle-client = { git = "https://github.com/get10101/rust-dlc", rev = "bc31c6167e304d7886c328da91e8c17dad1afce5" } -dlc-trie = { git = "https://github.com/get10101/rust-dlc", rev = "bc31c6167e304d7886c328da91e8c17dad1afce5" } +dlc-manager = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } +dlc-messages = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } +dlc = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } +p2pd-oracle-client = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } +dlc-trie = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } # We should usually track the `p2pderivatives/split-tx-experiment[-10101]` branch. For now we depend # on a special fork which removes a panic in `rust-lightning`. diff --git a/coordinator/src/collaborative_revert.rs b/coordinator/src/collaborative_revert.rs index 812b75a50..6b962b2aa 100644 --- a/coordinator/src/collaborative_revert.rs +++ b/coordinator/src/collaborative_revert.rs @@ -17,7 +17,7 @@ use diesel::r2d2::ConnectionManager; use diesel::r2d2::Pool; use diesel::r2d2::PooledConnection; use diesel::PgConnection; -use dlc::util::weight_to_fee; +use dlc::util::tx_weight_to_fee; use dlc_manager::channel::ClosedChannel; use dlc_manager::DlcChannelId; use dlc_manager::Signer; @@ -86,7 +86,7 @@ pub async fn propose_collaborative_revert( .checked_sub(trader_amount_sats) .context("Could not substract trader amount from total value without overflow")?; - let fee = weight_to_fee(COLLABORATIVE_REVERT_TX_WEIGHT, fee_rate_sats_vb) + let fee = tx_weight_to_fee(COLLABORATIVE_REVERT_TX_WEIGHT, fee_rate_sats_vb) .context("Could not calculate fee")?; let fee_half = fee.checked_div(2).context("Could not divide fee")?; diff --git a/crates/ln-dlc-node/src/tests/dlc_channel.rs b/crates/ln-dlc-node/src/tests/dlc_channel.rs index 664238566..2dc844322 100644 --- a/crates/ln-dlc-node/src/tests/dlc_channel.rs +++ b/crates/ln-dlc-node/src/tests/dlc_channel.rs @@ -312,6 +312,75 @@ async fn funding_transaction_pays_expected_fees() { assert_eq!(fund_tx_fee_rate_sats_per_vb, fee_rate_sats_per_vb); } +#[tokio::test(flavor = "multi_thread")] +#[ignore] +#[should_panic] +async fn dlc_channel_includes_expected_fee_reserve() { + init_tracing(); + + let app_dlc_collateral = Amount::from_sat(10_000); + let coordinator_dlc_collateral = Amount::from_sat(10_000); + + // We must fix the fee rate so that we can predict how many sats `rust-dlc` will allocate + // for transaction fees. + let fee_rate_sats_per_vb = 2; + + let buffer_weight_wu = dlc::channel::BUFFER_TX_WEIGHT; + + let cet_or_refund_weight_wu = { + let cet_or_refund_base_weight_wu = dlc::CET_BASE_WEIGHT; + // Because we are spending from a buffer transaction. + let cet_or_refund_extra_weight_wu = dlc::channel::CET_EXTRA_WEIGHT; + + // This is the standard length of a P2WPKH script pubkey. + let cet_or_refund_output_spk_bytes = 22; + + // Value = 8 bytes; var_int = 1 byte. + let cet_or_refund_output_weight_wu = (8 + 1 + cet_or_refund_output_spk_bytes) * 4; + + cet_or_refund_base_weight_wu + + cet_or_refund_extra_weight_wu + // 1 output per party. + + (2 * cet_or_refund_output_weight_wu) + }; + + let total_fee_reserve = { + let total_weight_vb = (buffer_weight_wu + cet_or_refund_weight_wu) as f32 / 4.0; + let total_weight_vb = total_weight_vb.ceil() as u64; + + Amount::from_sat(total_weight_vb * fee_rate_sats_per_vb) + }; + + let expected_fund_output_amount = + app_dlc_collateral + coordinator_dlc_collateral + total_fee_reserve; + + let (app, _running_app) = start_and_fund_app(app_dlc_collateral * 2, 1).await; + let (coordinator, _running_coordinator) = + start_and_fund_coordinator(coordinator_dlc_collateral * 2, 1).await; + + let (app_signed_channel, _) = open_channel_and_position( + app.clone(), + coordinator.clone(), + app_dlc_collateral, + coordinator_dlc_collateral, + Some(fee_rate_sats_per_vb), + ) + .await; + + let fund_output_vout = app_signed_channel.fund_output_index; + let fund_output_amount = &app_signed_channel.fund_tx.output[fund_output_vout].value; + + // We cannot easily assert equality because both `rust-dlc` and us have to round in several + // spots. + let epsilon = *fund_output_amount as i64 - expected_fund_output_amount.to_sat() as i64; + + assert!( + epsilon.abs() < 5, + "Error out of bounds: actual {fund_output_amount} != {}", + expected_fund_output_amount.to_sat() + ); +} + async fn start_and_fund_app( amount: Amount, n_utxos: u64, From 10047c72e7ab66b47192083a8f3cf974e3a051a0 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Tue, 20 Feb 2024 00:47:21 +1100 Subject: [PATCH 3/7] fix(ln-dlc-node): Calculate DLC channel fee reserve correctly --- Cargo.lock | 10 ++++---- Cargo.toml | 10 ++++---- crates/ln-dlc-node/src/dlc_wallet.rs | 28 ++++----------------- crates/ln-dlc-node/src/tests/dlc_channel.rs | 1 - 4 files changed, 15 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 77f8bbcea..570a38e5b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1366,7 +1366,7 @@ dependencies = [ [[package]] name = "dlc" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" +source = "git+https://github.com/get10101/rust-dlc?rev=1ab4bf5#1ab4bf594b2095ca96ea18e9ebf1838bb4e27504" dependencies = [ "bitcoin 0.29.2", "miniscript 8.0.2", @@ -1378,7 +1378,7 @@ dependencies = [ [[package]] name = "dlc-manager" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" +source = "git+https://github.com/get10101/rust-dlc?rev=1ab4bf5#1ab4bf594b2095ca96ea18e9ebf1838bb4e27504" dependencies = [ "async-trait", "bitcoin 0.29.2", @@ -1394,7 +1394,7 @@ dependencies = [ [[package]] name = "dlc-messages" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" +source = "git+https://github.com/get10101/rust-dlc?rev=1ab4bf5#1ab4bf594b2095ca96ea18e9ebf1838bb4e27504" dependencies = [ "bitcoin 0.29.2", "dlc", @@ -1407,7 +1407,7 @@ dependencies = [ [[package]] name = "dlc-trie" version = "0.4.0" -source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" +source = "git+https://github.com/get10101/rust-dlc?rev=1ab4bf5#1ab4bf594b2095ca96ea18e9ebf1838bb4e27504" dependencies = [ "bitcoin 0.29.2", "dlc", @@ -2928,7 +2928,7 @@ checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" [[package]] name = "p2pd-oracle-client" version = "0.1.0" -source = "git+https://github.com/get10101/rust-dlc?rev=5931e0e#5931e0ed5d549086e455eaeeca5b970fdb53fe5f" +source = "git+https://github.com/get10101/rust-dlc?rev=1ab4bf5#1ab4bf594b2095ca96ea18e9ebf1838bb4e27504" dependencies = [ "chrono", "dlc-manager", diff --git a/Cargo.toml b/Cargo.toml index 0314512f6..618994e85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,11 +20,11 @@ resolver = "2" # We are using our own fork of `rust-dlc` at least until we can drop all the LN-DLC features. Also, # `p2pderivatives/rust-dlc#master` is missing certain patches that can only be found in the LN-DLC # branch. -dlc-manager = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } -dlc-messages = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } -dlc = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } -p2pd-oracle-client = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } -dlc-trie = { git = "https://github.com/get10101/rust-dlc", rev = "5931e0e" } +dlc-manager = { git = "https://github.com/get10101/rust-dlc", rev = "1ab4bf5" } +dlc-messages = { git = "https://github.com/get10101/rust-dlc", rev = "1ab4bf5" } +dlc = { git = "https://github.com/get10101/rust-dlc", rev = "1ab4bf5" } +p2pd-oracle-client = { git = "https://github.com/get10101/rust-dlc", rev = "1ab4bf5" } +dlc-trie = { git = "https://github.com/get10101/rust-dlc", rev = "1ab4bf5" } # We should usually track the `p2pderivatives/split-tx-experiment[-10101]` branch. For now we depend # on a special fork which removes a panic in `rust-lightning`. diff --git a/crates/ln-dlc-node/src/dlc_wallet.rs b/crates/ln-dlc-node/src/dlc_wallet.rs index e4eab311c..e55bb4a5e 100644 --- a/crates/ln-dlc-node/src/dlc_wallet.rs +++ b/crates/ln-dlc-node/src/dlc_wallet.rs @@ -26,8 +26,6 @@ use bdk_coin_select::Target; use bitcoin::secp256k1::KeyPair; use bitcoin::Network; use bitcoin::TxIn; -use bitcoin::VarInt; -use lightning::chain::chaininterface::ConfirmationTarget; use ln_dlc_storage::DlcStorageProvider; use ln_dlc_storage::WalletStorage; use std::sync::Arc; @@ -180,16 +178,12 @@ impl dlc_manager::Wallet for DlcWallet, + base_weight_wu: u64, lock_utxos: bool, ) -> Result, dlc_manager::error::Error> { let network = self.on_chain_wallet.network(); - let fee_rate = fee_rate.map(|fee_rate| fee_rate as f32).unwrap_or_else(|| { - self.on_chain_wallet - .fee_rate_estimator - .get(ConfirmationTarget::Normal) - .as_sat_per_vb() - }); + let fee_rate = fee_rate.expect("always set by rust-dlc"); // Get temporarily reserved UTXOs from in-memory storage. let mut reserved_outpoints = self.on_chain_wallet.locked_utxos.lock(); @@ -211,15 +205,7 @@ impl dlc_manager::Wallet for DlcWallet dlc_manager::Wallet for DlcWallet>(); - // This is a standard base weight (without inputs or change outputs) for on-chain DLCs. We - // assume that this value is still correct for DLC channels. - let funding_tx_base_weight = 212; - let target = Target { - feerate: bdk_coin_select::FeeRate::from_sat_per_vb(fee_rate), + feerate: bdk_coin_select::FeeRate::from_sat_per_vb(fee_rate as f32), min_fee: 0, value: amount, }; - let mut coin_selector = CoinSelector::new(&candidates, funding_tx_base_weight); + let mut coin_selector = CoinSelector::new(&candidates, base_weight_wu as u32); let dust_limit = 0; let long_term_feerate = bdk_coin_select::FeeRate::default_min_relay_fee(); diff --git a/crates/ln-dlc-node/src/tests/dlc_channel.rs b/crates/ln-dlc-node/src/tests/dlc_channel.rs index 2dc844322..4f6f00b78 100644 --- a/crates/ln-dlc-node/src/tests/dlc_channel.rs +++ b/crates/ln-dlc-node/src/tests/dlc_channel.rs @@ -314,7 +314,6 @@ async fn funding_transaction_pays_expected_fees() { #[tokio::test(flavor = "multi_thread")] #[ignore] -#[should_panic] async fn dlc_channel_includes_expected_fee_reserve() { init_tracing(); From a9d20ffcc4a17c29662e28773171eeabb1a1cd4c Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Tue, 20 Feb 2024 01:01:35 +1100 Subject: [PATCH 4/7] chore: Extract function to estimate DLC channel fee reserve --- crates/ln-dlc-node/src/node/dlc_channel.rs | 38 +++++++++++++++++++++ crates/ln-dlc-node/src/tests/dlc_channel.rs | 27 ++------------- 2 files changed, 40 insertions(+), 25 deletions(-) diff --git a/crates/ln-dlc-node/src/node/dlc_channel.rs b/crates/ln-dlc-node/src/node/dlc_channel.rs index 30d3142c2..263765274 100644 --- a/crates/ln-dlc-node/src/node/dlc_channel.rs +++ b/crates/ln-dlc-node/src/node/dlc_channel.rs @@ -818,3 +818,41 @@ pub fn send_dlc_message Amount { + let buffer_weight_wu = dlc::channel::BUFFER_TX_WEIGHT; + + let cet_or_refund_weight_wu = { + let cet_or_refund_base_weight_wu = dlc::CET_BASE_WEIGHT; + // Because the CET spends from a buffer transaction, compared to a regular DLC that spends + // directly from the funding transaction. + let cet_or_refund_extra_weight_wu = dlc::channel::CET_EXTRA_WEIGHT; + + // This is the standard length of a P2WPKH script pubkey. + let cet_or_refund_output_spk_bytes = 22; + + // Value = 8 bytes; var_int = 1 byte. + let cet_or_refund_output_weight_wu = (8 + 1 + cet_or_refund_output_spk_bytes) * 4; + + cet_or_refund_base_weight_wu + + cet_or_refund_extra_weight_wu + // 1 output per party. + + (2 * cet_or_refund_output_weight_wu) + }; + + let total_weight_vb = (buffer_weight_wu + cet_or_refund_weight_wu) as f64 / 4.0; + + let total_fee_reserve = total_weight_vb * fee_rate_sats_per_vb; + let total_fee_reserve = total_fee_reserve.ceil() as u64; + + Amount::from_sat(total_fee_reserve) +} diff --git a/crates/ln-dlc-node/src/tests/dlc_channel.rs b/crates/ln-dlc-node/src/tests/dlc_channel.rs index 4f6f00b78..94ccb6872 100644 --- a/crates/ln-dlc-node/src/tests/dlc_channel.rs +++ b/crates/ln-dlc-node/src/tests/dlc_channel.rs @@ -1,4 +1,5 @@ use crate::bitcoin_conversion::to_secp_pk_29; +use crate::node::dlc_channel::estimated_dlc_channel_fee_reserve; use crate::node::InMemoryStore; use crate::node::Node; use crate::node::RunningNode; @@ -324,31 +325,7 @@ async fn dlc_channel_includes_expected_fee_reserve() { // for transaction fees. let fee_rate_sats_per_vb = 2; - let buffer_weight_wu = dlc::channel::BUFFER_TX_WEIGHT; - - let cet_or_refund_weight_wu = { - let cet_or_refund_base_weight_wu = dlc::CET_BASE_WEIGHT; - // Because we are spending from a buffer transaction. - let cet_or_refund_extra_weight_wu = dlc::channel::CET_EXTRA_WEIGHT; - - // This is the standard length of a P2WPKH script pubkey. - let cet_or_refund_output_spk_bytes = 22; - - // Value = 8 bytes; var_int = 1 byte. - let cet_or_refund_output_weight_wu = (8 + 1 + cet_or_refund_output_spk_bytes) * 4; - - cet_or_refund_base_weight_wu - + cet_or_refund_extra_weight_wu - // 1 output per party. - + (2 * cet_or_refund_output_weight_wu) - }; - - let total_fee_reserve = { - let total_weight_vb = (buffer_weight_wu + cet_or_refund_weight_wu) as f32 / 4.0; - let total_weight_vb = total_weight_vb.ceil() as u64; - - Amount::from_sat(total_weight_vb * fee_rate_sats_per_vb) - }; + let total_fee_reserve = estimated_dlc_channel_fee_reserve(fee_rate_sats_per_vb as f64); let expected_fund_output_amount = app_dlc_collateral + coordinator_dlc_collateral + total_fee_reserve; From b42ff545796fea66d0d143f49f0fd0ea0c3c8017 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Tue, 20 Feb 2024 01:38:26 +1100 Subject: [PATCH 5/7] feat(app): Display fee reserve and funding TX fee estimate The funding tx fee is just an estimate and it is usually slightly overestimated. We've had to further increase the size of the bottom sheets to fit all the information on screen. --- crates/ln-dlc-node/src/node/dlc_channel.rs | 27 +++++++++ mobile/lib/common/dlc_channel_service.dart | 9 +++ .../features/trade/channel_configuration.dart | 31 ++++++++++- .../features/trade/trade_bottom_sheet.dart | 2 +- .../trade_bottom_sheet_confirmation.dart | 55 +++++++++++++++---- mobile/lib/util/constants.dart | 2 +- mobile/native/src/api.rs | 12 ++++ mobile/native/src/ln_dlc/mod.rs | 39 +++++++++++++ mobile/test/trade_test.dart | 8 +++ 9 files changed, 171 insertions(+), 14 deletions(-) diff --git a/crates/ln-dlc-node/src/node/dlc_channel.rs b/crates/ln-dlc-node/src/node/dlc_channel.rs index 263765274..2db019519 100644 --- a/crates/ln-dlc-node/src/node/dlc_channel.rs +++ b/crates/ln-dlc-node/src/node/dlc_channel.rs @@ -856,3 +856,30 @@ pub fn estimated_dlc_channel_fee_reserve(fee_rate_sats_per_vb: f64) -> Amount { Amount::from_sat(total_fee_reserve) } + +/// Give an estimate for the fee paid to publish a DLC channel funding transaction, given a fee +/// rate. +/// +/// This estimate is based on a funding transaction spending _two_ P2WPKH inputs (one per party) and +/// including _two_ P2WPKH change outputs (also one per party). +/// +/// Values taken from +/// https://github.com/discreetlogcontracts/dlcspecs/blob/master/Transactions.md#fees. +pub fn estimated_funding_transaction_fee(fee_rate_sats_per_vb: f64) -> Amount { + let base_weight_wu = dlc::FUND_TX_BASE_WEIGHT; + + let input_script_pubkey_length = 22; + let max_witness_length = 108; + let input_weight_wu = 164 + (4 * input_script_pubkey_length) + max_witness_length; + + let output_script_pubkey_length = 22; + let output_weight_wu = 36 + (4 * output_script_pubkey_length); + + let total_weight_wu = base_weight_wu + (input_weight_wu * 2) + (output_weight_wu * 2); + let total_weight_vb = total_weight_wu as f64 / 4.0; + + let fee = total_weight_vb * fee_rate_sats_per_vb; + let fee = fee.ceil() as u64; + + Amount::from_sat(fee) +} diff --git a/mobile/lib/common/dlc_channel_service.dart b/mobile/lib/common/dlc_channel_service.dart index 28576b9c1..c60b961aa 100644 --- a/mobile/lib/common/dlc_channel_service.dart +++ b/mobile/lib/common/dlc_channel_service.dart @@ -1,4 +1,5 @@ import 'package:get_10101/common/domain/dlc_channel.dart'; +import 'package:get_10101/common/domain/model.dart'; import 'package:get_10101/ffi.dart' as rust; class DlcChannelService { @@ -13,4 +14,12 @@ class DlcChannelService { Future deleteDlcChannel(String dlcChannelId) async { await rust.api.deleteDlcChannel(dlcChannelId: dlcChannelId); } + + Amount getEstimatedChannelFeeReserve() { + return Amount(rust.api.getEstimatedChannelFeeReserve()); + } + + Amount getEstimatedFundingTxFee() { + return Amount(rust.api.getEstimatedFundingTxFee()); + } } diff --git a/mobile/lib/features/trade/channel_configuration.dart b/mobile/lib/features/trade/channel_configuration.dart index da839feab..3e1ad576d 100644 --- a/mobile/lib/features/trade/channel_configuration.dart +++ b/mobile/lib/features/trade/channel_configuration.dart @@ -1,3 +1,5 @@ +import 'package:get_10101/common/dlc_channel_change_notifier.dart'; +import 'package:get_10101/common/dlc_channel_service.dart'; import 'package:flutter/material.dart'; import 'package:get_10101/common/amount_text_field.dart'; import 'package:get_10101/common/amount_text_input_form_field.dart'; @@ -48,7 +50,7 @@ channelConfiguration({ }, child: SingleChildScrollView( child: SizedBox( - height: 450, + height: 500, child: ChannelConfiguration( tradeValues: tradeValues, onConfirmation: onConfirmation, @@ -73,6 +75,7 @@ class ChannelConfiguration extends StatefulWidget { class _ChannelConfiguration extends State { late final LspChangeNotifier lspChangeNotifier; + late final DlcChannelChangeNotifier dlcChannelChangeNotifier; Amount minMargin = Amount.zero(); Amount counterpartyMargin = Amount.zero(); @@ -86,6 +89,10 @@ class _ChannelConfiguration extends State { Amount orderMatchingFees = Amount.zero(); + Amount channelFeeReserve = Amount.zero(); + + Amount fundingTxFee = Amount.zero(); + final _formKey = GlobalKey(); @override @@ -95,6 +102,9 @@ class _ChannelConfiguration extends State { lspChangeNotifier = context.read(); var tradeConstraints = lspChangeNotifier.channelInfoService.getTradeConstraints(); + DlcChannelService dlcChannelService = + context.read().dlcChannelService; + maxCounterpartyCollateral = Amount(tradeConstraints.maxCounterpartyMarginSats); maxOnChainSpending = Amount(tradeConstraints.maxLocalMarginSats); @@ -112,6 +122,10 @@ class _ChannelConfiguration extends State { updateCounterpartyCollateral(); + channelFeeReserve = dlcChannelService.getEstimatedChannelFeeReserve(); + + fundingTxFee = dlcChannelService.getEstimatedFundingTxFee(); + // We add this so that the confirmation slider can be enabled immediately // _if_ the form is already valid. Otherwise we have to wait for the user to // interact with the form. @@ -210,12 +224,25 @@ class _ChannelConfiguration extends State { value: openingFee, label: 'Channel-opening fee', ), + ValueDataRow( + type: ValueType.amount, + value: fundingTxFee, + label: 'Transaction fee estimate', + ), + ValueDataRow( + type: ValueType.amount, + value: channelFeeReserve, + label: 'Channel fee reserve', + ), ], ), const Divider(), ValueDataRow( type: ValueType.amount, - value: ownTotalCollateral.add(openingFee), + value: ownTotalCollateral + .add(openingFee) + .add(fundingTxFee) + .add(channelFeeReserve), label: "Total"), const SizedBox(height: 17), Column( diff --git a/mobile/lib/features/trade/trade_bottom_sheet.dart b/mobile/lib/features/trade/trade_bottom_sheet.dart index 01243160b..70a342e26 100644 --- a/mobile/lib/features/trade/trade_bottom_sheet.dart +++ b/mobile/lib/features/trade/trade_bottom_sheet.dart @@ -33,7 +33,7 @@ tradeBottomSheet({required BuildContext context, required Direction direction}) child: SizedBox( // TODO: Find a way to make height dynamic depending on the children size // This is needed because otherwise the keyboard does not push the sheet up correctly - height: 470, + height: 500, child: TradeBottomSheet(direction: direction)), ), ), diff --git a/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart b/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart index 6efe603dc..e20589873 100644 --- a/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart +++ b/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart @@ -1,3 +1,5 @@ +import 'package:get_10101/common/dlc_channel_change_notifier.dart'; +import 'package:get_10101/common/dlc_channel_service.dart'; import 'package:flutter/material.dart'; import 'package:get_10101/common/amount_text.dart'; import 'package:get_10101/common/domain/model.dart'; @@ -35,6 +37,17 @@ tradeBottomSheetConfirmation( ? tradeScreenBottomSheetConfirmationSliderButtonBuy : tradeScreenBottomSheetConfirmationSliderButtonSell; + Amount? fundingTxFee; + Amount? channelFeeReserve; + + if (tradeAction == TradeAction.openChannel) { + final DlcChannelService dlcChannelService = + context.read().dlcChannelService; + + fundingTxFee = dlcChannelService.getEstimatedFundingTxFee(); + channelFeeReserve = dlcChannelService.getEstimatedChannelFeeReserve(); + } + showModalBottomSheet( shape: const RoundedRectangleBorder( borderRadius: BorderRadius.vertical( @@ -62,7 +75,7 @@ tradeBottomSheetConfirmation( }, child: SingleChildScrollView( child: SizedBox( - height: TradeAction.closePosition == tradeAction ? 330 : 450, + height: TradeAction.closePosition == tradeAction ? 330 : 500, child: TradeBottomSheetConfirmation( direction: direction, sliderButtonKey: sliderButtonKey, @@ -70,6 +83,8 @@ tradeBottomSheetConfirmation( onConfirmation: onConfirmation, tradeAction: tradeAction, traderCollateral: channelOpeningParams?.traderCollateral, + channelFeeReserve: channelFeeReserve, + fundingTxFee: fundingTxFee, )), ), ), @@ -111,15 +126,20 @@ class TradeBottomSheetConfirmation extends StatelessWidget { final TradeAction tradeAction; final Amount? traderCollateral; + final Amount? channelFeeReserve; + final Amount? fundingTxFee; - const TradeBottomSheetConfirmation( - {required this.direction, - super.key, - required this.sliderButtonKey, - required this.sliderKey, - required this.onConfirmation, - required this.tradeAction, - this.traderCollateral}); + const TradeBottomSheetConfirmation({ + required this.direction, + super.key, + required this.sliderButtonKey, + required this.sliderKey, + required this.onConfirmation, + required this.tradeAction, + this.traderCollateral, + this.channelFeeReserve, + this.fundingTxFee, + }); @override Widget build(BuildContext context) { @@ -199,7 +219,15 @@ class TradeBottomSheetConfirmation extends StatelessWidget { ), isChannelOpen ? ValueDataRow( - type: ValueType.amount, value: reserve, label: 'Channel reserve') + type: ValueType.amount, + value: reserve, + label: 'Channel collateral reserve') + : const SizedBox(height: 0), + isChannelOpen + ? ValueDataRow( + type: ValueType.amount, + value: channelFeeReserve, + label: 'Channel fee reserve') : const SizedBox(height: 0), isChannelOpen ? ValueDataRow( @@ -208,6 +236,13 @@ class TradeBottomSheetConfirmation extends StatelessWidget { label: 'Channel-opening fee', ) : const SizedBox(height: 0), + isChannelOpen + ? ValueDataRow( + type: ValueType.amount, + value: fundingTxFee, + label: 'Transaction fee estimate', + ) + : const SizedBox(height: 0), ], ), !isClose ? const Divider() : const SizedBox(height: 0), diff --git a/mobile/lib/util/constants.dart b/mobile/lib/util/constants.dart index 5dc65efa5..4364751da 100644 --- a/mobile/lib/util/constants.dart +++ b/mobile/lib/util/constants.dart @@ -47,7 +47,7 @@ const tradeScreenBottomSheetButtonBuy = Key(_trade + _bottomSheet + _button + _b const tradeScreenBottomSheetButtonSell = Key(_trade + _bottomSheet + _button + _sell); const tradeScreenBottomSheetChannelConfigurationConfirmButton = - Key(_trade + _bottomSheet + _configureChannel + _configureChannel); + Key(_trade + _bottomSheet + _configureChannel); const tradeScreenBottomSheetConfirmationConfigureChannelSlider = Key(_trade + _bottomSheet + _confirmSheet + _channelConfig + _slider + _openChannel); diff --git a/mobile/native/src/api.rs b/mobile/native/src/api.rs index 84bc8a91b..b8dd08e13 100644 --- a/mobile/native/src/api.rs +++ b/mobile/native/src/api.rs @@ -772,6 +772,18 @@ pub fn get_node_id() -> SyncReturn { SyncReturn(ln_dlc::get_node_pubkey().to_string()) } +pub fn get_estimated_channel_fee_reserve() -> Result> { + let reserve = ln_dlc::estimated_fee_reserve()?; + + Ok(SyncReturn(reserve.to_sat())) +} + +pub fn get_estimated_funding_tx_fee() -> Result> { + let fee = ln_dlc::estimated_funding_tx_fee()?; + + Ok(SyncReturn(fee.to_sat())) +} + pub fn get_expiry_timestamp(network: String) -> SyncReturn { let network = config::api::parse_network(&network); SyncReturn(commons::calculate_next_expiry(OffsetDateTime::now_utc(), network).unix_timestamp()) diff --git a/mobile/native/src/ln_dlc/mod.rs b/mobile/native/src/ln_dlc/mod.rs index b653f98da..0a8fbd342 100644 --- a/mobile/native/src/ln_dlc/mod.rs +++ b/mobile/native/src/ln_dlc/mod.rs @@ -59,6 +59,8 @@ use ln_dlc_node::bitcoin_conversion::to_tx_30; use ln_dlc_node::bitcoin_conversion::to_txid_29; use ln_dlc_node::bitcoin_conversion::to_txid_30; use ln_dlc_node::config::app_config; +use ln_dlc_node::node::dlc_channel::estimated_dlc_channel_fee_reserve; +use ln_dlc_node::node::dlc_channel::estimated_funding_transaction_fee; use ln_dlc_node::node::event::NodeEventHandler; use ln_dlc_node::node::rust_dlc_manager::channel::signed_channel::SignedChannel; use ln_dlc_node::node::rust_dlc_manager::channel::ClosedChannel; @@ -856,6 +858,24 @@ pub fn get_fee_rate_for_target(target: ConfirmationTarget) -> FeeRate { node.inner.fee_rate_estimator.get(target) } +pub fn estimated_fee_reserve() -> Result { + let node = state::get_node(); + + // Here we assume that the coordinator will use the same confirmation target AND that their fee + // rate source agrees with ours. + let fee_rate = node + .inner + .fee_rate_estimator + .get(ConfirmationTarget::Normal); + + let reserve = estimated_dlc_channel_fee_reserve(fee_rate.as_sat_per_vb() as f64); + + // The reserve is split evenly between the two parties. + let reserve = reserve / 2; + + Ok(reserve) +} + pub async fn send_payment(amount: u64, address: String, fee: Fee) -> Result { let address = Address::from_str(&address)?; @@ -867,6 +887,25 @@ pub async fn send_payment(amount: u64, address: String, fee: Fee) -> Result Result { + let node = state::get_node(); + + // Here we assume that the coordinator will use the same confirmation target AND that + // their fee rate source agrees with ours. + let fee_rate = node + .inner + .fee_rate_estimator + .get(ConfirmationTarget::Normal); + + let fee = estimated_funding_transaction_fee(fee_rate.as_sat_per_vb() as f64); + + // The estimated fee is split evenly between the two parties. In reality, each party will have + // to pay more or less depending on their inputs and change outputs. + let fee = fee / 2; + + Ok(fee) +} + pub async fn estimate_payment_fee(amount: u64, address: &str, fee: Fee) -> Result { let address = address.parse()?; diff --git a/mobile/test/trade_test.dart b/mobile/test/trade_test.dart index a21eefb95..38110fd91 100644 --- a/mobile/test/trade_test.dart +++ b/mobile/test/trade_test.dart @@ -108,6 +108,10 @@ void main() { quantity: anyNamed('quantity'), price: anyNamed('price'))) .thenReturn(Amount(42)); + when(dlcChannelService.getEstimatedChannelFeeReserve()).thenReturn((Amount(500))); + + when(dlcChannelService.getEstimatedFundingTxFee()).thenReturn((Amount(300))); + when(channelConstraintsService.getTradeConstraints()).thenAnswer((_) => const bridge.TradeConstraints( maxLocalMarginSats: 20000000000, @@ -225,6 +229,10 @@ void main() { isChannelBalance: true, minMargin: 1)); + when(dlcChannelService.getEstimatedChannelFeeReserve()).thenReturn((Amount(500))); + + when(dlcChannelService.getEstimatedFundingTxFee()).thenReturn((Amount(300))); + when(candlestickService.fetchCandles(1000)).thenAnswer((_) async { return getDummyCandles(1000); }); From 35a5333a3f19446ec9b23a47a482e00dbdab4182 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Tue, 20 Feb 2024 03:05:53 +1100 Subject: [PATCH 6/7] fix(app): Remove fee from total on confirmation bottom sheet On the confirmation bottom sheet, we tell the user that some coins will be locked up in the DLC channel. This message seems to imply that these coins will still be owned by the user. The margin and the collateral reserve make sense, but any fee paid or reserved should probably be excluded. We could summarise this information elswhere on the screen, but I don't think it is necessary as all the information is already present separately. --- .../lib/features/trade/trade_bottom_sheet_confirmation.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart b/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart index e20589873..40ee970be 100644 --- a/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart +++ b/mobile/lib/features/trade/trade_bottom_sheet_confirmation.dart @@ -161,9 +161,8 @@ class TradeBottomSheetConfirmation extends StatelessWidget { : Amount.zero(); // Fallback to 0 if we can't get the fee or the margin - Amount total = tradeValues.fee != null && tradeValues.margin != null - ? Amount(tradeValues.fee!.sats + tradeValues.margin!.sats).add(reserve) - : Amount(0); + Amount total = + tradeValues.margin != null ? Amount(tradeValues.margin!.sats).add(reserve) : Amount(0); Amount pnl = Amount(0); if (context.read().positions.containsKey(ContractSymbol.btcusd)) { final position = context.read().positions[ContractSymbol.btcusd]; From 0d8c36c74876d3c1e2033bf3fb5ee0c84628185b Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Tue, 20 Feb 2024 03:02:07 +1100 Subject: [PATCH 7/7] fix(app): Rename reservedFeeSats to fundingTxFee The value that we were displaying corresponded to half of the funding transaction fee. The fee reserve cannot be derived just by looking at the funding transaction, as it is included in the funding transaction shared output. Eventually, we should display the fee reserve in the corresponding wallet history entry (and perhaps in a channel details section too). --- mobile/lib/features/wallet/domain/wallet_history.dart | 6 +++--- mobile/lib/features/wallet/wallet_history_item.dart | 2 +- mobile/native/src/api.rs | 2 +- mobile/native/src/ln_dlc/mod.rs | 11 ++++++++--- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/mobile/lib/features/wallet/domain/wallet_history.dart b/mobile/lib/features/wallet/domain/wallet_history.dart index 20737e72a..00c64a8fa 100644 --- a/mobile/lib/features/wallet/domain/wallet_history.dart +++ b/mobile/lib/features/wallet/domain/wallet_history.dart @@ -81,7 +81,7 @@ abstract class WalletHistoryItemData { amount: amount, status: status, timestamp: timestamp, - reservedFeeSats: Amount(type.reservedFeeSats ?? 0), + fundingTxFee: Amount(type.fundingTxFeeSats ?? 0), fundingTxid: type.fundingTxid, confirmations: type.confirmations, ourChannelInputAmountSats: Amount(type.ourChannelInputAmountSats), @@ -183,7 +183,7 @@ class TradeData extends WalletHistoryItemData { } class DlcChannelFundingData extends WalletHistoryItemData { - final Amount reservedFeeSats; + final Amount fundingTxFee; final String fundingTxid; final int confirmations; final Amount ourChannelInputAmountSats; @@ -193,7 +193,7 @@ class DlcChannelFundingData extends WalletHistoryItemData { required super.amount, required super.status, required super.timestamp, - required this.reservedFeeSats, + required this.fundingTxFee, required this.confirmations, required this.ourChannelInputAmountSats, required this.fundingTxid}); diff --git a/mobile/lib/features/wallet/wallet_history_item.dart b/mobile/lib/features/wallet/wallet_history_item.dart index e410246c3..aeff7c137 100644 --- a/mobile/lib/features/wallet/wallet_history_item.dart +++ b/mobile/lib/features/wallet/wallet_history_item.dart @@ -443,7 +443,7 @@ class DlcChannelFundingHistoryItem extends WalletHistoryItem { displayWidget: TransactionIdText(data.fundingTxid)), HistoryDetail(label: "Confirmations", value: data.confirmations.toString()), HistoryDetail(label: "Channel input", value: formatSats(data.ourChannelInputAmountSats)), - HistoryDetail(label: "Reserved fee", value: formatSats(data.reservedFeeSats)), + HistoryDetail(label: "Funding TX fee estimate", value: formatSats(data.fundingTxFee)), ]; return details; diff --git a/mobile/native/src/api.rs b/mobile/native/src/api.rs index b8dd08e13..903b2b660 100644 --- a/mobile/native/src/api.rs +++ b/mobile/native/src/api.rs @@ -232,7 +232,7 @@ pub enum WalletHistoryItemType { funding_txid: String, // This fee represents the total fee reserved for all off-chain transactions, i.e. for the // fund/buffer/cet/refund. Only the part for the fund tx has been paid for now - reserved_fee_sats: Option, + funding_tx_fee_sats: Option, confirmations: u64, // The amount we hold in the channel our_channel_input_amount_sats: u64, diff --git a/mobile/native/src/ln_dlc/mod.rs b/mobile/native/src/ln_dlc/mod.rs index 0a8fbd342..9a59e352a 100644 --- a/mobile/native/src/ln_dlc/mod.rs +++ b/mobile/native/src/ln_dlc/mod.rs @@ -479,9 +479,14 @@ fn keep_wallet_balance_and_history_up_to_date(node: &Node) -> Result<()> { status, wallet_type: WalletHistoryItemType::DlcChannelFunding { funding_txid: details.transaction.txid().to_string(), - // this is not 100% correct as fees are not exactly divided by 2. The fee a - // user has to pay depends on his final address. - reserved_fee_sats: details.fee.as_ref().map(|fee| (*fee / 2).to_sat()).ok(), + // this is not 100% correct as fees are not exactly divided by 2. The share + // of the funding transaction fee that the user has paid depends on their + // inputs and change outputs. + funding_tx_fee_sats: details + .fee + .as_ref() + .map(|fee| (*fee / 2).to_sat()) + .ok(), confirmations: details.confirmation_status.n_confirmations() as u64, our_channel_input_amount_sats: channel.own_params.collateral, },