Merge pull request #3080 from jkczyz/2024-05-compact-blinded-path-creation-trait

Optional compact blinded path creation

Author: valentinewallace

fuzz/src/chanmon_consistency.rs

@@ -34,7 +34,6 @@ use bitcoin::hashes::sha256d::Hash as Sha256dHash;
 use bitcoin::hash_types::BlockHash;
 
 use lightning::blinded_path::BlindedPath;
-use lightning::blinded_path::message::ForwardNode;
 use lightning::blinded_path::payment::ReceiveTlvs;
 use lightning::chain;
 use lightning::chain::{BestBlock, ChannelMonitorUpdateStatus, chainmonitor, channelmonitor, Confirm, Watch};
@@ -124,7 +123,7 @@ impl MessageRouter for FuzzRouter {
 	}
 
 	fn create_blinded_paths<T: secp256k1::Signing + secp256k1::Verification>(
-		&self, _recipient: PublicKey, _peers: Vec<ForwardNode>, _secp_ctx: &Secp256k1<T>,
+		&self, _recipient: PublicKey, _peers: Vec<PublicKey>, _secp_ctx: &Secp256k1<T>,
 	) -> Result<Vec<BlindedPath>, ()> {
 		unreachable!()
 	}

fuzz/src/full_stack.rs

@@ -31,7 +31,6 @@ use bitcoin::hashes::sha256d::Hash as Sha256dHash;
 use bitcoin::hash_types::{Txid, BlockHash};
 
 use lightning::blinded_path::BlindedPath;
-use lightning::blinded_path::message::ForwardNode;
 use lightning::blinded_path::payment::ReceiveTlvs;
 use lightning::chain;
 use lightning::chain::{BestBlock, ChannelMonitorUpdateStatus, Confirm, Listen};
@@ -162,7 +161,7 @@ impl MessageRouter for FuzzRouter {
 	}
 
 	fn create_blinded_paths<T: secp256k1::Signing + secp256k1::Verification>(
-		&self, _recipient: PublicKey, _peers: Vec<ForwardNode>, _secp_ctx: &Secp256k1<T>,
+		&self, _recipient: PublicKey, _peers: Vec<PublicKey>, _secp_ctx: &Secp256k1<T>,
 	) -> Result<Vec<BlindedPath>, ()> {
 		unreachable!()
 	}

fuzz/src/onion_message.rs

@@ -7,7 +7,6 @@ use bitcoin::secp256k1::ecdsa::RecoverableSignature;
 use bitcoin::secp256k1::schnorr;
 
 use lightning::blinded_path::{BlindedPath, EmptyNodeIdLookUp};
-use lightning::blinded_path::message::ForwardNode;
 use lightning::ln::features::InitFeatures;
 use lightning::ln::msgs::{self, DecodeError, OnionMessageHandler};
 use lightning::ln::script::ShutdownScript;
@@ -89,7 +88,7 @@ impl MessageRouter for TestMessageRouter {
 	}
 
 	fn create_blinded_paths<T: secp256k1::Signing + secp256k1::Verification>(
-		&self, _recipient: PublicKey, _peers: Vec<ForwardNode>, _secp_ctx: &Secp256k1<T>,
+		&self, _recipient: PublicKey, _peers: Vec<PublicKey>, _secp_ctx: &Secp256k1<T>,
 	) -> Result<Vec<BlindedPath>, ()> {
 		unreachable!()
 	}

lightning/src/ln/channelmanager.rs

@@ -1554,8 +1554,9 @@ where
 /// #
 /// # fn example<T: AChannelManager>(channel_manager: T) -> Result<(), Bolt12SemanticError> {
 /// # let channel_manager = channel_manager.get_cm();
+/// # let absolute_expiry = None;
 /// let offer = channel_manager
-///     .create_offer_builder()?
+///     .create_offer_builder(absolute_expiry)?
 /// # ;
 /// # // Needed for compiling for c_bindings
 /// # let builder: lightning::offers::offer::OfferBuilder<_, _> = offer.into();
@@ -2287,6 +2288,19 @@ const MAX_UNFUNDED_CHANNEL_PEERS: usize = 50;
 /// many peers we reject new (inbound) connections.
 const MAX_NO_CHANNEL_PEERS: usize = 250;
 
