Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Note that abandon_payment does not persist the state update in docs #1907

Merged

Conversation

TheBlueMatt
Copy link
Collaborator

If a user calls abandon_payment, then restarts without freshly persisting the ChannelManager, the payment will still be pending on restart. This was unclear from the docs (and the docs seemed to imply otherwise). Because this doesn't materially impact the usability of abandon_payment (users shouldn't be called retry_payment on an abandoned one anyway), we simply document it.

Fixes #1804.

@TheBlueMatt TheBlueMatt added this to the 0.0.113 milestone Dec 8, 2022
@codecov-commenter
Copy link

codecov-commenter commented Dec 8, 2022

Codecov Report

Base: 90.60% // Head: 91.00% // Increases project coverage by +0.39% 🎉

Coverage data is based on head (9792ca3) compared to base (d9d4611).
Patch coverage: 50.00% of modified lines in pull request are covered.

❗ Current head 9792ca3 differs from pull request most recent head 1969b48. Consider uploading reports for the commit 1969b48 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1907      +/-   ##
==========================================
+ Coverage   90.60%   91.00%   +0.39%     
==========================================
  Files          91       94       +3     
  Lines       48656    51856    +3200     
  Branches    48656    51856    +3200     
==========================================
+ Hits        44087    47191    +3104     
- Misses       4569     4665      +96     
Impacted Files Coverage Δ
lightning/src/ln/channelmanager.rs 89.89% <50.00%> (+3.20%) ⬆️
lightning/src/ln/functional_tests.rs 96.96% <0.00%> (-0.18%) ⬇️
lightning/src/offers/payer.rs 33.33% <0.00%> (ø)
lightning/src/offers/invoice_request.rs 95.00% <0.00%> (ø)
lightning/src/offers/merkle.rs 100.00% <0.00%> (ø)
lightning/src/ln/features.rs 99.77% <0.00%> (+0.09%) ⬆️
lightning/src/chain/onchaintx.rs 93.77% <0.00%> (+0.82%) ⬆️
lightning/src/offers/parse.rs 94.87% <0.00%> (+1.39%) ⬆️
lightning/src/util/ser.rs 93.38% <0.00%> (+1.44%) ⬆️
lightning/src/offers/offer.rs 94.68% <0.00%> (+2.90%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
@tnull
Copy link
Contributor

tnull commented Dec 9, 2022

LGTM, feel free to squash.

lightning/src/ln/channelmanager.rs Show resolved Hide resolved
lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved
@dunxen
Copy link
Contributor

dunxen commented Dec 12, 2022

Looks good for squash from my side.

If a user calls `abandon_payment`, then restarts without freshly
persisting the `ChannelManager`, the payment will still be pending
on restart. This was unclear from the docs (and the docs seemed to
imply otherwise). Because this doesn't materially impact the
usability of `abandon_payment` (users shouldn't be called
`retry_payment` on an abandoned one anyway), we simply document it.

Fixes lightningdevkit#1804.
@TheBlueMatt
Copy link
Collaborator Author

Squashed without further changes, diff from the other day is:

$ git diff-tree -U1 cf63c49 1969b48b7
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 0aa89b5dc..29304cadb 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -2764,3 +2764,3 @@ impl<M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelManager<M, T, K, F
 	/// After this method returns, no future calls to [`retry_payment`] for the given `payment_id`
-	/// are allowed. If no such event has been generated, an [`Event::PaymentFailed`] event will be
+	/// are allowed. If no [`Event::PaymentFailed`] event had been generated before, one will be
 	/// generated as soon as there are no remaining pending HTLCs for this payment.

@TheBlueMatt TheBlueMatt merged commit b291f4a into lightningdevkit:main Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash Safety of abandon_payment/PaymentFailed
5 participants