staticaddr: dyn-conf-tracker

[hieblmi]

This PR surfaces static-address deposits as soon as they appear in the wallet, including mempool deposits, instead of waiting for the old confirmation readiness
threshold.

It also updates the static-address flows around those low-confirmation deposits:

• Track, reconcile, and list unconfirmed static-address deposits.
• Preserve logical deposit identity across RBF/replacement while hiding replaced deposits from normal listings.
• Allow static loop-ins to select low-confirmation deposits, while keeping withdrawals and channel opens restricted to confirmed inputs.
• Add CLI/RPC warnings and metadata so users can see when low-confirmation deposits may delay swap execution.
• Harden static loop-in lifecycle handling around orphan invoices, replaced inputs, failed swaps, and server confirmation-risk decisions.
• Persist confirmation-risk decisions so restart recovery preserves accepted/rejected risk state and payment deadline timing.
• Prevent notification-manager stalls by queueing blocking notification fan-out paths.
• Add regression coverage for deposit tracking, selection, replacement, recovery, notification delivery, and risk-decision persistence.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the reliability and responsiveness of static-address deposits. By allowing immediate visibility of mempool deposits while enforcing strict confirmation requirements for high-risk operations like channel opens and withdrawals, the system balances user experience with security. The changes also include robust handling of deposit replacement, durable persistence of risk decisions for recovery, and comprehensive regression testing to ensure stable behavior across the deposit lifecycle.

Highlights

• Immediate Deposit Visibility: Surfaces static-address deposits as soon as they appear in the wallet, including those currently in the mempool, removing the need to wait for the old confirmation threshold.
• Enhanced Deposit Lifecycle: Introduces robust tracking and reconciliation for unconfirmed deposits, including logic to handle RBF (Replace-by-Fee) scenarios by marking replaced outpoints as 'Replaced' to keep the database clean.
• Confirmation Enforcement: Restricts sensitive operations such as withdrawals and channel opens to confirmed inputs, ensuring that only finalized funds are used for these actions.
• Risk Decision Persistence: Persists confirmation-risk decisions to the database, allowing for consistent swap execution and payment deadline timing even after service restarts.
• User Feedback: Adds CLI and RPC warnings to inform users when low-confirmation deposits might delay swap execution.

New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces support for unconfirmed deposits in static address loop-ins, enabling faster swap initiation while managing confirmation risk. It includes client-side warnings for low-confirmation outputs, server-side persistence of risk decisions, and logic to handle deposits that vanish from the wallet view due to mempool replacement or reorgs. The notification manager was also enhanced to support asynchronous risk acceptance/rejection events. Feedback was provided regarding the duplication of coin selection logic between the CLI warning path and the loop-in manager, recommending a refactor into a shared utility for better maintainability.

[lightninglabs-deploy]

@hieblmi, remember to re-request review from reviewers when ready

[hieblmi]

@claude please review

[claude]

Code Review — PR #1141: staticaddr: dyn-conf-tracker

This PR is a substantial addition: it surfaces mempool deposits immediately (vs. waiting for MinConfs), adds RBF/replacement tracking, persist-and-replay for the server's confirmation-risk decision, notification manager queueing, and regression coverage across all the new flows. The overall design is sound and the test coverage is good. A few correctness issues and edge cases worth addressing before merge:

---

Bug (High) — Payment deadline reset to full duration on legacy MinConfs fallback

File: staticaddr/loopin/actions.go

The new startPaymentDeadline helper has a branching contract on its startedAt parameter: if non-zero it subtracts elapsed time; if zero it uses the full PaymentTimeoutDuration(). The risk-accepted notification path correctly passes the persisted ConfirmationRiskDecisionTime, so the remaining window is computed accurately. But the legacy MinConfs fallback call passes time.Time{}:

// Legacy path
startPaymentDeadline(
    "legacy confirmation fallback",
    time.Time{},  // <-- zero time → full timeout granted from NOW
)

