Only avoid duplicate payments if we didn't fail sending (and expose list_payments)
#96
Conversation
tnull
force-pushed
the
2023-05-list-all-payments
branch
from
b0ff3c8 to
c4f49c4
Compare
|
Rebased after #85 landed. |
tnull
force-pushed
the
2023-05-list-all-payments
branch
from
c4f49c4 to
94e87e5
Compare
| status: PaymentStatus::SendingFailed, | ||
| }; | ||
|
|
||
| self.payment_store.insert(payment)?; |
There was a problem hiding this comment.
So we have any tests that check the payment store? Might be worth adding a test case for duplicate payments.
There was a problem hiding this comment.
Ah, but checking that we fail to send duplicate payments is already covered in channel_full_cycle? Not entirely sure how we'd check the behavior on the payment store alone as the logic for handling duplicates lives (and has to live I think) in send_payment.
There was a problem hiding this comment.
But we don't check that the status remains the same as before in case of a duplicate error, IIUC.
There was a problem hiding this comment.
Ah, fair enough, now asserting that the payment details are unchanged after we receive Error::DuplicatePayment.
tnull
force-pushed
the
2023-05-list-all-payments
branch
from
94e87e5 to
4a9cc01
Compare
|
Rebased on main. |
tnull
force-pushed
the
2023-05-list-all-payments
branch
from
4a9cc01 to
f3e5658
Compare
Previously, we denied retrying any payment for which we already had seen the payment hash. However, this is rather invasive as payments might fail due to local connections issues or unavailability of usable channels, in which case the invoices/payment hashes would be 'burnt' and couldn't get retried. Here, we introduce a new `PaymentStatus::SendingFailed` to differentiate send failures and allow retrying tracked payments if they failed to send immediately. We also rename some error types accordingly for further clarification.
tnull
force-pushed
the
2023-05-list-all-payments
branch
from
f3e5658 to
ecd1b64
Compare
Squashed without further changes. |
Previously, we went from not tracking payments when they immediately failed to send to always tracking them but denying any retries for payments whose payment hash was known to us. However, this is of course much too restrictive as payments might fail due to local connections issues or unavailability of usable channels, in which case the invoices/payment hashes would be 'burnt' and couldn't get retried.
Here, we introduce a new
PaymentStatus::SendingFailedto differentiate send failures and allow retrying tracked payments if they failed to send immediately. We also rename some error types accordingly for further clarification.Additionally, we expose a simple
Node::list_paymentsvariant, mainly to be used in bindings where we can't make use of the_with_filtervariant.