lsp_plugin: add hook helper to the client
What the developer wrote
Authored by Peter Neuroth
lsp_plugin: add hook helper to the client
We nearly always want to return `{"result":"continue"}` in case that
something went wrong. This commits adds two helper macros that help us
to cleanly return from a hook while logging necessary information.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
What changed, and why it matters
This commit refactors error handling in a Core Lightning plugin so that when something unexpected happens during payment processing, the plugin returns 'continue' instead of stopping or crashing. The change is defensive: it makes the plugin more resilient by avoiding hard failures that could disrupt normal routing. However, it also means some errors are now silently swallowed and logged only at debug level, which could hide problems. There is no direct evidence this fixes a known security vulnerability; it appears to be a robustness improvement.
Treat as a routine robustness improvement. Review whether silently continuing on RPC or datastore errors is acceptable for the LSPS plugin's security model, and consider whether warn! or error! logging is more appropriate than debug! for operational failures. No urgent security patch is indicated by the commit alone.
Security signals we found
Defensive error-handling refactor in a payment/routing plugin
Errors that previously propagated via ? now return 'continue' and are logged at debug level
Datastore deletion failure now returns continue instead of propagating error
RPC connection failures in hook handlers now return continue
No explicit security bug or CVE referenced in commit message or diff
Evidence from the diff
The patch introduces two Rust macros, ok_or_continue! and some_or_continue!, in plugins/lsps-plugin/src/client.rs. These macros convert Result/Option failures into a return of {“result”:”continue”}, often with only a debug! log. The macros are applied to deserialization, RPC connection, datastore deletion/lookup, invoice lookup, and onion payload encoding steps in on_invoice_payment, on_htlc_accepted, and on_openchannel. Previously, some paths already returned continue explicitly, while others used ? to propagate errors. The commit unifies behavior so the plugin does not fail closed. The change is partial: not every fallible call is wrapped, and the plugin still returns Ok/continue on errors rather than a hard failure.
Changed components
plugins/lsps-plugin/src/client.rson_invoice_payment hook handleron_htlc_accepted hook handleron_openchannel hook handlerInspect captured patch +100 / −50
diff --git a/plugins/lsps-plugin/src/client.rs b/plugins/lsps-plugin/src/client.rs
index 90e7b6b8..4b56d543 100644
--- a/plugins/lsps-plugin/src/client.rs
+++ b/plugins/lsps-plugin/src/client.rs
@@ -496,21 +496,27 @@ async fn on_invoice_payment(
p: cln_plugin::Plugin<State>,
v: serde_json::Value,
) -> Result<serde_json::Value, anyhow::Error> {
- let req: InvoicePaymentRequest = serde_json::from_value(v).context("invalid hook request")?;
- let preimage = <[u8; 32]>::from_hex(&req.payment.preimage).context("invalid preimage hex")?;
+ let req: InvoicePaymentRequest = ok_or_continue!(
+ serde_json::from_value(v).context("failed to deserialize htlc_accepted request JSON")
+ );
+ let preimage = ok_or_continue!(
+ <[u8; 32]>::from_hex(&req.payment.preimage).context("invalid preimage hex")
+ );
let hash = payment_hash(&preimage);
// Delete DS-entries.
let dir = p.configuration().lightning_dir;
let rpc_path = Path::new(&dir).join(&p.configuration().rpc_file);
- let mut cln_client = cln_rpc::ClnRpc::new(rpc_path.clone()).await?;
- cln_client
+ let mut cln_client = ok_or_continue!(cln_rpc::ClnRpc::new(rpc_path.clone())
+ .await
+ .context("failed to connect to core-lightning"));
+ ok_or_continue!(cln_client
.call_typed(&DeldatastoreRequest {
key: vec!["lsps".to_string(), "invoice".to_string(), hash.to_string()],
generation: None,
})
.await
- .ok();
+ .context("failed to delete datastore record"));
Ok(serde_json::json!({"result": "continue"}))
}
@@ -519,23 +525,20 @@ async fn on_htlc_accepted(
p: cln_plugin::Plugin<State>,
v: serde_json::Value,
) -> Result<serde_json::Value, anyhow::Error> {
- let req: HtlcAcceptedRequest = serde_json::from_value(v)?;
+ let req: HtlcAcceptedRequest = ok_or_continue!(
+ serde_json::from_value(v).context("failed to deserialize htlc_accepted request JSON")
+ );
let htlc_amt = req.htlc.amount_msat;
- let onion_amt = match req.onion.forward_msat {
- Some(a) => a,
- None => {
- debug!("onion is missing forward_msat, continue");
- let value = serde_json::to_value(HtlcAcceptedResponse::continue_(None, None, None))?;
- return Ok(value);
- }
- };
+ let onion_amt = some_or_continue!(
+ req.onion.forward_msat,
+ "missing forward_msat in onion, continue"
+ );
- let Some(payment_data) = req.onion.payload.get(TLV_PAYMENT_SECRET) else {
- debug!("payment is a forward, continue");
- let value = serde_json::to_value(HtlcAcceptedResponse::continue_(None, None, None))?;
- return Ok(value);
- };
+ let payment_data = some_or_continue!(
+ req.onion.payload.get(TLV_PAYMENT_SECRET),
+ "htlc is a forward, continue"
+ );
let extra_fee_msat = req
.htlc
@@ -551,8 +554,11 @@ async fn on_htlc_accepted(
// Check that the htlc belongs to a jit-channel request.
let dir = p.configuration().lightning_dir;
let rpc_path = Path::new(&dir).join(&p.configuration().rpc_file);
- let mut cln_client = cln_rpc::ClnRpc::new(rpc_path.clone()).await?;
- let lsp_data = cln_client
+ let mut cln_client = ok_or_continue!(cln_rpc::ClnRpc::new(rpc_path.clone())
+ .await
+ .context("failed to connect to core-lightning"));
+
+ let lsp_data = ok_or_continue!(cln_client
.call_typed(&ListdatastoreRequest {
key: Some(vec![
"lsps".to_string(),
@@ -560,14 +566,11 @@ async fn on_htlc_accepted(
hex::encode(&req.htlc.payment_hash),
]),
})
- .await?;
+ .await
+ .context("failed to fetch datastore record"));
- if lsp_data.datastore.first().is_none() {
- // Not an LSP payment, just continue
- debug!("payment is a not a jit-channel-opening, continue");
- let value = serde_json::to_value(HtlcAcceptedResponse::continue_(None, None, None))?;
- return Ok(value);
- };
+ // If we don't know about this payment it's not an LSP payment, continue.
+ some_or_continue!(lsp_data.datastore.first());
debug!(
"incoming jit-channel htlc with htlc_amt={} and onion_amt={}",
@@ -575,7 +578,7 @@ async fn on_htlc_accepted(
onion_amt.msat()
);
- let inv_res = cln_client
+ let inv_res = ok_or_continue!(cln_client
.call_typed(&ListinvoicesRequest {
index: None,
invstring: None,
@@ -585,16 +588,14 @@ async fn on_htlc_accepted(
payment_hash: Some(hex::encode(&req.htlc.payment_hash)),
start: None,
})
- .await?;
+ .await
+ .context("failed to get invoice"));
- let Some(invoice) = inv_res.invoices.first() else {
- debug!(
- "no invoice found for jit-channel opening with payment_hash={}",
- hex::encode(&req.htlc.payment_hash)
- );
- let value = serde_json::to_value(HtlcAcceptedResponse::continue_(None, None, None))?;
- return Ok(value);
- };
+ let invoice = some_or_continue!(
+ inv_res.invoices.first(),
+ "no invoice found for jit-channel-opening with payment_hash={}",
+ hex::encode(&req.htlc.payment_hash)
+ );
let total_amt = match invoice.amount_msat {
Some(a) => {
@@ -620,14 +621,9 @@ async fn on_htlc_accepted(
ps.extend_from_slice(&payment_data[0..32]);
ps.extend(encode_tu64(total_amt));
payload.insert(TLV_PAYMENT_SECRET, ps);
- let payload_bytes = match payload.to_bytes() {
- Ok(b) => b,
- Err(e) => {
- warn!("can't encode payload to bytes {}", e);
- let value = serde_json::to_value(HtlcAcceptedResponse::continue_(None, None, None))?;
- return Ok(value);
- }
- };
+ let payload_bytes = ok_or_continue!(payload
+ .to_bytes()
+ .context("failed to encode payload as bytes"));
info!(
"Amended onion payload with forward_amt={} and total_msat={}",
@@ -652,11 +648,16 @@ async fn on_openchannel(
id: String,
}
- let req: Request = serde_json::from_value(v.get("openchannel").unwrap().clone())
- .context("Failed to parse request JSON")?;
+ let req: Request = ok_or_continue!(serde_json::from_value(
+ v.get("openchannel").unwrap().clone()
+ )
+ .context("failed to deserialize open_channel request JSON"));
+
let dir = p.configuration().lightning_dir;
let rpc_path = Path::new(&dir).join(&p.configuration().rpc_file);
- let mut cln_client = cln_rpc::ClnRpc::new(rpc_path.clone()).await?;
+ let mut cln_client = ok_or_continue!(cln_rpc::ClnRpc::new(rpc_path.clone())
+ .await
+ .context("failed to connect to core-lightning"));
let ds_req = ListdatastoreRequest {
key: Some(vec![
@@ -665,7 +666,11 @@ async fn on_openchannel(
req.id.clone(),
]),
};
- let ds_res = cln_client.call_typed(&ds_req).await?;
+ let ds_res = ok_or_continue!(cln_client
+ .call_typed(&ds_req)
+ .await
+ .context("failed to get datastore record"));
+
if let Some(_rec) = ds_res.datastore.iter().next() {
info!("Allowing zero-conf channel from LSP {}", &req.id);
let ds_req = DeldatastoreRequest {
@@ -791,6 +796,51 @@ pub fn payment_hash(preimage: &[u8]) -> sha256::Hash {
sha256::Hash::hash(preimage)
}
+fn continue_ok() -> Result<serde_json::Value, anyhow::Error> {
+ Ok(serde_json::json!({"result": "continue"}))
+}
+
+#[macro_export]
+macro_rules! some_or_continue {
+ ($expr:expr) => {
+ match $expr {
+ Some(v) => v,
+ None => return continue_ok(),
+ }
+ };
+ ($expr:expr, $($log:tt)+) => {
+ match $expr {
+ Some(v) => v,
+ None => {
+ debug!($($log)+);
+ return continue_ok();
+ },
+ }
+ };
+}
+
+#[macro_export]
+macro_rules! ok_or_continue {
+ ($expr:expr) => {
+ match $expr {
+ Ok(v) => v,
+ Err(e) => {
+ debug!("{:#}", e);
+ return continue_ok();
+ }
+ }
+ };
+ ($expr:expr, $($log:tt)+) => {
+ match $expr {
+ Ok(v) => v,
+ Err(e) => {
+ debug!("{}: {:#}",format_args!($($log)+), e);
+ return continue_ok();
+ }
+ }
+ };
+}
+
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
struct LspsBuyJitChannelResponse {
bolt11: String,
Why this scored 29/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.