This means a swap initiated 55 seconds ago with a 60-second payment window grants the server a fresh 60-second deadline from the moment deposits confirm, rather than the remaining 5 seconds. The server could use this discrepancy to delay payment well beyond the negotiated window. The fix is to pass f.loopIn.InitiationTime (or whichever timestamp records when the swap was started) so startPaymentDeadline can compute the true remaining budget.

---

Bug (Medium) — unlockDeposits failure at deadline fires silently and is unretried until htlc timeout

File: staticaddr/loopin/actions.go

When deadlineChan fires:

case <-deadlineChan:
    deadlineChan = nil   // won't fire again

    cancelInvoice()

    err = f.unlockDeposits(ctx)
    if err != nil {
        f.Errorf("unable to unlock deposits after payment deadline: %v", err)
        continue   // depositsUnlocked stays false
    }
    depositsUnlocked = true

After this continue, deadlineChan is nil and the only retry path is the !depositsUnlocked guard in the htlc-timeout branch — which only fires when the htlc actually times out on-chain. If the htlc is never published (or both unlock attempts fail), the deposits remain stuck in LoopingIn until daemon restart. Consider logging with a stronger warning and, at minimum, scheduling a bounded retry rather than relying solely on the htlc-timeout coincidence.

---

Bug (Medium) — TransitionDeposits "deposit in final state" error silently swallowed in background paths

Files: staticaddr/withdraw/manager.go, staticaddr/openchannel/manager.go

The PR adds a new final-state guard in TransitionDeposits that returns an error when a deposit is already in a final state (including the new Replaced state). The user-facing gRPC handlers propagate this correctly. However, several background goroutines only log.Errorf it:

• handleConfirmedWithdrawal — withdrawal confirmed but TransitionDeposits fails: deposits silently remain in Withdrawing state.
• openChannelPsbt on confirmation — deposits stay stuck in OpeningChannel instead of advancing to ChannelPublished.
• OpenChannel rollback path — rollback TransitionDeposits failure is only logged; deposits remain stuck in OpeningChannel rather than being returned to Deposited.

In all these cases the error is caused by a deposit being concurrently Replaced (a legitimate race this PR introduces). The user has no visibility that their deposits are stuck. Consider incrementing a metric, surfacing the state via ListStaticAddressDeposits, or adding a recovery path in reconcileDeposits.

---

Maintainability — autoSelectedWarningOutpoints duplicates server-side SelectDeposits logic

File: cmd/loop/staticaddr.go

The PR itself acknowledges this with a // KEEP IN SYNC with staticaddr/loopin.SelectDeopits comment. The duplication is structurally necessary (CLI uses RPC wire types; server uses internal types), so this isn't a blocking issue — but the "KEEP IN SYNC" pattern is fragile. If the server-side tie-break criteria change (e.g. adding a third criterion), the warning could silently diverge, showing spurious or missing confirmations warnings. Consider at least a unit test that runs both selection functions against the same simulated deposit set and asserts they agree on which deposits are selected, or document the exact shared invariants the comment refers to.

---

Good work on the test coverage — the reconciliation, replacement, recovery, and risk-decision-persistence tests are exactly what's needed here. The depositBlocksUntilExpiry helper correctly guards the unconfirmed case (returns int64(expiry) when confirmationHeight <= 0), which avoids the large-negative-number issue the old inline formula would have produced for mempool deposits.

[hieblmi]

@claude please re-review

[claude]

Code Review — PR #1141: staticaddr: dyn-conf-tracker

This PR surfaces static-address deposits from the mempool (without waiting for 6 confirmations), adds RBF/replacement tracking, introduces confirmation-risk notifications from the server, hardens the loop-in lifecycle, and persists risk decisions across restarts. It's a substantial, well-structured change. The findings below are ranked most-severe first.

---

Findings

1. deadlineChan case omits invoiceCanceledForNonPayment = true — deposit re-use window after HTLC confirms
staticaddr/loopin/actions.go — new MonitorInvoiceAndHtlcTxAction event loop

When the payment deadline fires, cancelInvoice() is called but invoiceCanceledForNonPayment is never set to true. That flag drives the htlcConfChan case:

