Had some questions about the validation of claimable_amount_msat in Bolt11Payment while reviewing lightningdevkit/ldk-server#258, below is a codex-generated issue:
The amount check in claim_for_id was originally intentional:
if claimable_amount_msat < invoice_amount_msat.saturating_sub(skimmed_fee_msat)
In PR #308 (#308 (comment)), claimable_amount_msat represented the actual event amount, while the stored amount represented the invoice expectation. This correctly rejected underpayments and allowed overpayments.
However, PR #948 (#948) changed new manual-payment records to derive details.amount_msat from the PaymentClaimable event itself. After subtracting the skimmed fee, the stored value therefore equals the event_s claimable amount, making the check ineffective for normal calls and confusing when the supplied argument differs.
Simply flipping the inequality is insufficient because migrated v0.7 records retain the original semantics.
We should:
- Define whether the argument is the event amount or the caller_s minimum expected amount.
- Preserve requested and actual claimable amounts separately if necessary.
- Add underpayment, overpayment, JIT-fee, mismatched-argument, and migrated-record tests.
Had some questions about the validation of
claimable_amount_msatinBolt11Paymentwhile reviewing lightningdevkit/ldk-server#258, below is a codex-generated issue:The amount check in claim_for_id was originally intentional:
In PR #308 (#308 (comment)), claimable_amount_msat represented the actual event amount, while the stored amount represented the invoice expectation. This correctly rejected underpayments and allowed overpayments.
However, PR #948 (#948) changed new manual-payment records to derive details.amount_msat from the PaymentClaimable event itself. After subtracting the skimmed fee, the stored value therefore equals the event_s claimable amount, making the check ineffective for normal calls and confusing when the supplied argument differs.
Simply flipping the inequality is insufficient because migrated v0.7 records retain the original semantics.
We should: