xpay: add helper for payment deliver
What the developer wrote
Authored by Lagrang3
xpay: add helper for payment deliver
Don't assume that payment->amount means the desired delivery amount.
Use a helper instead when we need that.
Changelog-None
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
What changed, and why it matters
This commit refactors how the xpay plugin tracks the intended delivery amount for a Lightning payment. Previously, the code assumed payment->amount always meant the amount the recipient should receive. The change introduces helper functions so the code can distinguish between the total desired delivery amount and the amount currently tied up in active routing attempts. This is a defensive cleanup that may prevent bugs where fees or partial payments are mis-accounted, but the commit itself does not describe a specific vulnerability or exploit.
Treat as a hardening/refactor commit. Review the semantic change in waitblockheight_done() to confirm that switching from total_being_delivered() to payment_current_amount() does not introduce a regression in amount accounting. No immediate security patch or incident response is indicated by the commit itself.
Security signals we found
Refactor of payment amount semantics in routing/fee logic
Change from total_being_delivered() to payment_current_amount() in waitblockheight_done()
Introduction of helper to centralize 'desired delivery amount' interpretation
No changelog entry and no explicit security framing by author
Evidence from the diff
The patch in plugins/xpay/xpay.c introduces payment_deliver() and payment_current_amount() helpers. payment_deliver() returns the intended deliverable amount (currently just payment->amount), while payment_current_amount() sums the amounts of all current routing attempts. Several call sites are updated: logging now uses payment_deliver() for clarity, getroutes_done() subtracts total_being_delivered() from payment_deliver() to decide if more routing is needed, and waitblockheight_done() now subtracts payment_current_amount() from payment->amount instead of total_being_delivered(). The new_payment() partial validation is also changed to check *partial directly rather than payment->amount. The commit message frames this as a refactor to avoid assuming payment->amount semantics.
Changed components
plugins/xpay/xpay.cLightning payment routing logicPartial payment handlingwaitblockheight and getroutes callbacksInspect captured patch +25 / −6
diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c
index ad9325d4..0fb5ff5c 100644
--- a/plugins/xpay/xpay.c
+++ b/plugins/xpay/xpay.c
@@ -88,7 +88,8 @@ struct payment {
struct pubkey destination;
/* Hash we want the preimage for */
struct sha256 payment_hash;
- /* Amount we're trying to pay */
+ /* Amount, either the desired deliver or desired spend amount depending
+ * on the context. */
struct amount_msat amount;
/* Relevant for partial payments. This is the value that must be written
* in the final hop's payload for MPP coordination. */
@@ -599,6 +600,12 @@ static struct amount_msat total_delivered(const struct payment *payment)
return sum;
}
+/* This payment should deliver this amount. */
+static struct amount_msat payment_deliver(const struct payment *payment)
+{
+ return payment->amount;
+}
+
/* We can notify others of what the details are, so they can do their own
* layer heuristics. */
static void json_add_attempt_fields(struct json_stream *js,
@@ -1172,7 +1179,7 @@ check_previous_success:
description,
fmt_amount_msat(tmpctx,
total_delivered(attempt->payment)),
- fmt_amount_msat(tmpctx, attempt->payment->amount));
+ fmt_amount_msat(tmpctx, payment_deliver(attempt->payment)));
}
}
@@ -1242,6 +1249,18 @@ static struct amount_msat total_being_delivered(const struct payment *payment)
return sum;
}
+static struct amount_msat payment_current_amount(const struct payment *payment)
+{
+ struct attempt *attempt;
+ struct amount_msat sum = AMOUNT_MSAT(0);
+
+ list_for_each(&payment->current_attempts, attempt, list) {
+ if (!amount_msat_accumulate(&sum, attempt->amount))
+ abort();
+ }
+ return sum;
+}
+
static struct amount_msat attempt_fee(const struct attempt *attempt)
{
const size_t len = tal_count(attempt->hops);
@@ -1591,7 +1610,7 @@ static struct command_result *getroutes_done(struct command *aux_cmd,
/* Do we have more that needs routing? If so, re-ask */
if (!amount_msat_sub(&needs_routing,
- payment->amount,
+ payment_deliver(payment),
total_being_delivered(payment)))
abort();
@@ -1745,7 +1764,7 @@ static struct command_result *waitblockheight_done(struct command *aux_cmd,
if (!amount_msat_sub(&needs_routing,
payment->amount,
- total_being_delivered(payment)))
+ payment_current_amount(payment)))
abort();
return getroutes_for(aux_cmd, payment, needs_routing);
}
@@ -2375,12 +2394,12 @@ static struct payment *new_payment(const tal_t *ctx,
payment->mpp_amount = mpp_amount;
if (partial) {
payment->amount = *partial;
- if (amount_msat_greater(payment->amount, payment->mpp_amount)) {
+ if (amount_msat_greater(*partial, payment->mpp_amount)) {
*err = tal_fmt(ctx, "partial_msat must be less or equal to total amount %s",
fmt_amount_msat(tmpctx, payment->mpp_amount));
return tal_free(payment);
}
- if (amount_msat_is_zero(payment->amount)) {
+ if (amount_msat_is_zero(*partial)) {
*err = tal_fmt(ctx, "partial_msat must be non-zero");
return tal_free(payment);
}
Why this scored 31/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.