lsp_plugin: add warning if extra_fee is wrong
What the developer wrote
Authored by Peter Neuroth
lsp_plugin: add warning if extra_fee is wrong
The extra_fee tlv appended to the update_add_htlc message is set by the
LSP and should specify what was deducted from a single htlc. If it does
not match the expected amount, we log a warning message.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
What changed, and why it matters
This change adds a warning log in the LSPS (Lightning Service Provider Specification) plugin when the extra fee reported by an LSP does not match the expected amount. It does not actually reject the problematic payment; it only logs a warning and includes a developer note saying a strict implementation should reject it. The change also removes some older placeholder comments and simplifies how the invoice total amount is determined.
Treat this as a minor hardening change, not a complete fix. If operating an LSPS client node, monitor for the new warning and consider whether to enforce rejection of HTLCs with mismatched extra_fee amounts until the FIXME is resolved upstream. Review the removed FIXME comments to ensure duplicate-payment and MPP protections are still tracked elsewhere.
Security signals we found
New validation check comparing htlc_amt + extra_fee against onion_amt
FIXME comment indicating current code does not reject mismatched amounts
extra_fee TLV read from untrusted update_add_htlc message
Removed defensive FIXME comments about duplicate payment and MPP checks
No functional rejection or error return on detected mismatch
Evidence from the diff
In plugins/lsps-plugin/src/client.rs, the on_htlc_accepted handler now reads the extra_fee TLV (type 65537) from the incoming HTLC, defaults it to 0 if absent, and checks whether htlc_amt + extra_fee equals onion_amt. If not, it emits a warning. A FIXME comment indicates the HTLC should arguably be rejected in strict mode, but currently it is not. The patch also removes several FIXME comments about duplicate-payment and MPP checks, and replaces a match on invoice.amount_msat with unwrap_or(htlc_amt).
Changed components
plugins/lsps-plugin/src/client.rsLSPS client plugin HTLC acceptance flowInspect captured patch +22 / −29
diff --git a/plugins/lsps-plugin/src/client.rs b/plugins/lsps-plugin/src/client.rs
index 4b56d543..57104b30 100644
--- a/plugins/lsps-plugin/src/client.rs
+++ b/plugins/lsps-plugin/src/client.rs
@@ -540,17 +540,6 @@ async fn on_htlc_accepted(
"htlc is a forward, continue"
);
- let extra_fee_msat = req
- .htlc
- .extra_tlvs
- .as_ref()
- .map(|tlvs| tlvs.get_u64(65537))
- .transpose()?
- .flatten();
- if let Some(amt) = extra_fee_msat {
- debug!("lsp htlc is deducted by an extra_fee={amt}");
- }
-
// 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);
@@ -572,12 +561,31 @@ async fn on_htlc_accepted(
// If we don't know about this payment it's not an LSP payment, continue.
some_or_continue!(lsp_data.datastore.first());
+ let extra_fee_msat = req
+ .htlc
+ .extra_tlvs
+ .as_ref()
+ .map(|tlvs| tlvs.get_u64(65537))
+ .transpose()?
+ .flatten()
+ .unwrap_or_default();
+
debug!(
- "incoming jit-channel htlc with htlc_amt={} and onion_amt={}",
+ "incoming jit-channel htlc with htlc_amt={}, onion_amt={} and extra_fee={}",
htlc_amt.msat(),
- onion_amt.msat()
+ onion_amt.msat(),
+ extra_fee_msat
);
+ if htlc_amt.msat() + extra_fee_msat != onion_amt.msat() {
+ warn!(
+ "amounts don't match (htlc_amt + extra_fee) = {} != onion_amt = {}",
+ (htlc_amt.msat() + extra_fee_msat),
+ onion_amt.msat()
+ );
+ // FIXME: If we are strict, we should reject the htlc here.
+ }
+
let inv_res = ok_or_continue!(cln_client
.call_typed(&ListinvoicesRequest {
index: None,
@@ -597,22 +605,7 @@ async fn on_htlc_accepted(
hex::encode(&req.htlc.payment_hash)
);
- let total_amt = match invoice.amount_msat {
- Some(a) => {
- debug!("invoice has total_amt={}msat", &a.msat());
- a.msat()
- }
- None => {
- debug!("invoice has no total amount, only accept single htlc");
- htlc_amt.msat()
- }
- };
-
- // Fixme: Check that we did not already pay for this channel.
- // - via datastore or invoice label.
-
- // Fixme: Check the if MPP or No-MPP, assuming No-MPP for now.
- // - check that extra_fee + htlc is the total_amount_msat of the onion.
+ let total_amt = invoice.amount_msat.unwrap_or(htlc_amt).msat();
let mut payload = req.onion.payload.clone();
payload.set_tu64(TLV_FORWARD_AMT, htlc_amt.msat());
Why this scored 26/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.