Merge pull request #3493 from tnull/2024-12-3436-followup

Follow-ups to #3436

Author: TheBlueMatt

lightning-liquidity/README.md

@@ -2,13 +2,18 @@
 
 The goal of this crate is to provide types and primitives to integrate a spec-compliant LSP with an LDK-based node. To this end, this crate provides client-side as well as service-side logic to implement the [LSP specifications].
 
+**Note**: Service-side support is currently considered "beta", i.e., not fully
+ready for production use.
+
 Currently the following specifications are supported:
 - [LSPS0] defines the transport protocol with the LSP over which the other protocols communicate.
 - [LSPS1] allows to order Lightning channels from an LSP. This is useful when the client needs
 inbound Lightning liquidity for which they are willing and able to pay in bitcoin.
-- [LSPS2] allows to generate a special invoice for which, when paid, an LSP will open a "just-in-time".
-This is useful for the initial on-boarding of clients as the channel opening fees are deducted
-from the incoming payment, i.e., no funds are required client-side to initiate this flow.
+- [LSPS2] allows to generate a special invoice for which, when paid, an LSP
+  will open a "just-in-time" channel. This is useful for the initial
+  on-boarding of clients as the channel opening fees are deducted from the
+  incoming payment, i.e., no funds are required client-side to initiate this
+  flow.
 
 To get started, you'll want to setup a `LiquidityManager` and configure it to be the `CustomMessageHandler` of your LDK node. You can then call `LiquidityManager::lsps1_client_handler` / `LiquidityManager::lsps2_client_handler`, or `LiquidityManager::lsps2_service_handler`, to access the respective client-side or service-side handlers.
 

lightning-liquidity/src/lib.rs

@@ -10,13 +10,17 @@
 
 //! The goal of this crate is to provide types and primitives to integrate a spec-compliant LSP with an LDK-based node. To this end, this crate provides client-side as well as service-side logic to implement the [LSP specifications].
 //!
+//! **Note**: Service-side support is currently considered "beta", i.e., not fully ready for
+//! production use.
+//!
 //! Currently the following specifications are supported:
 //! - [LSPS0] defines the transport protocol with the LSP over which the other protocols communicate.
 //! - [LSPS1] allows to order Lightning channels from an LSP. This is useful when the client needs
 //! inbound Lightning liquidity for which they are willing and able to pay in bitcoin.
-//! - [LSPS2] allows to generate a special invoice for which, when paid, an LSP will open a "just-in-time".
-//! This is useful for the initial on-boarding of clients as the channel opening fees are deducted
-//! from the incoming payment, i.e., no funds are required client-side to initiate this flow.
+//! - [LSPS2] allows to generate a special invoice for which, when paid, an LSP will open a
+//! "just-in-time" channel. This is useful for the initial on-boarding of clients as the channel
+//! opening fees are deducted from the incoming payment, i.e., no funds are required client-side to
+//! initiate this flow.
 //!
 //! To get started, you'll want to setup a [`LiquidityManager`] and configure it to be the
 //! [`CustomMessageHandler`] of your LDK node. You can then for example call

lightning-liquidity/src/lsps0/client.rs

@@ -1,4 +1,4 @@
-//! Contains the main LSPS2 client-side object, [`LSPS0ClientHandler`].
+//! Contains the main LSPS0 client-side object, [`LSPS0ClientHandler`].
 //!
 //! Please refer to the [LSPS0
 //! specifcation](https://github.com/BitcoinAndLightningLayerSpecs/lsp/tree/main/LSPS0) for more

lightning-liquidity/src/lsps0/msgs.rs

@@ -1,4 +1,4 @@
-//! Message, request, and other primitive types used to implement LSPS1.
+//! Message, request, and other primitive types used to implement LSPS0.
 
 use crate::lsps0::ser::{LSPSMessage, RequestId, ResponseError};
 use crate::prelude::Vec;

lightning-liquidity/src/manager.rs

