fix(payment): skip store insertion on routing failures to allow retry#906
Conversation
|
I've assigned @tnull as a reviewer! |
alexgrad42
force-pushed
the
feature/no_store_update_for_fail_finding_route
branch
from
09c0a1f to
c9d3974
Compare
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
benthecarman
left a comment
benthecarman
left a comment
There was a problem hiding this comment.
I think we already handle this, the test passes for me without your changes or at least the test doesn't actually cover the issue you're trying to fix
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
Adjust the error handling for outbound payments so that when a payment fails at the pathfinding stage (e.g., `RetryableSendFailure::RouteNotFound`), we bypass inserting a `PaymentStatus::Failed` entry into the database. Fix lightningdevkit#903
alexgrad42
force-pushed
the
feature/no_store_update_for_fail_finding_route
branch
from
c9d3974 to
894ceb6
Compare
Thanks for the review. Although the issue explicitly mentions DuplicatePayment, in the current implementation the send() method actually allows a retry if the payment status in the store is set to Failed. The underlying logic was that a RouteNotFound error would persist a redundant Failed payment record to the database, even though the HTLC was never sent out. I've updated the test to ensure that the payment store remains clean after a routing failure. I'll leave it up to you to decide whether we should keep this exact check or modify it (e.g., make it configurable). |
3 participants
Adjust the error handling for outbound payments so that when a payment fails at the pathfinding stage (e.g.,
RetryableSendFailure::RouteNotFound), we bypass inserting aPaymentStatus::Failedentry into the database.Fix #903