Async Persistence TODOs #3052

While we "shipped" async persistence quite a few releases ago, we've always maintained that it is an alpha feature and has some known (and probably unknown) bugs. This is a tracking issue for both, specifically things I think we should audit as well as things I'm confident are broken, or at least at some point or another took a note of because I was confident it was broken at that time.

Things that are probably broken

• Handle async persist of claims against closed channels (Support async ChannelMonitorUpdate persist for claims against closed channels #3414)
• we should update docs on PaymentPathSuccessful/ProbSuccessful/ProbeFailed/HTLCHandlingFailed to note that it is not a robust event and should only be used to update the scorer, or add an RAA-blocker to the event push in outbound_payments.rs
• claim from previous notes: if we hold monitor updates due to an event or so, route an htlc through a channel, the peer force-closes the channel, then we'll forget that HTLC and never fail it back. (my personal 2023-10-pending-htlcs-lost-on-mon-delay branch may have a test for this). Fixed in Detect and fail-back monitor-blocked un-forwarded HTLCs at close #3989
• If a ChannelMonitor learns a preimage for an outbound HTLC and needs to provide the preimage back to the inbound edge of the forwarded HTLC, the ChannelMonitor cannot be pruned until the inbound edge ChannelMonitor is persisted. See Add infra to block ChannelMonitorUpdates on forwarded claims #2167 (comment) Tracked as ChannelMonitor balances don't reflect preimages for other channels #2153, fixed in Ensure monitors are not archived if they have a preimage we need #3450
• If a payment is claimed then a second payment comes in with the same payment hash, we presumably will do our claim-replay logic on the second payment and then auto-claim it on restart.
• If a payment is claimed, the claim monitor update completes getting the preimage durably on disk, then the PaymentClaimed event is generated, the channel can go all the way through to a new state without the PaymentClaimed event being handled or the ChannelManager being persisted, allowing the preimage to be removed from disk and the PaymentClaimed event to be missed. We need to block the final RAA on the PaymentClaimed event, which we do in Block RAA ChannelMonitorUpdates on PaymentClaimed events #3988.

Things we should audit carefully

• Check that we re-claim payments from other MPP channels if we restart and only managed to claim on one channel, specifically:
  • test various scenarios around force-closes on the first channel we claimed on potentially removing any blocks on the second channel or otherwise ensure we re-claim.
  • Test mpp cases around blocking the paymentclaimable event getting handled before we remove the preimage from disk? (my personal 2023-11-mon-claim-bug branch branch - Block monitor updates to ensure preimages are in each MPP part #3120)
• PaymentClaiable purports to provide a guarantee that the payment is claimable, however if we start persisting the monitor, then the user sees the PaymentClaimable, then we restart without completing monitor or manager persistence, the counterparty could broadcast the previous state and we cant get the payment. (Very) marginally expand test_monitor_update_fail_claim #3126. Similarly, if we do persist the monitor but not the manager, we'll force-close and fail the HTLC back, which is the same end-result (this was never true, we'll obviously use the preimage in a monitor to claim on-chain 🤦).
  • we shouldn't provide the PaymentClaimable event until the last RAA persistence completes. also delays PaymentClaimable which is good for privacy (makes it look more like a forward)
• When doing async persistence for initial channel open, what happens when...
  • the initial persist is pending, never completed, and then we restart?
  • the initial persist is pending, is it possible to get an update before it completes (hitting the InvalidValue return in channelmanager deser)?
• What happens when we get an in-flight update from channelmanager on startup for a closed channel? We handle this now.
• Make sure all ChannelMonitor access in ChannelManager::read is RO, and make the monitors map non-mutable. Done in 6c203d8
• Check that PaymentClaimed is regenerated on restart properly if we have a preimage in at least one (potentially of MPP) channel
• What happens if we send a payment, dont persist the channelmanager at all until the payment has gone all the way through the state machine and been claimed/failed? presumably on claim the event processing blocks the raa but maybe does for paymentfailed too?

Finally, to actually "release" it, we should:

• Add lots of fuzz coverage and CPU time
• Ship a nicer interface to do async persist across LDK Async KV Store Persister #1470
  • Sweeper preparation for async kv store Move persist into async part of the sweeper #3819
  • Async Persister and async sweeper: Async background persistence #3905
  • Provide async FilesystemStore with some form of a queueing mechanism Async FilesystemStore #3931
  • Parallelize persistence in the async background processor. Remove background processor macro and parallelize persistence #3968
  • Full async kv store interface Add support for native async KVStore persist to ChainMonitor #4063 Async KV Store #3778