From 6a34baa270fe4fa846754b0f99a97562a9f53397 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 Sep 2023 23:02:26 +0000 Subject: [PATCH 1/9] Include an `outbound_payment` flag in `MaybeTimeoutClaimableHTLC` When the user is fetching their current balances after forwarding a payment (before it clears), they'll see a `MaybePreimageClaimableHTLC` and a `MaybeTimeoutClaimableHTLC` but if they sum up their balance using `Balance::claimable_amount_satoshis` neither will be included. Obviously, exactly one of the two balances should be included - one of the two resolutions should happen in our favor. This causes our visible balance to fluctuate up and down by the full value of any HTLCs we're in the middle of forwarding, which is incredibly confusing to see. If we want to stop the fluctuations, we need to pick one of the two balances to include. The obvious candidate is `MaybeTimeoutClaimableHTLC` as it is the lower of the two, and represents our balance without the fee we'd receive from the forward. Sadly, if we always include it, we'll end up also including any HTLCs which we've sent but which haven't yet been claimed by their recipient, which is the wrong behavior. Luckily, we have access to the `Option` while walking HTLCs, which allows us to add an `outbound_payment` flag to `MaybeTimeoutClaimableHTLC`. This allows us to only include forwarded payments in `claimable_amount_satoshis`. Sadly, even with this in place our balance still fluctuates by the changes in the commitment transaction fees we have to pay during forwarding, but addressing that is left for later. --- lightning/src/chain/channelmonitor.rs | 63 ++++++++++++++++++++------- lightning/src/ln/monitor_tests.rs | 11 +++++ 2 files changed, 58 insertions(+), 16 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 10aab828753..fb2f1145853 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -664,6 +664,10 @@ pub enum Balance { claimable_height: u32, /// The payment hash whose preimage our counterparty needs to claim this HTLC. payment_hash: PaymentHash, + /// Whether this HTLC represents a payment which was sent outbound from us. Otherwise it + /// represents an HTLC which was forwarded (and should, thus, have a corresponding inbound + /// edge on another channel). + outbound_payment: bool, }, /// HTLCs which we received from our counterparty which are claimable with a preimage which we /// do not currently have. This will only be claimable if we receive the preimage from the node @@ -693,9 +697,15 @@ pub enum Balance { } impl Balance { - /// The amount claimable, in satoshis. This excludes balances that we are unsure if we are able - /// to claim, this is because we are waiting for a preimage or for a timeout to expire. For more - /// information on these balances see [`Balance::MaybeTimeoutClaimableHTLC`] and + /// The amount claimable, in satoshis. + /// + /// For outbound payments, this excludes the balance from the possible HTLC timeout. + /// + /// For forwarded payments, this includes the balance from the possible HTLC timeout as + /// (to be conservative) that balance does not include routing fees we'd earn if we'd claim + /// the balance from a preimage in a successful forward. + /// + /// For more information on these balances see [`Balance::MaybeTimeoutClaimableHTLC`] and /// [`Balance::MaybePreimageClaimableHTLC`]. /// /// On-chain fees required to claim the balance are not included in this amount. @@ -706,9 +716,9 @@ impl Balance { Balance::ContentiousClaimable { amount_satoshis, .. }| Balance::CounterpartyRevokedOutputClaimable { amount_satoshis, .. } => *amount_satoshis, - Balance::MaybeTimeoutClaimableHTLC { .. }| - Balance::MaybePreimageClaimableHTLC { .. } - => 0, + Balance::MaybeTimeoutClaimableHTLC { amount_satoshis, outbound_payment, .. } + => if *outbound_payment { 0 } else { *amount_satoshis }, + Balance::MaybePreimageClaimableHTLC { .. } => 0, } } } @@ -1960,9 +1970,10 @@ impl ChannelMonitor { impl ChannelMonitorImpl { /// Helper for get_claimable_balances which does the work for an individual HTLC, generating up /// to one `Balance` for the HTLC. - fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, holder_commitment: bool, - counterparty_revoked_commitment: bool, confirmed_txid: Option) - -> Option { + fn get_htlc_balance(&self, htlc: &HTLCOutputInCommitment, source: Option<&HTLCSource>, + holder_commitment: bool, counterparty_revoked_commitment: bool, + confirmed_txid: Option + ) -> Option { let htlc_commitment_tx_output_idx = if let Some(v) = htlc.transaction_output_index { v } else { return None; }; @@ -2099,10 +2110,19 @@ impl ChannelMonitorImpl { confirmation_height: conf_thresh, }); } else { + let outbound_payment = match source { + None => { + debug_assert!(false, "Outbound HTLCs should have a source"); + true + }, + Some(&HTLCSource::PreviousHopData(_)) => false, + Some(&HTLCSource::OutboundRoute { .. }) => true, + }; return Some(Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: htlc.amount_msat / 1000, claimable_height: htlc.cltv_expiry, payment_hash: htlc.payment_hash, + outbound_payment, }); } } else if let Some(payment_preimage) = self.payment_preimages.get(&htlc.payment_hash) { @@ -2175,10 +2195,12 @@ impl ChannelMonitor { macro_rules! walk_htlcs { ($holder_commitment: expr, $counterparty_revoked_commitment: expr, $htlc_iter: expr) => { - for htlc in $htlc_iter { + for (htlc, source) in $htlc_iter { if htlc.transaction_output_index.is_some() { - if let Some(bal) = us.get_htlc_balance(htlc, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid) { + if let Some(bal) = us.get_htlc_balance( + htlc, source, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid + ) { res.push(bal); } } @@ -2209,9 +2231,9 @@ impl ChannelMonitor { } } if Some(txid) == us.current_counterparty_commitment_txid || Some(txid) == us.prev_counterparty_commitment_txid { - walk_htlcs!(false, false, counterparty_tx_htlcs.iter().map(|(a, _)| a)); + walk_htlcs!(false, false, counterparty_tx_htlcs.iter().map(|(a, b)| (a, b.as_ref().map(|b| &**b)))); } else { - walk_htlcs!(false, true, counterparty_tx_htlcs.iter().map(|(a, _)| a)); + walk_htlcs!(false, true, counterparty_tx_htlcs.iter().map(|(a, b)| (a, b.as_ref().map(|b| &**b)))); // The counterparty broadcasted a revoked state! // Look for any StaticOutputs first, generating claimable balances for those. // If any match the confirmed counterparty revoked to_self output, skip @@ -2251,7 +2273,7 @@ impl ChannelMonitor { } found_commitment_tx = true; } else if txid == us.current_holder_commitment_tx.txid { - walk_htlcs!(true, false, us.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, _)| a)); + walk_htlcs!(true, false, us.current_holder_commitment_tx.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref()))); if let Some(conf_thresh) = pending_commitment_tx_conf_thresh { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: us.current_holder_commitment_tx.to_self_value_sat, @@ -2261,7 +2283,7 @@ impl ChannelMonitor { found_commitment_tx = true; } else if let Some(prev_commitment) = &us.prev_holder_signed_commitment_tx { if txid == prev_commitment.txid { - walk_htlcs!(true, false, prev_commitment.htlc_outputs.iter().map(|(a, _, _)| a)); + walk_htlcs!(true, false, prev_commitment.htlc_outputs.iter().map(|(a, _, c)| (a, c.as_ref()))); if let Some(conf_thresh) = pending_commitment_tx_conf_thresh { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: prev_commitment.to_self_value_sat, @@ -2284,13 +2306,22 @@ impl ChannelMonitor { } } else { let mut claimable_inbound_htlc_value_sat = 0; - for (htlc, _, _) in us.current_holder_commitment_tx.htlc_outputs.iter() { + for (htlc, _, source) in us.current_holder_commitment_tx.htlc_outputs.iter() { if htlc.transaction_output_index.is_none() { continue; } if htlc.offered { + let outbound_payment = match source { + None => { + debug_assert!(false, "Outbound HTLCs should have a source"); + true + }, + Some(HTLCSource::PreviousHopData(_)) => false, + Some(HTLCSource::OutboundRoute { .. }) => true, + }; res.push(Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: htlc.amount_msat / 1000, claimable_height: htlc.cltv_expiry, payment_hash: htlc.payment_hash, + outbound_payment, }); } else if us.payment_preimages.get(&htlc.payment_hash).is_some() { claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000; diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 6f9a1457712..13a7f21aec5 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -402,11 +402,13 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { amount_satoshis: 3_000, claimable_height: htlc_cltv_timeout, payment_hash, + outbound_payment: true, }; let sent_htlc_timeout_balance = Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 4_000, claimable_height: htlc_cltv_timeout, payment_hash: timeout_payment_hash, + outbound_payment: true, }; let received_htlc_balance = Balance::MaybePreimageClaimableHTLC { amount_satoshis: 3_000, @@ -803,11 +805,13 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { amount_satoshis: 10_000, claimable_height: htlc_cltv_timeout, payment_hash, + outbound_payment: true, }; let htlc_balance_unknown_preimage = Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 20_000, claimable_height: htlc_cltv_timeout, payment_hash: payment_hash_2, + outbound_payment: true, }; let commitment_tx_fee = chan_feerate * @@ -950,6 +954,7 @@ fn test_no_preimage_inbound_htlc_balances() { amount_satoshis: 10_000, claimable_height: htlc_cltv_timeout, payment_hash: to_b_failed_payment_hash, + outbound_payment: true, }; let a_received_htlc_balance = Balance::MaybePreimageClaimableHTLC { amount_satoshis: 20_000, @@ -965,6 +970,7 @@ fn test_no_preimage_inbound_htlc_balances() { amount_satoshis: 20_000, claimable_height: htlc_cltv_timeout, payment_hash: to_a_failed_payment_hash, + outbound_payment: true, }; // Both A and B will have an HTLC that's claimable on timeout and one that's claimable if they @@ -1261,14 +1267,17 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ amount_satoshis: 2_000, claimable_height: missing_htlc_cltv_timeout, payment_hash: missing_htlc_payment_hash, + outbound_payment: true, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 4_000, claimable_height: htlc_cltv_timeout, payment_hash: timeout_payment_hash, + outbound_payment: true, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 5_000, claimable_height: live_htlc_cltv_timeout, payment_hash: live_payment_hash, + outbound_payment: true, }]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1811,10 +1820,12 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { amount_satoshis: 4_000, claimable_height: htlc_cltv_timeout, payment_hash: revoked_payment_hash, + outbound_payment: true, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 3_000, claimable_height: htlc_cltv_timeout, payment_hash: claimed_payment_hash, + outbound_payment: true, }]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); From 9e147a6213e3a495177a180b50526a3307f7616d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 29 Sep 2023 17:10:40 +0000 Subject: [PATCH 2/9] Drop `chan_utils` self-import There's no reason to `use` a module within that module to refer to that module... --- lightning/src/ln/chan_utils.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 4d2572e895f..6a31942069a 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -41,7 +41,6 @@ use bitcoin::{secp256k1, Sequence, Witness}; use crate::io; use core::cmp; -use crate::ln::chan_utils; use crate::util::transaction_utils::sort_outputs; use crate::ln::channel::{INITIAL_COMMITMENT_NUMBER, ANCHOR_OUTPUT_VALUE_SATOSHI}; use core::ops::Deref; @@ -810,7 +809,7 @@ pub fn get_anchor_redeemscript(funding_pubkey: &PublicKey) -> ScriptBuf { /// Locates the output with an anchor script paying to `funding_pubkey` within `commitment_tx`. pub(crate) fn get_anchor_output<'a>(commitment_tx: &'a Transaction, funding_pubkey: &PublicKey) -> Option<(u32, &'a TxOut)> { - let anchor_script = chan_utils::get_anchor_redeemscript(funding_pubkey).to_p2wsh(); + let anchor_script = get_anchor_redeemscript(funding_pubkey).to_p2wsh(); commitment_tx.output.iter().enumerate() .find(|(_, txout)| txout.script_pubkey == anchor_script) .map(|(idx, txout)| (idx as u32, txout)) @@ -818,7 +817,7 @@ pub(crate) fn get_anchor_output<'a>(commitment_tx: &'a Transaction, funding_pubk /// Returns the witness required to satisfy and spend an anchor input. pub fn build_anchor_input_witness(funding_key: &PublicKey, funding_sig: &Signature) -> Witness { - let anchor_redeem_script = chan_utils::get_anchor_redeemscript(funding_key); + let anchor_redeem_script = get_anchor_redeemscript(funding_key); let mut ret = Witness::new(); ret.push_ecdsa_signature(&BitcoinSignature::sighash_all(*funding_sig)); ret.push(anchor_redeem_script.as_bytes()); @@ -1510,7 +1509,7 @@ impl CommitmentTransaction { let mut htlcs = Vec::with_capacity(htlcs_with_aux.len()); for (htlc, _) in htlcs_with_aux { - let script = chan_utils::get_htlc_redeemscript(&htlc, &channel_parameters.channel_type_features(), &keys); + let script = get_htlc_redeemscript(&htlc, &channel_parameters.channel_type_features(), &keys); let txout = TxOut { script_pubkey: script.to_p2wsh(), value: htlc.to_bitcoin_amount(), @@ -1733,7 +1732,7 @@ impl<'a> TrustedCommitmentTransaction<'a> { &self.inner.htlcs[htlc_index], &self.channel_type_features, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key ); - chan_utils::build_htlc_input_witness( + build_htlc_input_witness( signature, counterparty_signature, preimage, &htlc_redeemscript, &self.channel_type_features, ) } From f1f28c6010139de04359aa04f98e708e15bd6a7d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 29 Sep 2023 17:26:01 +0000 Subject: [PATCH 3/9] Move commitment tx fee calculation helpers to `chan_utils` These don't really belong in `channel` as they're now used in other parts of the codebase. --- lightning/src/ln/chan_utils.rs | 31 ++++++++++++ lightning/src/ln/channel.rs | 72 ++++++++-------------------- lightning/src/ln/functional_tests.rs | 4 +- lightning/src/ln/monitor_tests.rs | 29 +++++------ lightning/src/ln/payment_tests.rs | 7 +-- 5 files changed, 73 insertions(+), 70 deletions(-) diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 6a31942069a..e8b682d2798 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -171,6 +171,37 @@ impl HTLCClaim { } } +#[cfg(not(test))] +const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172; +#[cfg(test)] +pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172; + +pub(crate) fn commitment_tx_base_weight(channel_type_features: &ChannelTypeFeatures) -> u64 { + const COMMITMENT_TX_BASE_WEIGHT: u64 = 724; + const COMMITMENT_TX_BASE_ANCHOR_WEIGHT: u64 = 1124; + if channel_type_features.supports_anchors_zero_fee_htlc_tx() { COMMITMENT_TX_BASE_ANCHOR_WEIGHT } else { COMMITMENT_TX_BASE_WEIGHT } +} + +/// Get the fee cost of a commitment tx with a given number of HTLC outputs. +/// Note that num_htlcs should not include dust HTLCs. +pub(crate) fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 { + feerate_per_kw as u64 * + (commitment_tx_base_weight(channel_type_features) + + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) + / 1000 +} + +pub(crate) fn per_outbound_htlc_counterparty_commit_tx_fee_msat(feerate_per_kw: u32, channel_type_features: &ChannelTypeFeatures) -> u64 { + // Note that we need to divide before multiplying to round properly, + // since the lowest denomination of bitcoin on-chain is the satoshi. + let commitment_tx_fee = COMMITMENT_TX_WEIGHT_PER_HTLC * feerate_per_kw as u64 / 1000 * 1000; + if channel_type_features.supports_anchors_zero_fee_htlc_tx() { + commitment_tx_fee + htlc_success_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 + } else { + commitment_tx_fee + } +} + // Various functions for key derivation and transaction creation for use within channels. Primarily // used in Channel and ChannelMonitor. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index afd4e73578f..7a03fe78536 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -32,7 +32,14 @@ use crate::ln::msgs::DecodeError; use crate::ln::script::{self, ShutdownScript}; use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails}; use crate::ln::channelmanager::{self, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT}; -use crate::ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction}; +use crate::ln::chan_utils::{ + CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, + htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, + HolderCommitmentTransaction, ChannelTransactionParameters, + CounterpartyChannelTransactionParameters, MAX_HTLCS, + get_commitment_transaction_number_obscure_factor, + ClosingTransaction, commit_tx_fee_sat, per_outbound_htlc_counterparty_commit_tx_fee_msat, +}; use crate::ln::chan_utils; use crate::ln::onion_utils::HTLCFailReason; use crate::chain::BestBlock; @@ -655,17 +662,6 @@ pub const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; pub const DEFAULT_MAX_HTLCS: u16 = 50; -pub(crate) fn commitment_tx_base_weight(channel_type_features: &ChannelTypeFeatures) -> u64 { - const COMMITMENT_TX_BASE_WEIGHT: u64 = 724; - const COMMITMENT_TX_BASE_ANCHOR_WEIGHT: u64 = 1124; - if channel_type_features.supports_anchors_zero_fee_htlc_tx() { COMMITMENT_TX_BASE_ANCHOR_WEIGHT } else { COMMITMENT_TX_BASE_WEIGHT } -} - -#[cfg(not(test))] -const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172; -#[cfg(test)] -pub const COMMITMENT_TX_WEIGHT_PER_HTLC: u64 = 172; - pub const ANCHOR_OUTPUT_VALUE_SATOSHI: u64 = 330; /// The percentage of the channel value `holder_max_htlc_value_in_flight_msat` used to be set to, @@ -1621,7 +1617,7 @@ impl ChannelContext where SP::Target: SignerProvider { 0 }; let funders_amount_msat = open_channel_fields.funding_satoshis * 1000 - msg_push_msat; - let commitment_tx_fee = commit_tx_fee_msat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000; + let commitment_tx_fee = commit_tx_fee_sat(open_channel_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commitment_tx_fee { return Err(ChannelError::close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commitment_tx_fee))); } @@ -1887,7 +1883,7 @@ impl ChannelContext where SP::Target: SignerProvider { let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target); let value_to_self_msat = channel_value_satoshis * 1000 - push_msat; - let commitment_tx_fee = commit_tx_fee_msat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type); + let commitment_tx_fee = commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) * 1000; if value_to_self_msat.saturating_sub(anchor_outputs_value_msat) < commitment_tx_fee { return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commitment_tx_fee / 1000) }); } @@ -2948,7 +2944,7 @@ impl ChannelContext where SP::Target: SignerProvider { let on_counterparty_tx_nondust_htlcs = on_counterparty_tx_accepted_nondust_htlcs + on_counterparty_tx_offered_nondust_htlcs; on_counterparty_tx_dust_exposure_msat += - commit_tx_fee_msat(excess_feerate, on_counterparty_tx_nondust_htlcs, &self.channel_type); + commit_tx_fee_sat(excess_feerate, on_counterparty_tx_nondust_htlcs, &self.channel_type) * 1000; if !self.channel_type.supports_anchors_zero_fee_htlc_tx() { on_counterparty_tx_dust_exposure_msat += on_counterparty_tx_accepted_nondust_htlcs as u64 * htlc_success_tx_weight(&self.channel_type) @@ -3313,12 +3309,12 @@ impl ChannelContext where SP::Target: SignerProvider { } let num_htlcs = included_htlcs + addl_htlcs; - let res = commit_tx_fee_msat(context.feerate_per_kw, num_htlcs, &context.channel_type); + let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, &context.channel_type) * 1000; #[cfg(any(test, fuzzing))] { let mut fee = res; if fee_spike_buffer_htlc.is_some() { - fee = commit_tx_fee_msat(context.feerate_per_kw, num_htlcs - 1, &context.channel_type); + fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, &context.channel_type) * 1000; } let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len() + context.holding_cell_htlc_updates.len(); @@ -3404,12 +3400,12 @@ impl ChannelContext where SP::Target: SignerProvider { } let num_htlcs = included_htlcs + addl_htlcs; - let res = commit_tx_fee_msat(context.feerate_per_kw, num_htlcs, &context.channel_type); + let res = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, &context.channel_type) * 1000; #[cfg(any(test, fuzzing))] { let mut fee = res; if fee_spike_buffer_htlc.is_some() { - fee = commit_tx_fee_msat(context.feerate_per_kw, num_htlcs - 1, &context.channel_type); + fee = commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, &context.channel_type) * 1000; } let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len(); let commitment_tx_info = CommitmentTxInfoCached { @@ -3675,32 +3671,6 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } -// Get the fee cost in SATS of a commitment tx with a given number of HTLC outputs. -// Note that num_htlcs should not include dust HTLCs. -#[inline] -fn commit_tx_fee_sat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 { - feerate_per_kw as u64 * (commitment_tx_base_weight(channel_type_features) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000 -} - -// Get the fee cost in MSATS of a commitment tx with a given number of HTLC outputs. -// Note that num_htlcs should not include dust HTLCs. -pub(crate) fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize, channel_type_features: &ChannelTypeFeatures) -> u64 { - // Note that we need to divide before multiplying to round properly, - // since the lowest denomination of bitcoin on-chain is the satoshi. - (commitment_tx_base_weight(channel_type_features) + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate_per_kw as u64 / 1000 * 1000 -} - -pub(crate) fn per_outbound_htlc_counterparty_commit_tx_fee_msat(feerate_per_kw: u32, channel_type_features: &ChannelTypeFeatures) -> u64 { - // Note that we need to divide before multiplying to round properly, - // since the lowest denomination of bitcoin on-chain is the satoshi. - let commitment_tx_fee = COMMITMENT_TX_WEIGHT_PER_HTLC * feerate_per_kw as u64 / 1000 * 1000; - if channel_type_features.supports_anchors_zero_fee_htlc_tx() { - commitment_tx_fee + htlc_success_tx_weight(channel_type_features) * feerate_per_kw as u64 / 1000 - } else { - commitment_tx_fee - } -} - /// Context for dual-funded channels. #[cfg(any(dual_funding, splicing))] pub(super) struct DualFundingChannelContext { @@ -7395,7 +7365,7 @@ impl Channel where && info.next_holder_htlc_id == self.context.next_holder_htlc_id && info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id && info.feerate == self.context.feerate_per_kw { - let actual_fee = commit_tx_fee_msat(self.context.feerate_per_kw, commitment_stats.num_nondust_htlcs, self.context.get_channel_type()); + let actual_fee = commit_tx_fee_sat(self.context.feerate_per_kw, commitment_stats.num_nondust_htlcs, self.context.get_channel_type()) * 1000; assert_eq!(actual_fee, info.fee); } } @@ -9596,7 +9566,7 @@ mod tests { use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint}; use crate::ln::channelmanager::{self, HTLCSource, PaymentId}; use crate::ln::channel::InitFeatures; - use crate::ln::channel::{AwaitingChannelReadyFlags, Channel, ChannelState, InboundHTLCOutput, OutboundV1Channel, InboundV1Channel, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator, HTLCUpdateAwaitingACK, commit_tx_fee_msat}; + use crate::ln::channel::{AwaitingChannelReadyFlags, Channel, ChannelState, InboundHTLCOutput, OutboundV1Channel, InboundV1Channel, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator, HTLCUpdateAwaitingACK, commit_tx_fee_sat}; use crate::ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS, MIN_THEIR_CHAN_RESERVE_SATOSHIS}; use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, NodeFeatures}; use crate::ln::msgs; @@ -9819,13 +9789,13 @@ mod tests { // the dust limit check. let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered); let local_commit_tx_fee = node_a_chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None); - let local_commit_fee_0_htlcs = commit_tx_fee_msat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.context.get_channel_type()); + let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.context.get_channel_type()) * 1000; assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs); // Finally, make sure that when Node A calculates the remote's commitment transaction fees, all // of the HTLCs are seen to be above the dust limit. node_a_chan.context.channel_transaction_parameters.is_outbound_from_holder = false; - let remote_commit_fee_3_htlcs = commit_tx_fee_msat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.context.get_channel_type()); + let remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.context.get_channel_type()) * 1000; let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered); let remote_commit_tx_fee = node_a_chan.context.next_remote_commit_tx_fee_msat(htlc_candidate, None); assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs); @@ -9848,8 +9818,8 @@ mod tests { let config = UserConfig::default(); let mut chan = OutboundV1Channel::<&TestKeysInterface>::new(&fee_est, &&keys_provider, &&keys_provider, node_id, &channelmanager::provided_init_features(&config), 10000000, 100000, 42, &config, 0, 42, None, &logger).unwrap(); - let commitment_tx_fee_0_htlcs = commit_tx_fee_msat(chan.context.feerate_per_kw, 0, chan.context.get_channel_type()); - let commitment_tx_fee_1_htlc = commit_tx_fee_msat(chan.context.feerate_per_kw, 1, chan.context.get_channel_type()); + let commitment_tx_fee_0_htlcs = commit_tx_fee_sat(chan.context.feerate_per_kw, 0, chan.context.get_channel_type()) * 1000; + let commitment_tx_fee_1_htlc = commit_tx_fee_sat(chan.context.feerate_per_kw, 1, chan.context.get_channel_type()) * 1000; // If HTLC_SUCCESS_TX_WEIGHT and HTLC_TIMEOUT_TX_WEIGHT were swapped: then this HTLC would be // counted as dust when it shouldn't be. diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 3dd16ae5c2c..99063334a28 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -20,11 +20,11 @@ use crate::chain::transaction::OutPoint; use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider}; use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason}; use crate::ln::types::{ChannelId, PaymentPreimage, PaymentSecret, PaymentHash}; -use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase}; +use crate::ln::channel::{CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase}; use crate::ln::channelmanager::{self, PaymentId, RAACommitmentOrder, PaymentSendFailure, RecipientOnionFields, BREAKDOWN_TIMEOUT, ENABLE_GOSSIP_TICKS, DISABLE_GOSSIP_TICKS, MIN_CLTV_EXPIRY_DELTA}; use crate::ln::channel::{DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, ChannelError}; use crate::ln::{chan_utils, onion_utils}; -use crate::ln::chan_utils::{OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment}; +use crate::ln::chan_utils::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, OFFERED_HTLC_SCRIPT_WEIGHT, htlc_success_tx_weight, htlc_timeout_tx_weight, HTLCOutputInCommitment}; use crate::routing::gossip::{NetworkGraph, NetworkUpdate}; use crate::routing::router::{Path, PaymentParameters, Route, RouteHop, get_route, RouteParameters}; use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, NodeFeatures}; diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 13a7f21aec5..486c0bdedcb 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -17,6 +17,7 @@ use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource}; use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, ClosureReason, HTLCDestination}; use crate::ln::channel; use crate::ln::types::ChannelId; +use crate::ln::chan_utils; use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, PaymentId, RecipientOnionFields}; use crate::ln::msgs::ChannelMessageHandler; use crate::crypto::utils::sign; @@ -236,7 +237,7 @@ fn do_chanmon_claim_value_coop_close(anchors: bool) { let chan_feerate = get_feerate!(nodes[0], nodes[1], chan_id) as u64; let channel_type_features = get_channel_type_features!(nodes[0], nodes[1], chan_id); - let commitment_tx_fee = chan_feerate * channel::commitment_tx_base_weight(&channel_type_features) / 1000; + let commitment_tx_fee = chan_feerate * chan_utils::commitment_tx_base_weight(&channel_type_features) / 1000; let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 1_000 - commitment_tx_fee - anchor_outputs_value @@ -436,7 +437,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { // Before B receives the payment preimage, it only suggests the push_msat value of 1_000 sats // as claimable. A lists both its to-self balance and the (possibly-claimable) HTLCs. let commitment_tx_fee = chan_feerate as u64 * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { 2 * channel::ANCHOR_OUTPUT_VALUE_SATOSHI } else { 0 }; assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value, @@ -480,8 +481,8 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { // Once B has received the payment preimage, it includes the value of the HTLC in its // "claimable if you were to close the channel" balance. let commitment_tx_fee = chan_feerate as u64 * - (channel::commitment_tx_base_weight(&channel_type_features) + - if prev_commitment_tx { 1 } else { 2 } * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + + if prev_commitment_tx { 1 } else { 2 } * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let mut a_expected_balances = vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - // Channel funding value in satoshis 4_000 - // The to-be-failed HTLC value in satoshis @@ -559,7 +560,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); assert!(nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); let commitment_tx_fee = chan_feerate as u64 * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value, confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, @@ -815,7 +816,7 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { }; let commitment_tx_fee = chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { 2 * channel::ANCHOR_OUTPUT_VALUE_SATOSHI } else { 0 }; assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 10_000 - 20_000 - commitment_tx_fee - anchor_outputs_value, @@ -979,7 +980,7 @@ fn test_no_preimage_inbound_htlc_balances() { assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, }, a_received_htlc_balance.clone(), a_sent_htlc_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -999,7 +1000,7 @@ fn test_no_preimage_inbound_htlc_balances() { let node_a_commitment_claimable = nodes[0].best_block_info().1 + BREAKDOWN_TIMEOUT as u32; let as_pre_spend_claims = sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, }, a_received_htlc_balance.clone(), a_sent_htlc_balance.clone()]); @@ -1071,7 +1072,7 @@ fn test_no_preimage_inbound_htlc_balances() { let as_timeout_claimable_height = nodes[0].best_block_info().1 + (BREAKDOWN_TIMEOUT as u32) - 1; assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, }, a_received_htlc_balance.clone(), Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, @@ -1082,7 +1083,7 @@ fn test_no_preimage_inbound_htlc_balances() { mine_transaction(&nodes[0], &bs_htlc_timeout_claim[0]); assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, }, a_received_htlc_balance.clone(), Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, @@ -1098,7 +1099,7 @@ fn test_no_preimage_inbound_htlc_balances() { connect_blocks(&nodes[0], 1); assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, @@ -1301,7 +1302,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ fuzzy_assert_eq(claim_txn[3].weight().to_wu(), BS_TO_SELF_CLAIM_EXP_WEIGHT); let commitment_tx_fee = chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 3 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 3 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; let inbound_htlc_claim_fee = chan_feerate * inbound_htlc_claim_exp_weight / 1000; let outbound_htlc_claim_fee = chan_feerate * outbound_htlc_claim_exp_weight / 1000; @@ -1561,7 +1562,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { // transaction our balance tracking doesn't use the on-chain value so the // `CounterpartyRevokedOutputClaimable` entry doesn't change. let commitment_tx_fee = chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; let as_balances = sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in B's revoked commitment @@ -1857,7 +1858,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { let to_remote_maturity = nodes[1].best_block_info().1 + ANTI_REORG_DELAY - 1; let commitment_tx_fee = chan_feerate * - (channel::commitment_tx_base_weight(&channel_type_features) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in A's revoked commitment diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 55dee794612..3ab055d2ada 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -15,11 +15,12 @@ use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen}; use crate::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS}; use crate::sign::EntropySource; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason, PaymentPurpose}; -use crate::ln::channel::{EXPIRE_PREV_CONFIG_TICKS, commit_tx_fee_msat, get_holder_selected_channel_reserve_satoshis, ANCHOR_OUTPUT_VALUE_SATOSHI}; +use crate::ln::channel::{EXPIRE_PREV_CONFIG_TICKS, get_holder_selected_channel_reserve_satoshis, ANCHOR_OUTPUT_VALUE_SATOSHI}; use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; use crate::ln::features::{Bolt11InvoiceFeatures, ChannelTypeFeatures}; use crate::ln::msgs; use crate::ln::types::{ChannelId, PaymentHash, PaymentSecret, PaymentPreimage}; +use crate::ln::chan_utils; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::onion_utils; use crate::ln::outbound_payment::{IDEMPOTENCY_TIMEOUT_TICKS, Retry}; @@ -4182,9 +4183,9 @@ fn test_htlc_forward_considers_anchor_outputs_value() { let (_, _, chan_id_2, _) = create_announced_chan_between_nodes_with_value(&nodes, 1, 2, CHAN_AMT, PUSH_MSAT); let channel_reserve_msat = get_holder_selected_channel_reserve_satoshis(CHAN_AMT, &config) * 1000; - let commitment_fee_msat = commit_tx_fee_msat( + let commitment_fee_msat = chan_utils::commit_tx_fee_sat( *nodes[1].fee_estimator.sat_per_kw.lock().unwrap(), 2, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() - ); + ) * 1000; let anchor_outpus_value_msat = ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000; let sendable_balance_msat = CHAN_AMT * 1000 - PUSH_MSAT - channel_reserve_msat - commitment_fee_msat - anchor_outpus_value_msat; let channel_details = nodes[1].node.list_channels().into_iter().find(|channel| channel.channel_id == chan_id_2).unwrap(); From a39357e08a86a71e80451ab56f36a17f36178a55 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 29 Sep 2023 17:54:24 +0000 Subject: [PATCH 4/9] Add tx fee information to `Balance::ClaimableOnChannelClose` `Balance::ClaimableOnChannelClose` excludes the commitment transaction fee, which makes it hard to use for current balance calculation. Here we add it, setting the value to zero for inbound channels (i.e. ones for which we don't pay the fee). --- lightning/src/chain/channelmonitor.rs | 32 +++++++++++++++++++++++---- lightning/src/ln/channel.rs | 4 ++-- lightning/src/ln/monitor_tests.rs | 19 +++++++++++----- 3 files changed, 44 insertions(+), 11 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index fb2f1145853..e12873d2f79 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -622,6 +622,13 @@ pub enum Balance { /// The amount available to claim, in satoshis, excluding the on-chain fees which will be /// required to do so. amount_satoshis: u64, + /// The transaction fee we pay for the closing commitment transaction. This amount is not + /// included in the [`Balance::ClaimableOnChannelClose::amount_satoshis`] value. + /// + /// Note that if this channel is inbound (and thus our counterparty pays the commitment + /// transaction fee) this value will be zero. For [`ChannelMonitor`]s created prior to LDK + /// 0.0.124, the channel is always treated as outbound (and thus this value is never zero). + transaction_fee_satoshis: u64, }, /// The channel has been closed, and the given balance is ours but awaiting confirmations until /// we consider it spendable. @@ -912,6 +919,10 @@ pub(crate) struct ChannelMonitorImpl { // of block connection between ChannelMonitors and the ChannelManager. funding_spend_seen: bool, + /// True if the commitment transaction fee is paid by us. + /// Added in 0.0.124. + holder_pays_commitment_tx_fee: Option, + /// Set to `Some` of the confirmed transaction spending the funding input of the channel after /// reaching `ANTI_REORG_DELAY` confirmations. funding_spend_confirmed: Option, @@ -1160,6 +1171,7 @@ impl Writeable for ChannelMonitorImpl { (17, self.initial_counterparty_commitment_info, option), (19, self.channel_id, required), (21, self.balances_empty_height, option), + (23, self.holder_pays_commitment_tx_fee, option), }); Ok(()) @@ -1261,7 +1273,7 @@ impl ChannelMonitor { pub(crate) fn new(secp_ctx: Secp256k1, keys: Signer, shutdown_script: Option, on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, ScriptBuf), - channel_parameters: &ChannelTransactionParameters, + channel_parameters: &ChannelTransactionParameters, holder_pays_commitment_tx_fee: bool, funding_redeemscript: ScriptBuf, channel_value_satoshis: u64, commitment_transaction_number_obscure_factor: u64, initial_holder_commitment_tx: HolderCommitmentTransaction, @@ -1353,6 +1365,7 @@ impl ChannelMonitor { onchain_tx_handler, + holder_pays_commitment_tx_fee: Some(holder_pays_commitment_tx_fee), lockdown_from_offchain: false, holder_tx_signed: false, funding_spend_seen: false, @@ -2306,8 +2319,11 @@ impl ChannelMonitor { } } else { let mut claimable_inbound_htlc_value_sat = 0; + let mut nondust_htlc_count = 0; for (htlc, _, source) in us.current_holder_commitment_tx.htlc_outputs.iter() { - if htlc.transaction_output_index.is_none() { continue; } + if htlc.transaction_output_index.is_some() { + nondust_htlc_count += 1; + } else { continue; } if htlc.offered { let outbound_payment = match source { None => { @@ -2337,6 +2353,11 @@ impl ChannelMonitor { } res.push(Balance::ClaimableOnChannelClose { amount_satoshis: us.current_holder_commitment_tx.to_self_value_sat + claimable_inbound_htlc_value_sat, + transaction_fee_satoshis: if us.holder_pays_commitment_tx_fee.unwrap_or(true) { + chan_utils::commit_tx_fee_sat( + us.current_holder_commitment_tx.feerate_per_kw, nondust_htlc_count, + us.onchain_tx_handler.channel_type_features()) + } else { 0 }, }); } @@ -4743,6 +4764,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP let mut initial_counterparty_commitment_info = None; let mut balances_empty_height = None; let mut channel_id = None; + let mut holder_pays_commitment_tx_fee = None; read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, optional_vec), @@ -4755,6 +4777,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP (17, initial_counterparty_commitment_info, option), (19, channel_id, option), (21, balances_empty_height, option), + (23, holder_pays_commitment_tx_fee, option), }); // `HolderForceClosedWithInfo` replaced `HolderForceClosed` in v0.0.122. If we have both @@ -4824,6 +4847,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP lockdown_from_offchain, holder_tx_signed, + holder_pays_commitment_tx_fee, funding_spend_seen: funding_spend_seen.unwrap(), funding_spend_confirmed, confirmed_commitment_tx_counterparty_output, @@ -5063,7 +5087,7 @@ mod tests { let monitor = ChannelMonitor::new(Secp256k1::new(), keys, Some(ShutdownScript::new_p2wpkh_from_pubkey(shutdown_pubkey).into_inner()), 0, &ScriptBuf::new(), (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, ScriptBuf::new()), - &channel_parameters, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(&mut Vec::new()), + &channel_parameters, true, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(&mut Vec::new()), best_block, dummy_key, channel_id); let mut htlcs = preimages_slice_to_htlcs!(preimages[0..10]); @@ -5311,7 +5335,7 @@ mod tests { let monitor = ChannelMonitor::new(Secp256k1::new(), keys, Some(ShutdownScript::new_p2wpkh_from_pubkey(shutdown_pubkey).into_inner()), 0, &ScriptBuf::new(), (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, ScriptBuf::new()), - &channel_parameters, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(&mut Vec::new()), + &channel_parameters, true, ScriptBuf::new(), 46, 0, HolderCommitmentTransaction::dummy(&mut Vec::new()), best_block, dummy_key, channel_id); let chan_id = monitor.inner.lock().unwrap().channel_id(); diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7a03fe78536..daa7b63dfb4 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7861,7 +7861,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { let channel_monitor = ChannelMonitor::new(self.context.secp_ctx.clone(), monitor_signer, shutdown_script, self.context.get_holder_selected_contest_delay(), &self.context.destination_script, (funding_txo, funding_txo_script), - &self.context.channel_transaction_parameters, + &self.context.channel_transaction_parameters, self.context.is_outbound(), funding_redeemscript.clone(), self.context.channel_value_satoshis, obscure_factor, holder_commitment_tx, best_block, self.context.counterparty_node_id, self.context.channel_id()); @@ -8173,7 +8173,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { let channel_monitor = ChannelMonitor::new(self.context.secp_ctx.clone(), monitor_signer, shutdown_script, self.context.get_holder_selected_contest_delay(), &self.context.destination_script, (funding_txo, funding_txo_script.clone()), - &self.context.channel_transaction_parameters, + &self.context.channel_transaction_parameters, self.context.is_outbound(), funding_redeemscript.clone(), self.context.channel_value_satoshis, obscure_factor, holder_commitment_tx, best_block, self.context.counterparty_node_id, self.context.channel_id()); diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 486c0bdedcb..757ca41e1b9 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -240,10 +240,11 @@ fn do_chanmon_claim_value_coop_close(anchors: bool) { let commitment_tx_fee = chan_feerate * chan_utils::commitment_tx_base_weight(&channel_type_features) / 1000; let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; assert_eq!(vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 1_000_000 - 1_000 - commitment_tx_fee - anchor_outputs_value + amount_satoshis: 1_000_000 - 1_000 - commitment_tx_fee - anchor_outputs_value, + transaction_fee_satoshis: commitment_tx_fee, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); - assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000, }], + assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000, transaction_fee_satoshis: 0 }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); nodes[0].node.close_channel(&chan_id, &nodes[1].node.get_our_node_id()).unwrap(); @@ -441,10 +442,12 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { let anchor_outputs_value = if anchors { 2 * channel::ANCHOR_OUTPUT_VALUE_SATOSHI } else { 0 }; assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value, + transaction_fee_satoshis: commitment_tx_fee, }, sent_htlc_balance.clone(), sent_htlc_timeout_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000, + transaction_fee_satoshis: 0, }, received_htlc_balance.clone(), received_htlc_timeout_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -491,6 +494,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { 3 - // The dust HTLC value in satoshis commitment_tx_fee - // The commitment transaction fee with two HTLC outputs anchor_outputs_value, // The anchor outputs value in satoshis + transaction_fee_satoshis: commitment_tx_fee, }, sent_htlc_timeout_balance.clone()]; if !prev_commitment_tx { a_expected_balances.push(sent_htlc_balance.clone()); @@ -499,6 +503,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000 + 3_000 + 4_000, + transaction_fee_satoshis: 0, }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); @@ -977,15 +982,17 @@ fn test_no_preimage_inbound_htlc_balances() { // Both A and B will have an HTLC that's claimable on timeout and one that's claimable if they // receive the preimage. These will remain the same through the channel closure and until the // HTLC output is spent. - + let commitment_tx_fee = chan_feerate * + (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * - (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, + amount_satoshis: 1_000_000 - 500_000 - 10_000 - commitment_tx_fee, + transaction_fee_satoshis: commitment_tx_fee, }, a_received_htlc_balance.clone(), a_sent_htlc_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 500_000 - 20_000, + transaction_fee_satoshis: 0, }, b_received_htlc_balance.clone(), b_sent_htlc_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1264,6 +1271,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // lists the two on-chain timeout-able HTLCs as claimable balances. assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 2_000 + 3_000, + transaction_fee_satoshis: 0, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 2_000, claimable_height: missing_htlc_cltv_timeout, @@ -1817,6 +1825,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 100_000 - 4_000 - 3_000, + transaction_fee_satoshis: 0, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 4_000, claimable_height: htlc_cltv_timeout, From f5a614eb0b022a2c8f8ea170d822eb28f0b828c9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 14 Nov 2023 22:19:19 +0000 Subject: [PATCH 5/9] Include rounded msat balances in `Balance::ClaimableOnChannelClose` If we're gonna push users towards using `Balance` to determine their current balances, we really need to provide more information, including msat balances. Here we add rounded-out msat balances to the pre-close balance information --- lightning/src/chain/channelmonitor.rs | 80 ++++++++++++++++++++++----- lightning/src/ln/monitor_tests.rs | 44 ++++++++++++++- 2 files changed, 108 insertions(+), 16 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index e12873d2f79..ac70434c710 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -629,6 +629,32 @@ pub enum Balance { /// transaction fee) this value will be zero. For [`ChannelMonitor`]s created prior to LDK /// 0.0.124, the channel is always treated as outbound (and thus this value is never zero). transaction_fee_satoshis: u64, + /// The amount of millisatoshis which has been burned to fees from HTLCs which are outbound + /// from us and are related to a payment which was sent by us. This is the sum of the + /// millisatoshis part of all HTLCs which are otherwise represented by + /// [`Balance::MaybeTimeoutClaimableHTLC`] with their + /// [`Balance::MaybeTimeoutClaimableHTLC::outbound_payment`] flag set, as well as any dust + /// HTLCs which would otherwise be represented the same. + outbound_payment_htlc_rounded_msat: u64, + /// The amount of millisatoshis which has been burned to fees from HTLCs which are outbound + /// from us and are related to a forwarded HTLC. This is the sum of the millisatoshis part + /// of all HTLCs which are otherwise represented by [`Balance::MaybeTimeoutClaimableHTLC`] + /// with their [`Balance::MaybeTimeoutClaimableHTLC::outbound_payment`] flag *not* set, as + /// well as any dust HTLCs which would otherwise be represented the same. + outbound_forwarded_htlc_rounded_msat: u64, + /// The amount of millisatoshis which has been burned to fees from HTLCs which are inbound + /// to us and for which we know the preimage. This is the sum of the millisatoshis part of + /// all HTLCs which would be represented by [`Balance::ContentiousClaimable`] on channel + /// close, but whose current value is included in + /// [`Balance::ClaimableOnChannelClose::amount_satoshis`], as well as any dust HTLCs which + /// would otherwise be represented the same. + inbound_claiming_htlc_rounded_msat: u64, + /// The amount of millisatoshis which has been burned to fees from HTLCs which are inbound + /// to us and for which we do not know the preimage. This is the sum of the millisatoshis + /// part of all HTLCs which would be represented by [`Balance::MaybePreimageClaimableHTLC`] + /// on channel close, as well as any dust HTLCs which would otherwise be represented the + /// same. + inbound_htlc_rounded_msat: u64, }, /// The channel has been closed, and the given balance is ours but awaiting confirmations until /// we consider it spendable. @@ -2320,10 +2346,17 @@ impl ChannelMonitor { } else { let mut claimable_inbound_htlc_value_sat = 0; let mut nondust_htlc_count = 0; + let mut outbound_payment_htlc_rounded_msat = 0; + let mut outbound_forwarded_htlc_rounded_msat = 0; + let mut inbound_claiming_htlc_rounded_msat = 0; + let mut inbound_htlc_rounded_msat = 0; for (htlc, _, source) in us.current_holder_commitment_tx.htlc_outputs.iter() { if htlc.transaction_output_index.is_some() { nondust_htlc_count += 1; - } else { continue; } + } + let rounded_value_msat = if htlc.transaction_output_index.is_none() { + htlc.amount_msat + } else { htlc.amount_msat % 1000 }; if htlc.offered { let outbound_payment = match source { None => { @@ -2333,22 +2366,35 @@ impl ChannelMonitor { Some(HTLCSource::PreviousHopData(_)) => false, Some(HTLCSource::OutboundRoute { .. }) => true, }; - res.push(Balance::MaybeTimeoutClaimableHTLC { - amount_satoshis: htlc.amount_msat / 1000, - claimable_height: htlc.cltv_expiry, - payment_hash: htlc.payment_hash, - outbound_payment, - }); + if outbound_payment { + outbound_payment_htlc_rounded_msat += rounded_value_msat; + } else { + outbound_forwarded_htlc_rounded_msat += rounded_value_msat; + } + if htlc.transaction_output_index.is_some() { + res.push(Balance::MaybeTimeoutClaimableHTLC { + amount_satoshis: htlc.amount_msat / 1000, + claimable_height: htlc.cltv_expiry, + payment_hash: htlc.payment_hash, + outbound_payment, + }); + } } else if us.payment_preimages.get(&htlc.payment_hash).is_some() { - claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000; + inbound_claiming_htlc_rounded_msat += rounded_value_msat; + if htlc.transaction_output_index.is_some() { + claimable_inbound_htlc_value_sat += htlc.amount_msat / 1000; + } } else { - // As long as the HTLC is still in our latest commitment state, treat - // it as potentially claimable, even if it has long-since expired. - res.push(Balance::MaybePreimageClaimableHTLC { - amount_satoshis: htlc.amount_msat / 1000, - expiry_height: htlc.cltv_expiry, - payment_hash: htlc.payment_hash, - }); + inbound_htlc_rounded_msat += rounded_value_msat; + if htlc.transaction_output_index.is_some() { + // As long as the HTLC is still in our latest commitment state, treat + // it as potentially claimable, even if it has long-since expired. + res.push(Balance::MaybePreimageClaimableHTLC { + amount_satoshis: htlc.amount_msat / 1000, + expiry_height: htlc.cltv_expiry, + payment_hash: htlc.payment_hash, + }); + } } } res.push(Balance::ClaimableOnChannelClose { @@ -2358,6 +2404,10 @@ impl ChannelMonitor { us.current_holder_commitment_tx.feerate_per_kw, nondust_htlc_count, us.onchain_tx_handler.channel_type_features()) } else { 0 }, + outbound_payment_htlc_rounded_msat, + outbound_forwarded_htlc_rounded_msat, + inbound_claiming_htlc_rounded_msat, + inbound_htlc_rounded_msat, }); } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 757ca41e1b9..8af3c84fb17 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -242,9 +242,19 @@ fn do_chanmon_claim_value_coop_close(anchors: bool) { assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 1_000 - commitment_tx_fee - anchor_outputs_value, transaction_fee_satoshis: commitment_tx_fee, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); - assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000, transaction_fee_satoshis: 0 }], + assert_eq!(vec![Balance::ClaimableOnChannelClose { + amount_satoshis: 1_000, transaction_fee_satoshis: 0, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, + }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); nodes[0].node.close_channel(&chan_id, &nodes[1].node.get_our_node_id()).unwrap(); @@ -443,11 +453,19 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value, transaction_fee_satoshis: commitment_tx_fee, + outbound_payment_htlc_rounded_msat: 3000, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }, sent_htlc_balance.clone(), sent_htlc_timeout_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000, transaction_fee_satoshis: 0, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 3000, }, received_htlc_balance.clone(), received_htlc_timeout_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -495,6 +513,10 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { commitment_tx_fee - // The commitment transaction fee with two HTLC outputs anchor_outputs_value, // The anchor outputs value in satoshis transaction_fee_satoshis: commitment_tx_fee, + outbound_payment_htlc_rounded_msat: 3000, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }, sent_htlc_timeout_balance.clone()]; if !prev_commitment_tx { a_expected_balances.push(sent_htlc_balance.clone()); @@ -504,6 +526,10 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000 + 3_000 + 4_000, transaction_fee_satoshis: 0, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 3000, + inbound_htlc_rounded_msat: 0, }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); @@ -987,12 +1013,20 @@ fn test_no_preimage_inbound_htlc_balances() { assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 1_000_000 - 500_000 - 10_000 - commitment_tx_fee, transaction_fee_satoshis: commitment_tx_fee, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }, a_received_htlc_balance.clone(), a_sent_htlc_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 500_000 - 20_000, transaction_fee_satoshis: 0, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }, b_received_htlc_balance.clone(), b_sent_htlc_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1272,6 +1306,10 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 2_000 + 3_000, transaction_fee_satoshis: 0, + outbound_payment_htlc_rounded_msat: 3000, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 2_000, claimable_height: missing_htlc_cltv_timeout, @@ -1826,6 +1864,10 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { amount_satoshis: 100_000 - 4_000 - 3_000, transaction_fee_satoshis: 0, + outbound_payment_htlc_rounded_msat: 0, + outbound_forwarded_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 0, + inbound_htlc_rounded_msat: 0, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 4_000, claimable_height: htlc_cltv_timeout, From fe73ce789b5f93089fa35a8e480fe5eac2681f8a Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Wed, 7 Aug 2024 09:56:46 +0200 Subject: [PATCH 6/9] Test rounded msat balances --- lightning/src/ln/monitor_tests.rs | 72 ++++++++++++++++++------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 8af3c84fb17..6ab554cbd4e 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -397,10 +397,10 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(ChannelId::v1_from_funding_outpoint(funding_outpoint), chan_id); // This HTLC is immediately claimed, giving node B the preimage - let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 3_000_000); + let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 3_000_100); // This HTLC is allowed to time out, letting A claim it. However, in order to test claimable // balances more fully we also give B the preimage for this HTLC. - let (timeout_payment_preimage, timeout_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 4_000_000); + let (timeout_payment_preimage, timeout_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 4_000_200); // This HTLC will be dust, and not be claimable at all: let (dust_payment_preimage, dust_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 3_000); @@ -451,9 +451,9 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; let anchor_outputs_value = if anchors { 2 * channel::ANCHOR_OUTPUT_VALUE_SATOSHI } else { 0 }; assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value, + amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value - 1 /* msat amount that is burned to fees */, transaction_fee_satoshis: commitment_tx_fee, - outbound_payment_htlc_rounded_msat: 3000, + outbound_payment_htlc_rounded_msat: 3300, outbound_forwarded_htlc_rounded_msat: 0, inbound_claiming_htlc_rounded_msat: 0, inbound_htlc_rounded_msat: 0, @@ -465,13 +465,13 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { outbound_payment_htlc_rounded_msat: 0, outbound_forwarded_htlc_rounded_msat: 0, inbound_claiming_htlc_rounded_msat: 0, - inbound_htlc_rounded_msat: 3000, + inbound_htlc_rounded_msat: 3300, }, received_htlc_balance.clone(), received_htlc_timeout_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); nodes[1].node.claim_funds(payment_preimage); check_added_monitors!(nodes[1], 1); - expect_payment_claimed!(nodes[1], payment_hash, 3_000_000); + expect_payment_claimed!(nodes[1], payment_hash, 3_000_100); let b_htlc_msgs = get_htlc_update_msgs!(&nodes[1], nodes[0].node.get_our_node_id()); // We claim the dust payment here as well, but it won't impact our claimable balances as its @@ -482,7 +482,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { nodes[1].node.claim_funds(timeout_payment_preimage); check_added_monitors!(nodes[1], 1); - expect_payment_claimed!(nodes[1], timeout_payment_hash, 4_000_000); + expect_payment_claimed!(nodes[1], timeout_payment_hash, 4_000_200); if prev_commitment_tx { // To build a previous commitment transaction, deliver one round of commitment messages. @@ -511,9 +511,11 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { 1_000 - // The push_msat value in satoshis 3 - // The dust HTLC value in satoshis commitment_tx_fee - // The commitment transaction fee with two HTLC outputs - anchor_outputs_value, // The anchor outputs value in satoshis + anchor_outputs_value - // The anchor outputs value in satoshis + 1, // The rounded up msat part of the one HTLC transaction_fee_satoshis: commitment_tx_fee, - outbound_payment_htlc_rounded_msat: 3000, + outbound_payment_htlc_rounded_msat: 3000 + if prev_commitment_tx { + 200 /* 1 to-be-failed HTLC */ } else { 300 /* 2 HTLCs */ }, outbound_forwarded_htlc_rounded_msat: 0, inbound_claiming_htlc_rounded_msat: 0, inbound_htlc_rounded_msat: 0, @@ -528,7 +530,8 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { transaction_fee_satoshis: 0, outbound_payment_htlc_rounded_msat: 0, outbound_forwarded_htlc_rounded_msat: 0, - inbound_claiming_htlc_rounded_msat: 3000, + inbound_claiming_htlc_rounded_msat: 3000 + if prev_commitment_tx { + 200 /* 1 HTLC */ } else { 300 /* 2 HTLCs */ }, inbound_htlc_rounded_msat: 0, }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); @@ -593,7 +596,14 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { let commitment_tx_fee = chan_feerate as u64 * (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { - amount_satoshis: 1_000_000 - 3_000 - 4_000 - 1_000 - 3 - commitment_tx_fee - anchor_outputs_value, + amount_satoshis: 1_000_000 - // Channel funding value in satoshis + 4_000 - // The to-be-failed HTLC value in satoshis + 3_000 - // The claimed HTLC value in satoshis + 1_000 - // The push_msat value in satoshis + 3 - // The dust HTLC value in satoshis + commitment_tx_fee - // The commitment transaction fee with two HTLC outputs + anchor_outputs_value - // The anchor outputs value in satoshis + 1, // The rounded up msat parts of HTLCs confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, }, sent_htlc_balance.clone(), sent_htlc_timeout_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1240,8 +1250,8 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // are all currently claimed in separate transactions, which helps us test as we can claim // HTLCs individually. - let (claimed_payment_preimage, claimed_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 3_000_000); - let timeout_payment_hash = route_payment(&nodes[1], &[&nodes[0]], 4_000_000).1; + let (claimed_payment_preimage, claimed_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 3_000_100); + let timeout_payment_hash = route_payment(&nodes[1], &[&nodes[0]], 4_000_200).1; let dust_payment_hash = route_payment(&nodes[1], &[&nodes[0]], 3_000).1; let htlc_cltv_timeout = nodes[0].best_block_info().1 + TEST_FINAL_CLTV + 1; // Note ChannelManager adds one to CLTV timeouts for safety @@ -1262,7 +1272,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ let missing_htlc_payment_hash = route_payment(&nodes[1], &[&nodes[0]], 2_000_000).1; nodes[1].node.claim_funds(claimed_payment_preimage); - expect_payment_claimed!(nodes[1], claimed_payment_hash, 3_000_000); + expect_payment_claimed!(nodes[1], claimed_payment_hash, 3_000_100); check_added_monitors!(nodes[1], 1); let _b_htlc_msgs = get_htlc_update_msgs!(&nodes[1], nodes[0].node.get_our_node_id()); @@ -1304,11 +1314,11 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // Prior to channel closure, B considers the preimage HTLC as its own, and otherwise only // lists the two on-chain timeout-able HTLCs as claimable balances. assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 2_000 + 3_000, + amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 2_000 + 3_000 - 1 /* rounded up msat parts of HTLCs */, transaction_fee_satoshis: 0, - outbound_payment_htlc_rounded_msat: 3000, + outbound_payment_htlc_rounded_msat: 3200, outbound_forwarded_htlc_rounded_msat: 0, - inbound_claiming_htlc_rounded_msat: 0, + inbound_claiming_htlc_rounded_msat: 100, inbound_htlc_rounded_msat: 0, }, Balance::MaybeTimeoutClaimableHTLC { amount_satoshis: 2_000, @@ -1358,7 +1368,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // claim balances separated out. let expected_balance = vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in A's revoked commitment - amount_satoshis: 100_000 - 5_000 - 4_000 - 3, + amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: nodes[1].best_block_info().1 + 5, }, Balance::CounterpartyRevokedOutputClaimable { amount_satoshis: 3_000, @@ -1367,7 +1377,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ }]; let to_self_unclaimed_balance = Balance::CounterpartyRevokedOutputClaimable { - amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value, + amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - 1 /* rounded up msat parts of HTLCs */, }; let to_self_claimed_avail_height; let largest_htlc_unclaimed_balance = Balance::CounterpartyRevokedOutputClaimable { @@ -1396,7 +1406,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ confirmation_height: largest_htlc_claimed_avail_height, }; let to_self_claimed_balance = Balance::ClaimableAwaitingConfirmations { - amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - to_self_claim_fee, + amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - to_self_claim_fee - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_self_claimed_avail_height, }; @@ -1423,10 +1433,10 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in A's revoked commitment - amount_satoshis: 100_000 - 5_000 - 4_000 - 3, + amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: nodes[1].best_block_info().1 + 1, }, Balance::ClaimableAwaitingConfirmations { - amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - to_self_claim_fee, + amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - to_self_claim_fee - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_self_claimed_avail_height, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 3_000 - outbound_htlc_claim_fee, @@ -1517,7 +1527,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { let funding_outpoint = OutPoint { txid: funding_tx.txid(), index: 0 }; assert_eq!(ChannelId::v1_from_funding_outpoint(funding_outpoint), chan_id); - let payment_preimage = route_payment(&nodes[0], &[&nodes[1]], 3_000_000).0; + let payment_preimage = route_payment(&nodes[0], &[&nodes[1]], 3_000_100).0; let failed_payment_hash = route_payment(&nodes[1], &[&nodes[0]], 1_000_000).1; let revoked_local_txn = get_local_commitment_txn!(nodes[1], chan_id); assert_eq!(revoked_local_txn[0].input.len(), 1); @@ -1612,7 +1622,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; let as_balances = sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in B's revoked commitment - amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value, + amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - 1 /* The rounded up msat part of the one HTLC */, confirmation_height: to_remote_conf_height, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in B's revoked commitment @@ -1657,7 +1667,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { mine_transaction(&nodes[0], &as_htlc_claim_tx[0]); assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in B's revoked commitment - amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value, + amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_remote_conf_height, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in B's revoked commitment @@ -1823,7 +1833,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { // transaction, and one which we will not, allowing B to claim the HTLC output in an aggregated // revocation-claim transaction. - let (claimed_payment_preimage, claimed_payment_hash, ..) = route_payment(&nodes[1], &[&nodes[0]], 3_000_000); + let (claimed_payment_preimage, claimed_payment_hash, ..) = route_payment(&nodes[1], &[&nodes[0]], 3_000_100); let revoked_payment_hash = route_payment(&nodes[1], &[&nodes[0]], 4_000_000).1; let htlc_cltv_timeout = nodes[1].best_block_info().1 + TEST_FINAL_CLTV + 1; // Note ChannelManager adds one to CLTV timeouts for safety @@ -1857,14 +1867,14 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { commitment_signed_dance!(nodes[1], nodes[0], fee_update.commitment_signed, false); nodes[0].node.claim_funds(claimed_payment_preimage); - expect_payment_claimed!(nodes[0], claimed_payment_hash, 3_000_000); + expect_payment_claimed!(nodes[0], claimed_payment_hash, 3_000_100); check_added_monitors!(nodes[0], 1); let _a_htlc_msgs = get_htlc_update_msgs!(&nodes[0], nodes[1].node.get_our_node_id()); assert_eq!(sorted_vec(vec![Balance::ClaimableOnChannelClose { - amount_satoshis: 100_000 - 4_000 - 3_000, + amount_satoshis: 100_000 - 4_000 - 3_000 - 1 /* rounded up msat parts of HTLCs */, transaction_fee_satoshis: 0, - outbound_payment_htlc_rounded_msat: 0, + outbound_payment_htlc_rounded_msat: 100, outbound_forwarded_htlc_rounded_msat: 0, inbound_claiming_htlc_rounded_msat: 0, inbound_htlc_rounded_msat: 0, @@ -1913,7 +1923,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { let anchor_outputs_value = if anchors { channel::ANCHOR_OUTPUT_VALUE_SATOSHI * 2 } else { 0 }; assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in A's revoked commitment - amount_satoshis: 100_000 - 4_000 - 3_000, + amount_satoshis: 100_000 - 4_000 - 3_000 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_remote_maturity, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in A's revoked commitment @@ -1968,7 +1978,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in A's revoked commitment - amount_satoshis: 100_000 - 4_000 - 3_000, + amount_satoshis: 100_000 - 4_000 - 3_000 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_remote_maturity, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in A's revoked commitment From 8b729b606f8b6d857e964ebdce47206808a49274 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 29 Sep 2023 18:31:21 +0000 Subject: [PATCH 7/9] Explain how rounded millisat values are included in overall balance --- lightning/src/chain/channelmonitor.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index ac70434c710..324e08a542c 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -635,12 +635,16 @@ pub enum Balance { /// [`Balance::MaybeTimeoutClaimableHTLC`] with their /// [`Balance::MaybeTimeoutClaimableHTLC::outbound_payment`] flag set, as well as any dust /// HTLCs which would otherwise be represented the same. + /// + /// This amount (rounded up to a whole satoshi value) will not be included in `amount_satoshis`. outbound_payment_htlc_rounded_msat: u64, /// The amount of millisatoshis which has been burned to fees from HTLCs which are outbound /// from us and are related to a forwarded HTLC. This is the sum of the millisatoshis part /// of all HTLCs which are otherwise represented by [`Balance::MaybeTimeoutClaimableHTLC`] /// with their [`Balance::MaybeTimeoutClaimableHTLC::outbound_payment`] flag *not* set, as /// well as any dust HTLCs which would otherwise be represented the same. + /// + /// This amount (rounded up to a whole satoshi value) will not be included in `amount_satoshis`. outbound_forwarded_htlc_rounded_msat: u64, /// The amount of millisatoshis which has been burned to fees from HTLCs which are inbound /// to us and for which we know the preimage. This is the sum of the millisatoshis part of @@ -648,12 +652,18 @@ pub enum Balance { /// close, but whose current value is included in /// [`Balance::ClaimableOnChannelClose::amount_satoshis`], as well as any dust HTLCs which /// would otherwise be represented the same. + /// + /// This amount (rounded up to a whole satoshi value) will not be included in the counterparty's + /// `amount_satoshis`. inbound_claiming_htlc_rounded_msat: u64, /// The amount of millisatoshis which has been burned to fees from HTLCs which are inbound /// to us and for which we do not know the preimage. This is the sum of the millisatoshis /// part of all HTLCs which would be represented by [`Balance::MaybePreimageClaimableHTLC`] /// on channel close, as well as any dust HTLCs which would otherwise be represented the /// same. + /// + /// This amount (rounded up to a whole satoshi value) will not be included in the counterparty's + /// `amount_satoshis`. inbound_htlc_rounded_msat: u64, }, /// The channel has been closed, and the given balance is ours but awaiting confirmations until @@ -2403,7 +2413,7 @@ impl ChannelMonitor { chan_utils::commit_tx_fee_sat( us.current_holder_commitment_tx.feerate_per_kw, nondust_htlc_count, us.onchain_tx_handler.channel_type_features()) - } else { 0 }, + } else { 0 }, outbound_payment_htlc_rounded_msat, outbound_forwarded_htlc_rounded_msat, inbound_claiming_htlc_rounded_msat, From 728192ecdc225f0ffb57b40239f5b9c170550041 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Thu, 25 Jul 2024 11:54:28 +0200 Subject: [PATCH 8/9] Indicate source of balances Introduce the `BalanceSource` enum to differentiate between force-close, coop-close, and HTLCs in `Balance::ClaimableAwaitingConfirmations`. --- lightning/src/chain/channelmonitor.rs | 25 +++++++++++ lightning/src/ln/monitor_tests.rs | 62 ++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 324e08a542c..b9ced655399 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -608,6 +608,21 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, }, ); +/// Indicates whether the balance is derived from a cooperative close, a force-close +/// (for holder or counterparty), or whether it is for an HTLC. +#[derive(Clone, Debug, PartialEq, Eq)] +#[cfg_attr(test, derive(PartialOrd, Ord))] +pub enum BalanceSource { + /// The channel was force closed by the holder. + HolderForceClosed, + /// The channel was force closed by the counterparty. + CounterpartyForceClosed, + /// The channel was cooperatively closed. + CoopClose, + /// This balance is the result of an HTLC. + Htlc, +} + /// Details about the balance(s) available for spending once the channel appears on chain. /// /// See [`ChannelMonitor::get_claimable_balances`] for more details on when these will or will not @@ -675,6 +690,8 @@ pub enum Balance { /// The height at which an [`Event::SpendableOutputs`] event will be generated for this /// amount. confirmation_height: u32, + /// Whether this balance is a result of cooperative close, a force-close, or an HTLC. + source: BalanceSource, }, /// The channel has been closed, and the given balance should be ours but awaiting spending /// transaction confirmation. If the spending transaction does not confirm in time, it is @@ -2110,6 +2127,7 @@ impl ChannelMonitorImpl { return Some(Balance::ClaimableAwaitingConfirmations { amount_satoshis: htlc.amount_msat / 1000, confirmation_height: conf_thresh, + source: BalanceSource::Htlc, }); } else if htlc_resolved.is_some() && !htlc_output_spend_pending { // Funding transaction spends should be fully confirmed by the time any @@ -2157,6 +2175,7 @@ impl ChannelMonitorImpl { return Some(Balance::ClaimableAwaitingConfirmations { amount_satoshis: htlc.amount_msat / 1000, confirmation_height: conf_thresh, + source: BalanceSource::Htlc, }); } else { let outbound_payment = match source { @@ -2185,6 +2204,7 @@ impl ChannelMonitorImpl { return Some(Balance::ClaimableAwaitingConfirmations { amount_satoshis: htlc.amount_msat / 1000, confirmation_height: conf_thresh, + source: BalanceSource::Htlc, }); } else { return Some(Balance::ContentiousClaimable { @@ -2272,6 +2292,7 @@ impl ChannelMonitor { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: value.to_sat(), confirmation_height: conf_thresh, + source: BalanceSource::CounterpartyForceClosed, }); } else { // If a counterparty commitment transaction is awaiting confirmation, we @@ -2295,6 +2316,7 @@ impl ChannelMonitor { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: output.value.to_sat(), confirmation_height: event.confirmation_threshold(), + source: BalanceSource::CounterpartyForceClosed, }); if let Some(confirmed_to_self_idx) = confirmed_counterparty_output.map(|(idx, _)| idx) { if event.transaction.as_ref().map(|tx| @@ -2327,6 +2349,7 @@ impl ChannelMonitor { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: us.current_holder_commitment_tx.to_self_value_sat, confirmation_height: conf_thresh, + source: BalanceSource::HolderForceClosed, }); } found_commitment_tx = true; @@ -2337,6 +2360,7 @@ impl ChannelMonitor { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: prev_commitment.to_self_value_sat, confirmation_height: conf_thresh, + source: BalanceSource::HolderForceClosed, }); } found_commitment_tx = true; @@ -2350,6 +2374,7 @@ impl ChannelMonitor { res.push(Balance::ClaimableAwaitingConfirmations { amount_satoshis: us.current_holder_commitment_tx.to_self_value_sat, confirmation_height: conf_thresh, + source: BalanceSource::CoopClose, }); } } diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 6ab554cbd4e..059f7ba6cb0 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -10,7 +10,7 @@ //! Further functional tests which test blockchain reorganizations. use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor}; -use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance}; +use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource}; use crate::chain::transaction::OutPoint; use crate::chain::chaininterface::{LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight}; use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource}; @@ -288,11 +288,13 @@ fn do_chanmon_claim_value_coop_close(anchors: bool) { assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 1_000 - commitment_tx_fee - anchor_outputs_value, confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::CoopClose, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1000, confirmation_height: nodes[1].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::CoopClose, }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); @@ -605,6 +607,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { anchor_outputs_value - // The anchor outputs value in satoshis 1, // The rounded up msat parts of HTLCs confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::CounterpartyForceClosed, }, sent_htlc_balance.clone(), sent_htlc_timeout_balance.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); // The main non-HTLC balance is just awaiting confirmations, but the claimable height is the @@ -612,6 +615,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000, confirmation_height: node_b_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, // Both HTLC balances are "contentious" as our counterparty could claim them if we wait too // long. @@ -629,6 +633,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000, confirmation_height: node_b_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, received_htlc_claiming_balance.clone(), received_htlc_timeout_claiming_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -670,6 +675,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 4_000, confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::Htlc, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); // After ANTI_REORG_DELAY, A will generate a SpendableOutputs event and drop the claimable @@ -690,9 +696,11 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000, confirmation_height: node_b_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 3_000, confirmation_height: node_b_htlc_claimable, + source: BalanceSource::Htlc, }, received_htlc_timeout_claiming_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -704,6 +712,7 @@ fn do_test_claim_value_force_close(anchors: bool, prev_commitment_tx: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 3_000, confirmation_height: node_b_htlc_claimable, + source: BalanceSource::Htlc, }, received_htlc_timeout_claiming_balance.clone()]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -862,6 +871,7 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 10_000 - 20_000 - commitment_tx_fee - anchor_outputs_value, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, htlc_balance_known_preimage.clone(), htlc_balance_unknown_preimage.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -880,6 +890,7 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 10_000 - 20_000 - commitment_tx_fee - anchor_outputs_value, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, htlc_balance_known_preimage.clone(), htlc_balance_unknown_preimage.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); if anchors { @@ -900,9 +911,11 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 10_000 - 20_000 - commitment_tx_fee - anchor_outputs_value, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: node_a_htlc_claimable, + source: BalanceSource::Htlc, }, htlc_balance_unknown_preimage.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -913,9 +926,11 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 10_000 - 20_000 - commitment_tx_fee - anchor_outputs_value, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: node_a_htlc_claimable, + source: BalanceSource::Htlc, }, htlc_balance_unknown_preimage.clone()]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -928,9 +943,11 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 10_000 - 20_000 - commitment_tx_fee - anchor_outputs_value, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: node_a_htlc_claimable, + source: BalanceSource::Htlc, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -943,6 +960,7 @@ fn do_test_balances_on_local_commitment_htlcs(anchors: bool) { assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: node_a_htlc_claimable, + source: BalanceSource::Htlc, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); let to_self_spendable_output = test_spendable_output(&nodes[0], &commitment_tx, false); @@ -1053,6 +1071,7 @@ fn test_no_preimage_inbound_htlc_balances() { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, a_received_htlc_balance.clone(), a_sent_htlc_balance.clone()]); mine_transaction(&nodes[0], &as_txn[0]); @@ -1073,6 +1092,7 @@ fn test_no_preimage_inbound_htlc_balances() { let mut bs_pre_spend_claims = sorted_vec(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 500_000 - 20_000, confirmation_height: node_b_commitment_claimable, + source: BalanceSource::CounterpartyForceClosed, }, b_received_htlc_balance.clone(), b_sent_htlc_balance.clone()]); assert_eq!(bs_pre_spend_claims, sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1125,9 +1145,11 @@ fn test_no_preimage_inbound_htlc_balances() { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, a_received_htlc_balance.clone(), Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: as_timeout_claimable_height, + source: BalanceSource::Htlc, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1136,9 +1158,11 @@ fn test_no_preimage_inbound_htlc_balances() { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, a_received_htlc_balance.clone(), Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: as_timeout_claimable_height, + source: BalanceSource::Htlc, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1152,9 +1176,11 @@ fn test_no_preimage_inbound_htlc_balances() { amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate * (chan_utils::commitment_tx_base_weight(&channel_type_features) + 2 * chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000, confirmation_height: node_a_commitment_claimable, + source: BalanceSource::HolderForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: core::cmp::max(as_timeout_claimable_height, htlc_cltv_timeout), + source: BalanceSource::Htlc, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1162,6 +1188,7 @@ fn test_no_preimage_inbound_htlc_balances() { assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: 10_000, confirmation_height: core::cmp::max(as_timeout_claimable_height, htlc_cltv_timeout), + source: BalanceSource::Htlc, }], nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); test_spendable_output(&nodes[0], &as_txn[0], false); @@ -1177,6 +1204,7 @@ fn test_no_preimage_inbound_htlc_balances() { assert_eq!(sorted_vec(vec![b_received_htlc_balance.clone(), Balance::ClaimableAwaitingConfirmations { amount_satoshis: 20_000, confirmation_height: bs_timeout_claimable_height, + source: BalanceSource::Htlc, }]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1184,6 +1212,7 @@ fn test_no_preimage_inbound_htlc_balances() { assert_eq!(sorted_vec(vec![b_received_htlc_balance.clone(), Balance::ClaimableAwaitingConfirmations { amount_satoshis: 20_000, confirmation_height: bs_timeout_claimable_height, + source: BalanceSource::Htlc, }]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1366,10 +1395,11 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // The expected balance for the next three checks, with the largest-HTLC and to_self output // claim balances separated out. - let expected_balance = vec![Balance::ClaimableAwaitingConfirmations { + let expected_balance_b = vec![Balance::ClaimableAwaitingConfirmations { // to_remote output in A's revoked commitment amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: nodes[1].best_block_info().1 + 5, + source: BalanceSource::CounterpartyForceClosed, }, Balance::CounterpartyRevokedOutputClaimable { amount_satoshis: 3_000, }, Balance::CounterpartyRevokedOutputClaimable { @@ -1387,7 +1417,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // Once the channel has been closed by A, B now considers all of the commitment transactions' // outputs as `CounterpartyRevokedOutputClaimable`. - assert_eq!(sorted_vec_with_additions(&expected_balance, &[&to_self_unclaimed_balance, &largest_htlc_unclaimed_balance]), + assert_eq!(sorted_vec_with_additions(&expected_balance_b, &[&to_self_unclaimed_balance, &largest_htlc_unclaimed_balance]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); if confirm_htlc_spend_first { @@ -1404,17 +1434,19 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ let largest_htlc_claimed_balance = Balance::ClaimableAwaitingConfirmations { amount_satoshis: 5_000 - inbound_htlc_claim_fee, confirmation_height: largest_htlc_claimed_avail_height, + source: BalanceSource::CounterpartyForceClosed, }; let to_self_claimed_balance = Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - to_self_claim_fee - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_self_claimed_avail_height, + source: BalanceSource::CounterpartyForceClosed, }; if confirm_htlc_spend_first { - assert_eq!(sorted_vec_with_additions(&expected_balance, &[&to_self_unclaimed_balance, &largest_htlc_claimed_balance]), + assert_eq!(sorted_vec_with_additions(&expected_balance_b, &[&to_self_unclaimed_balance, &largest_htlc_claimed_balance]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); } else { - assert_eq!(sorted_vec_with_additions(&expected_balance, &[&to_self_claimed_balance, &largest_htlc_unclaimed_balance]), + assert_eq!(sorted_vec_with_additions(&expected_balance_b, &[&to_self_claimed_balance, &largest_htlc_unclaimed_balance]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); } @@ -1423,7 +1455,7 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ } else { mine_transaction(&nodes[1], &claim_txn[2]); } - assert_eq!(sorted_vec_with_additions(&expected_balance, &[&to_self_claimed_balance, &largest_htlc_claimed_balance]), + assert_eq!(sorted_vec_with_additions(&expected_balance_b, &[&to_self_claimed_balance, &largest_htlc_claimed_balance]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); // Finally, connect the last two remaining HTLC spends and check that they move to @@ -1435,18 +1467,23 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_ // to_remote output in A's revoked commitment amount_satoshis: 100_000 - 5_000 - 4_000 - 3 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: nodes[1].best_block_info().1 + 1, + source: BalanceSource::CounterpartyForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 1_000_000 - 100_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - to_self_claim_fee - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_self_claimed_avail_height, + source: BalanceSource::CounterpartyForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 3_000 - outbound_htlc_claim_fee, confirmation_height: nodes[1].best_block_info().1 + 4, + source: BalanceSource::CounterpartyForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 4_000 - inbound_htlc_claim_fee, confirmation_height: nodes[1].best_block_info().1 + 5, + source: BalanceSource::CounterpartyForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: 5_000 - inbound_htlc_claim_fee, confirmation_height: largest_htlc_claimed_avail_height, + source: BalanceSource::CounterpartyForceClosed, }]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1624,6 +1661,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { // to_remote output in B's revoked commitment amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - 1 /* The rounded up msat part of the one HTLC */, confirmation_height: to_remote_conf_height, + source: BalanceSource::CounterpartyForceClosed, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in B's revoked commitment amount_satoshis: 11_000, @@ -1669,6 +1707,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { // to_remote output in B's revoked commitment amount_satoshis: 1_000_000 - 12_000 - 3_000 - commitment_tx_fee - anchor_outputs_value - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_remote_conf_height, + source: BalanceSource::CounterpartyForceClosed, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in B's revoked commitment amount_satoshis: 11_000, @@ -1677,6 +1716,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: as_htlc_claim_tx[0].output[0].value.to_sat(), confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::CounterpartyForceClosed, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1690,6 +1730,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: as_htlc_claim_tx[0].output[0].value.to_sat(), confirmation_height: nodes[0].best_block_info().1 + 2, + source: BalanceSource::CounterpartyForceClosed, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1759,6 +1800,7 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: revoked_htlc_timeout_claim.output[0].value.to_sat(), confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::CounterpartyForceClosed, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1767,9 +1809,11 @@ fn do_test_revoked_counterparty_htlc_tx_balances(anchors: bool) { // to_self output in B's revoked commitment amount_satoshis: revoked_to_self_claim.output[0].value.to_sat(), confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 1, + source: BalanceSource::CounterpartyForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: revoked_htlc_timeout_claim.output[0].value.to_sat(), confirmation_height: nodes[0].best_block_info().1 + ANTI_REORG_DELAY - 2, + source: BalanceSource::CounterpartyForceClosed, }]), sorted_vec(nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -1925,6 +1969,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { // to_remote output in A's revoked commitment amount_satoshis: 100_000 - 4_000 - 3_000 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_remote_maturity, + source: BalanceSource::CounterpartyForceClosed, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in A's revoked commitment amount_satoshis: 1_000_000 - 100_000 - commitment_tx_fee - anchor_outputs_value, @@ -1980,6 +2025,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { // to_remote output in A's revoked commitment amount_satoshis: 100_000 - 4_000 - 3_000 - 1 /* rounded up msat parts of HTLCs */, confirmation_height: to_remote_maturity, + source: BalanceSource::CounterpartyForceClosed, }, Balance::CounterpartyRevokedOutputClaimable { // to_self output in A's revoked commitment amount_satoshis: 1_000_000 - 100_000 - commitment_tx_fee - anchor_outputs_value, @@ -2020,6 +2066,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { }, Balance::ClaimableAwaitingConfirmations { // HTLC 2 amount_satoshis: claim_txn_2[0].output[0].value.to_sat(), confirmation_height: htlc_2_claim_maturity, + source: BalanceSource::CounterpartyForceClosed, }]), sorted_vec(nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances())); @@ -2045,15 +2092,18 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) { assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: claim_txn_2[1].output[0].value.to_sat(), confirmation_height: rest_claim_maturity, + source: BalanceSource::CounterpartyForceClosed, }, Balance::ClaimableAwaitingConfirmations { amount_satoshis: revoked_to_self_claim.as_ref().unwrap().output[0].value.to_sat(), confirmation_height: rest_claim_maturity, + source: BalanceSource::CounterpartyForceClosed, }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); } else { assert_eq!(vec![Balance::ClaimableAwaitingConfirmations { amount_satoshis: claim_txn_2[1].output[0].value.to_sat(), confirmation_height: rest_claim_maturity, + source: BalanceSource::CounterpartyForceClosed, }], nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); } From d6c540df124318b4deb8ef85a0d3e6e74daaac46 Mon Sep 17 00:00:00 2001 From: Duncan Dean Date: Tue, 30 Jul 2024 20:11:51 +0200 Subject: [PATCH 9/9] Test claimable balance is expected for forwarded/outbound payments --- lightning/src/ln/monitor_tests.rs | 94 +++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 059f7ba6cb0..2e4b7326cbd 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2145,6 +2145,100 @@ fn test_revoked_counterparty_aggregated_claims() { do_test_revoked_counterparty_aggregated_claims(true); } +fn do_test_claimable_balance_correct_while_payment_pending(outbound_payment: bool, anchors: bool) { + // Previously when a user fetched their balances via `get_claimable_balances` after forwarding a + // payment, but before it cleared, and summed up their balance using `Balance::claimable_amount_satoshis` + // neither the value of preimage claimable HTLC nor the timeout claimable HTLC would be included. + // This was incorrect as exactly one of these outcomes is true. This has been fixed by including the + // timeout claimable HTLC value in the balance as this excludes the routing fees and is the more + // prudent approach. + // + // In the case of the holder sending a payment, the above value will not be included while the payment + // is pending. + // + // This tests that we get the correct balance in either of the cases above. + let mut chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let mut user_config = test_default_channel_config(); + if anchors { + user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true; + user_config.manually_accept_inbound_channels = true; + } + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[Some(user_config), Some(user_config), Some(user_config)]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + let coinbase_tx = Transaction { + version: Version::TWO, + lock_time: LockTime::ZERO, + input: vec![TxIn { ..Default::default() }], + output: vec![ + TxOut { + value: Amount::ONE_BTC, + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }, + TxOut { + value: Amount::ONE_BTC, + script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), + }, + ], + }; + if anchors { + nodes[0].wallet_source.add_utxo(bitcoin::OutPoint { txid: coinbase_tx.txid(), vout: 0 }, coinbase_tx.output[0].value); + nodes[1].wallet_source.add_utxo(bitcoin::OutPoint { txid: coinbase_tx.txid(), vout: 1 }, coinbase_tx.output[1].value); + } + + // Create a channel from A -> B + let (_, _, chan_ab_id, funding_tx_ab) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000 /* channel_value (sat) */, 0 /* push_msat */); + let funding_outpoint_ab = OutPoint { txid: funding_tx_ab.txid(), index: 0 }; + assert_eq!(ChannelId::v1_from_funding_outpoint(funding_outpoint_ab), chan_ab_id); + // Create a channel from B -> C + let (_, _, chan_bc_id, funding_tx_bc) = + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000 /* channel_value (sat) */, 0 /* push_msat */); + let funding_outpoint_bc = OutPoint { txid: funding_tx_bc.txid(), index: 0 }; + assert_eq!(ChannelId::v1_from_funding_outpoint(funding_outpoint_bc), chan_bc_id); + + let (chan_feerate, channel_type_features) = if outbound_payment { + let chan_ab_feerate = get_feerate!(nodes[0], nodes[1], chan_ab_id); + let channel_type_features_ab = get_channel_type_features!(nodes[0], nodes[1], chan_ab_id); + (chan_ab_feerate, channel_type_features_ab) + } else { + let chan_bc_feerate = get_feerate!(nodes[1], nodes[2], chan_bc_id); + let channel_type_features_bc = get_channel_type_features!(nodes[1], nodes[2], chan_bc_id); + (chan_bc_feerate, channel_type_features_bc) + }; + let commitment_tx_fee = chan_feerate as u64 * + (chan_utils::commitment_tx_base_weight(&channel_type_features) + chan_utils::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000; + + // This HTLC will be forwarded by B from A -> C + let _ = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 4_000_000); + let anchor_outputs_value = if anchors { 2 * channel::ANCHOR_OUTPUT_VALUE_SATOSHI } else { 0 }; + + if outbound_payment { + assert_eq!( + 1_000_000 - commitment_tx_fee - anchor_outputs_value - 4_001 /* Note HTLC timeout amount of 4001 sats is excluded for outbound payment */, + nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint_ab).unwrap().get_claimable_balances().iter().map( + |x| x.claimable_amount_satoshis()).sum()); + } else { + assert_eq!( + 0u64, + nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint_ab).unwrap().get_claimable_balances().iter().map( + |x| x.claimable_amount_satoshis()).sum()); + assert_eq!( + 1_000_000 - commitment_tx_fee - anchor_outputs_value /* Note HTLC timeout amount of 4000 sats is included */, + nodes[1].chain_monitor.chain_monitor.get_monitor(funding_outpoint_bc).unwrap().get_claimable_balances().iter().map( + |x| x.claimable_amount_satoshis()).sum()); + } +} + +#[test] +fn test_claimable_balance_correct_while_payment_pending() { + do_test_claimable_balance_correct_while_payment_pending(false, false); + do_test_claimable_balance_correct_while_payment_pending(false, true); + do_test_claimable_balance_correct_while_payment_pending(true, false); + do_test_claimable_balance_correct_while_payment_pending(true, true); +} + fn do_test_restored_packages_retry(check_old_monitor_retries_after_upgrade: bool) { // Tests that we'll retry packages that were previously timelocked after we've restored them. let chanmon_cfgs = create_chanmon_cfgs(2);