injectpayment_onion: fix fees for blinded paths
What changed, and why it matters
This commit fixes a bug in Core Lightning's `injectpaymentonion` RPC command. Previously, when a user manually injected a payment onion (a way to send a Lightning payment with a pre-built route), the command treated the `amount_msat` parameter as the amount to forward to the next peer, rather than the amount this node is expected to receive. For normal routes this was fine, but for blinded paths—where the true recipient and fees are hidden inside an encrypted onion—the node could accidentally send too much money to the next hop, effectively overpaying fees or losing funds. The fix changes the code to use the amount encoded inside the onion payload (`amt_to_forward`) as the actual amount to send onward, which is correct for blinded paths. The commit also removes an 'expected failure' marker from a test, showing the bug is now fixed.
Users and operators exposing `injectpaymentonion` should upgrade to a release containing this commit, especially if they build routes involving blinded paths. Review any past uses of `injectpaymentonion` with blinded paths for possible overpayment. No immediate emergency response is indicated, but the fix should be included in the next maintenance release.
Security signals we found
Incorrect amount used for outgoing HTLC in manual onion injection path
Blinded path fees could cause overpayment to next hop
Fix changes forward amount from user parameter to onion payload value
Test previously expected to fail now expected to pass
Documentation updated to clarify semantics of amount_msat
Evidence from the diff
In json_injectpaymentonion() in lightningd/pay.c, the code was using the user-supplied *msat (the HTLC amount held by this node) as the forward amount in send_htlc_out(), htlc_set_add(), best_channel(), and HTLC min/max checks. For non-blinded self-payments this coincides with the forward amount because no fees are charged locally, but for blinded paths the onion’s payload->amt_to_forward is the correct outgoing amount. The patch replaces *msat with payload->amt_to_forward in those call sites. Documentation in injectpaymentonion.json and the generated schema is updated to clarify that amount_msat is the amount received from a peer, not the amount forwarded. A test in test_xpay.py that was marked xfail is now expected to pass, and a selfpay test amount is adjusted.
Changed components
lightningd/pay.cdoc/schemas/injectpaymentonion.jsoncontrib/msggen/msggen/schema.jsontests/test_pay.pytests/test_xpay.pyInspect captured patch +14 / −15
diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json
index 156151ce..251db75c 100644
--- a/contrib/msggen/msggen/schema.json
+++ b/contrib/msggen/msggen/schema.json
@@ -16124,7 +16124,7 @@
"amount_msat": {
"type": "msat",
"description": [
- "The amount for the first HTLC in millisatoshis. This is also the amount which will be forwarded to the first peer (if any) as we do not charge fees on our own payments. Note: this is shown in listsendpays as `amount_sent_msat`."
+ "The amount in millisatoshis this node would receive from a peer before forwarding the payment to the next."
]
},
"cltv_expiry": {
diff --git a/doc/schemas/injectpaymentonion.json b/doc/schemas/injectpaymentonion.json
index 71d7aaaa..b551705f 100644
--- a/doc/schemas/injectpaymentonion.json
+++ b/doc/schemas/injectpaymentonion.json
@@ -34,7 +34,7 @@
"amount_msat": {
"type": "msat",
"description": [
- "The amount for the first HTLC in millisatoshis. This is also the amount which will be forwarded to the first peer (if any) as we do not charge fees on our own payments. Note: this is shown in listsendpays as `amount_sent_msat`."
+ "The amount in millisatoshis this node would receive from a peer before forwarding the payment to the next."
]
},
"cltv_expiry": {
diff --git a/lightningd/pay.c b/lightningd/pay.c
index 1669f2f9..9fffd956 100644
--- a/lightningd/pay.c
+++ b/lightningd/pay.c
@@ -1973,7 +1973,7 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
register_payment_and_waiter(cmd,
payment_hash,
*partid, *groupid,
- *destination_msat, *msat, AMOUNT_MSAT(0),
+ *destination_msat, payload->amt_to_forward, AMOUNT_MSAT(0),
label, invstring, local_invreq_id,
&shared_secret,
destination);
@@ -1981,7 +1981,7 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
/* Mark it pending now, though htlc_set_add might
* not resolve immediately */
fixme_ignore(command_still_pending(cmd));
- htlc_set_add(cmd->ld, cmd->ld->log, *msat, *payload->total_msat,
+ htlc_set_add(cmd->ld, cmd->ld->log, payload->amt_to_forward, *payload->total_msat,
NULL, payment_hash, payload->payment_secret,
selfpay_mpp_fail, selfpay_mpp_succeeded,
selfpay);
@@ -2017,22 +2017,22 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
"Unknown peer %s",
fmt_node_id(tmpctx, &nid));
- next = best_channel(cmd->ld, next_peer, *msat, NULL);
+ next = best_channel(cmd->ld, next_peer, payload->amt_to_forward, NULL);
if (!next)
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"No available channel with peer %s",
fmt_node_id(tmpctx, &nid));
}
- if (amount_msat_greater(*msat, next->htlc_maximum_msat)
- || amount_msat_less(*msat, next->htlc_minimum_msat)) {
+ if (amount_msat_greater(payload->amt_to_forward, next->htlc_maximum_msat)
+ || amount_msat_less(payload->amt_to_forward, next->htlc_minimum_msat)) {
/* Are we in old-range grace-period? */
if (!timemono_before(time_mono(), next->old_feerate_timeout)
- || amount_msat_less(*msat, next->old_htlc_minimum_msat)
- || amount_msat_greater(*msat, next->old_htlc_maximum_msat)) {
+ || amount_msat_less(payload->amt_to_forward, next->old_htlc_minimum_msat)
+ || amount_msat_greater(payload->amt_to_forward, next->old_htlc_maximum_msat)) {
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Amount %s not in htlc min/max range %s-%s",
- fmt_amount_msat(tmpctx, *msat),
+ fmt_amount_msat(tmpctx, payload->amt_to_forward),
fmt_amount_msat(tmpctx, next->htlc_minimum_msat),
fmt_amount_msat(tmpctx, next->htlc_maximum_msat));
}
@@ -2085,11 +2085,11 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
if (command_check_only(cmd))
return command_check_done(cmd);
- failmsg = send_htlc_out(tmpctx, next, *msat,
+ failmsg = send_htlc_out(tmpctx, next, payload->amt_to_forward,
*cltv,
/* If unknown, we set this equal (so accounting logs 0 fees) */
amount_msat_eq(*destination_msat, AMOUNT_MSAT(0))
- ? *msat : *destination_msat,
+ ? payload->amt_to_forward : *destination_msat,
payment_hash,
next_path_key, NULL, *partid, *groupid,
serialize_onionpacket(tmpctx, rs->next),
@@ -2104,7 +2104,7 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
register_payment_and_waiter(cmd,
payment_hash,
*partid, *groupid,
- *destination_msat, *msat, AMOUNT_MSAT(0),
+ *destination_msat, payload->amt_to_forward, AMOUNT_MSAT(0),
label, invstring, local_invreq_id,
&shared_secret,
destination);
diff --git a/tests/test_pay.py b/tests/test_pay.py
index 2d0c30f8..9d1fc2f6 100644
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -6360,7 +6360,7 @@ def test_injectpaymentonion_selfpay(node_factory, executor):
'payload': serialize_payload_final_tlv(333, 18, 1000, blockheight, inv5['payment_secret']).hex()}]
onion1 = l1.rpc.createonion(hops=hops1, assocdata=inv5['payment_hash'])
hops2 = [{'pubkey': l1.info['id'],
- 'payload': serialize_payload_final_tlv(666, 18, 1000, blockheight, inv5['payment_secret']).hex()}]
+ 'payload': serialize_payload_final_tlv(667, 18, 1000, blockheight, inv5['payment_secret']).hex()}]
onion2 = l1.rpc.createonion(hops=hops2, assocdata=inv5['payment_hash'])
fut1 = executor.submit(l1.rpc.injectpaymentonion,
diff --git a/tests/test_xpay.py b/tests/test_xpay.py
index 2d909ccf..77b10b51 100644
--- a/tests/test_xpay.py
+++ b/tests/test_xpay.py
@@ -1074,7 +1074,6 @@ def test_xpay_blockheight_mismatch(node_factory, bitcoind, executor):
fut.result(TIMEOUT)
-@pytest.mark.xfail(strict=True)
def test_blinded_path_fees(node_factory):
"""Test that we don't send the amount+fees to our direct peer (we should
only send the required amount) when the sending node is the entry point in
Why this scored 61/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.