diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f8f563c7..3a7d080bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,13 @@ current chain tip now aborts with a new `BuildError::ChainTipFetchFailed` variant instead of silently pinning the wallet birthday to genesis, which would have forced a full-history rescan once the chain source became reachable again. (#884) +- `Bolt11Payment::claim_for_id`'s `claimable_amount_msat` argument is now checked against the + amount actually reported by the triggering `PaymentClaimable` event, catching a caller mixing + up arguments across concurrent manual claims. Previously, for any payment received since the + payment-ID refactor in v0.8-development, this check compared the argument against a value + derived from the very same event, making it ineffective for well-behaved callers and silently + permissive of mismatched ones. The historic guard against underpayment (net of any + JIT-channel-opening LSP fee) is preserved for payments serialized before that refactor. # 0.7.0 - Dec. 3, 2025 This seventh minor release introduces numerous new features, bug fixes, and API improvements. In particular, it adds support for channel Splicing, Async Payments, as well as sourcing chain data from a Bitcoin Core REST backend. diff --git a/explanation.md b/explanation.md new file mode 100644 index 000000000..b31db3fc0 --- /dev/null +++ b/explanation.md @@ -0,0 +1,210 @@ +# Explanation: `claim_for_id` amount validation fix + +Branch: `fix/claim-for-id-amount-validation` +Commit: `91ac482` — "fix: validate claim_for_id amount against the observed PaymentClaimable event" + +## The bug + +`Bolt11Payment::claim_for_id(payment_id, claimable_amount_msat, preimage)` lets an +application manually claim a Lightning payment after receiving a `PaymentClaimable` +event. The caller is supposed to pass the `claimable_amount_msat` that the event +reported, and the code was supposed to check that value made sense before actually +claiming the funds (calling `claim_funds` is irreversible-ish — you're telling LDK +"yes, release the preimage, accept this payment"). + +The old check looked like this (simplified): + +```rust +if let Some(invoice_amount_msat) = details.amount_msat { + if claimable_amount_msat < invoice_amount_msat.saturating_sub(skimmed_fee_msat) { + return Err(Error::InvalidAmount); + } +} +``` + +It compared the caller's argument against `details.amount_msat` — the amount stored +on the `PaymentDetails` record. The problem: since the "payment-ID refactor" landed +in v0.8-development, `details.amount_msat` for an inbound payment is *itself* set +from the very same `PaymentClaimable` event's amount. So this check had degenerated +into: + +```rust +claimable_amount_msat < claimable_amount_msat.saturating_sub(skimmed_fee_msat) +``` + +i.e. comparing the argument against essentially itself. Concretely: +- A well-behaved caller who passed the *correct* amount would trivially pass. +- A caller who mixed up arguments — e.g. swapped the amount from a *different*, + concurrently-claimable payment — could also pass, because the check no longer + had any independent value to compare against. There was nothing actually + verifying "does this argument match what LDK told us for *this* payment." + +This matters when an application is juggling multiple concurrent manual claims +(multiple `PaymentClaimable` events outstanding at once) — it's easy to accidentally +pass payment A's amount while claiming payment B, and the old code wouldn't catch it. + +## The fix, piece by piece + +### 1. `src/payment/store.rs` — a new field: `claimable_amount_msat` + +Added a new field to `PaymentKind::Bolt11`: + +```rust +PaymentKind::Bolt11 { + hash: PaymentHash, + preimage: Option, + secret: Option, + counterparty_skimmed_fee_msat: Option, + claimable_amount_msat: Option, // <-- new +} +``` + +This stores **the amount reported by the most recent `PaymentClaimable` event** +for that payment — independent of `amount_msat`, which can be overwritten/derived +elsewhere. Think of it as "the last thing LDK actually told us was claimable," +kept separately so it can later be used as a *ground truth* to check the caller's +argument against. + +Supporting changes: +- `PaymentDetailsUpdate` gained a matching `claimable_amount_msat: Option>` + field (the usual double-`Option` pattern: outer `None` = "don't touch this field", + inner `None` = "set it to None"). +- `UpdatableObject::update()` for `PaymentDetails` now applies this update, with a + `debug_assert!` that it's only ever set for `Bolt11` payments (a spontaneous or + BOLT12 payment shouldn't have one). +- TLV (de)serialization: added as field `8` (a new optional field, so existing + serialized records without it just decode to `None` — backwards compatible). + This is why the `Readable for PaymentDetails` migration path (for pre-v0.8 + serialized records) also explicitly sets `claimable_amount_msat: None`. +- All the other places across the codebase that construct a `PaymentKind::Bolt11` + literal (there are several, in `bolt11.rs`, `event.rs`, tests, etc.) needed a + `claimable_amount_msat: None` (or `Some(...)` in test data) added, since it's a + new required struct field. That's most of the "noise" diff you see repeated in + many places — it's mechanical, not logic-bearing. + +### 2. `src/event.rs` — actually recording the observed amount + +When LDK fires `Event::PaymentClaimable` and ldk-node is about to forward it to +the user as `crate::Event::PaymentClaimable`, it now writes the event's amount +into the new field *before* emitting the event: + +```rust +let claimable_update = PaymentDetailsUpdate { + claimable_amount_msat: Some(Some(amount_msat)), + ..PaymentDetailsUpdate::new(payment_id) +}; +self.payment_store.update(claimable_update).await?; +``` + +So by the time your application code sees the `PaymentClaimable` event and later +calls `claim_for_id`, the payment store already has a durable record of "this is +the amount LDK told us was claimable for this payment." Every other place that +constructs a fresh `PaymentDetails`/update for a Bolt11 payment (initial invoice +creation, spontaneous payment received, etc.) sets `claimable_amount_msat: None`, +since no claimable event has fired yet for those. + +### 3. `src/payment/bolt11.rs` — the actual validation logic + +New free function `validate_claimable_amount`: + +```rust +fn validate_claimable_amount( + claimable_amount_msat: u64, + requested_amount_msat: Option, + observed_claimable_amount_msat: Option, + counterparty_skimmed_fee_msat: u64, +) -> Result<(), Error> { + if let Some(observed_amount_msat) = observed_claimable_amount_msat { + if claimable_amount_msat != observed_amount_msat { + return Err(Error::InvalidAmount); + } + } + + if let Some(requested_amount_msat) = requested_amount_msat { + if claimable_amount_msat < requested_amount_msat.saturating_sub(counterparty_skimmed_fee_msat) { + return Err(Error::InvalidAmount); + } + } + + Ok(()) +} +``` + +Two independent checks, both must pass: + +1. **Equality against the observed event amount** (new, the actual fix): if we + have a recorded `claimable_amount_msat` from a real `PaymentClaimable` event + (i.e. any payment received since v0.8), the caller's argument must match it + *exactly*. This is what catches the argument mix-up bug. Since the event + amount already accounts for LSP fee-skimming etc., an exact match is the + right bar — no under/over slack needed here. + +2. **Historic underpayment guard** (preserved from before, for backwards + compatibility): if `details.amount_msat` (the originally requested invoice + amount) is known — which for *pre-v0.8 serialized* payments is independent + of the event — the claimable amount must be at least the requested amount + minus any JIT-channel LSP skimmed fee. For payments received since v0.8, + `amount_msat` is itself event-derived, so this check degenerates to a no-op + for them (which is fine — check #1 already protects them). + +`claim_for_id` was updated to pull `counterparty_skimmed_fee_msat` and the new +`claimable_amount_msat` out of `details.kind` alongside `hash`, and call +`validate_claimable_amount(...)` instead of the old inline check. + +Doc comments on `claim_for_id` and on the new `PaymentKind::Bolt11::claimable_amount_msat` +field were expanded to explain this two-tier behavior for future readers. + +### 4. Tests + +**Unit tests** in `bolt11.rs` (all against `validate_claimable_amount` directly, +no node/network needed): +- `migrated_record_rejects_underpayment` / `_allows_overpayment` / `_accounts_for_jit_fee` + — cover the pre-v0.8 "no observed amount" path, confirming the historic guard + still works (including the fee-adjusted JIT-channel case). +- `current_record_rejects_mismatched_argument` / `_accepts_the_observed_amount` + / `_accepts_fee_adjusted_jit_amount` — cover the new post-v0.8 "observed amount + present" path, confirming mismatches are now rejected and the correct + (possibly fee-adjusted) amount is accepted. +- `unregistered_record_relies_on_observed_amount_only` — a payment with no prior + registration, showing the observed-amount check alone is sufficient protection + even when `amount_msat` is self-referential. + +**Integration test** in `tests/integration_tests_rust.rs`, inside the LSPS2 +JIT-channel test (`do_lsps2_client_service_integration`): after a JIT payment +becomes claimable, it now asserts that calling `claim_for_id` with (a) the full +invoice amount (forgetting to subtract the LSP's skimmed fee) and (b) an amount +one msat off from the true claimable amount, both return `Err(NodeError::InvalidAmount)` +— i.e., the bug this PR fixes is now actually exercised end-to-end before the +real, successful claim proceeds. + +### 5. `CHANGELOG.md` + +A new "Fixed"-style entry under the unreleased section summarizing the above for +downstream consumers of the crate/changelog. + +## What you need to know / watch out for + +- **This is a backwards-compatible storage change.** The new TLV field (`8`) is + optional on read, so old serialized `PaymentDetails` records deserialize fine + with `claimable_amount_msat: None` — no migration script needed, no breaking + change to on-disk format. +- **Behavior change for API consumers:** if any external caller of `claim_for_id` + was previously (accidentally or not) passing an amount that *didn't* exactly + match the `PaymentClaimable` event's amount but happened to still satisfy the + old (broken) invoice-amount check, that call will now correctly fail with + `Error::InvalidAmount`. This is the intended fix, but it's worth flagging in + the PR description as a behavior change, not just a "pure bug fix with zero + observable difference" — well-behaved callers are unaffected, sloppy ones will + now see errors they should have been seeing all along. +- **`cargo fmt` was not run** and the build/tests were not compiled or executed + for this commit, per your explicit instruction. Before merging, you'll want to + run `cargo fmt --all` and the full test suite (per the repo's own CLAUDE.md + rules) since that wasn't done as part of this commit. +- **AI tooling disclosure** was added to the commit message body (per CLAUDE.md's + requirement to disclose AI tool use in commit messages/PR descriptions), noting + Claude Code was used to help implement and test the change. No co-author trailer + was added, per your request. +- The branch has already been pushed to your fork + (`fix/claim-for-id-amount-validation`); GitHub returned a compare/PR link: + `https://github.com/yahia008/ldk-node/pull/new/fix/claim-for-id-amount-validation` + if you want to open a PR. diff --git a/src/event.rs b/src/event.rs index 728f625ab..867846761 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1000,6 +1000,8 @@ where } else { None }, + // Set below, alongside the `Event::PaymentClaimable` we're about to emit. + claimable_amount_msat: None, }; let payment = PaymentDetails::new( payment_id, @@ -1035,6 +1037,21 @@ where "We would have registered the preimage if we knew" ); + // Record the amount this specific event reports as claimable so that + // `claim_for_id` can later verify the caller echoes it back correctly. + let claimable_update = PaymentDetailsUpdate { + claimable_amount_msat: Some(Some(amount_msat)), + ..PaymentDetailsUpdate::new(payment_id) + }; + if let Err(e) = self.payment_store.update(claimable_update).await { + log_error!( + self.logger, + "Failed to access payment store: {}", + e + ); + return Err(ReplayEvent()); + } + let custom_records = onion_fields .map(|cf| { cf.custom_tlvs().into_iter().map(|tlv| tlv.into()).collect() @@ -1089,6 +1106,7 @@ where } else { None }, + claimable_amount_msat: None, }; let payment = PaymentDetails::new( @@ -1277,6 +1295,7 @@ where preimage: payment_preimage, secret: Some(payment_secret), counterparty_skimmed_fee_msat: None, + claimable_amount_msat: None, }; let update = PaymentDetailsUpdate { preimage: Some(payment_preimage), diff --git a/src/payment/bolt11.rs b/src/payment/bolt11.rs index 5f0a70c2c..56ccf70b0 100644 --- a/src/payment/bolt11.rs +++ b/src/payment/bolt11.rs @@ -165,6 +165,41 @@ impl Bolt11Payment { } } +/// Validates the `claimable_amount_msat` argument given to [`Bolt11Payment::claim_for_id`]. +/// +/// `observed_claimable_amount_msat` is the amount reported by the last `PaymentClaimable` event +/// we stored for this payment. When present (i.e., for any payment received since LDK Node v0.8), +/// the caller must echo it back exactly, which both catches a caller mixing up arguments across +/// concurrent claims and, since the event amount already accounts for the difference, correctly +/// allows overpayments while implicitly enforcing the LSPS2 fee limits already checked before the +/// event was emitted. +/// +/// `requested_amount_msat` is `details.amount_msat`. For payments serialized before v0.8, this +/// still holds the amount originally requested by the invoice, independent of any received event, +/// so we enforce it as a historic underpayment guard (net of `counterparty_skimmed_fee_msat`). +/// Payments received since v0.8 populate `amount_msat` from the event amount itself, making this +/// check a no-op for them; they rely on the equality check above instead. +fn validate_claimable_amount( + claimable_amount_msat: u64, requested_amount_msat: Option, + observed_claimable_amount_msat: Option, counterparty_skimmed_fee_msat: u64, +) -> Result<(), Error> { + if let Some(observed_amount_msat) = observed_claimable_amount_msat { + if claimable_amount_msat != observed_amount_msat { + return Err(Error::InvalidAmount); + } + } + + if let Some(requested_amount_msat) = requested_amount_msat { + if claimable_amount_msat + < requested_amount_msat.saturating_sub(counterparty_skimmed_fee_msat) + { + return Err(Error::InvalidAmount); + } + } + + Ok(()) +} + #[cfg(test)] mod tests { use lightning::util::ser::{Readable, Writeable}; @@ -194,6 +229,164 @@ mod tests { assert_eq!(metadata, decoded); } + + // A migrated (pre-v0.8) record: `amount_msat` holds the originally requested invoice amount, + // and no `PaymentClaimable` event has been observed under the current, event-derived scheme. + #[test] + fn migrated_record_rejects_underpayment() { + let requested_amount_msat = Some(100_000); + let observed_claimable_amount_msat = None; + + assert_eq!( + validate_claimable_amount( + 99_999, + requested_amount_msat, + observed_claimable_amount_msat, + 0 + ), + Err(Error::InvalidAmount) + ); + } + + #[test] + fn migrated_record_allows_overpayment() { + let requested_amount_msat = Some(100_000); + let observed_claimable_amount_msat = None; + + assert_eq!( + validate_claimable_amount( + 150_000, + requested_amount_msat, + observed_claimable_amount_msat, + 0 + ), + Ok(()) + ); + } + + #[test] + fn migrated_record_accounts_for_jit_fee() { + let requested_amount_msat = Some(100_000); + let observed_claimable_amount_msat = None; + let skimmed_fee_msat = 10_000; + + // Exactly matching the fee-adjusted amount succeeds... + assert_eq!( + validate_claimable_amount( + 90_000, + requested_amount_msat, + observed_claimable_amount_msat, + skimmed_fee_msat + ), + Ok(()) + ); + // ...while anything less is still rejected as an underpayment. + assert_eq!( + validate_claimable_amount( + 89_999, + requested_amount_msat, + observed_claimable_amount_msat, + skimmed_fee_msat + ), + Err(Error::InvalidAmount) + ); + } + + // A payment received since v0.8: `amount_msat` was itself derived from the event, so it carries + // no independent expectation; only the observed-amount equality check applies. + #[test] + fn current_record_rejects_mismatched_argument() { + let requested_amount_msat = Some(100_000); + let observed_claimable_amount_msat = Some(100_000); + + assert_eq!( + validate_claimable_amount( + 150_000, + requested_amount_msat, + observed_claimable_amount_msat, + 0 + ), + Err(Error::InvalidAmount) + ); + assert_eq!( + validate_claimable_amount( + 50_000, + requested_amount_msat, + observed_claimable_amount_msat, + 0 + ), + Err(Error::InvalidAmount) + ); + } + + #[test] + fn current_record_accepts_the_observed_amount() { + let requested_amount_msat = Some(100_000); + let observed_claimable_amount_msat = Some(100_000); + + assert_eq!( + validate_claimable_amount( + 100_000, + requested_amount_msat, + observed_claimable_amount_msat, + 0 + ), + Ok(()) + ); + } + + // JIT channel scenario: the LSP skims a fee, so the event (and thus the observed amount) + // reports less than the invoice originally requested. + #[test] + fn current_record_accepts_fee_adjusted_jit_amount() { + let requested_amount_msat = Some(100_000); + let skimmed_fee_msat = 10_000; + let observed_claimable_amount_msat = Some(90_000); + + // The caller passing the true (fee-adjusted) event amount succeeds. + assert_eq!( + validate_claimable_amount( + 90_000, + requested_amount_msat, + observed_claimable_amount_msat, + skimmed_fee_msat + ), + Ok(()) + ); + // Forgetting to account for the fee and passing the full invoice amount is rejected. + assert_eq!( + validate_claimable_amount( + 100_000, + requested_amount_msat, + observed_claimable_amount_msat, + skimmed_fee_msat + ), + Err(Error::InvalidAmount) + ); + } + + // Fully unregistered (never pre-known) manual claims still populate `amount_msat` from the + // event, so it can't be used as an independent expectation, but the observed-amount check still + // protects them. + #[test] + fn unregistered_record_relies_on_observed_amount_only() { + let requested_amount_msat = Some(90_000); // self-referentially derived from the same event + let observed_claimable_amount_msat = Some(90_000); + + assert_eq!( + validate_claimable_amount( + 90_000, + requested_amount_msat, + observed_claimable_amount_msat, + 0 + ), + Ok(()) + ); + assert_eq!( + validate_claimable_amount(1, requested_amount_msat, observed_claimable_amount_msat, 0), + Err(Error::InvalidAmount) + ); + } } impl Bolt11Payment { @@ -255,6 +448,7 @@ impl Bolt11Payment { preimage: None, secret: payment_secret, counterparty_skimmed_fee_msat: None, + claimable_amount_msat: None, }; let payment = PaymentDetails::new( payment_id, @@ -283,6 +477,7 @@ impl Bolt11Payment { preimage: None, secret: payment_secret, counterparty_skimmed_fee_msat: None, + claimable_amount_msat: None, }; let payment = PaymentDetails::new( payment_id, @@ -412,6 +607,15 @@ impl Bolt11Payment { /// This should be called in response to a [`PaymentClaimable`] event as soon as the preimage is /// available. /// + /// `claimable_amount_msat` must equal the `claimable_amount_msat` carried by that very event: + /// we check the two match to guard against a caller mixing up arguments when resolving + /// multiple concurrent manual claims. For payments received before LDK Node v0.8, we + /// additionally require the amount to cover what was originally requested by the invoice, net + /// of any [`counterparty_skimmed_fee_msat`] taken by a JIT-channel-opening LSP, rejecting + /// underpayments while still allowing overpayments; payments received since v0.8 rely solely + /// on the equality check above, as protocol-level and LSPS2 fee-limit checks already guard + /// against underpayment before the event is ever emitted. + /// /// Will check that the payment is known, and that the given preimage and claimable amount /// match our expectations before attempting to claim the payment, and will return an error /// otherwise. @@ -420,6 +624,7 @@ impl Bolt11Payment { /// /// [`PaymentClaimable`]: crate::Event::PaymentClaimable /// [`PaymentReceived`]: crate::Event::PaymentReceived + /// [`counterparty_skimmed_fee_msat`]: crate::payment::PaymentKind::Bolt11::counterparty_skimmed_fee_msat pub fn claim_for_id( &self, payment_id: PaymentId, claimable_amount_msat: u64, preimage: PaymentPreimage, ) -> Result<(), Error> { @@ -433,17 +638,23 @@ impl Bolt11Payment { Error::InvalidPaymentId })?; - let payment_hash = match details.kind { - PaymentKind::Bolt11 { hash, .. } => hash, - _ => { - log_error!( - self.logger, - "Failed to manually claim payment with ID {} of unsupported kind", - payment_id - ); - return Err(Error::InvalidPaymentId); - }, - }; + let (payment_hash, counterparty_skimmed_fee_msat, observed_claimable_amount_msat) = + match details.kind { + PaymentKind::Bolt11 { + hash, + counterparty_skimmed_fee_msat, + claimable_amount_msat, + .. + } => (hash, counterparty_skimmed_fee_msat.unwrap_or(0), claimable_amount_msat), + _ => { + log_error!( + self.logger, + "Failed to manually claim payment with ID {} of unsupported kind", + payment_id + ); + return Err(Error::InvalidPaymentId); + }, + }; let expected_payment_hash = PaymentHash(Sha256::hash(&preimage.0).to_byte_array()); if expected_payment_hash != payment_hash { @@ -455,23 +666,18 @@ impl Bolt11Payment { return Err(Error::InvalidPaymentPreimage); } - // For payments requested via `receive*_via_jit_channel_for_hash()` - // `skimmed_fee_msat` held by LSP must be taken into account. - let skimmed_fee_msat = match details.kind { - PaymentKind::Bolt11 { - counterparty_skimmed_fee_msat: Some(skimmed_fee_msat), .. - } => skimmed_fee_msat, - _ => 0, - }; - if let Some(invoice_amount_msat) = details.amount_msat { - if claimable_amount_msat < invoice_amount_msat.saturating_sub(skimmed_fee_msat) { - log_error!( - self.logger, - "Failed to manually claim payment {} as the claimable amount is less than expected", - payment_id - ); - return Err(Error::InvalidAmount); - } + if let Err(e) = validate_claimable_amount( + claimable_amount_msat, + details.amount_msat, + observed_claimable_amount_msat, + counterparty_skimmed_fee_msat, + ) { + log_error!( + self.logger, + "Failed to manually claim payment {} as the given claimable amount didn't match our expectations", + payment_id + ); + return Err(e); } self.channel_manager.claim_funds(preimage); diff --git a/src/payment/store.rs b/src/payment/store.rs index 41c39045f..1f62281a6 100644 --- a/src/payment/store.rs +++ b/src/payment/store.rs @@ -163,7 +163,13 @@ impl Readable for PaymentDetails { let hash = PaymentHash(id.0); if secret.is_some() { - PaymentKind::Bolt11 { hash, preimage, secret, counterparty_skimmed_fee_msat: None } + PaymentKind::Bolt11 { + hash, + preimage, + secret, + counterparty_skimmed_fee_msat: None, + claimable_amount_msat: None, + } } else { PaymentKind::Spontaneous { hash, preimage } } @@ -319,6 +325,18 @@ impl UpdatableObject for PaymentDetails { } } + if let Some(claimable_amount_msat_opt) = update.claimable_amount_msat { + match self.kind { + PaymentKind::Bolt11 { ref mut claimable_amount_msat, .. } => { + update_if_necessary!(*claimable_amount_msat, claimable_amount_msat_opt); + }, + _ => debug_assert!( + false, + "We should only ever override claimable_amount_msat for BOLT11 payments" + ), + } + } + if let Some(status) = update.status { update_if_necessary!(self.status, status); } @@ -589,6 +607,20 @@ pub enum PaymentKind { /// /// [bLIP-52 / LSPS 2]: https://github.com/lightning/blips/blob/master/blip-0052.md counterparty_skimmed_fee_msat: Option, + /// The amount reported as claimable by the most recent [`PaymentClaimable`] event for this + /// payment, pending resolution via [`claim_for_id`] or [`fail_for_id`]. + /// + /// Used to verify that the `claimable_amount_msat` argument given to [`claim_for_id`] + /// matches what was actually observed, guarding against a caller mixing up arguments when + /// resolving multiple concurrent manual claims. + /// + /// Will always be `None` for payments serialized with LDK Node v0.7.x and earlier, as well + /// as for outbound payments and inbound payments that are claimed automatically. + /// + /// [`PaymentClaimable`]: crate::Event::PaymentClaimable + /// [`claim_for_id`]: crate::payment::Bolt11Payment::claim_for_id + /// [`fail_for_id`]: crate::payment::Bolt11Payment::fail_for_id + claimable_amount_msat: Option, }, /// A [BOLT 12] 'offer' payment, i.e., a payment for an [`Offer`]. /// @@ -663,6 +695,7 @@ impl_writeable_tlv_based_enum!(PaymentKind, (1, counterparty_skimmed_fee_msat, option), (2, preimage, option), (4, secret, option), + (8, claimable_amount_msat, option), }, (4, Bolt11) => { (0, hash, required), @@ -673,6 +706,7 @@ impl_writeable_tlv_based_enum!(PaymentKind, |_| Ok(()), |_: &PaymentKind| None::> )), + (8, claimable_amount_msat, option), }, (6, Bolt12Offer) => { (0, hash, option), @@ -751,6 +785,7 @@ pub(crate) struct PaymentDetailsUpdate { pub amount_msat: Option>, pub fee_paid_msat: Option>, pub counterparty_skimmed_fee_msat: Option>, + pub claimable_amount_msat: Option>, pub direction: Option, pub status: Option, pub confirmation_status: Option, @@ -768,6 +803,7 @@ impl PaymentDetailsUpdate { amount_msat: None, fee_paid_msat: None, counterparty_skimmed_fee_msat: None, + claimable_amount_msat: None, direction: None, status: None, confirmation_status: None, @@ -828,6 +864,11 @@ impl From<&PaymentDetails> for PaymentDetailsUpdate { _ => None, }; + let claimable_amount_msat = match value.kind { + PaymentKind::Bolt11 { claimable_amount_msat, .. } => Some(claimable_amount_msat), + _ => None, + }; + Self { id: value.id, hash: Some(hash), @@ -836,6 +877,7 @@ impl From<&PaymentDetails> for PaymentDetailsUpdate { amount_msat: Some(value.amount_msat), fee_paid_msat: Some(value.fee_paid_msat), counterparty_skimmed_fee_msat, + claimable_amount_msat, direction: Some(value.direction), status: Some(value.status), confirmation_status, @@ -914,11 +956,13 @@ mod tests { preimage: p, secret: s, counterparty_skimmed_fee_msat: c, + claimable_amount_msat: claimable, } => { assert_eq!(hash, h); assert_eq!(preimage, p); assert_eq!(secret, s); assert_eq!(None, c); + assert_eq!(None, claimable); }, _ => { panic!("Unexpected kind!"); @@ -1531,11 +1575,13 @@ mod tests { preimage: p, secret: s, counterparty_skimmed_fee_msat: c, + claimable_amount_msat: claimable, } => { assert_eq!(hash, h); assert_eq!(preimage, p); assert_eq!(secret, s); assert_eq!(counterparty_skimmed_fee_msat, c); + assert_eq!(None, claimable); }, other => panic!("Expected Bolt11, got {:?}", other), } @@ -1584,6 +1630,7 @@ mod bounded_cache_tests { preimage: Some(PaymentPreimage([seed.wrapping_add(1); 32])), secret: Some(PaymentSecret([seed.wrapping_add(2); 32])), counterparty_skimmed_fee_msat: Some(seed as u64 * 7), + claimable_amount_msat: Some(seed as u64 * 11), }, Some(seed as u64 * 1_000), Some(seed as u64 * 3), diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 729b2ed63..bcbff2c28 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -4032,6 +4032,7 @@ mod tests { preimage: None, secret: None, counterparty_skimmed_fee_msat: None, + claimable_amount_msat: None, }, Some(1_000), None, diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 83e80c104..2ce4458ed 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -3901,6 +3901,29 @@ async fn do_lsps2_client_service_integration(client_trusts_lsp: bool) { client_node.payment(&client_payment_id).unwrap().unwrap().amount_msat, Some(jit_amount_msat) ); + + println!("Claiming with a mismatched amount should fail!"); + // Forgetting to account for the LSP's skimmed fee and passing the full invoice amount must be + // rejected rather than silently accepted. + assert_eq!( + client_node.bolt11_payment().claim_for_id( + client_payment_id, + jit_amount_msat, + manual_preimage + ), + Err(NodeError::InvalidAmount) + ); + // An amount unrelated to either the invoice or the actually-claimable amount must also be + // rejected. + assert_eq!( + client_node.bolt11_payment().claim_for_id( + client_payment_id, + claimable_amount_msat + 1, + manual_preimage + ), + Err(NodeError::InvalidAmount) + ); + println!("Claiming payment!"); client_node .bolt11_payment()