case <-htlcConfChan:
    htlcConfirmed = true
    if invoiceCanceledForNonPayment {          // ← always false after deadlineChan fires
        transitionDepositsToHtlcTimeout(...)  // ← skipped
    }

The consequence: if the server publishes the HTLC on-chain after the deadline has already expired, deposits are left in Deposited state (they were unlocked by unlockDepositsAfterInvoiceCancel) until the HTLC-timeout block fires. In that window the deposit manager may select them for a new loop-in. When the original FSM's blockChan htlc-timeout path eventually fires and calls transitionDepositsToHtlcTimeout, the deposits may now be in LoopingIn state for the new swap — OnSweepingHtlcTimeout is not a valid transition from LoopingIn, producing an FSM error that leaves the original HTLC un-swept.

Fix: add invoiceCanceledForNonPayment = true after cancelInvoice() inside the deadlineChan case.

---

2. Legacy MinConfs fallback grants the server the full payment timeout instead of the remaining time
staticaddr/loopin/actions.go — blockChan case

startPaymentDeadline(
    "legacy confirmation fallback",
    time.Time{},        // ← zero value
)

Inside startPaymentDeadline:

timeout := f.loopIn.PaymentTimeoutDuration()
if !startedAt.IsZero() {
    timeout -= time.Since(startedAt)  // ← never executed for zero value
    ...
}

When deposits confirm 30 minutes after swap initiation with a 60-minute PaymentTimeoutSeconds, the server receives 60 minutes from the legacy-MinConfs block instead of the ~30 remaining minutes the contract specifies. The old code used RemainingPaymentTimeSeconds() which measured from InitiationTime. Fix: pass f.loopIn.InitiationTime as startedAt for the legacy path so elapsed time is always deducted.

---

3. AfterFunc in SubscribeStaticLoopInRiskAccepted/Rejected can evict a notification the FSM never consumed
notifications/manager.go — SubscribeStaticLoopInRiskAccepted / SubscribeStaticLoopInRiskRejected

context.AfterFunc(ctx, func() {
    m.removeSubscriber(NotificationTypeStaticLoopInRiskAccepted, sub)
    m.Lock()
    delete(m.staticLoopInRiskAccepted, swapHash)   // ← unconditional
    m.Unlock()
    close(notifChan)
})

Problematic sequence:

