Update splice_in/splice_out to use new LDK two-phase funding API #794
Update splice_in/splice_out to use new LDK two-phase funding API #794jkczyz wants to merge 1 commit intolightningdevkit:mainfrom
splice_in/splice_out to use new LDK two-phase funding API #794Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
| Ok(txid) | ||
| } | ||
|
|
||
| pub(crate) fn select_confirmed_utxos( |
There was a problem hiding this comment.
@tnull I'm wondering if we should implement CoinSelectionSource and use this rather than using LdkWallet for UTXO selection. It would prevent double persistence when we need a change output and the need for an explicit persist method after selection.
There was a problem hiding this comment.
Yeah, that seems preferable, if we have everything we need in the API by now?
There was a problem hiding this comment.
Right, I think the question may be whether we use our own CoinSelectionSource for anchor bumping, too. It depends on how UTXO locking will work. LdkWallet manages whether or not to try to double-spend based on the claim id, whereas for splicing we may accidentally double spend one of those if using our own CoinSelectionSource. Pushed a fixup commit implementing CoinSelectionSource for splicing.
There was a problem hiding this comment.
Huh, well the branch shows the fixup. Not sure why the PR hasn't updated.
There was a problem hiding this comment.
Right, I think the question may be whether we use our own CoinSelectionSource for anchor bumping, too.
It would be great to reuse the same codepaths, IMO. That is, if the API now permits for it.
It depends on how UTXO locking will work. LdkWallet manages whether or not to try to double-spend based on the claim id, whereas for splicing we may accidentally double spend one of those if using our own CoinSelectionSource.
Right, we're still waiting on bitcoindevkit/bdk_wallet#259 to ship with BDK 3.0. For now I'm not sure if we'd then need to also do some tracking based on ClaimId.
There was a problem hiding this comment.
Discussed offline. We'll use the current approach and then once BDK ships a new release we can revisit coin selection for anchor bumping.
Alternatives would be to either (1) drop the fixup such that we use LdkWallet for both coin selections or (2) maintain a locked UTXO list for splice coin selection and filter WalletSource::list_confirmed_utxos so that anchor bumping doesn't double spend splicing.
| Ok(txid) | ||
| } | ||
|
|
||
| pub(crate) fn select_confirmed_utxos( |
There was a problem hiding this comment.
Yeah, that seems preferable, if we have everything we need in the API by now?
4c01c07 to
4891c0b
Compare
4891c0b to
7ccaa2a
Compare
Update splice_in/splice_out to use new LDK two-phase funding API The LDK dependency bump introduced a new splicing API that separates negotiation from coin selection, letting LDK handle transaction construction internally rather than requiring manual UTXO selection and change address management. Generated with the assistance of AI (Claude Code). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> f - use select_confirmed_utxos
7ccaa2a to
01102e6
Compare
There was a problem hiding this comment.
Seems this should be based on lightningdevkit/rust-lightning#4382 as it now has landed, as is the next related breaking change?
The LDK dependency bump introduced a new splicing API that separates negotiation from coin selection, letting LDK handle transaction construction internally rather than requiring manual UTXO selection and change address management.