Fix BOLT11 DuplicatePayment triggering on-chain fallback in unified payment - #1038
Fix BOLT11 DuplicatePayment triggering on-chain fallback in unified payment#1038elnafateh wants to merge 2 commits into
Conversation
|
👋 Thanks for assigning @ajaysehwal as a reviewer! |
…ayments Error::DuplicatePayment is now terminal in UnifiedPayment::send, preventing a duplicate Lightning payment from falling back to an on-chain payment.
d2e30c9 to
981bc8a
Compare
| log_error!(self.logger, "Failed to send BOLT11 invoice: DuplicatePayment. This is part of a unified payment. Aborting to avoid duplicate payment."); | ||
| return Err(Error::DuplicatePayment); | ||
| }, | ||
| Err(e) => { |
There was a problem hiding this comment.
It looks like this error can be just a persistence error happening in send/send_internal, with the payment being initiated. Fall back would be a duplicate payment.
There was a problem hiding this comment.
No, persistence failures return a separate PersistenceFailed variant, so aborting on DuplicatePayment here is safe.
There was a problem hiding this comment.
PersistenceFailed is the actual concern. It is caught by the Err(e) branch, and then leads to fallback, even though the payment might already have been initiated?
There was a problem hiding this comment.
I understand your concern here, but that's a real pre-existing bug. It's also orthogonal to this PR, so I'll file it as a second follow-up rather than widen this change, as this PR is scoped to #1033.
4 participants
UnifiedPayment::sendpreviously fell back to the on-chain method after anyBOLT11 error, including
Error::DuplicatePayment. Retrying a unified BIP21payment could pay the recipient twice — once over Lightning, once on-chain.
Error::DuplicatePaymentis now treated as terminal and returned to thecaller immediately, preventing the unsafe fallback.
Adds an integration test covering the retry scenario.
#1033