From 7c80aec4542af285b4f281622c7d0dbe6d39c934 Mon Sep 17 00:00:00 2001 From: Alekos Filini Date: Tue, 10 Nov 2020 15:06:14 +0100 Subject: [PATCH] [wallet] Take both spending policies into account in create_tx This allows specifying different "policy paths" for the internal and external descriptors, and adds additional checks to make sure they are compatibile (i.e. the timelocks are expressed in the same unit). It's still suboptimal, since the `n_sequence`s are per-input and not per-transaction, so it should be possibile to spend different inputs with different, otherwise incompatible, `CSV` timelocks, but that requires a larger refactor that can be done in a future patch. This commit also tries to clarify how the "policy path" should be used by adding a fairly detailed example to the docs. --- src/descriptor/policy.rs | 2 +- src/error.rs | 2 +- src/wallet/mod.rs | 118 ++++++++++++++++++++++++++++++++++++--- src/wallet/tx_builder.rs | 75 ++++++++++++++++++++++--- 4 files changed, 181 insertions(+), 16 deletions(-) diff --git a/src/descriptor/policy.rs b/src/descriptor/policy.rs index 961b1ba7..fe0056cd 100644 --- a/src/descriptor/policy.rs +++ b/src/descriptor/policy.rs @@ -433,7 +433,7 @@ impl Condition { } } - fn merge(mut self, other: &Condition) -> Result { + pub(crate) fn merge(mut self, other: &Condition) -> Result { match (self.csv, other.csv) { (Some(a), Some(b)) => self.csv = Some(Self::merge_timelock(a, b)?), (None, any) => self.csv = any, diff --git a/src/error.rs b/src/error.rs index f4ecd65d..925217e9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -60,7 +60,7 @@ pub enum Error { ChecksumMismatch, DifferentDescriptorStructure, - SpendingPolicyRequired, + SpendingPolicyRequired(crate::types::ScriptType), InvalidPolicyPathError(crate::descriptor::policy::PolicyError), Signer(crate::wallet::signer::SignerError), diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 05e3787e..79ce7789 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -244,17 +244,62 @@ where &self, builder: TxBuilder, ) -> Result<(PSBT, TransactionDetails), Error> { - // TODO: fetch both internal and external policies - let policy = self + let external_policy = self .descriptor .extract_policy(Arc::clone(&self.signers))? .unwrap(); - if policy.requires_path() && builder.policy_path.is_none() { - return Err(Error::SpendingPolicyRequired); + let internal_policy = self + .change_descriptor + .as_ref() + .map(|desc| { + Ok::<_, Error>( + desc.extract_policy(Arc::clone(&self.change_signers))? + .unwrap(), + ) + }) + .transpose()?; + + // The policy allows spending external outputs, but it requires a policy path that hasn't been + // provided + if builder.change_policy != tx_builder::ChangeSpendPolicy::OnlyChange + && external_policy.requires_path() + && builder.external_policy_path.is_none() + { + return Err(Error::SpendingPolicyRequired(ScriptType::External)); + }; + // Same for the internal_policy path, if present + if let Some(internal_policy) = &internal_policy { + if builder.change_policy != tx_builder::ChangeSpendPolicy::ChangeForbidden + && internal_policy.requires_path() + && builder.internal_policy_path.is_none() + { + return Err(Error::SpendingPolicyRequired(ScriptType::Internal)); + }; } - let requirements = - policy.get_condition(builder.policy_path.as_ref().unwrap_or(&BTreeMap::new()))?; - debug!("requirements: {:?}", requirements); + + let external_requirements = external_policy.get_condition( + builder + .external_policy_path + .as_ref() + .unwrap_or(&BTreeMap::new()), + )?; + let internal_requirements = internal_policy + .map(|policy| { + Ok::<_, Error>( + policy.get_condition( + builder + .internal_policy_path + .as_ref() + .unwrap_or(&BTreeMap::new()), + )?, + ) + }) + .transpose()?; + + let requirements = external_requirements + .clone() + .merge(&internal_requirements.unwrap_or_default())?; + debug!("Policy requirements: {:?}", requirements); let version = match builder.version { Some(tx_builder::Version(0)) => { @@ -1393,6 +1438,11 @@ mod test { "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),older(6)))" } + pub(crate) fn get_test_a_or_b_plus_csv() -> &'static str { + // or(pk(Alice),and(pk(Bob),older(144))) + "wsh(or_d(pk(cRjo6jqfVNP33HhSS76UhXETZsGTZYx8FMFvR9kpbtCSV1PmdZdu),and_v(v:pk(cMnkdebixpXMPfkcNEjjGin7s94hiehAH4mLbYkZoh9KSiNNmqC8),older(144))))" + } + pub(crate) fn get_test_single_sig_cltv() -> &'static str { // and(pk(Alice),after(100000)) "wsh(and_v(v:pk(cVpPVruEDdmutPzisEsYvtST1usBR3ntr8pXSyt6D2YYqXRyPcFW),after(100000)))" @@ -2134,6 +2184,60 @@ mod test { .unwrap(); } + #[test] + #[should_panic(expected = "SpendingPolicyRequired(External)")] + fn test_create_tx_policy_path_required() { + let (wallet, _, _) = get_funded_wallet(get_test_a_or_b_plus_csv()); + + let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); + wallet + .create_tx(TxBuilder::with_recipients(vec![( + addr.script_pubkey(), + 30_000, + )])) + .unwrap(); + } + + #[test] + fn test_create_tx_policy_path_no_csv() { + let (wallet, _, _) = get_funded_wallet(get_test_a_or_b_plus_csv()); + + let external_policy = wallet.policies(ScriptType::External).unwrap().unwrap(); + let root_id = external_policy.id; + // child #0 is just the key "A" + let path = vec![(root_id, vec![0])].into_iter().collect(); + + let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); + let (psbt, _) = wallet + .create_tx( + TxBuilder::with_recipients(vec![(addr.script_pubkey(), 30_000)]) + .policy_path(path, ScriptType::External), + ) + .unwrap(); + + assert_eq!(psbt.global.unsigned_tx.input[0].sequence, 0xFFFFFFFF); + } + + #[test] + fn test_create_tx_policy_path_use_csv() { + let (wallet, _, _) = get_funded_wallet(get_test_a_or_b_plus_csv()); + + let external_policy = wallet.policies(ScriptType::External).unwrap().unwrap(); + let root_id = external_policy.id; + // child #1 is or(pk(B),older(144)) + let path = vec![(root_id, vec![1])].into_iter().collect(); + + let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap(); + let (psbt, _) = wallet + .create_tx( + TxBuilder::with_recipients(vec![(addr.script_pubkey(), 30_000)]) + .policy_path(path, ScriptType::External), + ) + .unwrap(); + + assert_eq!(psbt.global.unsigned_tx.input[0].sequence, 144); + } + #[test] #[should_panic(expected = "IrreplaceableTransaction")] fn test_bump_fee_irreplaceable_tx() { diff --git a/src/wallet/tx_builder.rs b/src/wallet/tx_builder.rs index 5d46e3e3..672602f7 100644 --- a/src/wallet/tx_builder.rs +++ b/src/wallet/tx_builder.rs @@ -51,7 +51,7 @@ use bitcoin::{OutPoint, Script, SigHashType, Transaction}; use super::coin_selection::{CoinSelectionAlgorithm, DefaultCoinSelectionAlgorithm}; use crate::database::Database; -use crate::types::{FeeRate, UTXO}; +use crate::types::{FeeRate, ScriptType, UTXO}; /// Context in which the [`TxBuilder`] is valid pub trait TxBuilderContext: std::fmt::Debug + Default + Clone {} @@ -77,7 +77,8 @@ pub struct TxBuilder, Ctx: TxBuilderC pub(crate) drain_wallet: bool, pub(crate) single_recipient: Option