@@ -75,13 +75,18 @@ pub struct LiquidityClientConfig {
 /// Users need to continually poll [`LiquidityManager::get_and_clear_pending_events`] in order to surface
 /// [`Event`]'s that likely need to be handled.
 ///
-/// If configured, users must forward the [`Event::HTLCIntercepted`] event parameters to [`LSPS2ServiceHandler::htlc_intercepted`]
-/// and the [`Event::ChannelReady`] event parameters to [`LSPS2ServiceHandler::channel_ready`].
+/// If the LSPS2 service is configured, users must forward the following parameters from LDK events:
+/// - [`Event::HTLCIntercepted`] to [`LSPS2ServiceHandler::htlc_intercepted`]
+/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
+/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
+/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
 ///
 /// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
 /// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
 /// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
 /// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
+/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
+/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
 pub struct LiquidityManager<ES: Deref + Clone, CM: Deref + Clone, C: Deref + Clone>
 where
 	ES::Target: EntropySource,
@@ -102,7 +107,7 @@ where
 	lsps2_client_handler: Option<LSPS2ClientHandler<ES>>,
 	service_config: Option<LiquidityServiceConfig>,
 	_client_config: Option<LiquidityClientConfig>,
-	best_block: Option<RwLock<BestBlock>>,
+	best_block: RwLock<Option<BestBlock>>,
 	_chain_source: Option<C>,
 }
 
@@ -210,7 +215,7 @@ where {
 			lsps2_service_handler,
 			service_config,
 			_client_config: client_config,
-			best_block: chain_params.map(|chain_params| RwLock::new(chain_params.best_block)),
+			best_block: RwLock::new(chain_params.map(|chain_params| chain_params.best_block)),
 			_chain_source: chain_source,
 		}
 	}
@@ -613,6 +618,9 @@ where
 	}
 
 	fn peer_disconnected(&self, counterparty_node_id: bitcoin::secp256k1::PublicKey) {
+		// If the peer was misbehaving, drop it from the ignored list to cleanup the kept state.
+		self.ignored_peers.write().unwrap().remove(&counterparty_node_id);
+
 		if let Some(lsps2_service_handler) = self.lsps2_service_handler.as_ref() {
 			lsps2_service_handler.peer_disconnected(counterparty_node_id);
 		}
@@ -634,8 +642,7 @@ where
 		&self, header: &bitcoin::block::Header, txdata: &chain::transaction::TransactionData,
 		height: u32,
 	) {
-		if let Some(best_block) = &self.best_block {
-			let best_block = best_block.read().unwrap();
+		if let Some(best_block) = self.best_block.read().unwrap().as_ref() {
 			assert_eq!(best_block.block_hash, header.prev_blockhash,
 			"Blocks must be connected in chain-order - the connected header must build on the last connected header");
 			assert_eq!(best_block.height, height - 1,
@@ -648,8 +655,7 @@ where
 
 	fn block_disconnected(&self, header: &bitcoin::block::Header, height: u32) {
 		let new_height = height - 1;
-		if let Some(best_block) = &self.best_block {
-			let mut best_block = best_block.write().unwrap();
+		if let Some(best_block) = self.best_block.write().unwrap().as_mut() {
 			assert_eq!(best_block.block_hash, header.block_hash(),
 				"Blocks must be disconnected in chain-order - the disconnected header must be the last connected header");
 			assert_eq!(best_block.height, height,
@@ -682,7 +688,10 @@ where
 		// confirmed at a height <= the one we now unconfirmed.
 	}
 
-	fn best_block_updated(&self, _header: &bitcoin::block::Header, _height: u32) {
+	fn best_block_updated(&self, header: &bitcoin::block::Header, height: u32) {
+		let new_best_block = BestBlock::new(header.block_hash(), height);
+		*self.best_block.write().unwrap() = Some(new_best_block);
+
 		// TODO: Call best_block_updated on all sub-modules that require it, e.g., LSPS1MessageHandler.
 		if let Some(lsps2_service_handler) = self.lsps2_service_handler.as_ref() {
 			lsps2_service_handler.prune_peer_state();