[jkczyz]

Track each channel funding — open or splice — in a single payment record that every observer resolves to instead of creating its own.

Several independent writers observe a funding: the record is written at signing (since #1057) or when the funding transaction comes off the broadcast queue, wallet sync sees the transaction in the mempool (possibly broadcast by the counterparty first), and RBF rounds replace the transaction outright. A txid is no identity for a replaceable transaction, so a txid-derived key names whichever round a writer saw first, and a writer that can't find the record creates its own.

What changes

• Funding records get a random PaymentId, generated at creation; txids resolve to the record through its transaction history.
• Recording a splice's first round adopts the PaymentId of the channel's persisted splice intent instead of generating one, so the intent and the funding payment share a record. A round already on a record keeps that record, a failed record excepted; the intent decides the id only for a history no live record tracks yet, so a fee bump this node signs of a round wallet sync recorded first joins sync's record. Nothing persists an intent yet (Track in-flight splices for failure reporting and crash recovery #1080 does), so the intent path is dormant here.
• A record wallet sync created for a round before it was recorded as a candidate — a counterparty round this node did not contribute to, which nothing records until this node signs a later round of the same splice — is folded back into the funding record once the round is a recorded candidate: the duplicate-record window discussed in #1057. The fold runs when this node signs a round, where a failure is only logged, and again when LDK reports the round negotiated, where a failure replays the event. Keep funding payment records accurate #1057 closes the window for rounds this node contributes to by recording them at signing time; the fold stays as the backstop for rounds it never signs.
• Pending payments become an enum so a record can represent a splice with no funding transaction yet. Nothing outside the tests constructs that variant here — the splice-tracking PR next in the stack persists splice intents through it.

Compatibility

• The id scheme can still change: funding records first ship in the upcoming release (v0.7.0 shipped splice_in with no record machinery; a funding transaction was an untyped on-chain payment like any other).
• The enum changes the pending store's serialization format, which is safe because that store has never shipped in a release.

Second in the PR stack replacing #930 for this release, per the discussion there; stacked on #1057; a splice-recovery PR (#1080: persist intents, enrich failure events, release lost input reservations at startup) follows. Automatic retries are deferred to a post-release follow-up.

Developed with assistance from Claude Code.
Track each channel funding — open or splice — in a single payment record that every observer resolves to instead of creating its own.

Several independent writers observe a funding: classification writes the record when the funding transaction comes off the broadcast queue, wallet sync sees the transaction in the mempool (possibly broadcast by the counterparty first), and RBF rounds replace the transaction outright. An id derived from a txid stops matching once a replacement lands, and a writer that can't find the record creates its own.

What changes

• Funding records get a random PaymentId, generated at creation; txids resolve to the record through its candidate history.
• Classification adopts the id a splice was assigned at initiation instead of creating a second record.
• A record wallet sync created while a round's classification was still pending is folded back into the funding record once the round classifies — the duplicate-record window discussed in #1057. The splice-tracking PR (Track in-flight splices for failure reporting and crash recovery #1080) closes that window for rounds this node signs by recording them at signing time; the fold stays as the backstop for rounds we never sign.
• Pending payments become an enum so a record can represent a splice with no funding transaction yet. Nothing constructs that variant here — the splice-tracking PR next in the stack persists splice intents through it.

Compatibility

• The id scheme can still change: funding records first ship in the upcoming release (v0.7.0 shipped splice_in with no record machinery).
• The enum changes the pending store's serialization format, which is safe because that store has never shipped in a release.

Second in the PR stack replacing #930 for this release, per the discussion there; stacked on #1057; a splice-recovery PR (#1080: persist intents, enrich failure events, release lost input reservations at startup) follows. Automatic retries are deferred to a post-release follow-up.

Developed with assistance from Claude Code.

[ldk-reviews-bot]

👋 Thanks for assigning @tnull 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.

[tnull]

Please rebase now that #1057 landed.

[jkczyz]

Rebased

[tnull]

CI is failing. One job is adjusting to the log macro changes, but the CLN test might be more substantial:

thread 'test_basic_channel_cycle' (10011) panicked at tests/common/scenarios/channel.rs:34:27:
03bd5bf03dcdac5871bdf172bb742563551c1505fa145e7459eb644228d11b77d7 timed out waiting for ChannelReady event after 60s
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_basic_channel_cycle

test result: FAILED. 4 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 294.73s

[jkczyz]

CI is failing. One job is adjusting to the log macro changes, but the CLN test might be more substantial:

What is "the log macro changes" referring to?

thread 'test_basic_channel_cycle' (10011) panicked at tests/common/scenarios/channel.rs:34:27:
03bd5bf03dcdac5871bdf172bb742563551c1505fa145e7459eb644228d11b77d7 timed out waiting for ChannelReady event after 60s
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_basic_channel_cycle

test result: FAILED. 4 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 294.73s

Opened #1171 to fix the test flakiness. Re-running them here shows them passing.

[tnull]

What is "the log macro changes" referring to?

The ; changes as in #1123 need to be applied to any new log_ calls (silent rebase conflict, essentially).

[tnull]

We must never loop over all payments, this is prohibitively expensive in general, but in particular for remote stores like Postgres and VSS.

In #1024 we went out of our way to make this impossible to do on DataStore on a type level (list_ methods are only available through separate impl blocks on stores that KeepAllEntries in memory. Maybe it would be good to add a prefactor commit here to introduce the same pattern on PaymentStores, to make sure we cannot ever re-introduce this by accident (that is if after dropping this instance we need listing any payments at all - I doubt it).

[jkczyz]

Claude:

Fixed: the listing is gone along with PaymentStores::payments_page, so the wrapper has no way to list the bounded payment store. A graduated record's rounds resolve through the facts store instead, which the signing now stamps with the payment id for the candidates this node did not contribute to as well. No prefactor needed: the raw store accessors are #[cfg(test)], and the wrapper's only remaining reads of the payment store are by id, so a listing cannot come back without adding a method to it.

[tnull]

Fine by me to add SpliceIntents, but if we do that, shouldn't we also track OpenChannelIntents and CloseChannelIntents? Also, wouldn[t it make more sense to track these in ChannelFactsStore?

[jkczyz]

Fine by me to add SpliceIntents, but if we do that, shouldn't we also track OpenChannelIntents and CloseChannelIntents?

Yeah, that seems reasonable. Should we do it here?

Also, wouldn[t it make more sense to track these in ChannelFactsStore?

Hmmm... IIRC, a little while back @TheBlueMatt was thinking that splices should be thought in terms of payments (e.g., you want to pay an on-chain address but need to splice-out to do so). So we decided to not use a channel-centric storage. But may that changes now that we have ChannelFactsStore? Though note that batching, if supported, wouldn't quite fit there.

Relatedly, I also seem to recall we want to eventually have the user pass the payment id, otherwise, in some situations the status couldn't be checked later. This came up in LDK Server:

• Expose BOLT 12 refunds over gRPC ldk-server#262 (comment)
• Expose BOLT 12 refunds over gRPC ldk-server#262 (comment)

[jkczyz]

FWIW, Claude also mentions this:

The intent is written before any transaction exists and is replaced by a fee bump, re-anchored when the channel's funding moves on, and removed when the splice locks or fails. The facts store is keyed by txid, immutable and only pruned by retention, so it can't hold it. Opens and closes don't have the gap this covers: nothing is reserved in the wallet before FundingGenerationReady, nothing at all for a close, and LDK persists a close's shutdown state. LDK does drop an unfunded open on reload without an event; an open intent would be the way to report that, but that's a separate concern from recovering what a splice reserved.

[tnull]

Yeah, that seems reasonable. Should we do it here?

Yeah, probably better to do it right away, if it's not too much additional code/complexity?

So we decided to not use a channel-centric storage. But may that changes now that we have ChannelFactsStore? Though note that batching, if supported, wouldn't quite fit there.

Alright, can probably keep as is for now then if we think it's a better fit.

[jkczyz]

Yeah, probably better to do it right away, if it's not too much additional code/complexity?

Maybe better to do in #1080 as there wouldn't be much added here. Also, were you thinking using a payment id for opens? See below.

Claude:

Opens: an intent would buy a payment id known at open_channel time and a report of an unfunded open LDK forgets on reload (unfunded channels aren't persisted). Both need the startup reconciliation #1080 adds, so here it would be dead state. I'd rather add it in #1080 or right after, together with returning the payment id from open_channel. Closes: nothing to recover and LDK persists the shutdown state, so I'd leave those out.

[tnull]

Sounds good.

[tnull]

Please squash fixups!

[jkczyz]

Squashed

[tnull]

Claude

2. A contradicting fact for a non-contributed candidate aborts the signing write (low). The new facts loop (:2854-2875) runs after the signed round's facts
   are written and before the pending entry. A resource-limit rejection is fine because it degrades to Incomplete. A contradiction, though, makes
   record_signed_funding return Err. A replay then skips that candidate, because it is attributed by then, so this probably settles. Since these facts are only
   a convenience for later lookups, logging and continuing seems more proportionate.

And:

3. Nits

• DataStore::mutate_async (src/data_store.rs:478) only forwards to mutate_with. Making mutate_with pub(crate) would drop the wrapper.
• find_splice_payment_id takes the first matching intent when there's more than one. A debug_assert!(matches.len() <= 1) would make the doc's "at most one
  in-flight intent" claim checkable once intents are actually created.
• The PR adds 37 new em-dashes in comments and docs. Per your preference, those should be - or rephrased; the existing code in wallet/mod.rs already has
  many.
• The commit trailers are inconsistent: some carry several Co-Authored-By: Claude … lines with different model names. That's worth tidying when the fixups
  are squashed.

I think these could be addressed in #1080 though. Just maybe the inline comment should be addressed here still?

[tnull]

Claude:

1. The merge can delete a Failed record that id resolution deliberately treats as a separate payment (plausible, medium). The filter at
   src/wallet/mod.rs:3268 accepts any txid-keyed record whose transaction is a candidate, with tx_type of None or Funding, whatever its PaymentStatus. But
   resolve_interactive_funding_id (:2652-2664) skips failed records on purpose so that a bump gets its own record.

Scenario:

• Wallet sync creates PaymentId(A) for round A and classifies it as Funding.
• A loses to a conflicting spend, and fail_unconfirmed_funding_payment_locked marks it Failed (that function accepts Funding).
• This node signs bump B with history [A, B]. Resolution skips the failed record and creates a fresh record Y.
• merge_duplicate_candidate_records(Y, [A, B]) sees PaymentId(A) as a duplicate. A is unconfirmed, so nothing is adopted, and the failed record and its entry
  are deleted.

The user's payment history silently loses a failed payment. Either skip duplicate.status == PaymentStatus::Failed in the merge, or decide explicitly that
failed duplicates should go and add a test. None of the ten merge tests uses a failed duplicate. I haven't written a test that reproduces this, so whether a
splice round really gets classified as plain Funding before failing needs confirming.

[jkczyz]

Claude:

Fixed. The merge leaves a duplicate wallet sync has already failed alone, with a test (recording_keeps_a_failed_duplicate). The scenario is narrow: the failed record must be txid-keyed, so the round is one this node contributed nothing to (a round it signed is filed under the id its facts carry from the start). The deletion was real, though: the failure is lost, and the bump has no payment record until the bump itself is seen.

[jkczyz]

Claude

2. A contradicting fact for a non-contributed candidate aborts the signing write (low). The new facts loop (:2854-2875) runs after the signed round's facts
   are written and before the pending entry. A resource-limit rejection is fine because it degrades to Incomplete. A contradiction, though, makes
   record_signed_funding return Err. A replay then skips that candidate, because it is attributed by then, so this probably settles. Since these facts are only
   a convenience for later lookups, logging and continuing seems more proportionate.

🤖 I'd keep the error. The loop writes only local_figures, and only for candidates whose facts carry none, so a contradiction would take another writer landing between that check and the write. The signing path is the only writer of those figures for splice rounds, and it holds the lock. If the error ever fires it means that stopped being true, which is worth failing on rather than logging past. Happy to switch it if you'd rather.

And:

3. Nits

• DataStore::mutate_async (src/data_store.rs:478) only forwards to mutate_with. Making mutate_with pub(crate) would drop the wrapper.
• find_splice_payment_id takes the first matching intent when there's more than one. A debug_assert!(matches.len() <= 1) would make the doc's "at most one
  in-flight intent" claim checkable once intents are actually created.
• The PR adds 37 new em-dashes in comments and docs. Per your preference, those should be - or rephrased; the existing code in wallet/mod.rs already has
  many.
• The commit trailers are inconsistent: some carry several Co-Authored-By: Claude … lines with different model names. That's worth tidying when the fixups
  are squashed.

I think these could be addressed in #1080 though. Just maybe the inline comment should be addressed here still?

🤖 mutate_async is dropped in #1080 (mutate_with is pub(crate) now). #1080's find_splice_payment_id no longer takes the first match: it picks the bare intent carrying the round's contribution, falls back to the channel's only bare intent, and gives up otherwise, so the "at most one" claim is gone along with the need for the assertion. The trailers get tidied at the squash.

[tnull]

Please squash!

[tnull]

Hmm, Claude raised these still:

Correctness

1. An unconfirmed duplicate is deleted even when nothing replaces it (src/wallet/mod.rs:3280-3335). The block that moves a duplicate under id only runs when the duplicate is confirmed. An unconfirmed one falls straight through to remove_pending_payment + remove_payment. Suppose signing wrote only a pending entry under the random id, and wallet sync filed the counterparty's
   round under PaymentId(txid). Then the merge leaves no record of that transaction under any id. If the round is later replaced or evicted, nothing brings it back.
2. Empty figures overwrite real amounts when a confirmed duplicate is moved (:3291). Counterparty rounds now get LocalFundingFigures { amount_msat: None, fee_paid_msat: None, direction }. The code takes any Some(figures), even when both values inside are None. So the moved record loses the amount and fee the wallet had, and takes its direction from the round this node signed
   rather than from the round itself.
3. The same empty figures reach create_payment_from_tx (:3393). For example, a counterparty splice-out that pays our address would show no amount or fee and the wrong direction. Before this PR it showed the wallet's inbound amount.
4. Some contributed rounds never get facts (:2855). The loop skips every round where this node has a stake, assuming its signing already wrote facts. But if signing recorded nothing (e.g. a splice-out to an external address), that round has no facts. If it confirms after the record graduates, it becomes a separate untyped record. That breaks the stated rule that every round
   can be traced back to its record.
5. Merge failures can now block event processing (:2960). record_broadcast_splice_round returns merge errors, so a persistent store error makes SpliceNegotiated replay forever and holds up later events. Signing-time code treats the same merge as optional and only logs failures, which is inconsistent.
6. Conflict tracking is lost in the merge (:3330). The duplicate's pending entry is removed without copying its conflicting_txids to the funding record's entry.

[jkczyz]

Mine's pushing back on all but the first. Responces inlined below, with some caveats bolded. Could you have yours check against them?

Hmm, Claude raised these still:

Correctness

1. An unconfirmed duplicate is deleted even when nothing replaces it (src/wallet/mod.rs:3280-3335). The block that moves a duplicate under id only runs when the duplicate is confirmed. An unconfirmed one falls straight through to remove_pending_payment + remove_payment. Suppose signing wrote only a pending entry under the random id, and wallet sync filed the counterparty's
   round under PaymentId(txid). Then the merge leaves no record of that transaction under any id. If the round is later replaced or evicted, nothing brings it back.

The deletion itself is fine, but checking it found a related bug, now fixed. The dropped duplicate's round already carries facts mapping it to the funding payment, so its next TxConfirmed or TxDropped records it under the payment's id (new test, for the confirmation). The bug: sync normally classifies a counterparty round's record as an InteractiveFunding from its SpliceNegotiated facts, and the merge took only untyped and plain-Funding duplicates, so that record was left in place and stayed Pending forever once a later signing routed the round's events to the funding payment. The latest fixup merges InteractiveFunding duplicates too (new test).

2. Empty figures overwrite real amounts when a confirmed duplicate is moved (:3291). Counterparty rounds now get LocalFundingFigures { amount_msat: None, fee_paid_msat: None, direction }. The code takes any Some(figures), even when both values inside are None. So the moved record loses the amount and fee the wallet had, and takes its direction from the round this node signed
   rather than from the round itself.

Leaving as is. A funding record's figures are what this node put into the round, so a round the counterparty built alone gets None. apply_funding_status_update_locked already records that when such a round confirms into an existing record, on main too; in this PR so does create_payment_from_tx, through the facts the signing now writes for those rounds. The moved line only makes the merge's own record for a confirmed duplicate agree with them. The one case where the wallet's view shows more, a counterparty splice-out paying our address, would be a change to what funding records show. I'd take that up separately if we want it.

3. The same empty figures reach create_payment_from_tx (:3393). For example, a counterparty splice-out that pays our address would show no amount or fee and the wrong direction. Before this PR it showed the wallet's inbound amount.

Leaving as is. The direction is the funding payment's, not the confirming round's: a splice-in is Outbound and a splice-out Inbound, whichever candidate confirms. apply_funding_status_update_locked already treats it that way, copying a confirming candidate's amount and fee but never its direction, on main too. A round the counterparty built alone doesn't carry a direction of its own, so the signed round's is the only one there is to record.

One gap in that rule, already on main: a splice-out that pays an external address is recorded as Inbound for the whole net contribution, which counts the external output and the fee though neither reached the wallet, when the round also moves wallet funds (a round that moves none gets no payment record of its own). I'd fix that in a follow-up rather than this PR, unless you'd prefer it here.

4. Some contributed rounds never get facts (:2855). The loop skips every round where this node has a stake, assuming its signing already wrote facts. But if signing recorded nothing (e.g. a splice-out to an external address), that round has no facts. If it confirms after the record graduates, it becomes a separate untyped record. That breaks the stated rule that every round
   can be traced back to its record.

Leaving as is. The signing skips a round whose net wallet figure is zero. Usually that is a round that doesn't touch the wallet at all, so BDK emits no TxConfirmed or TxUnconfirmed for it before or after graduation; it only appears in another round's TxReplaced conflicts. A zero-net round that does spend wallet inputs is reachable, but for it to confirm after graduation the graduated round has to be reorged out at ANTI_REORG_DELAY depth or more. Main writes facts for the signed round only, so such a round has none there either. Attribution-only facts would also make the close pass write a Failed record for a round that never touched the wallet (closing_drops_an_unobserved_round_that_moved_no_wallet_funds_without_a_record).

5. Merge failures can now block event processing (:2960). record_broadcast_splice_round returns merge errors, so a persistent store error makes SpliceNegotiated replay forever and holds up later events. Signing-time code treats the same merge as optional and only logs failures, which is inconsistent.

Intentional. A replayed FundingTransactionReadyForSigning returns early before the merge, so the SpliceNegotiated run is the one that must complete; a store failure replaying that event is what the handler already did on main for its pending-store write. Only persistence failures can come out of the merge. A round abandoned before its SpliceNegotiated leaves the duplicate as its own record, as every duplicate was on main; the comment now says so.

6. Conflict tracking is lost in the merge (:3330). The duplicate's pending entry is removed without copying its conflicting_txids to the funding record's entry.

Leaving as is. The duplicate's entry can only list rounds of the same splice, which fail_funding_payment_lost_to_conflict filters out, or a foreign spend conflicting with the counterparty round. A close is handled by resolve_closed_channel_splice_rounds without reading conflict lists; any other spend must block the signed round in the graph, and if that round was ever canonical its own TxReplaced lists the conflict on the funding record. The one case I found where that matters is narrow. A wallet-touching non-close spend must have replaced the counterparty round before our later round was signed and must confirm no later than the sync where the signed round is first seen, and an earlier round must already have given the funding payment a record, since a signed round that is never canonical doesn't get one from wallet sync. Even there the funding payment's entry doesn't get those conflicts on main; what main did with the duplicate's list was fail the duplicate's own record, which this PR merges away.

[tnull]

Mine's pushing back on all but the first. Responces inlined below, with some caveats bolded. Could you have yours check against them?

The bot:

Mine agrees with 1, 4, 5 and 6 (on 1, the merge only deletes in narrow cases, since resolve_interactive_funding_id adopts an existing duplicate's id), but not fully with 2/3.

2/3: the facts outlive an abandoned round. record_signed_funding now writes immutable facts for the counterparty's rounds, pointing at our payment id with no figures. On main, only
the signed round gets facts. So:

1. The counterparty splices out to our address. Wallet sync records Inbound 90k sat.
2. We RBF-join and sign, then the counterparty aborts. drop_abandoned_splice_rounds runs.
3. The counterparty's round confirms, and the record's amount and fee become None.

On main it keeps the 90k sat. Nothing of ours was broadcast, so I think this belongs in this PR, not a follow-up. Options: ignore those facts once the signing round is abandoned, or fall
back to the wallet's view when the facts carry no figures.

Repro (fails at 0dbbe1f)

#[tokio::test]
async fn abandoned_signing_keeps_the_wallet_view_of_a_counterparty_round() {
        let store: Arc<DynStore> = Arc::new(DynStoreWrapper(InMemoryStore::new()));
        let wallet = new_test_wallet(Arc::clone(&store), false).await;
        let (counterparty_node_id, channel_id) = test_counterparty_and_channel();
        let (counterparty_tx, _) = splice_out_round(&wallet, 1, 500_000, 300);
        let counterparty_txid = counterparty_tx.compute_txid();
        observe_unconfirmed(&wallet, &counterparty_tx).await;
        let duplicate_id = PaymentId(counterparty_txid.to_byte_array());
        let duplicate =
                wallet.payment_stores.payment_store().get(&duplicate_id).await.unwrap().expect("dup");

        let (tx, contribution) = splice_out_round(&wallet, 2, 499_000, 700);
        let candidates = splice_candidates(
                counterparty_node_id,
                channel_id,
                &[(counterparty_txid, None), (tx.compute_txid(), Some(contribution))],
        );
        wallet.record_signed_funding(&tx, &candidates).await.unwrap();
        wallet.drop_abandoned_splice_rounds(channel_id, &[counterparty_txid]).await.unwrap();

        insert_confirmed_tx(&wallet, counterparty_tx.clone(), 100);
        let event = WalletEvent::TxConfirmed {
                txid: counterparty_txid,
                tx: Arc::new(counterparty_tx),
                block_time: confirmed_block_time(100),
                old_block_time: None,
        };
        wallet.update_payment_store(vec![event]).await.unwrap();
        let payments = wallet.payment_stores.payment_store().list_page(None).await.unwrap().objects;
        assert_eq!(payments[0].amount_msat, duplicate.amount_msat, "wallet's view lost");
}