Skip to content

Separate ChannelDetails' outbound capacity from the next HTLC max #1435

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 28, 2022
Merged

Separate ChannelDetails' outbound capacity from the next HTLC max #1435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1af7055
Separate `ChannelDetails`' outbound capacity from the next HTLC max
TheBlueMatt Apr 16, 2022
68ee7ef
Sort `Event` variants somewhat more sensibly
TheBlueMatt Apr 18, 2022
61629bc
Consolidate Channel balance fetching into one fn returning struct
TheBlueMatt Apr 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions fuzz/src/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use bitcoin::blockdata::constants::genesis_block;

use utils::test_logger;

use std::convert::TryInto;
use std::collections::HashSet;
use std::sync::Arc;
use std::sync::atomic::{AtomicUsize, Ordering};
Expand Down Expand Up @@ -205,7 +206,8 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
count => {
for _ in 0..count {
scid += 1;
let rnid = node_pks.iter().skip(slice_to_be16(get_slice!(2))as usize % node_pks.len()).next().unwrap();
let rnid = node_pks.iter().skip(u16::from_be_bytes(get_slice!(2).try_into().unwrap()) as usize % node_pks.len()).next().unwrap();
let capacity = u64::from_be_bytes(get_slice!(8).try_into().unwrap());
first_hops_vec.push(ChannelDetails {
channel_id: [0; 32],
counterparty: ChannelCounterparty {
Expand All @@ -220,15 +222,16 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
channel_type: None,
short_channel_id: Some(scid),
inbound_scid_alias: None,
channel_value_satoshis: slice_to_be64(get_slice!(8)),
channel_value_satoshis: capacity,
user_channel_id: 0, inbound_capacity_msat: 0,
unspendable_punishment_reserve: None,
confirmations_required: None,
force_close_spend_delay: None,
is_outbound: true, is_funding_locked: true,
is_usable: true, is_public: true,
balance_msat: 0,
outbound_capacity_msat: 0,
outbound_capacity_msat: capacity.saturating_mul(1000),
next_outbound_htlc_limit_msat: capacity.saturating_mul(1000),
inbound_htlc_minimum_msat: None,
inbound_htlc_maximum_msat: None,
});
Expand Down
56 changes: 33 additions & 23 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ pub struct ChannelValueStat {
pub counterparty_dust_limit_msat: u64,
}

pub struct AvailableBalances {
/// The amount that would go to us if we close the channel, ignoring any on-chain fees.
pub balance_msat: u64,
/// Total amount available for our counterparty to send to us.
pub inbound_capacity_msat: u64,
/// Total amount available for us to send to our counterparty.
pub outbound_capacity_msat: u64,
/// The maximum value we can assign to the next outbound HTLC
pub next_outbound_htlc_limit_msat: u64,
}

#[derive(Debug, Clone, Copy, PartialEq)]
enum FeeUpdateState {
// Inbound states mirroring InboundHTLCState
Expand Down Expand Up @@ -2330,40 +2341,39 @@ impl<Signer: Sign> Channel<Signer> {
stats
}

/// Get the available (ie not including pending HTLCs) inbound and outbound balance in msat.
/// Get the available balances, see [`AvailableBalances`]'s fields for more info.
/// Doesn't bother handling the
/// if-we-removed-it-already-but-haven't-fully-resolved-they-can-still-send-an-inbound-HTLC
/// corner case properly.
/// The channel reserve is subtracted from each balance.
/// See also [`Channel::get_balance_msat`]
pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) {
pub fn get_available_balances(&self) -> AvailableBalances {
// Note that we have to handle overflow due to the above case.
(
cmp::max(self.channel_value_satoshis as i64 * 1000
- self.value_to_self_msat as i64
- self.get_inbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64
- self.holder_selected_channel_reserve_satoshis as i64 * 1000,
0) as u64,
cmp::max(self.value_to_self_msat as i64
- self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64
- self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000,
0) as u64
)
}
let outbound_stats = self.get_outbound_pending_htlc_stats(None);

/// Get our total balance in msat.
/// This is the amount that would go to us if we close the channel, ignoring any on-chain fees.
/// See also [`Channel::get_inbound_outbound_available_balance_msat`]
pub fn get_balance_msat(&self) -> u64 {
// Include our local balance, plus any inbound HTLCs we know the preimage for, minus any
// HTLCs sent or which will be sent after commitment signed's are exchanged.
let mut balance_msat = self.value_to_self_msat;
for ref htlc in self.pending_inbound_htlcs.iter() {
if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) = htlc.state {
balance_msat += htlc.amount_msat;
}
}
balance_msat - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat
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,
0) as u64;
AvailableBalances {
inbound_capacity_msat: cmp::max(self.channel_value_satoshis as i64 * 1000
- self.value_to_self_msat as i64
- self.get_inbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64
- self.holder_selected_channel_reserve_satoshis as i64 * 1000,
0) as u64,
outbound_capacity_msat,
next_outbound_htlc_limit_msat: cmp::max(cmp::min(outbound_capacity_msat as i64,
self.counterparty_max_htlc_value_in_flight_msat as i64
- outbound_stats.pending_htlcs_value_msat as i64),
0) as u64,
balance_msat,
}
}

pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option<u64>) {
Expand Down
20 changes: 15 additions & 5 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,13 @@ pub struct ChannelDetails {
/// conflict-avoidance policy, exactly this amount is not likely to be spendable. However, we
/// should be able to spend nearly this amount.
pub outbound_capacity_msat: u64,
/// The available outbound capacity for sending a single HTLC to the remote peer. This is
/// similar to [`ChannelDetails::outbound_capacity_msat`] but it may be further restricted by
/// the current state and per-HTLC limit(s). This is intended for use when routing, allowing us
/// to use a limit as close as possible to the HTLC limit we can currently send.
///
/// See also [`ChannelDetails::balance_msat`] and [`ChannelDetails::outbound_capacity_msat`].
pub next_outbound_htlc_limit_msat: u64,
/// The available inbound capacity for the remote peer to send HTLCs to us. This does not
/// include any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not
/// available for inclusion in new inbound HTLCs).
Expand Down Expand Up @@ -1670,8 +1677,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
let channel_state = self.channel_state.lock().unwrap();
res.reserve(channel_state.by_id.len());
for (channel_id, channel) in channel_state.by_id.iter().filter(f) {
let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat();
let balance_msat = channel.get_balance_msat();
let balance = channel.get_available_balances();
let (to_remote_reserve_satoshis, to_self_reserve_satoshis) =
channel.get_holder_counterparty_selected_channel_reserve_satoshis();
res.push(ChannelDetails {
Expand All @@ -1698,9 +1704,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
inbound_scid_alias: channel.latest_inbound_scid_alias(),
channel_value_satoshis: channel.get_value_satoshis(),
unspendable_punishment_reserve: to_self_reserve_satoshis,
balance_msat,
inbound_capacity_msat,
outbound_capacity_msat,
balance_msat: balance.balance_msat,
inbound_capacity_msat: balance.inbound_capacity_msat,
outbound_capacity_msat: balance.outbound_capacity_msat,
next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat,
user_channel_id: channel.get_user_id(),
confirmations_required: channel.minimum_depth(),
force_close_spend_delay: channel.get_counterparty_selected_contest_delay(),
Expand Down Expand Up @@ -5937,6 +5944,9 @@ impl_writeable_tlv_based!(ChannelDetails, {
(14, user_channel_id, required),
(16, balance_msat, required),
(18, outbound_capacity_msat, required),
// Note that by the time we get past the required read above, outbound_capacity_msat will be
// filled in, so we can safely unwrap it here.
(19, next_outbound_htlc_limit_msat, (default_value, outbound_capacity_msat.0.unwrap())),
(20, inbound_capacity_msat, required),
(22, confirmations_required, option),
(24, force_close_spend_delay, option),
Expand Down
17 changes: 10 additions & 7 deletions lightning/src/routing/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ impl<'a> CandidateRouteHop<'a> {
fn effective_capacity(&self) -> EffectiveCapacity {
match self {
CandidateRouteHop::FirstHop { details } => EffectiveCapacity::ExactLiquidity {
liquidity_msat: details.outbound_capacity_msat,
liquidity_msat: details.next_outbound_htlc_limit_msat,
},
CandidateRouteHop::PublicHop { info, .. } => info.effective_capacity(),
CandidateRouteHop::PrivateHop { .. } => EffectiveCapacity::Infinite,
Expand Down Expand Up @@ -818,7 +818,8 @@ where L::Target: Logger {
// We don't want multiple paths (as per MPP) share liquidity of the same channels.
// This map allows paths to be aware of the channel use by other paths in the same call.
// This would help to make a better path finding decisions and not "overbook" channels.
// It is unaware of the directions (except for `outbound_capacity_msat` in `first_hops`).
// It is unaware of the directions (except for `next_outbound_htlc_limit_msat` in
// `first_hops`).
let mut bookkept_channels_liquidity_available_msat = HashMap::with_capacity(network_nodes.len());

// Keeping track of how much value we already collected across other paths. Helps to decide:
Expand All @@ -841,12 +842,12 @@ where L::Target: Logger {
// sort channels above `recommended_value_msat` in ascending order, preferring channels
// which have enough, but not too much, capacity for the payment.
channels.sort_unstable_by(|chan_a, chan_b| {
if chan_b.outbound_capacity_msat < recommended_value_msat || chan_a.outbound_capacity_msat < recommended_value_msat {
if chan_b.next_outbound_htlc_limit_msat < recommended_value_msat || chan_a.next_outbound_htlc_limit_msat < recommended_value_msat {
// Sort in descending order
chan_b.outbound_capacity_msat.cmp(&chan_a.outbound_capacity_msat)
chan_b.next_outbound_htlc_limit_msat.cmp(&chan_a.next_outbound_htlc_limit_msat)
} else {
// Sort in ascending order
chan_a.outbound_capacity_msat.cmp(&chan_b.outbound_capacity_msat)
chan_a.next_outbound_htlc_limit_msat.cmp(&chan_b.next_outbound_htlc_limit_msat)
}
});
}
Expand Down Expand Up @@ -1746,6 +1747,7 @@ mod tests {
user_channel_id: 0,
balance_msat: 0,
outbound_capacity_msat,
next_outbound_htlc_limit_msat: outbound_capacity_msat,
inbound_capacity_msat: 42,
unspendable_punishment_reserve: None,
confirmations_required: None,
Expand Down Expand Up @@ -3407,7 +3409,7 @@ mod tests {
assert_eq!(path.last().unwrap().fee_msat, 250_000_000);
}

// Check that setting outbound_capacity_msat in first_hops limits the channels.
// Check that setting next_outbound_htlc_limit_msat in first_hops limits the channels.
// Disable channel #1 and use another first hop.
update_channel(&net_graph_msg_handler, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
chain_hash: genesis_block(Network::Testnet).header.block_hash(),
Expand All @@ -3422,7 +3424,7 @@ mod tests {
excess_data: Vec::new()
});

// Now, limit the first_hop by the outbound_capacity_msat of 200_000 sats.
// Now, limit the first_hop by the next_outbound_htlc_limit_msat of 200_000 sats.
let our_chans = vec![get_channel_details(Some(42), nodes[0].clone(), InitFeatures::from_le_bytes(vec![0b11]), 200_000_000)];

{
Expand Down Expand Up @@ -5473,6 +5475,7 @@ mod benches {
user_channel_id: 0,
balance_msat: 10_000_000,
outbound_capacity_msat: 10_000_000,
next_outbound_htlc_limit_msat: 10_000_000,
inbound_capacity_msat: 0,
unspendable_punishment_reserve: None,
confirmations_required: None,
Expand Down
82 changes: 41 additions & 41 deletions lightning/src/util/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,47 @@ pub enum Event {
/// [`Route::get_total_fees`]: crate::routing::router::Route::get_total_fees
fee_paid_msat: Option<u64>,
},
/// Indicates an outbound payment failed. Individual [`Event::PaymentPathFailed`] events
/// provide failure information for each MPP part in the payment.
///
/// This event is provided once there are no further pending HTLCs for the payment and the
/// payment is no longer retryable, either due to a several-block timeout or because
/// [`ChannelManager::abandon_payment`] was previously called for the corresponding payment.
///
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
PaymentFailed {
/// The id returned by [`ChannelManager::send_payment`] and used with
/// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
payment_id: PaymentId,
/// The hash that was given to [`ChannelManager::send_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
payment_hash: PaymentHash,
},
/// Indicates that a path for an outbound payment was successful.
///
/// Always generated after [`Event::PaymentSent`] and thus useful for scoring channels. See
/// [`Event::PaymentSent`] for obtaining the payment preimage.
PaymentPathSuccessful {
/// The id returned by [`ChannelManager::send_payment`] and used with
/// [`ChannelManager::retry_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
payment_id: PaymentId,
/// The hash that was given to [`ChannelManager::send_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
payment_hash: Option<PaymentHash>,
/// The payment path that was successful.
///
/// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
path: Vec<RouteHop>,
},
/// Indicates an outbound HTLC we sent failed. Probably some intermediary node dropped
/// something. You may wish to retry with a different route.
///
Expand Down Expand Up @@ -299,27 +340,6 @@ pub enum Event {
#[cfg(test)]
error_data: Option<Vec<u8>>,
},
/// Indicates an outbound payment failed. Individual [`Event::PaymentPathFailed`] events
/// provide failure information for each MPP part in the payment.
///
/// This event is provided once there are no further pending HTLCs for the payment and the
/// payment is no longer retryable, either due to a several-block timeout or because
/// [`ChannelManager::abandon_payment`] was previously called for the corresponding payment.
///
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
PaymentFailed {
/// The id returned by [`ChannelManager::send_payment`] and used with
/// [`ChannelManager::retry_payment`] and [`ChannelManager::abandon_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
/// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment
payment_id: PaymentId,
/// The hash that was given to [`ChannelManager::send_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
payment_hash: PaymentHash,
},
/// Used to indicate that [`ChannelManager::process_pending_htlc_forwards`] should be called at
/// a time in the future.
///
Expand Down Expand Up @@ -390,26 +410,6 @@ pub enum Event {
/// The full transaction received from the user
transaction: Transaction
},
/// Indicates that a path for an outbound payment was successful.
///
/// Always generated after [`Event::PaymentSent`] and thus useful for scoring channels. See
/// [`Event::PaymentSent`] for obtaining the payment preimage.
PaymentPathSuccessful {
/// The id returned by [`ChannelManager::send_payment`] and used with
/// [`ChannelManager::retry_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
/// [`ChannelManager::retry_payment`]: crate::ln::channelmanager::ChannelManager::retry_payment
payment_id: PaymentId,
/// The hash that was given to [`ChannelManager::send_payment`].
///
/// [`ChannelManager::send_payment`]: crate::ln::channelmanager::ChannelManager::send_payment
payment_hash: Option<PaymentHash>,
/// The payment path that was successful.
///
/// May contain a closed channel if the HTLC sent along the path was fulfilled on chain.
path: Vec<RouteHop>,
},
/// Indicates a request to open a new channel by a peer.
///
/// To accept the request, call [`ChannelManager::accept_inbound_channel`]. To reject the
Expand Down