-
Notifications
You must be signed in to change notification settings - Fork 436
Add FundingNeeded event for splicing
#4290
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
jkczyz
wants to merge
6
commits into
lightningdevkit:main
Choose a base branch
from
jkczyz:2025-12-new-splice-api
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,649
−1,004
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
52f87a8
FIXME: Why does handle_channel_close override the witness?
jkczyz f92406c
Move FundingTxInput::sequence to Utxo
jkczyz e5e9d12
Use FundingTxInput instead of Utxo in CoinSelection
jkczyz 3a7ad0c
Make ClaimId optional in coin selection
jkczyz 61caf7f
Add FundingNeeded event for splicing
jkczyz 42d0251
Use CoinSelection::change_output when splicing
jkczyz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ use crate::ln::chan_utils::{ | |
| HTLC_TIMEOUT_INPUT_KEYED_ANCHOR_WITNESS_WEIGHT, HTLC_TIMEOUT_INPUT_P2A_ANCHOR_WITNESS_WEIGHT, | ||
| P2WSH_TXOUT_WEIGHT, SEGWIT_MARKER_FLAG_WEIGHT, TRUC_CHILD_MAX_WEIGHT, TRUC_MAX_WEIGHT, | ||
| }; | ||
| use crate::ln::funding::FundingTxInput; | ||
| use crate::ln::types::ChannelId; | ||
| use crate::prelude::*; | ||
| use crate::sign::ecdsa::EcdsaChannelSigner; | ||
|
|
@@ -270,6 +271,12 @@ pub struct Input { | |
| pub satisfaction_weight: u64, | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(Input, { | ||
| (1, outpoint, required), | ||
| (3, previous_utxo, required), | ||
| (5, satisfaction_weight, required), | ||
| }); | ||
|
|
||
| /// An unspent transaction output that is available to spend resulting from a successful | ||
| /// [`CoinSelection`] attempt. | ||
| #[derive(Clone, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)] | ||
|
|
@@ -282,12 +289,15 @@ pub struct Utxo { | |
| /// with their lengths included, required to satisfy the output's script. The weight consumed by | ||
| /// the input's `script_sig` must account for [`WITNESS_SCALE_FACTOR`]. | ||
| pub satisfaction_weight: u64, | ||
| /// The sequence number to use in the [`TxIn`] when spending the UTXO. | ||
| pub sequence: Sequence, | ||
| } | ||
|
|
||
| impl_writeable_tlv_based!(Utxo, { | ||
| (1, outpoint, required), | ||
| (3, output, required), | ||
| (5, satisfaction_weight, required), | ||
| (7, sequence, (default_value, Sequence::ENABLE_RBF_NO_LOCKTIME)), | ||
| }); | ||
|
|
||
| impl Utxo { | ||
|
|
@@ -302,6 +312,7 @@ impl Utxo { | |
| outpoint, | ||
| output: TxOut { value, script_pubkey: ScriptBuf::new_p2pkh(pubkey_hash) }, | ||
| satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + 1, /* empty witness */ | ||
| sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -321,6 +332,7 @@ impl Utxo { | |
| }, | ||
| satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 | ||
| + P2WPKH_WITNESS_WEIGHT, | ||
| sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -330,24 +342,38 @@ impl Utxo { | |
| outpoint, | ||
| output: TxOut { value, script_pubkey: ScriptBuf::new_p2wpkh(pubkey_hash) }, | ||
| satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + P2WPKH_WITNESS_WEIGHT, | ||
| sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// An unspent transaction output with at least one confirmation. | ||
| pub type ConfirmedUtxo = FundingTxInput; | ||
|
|
||
| /// The result of a successful coin selection attempt for a transaction requiring additional UTXOs | ||
| /// to cover its fees. | ||
| #[derive(Clone, Debug)] | ||
| pub struct CoinSelection { | ||
| /// The set of UTXOs (with at least 1 confirmation) to spend and use within a transaction | ||
| /// requiring additional fees. | ||
| pub confirmed_utxos: Vec<Utxo>, | ||
| pub confirmed_utxos: Vec<ConfirmedUtxo>, | ||
| /// An additional output tracking whether any change remained after coin selection. This output | ||
| /// should always have a value above dust for its given `script_pubkey`. It should not be | ||
| /// spent until the transaction it belongs to confirms to ensure mempool descendant limits are | ||
| /// not met. This implies no other party should be able to spend it except us. | ||
| pub change_output: Option<TxOut>, | ||
| } | ||
|
|
||
| impl CoinSelection { | ||
| fn satisfaction_weight(&self) -> u64 { | ||
| self.confirmed_utxos.iter().map(|ConfirmedUtxo { utxo, .. }| utxo.satisfaction_weight).sum() | ||
| } | ||
|
|
||
| fn amount(&self) -> Amount { | ||
| self.confirmed_utxos.iter().map(|ConfirmedUtxo { utxo, .. }| utxo.output.value).sum() | ||
| } | ||
| } | ||
|
|
||
| /// An abstraction over a bitcoin wallet that can perform coin selection over a set of UTXOs and can | ||
| /// sign for them. The coin selection method aims to mimic Bitcoin Core's `fundrawtransaction` RPC, | ||
| /// which most wallets should be able to satisfy. Otherwise, consider implementing [`WalletSource`], | ||
|
|
@@ -393,7 +419,7 @@ pub trait CoinSelectionSource { | |
| /// | ||
| /// [`ChannelMonitor::rebroadcast_pending_claims`]: crate::chain::channelmonitor::ChannelMonitor::rebroadcast_pending_claims | ||
| fn select_confirmed_utxos<'a>( | ||
| &'a self, claim_id: ClaimId, must_spend: Vec<Input>, must_pay_to: &'a [TxOut], | ||
| &'a self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &'a [TxOut], | ||
| target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, | ||
| ) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a; | ||
| /// Signs and provides the full witness for all inputs within the transaction known to the | ||
|
|
@@ -418,11 +444,18 @@ pub trait WalletSource { | |
| fn list_confirmed_utxos<'a>( | ||
| &'a self, | ||
| ) -> impl Future<Output = Result<Vec<Utxo>, ()>> + MaybeSend + 'a; | ||
|
|
||
| /// Returns the previous transaction containing the UTXO. | ||
| fn get_prevtx<'a>( | ||
| &'a self, utxo: &Utxo, | ||
| ) -> impl Future<Output = Result<Transaction, ()>> + MaybeSend + 'a; | ||
|
|
||
| /// Returns a script to use for change above dust resulting from a successful coin selection | ||
| /// attempt. | ||
| fn get_change_script<'a>( | ||
| &'a self, | ||
| ) -> impl Future<Output = Result<ScriptBuf, ()>> + MaybeSend + 'a; | ||
|
|
||
| /// Signs and provides the full [`TxIn::script_sig`] and [`TxIn::witness`] for all inputs within | ||
| /// the transaction known to the wallet (i.e., any provided via | ||
| /// [`WalletSource::list_confirmed_utxos`]). | ||
|
|
@@ -452,7 +485,7 @@ where | |
| // TODO: Do we care about cleaning this up once the UTXOs have a confirmed spend? We can do so | ||
| // by checking whether any UTXOs that exist in the map are no longer returned in | ||
| // `list_confirmed_utxos`. | ||
| locked_utxos: Mutex<HashMap<OutPoint, ClaimId>>, | ||
| locked_utxos: Mutex<HashMap<OutPoint, Option<ClaimId>>>, | ||
| } | ||
|
|
||
| impl<W: Deref + MaybeSync + MaybeSend, L: Deref + MaybeSync + MaybeSend> Wallet<W, L> | ||
|
|
@@ -475,7 +508,7 @@ where | |
| /// least 1 satoshi at the current feerate, otherwise, we'll only attempt to spend those which | ||
| /// contribute at least twice their fee. | ||
| async fn select_confirmed_utxos_internal( | ||
| &self, utxos: &[Utxo], claim_id: ClaimId, force_conflicting_utxo_spend: bool, | ||
| &self, utxos: &[Utxo], claim_id: Option<ClaimId>, force_conflicting_utxo_spend: bool, | ||
| tolerate_high_network_feerates: bool, target_feerate_sat_per_1000_weight: u32, | ||
| preexisting_tx_weight: u64, input_amount_sat: Amount, target_amount_sat: Amount, | ||
| max_tx_weight: u64, | ||
|
|
@@ -499,7 +532,9 @@ where | |
| .iter() | ||
| .filter_map(|utxo| { | ||
| if let Some(utxo_claim_id) = locked_utxos.get(&utxo.outpoint) { | ||
| if *utxo_claim_id != claim_id && !force_conflicting_utxo_spend { | ||
| if (utxo_claim_id.is_none() || *utxo_claim_id != claim_id) | ||
| && !force_conflicting_utxo_spend | ||
| { | ||
| log_trace!( | ||
| self.logger, | ||
| "Skipping UTXO {} to prevent conflicting spend", | ||
|
|
@@ -610,10 +645,13 @@ where | |
| Some(TxOut { script_pubkey: change_script, value: change_output_amount }) | ||
| }; | ||
|
|
||
| Ok(CoinSelection { | ||
| confirmed_utxos: selected_utxos.into_iter().map(|(utxo, _)| utxo).collect(), | ||
| change_output, | ||
| }) | ||
| let mut confirmed_utxos = Vec::with_capacity(selected_utxos.len()); | ||
| for (utxo, _) in selected_utxos { | ||
| let prevtx = self.source.get_prevtx(&utxo).await?; | ||
| confirmed_utxos.push(ConfirmedUtxo { utxo, prevtx }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth sanity checking the transaction actually matches the utxo? |
||
| } | ||
|
|
||
| Ok(CoinSelection { confirmed_utxos, change_output }) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -624,7 +662,7 @@ where | |
| L::Target: Logger + MaybeSend + MaybeSync, | ||
| { | ||
| fn select_confirmed_utxos<'a>( | ||
| &'a self, claim_id: ClaimId, must_spend: Vec<Input>, must_pay_to: &'a [TxOut], | ||
| &'a self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &'a [TxOut], | ||
| target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, | ||
| ) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a { | ||
| async move { | ||
|
|
@@ -724,11 +762,11 @@ where | |
|
|
||
| /// Updates a transaction with the result of a successful coin selection attempt. | ||
| fn process_coin_selection(&self, tx: &mut Transaction, coin_selection: &CoinSelection) { | ||
| for utxo in coin_selection.confirmed_utxos.iter() { | ||
| for ConfirmedUtxo { utxo, .. } in coin_selection.confirmed_utxos.iter() { | ||
| tx.input.push(TxIn { | ||
| previous_output: utxo.outpoint, | ||
| script_sig: ScriptBuf::new(), | ||
| sequence: Sequence::ZERO, | ||
| sequence: utxo.sequence, | ||
| witness: Witness::new(), | ||
| }); | ||
| } | ||
|
|
@@ -818,7 +856,7 @@ where | |
| let coin_selection: CoinSelection = self | ||
| .utxo_source | ||
| .select_confirmed_utxos( | ||
| claim_id, | ||
| Some(claim_id), | ||
| must_spend, | ||
| &[], | ||
| package_target_feerate_sat_per_1000_weight, | ||
|
|
@@ -846,12 +884,10 @@ where | |
| output: vec![], | ||
| }; | ||
|
|
||
| let input_satisfaction_weight: u64 = | ||
| coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum(); | ||
| let input_satisfaction_weight = coin_selection.satisfaction_weight(); | ||
| let total_satisfaction_weight = | ||
| anchor_input_witness_weight + EMPTY_SCRIPT_SIG_WEIGHT + input_satisfaction_weight; | ||
| let total_input_amount = must_spend_amount | ||
| + coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum(); | ||
| let total_input_amount = must_spend_amount + coin_selection.amount(); | ||
|
|
||
| self.process_coin_selection(&mut anchor_tx, &coin_selection); | ||
| let anchor_txid = anchor_tx.compute_txid(); | ||
|
|
@@ -866,10 +902,10 @@ where | |
| let index = idx + 1; | ||
| debug_assert_eq!( | ||
| anchor_psbt.unsigned_tx.input[index].previous_output, | ||
| utxo.outpoint | ||
| utxo.outpoint() | ||
| ); | ||
| if utxo.output.script_pubkey.is_witness_program() { | ||
| anchor_psbt.inputs[index].witness_utxo = Some(utxo.output); | ||
| if utxo.output().script_pubkey.is_witness_program() { | ||
| anchor_psbt.inputs[index].witness_utxo = Some(utxo.into_output()); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1076,7 +1112,7 @@ where | |
| let coin_selection: CoinSelection = match self | ||
| .utxo_source | ||
| .select_confirmed_utxos( | ||
| utxo_id, | ||
| Some(utxo_id), | ||
| must_spend, | ||
| &htlc_tx.output, | ||
| target_feerate_sat_per_1000_weight, | ||
|
|
@@ -1101,13 +1137,11 @@ where | |
| utxo_id = claim_id.step_with_bytes(&broadcasted_htlcs.to_be_bytes()); | ||
|
|
||
| #[cfg(debug_assertions)] | ||
| let input_satisfaction_weight: u64 = | ||
| coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum(); | ||
| let input_satisfaction_weight = coin_selection.satisfaction_weight(); | ||
| #[cfg(debug_assertions)] | ||
| let total_satisfaction_weight = must_spend_satisfaction_weight + input_satisfaction_weight; | ||
| #[cfg(debug_assertions)] | ||
| let input_value: u64 = | ||
| coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value.to_sat()).sum(); | ||
| let input_value = coin_selection.amount().to_sat(); | ||
| #[cfg(debug_assertions)] | ||
| let total_input_amount = must_spend_amount + input_value; | ||
|
|
||
|
|
@@ -1128,9 +1162,12 @@ where | |
| for (idx, utxo) in coin_selection.confirmed_utxos.into_iter().enumerate() { | ||
| // offset to skip the htlc inputs | ||
| let index = idx + selected_htlcs.len(); | ||
| debug_assert_eq!(htlc_psbt.unsigned_tx.input[index].previous_output, utxo.outpoint); | ||
| if utxo.output.script_pubkey.is_witness_program() { | ||
| htlc_psbt.inputs[index].witness_utxo = Some(utxo.output); | ||
| debug_assert_eq!( | ||
| htlc_psbt.unsigned_tx.input[index].previous_output, | ||
| utxo.outpoint() | ||
| ); | ||
| if utxo.output().script_pubkey.is_witness_program() { | ||
| htlc_psbt.inputs[index].witness_utxo = Some(utxo.into_output()); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1274,17 +1311,16 @@ mod tests { | |
| use crate::util::ser::Readable; | ||
| use crate::util::test_utils::{TestBroadcaster, TestLogger}; | ||
|
|
||
| use bitcoin::hashes::Hash; | ||
| use bitcoin::hex::FromHex; | ||
| use bitcoin::{Network, ScriptBuf, Transaction, Txid}; | ||
| use bitcoin::{Network, ScriptBuf, Transaction}; | ||
|
|
||
| struct TestCoinSelectionSource { | ||
| // (commitment + anchor value, commitment + input weight, target feerate, result) | ||
| expected_selects: Mutex<Vec<(u64, u64, u32, CoinSelection)>>, | ||
| } | ||
| impl CoinSelectionSourceSync for TestCoinSelectionSource { | ||
| fn select_confirmed_utxos( | ||
| &self, _claim_id: ClaimId, must_spend: Vec<Input>, _must_pay_to: &[TxOut], | ||
| &self, _claim_id: Option<ClaimId>, must_spend: Vec<Input>, _must_pay_to: &[TxOut], | ||
| target_feerate_sat_per_1000_weight: u32, _max_tx_weight: u64, | ||
| ) -> Result<CoinSelection, ()> { | ||
| let mut expected_selects = self.expected_selects.lock().unwrap(); | ||
|
|
@@ -1297,12 +1333,10 @@ mod tests { | |
| } | ||
| fn sign_psbt(&self, psbt: Psbt) -> Result<Transaction, ()> { | ||
| let mut tx = psbt.unsigned_tx; | ||
| for input in tx.input.iter_mut() { | ||
| if input.previous_output.txid != Txid::from_byte_array([44; 32]) { | ||
| // Channel output, add a realistic size witness to make the assertions happy | ||
| input.witness = Witness::from_slice(&[vec![42; 162]]); | ||
| } | ||
| } | ||
| // Channel output, add a realistic size witness to make the assertions happy | ||
| // | ||
| // FIXME: This doesn't seem to be needed since handle_channel_close overrides it | ||
| tx.input.first_mut().unwrap().witness = Witness::from_slice(&[vec![42; 162]]); | ||
| Ok(tx) | ||
| } | ||
| } | ||
|
|
@@ -1339,6 +1373,13 @@ mod tests { | |
| .weight() | ||
| .to_wu(); | ||
|
|
||
| let prevtx = Transaction { | ||
| version: Version::TWO, | ||
| lock_time: LockTime::ZERO, | ||
| input: vec![], | ||
| output: vec![TxOut { value: Amount::from_sat(200), script_pubkey: ScriptBuf::new() }], | ||
| }; | ||
|
|
||
| let broadcaster = TestBroadcaster::new(Network::Testnet); | ||
| let source = TestCoinSelectionSource { | ||
| expected_selects: Mutex::new(vec![ | ||
|
|
@@ -1353,13 +1394,14 @@ mod tests { | |
| commitment_and_anchor_fee, | ||
| 868, | ||
| CoinSelection { | ||
| confirmed_utxos: vec![Utxo { | ||
| outpoint: OutPoint { txid: Txid::from_byte_array([44; 32]), vout: 0 }, | ||
| output: TxOut { | ||
| value: Amount::from_sat(200), | ||
| script_pubkey: ScriptBuf::new(), | ||
| confirmed_utxos: vec![ConfirmedUtxo { | ||
| utxo: Utxo { | ||
| outpoint: OutPoint { txid: prevtx.compute_txid(), vout: 0 }, | ||
| output: prevtx.output[0].clone(), | ||
| satisfaction_weight: 5, // Just the script_sig and witness lengths | ||
| sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, | ||
| }, | ||
| satisfaction_weight: 5, // Just the script_sig and witness lengths | ||
| prevtx, | ||
| }], | ||
| change_output: None, | ||
| }, | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a fan that we're complicating this already complex argument even more, but this really needs docs explaining the desired behavior when
None