nik/liana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

spend: let caller update next derivation index

Browse Source 
This is a first step toward removing the database accesses from the
spend PSBT creation helper. It now always take a change address, and
return whether it used it. If it did the caller retrieves the
information about the change address and if necessary bumps the next
derivation index to use.
This commit is contained in:
Antoine Poinsot 2023-11-30 10:14:09 +01:00
parent 9fdb75cf88
commit 22f97e11b7
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
2 changed files with 92 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
src/commands/mod.rs
View File

@ -6,11 +6,11 @@ mod utils;
use crate::{
bitcoin::BitcoinInterface,
database::{Coin, DatabaseInterface},
database::{Coin, DatabaseConnection, DatabaseInterface},
descriptors,
spend::{
check_output_value, create_spend, sanity_check_psbt, unsigned_tx_max_vbytes, CandidateCoin,
SpendCreationError,
CreateSpendRes, SpendCreationError,
},
DaemonControl, VERSION,
};
@ -175,6 +175,28 @@ impl DaemonControl {
addr.require_network(self.config.bitcoin_config.network)
.map_err(CommandError::Address)
}
// If we detect the given address as ours, and it has a higher derivation index than our next
// derivation index, update our next derivation index to the one after the address'.
fn maybe_increase_next_deriv_index(
&self,
db_conn: &mut Box<dyn DatabaseConnection>,
addr: &bitcoin::Address,
) {
if let Some((index, is_change)) = db_conn.derivation_index_by_address(addr) {
if is_change && db_conn.change_index() < index {
let next_index = index
.increment()
.expect("Must not get into hardened territory");
db_conn.set_change_index(next_index, &self.secp);
} else if !is_change && db_conn.receive_index() < index {
let next_index = index
.increment()
.expect("Must not get into hardened territory");
db_conn.set_receive_index(next_index, &self.secp);
}
}
}
}
impl DaemonControl {
@ -340,10 +362,23 @@ impl DaemonControl {
check_output_value(amount)?;
destinations_checked.insert(address, amount);
}
// Check also the change address if one has been given.
// The change address to be used if a change output needs to be created. It may be
// specified by the caller (for instance for the purpose of a sweep, or to avoid us
// creating a new change address on every call).
let change_address = change_address
.map(|addr| self.validate_address(addr))
.transpose()?;
.transpose()?
.unwrap_or_else(|| {
let index = db_conn.change_index();
let desc = self
.config
.main_descriptor
.change_descriptor()
.derive(index, &self.secp);
desc.address(self.config.bitcoin_config.network)
});
// The candidate coins will be either all optional or all mandatory.
// If no coins have been specified, then coins will be selected automatically for
// the spend from a set of optional candidates.
@ -380,20 +415,25 @@ impl DaemonControl {
.collect()
};
Ok(CreateSpendResult {
psbt: create_spend(
&mut db_conn,
&self.config.main_descriptor,
&self.secp,
&self.bitcoin,
self.config.bitcoin_config.network,
&destinations_checked,
&candidate_coins,
feerate_vb,
0, // No min fee required.
change_address,
)?,
})
// Create the PSBT. If there was no error in doing so make sure to update our next
// derivation index in case the change address which we generated or was provided to us was
// for a future derivation index.
let CreateSpendRes { psbt, has_change } = create_spend(
&mut db_conn,
&self.config.main_descriptor,
&self.secp,
&self.bitcoin,
&destinations_checked,
&candidate_coins,
feerate_vb,
0, // No min fee required.
change_address.clone(),
)?;
if has_change {
self.maybe_increase_next_deriv_index(&mut db_conn, &change_address);
}
Ok(CreateSpendResult { psbt })
}
pub fn update_spend(&self, mut psbt: Psbt) -> Result<(), CommandError> {
@ -706,17 +746,19 @@ impl DaemonControl {
let mut replacement_vsize = 0;
for incremental_feerate in 0.. {
let min_fee = descendant_fees.to_sat() + replacement_vsize * incremental_feerate;
let rbf_psbt = match create_spend(
let CreateSpendRes {
psbt: rbf_psbt,
has_change,
} = match create_spend(
&mut db_conn,
&self.config.main_descriptor,
&self.secp,
&self.bitcoin,
self.config.bitcoin_config.network,
&destinations,
&candidate_coins,
feerate_vb,
min_fee,
Some(change_address.clone()),
change_address.clone(),
) {
Ok(psbt) => psbt,
// If we get a coin selection error due to insufficient funds and we want to cancel the
@ -741,6 +783,12 @@ impl DaemonControl {
if rbf_psbt.fee().expect("has already been sanity checked")
>= descendant_fees + bitcoin::Amount::from_sat(replacement_vsize)
{
// In case of success, make sure to update our next derivation index if the change
// address used was from the future.
if has_change {
self.maybe_increase_next_deriv_index(&mut db_conn, &change_address);
}
return Ok(CreateSpendResult { psbt: rbf_psbt });
}
}

72
src/spend.rs
View File

@ -21,7 +21,6 @@ use bdk_coin_select::{
use miniscript::bitcoin::{
self,
absolute::{Height, LockTime},
bip32,
constants::WITNESS_SCALE_FACTOR,
psbt::{Input as PsbtIn, Output as PsbtOut, Psbt},
secp256k1,
@ -393,18 +392,24 @@ pub fn unsigned_tx_max_vbytes(tx: &bitcoin::Transaction, max_sat_weight: u64) ->
.unwrap()
}
pub struct CreateSpendRes {
/// The created PSBT.
pub psbt: Psbt,
/// Whether the created PSBT has a change output.
pub has_change: bool,
}
pub fn create_spend(
db_conn: &mut Box<dyn DatabaseConnection>,
main_descriptor: &descriptors::LianaDescriptor,
secp: &secp256k1::Secp256k1<secp256k1::VerifyOnly>,
bitcoin: &sync::Arc<sync::Mutex<dyn BitcoinInterface>>,
network: bitcoin::Network,
destinations: &HashMap<bitcoin::Address, bitcoin::Amount>,
candidate_coins: &[CandidateCoin],
feerate_vb: u64,
min_fee: u64,
change_address: Option<bitcoin::Address>,
) -> Result<Psbt, SpendCreationError> {
change_addr: bitcoin::Address,
) -> Result<CreateSpendRes, SpendCreationError> {
// This method is a bit convoluted, but it's the nature of creating a Bitcoin transaction
// with a target feerate and outputs. In addition, we support different modes (coin control
// vs automated coin selection, self-spend, sweep, etc..) which make the logic a bit more
@ -466,23 +471,6 @@ pub fn create_spend(
// used as input if necessary.
// We need to get the size of a potential change output to select coins / determine whether
// we should include one, so get the change address and create a dummy txo for this purpose.
// The change address may be externally specified for the purpose of a "sweep": the user
// would set the value of some outputs (or none) and fill-in an address to be used for "all
// the rest". This is the same logic as for a change output, except it's external.
struct InternalChangeInfo {
pub desc: descriptors::DerivedSinglePathLianaDesc,
pub index: bip32::ChildNumber,
}
let (change_addr, int_change_info) = if let Some(addr) = change_address {
(addr, None)
} else {
let index = db_conn.change_index();
let desc = main_descriptor.change_descriptor().derive(index, secp);
(
desc.address(network),
Some(InternalChangeInfo { desc, index }),
)
};
let mut change_txo = bitcoin::TxOut {
value: std::u64::MAX,
script_pubkey: change_addr.script_pubkey(),
@ -521,37 +509,23 @@ pub fn create_spend(
// If necessary, add a change output.
// For a self-send, coin selection will only find solutions with change and will otherwise
// return an error. In any case, the PSBT sanity check will catch a transaction with no outputs.
if change_amount.to_sat() > 0 {
let has_change = change_amount.to_sat() > 0;
if has_change {
check_output_value(change_amount)?;
// If we generated a change address internally, set the BIP32 derivations in the PSBT
// output to tell the signers it's an internal address and make sure to update our next
// change index. Otherwise it's a sweep, so no need to set anything.
// If the change address was set by the caller, check whether it's one of ours. If it
// is, set the BIP32 derivations accordingly. In addition, if it's a change address for
// a later index than we currently have set as next change derivation index, update it.
let bip32_derivation = if let Some(InternalChangeInfo { desc, index }) = int_change_info {
let next_index = index
.increment()
.expect("Must not get into hardened territory");
db_conn.set_change_index(next_index, secp);
desc.bip32_derivations()
} else if let Some((index, is_change)) = db_conn.derivation_index_by_address(&change_addr) {
let desc = if is_change {
if db_conn.change_index() < index {
let next_index = index
.increment()
.expect("Must not get into hardened territory");
db_conn.set_change_index(next_index, secp);
}
main_descriptor.change_descriptor()
// If the change address is ours, tell the signers by setting the BIP32 derivations in the
// PSBT output.
let bip32_derivation =
if let Some((index, is_change)) = db_conn.derivation_index_by_address(&change_addr) {
let desc = if is_change {
main_descriptor.change_descriptor()
} else {
main_descriptor.receive_descriptor()
};
desc.derive(index, secp).bip32_derivations()
} else {
main_descriptor.receive_descriptor()
Default::default()
};
desc.derive(index, secp).bip32_derivations()
} else {
Default::default()
};
// TODO: shuffle once we have Taproot
change_txo.value = change_amount.to_sat();
@ -612,5 +586,5 @@ pub fn create_spend(
sanity_check_psbt(main_descriptor, &psbt)?;
// TODO: maybe check for common standardness rules (max size, ..)?
Ok(psbt)
Ok(CreateSpendRes { psbt, has_change })
}