Skip to content

Do not fail to load ChannelManager when we see claiming payments #3772

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TheBlueMatt
Copy link
Collaborator

When we begin claiming a payment, we move the tracking of it from claimable_payments to claiming_payments. This ensures we only ever have one payment which is in the process of being claimed with a given payment hash at a time and lets us keep track of when all parts have been claimed with their ChannelMonitors.

However, on startup, we check that failing to move a payment from claimable_payments to claiming_payments we check that it is not present in claiming_payments. This is fine if the payment doesn't exist, but if the payment has already started being claimed, this will fail and we'll refuse to deserialize the ChannelManager (with a debug_assert failure in debug mode).

Here we resolve this by checking if a payment is already being claimed before we attempt to initiate claiming and skip the failing check in that case.

@TheBlueMatt TheBlueMatt added this to the 0.1.4 milestone May 12, 2025
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented May 12, 2025

I've assigned @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@TheBlueMatt TheBlueMatt force-pushed the 2025-05-chanman-double-restart-claim-fail branch from bc1a7d0 to d24c7a3 Compare May 12, 2025 02:06
@ldk-reviews-bot ldk-reviews-bot requested a review from wpaulino May 12, 2025 02:14
When we begin claiming a payment, we move the tracking of it from
`claimable_payments` to `claiming_payments`. This ensures we only
ever have one payment which is in the process of being claimed with
a given payment hash at a time and lets us keep track of when all
parts have been claimed with their `ChannelMonitor`s.

However, on startup, we check that failing to move a payment from
`claimable_payments` to `claiming_payments` we check that it is not
present in `claiming_payments`. This is fine if the payment doesn't
exist, but if the payment has already started being claimed, this
will fail and we'll refuse to deserialize the `ChannelManager`
(with a `debug_assert` failure in debug mode).

Here we resolve this by checking if a payment is already being
claimed before we attempt to initiate claiming and skip the failing
check in that case.
@TheBlueMatt TheBlueMatt force-pushed the 2025-05-chanman-double-restart-claim-fail branch from d24c7a3 to 47cccb7 Compare May 12, 2025 02:19
@MaxFangX
Copy link
Contributor

Thanks for implementing this so quickly!

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.

3 participants