lsp_plugin: relax LSP feateture bit handling
What the developer wrote
Authored by Peter Neuroth
lsp_plugin: relax LSP feateture bit handling
Replace ensure_lsp_connected() by check_peer_lsp_status() which only
returns the status of the peer (connected, has_lsp_feature). This allows
us to be more tolearant about the LSP feature bit since it is only
optional according to the spec.
We still check for the feature but only return a warning in the logs.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
What changed, and why it matters
This change makes a Core Lightning plugin less strict about a peer advertising a specific optional feature flag (the 'LSP feature bit'). Previously, if a peer didn't advertise that bit, the plugin would refuse to talk to it. Now it logs a warning and continues. This is a spec-compliance/usability improvement, not a clear security fix, but it slightly increases the chance of connecting to a peer that doesn't truthfully advertise its capabilities.
Treat as a normal code review item. Verify that downstream LSPS protocol logic does not assume the feature bit implies the peer is a trustworthy LSP, since the bit is now only advisory. No urgent security action is indicated by the diff alone.
Security signals we found
Reduced strictness on optional feature-bit enforcement
Continued operation against peers that do not advertise LSP capability
No change to transport encryption or authentication
No explicit security relevance disclosed by vendor
Evidence from the diff
The commit replaces ensure_lsp_connected() with check_peer_lsp_status() in plugins/lsps-plugin/src/client.rs. The new helper returns a PeerLspStatus struct (connected, has_lsp_feature) instead of failing when the LSP feature bit is absent. Callers now bail only if the peer is not connected, and log a debug message when the feature bit is missing. The change also converts an error context into an explicit anyhow! for list_protocols failures and adds hex-decode error handling for invalid feature bit strings.
Changed components
plugins/lsps-plugin/src/client.rson_lsps_lsps2_getinfoon_lsps_lsps2_buyon_lsps_listprotocolsBolt8Transport initialization pathsInspect captured patch +73 / −39
diff --git a/plugins/lsps-plugin/src/client.rs b/plugins/lsps-plugin/src/client.rs
index 6116fa4a..c72ccc64 100644
--- a/plugins/lsps-plugin/src/client.rs
+++ b/plugins/lsps-plugin/src/client.rs
@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Context};
+use anyhow::{anyhow, bail, Context};
use chrono::{Duration, Utc};
use cln_lsps::jsonrpc::client::JsonRpcClient;
use cln_lsps::lsps0::primitives::Msat;
@@ -112,9 +112,19 @@ async fn on_lsps_lsps2_getinfo(
let rpc_path = Path::new(&dir).join(&p.configuration().rpc_file);
let mut cln_client = cln_rpc::ClnRpc::new(rpc_path.clone()).await?;
- // Fail early: Check that we are connected to the peer and that it has the
- // LSP feature bit set.
- ensure_lsp_connected(&mut cln_client, &req.lsp_id).await?;
+ let lsp_status = check_peer_lsp_status(&mut cln_client, &req.lsp_id).await?;
+
+ // Fail early: Check that we are connected to the peer.
+ if !lsp_status.connected {
+ bail!("Not connected to peer {}", &req.lsp_id);
+ };
+
+ // From Blip52: LSPs MAY set the features bit numbered 729
+ // (option_supports_lsps)...
+ // We only log that it is not set but don't fail.
+ if !lsp_status.has_lsp_feature {
+ debug!("Peer {} doesn't have the LSP feature bit set.", &req.lsp_id);
+ }
// Create Transport and Client
let transport = Bolt8Transport::new(
@@ -153,9 +163,19 @@ async fn on_lsps_lsps2_buy(
let rpc_path = Path::new(&dir).join(&p.configuration().rpc_file);
let mut cln_client = cln_rpc::ClnRpc::new(rpc_path.clone()).await?;
- // Fail early: Check that we are connected to the peer and that it has the
- // LSP feature bit set.
- ensure_lsp_connected(&mut cln_client, &req.lsp_id).await?;
+ let lsp_status = check_peer_lsp_status(&mut cln_client, &req.lsp_id).await?;
+
+ // Fail early: Check that we are connected to the peer.
+ if !lsp_status.connected {
+ bail!("Not connected to peer {}", &req.lsp_id);
+ };
+
+ // From Blip52: LSPs MAY set the features bit numbered 729
+ // (option_supports_lsps)...
+ // We only log that it is not set but don't fail.
+ if !lsp_status.has_lsp_feature {
+ debug!("Peer {} doesn't have the LSP feature bit set.", &req.lsp_id);
+ }
// Create Transport and Client
let transport = Bolt8Transport::new(
@@ -542,10 +562,19 @@ async fn on_lsps_listprotocols(
let mut cln_client = cln_rpc::ClnRpc::new(rpc_path.clone()).await?;
let req: Request = serde_json::from_value(v).context("Failed to parse request JSON")?;
+ let lsp_status = check_peer_lsp_status(&mut cln_client, &req.lsp_id).await?;
+
+ // Fail early: Check that we are connected to the peer.
+ if !lsp_status.connected {
+ bail!("Not connected to peer {}", &req.lsp_id);
+ };
- // Fail early: Check that we are connected to the peer and that it has the
- // LSP feature bit set.
- ensure_lsp_connected(&mut cln_client, &req.lsp_id).await?;
+ // From Blip52: LSPs MAY set the features bit numbered 729
+ // (option_supports_lsps)...
+ // We only log that it is not set but don't fail.
+ if !lsp_status.has_lsp_feature {
+ debug!("Peer {} doesn't have the LSP feature bit set.", &req.lsp_id);
+ }
// Create the transport first and handle potential errors
let transport = Bolt8Transport::new(
@@ -563,48 +592,53 @@ async fn on_lsps_listprotocols(
let res: lsps0::model::Lsps0listProtocolsResponse = client
.call_typed(request)
.await
- .context("lsps0.list_protocols call failed")?;
+ .map_err(|e| anyhow!("lsps0.list_protocols call failed: {}", e))?;
debug!("Received lsps0.list_protocols response: {:?}", res);
Ok(serde_json::to_value(res)?)
}
-/// Checks that the node is connected to the peer and that it has the LSP
-/// feature bit set.
-async fn ensure_lsp_connected(cln_client: &mut ClnRpc, lsp_id: &str) -> Result<(), anyhow::Error> {
+struct PeerLspStatus {
+ connected: bool,
+ has_lsp_feature: bool,
+}
+
+/// Returns the `PeerLspStatus`, containing information about the connectivity
+/// and the LSP feature bit.
+async fn check_peer_lsp_status(
+ cln_client: &mut ClnRpc,
+ peer_id: &str,
+) -> Result<PeerLspStatus, anyhow::Error> {
let res = cln_client
.call_typed(&ListpeersRequest {
- id: Some(PublicKey::from_str(lsp_id)?),
+ id: Some(PublicKey::from_str(peer_id)?),
level: None,
})
.await?;
- // unwrap in next line is safe as we checked that an item exists before.
- if res.peers.is_empty() || !res.peers.first().unwrap().connected {
- debug!("Node isn't connected to lsp {lsp_id}");
- return Err(anyhow!("not connected to lsp"));
- }
-
- res.peers
- .first()
- .filter(|peer| {
- // Check that feature bit is set
- peer.features.as_deref().map_or(false, |f_str| {
- if let Some(feature_bits) = hex::decode(f_str).ok() {
- util::is_feature_bit_set_reversed(&feature_bits, LSP_FEATURE_BIT)
- } else {
- false
- }
+ let peer = match res.peers.first() {
+ None => {
+ return Ok(PeerLspStatus {
+ connected: false,
+ has_lsp_feature: false,
})
- })
- .ok_or_else(|| {
- anyhow!(
- "peer is not an lsp, feature bit {} is missing",
- LSP_FEATURE_BIT,
- )
- })?;
+ }
+ Some(p) => p,
+ };
+
+ let connected = peer.connected;
+ let has_lsp_feature = if let Some(f_str) = &peer.features {
+ let feature_bits = hex::decode(f_str)
+ .map_err(|e| anyhow!("Invalid feature bits hex for peer {peer_id}, {f_str}: {e}"))?;
+ util::is_feature_bit_set_reversed(&feature_bits, LSP_FEATURE_BIT)
+ } else {
+ false
+ };
- Ok(())
+ Ok(PeerLspStatus {
+ connected,
+ has_lsp_feature,
+ })
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
Why this scored 30/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.