-
Notifications
You must be signed in to change notification settings - Fork 418
Mind commit tx fee while assembling a route #2080
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2611,10 +2611,29 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> { | |
} | ||
balance_msat -= outbound_stats.pending_htlcs_value_msat; | ||
|
||
let outbound_capacity_msat = cmp::max(self.value_to_self_msat as i64 | ||
- outbound_stats.pending_htlcs_value_msat as i64 | ||
- self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, | ||
let mut outbound_capacity_msat = cmp::max(self.value_to_self_msat as i64 | ||
- outbound_stats.pending_htlcs_value_msat as i64 | ||
- self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000, | ||
0) as u64; | ||
if self.is_outbound() { | ||
// Mind commit tx fee. | ||
let mut real_dust_limit_success_sat = self.holder_dust_limit_satoshis; | ||
if !self.opt_anchors() { | ||
real_dust_limit_success_sat += self.feerate_per_kw as u64 * htlc_success_tx_weight(false) / 1000; | ||
} | ||
|
||
let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat, HTLCInitiator::RemoteOffered); | ||
let max_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_above_dust, None); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may also want to set |
||
let htlc_dust = HTLCCandidate::new(real_dust_limit_success_sat - 1, HTLCInitiator::RemoteOffered); | ||
let min_reserved_commit_tx_fee_msat = FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE * self.next_local_commit_tx_fee_msat(htlc_dust, None); | ||
let one_htlc_difference_msat = max_reserved_commit_tx_fee_msat - min_reserved_commit_tx_fee_msat; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, if I add an |
||
|
||
outbound_capacity_msat -= max_reserved_commit_tx_fee_msat; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want to as aggressively change
|
||
if outbound_capacity_msat < real_dust_limit_success_sat { | ||
outbound_capacity_msat += one_htlc_difference_msat; | ||
outbound_capacity_msat = std::cmp::max(real_dust_limit_success_sat - 1, outbound_capacity_msat); | ||
} | ||
} | ||
AvailableBalances { | ||
inbound_capacity_msat: cmp::max(self.channel_value_satoshis as i64 * 1000 | ||
- self.value_to_self_msat as i64 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1609,6 +1609,19 @@ macro_rules! get_route_and_payment_hash { | |
}} | ||
} | ||
|
||
#[cfg(test)] | ||
#[macro_export] | ||
macro_rules! get_route_or_error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: no reason this shouldn't be a function rather than a macro. |
||
($send_node: expr, $recv_node: expr, $recv_value: expr) => {{ | ||
let payment_params = $crate::routing::router::PaymentParameters::from_node_id($recv_node.node.get_our_node_id(), TEST_FINAL_CLTV) | ||
.with_features($recv_node.node.invoice_features()); | ||
let (payment_preimage, payment_hash, payment_secret) = $crate::get_payment_preimage_hash!($recv_node, Some($recv_value)); | ||
let route_or_error = $crate::get_route!($send_node, payment_params, $recv_value, TEST_FINAL_CLTV); | ||
(route_or_error, payment_hash, payment_preimage, payment_secret) | ||
}} | ||
} | ||
|
||
|
||
#[macro_export] | ||
#[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))] | ||
macro_rules! expect_payment_claimable { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add some comments or maybe DRY this up with some of the existing tx fee construction logic? Its kinda hard to read what's going on here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it would be nice to re-use some of the
send_htlc
logic. Ultimately, I think it'd be useful to refactorChannel
such that we have one method that can provide any type of balance we may need across LDK.