+/// The maximum expiration from the current time where an [`Offer`] or [`Refund`] is considered
+/// short-lived, while anything with a greater expiration is considered long-lived.
+///
+/// Using [`ChannelManager::create_offer_builder`] or [`ChannelManager::create_refund_builder`],
+/// will included a [`BlindedPath`] created using:
+/// - [`MessageRouter::create_compact_blinded_paths`] when short-lived, and
+/// - [`MessageRouter::create_blinded_paths`] when long-lived.
+///
+/// Using compact [`BlindedPath`]s may provide better privacy as the [`MessageRouter`] could select
+/// more hops. However, since they use short channel ids instead of pubkeys, they are more likely to
+/// become invalid over time as channels are closed. Thus, they are only suitable for short-term use.
+pub const MAX_SHORT_LIVED_RELATIVE_EXPIRY: Duration = Duration::from_secs(60 * 60 * 24);
+
 /// Used by [`ChannelManager::list_recent_payments`] to express the status of recent payments.
 /// These include payments that have yet to find a successful path, or have unresolved HTLCs.
 #[derive(Debug, PartialEq)]
@@ -8240,16 +8254,15 @@ where
 
 macro_rules! create_offer_builder { ($self: ident, $builder: ty) => {
 	/// Creates an [`OfferBuilder`] such that the [`Offer`] it builds is recognized by the
-	/// [`ChannelManager`] when handling [`InvoiceRequest`] messages for the offer. The offer will
-	/// not have an expiration unless otherwise set on the builder.
+	/// [`ChannelManager`] when handling [`InvoiceRequest`] messages for the offer. The offer's
+	/// expiration will be `absolute_expiry` if `Some`, otherwise it will not expire.
 	///
 	/// # Privacy
 	///
-	/// Uses [`MessageRouter::create_blinded_paths`] to construct a [`BlindedPath`] for the offer.
-	/// However, if one is not found, uses a one-hop [`BlindedPath`] with
-	/// [`ChannelManager::get_our_node_id`] as the introduction node instead. In the latter case,
-	/// the node must be announced, otherwise, there is no way to find a path to the introduction in
-	/// order to send the [`InvoiceRequest`].
+	/// Uses [`MessageRouter`] to construct a [`BlindedPath`] for the offer based on the given
+	/// `absolute_expiry` according to [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. See those docs for
+	/// privacy implications as well as those of the parameterized [`Router`], which implements
+	/// [`MessageRouter`].
 	///
 	/// Also, uses a derived signing pubkey in the offer for recipient privacy.
 	///
@@ -8264,19 +8277,27 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => {
 	///
 	/// [`Offer`]: crate::offers::offer::Offer
 	/// [`InvoiceRequest`]: crate::offers::invoice_request::InvoiceRequest
-	pub fn create_offer_builder(&$self) -> Result<$builder, Bolt12SemanticError> {
+	pub fn create_offer_builder(
+		&$self, absolute_expiry: Option<Duration>
+	) -> Result<$builder, Bolt12SemanticError> {
 		let node_id = $self.get_our_node_id();
 		let expanded_key = &$self.inbound_payment_key;
 		let entropy = &*$self.entropy_source;
 		let secp_ctx = &$self.secp_ctx;
 
-		let path = $self.create_blinded_path().map_err(|_| Bolt12SemanticError::MissingPaths)?;
+		let path = $self.create_blinded_path_using_absolute_expiry(absolute_expiry)
+			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
 		let builder = OfferBuilder::deriving_signing_pubkey(
 			node_id, expanded_key, entropy, secp_ctx
 		)
 			.chain_hash($self.chain_hash)
 			.path(path);
 
+		let builder = match absolute_expiry {
+			None => builder,
+			Some(absolute_expiry) => builder.absolute_expiry(absolute_expiry),
+		};
+
 		Ok(builder.into())
 	}
 } }
@@ -8304,11 +8325,10 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 	///
 	/// # Privacy
 	///
-	/// Uses [`MessageRouter::create_blinded_paths`] to construct a [`BlindedPath`] for the refund.
-	/// However, if one is not found, uses a one-hop [`BlindedPath`] with
-	/// [`ChannelManager::get_our_node_id`] as the introduction node instead. In the latter case,
-	/// the node must be announced, otherwise, there is no way to find a path to the introduction in
-	/// order to send the [`Bolt12Invoice`].
+	/// Uses [`MessageRouter`] to construct a [`BlindedPath`] for the refund based on the given
+	/// `absolute_expiry` according to [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. See those docs for
+	/// privacy implications as well as those of the parameterized [`Router`], which implements
+	/// [`MessageRouter`].
 	///
 	/// Also, uses a derived payer id in the refund for payer privacy.
 	///
@@ -8337,7 +8357,8 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => {
 		let entropy = &*$self.entropy_source;
 		let secp_ctx = &$self.secp_ctx;
 
-		let path = $self.create_blinded_path().map_err(|_| Bolt12SemanticError::MissingPaths)?;
+		let path = $self.create_blinded_path_using_absolute_expiry(Some(absolute_expiry))
+			.map_err(|_| Bolt12SemanticError::MissingPaths)?;
 		let builder = RefundBuilder::deriving_payer_id(
 			node_id, expanded_key, entropy, secp_ctx, amount_msats, payment_id
 		)?
@@ -8406,10 +8427,9 @@ where
 	///
 	/// # Privacy
 	///
-	/// Uses a one-hop [`BlindedPath`] for the reply path with [`ChannelManager::get_our_node_id`]
-	/// as the introduction node and a derived payer id for payer privacy. As such, currently, the
-	/// node must be announced. Otherwise, there is no way to find a path to the introduction node
-	/// in order to send the [`Bolt12Invoice`].
+	/// For payer privacy, uses a derived payer id and uses [`MessageRouter::create_blinded_paths`]
+	/// to construct a [`BlindedPath`] for the reply path. For further privacy implications, see the
+	/// docs of the parameterized [`Router`], which implements [`MessageRouter`].
 	///
 	/// # Limitations
 	///
@@ -8686,6 +8706,38 @@ where
 		inbound_payment::get_payment_preimage(payment_hash, payment_secret, &self.inbound_payment_key)
 	}
 
+	/// Creates a blinded path by delegating to [`MessageRouter`] based on the path's intended
+	/// lifetime.
+	///
+	/// Whether or not the path is compact depends on whether the path is short-lived or long-lived,
+	/// respectively, based on the given `absolute_expiry` as seconds since the Unix epoch. See
+	/// [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`].
+	fn create_blinded_path_using_absolute_expiry(
+		&self, absolute_expiry: Option<Duration>
+	) -> Result<BlindedPath, ()> {
+		let now = self.duration_since_epoch();
+		let max_short_lived_absolute_expiry = now.saturating_add(MAX_SHORT_LIVED_RELATIVE_EXPIRY);
+
+		if absolute_expiry.unwrap_or(Duration::MAX) <= max_short_lived_absolute_expiry {
+			self.create_compact_blinded_path()
+		} else {
+			self.create_blinded_path()
+		}
+	}
+
+	pub(super) fn duration_since_epoch(&self) -> Duration {
+		#[cfg(not(feature = "std"))]
+		let now = Duration::from_secs(
+			self.highest_seen_timestamp.load(Ordering::Acquire) as u64
+		);
+		#[cfg(feature = "std")]
+		let now = std::time::SystemTime::now()
+			.duration_since(std::time::SystemTime::UNIX_EPOCH)
+			.expect("SystemTime::now() should come after SystemTime::UNIX_EPOCH");
+
+		now
+	}
+
 	/// Creates a blinded path by delegating to [`MessageRouter::create_blinded_paths`].
 	///
 	/// Errors if the `MessageRouter` errors or returns an empty `Vec`.
@@ -8696,6 +8748,27 @@ where
 		let peers = self.per_peer_state.read().unwrap()
 			.iter()
 			.map(|(node_id, peer_state)| (node_id, peer_state.lock().unwrap()))
+			.filter(|(_, peer)| peer.is_connected)
+			.filter(|(_, peer)| peer.latest_features.supports_onion_messages())
+			.map(|(node_id, _)| *node_id)
+			.collect::<Vec<_>>();
+
+		self.router
+			.create_blinded_paths(recipient, peers, secp_ctx)
+			.and_then(|paths| paths.into_iter().next().ok_or(()))
+	}
+
+	/// Creates a blinded path by delegating to [`MessageRouter::create_compact_blinded_paths`].
+	///
+	/// Errors if the `MessageRouter` errors or returns an empty `Vec`.
+	fn create_compact_blinded_path(&self) -> Result<BlindedPath, ()> {
+		let recipient = self.get_our_node_id();
+		let secp_ctx = &self.secp_ctx;
+
+		let peers = self.per_peer_state.read().unwrap()
+			.iter()
+			.map(|(node_id, peer_state)| (node_id, peer_state.lock().unwrap()))
+			.filter(|(_, peer)| peer.is_connected)
 			.filter(|(_, peer)| peer.latest_features.supports_onion_messages())
 			.map(|(node_id, peer)| ForwardNode {
 				node_id: *node_id,
@@ -8708,7 +8781,7 @@ where
 			.collect::<Vec<_>>();
 
 		self.router
-			.create_blinded_paths(recipient, peers, secp_ctx)
+			.create_compact_blinded_paths(recipient, peers, secp_ctx)
 			.and_then(|paths| paths.into_iter().next().ok_or(()))
 	}
 

lightning/src/ln/functional_test_utils.rs

@@ -3259,30 +3259,34 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
 
 	for i in 0..node_count {
 		for j in (i+1)..node_count {
-			let node_id_i = nodes[i].node.get_our_node_id();
-			let node_id_j = nodes[j].node.get_our_node_id();
-
-			let init_i = msgs::Init {
-				features: nodes[i].init_features(&node_id_j),
-				networks: None,
-				remote_network_address: None,
-			};
-			let init_j = msgs::Init {
-				features: nodes[j].init_features(&node_id_i),
-				networks: None,
-				remote_network_address: None,
-			};
-
-			nodes[i].node.peer_connected(&node_id_j, &init_j, true).unwrap();
-			nodes[j].node.peer_connected(&node_id_i, &init_i, false).unwrap();
-			nodes[i].onion_messenger.peer_connected(&node_id_j, &init_j, true).unwrap();
-			nodes[j].onion_messenger.peer_connected(&node_id_i, &init_i, false).unwrap();
+			connect_nodes(&nodes[i], &nodes[j]);
 		}
 	}
 
 	nodes
 }
 
+fn connect_nodes<'a, 'b: 'a, 'c: 'b>(node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>) {
+	let node_id_a = node_a.node.get_our_node_id();
+	let node_id_b = node_b.node.get_our_node_id();
+
+	let init_a = msgs::Init {
+		features: node_a.init_features(&node_id_b),
+		networks: None,
+		remote_network_address: None,
+	};
+	let init_b = msgs::Init {
+		features: node_b.init_features(&node_id_a),
+		networks: None,
+		remote_network_address: None,
+	};
+
+	node_a.node.peer_connected(&node_id_b, &init_b, true).unwrap();
+	node_b.node.peer_connected(&node_id_a, &init_a, false).unwrap();
+	node_a.onion_messenger.peer_connected(&node_id_b, &init_b, true).unwrap();
+	node_b.onion_messenger.peer_connected(&node_id_a, &init_a, false).unwrap();
+}
+
 pub fn connect_dummy_node<'a, 'b: 'a, 'c: 'b>(node: &Node<'a, 'b, 'c>) {
 	let node_id_dummy = PublicKey::from_slice(&[2; 33]).unwrap();
 
@@ -3643,13 +3647,8 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 		pending_cell_htlc_claims, pending_cell_htlc_fails, pending_raa,
 		pending_responding_commitment_signed, pending_responding_commitment_signed_dup_monitor,
 	} = args;
-	node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init {
-		features: node_b.node.init_features(), networks: None, remote_network_address: None
-	}, true).unwrap();
+	connect_nodes(node_a, node_b);
 	let reestablish_1 = get_chan_reestablish_msgs!(node_a, node_b);
-	node_b.node.peer_connected(&node_a.node.get_our_node_id(), &msgs::Init {
-		features: node_a.node.init_features(), networks: None, remote_network_address: None
-	}, false).unwrap();
 	let reestablish_2 = get_chan_reestablish_msgs!(node_b, node_a);
 
 	if send_channel_ready.0 {