1. FSM subscribes for swap H (no notification yet in cache).
2. FSM's notificationCtx is cancelled (transient error before the for/select reads from riskAcceptedChan).
3. Server sends risk-accepted — handleStaticLoopInRiskDecision persists to DB (best-effort), caches in m.staticLoopInRiskAccepted[H], and calls dropNotification. Because sub.subCtx.Done() is already closed, dropNotification's select takes the ctx.Done() branch — notification is NOT delivered to notifChan.
4. AfterFunc fires → deletes m.staticLoopInRiskAccepted[H], the only in-memory copy.
5. If the DB persist also failed, an FSM restart finds neither a cache entry nor a persisted decision. The payment deadline never starts; the swap is stuck until the legacy MinConfs fallback fires (which itself has the zero-time bug from finding Update README.md #2).

Consider not deleting the cache entry in AfterFunc, or only deleting it after verifying the notification was actually delivered to the subscriber.

---

4. ListUnspentDeposits becomes blind to freshly-arrived mempool deposits
loopd/swapclient_server.go — ListUnspentDeposits

Old code added any confirmed UTXO that had no DB record to isUnspent via the confirmedToCheck map, guaranteeing immediate visibility. New code:

ignoreUnknownOutpoints := true   // was false
deposits, err := s.depositManager.DepositsForOutpoints(...)
for _, d := range deposits {
    if d == nil { continue }      // unknown outpoints silently skipped
    if d.IsInState(deposit.Deposited) {
        isUnspent[d.OutPoint] = struct{}{}
    }
}

A deposit that arrived between two reconcile ticks (up to PollInterval = 10 s) has no DB record yet and returns nil from DepositsForOutpoints, so it is silently excluded from the response. This contradicts the PR's stated goal of surfacing deposits "as soon as they appear in the wallet." Consider keeping the confirmedToCheck fallback, or running a reconcile synchronously before returning results.

---

5. autoSelectedWarningOutpoints re-implements SelectDeposits with a // KEEP IN SYNC comment
cmd/loop/staticaddr.go — autoSelectedWarningOutpoints

The function manually reproduces the sort/filter logic from staticaddr/loopin.SelectDeposits, acknowledged with a comment. Future changes to SelectDeposits (e.g. a new deposit state, a tie-breaking rule) will silently diverge the CLI warning, causing it to warn about different deposits than those the server will actually wait on. The existing TestWarningDepositSelectionMatchesLoopInSelection helps, but only if that test is kept in sync too. Consider factoring the pure selection logic into a shared helper that both the CLI and the FSM import.

---

6. selectedDepositConfirmationHeights allocates and acquires locks on every block after the deadline has started
staticaddr/loopin/actions.go — blockChan case

After deadlineStarted = true, legacyMinConfsReached will immediately return true on every subsequent block, but selectedDepositConfirmationHeights still runs: it allocates two maps and acquires each deposit's lock on every block for the lifetime of the swap. Gate the block-driven check on !deadlineStarted:

if !deadlineStarted && legacyMinConfsReached(...) {
    startPaymentDeadline(...)
}

---

7. riskDecisionTime closure issues two sequential DB reads in the non-persisted path
staticaddr/loopin/actions.go — riskDecisionTime closure

When the decision has not yet been persisted, the function calls GetLoopInByHash → RecordStaticAddressRiskDecision → GetLoopInByHash again only to read back ConfirmationRiskDecisionTime. That timestamp is exactly what was passed to RecordStaticAddressRiskDecision. The second read can be eliminated by having RecordStaticAddressRiskDecision return (or accept-and-echo) the stored timestamp, or by capturing time.Now() before the write and accepting the minor precision difference.

---

8. handleStaticLoopInRiskDecision does an O(N) linear scan over all risk subscribers
notifications/manager.go — handleStaticLoopInRiskDecision

for _, sub := range m.subscribers[notifType] {
    if !hasSwapHash || sub.swapHash == nil ||
        *sub.swapHash != swapHash {
        continue
    }
    notifySubscriber(sub)
}

Risk notifications are swap-scoped and there is at most one subscriber per in-flight swap. With N concurrent loop-ins, every incoming risk notification scans all N entries. The existing staticLoopInRiskAccepted / staticLoopInRiskRejected maps already key by hash; a parallel map[lntypes.Hash]subscriber for the per-swap subscription would provide O(1) dispatch.

---

Overall the approach is sound and the test coverage (reconcile, replacement, notification delivery, risk-decision persistence) is thorough. The two highest-priority fixes are the missing invoiceCanceledForNonPayment = true (finding #1) and the zero startedAt in the legacy fallback (finding #2), both of which affect swap-payment timing correctness.

[hieblmi]

On the two priority items:

1. invoiceCanceledForNonPayment: this is already covered on the current branch via the shared cancellation paths. cancelInvoice() and handleRiskRejected() both set the invoice state to canceled and set invoiceCanceledForNonPayment = true, so a later HTLC confirmation moves the deposits back to SweepHtlcTimeout instead of leaving them reusable.

2. Legacy fallback startedAt: using time.Time{} is intentional. The legacy MinConfs path is the compatibility substitute for a risk-accepted notification; before that point the client is still waiting for the deposits to become acceptable. Charging the payment timeout from swap initiation would make slow-confirming deposits consume the server payment window before the server is expected to pay, which regresses the dyn-conf behavior.

[hieblmi]

@claude please review the latest set of changes. The Replaced deposit state was removed in favor of just-in-time reconciliation before withdraw/swap/openchannel.

[claude]

test comment

[claude]

Finding 1/5: Notification cache deleted on subscriber cancellation

File: notifications/manager.go (SubscribeStaticLoopInRiskAccepted / SubscribeStaticLoopInRiskRejected)

The context.AfterFunc callback unconditionally deletes the in-memory cache entry on subscriber context cancellation. The live-delivery path in handleStaticLoopInRiskDecision sets the cache via cacheDecision and sends to the subscriber via dropNotification, but never clears the cache after delivery. After a live delivery the cache still holds the notification AND it is in the buffered channel.

If the FSM context is cancelled (daemon shutdown, restart) before it drains the channel, AfterFunc fires, deletes the cache, and closes the channel. On restart: if DB persistence succeeded (common path), recovery works fine via the stored ConfirmationRiskDecision field. If DB persistence failed (the swap row did not exist yet when the notification arrived — a real race the code comments explicitly acknowledge), there is no recovery path: no DB record, no in-memory cache, no channel.

Fix: skip the cache deletion in AfterFunc when the subscribe-time cache-hit path already moved the notification to the channel buffer (and cleared the cache there); and consider not deleting the cache on the live-delivery AfterFunc path so restarting subscribers can replay.

[claude]

Finding 2/5: ListUnspentDeposits misses EnsureDepositsFresh call unlike its sibling endpoints

File: loopd/swapclient_server.go (ListUnspentDeposits)

WithdrawDeposits and GetLoopInQuote both call s.depositManager.EnsureDepositsFresh(ctx) before querying deposits. ListUnspentDeposits — the primary endpoint clients use to pick deposits for a loop-in — makes no such call.

The PR also changes ignoreUnknownOutpoints from false to true and removes the fallback that previously added confirmed-but-unknown UTXOs directly to the unspent set. Combined: a deposit that confirmed (or entered the mempool) since the last reconciliation tick may be invisible to ListUnspentDeposits for up to the poll interval. A user who sends a deposit and immediately calls this RPC to select it would see an empty list.

Fix: add a single EnsureDepositsFresh call at the top of ListUnspentDeposits to restore the guaranteed-fresh view.

[claude]

Finding 3/5: GetStaticAddressSummary unconfirmed-value field blind to pre-tracker mempool UTXOs

File: loopd/swapclient_server.go (GetStaticAddressSummary)

The old code queried the live wallet directly (ListUnspent(ctx, 0, MinConfs-1)) for unconfirmed value. The new code derives valueUnconfirmed from the deposit manager tracked state (deposits with ConfirmationHeight <= 0 in state Deposited). A mempool UTXO not yet picked up by reconcileDeposits reports valueUnconfirmed=0 even though the funds are visible in the wallet.

This is a user-visible regression: a user who sends to the static address and immediately calls GetStaticAddressSummary will see zero unconfirmed funds for up to the poll interval.

Fix: call EnsureDepositsFresh before computing the summary, or keep a live wallet fallback for the unconfirmed-value calculation.

[claude]

Finding 4/5: listUnspentWithBestHeight relist path returns a stale height

File: staticaddr/deposit/manager.go (listUnspentWithBestHeight)

On the relist path (when a block arrives mid-listing), afterHeight is captured from the second GetBestBlock call and then a third ListUnspent is issued:

_, afterHeight, err := m.cfg.ChainKit.GetBestBlock(ctx)
if beforeHeight != afterHeight {
utxos, err = m.cfg.AddressManager.ListUnspent(ctx, 0, MaxConfs) // third call
}
return utxos, afterHeight, nil // afterHeight may be stale

If another block arrives between that GetBestBlock and the third ListUnspent, the returned UTXOs reflect height afterHeight+1 but afterHeight is returned as the stable anchor. confirmationHeightForUtxo(afterHeight, utxo) then computes a confirmation height one block too high. The error is transient and corrects on the next updateDepositConfirmations call, but there is no test for this race path.

Fix: issue a final GetBestBlock after the third ListUnspent and use that as the returned height.

[claude]

Finding 5/5: CLI deposit-selection warning logic coupled to server path via comment only

File: cmd/loop/staticaddr.go (autoSelectedWarningOutpoints / filterSwappableWarningDeposits)

The comment '// KEEP IN SYNC with staticaddr/loopin.SelectDeposits' documents that the CLI warning duplicates the server-side sort order and swappability check. TestWarningDepositSelectionMatchesLoopInSelection validates current alignment, which is good. However, a future change to SelectDeposits (sort order, minimum-blocks threshold, etc.) will not automatically break any CLI test, so the warning could silently diverge.

Suggestion: extract the common selection logic into a shared tested helper in the loopin package that both paths call directly.

---

Minor nits:

• cancelSwapInvoice guard changed from SwapInvoice == "" to SwapHash == zero. If a swap has a hash but invoice creation failed, the new code attempts CancelInvoice where the old code would not. The call is best-effort with a timeout context so an lnd error is just logged, but it is a semantic change worth noting.

• The 'if d == nil { continue }' guard in the ListUnspentDeposits loop is now dead code: with ignoreUnknownOutpoints=true, unknown outpoints are silently skipped rather than returning a nil entry, so a nil pointer can no longer reach this guard.

---

Overall the PR is well-engineered with strong test coverage across the new reconciliation, RBF/reorg handling, and risk-decision persistence paths. The findings above are mostly about freshness guarantees on the RPC layer (2, 3) and a subtle notification lifecycle issue (1) that the DB recovery path largely masks in the common case.

[starius]

A brand-new mempool/confirmed deposit visible to lnd but not yet reconciled is silently omitted for up to PollInterval. Since this endpoint already hits lnd and is used by clients to pick deposits, I’d add EnsureDepositsFresh(ctx) before ListUnspentRaw or before DepositsForOutpoints.

[starius]

Here it might also return stale information. Should we call EnsureDepositsFresh here?

[starius]

Here it might also return stale information. Should we call EnsureDepositsFresh here?

[starius]

Should ConfirmationHeight now be treated as mutex-protected deposit state?

This PR makes ConfirmationHeight mutable during reconciliation, but several callers still read it directly from *deposit.Deposit values, for example in staticaddr/loopin, staticaddr/openchannel, and loopd/swapclient_server.go. It is not obvious where the boundary is between deposit-manager-owned mutable state and fields that outside packages may read directly.

Could we either:

1. guard all ConfirmationHeight reads/writes with deposit methods, or
2. return an immutable read-only snapshot type for list/summary/selection paths?

I think the snapshot approach would make the boundary clearer: FSM/transition paths can keep using live *Deposit pointers, while read-only consumers get a copy like DepositSnapshot/DepositView that does not expose the mutex-backed mutable struct.

Either way, it would help to document on Deposit and its mutex which fields are protected by the lock. Since ConfirmationHeight is now updated after creation, direct field access outside the deposit package is easy to misuse.

[starius]

AI found a bug here. Here is the patch: https://gist.github.com/starius/316e644ea82b9a76625ade9512345016

Bug: if the Loop server sent a confrisk accept/reject before the swap FSM was persisted, PersistStaticLoopInRiskDecision could fail. The manager still cached the decision for replay, but if the current subscriber context was canceled before the FSM consumed the buffered notification, the cleanup path deleted the cached decision. After restart/resubscribe, the decision was gone, so the swap could miss the server’s risk response and stall.

Fix: cached risk decisions now track whether they were successfully persisted. The manager only deletes cached decisions on subscriber replay/cancel when persistence succeeded. If persistence failed, the cache entry is retained as the only recovery path until a later subscriber/FSM can receive and persist/consume it.

How this can happen in real loopd:

1. loopd receives a static loop-in risk accepted/rejected notification from the Loop server.
2. The notification manager tries to persist it through RecordStaticAddressRiskDecision.
3. Persistence can fail in real code, for example if the swap row is not visible/stored yet (ErrLoopInNotFound), or because of a transient DB/context error.
4. The manager still forwards/caches the notification for the FSM.
5. If the FSM action context is then canceled before it reads the buffered notification, old code deleted the cached notification even though it had not been persisted.
6. On the next same-process FSM subscription, there was no durable decision and no cached decision, so the client behaved as if the Loop server never sent it.

Real cancellation cases include daemon shutdown, monitor action restart/recovery, or an error path while MonitorInvoiceAndHtlcTxAction is active.

Important nuance: this fix does not make an unpersisted decision survive a hard process death. It fixes the in-process loss where the notification manager already has the decision, persistence failed, and the current subscriber goes away before consuming it.