Merge pull request #1435 from TheBlueMatt/2022-04-1126-first-step

Author: TheBlueMatt

fuzz/src/router.rs

@@ -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};
@@ -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 {
@@ -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,
 							});

lightning/src/ln/channel.rs

@@ -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
@@ -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>) {

lightning/src/ln/channelmanager.rs

@@ -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).
@@ -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 {
@@ -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(),
@@ -5936,6 +5943,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),

lightning/src/routing/router.rs

@@ -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,
@@ -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:
@@ -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)
 			}
 		});
 	}
@@ -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,
@@ -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(),
@@ -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)];
 
 		{
@@ -5476,6 +5478,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,

lightning/src/util/events.rs

@@ -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.
 	///
@@ -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.
 	///
@@ -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