Allow for withdrawing on-chain funds #61
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,7 +22,7 @@ use bitcoin::bech32::u5; | |
| use bitcoin::secp256k1::ecdh::SharedSecret; | ||
| use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature}; | ||
| use bitcoin::secp256k1::{PublicKey, Scalar, Secp256k1, Signing}; | ||
| use bitcoin::{Script, Transaction, TxOut, Txid}; | ||
|
|
||
| use std::collections::HashMap; | ||
| use std::sync::{Arc, Condvar, Mutex, RwLock}; | ||
|
|
@@ -187,7 +187,7 @@ where | |
| match locked_wallet.sign(&mut psbt, SignOptions::default()) { | ||
| Ok(finalized) => { | ||
| if !finalized { | ||
| return Err(Error::OnchainTxCreationFailed); | ||
| } | ||
| } | ||
| Err(err) => { | ||
|
|
@@ -208,6 +208,82 @@ where | |
| Ok(self.inner.lock().unwrap().get_balance()?) | ||
| } | ||
|
|
||
| /// Send funds to the given address. | ||
| /// | ||
| /// If `amount_msat_or_drain` is `None` the wallet will be drained, i.e., all available funds will be | ||
| /// spent. | ||
| pub(crate) fn send_to_address( | ||
|
There was a problem hiding this comment. Perhaps it is better to allow pass an optional fee rate? There was a problem hiding this comment.
We likely don't want to expose all the complexity of on-chain fee estimation to the user, but provide sane defaults, otherwise we'll end up replicating more and more of BDK/LDK APIs. We could consider letting them set a specific
Ah, good idea. I now added a |
||
| &self, address: &bitcoin::Address, amount_msat_or_drain: Option<u64>, | ||
| ) -> Result<Txid, Error> { | ||
| let confirmation_target = ConfirmationTarget::Normal; | ||
| let fee_rate = self.estimate_fee_rate(confirmation_target); | ||
|
|
||
| let tx = { | ||
| let locked_wallet = self.inner.lock().unwrap(); | ||
| let mut tx_builder = locked_wallet.build_tx(); | ||
|
|
||
| if let Some(amount_sats) = amount_msat_or_drain { | ||
| tx_builder | ||
| .add_recipient(address.script_pubkey(), amount_sats) | ||
| .fee_rate(fee_rate) | ||
| .enable_rbf(); | ||
| } else { | ||
| tx_builder | ||
| .drain_wallet() | ||
| .drain_to(address.script_pubkey()) | ||
| .fee_rate(fee_rate) | ||
| .enable_rbf(); | ||
| } | ||
|
|
||
| let mut psbt = match tx_builder.finish() { | ||
| Ok((psbt, _)) => { | ||
| log_trace!(self.logger, "Created PSBT: {:?}", psbt); | ||
| psbt | ||
| } | ||
| Err(err) => { | ||
| log_error!(self.logger, "Failed to create transaction: {}", err); | ||
| return Err(err.into()); | ||
| } | ||
| }; | ||
|
|
||
| match locked_wallet.sign(&mut psbt, SignOptions::default()) { | ||
| Ok(finalized) => { | ||
| if !finalized { | ||
| return Err(Error::OnchainTxCreationFailed); | ||
| } | ||
| } | ||
| Err(err) => { | ||
| log_error!(self.logger, "Failed to create transaction: {}", err); | ||
| return Err(err.into()); | ||
| } | ||
| } | ||
| psbt.extract_tx() | ||
| }; | ||
|
|
||
| self.broadcast_transaction(&tx); | ||
|
|
||
| let txid = tx.txid(); | ||
|
|
||
| if let Some(amount_sats) = amount_msat_or_drain { | ||
| log_info!( | ||
| self.logger, | ||
| "Created new transaction {} sending {}sats on-chain to address {}", | ||
| txid, | ||
| amount_sats, | ||
| address | ||
| ); | ||
| } else { | ||
| log_info!( | ||
| self.logger, | ||
| "Created new transaction {} sending all available on-chain funds to address {}", | ||
| txid, | ||
| address | ||
| ); | ||
| } | ||
|
|
||
| Ok(txid) | ||
| } | ||
|
|
||
| fn estimate_fee_rate(&self, confirmation_target: ConfirmationTarget) -> FeeRate { | ||
| let locked_fee_rate_cache = self.fee_rate_cache.read().unwrap(); | ||
|
|
||
|
|
||
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.
Any reason why we don't expose the wallet? Something to do with the runtime?
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.
I think there are a number of reasons. For example I'd really like to keep the main API centered around a single
Nodeobject, at least as long as that is still feasible. We hopefully never get to a place where the API surface is getting unfeasibly large for a single object. If it does this might rather act as a canary that we should refactor it into something more minimal rather than breaking out individual objects.Additionally, it's tbd how and what additional wallet behavior we will/should expose to give users more flexibility. Post-0.1 we might think about offering a wallet interface to allow users to bring their own implementation, which I'm however still not convinced of, as this might as well be quite a footgun. However, if we were to go this route, the exposed API also needs to be separate from the concrete wallet implementation. We'll see..