-
Notifications
You must be signed in to change notification settings - Fork 125
Update splice_in/splice_out to use new LDK two-phase funding API
#794
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,14 +25,17 @@ use bitcoin::psbt::{self, Psbt}; | |
| use bitcoin::secp256k1::ecdh::SharedSecret; | ||
| use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature}; | ||
| use bitcoin::secp256k1::{All, PublicKey, Scalar, Secp256k1, SecretKey}; | ||
| use bitcoin::transaction::Sequence; | ||
| use bitcoin::{ | ||
| Address, Amount, FeeRate, OutPoint, ScriptBuf, Transaction, TxOut, Txid, WPubkeyHash, Weight, | ||
| WitnessProgram, WitnessVersion, | ||
| }; | ||
| use lightning::chain::chaininterface::BroadcasterInterface; | ||
| use lightning::chain::channelmonitor::ANTI_REORG_DELAY; | ||
| use lightning::chain::{BestBlock, Listen}; | ||
| use lightning::events::bump_transaction::{Input, Utxo, WalletSource}; | ||
| use lightning::chain::{BestBlock, ClaimId, Listen}; | ||
| use lightning::events::bump_transaction::{ | ||
| CoinSelection, CoinSelectionSource, Input, Utxo, WalletSource, | ||
| }; | ||
| use lightning::ln::channelmanager::PaymentId; | ||
| use lightning::ln::funding::FundingTxInput; | ||
| use lightning::ln::inbound_payment::ExpandedKey; | ||
|
|
@@ -710,8 +713,10 @@ impl Wallet { | |
|
|
||
| pub(crate) fn select_confirmed_utxos( | ||
|
Contributor
Author
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. @tnull I'm wondering if we should implement
Collaborator
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. Yeah, that seems preferable, if we have everything we need in the API by now?
Contributor
Author
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. Right, I think the question may be whether we use our own
Contributor
Author
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. Huh, well the branch shows the fixup. Not sure why the PR hasn't updated.
Collaborator
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.
It would be great to reuse the same codepaths, IMO. That is, if the API now permits for it.
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
Contributor
Author
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. 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 |
||
| &self, must_spend: Vec<Input>, must_pay_to: &[TxOut], fee_rate: FeeRate, | ||
| ) -> Result<Vec<FundingTxInput>, ()> { | ||
| ) -> Result<CoinSelection, ()> { | ||
| let mut locked_wallet = self.inner.lock().unwrap(); | ||
| let mut locked_persister = self.persister.lock().unwrap(); | ||
|
|
||
| debug_assert!(matches!( | ||
| locked_wallet.public_descriptor(KeychainKind::External), | ||
| ExtendedDescriptor::Wpkh(_) | ||
|
|
@@ -740,12 +745,14 @@ impl Wallet { | |
| tx_builder.fee_rate(fee_rate); | ||
| tx_builder.exclude_unconfirmed(); | ||
|
|
||
| tx_builder | ||
| let unsigned_tx = tx_builder | ||
| .finish() | ||
| .map_err(|e| { | ||
| log_error!(self.logger, "Failed to select confirmed UTXOs: {}", e); | ||
| })? | ||
| .unsigned_tx | ||
| .unsigned_tx; | ||
|
|
||
| let confirmed_utxos = unsigned_tx | ||
| .input | ||
| .iter() | ||
| .filter(|txin| must_spend.iter().all(|input| input.outpoint != txin.previous_output)) | ||
|
|
@@ -755,7 +762,29 @@ impl Wallet { | |
| .map(|tx_details| tx_details.tx.deref().clone()) | ||
| .map(|prevtx| FundingTxInput::new_p2wpkh(prevtx, txin.previous_output.vout)) | ||
| }) | ||
| .collect::<Result<Vec<_>, ()>>() | ||
| .collect::<Result<Vec<_>, ()>>()?; | ||
|
|
||
| if unsigned_tx.output.len() > must_pay_to.len() + 1 { | ||
| log_error!( | ||
| self.logger, | ||
| "Unexpected number of change outputs during coin selection: {}", | ||
| unsigned_tx.output.len() - must_pay_to.len(), | ||
| ); | ||
| return Err(()); | ||
| } | ||
|
|
||
| let change_output = unsigned_tx | ||
| .output | ||
| .into_iter() | ||
| .filter(|txout| must_pay_to.iter().all(|output| output != txout)) | ||
| .next(); | ||
|
|
||
| locked_wallet.persist(&mut locked_persister).map_err(|e| { | ||
| log_error!(self.logger, "Failed to persist wallet: {}", e); | ||
| () | ||
| })?; | ||
|
|
||
| Ok(CoinSelection { confirmed_utxos, change_output }) | ||
| } | ||
|
|
||
| fn list_confirmed_utxos_inner(&self) -> Result<Vec<Utxo>, ()> { | ||
|
|
@@ -831,6 +860,7 @@ impl Wallet { | |
| }, | ||
| satisfaction_weight: 1 /* empty script_sig */ * WITNESS_SCALE_FACTOR as u64 + | ||
| 1 /* witness items */ + 1 /* schnorr sig len */ + 64, // schnorr sig | ||
| sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, | ||
| }; | ||
| utxos.push(utxo); | ||
| }, | ||
|
|
@@ -1094,9 +1124,47 @@ impl WalletSource for Wallet { | |
| async move { self.get_change_script_inner() } | ||
| } | ||
|
|
||
| fn get_prevtx<'a>( | ||
tnull marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| &'a self, outpoint: OutPoint, | ||
| ) -> impl Future<Output = Result<Transaction, ()>> + Send + 'a { | ||
| async move { | ||
| let locked_wallet = self.inner.lock().unwrap(); | ||
| locked_wallet | ||
| .tx_details(outpoint.txid) | ||
| .map(|tx_details| tx_details.tx.deref().clone()) | ||
| .ok_or_else(|| { | ||
| log_error!( | ||
| self.logger, | ||
| "Failed to get previous transaction for {}", | ||
| outpoint.txid | ||
| ); | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| fn sign_psbt<'a>( | ||
| &'a self, psbt: Psbt, | ||
| ) -> impl Future<Output = Result<Transaction, ()>> + Send + 'a { | ||
| async move { self.sign_psbt_inner(psbt) } | ||
| } | ||
| } | ||
|
|
||
| // Anchor bumping uses LdkWallet for coin selection, which wraps a WalletSource to implement | ||
| // CoinSelectionSource. Splicing uses this implementation of coin selection instead. | ||
| impl CoinSelectionSource for Wallet { | ||
| fn select_confirmed_utxos<'a>( | ||
| &'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, ()>> + Send + 'a { | ||
| debug_assert!(claim_id.is_none()); | ||
| let fee_rate = FeeRate::from_sat_per_kwu(target_feerate_sat_per_1000_weight as u64); | ||
| async move { self.select_confirmed_utxos(must_spend, must_pay_to, fee_rate) } | ||
| } | ||
|
|
||
| fn sign_psbt<'a>( | ||
| &'a self, psbt: Psbt, | ||
| ) -> impl Future<Output = Result<Transaction, ()>> + Send + 'a { | ||
| debug_assert!(false); | ||
| async move { self.sign_psbt_inner(psbt) } | ||
| } | ||
| } | ||
|
|
||
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.
Uh, let's not panic if someone includes Cashu instructions in the string above. Could just error and skip if this is hit.