From cb29b3ab0d740f9f5228289fd308741b1f91e711 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 1 Apr 2021 12:57:55 +0200 Subject: [PATCH 01/18] Tree-wide: update rust-miniscript to the latest major version This release of rust-miniscript featured an entire refactor of the type system, which lead to pretty invasive changes here. The main modification of the API is the introduction of a new newtype for each descriptor ('concrete' ones, aka already-derived) and restricting the existing types to only contain wildcard-xpubs. This further allowed to contain the derivation in the descriptor types themselves, therefore removing any secp context argument of the transactions creation routines. Signed-off-by: Antoine Poinsot --- Cargo.toml | 4 +- src/error.rs | 17 ++- src/scripts.rs | 353 ++++++++++++++++++++++++++------------------ src/transactions.rs | 313 ++++++++++++++++++--------------------- src/txins.rs | 15 +- src/txouts.rs | 36 ++--- 6 files changed, 394 insertions(+), 344 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ca8543ec..f824987d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,11 +14,11 @@ use-serde = ["serde", "miniscript/use-serde"] [dependencies] bitcoinconsensus = "0.19.0-2" -miniscript = { version = "4.0.3", features = ["compiler"] } +miniscript = { version = "5.1.0", features = ["compiler"] } base64 = { version = "0.13" } serde = { version = "1.0", optional = true } [dev-dependencies] -miniscript = { version = "4.0.3", features = ["compiler", "rand"] } +miniscript = { version = "5.1.0", features = ["compiler", "rand"] } serde_json = "1.0" diff --git a/src/error.rs b/src/error.rs index b4600d58..29177c4b 100644 --- a/src/error.rs +++ b/src/error.rs @@ -14,12 +14,17 @@ use miniscript::{ use std::{convert::From, error, fmt}; /// Error when creating a Revault Bitcoin Script -#[derive(PartialEq, Eq, Debug)] +#[derive(Debug)] pub enum ScriptCreationError { /// Invalid number of keys, threshold, or timelock BadParameters, + /// At least one of the keys was not derivable + NonWildcardKeys, /// Miniscript policy compilation error PolicyCompilation(CompilerError), + /// Miniscript general error, currently only for sanity checks in descriptor + /// constructors + MiniscriptError(miniscript::Error), } impl fmt::Display for ScriptCreationError { @@ -27,6 +32,8 @@ impl fmt::Display for ScriptCreationError { match self { Self::BadParameters => write!(f, "Bad parameters"), Self::PolicyCompilation(e) => write!(f, "Policy compilation error: '{}'", e), + Self::MiniscriptError(e) => write!(f, "Miniscript error: '{}'", e), + Self::NonWildcardKeys => write!(f, "Not all xpubs were wildcard"), } } } @@ -37,6 +44,12 @@ impl From for ScriptCreationError { } } +impl From for ScriptCreationError { + fn from(e: miniscript::Error) -> Self { + Self::MiniscriptError(e) + } +} + impl error::Error for ScriptCreationError {} /// Error when creating a Revault Bitcoin transaction output @@ -232,7 +245,7 @@ impl From for TransactionSerialisationError { impl error::Error for TransactionSerialisationError {} /// An error specific to the management of Revault transactions and scripts. -#[derive(PartialEq, Debug)] +#[derive(Debug)] pub enum Error { /// Error when creating a Revault Bitcoin Script ScriptCreation(ScriptCreationError), diff --git a/src/scripts.rs b/src/scripts.rs index e329b06f..3925284e 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -14,10 +14,10 @@ use crate::error::*; use miniscript::{ - bitcoin::{util::bip32, Address}, - descriptor::DescriptorPublicKey, + bitcoin::{secp256k1, util::bip32, Address, PublicKey}, + descriptor::{DescriptorPublicKey, Wildcard}, policy::concrete::Policy, - Descriptor, MiniscriptKey, Segwitv0, + Descriptor, ForEachKey, Segwitv0, TranslatePk2, }; use std::fmt; @@ -28,18 +28,28 @@ use serde::de; // These are useful to create TxOuts out of the right Script descriptor macro_rules! impl_descriptor_newtype { - ($struct_name:ident, $doc_comment:meta ) => { + ($struct_name:ident, $derived_struct_name:ident, $doc_comment:meta, $der_doc_comment:meta) => { #[$doc_comment] #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] - pub struct $struct_name(pub Descriptor); + pub struct $struct_name(pub Descriptor); - impl $struct_name { + #[$der_doc_comment] + #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] + pub struct $derived_struct_name(pub Descriptor); + + impl $struct_name { /// Derives all wildcard keys in the descriptor using the supplied `child_number` - pub fn derive( + pub fn derive( &self, child_number: bip32::ChildNumber, - ) -> $struct_name { - $struct_name(self.0.derive(child_number)) + secp: &secp256k1::Secp256k1, + ) -> $derived_struct_name { + $derived_struct_name( + self.0 + .derive(child_number.into()) + .translate_pk2(|xpk| xpk.derive_public_key(secp)) + .expect("All pubkeys are derived, no wildcard."), + ) } } }; @@ -47,52 +57,58 @@ macro_rules! impl_descriptor_newtype { impl_descriptor_newtype!( DepositDescriptor, - doc = "The vault / deposit miniscript descriptor. See the [deposit_descriptor] function for more information." + DerivedDepositDescriptor, + doc = "A **generalistic** (with wildcard xpubs) vault / deposit miniscript descriptor. \ + See the [deposit_descriptor] function for more information.", + doc = "A **concrete** (with raw public keys) vault / deposit miniscript descriptor. \ + See the [deposit_descriptor] function for more information." ); impl_descriptor_newtype!( UnvaultDescriptor, - doc = "The unvault miniscript descriptor. See the [unvault_descriptor] function for more information." + DerivedUnvaultDescriptor, + doc = "A **generalistic** (with wildcard xpubs) Unvault miniscript descriptor. \ + See the [unvault_descriptor] function for more information.", + doc = "A **concrete** (with raw public keys) Unvault miniscript descriptor. \ + See the [unvault_descriptor] function for more information." ); impl_descriptor_newtype!( CpfpDescriptor, - doc = - "The CPFP miniscript descriptor. See the [cpfp_descriptor] function for more information." + DerivedCpfpDescriptor, + doc = "A **generalistic** (with wildcard xpubs) CPFP miniscript descriptor. \ + See the [cpfp_descriptor] function for more information.", + doc = "A **concrete** (with raw public keys) CPFP miniscript descriptor. \ + See the [cpfp_descriptor] function for more information." ); -/// Get the miniscript descriptor for the deposit outputs. +/// Get the xpub miniscript descriptor for the deposit outputs. /// /// The deposit policy is an N-of-N, so `thresh(len(all_pubkeys), all_pubkeys)`. /// /// # Examples /// ```rust -/// use revault_tx::{scripts, miniscript::{NullCtx, bitcoin::{self, secp256k1}}}; +/// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; +/// use std::str::FromStr; +/// +/// let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); +/// let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); /// -/// let secp = secp256k1::Secp256k1::new(); -/// let secret_key = secp256k1::SecretKey::from_slice(&[0xcd; 32]).expect("32 bytes, within curve order"); -/// let secret_key_b = secp256k1::SecretKey::from_slice(&[0xcc; 32]).expect("32 bytes, within curve order"); -/// let public_key = bitcoin::PublicKey { -/// compressed: true, -/// key: secp256k1::PublicKey::from_secret_key(&secp, &secret_key), -/// }; -/// let public_key_b = bitcoin::PublicKey { -/// compressed: true, -/// key: secp256k1::PublicKey::from_secret_key(&secp, &secret_key_b), -/// }; /// let deposit_descriptor = -/// scripts::deposit_descriptor(vec![public_key, public_key_b]).expect("Compiling descriptor"); +/// scripts::deposit_descriptor(vec![first_stakeholder, second_stakeholder]).expect("Compiling descriptor"); +/// println!("Deposit descriptor: {}", deposit_descriptor.0); /// -/// println!("Deposit descriptor redeem script: {}", deposit_descriptor.0.witness_script(NullCtx)); +/// let secp = secp256k1::Secp256k1::verification_only(); +/// println!("Tenth child witness script: {}", deposit_descriptor.derive(bip32::ChildNumber::from(10), &secp).0.explicit_script()); /// ``` /// /// # Errors /// - If the passed slice contains less than 2 public keys. /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a /// bug. -pub fn deposit_descriptor( - participants: Vec, -) -> Result, ScriptCreationError> { +pub fn deposit_descriptor( + participants: Vec, +) -> Result { if participants.len() < 2 { return Err(ScriptCreationError::BadParameters); } @@ -100,59 +116,55 @@ pub fn deposit_descriptor( let pubkeys = participants .into_iter() .map(Policy::Key) - .collect::>>(); + .collect::>>(); let policy = Policy::Threshold(pubkeys.len(), pubkeys); // This handles the non-safe or malleable cases. let ms = policy.compile::()?; - Ok(DepositDescriptor(Descriptor::::Wsh(ms))) + let desc = Descriptor::new_wsh(ms)?; + if !desc.for_each_key(|k| k.as_key().is_deriveable()) { + return Err(ScriptCreationError::NonWildcardKeys); + } + + Ok(DepositDescriptor(desc)) } /// Get the miniscript descriptors for the unvault outputs. /// -/// The unvault policy allows either all the participants together to spend, or (the fund managers -/// + the cosigners) after a timelock. -/// -/// As the managers are part of the participants we can have a more efficient Script by expliciting -/// to the compiler that the spenders are always going to sign. Thus we end up with: -/// ```text -/// and(thresh(len(managers), spenders), or(thresh(len(non_managers), non_managers), -/// and(thresh(len(cosigners), cosigners), older(X)))) -/// ```` -/// -/// As we expect the usual operations to be far more likely, we further optimize the policy to: -/// ```text -/// and(thresh(len(managers), managers), or(1@thresh(len(non_managers), non_managers), -/// 10@and(thresh(len(cosigners), cosigners), older(X)))) -/// ``` +/// The unvault policy allows either all the stakeholders to spend, or (the fund managers + the cosigners) +/// after a timelock. /// /// # Examples /// ```rust -/// use revault_tx::{scripts, miniscript::{NullCtx, bitcoin::{self, secp256k1}}}; +/// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; +/// use std::str::FromStr; +/// +/// let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); +/// let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); +/// let third_stakeholder = DescriptorPublicKey::from_str("xpub6Br1DUfrzxTVGo1sanuKDCUmSxDfLRrxLQBqpMqygkQLkQWodoyvvGtUV8Rp3r6d6BNYvedBSU8c7whhn2U8haRVxsWwuQiZ9LoFp7jXPQA/*").unwrap(); +/// +/// let first_cosig = DescriptorPublicKey::from_str("02a489e0ea42b56148d212d325b7c67c6460483ff931c303ea311edfef667c8f35").unwrap(); +/// let second_cosig = DescriptorPublicKey::from_str("02767e6dde4877dcbf64de8a45fe1a0575dfc6b0ed06648f1022412c172ebd875c").unwrap(); +/// let third_cosig = DescriptorPublicKey::from_str("0371cdea381b365ea159a3cf4f14029d1bff5b36b4cf12ac9e42be6955d2ed4ecf").unwrap(); +/// +/// let first_manager = DescriptorPublicKey::from_str("xpub6Duq1ob3cQ8Wxees2fTGNK2wTsVjgTPQcKJiPquXY2rQJTDjeCxkXFxTCGhcunFDt26Ddz45KQu7pbLmmUGG2PXTRVx3iDpBPEhdrijJf4U/*").unwrap(); +/// let second_manager = DescriptorPublicKey::from_str("xpub6EWL35hY9uZZs5Ljt6J3G2ZK1Tu4GPVkFdeGvMknG3VmwVRHhtadCaw5hdRDBgrmx1nPVHWjGBb5xeuC1BfbJzjjcic2gNm1aA7ywWjj7G8/*").unwrap(); +/// /// -/// let secp = secp256k1::Secp256k1::new(); -/// let keys: Vec = (0..7) -/// .map(|i| secp256k1::SecretKey::from_slice(&[i + 1; 32]) -/// .expect("32 bytes, within curve order")) -/// .map(|sk| bitcoin::PublicKey { -/// compressed: true, -/// key: secp256k1::PublicKey::from_secret_key(&secp, &sk), -/// }) -/// .collect(); /// let unvault_descriptor = scripts::unvault_descriptor( -/// // Stakeholders -/// keys[0..2].to_vec(), -/// // Managers -/// keys[3..5].to_vec(), -/// 2, +/// vec![first_stakeholder, second_stakeholder, third_stakeholder], +/// vec![first_manager, second_manager], +/// 1, /// // Cosigners -/// keys[5..7].to_vec(), +/// vec![first_cosig, second_cosig, third_cosig], /// // CSV /// 42 /// ).expect("Compiling descriptor"); +/// println!("Unvault descriptor: {}", unvault_descriptor.0); /// -/// println!("Unvault descriptor redeem script: {}", unvault_descriptor.0.witness_script(NullCtx)); +/// let secp = secp256k1::Secp256k1::verification_only(); +/// println!("Tenth child witness script: {}", unvault_descriptor.derive(bip32::ChildNumber::from(10), &secp).0.explicit_script()); /// ``` /// /// # Errors @@ -160,13 +172,13 @@ pub fn deposit_descriptor( /// not the same as the number of cosigners public key. /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a /// bug. -pub fn unvault_descriptor( - stakeholders: Vec, - managers: Vec, +pub fn unvault_descriptor( + stakeholders: Vec, + managers: Vec, managers_threshold: usize, - cosigners: Vec, + cosigners: Vec, csv_value: u32, -) -> Result, ScriptCreationError> { +) -> Result { if stakeholders.is_empty() || managers.is_empty() || cosigners.len() != stakeholders.len() { return Err(ScriptCreationError::BadParameters); } @@ -175,6 +187,22 @@ pub fn unvault_descriptor( return Err(ScriptCreationError::BadParameters); } + // Stakeholders' and managers' must be deriveable xpubs. + for key in stakeholders.iter().chain(managers.iter()) { + match key { + DescriptorPublicKey::XPub(xpub) => { + if matches!(xpub.wildcard, Wildcard::None) { + return Err(ScriptCreationError::NonWildcardKeys); + } + } + DescriptorPublicKey::SinglePub(_) => { + return Err(ScriptCreationError::NonWildcardKeys); + } + } + } + // Cosigners' key may not be. We use DescriptorSinglePub for them downstream with static raw + // keys, but it's not hardcoded into the type system there to allow a more generic usage. + // We require the locktime to be in number of blocks, and of course to not be disabled. // TODO: use rust-miniscript's constants after upgrading! if (csv_value & (1 << 31)) != 0 || (csv_value & (1 << 22)) != 0 { @@ -184,19 +212,19 @@ pub fn unvault_descriptor( let mut pubkeys = managers .into_iter() .map(Policy::Key) - .collect::>>(); + .collect::>>(); let spenders_thres = Policy::Threshold(managers_threshold, pubkeys); pubkeys = stakeholders .into_iter() .map(Policy::Key) - .collect::>>(); + .collect::>>(); let stakeholders_thres = Policy::Threshold(pubkeys.len(), pubkeys); pubkeys = cosigners .into_iter() .map(Policy::Key) - .collect::>>(); + .collect::>>(); let cosigners_thres = Policy::Threshold(pubkeys.len(), pubkeys); let cosigners_and_csv = Policy::And(vec![cosigners_thres, Policy::Older(csv_value)]); @@ -210,7 +238,8 @@ pub fn unvault_descriptor( // This handles the non-safe or malleable cases. let ms = policy.compile::()?; - Ok(UnvaultDescriptor(Descriptor::::Wsh(ms))) + + Ok(UnvaultDescriptor(Descriptor::new_wsh(ms)?)) } /// Get the miniscript descriptor for the unvault transaction CPFP output. @@ -220,19 +249,24 @@ pub fn unvault_descriptor( /// # Errors /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a /// bug. -pub fn cpfp_descriptor( - managers: Vec, -) -> Result, ScriptCreationError> { +pub fn cpfp_descriptor( + managers: Vec, +) -> Result { let pubkeys = managers .into_iter() .map(Policy::Key) - .collect::>>(); + .collect::>>(); let policy = Policy::Threshold(1, pubkeys); // This handles the non-safe or malleable cases. let ms = policy.compile::()?; - Ok(CpfpDescriptor(Descriptor::::Wsh(ms))) + let desc = Descriptor::new_wsh(ms)?; + if !desc.for_each_key(|k| k.as_key().is_deriveable()) { + return Err(ScriptCreationError::NonWildcardKeys); + } + + Ok(CpfpDescriptor(desc)) } /// The "emergency address", it's kept obfuscated for the entire duration of the vault and is @@ -285,21 +319,41 @@ mod tests { bitcoin::{ secp256k1::{ self, - rand::{rngs::SmallRng, FromEntropy}, + rand::{rngs::SmallRng, FromEntropy, RngCore}, }, - PublicKey, + util::bip32, + Network, }, + descriptor::{DescriptorPublicKey, DescriptorXKey, Wildcard}, policy::compiler::CompilerError, }; - fn get_random_pubkey(rng: &mut SmallRng) -> PublicKey { - let secp = secp256k1::Secp256k1::new(); - let (_, public_key) = secp.generate_keypair(rng); + fn rand_xpub( + rng: &mut SmallRng, + secp: &secp256k1::Secp256k1, + ) -> bip32::ExtendedPrivKey { + let mut rand_bytes = [0u8; 64]; - PublicKey { - compressed: true, - key: public_key, - } + rng.fill_bytes(&mut rand_bytes); + + bip32::ExtendedPrivKey::new_master(Network::Bitcoin, &rand_bytes) + .unwrap_or_else(|_| rand_xpub(rng, secp)) + } + + fn get_random_pubkey( + rng: &mut SmallRng, + secp: &secp256k1::Secp256k1, + ) -> DescriptorPublicKey { + let mut rand_bytes = [0u8; 64]; + + rng.fill_bytes(&mut rand_bytes); + + DescriptorPublicKey::XPub(DescriptorXKey { + origin: None, + xkey: bip32::ExtendedPubKey::from_private(&secp, &rand_xpub(rng, secp)), + derivation_path: bip32::DerivationPath::from(vec![]), + wildcard: Wildcard::Unhardened, + }) } #[test] @@ -322,18 +376,19 @@ mod tests { ((8, 8), 12), ((3, 3), 18), ]; + let secp = secp256k1::Secp256k1::signing_only(); let mut rng = SmallRng::from_entropy(); for ((thresh, n_managers), n_stakeholders) in configurations.iter() { let managers = (0..*n_managers) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); let stakeholders = (0..*n_stakeholders) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); let cosigners = (0..*n_stakeholders) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); unvault_descriptor( stakeholders.clone(), @@ -351,8 +406,8 @@ mod tests { .clone() .iter() .chain(stakeholders.iter()) - .copied() - .collect::>(), + .cloned() + .collect::>(), ) .expect(&format!( "Deposit descriptors creation error with ({}, {})", @@ -368,104 +423,112 @@ mod tests { #[test] fn test_default_configuration_limits() { let mut rng = SmallRng::from_entropy(); + let secp = secp256k1::Secp256k1::signing_only(); assert_eq!( - deposit_descriptor(vec![get_random_pubkey(&mut rng)]), - Err(ScriptCreationError::BadParameters) + deposit_descriptor(vec![get_random_pubkey(&mut rng, &secp)]) + .unwrap_err() + .to_string(), + ScriptCreationError::BadParameters.to_string() ); assert_eq!( unvault_descriptor( - vec![get_random_pubkey(&mut rng)], - vec![get_random_pubkey(&mut rng)], + vec![get_random_pubkey(&mut rng, &secp)], + vec![get_random_pubkey(&mut rng, &secp)], 1, - vec![get_random_pubkey(&mut rng), get_random_pubkey(&mut rng)], + vec![ + get_random_pubkey(&mut rng, &secp), + get_random_pubkey(&mut rng, &secp) + ], 6 - ), - Err(ScriptCreationError::BadParameters) + ) + .unwrap_err() + .to_string(), + ScriptCreationError::BadParameters.to_string() ); assert_eq!( unvault_descriptor( - vec![get_random_pubkey(&mut rng)], - vec![get_random_pubkey(&mut rng)], + vec![get_random_pubkey(&mut rng, &secp)], + vec![get_random_pubkey(&mut rng, &secp)], 1, - vec![get_random_pubkey(&mut rng)], + vec![get_random_pubkey(&mut rng, &secp)], 4194305 - ), - Err(ScriptCreationError::BadParameters) + ) + .unwrap_err() + .to_string(), + ScriptCreationError::BadParameters.to_string() ); assert_eq!( unvault_descriptor( - vec![get_random_pubkey(&mut rng)], - vec![get_random_pubkey(&mut rng)], + vec![get_random_pubkey(&mut rng, &secp)], + vec![get_random_pubkey(&mut rng, &secp)], 2, - vec![get_random_pubkey(&mut rng)], + vec![get_random_pubkey(&mut rng, &secp)], 4194305 - ), - Err(ScriptCreationError::BadParameters) + ) + .unwrap_err() + .to_string(), + ScriptCreationError::BadParameters.to_string() ); // Maximum N-of-N let participants = (0..99) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); deposit_descriptor(participants).expect("Should be OK: max allowed value"); // Now hit the limit let participants = (0..100) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); assert_eq!( - deposit_descriptor(participants), - Err(ScriptCreationError::PolicyCompilation( - CompilerError::LimitsExceeded - )) + deposit_descriptor(participants).unwrap_err().to_string(), + ScriptCreationError::PolicyCompilation(CompilerError::LimitsExceeded).to_string() ); // Maximum 1-of-N let managers = (0..20) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); cpfp_descriptor(managers).expect("Should be OK, that's the maximum allowed value"); // Hit the limit let managers = (0..21) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); assert_eq!( - cpfp_descriptor(managers), - Err(ScriptCreationError::PolicyCompilation( - CompilerError::LimitsExceeded - )) + cpfp_descriptor(managers).unwrap_err().to_string(), + ScriptCreationError::PolicyCompilation(CompilerError::LimitsExceeded).to_string() ); // Maximum non-managers for 2 managers let stakeholders = (0..38) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); let managers = (0..2) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); let cosigners = (0..38) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); unvault_descriptor(stakeholders, managers, 2, cosigners, 145).unwrap(); // Now hit the limit let stakeholders = (0..39) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); let managers = (0..2) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); let cosigners = (0..39) - .map(|_| get_random_pubkey(&mut rng)) - .collect::>(); + .map(|_| get_random_pubkey(&mut rng, &secp)) + .collect::>(); assert_eq!( - unvault_descriptor(stakeholders, managers, 2, cosigners, 32), - Err(ScriptCreationError::PolicyCompilation( - CompilerError::LimitsExceeded - )) + unvault_descriptor(stakeholders, managers, 2, cosigners, 32) + .unwrap_err() + .to_string(), + ScriptCreationError::PolicyCompilation(CompilerError::LimitsExceeded).to_string() ); } diff --git a/src/transactions.rs b/src/transactions.rs index 9825550d..63c4161f 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -5,12 +5,7 @@ //! We use PSBTs as defined in [bip-0174](https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki) //! for data structure as well as roles distribution. -use crate::{ - error::*, - scripts::{CpfpDescriptor, DepositDescriptor, EmergencyAddress, UnvaultDescriptor}, - txins::*, - txouts::*, -}; +use crate::{error::*, scripts::*, txins::*, txouts::*}; use miniscript::{ bitcoin::{ @@ -27,7 +22,7 @@ use miniscript::{ Address, Network, OutPoint, PublicKey as BitcoinPubKey, Script, SigHash, SigHashType, Transaction, }, - BitcoinSig, DescriptorPublicKey, DescriptorPublicKeyCtx, MiniscriptKey, ToPublicKey, + BitcoinSig, DescriptorTrait, }; #[cfg(feature = "use-serde")] @@ -438,7 +433,7 @@ macro_rules! create_tx { Psbt { global: PsbtGlobal { unsigned_tx: Transaction { - version: 2, + version: TX_VERSION, lock_time: $lock_time, input: vec![$( $revault_txin.unsigned_txin(), @@ -447,6 +442,9 @@ macro_rules! create_tx { $txout.clone().into_txout(), )*], }, + version: 0, + xpub: BTreeMap::new(), + proprietary: BTreeMap::new(), unknown: BTreeMap::new(), }, inputs: vec![$( @@ -647,16 +645,15 @@ impl UnvaultTransaction { /// It's always created using a fixed feerate and the CPFP output value is fixed as well. /// /// BIP174 Creator and Updater roles. - pub fn new>( + pub fn new( deposit_input: DepositTxIn, - unvault_descriptor: &UnvaultDescriptor, - cpfp_descriptor: &CpfpDescriptor, - to_pk_ctx: ToPkCtx, + unvault_descriptor: &DerivedUnvaultDescriptor, + cpfp_descriptor: &DerivedCpfpDescriptor, lock_time: u32, ) -> Result { // First, create a dummy transaction to get its weight without Witness - let dummy_unvault_txout = UnvaultTxOut::new(u64::MAX, unvault_descriptor, to_pk_ctx); - let dummy_cpfp_txout = CpfpTxOut::new(u64::MAX, cpfp_descriptor, to_pk_ctx); + let dummy_unvault_txout = UnvaultTxOut::new(u64::MAX, unvault_descriptor); + let dummy_cpfp_txout = CpfpTxOut::new(u64::MAX, cpfp_descriptor); let dummy_tx = create_tx!( [(deposit_input.clone(), SigHashType::All)], [dummy_unvault_txout, dummy_cpfp_txout], @@ -687,8 +684,8 @@ impl UnvaultTransaction { } let unvault_value = deposit_value - fees - UNVAULT_CPFP_VALUE; // Arithmetic checked above - let unvault_txout = UnvaultTxOut::new(unvault_value, unvault_descriptor, to_pk_ctx); - let cpfp_txout = CpfpTxOut::new(UNVAULT_CPFP_VALUE, cpfp_descriptor, to_pk_ctx); + let unvault_txout = UnvaultTxOut::new(unvault_value, unvault_descriptor); + let cpfp_txout = CpfpTxOut::new(UNVAULT_CPFP_VALUE, cpfp_descriptor); Ok(UnvaultTransaction(create_tx!( [(deposit_input, SigHashType::All)], [unvault_txout, cpfp_txout], @@ -696,13 +693,12 @@ impl UnvaultTransaction { ))) } - fn unvault_txin>( + fn unvault_txin( &self, - unvault_descriptor: &UnvaultDescriptor, - to_pk_ctx: ToPkCtx, + unvault_descriptor: &DerivedUnvaultDescriptor, sequence: u32, ) -> UnvaultTxIn { - let spk = unvault_descriptor.0.script_pubkey(to_pk_ctx); + let spk = unvault_descriptor.0.script_pubkey(); let index = self .inner_tx() .global @@ -714,7 +710,7 @@ impl UnvaultTransaction { // Unwraped above let txo = &self.inner_tx().global.unsigned_tx.output[index]; - let prev_txout = UnvaultTxOut::new(txo.value, unvault_descriptor, to_pk_ctx); + let prev_txout = UnvaultTxOut::new(txo.value, unvault_descriptor); UnvaultTxIn::new( OutPoint { txid: self.inner_tx().global.unsigned_tx.txid(), @@ -726,31 +722,25 @@ impl UnvaultTransaction { } /// Get the Unvault txo to be referenced in a spending transaction - pub fn spend_unvault_txin>( + pub fn spend_unvault_txin( &self, - unvault_descriptor: &UnvaultDescriptor, - to_pk_ctx: ToPkCtx, + unvault_descriptor: &DerivedUnvaultDescriptor, csv: u32, ) -> UnvaultTxIn { - self.unvault_txin(unvault_descriptor, to_pk_ctx, csv) + self.unvault_txin(unvault_descriptor, csv) } /// Get the Unvault txo to be referenced in a revocation transaction - pub fn revault_unvault_txin>( + pub fn revault_unvault_txin( &self, - unvault_descriptor: &UnvaultDescriptor, - to_pk_ctx: ToPkCtx, + unvault_descriptor: &DerivedUnvaultDescriptor, ) -> UnvaultTxIn { - self.unvault_txin(unvault_descriptor, to_pk_ctx, RBF_SEQUENCE) + self.unvault_txin(unvault_descriptor, RBF_SEQUENCE) } /// Get the CPFP txo to be referenced in a spending transaction - pub fn cpfp_txin>( - &self, - cpfp_descriptor: &CpfpDescriptor, - to_pk_ctx: ToPkCtx, - ) -> CpfpTxIn { - let spk = cpfp_descriptor.0.script_pubkey(to_pk_ctx); + pub fn cpfp_txin(&self, cpfp_descriptor: &DerivedCpfpDescriptor) -> CpfpTxIn { + let spk = cpfp_descriptor.0.script_pubkey(); let index = self .inner_tx() .global @@ -762,7 +752,7 @@ impl UnvaultTransaction { // Unwraped above let txo = &self.inner_tx().global.unsigned_tx.output[index]; - let prev_txout = CpfpTxOut::new(txo.value, cpfp_descriptor, to_pk_ctx); + let prev_txout = CpfpTxOut::new(txo.value, cpfp_descriptor); CpfpTxIn::new( OutPoint { txid: self.inner_tx().global.unsigned_tx.txid(), @@ -838,16 +828,15 @@ impl CancelTransaction { /// may have a fee-bumping input. /// /// BIP174 Creator and Updater roles. - pub fn new>( + pub fn new( unvault_input: UnvaultTxIn, feebump_input: Option, - deposit_descriptor: &DepositDescriptor, - to_pk_ctx: ToPkCtx, + deposit_descriptor: &DerivedDepositDescriptor, lock_time: u32, ) -> CancelTransaction { // First, create a dummy transaction to get its weight without Witness. Note that we always // account for the weight *without* feebump input. It pays for itself. - let deposit_txo = DepositTxOut::new(u64::MAX, deposit_descriptor, to_pk_ctx); + let deposit_txo = DepositTxOut::new(u64::MAX, deposit_descriptor); let dummy_tx = create_tx!( [(unvault_input.clone(), SigHashType::AllPlusAnyoneCanPay)], [deposit_txo], @@ -874,7 +863,7 @@ impl CancelTransaction { let revault_value = unvault_value .checked_sub(fees) .expect("We would not create a dust unvault txo"); - let deposit_txo = DepositTxOut::new(revault_value, deposit_descriptor, to_pk_ctx); + let deposit_txo = DepositTxOut::new(revault_value, deposit_descriptor); CancelTransaction(if let Some(feebump_input) = feebump_input { create_tx!( @@ -1148,11 +1137,10 @@ impl SpendTransaction { /// may want to create a transaction without a change output. /// /// BIP174 Creator and Updater roles. - pub fn new>( + pub fn new( unvault_inputs: Vec, spend_txouts: Vec, - cpfp_descriptor: &CpfpDescriptor, - to_pk_ctx: ToPkCtx, + cpfp_descriptor: &DerivedCpfpDescriptor, lock_time: u32, insane_fee_check: bool, ) -> Result { @@ -1162,7 +1150,6 @@ impl SpendTransaction { unvault_inputs.clone(), spend_txouts.clone(), cpfp_descriptor, - to_pk_ctx, lock_time, ); @@ -1199,6 +1186,9 @@ impl SpendTransaction { .collect(), output: txos, }, + version: 0, + xpub: BTreeMap::new(), + proprietary: BTreeMap::new(), unknown: BTreeMap::new(), }, inputs: unvault_inputs @@ -1240,15 +1230,14 @@ impl SpendTransaction { /// The CPFP output value is dependant on the transaction size, see [practical-revaul /// t](https://github.com/revault/practical-revault/blob/master/transactions.md#spend_tx) for /// more details. - pub fn cpfp_txout>( + pub fn cpfp_txout( unvault_inputs: Vec, spend_txouts: Vec, - cpfp_descriptor: &CpfpDescriptor, - to_pk_ctx: ToPkCtx, + cpfp_descriptor: &DerivedCpfpDescriptor, lock_time: u32, ) -> CpfpTxOut { let mut txos = Vec::with_capacity(spend_txouts.len() + 1); - let dummy_cpfp_txo = CpfpTxOut::new(u64::MAX, &cpfp_descriptor, to_pk_ctx); + let dummy_cpfp_txo = CpfpTxOut::new(u64::MAX, &cpfp_descriptor); txos.push(dummy_cpfp_txo.txout().clone()); txos.extend(spend_txouts.iter().map(|spend_txout| match spend_txout { SpendTxOut::Destination(ref txo) => txo.clone().into_txout(), @@ -1281,7 +1270,7 @@ impl SpendTransaction { // See https://github.com/revault/practical-revault/blob/master/transactions.md#spend_tx // for this arbirtrary value. let cpfp_value = 16 * total_weight; - CpfpTxOut::new(cpfp_value, &cpfp_descriptor, to_pk_ctx) + CpfpTxOut::new(cpfp_value, &cpfp_descriptor) } /// Get the feerate of this transaction, assuming fully-satisfied inputs. If the transaction @@ -1316,7 +1305,7 @@ impl SpendTransaction { .try_into() .expect("Bug: witness size >u64::MAX") } else { - miniscript::Descriptor::Wsh( + miniscript::descriptor::Wsh::new( miniscript::Miniscript::parse( txin.witness_script .as_ref() @@ -1324,7 +1313,8 @@ impl SpendTransaction { ) .expect("UnvaultTxIn witness_script is created from a Miniscript"), ) - .max_satisfaction_weight(miniscript::NullCtx) + .expect("") + .max_satisfaction_weight() .expect("It's a sane Script, derived from a Miniscript") .try_into() .expect("Can't be >u64::MAX") @@ -1377,19 +1367,15 @@ impl SpendTransaction { pub struct DepositTransaction(pub Transaction); impl DepositTransaction { /// Assumes that the outpoint actually refers to this transaction. Will panic otherwise. - pub fn deposit_txin>( + pub fn deposit_txin( &self, outpoint: OutPoint, - deposit_descriptor: &DepositDescriptor, - to_pk_ctx: ToPkCtx, + deposit_descriptor: &DerivedDepositDescriptor, ) -> DepositTxIn { assert!(outpoint.txid == self.0.txid()); let txo = self.0.output[outpoint.vout as usize].clone(); - DepositTxIn::new( - outpoint, - DepositTxOut::new(txo.value, deposit_descriptor, to_pk_ctx), - ) + DepositTxIn::new(outpoint, DepositTxOut::new(txo.value, deposit_descriptor)) } } @@ -1399,40 +1385,46 @@ pub struct FeeBumpTransaction(pub Transaction); /// Get the chain of pre-signed transaction out of a deposit available for a manager. /// No feebump input. -pub fn transaction_chain_manager>( +pub fn transaction_chain_manager( deposit_txin: DepositTxIn, - deposit_descriptor: &DepositDescriptor, - unvault_descriptor: &UnvaultDescriptor, - cpfp_descriptor: &CpfpDescriptor, - to_pk_ctx: ToPkCtx, + deposit_descriptor: &DepositDescriptor, + unvault_descriptor: &UnvaultDescriptor, + cpfp_descriptor: &CpfpDescriptor, + derivation_index: ChildNumber, lock_time: u32, + secp: &secp256k1::Secp256k1, ) -> Result<(UnvaultTransaction, CancelTransaction), Error> { + let (der_deposit_descriptor, der_unvault_descriptor, der_cpfp_descriptor) = ( + deposit_descriptor.derive(derivation_index, secp), + unvault_descriptor.derive(derivation_index, secp), + cpfp_descriptor.derive(derivation_index, secp), + ); + let unvault_tx = UnvaultTransaction::new( deposit_txin.clone(), - &unvault_descriptor, - &cpfp_descriptor, - to_pk_ctx, + &der_unvault_descriptor, + &der_cpfp_descriptor, lock_time, )?; let cancel_tx = CancelTransaction::new( - unvault_tx.revault_unvault_txin(&unvault_descriptor, to_pk_ctx), + unvault_tx.revault_unvault_txin(&der_unvault_descriptor), None, - &deposit_descriptor, - to_pk_ctx, + &der_deposit_descriptor, lock_time, ); Ok((unvault_tx, cancel_tx)) } -/// Get the entire chain of pre-signed transaction out of a deposit. No feebump input. -pub fn transaction_chain>( +/// Get the entire chain of pre-signed transaction for this derivation index out of a deposit. No feebump input. +pub fn transaction_chain( deposit_txin: DepositTxIn, - deposit_descriptor: &DepositDescriptor, - unvault_descriptor: &UnvaultDescriptor, - cpfp_descriptor: &CpfpDescriptor, + deposit_descriptor: &DepositDescriptor, + unvault_descriptor: &UnvaultDescriptor, + cpfp_descriptor: &CpfpDescriptor, + derivation_index: ChildNumber, emer_address: EmergencyAddress, - to_pk_ctx: ToPkCtx, lock_time: u32, + secp: &secp256k1::Secp256k1, ) -> Result< ( UnvaultTransaction, @@ -1447,13 +1439,16 @@ pub fn transaction_chain deposit_descriptor, unvault_descriptor, cpfp_descriptor, - to_pk_ctx, + derivation_index, lock_time, + secp, )?; + let emergency_tx = EmergencyTransaction::new(deposit_txin, None, emer_address.clone(), lock_time)?; + let der_unvault_descriptor = unvault_descriptor.derive(derivation_index, secp); let unvault_emergency_tx = UnvaultEmergencyTransaction::new( - unvault_tx.revault_unvault_txin(&unvault_descriptor, to_pk_ctx), + unvault_tx.revault_unvault_txin(&der_unvault_descriptor), None, emer_address, lock_time, @@ -1465,24 +1460,18 @@ pub fn transaction_chain /// Get a spend transaction out of a list of deposits and derivation indexes. The /// `unvault_descriptor` will be derived for each and should not be beforehand. pub fn spend_tx_from_deposits( - deposit_txins: Vec<(DepositTxIn, ChildNumber)>, + deposit_txins: Vec<(DepositTxIn, DerivedUnvaultDescriptor, DerivedCpfpDescriptor)>, spend_txos: Vec, - unvault_descriptor: &UnvaultDescriptor, - cpfp_descriptor: &CpfpDescriptor, - to_pk_ctx: DescriptorPublicKeyCtx, + cpfp_descriptor: &DerivedCpfpDescriptor, unvault_csv: u32, lock_time: u32, check_insane_fees: bool, ) -> Result { let unvault_txins = deposit_txins .into_iter() - .map(|(txin, index)| { - let unvault_desc = unvault_descriptor.derive(index); - let cpfp_desc = cpfp_descriptor.derive(index); - UnvaultTransaction::new(txin, &unvault_desc, &cpfp_desc, to_pk_ctx, lock_time).and_then( - |unvault_tx| { - Ok(unvault_tx.spend_unvault_txin(&unvault_desc, to_pk_ctx, unvault_csv)) - }, + .map(|(txin, unvault_desc, cpfp_desc)| { + UnvaultTransaction::new(txin, &unvault_desc, &cpfp_desc, lock_time).and_then( + |unvault_tx| Ok(unvault_tx.spend_unvault_txin(&unvault_desc, unvault_csv)), ) }) .collect::, TransactionCreationError>>()?; @@ -1491,7 +1480,6 @@ pub fn spend_tx_from_deposits( unvault_txins, spend_txos, cpfp_descriptor, - to_pk_ctx, lock_time, check_insane_fees, ) @@ -1515,8 +1503,8 @@ mod tests { util::bip32, Address, Network, OutPoint, SigHash, SigHashType, Transaction, TxIn, TxOut, }, - descriptor::{DescriptorPublicKey, DescriptorXKey}, - Descriptor, DescriptorPublicKeyCtx, ToPublicKey, + descriptor::{DescriptorPublicKey, DescriptorXKey, Wildcard}, + Descriptor, DescriptorTrait, }; fn get_random_privkey(rng: &mut SmallRng) -> bip32::ExtendedPrivKey { @@ -1551,7 +1539,7 @@ mod tests { origin: None, xkey: bip32::ExtendedPubKey::from_private(&secp, &xpriv), derivation_path: bip32::DerivationPath::from(vec![]), - is_wildcard: true, + wildcard: Wildcard::Unhardened, }) }) .collect::>(); @@ -1566,7 +1554,7 @@ mod tests { origin: None, xkey: bip32::ExtendedPubKey::from_private(&secp, &xpriv), derivation_path: bip32::DerivationPath::from(vec![]), - is_wildcard: true, + wildcard: Wildcard::Unhardened, }) }) .collect::>(); @@ -1581,7 +1569,7 @@ mod tests { origin: None, xkey: bip32::ExtendedPubKey::from_private(&secp, &xpriv), derivation_path: bip32::DerivationPath::from(vec![]), - is_wildcard: true, + wildcard: Wildcard::Unhardened, }) }) .collect::>(); @@ -1627,14 +1615,21 @@ mod tests { origin: None, xkey: bip32::ExtendedPubKey::from_private(&secp, xpriv), derivation_path: bip32::DerivationPath::from(vec![]), - is_wildcard: child_number.is_some(), + wildcard: if child_number.is_some() { + Wildcard::Unhardened + } else { + Wildcard::None + }, }); - let xpub_ctx = DescriptorPublicKeyCtx::new( - &secp, - // If the xpub is not a wildcard, it's not taken into account....... - child_number.unwrap_or_else(|| bip32::ChildNumber::from(0)), - ); - tx.add_signature(input_index, xpub.to_public_key(xpub_ctx), sig)?; + let key = if let Some(index) = child_number { + xpub.derive(index.into()) + } else { + xpub + } + .derive_public_key(secp) + .unwrap(); + + tx.add_signature(input_index, key, sig)?; } Ok(()) @@ -1650,8 +1645,10 @@ mod tests { // Test the dust limit assert_eq!( - derive_transactions(2, 1, csv, 234_631, &secp), - Err(Error::TransactionCreation(TransactionCreationError::Dust)) + derive_transactions(2, 1, csv, 234_631, &secp) + .unwrap_err() + .to_string(), + Error::TransactionCreation(TransactionCreationError::Dust).to_string() ); // Absolute minimum derive_transactions(2, 1, csv, 234_632, &secp).expect(&format!( @@ -1684,7 +1681,6 @@ mod tests { ) -> Result<(), Error> { // Let's get the 10th key of each let child_number = bip32::ChildNumber::from(10); - let xpub_ctx = DescriptorPublicKeyCtx::new(&secp, child_number); // Keys, keys, keys everywhere ! let ( @@ -1709,13 +1705,20 @@ mod tests { // We reuse the deposit descriptor for the emergency address let emergency_address = EmergencyAddress::from(Address::p2wsh( - &deposit_descriptor.0.witness_script(xpub_ctx), + &deposit_descriptor + .derive(child_number, secp) + .0 + .explicit_script(), Network::Bitcoin, )) .expect("It's a P2WSH"); + let der_deposit_descriptor = deposit_descriptor.derive(child_number, secp); + let der_unvault_descriptor = unvault_descriptor.derive(child_number, secp); + let der_cpfp_descriptor = cpfp_descriptor.derive(child_number, secp); + // The funding transaction does not matter (random txid from my mempool) - let deposit_scriptpubkey = deposit_descriptor.0.script_pubkey(xpub_ctx); + let deposit_scriptpubkey = der_deposit_descriptor.0.script_pubkey(); let deposit_raw_tx = Transaction { version: 2, lock_time: 0, @@ -1731,11 +1734,8 @@ mod tests { script_pubkey: deposit_scriptpubkey.clone(), }], }; - let deposit_txo = DepositTxOut::new( - deposit_raw_tx.output[0].value, - &deposit_descriptor, - xpub_ctx, - ); + let deposit_txo = + DepositTxOut::new(deposit_raw_tx.output[0].value, &der_deposit_descriptor); let deposit_tx = DepositTransaction(deposit_raw_tx); let deposit_txin = DepositTxIn::new( OutPoint { @@ -1751,9 +1751,10 @@ mod tests { &deposit_descriptor, &unvault_descriptor, &cpfp_descriptor, + child_number, emergency_address.clone(), - xpub_ctx, 0, + secp, )?; // The fee-bumping utxo, used in revaulting transactions inputs to bump their feerate. @@ -1761,13 +1762,17 @@ mod tests { let mut rng = SmallRng::from_entropy(); let feebump_xpriv = get_random_privkey(&mut rng); let feebump_xpub = bip32::ExtendedPubKey::from_private(&secp, &feebump_xpriv); - let feebump_descriptor = - Descriptor::::Wpkh(DescriptorPublicKey::XPub(DescriptorXKey { + let feebump_descriptor = Descriptor::new_wpkh( + DescriptorPublicKey::XPub(DescriptorXKey { origin: None, xkey: feebump_xpub, derivation_path: bip32::DerivationPath::from(vec![]), - is_wildcard: false, // We are not going to derive from this one - })); + wildcard: Wildcard::None, // We are not going to derive from this one + }) + .derive_public_key(secp) + .unwrap(), + ) + .unwrap(); let raw_feebump_tx = Transaction { version: 2, lock_time: 0, @@ -1780,7 +1785,7 @@ mod tests { }], output: vec![TxOut { value: 56730, - script_pubkey: feebump_descriptor.script_pubkey(xpub_ctx), + script_pubkey: feebump_descriptor.script_pubkey(), }], }; let feebump_txo = @@ -1820,10 +1825,8 @@ mod tests { SigHashType::All, ); assert_eq!( - err, - Err(Error::InputSatisfaction( - InputSatisfactionError::UnexpectedSighashType - )) + err.unwrap_err().to_string(), + Error::InputSatisfaction(InputSatisfactionError::UnexpectedSighashType).to_string() ); // Now, that's the right SIGHASH satisfy_transaction_input( @@ -1853,11 +1856,7 @@ mod tests { ) .unwrap(); let emergency_tx_sighash_feebump = emergency_tx - .signature_hash_feebump_input( - 1, - &feebump_descriptor.script_code(xpub_ctx), - SigHashType::All, - ) + .signature_hash_feebump_input(1, &feebump_descriptor.script_code(), SigHashType::All) .expect("Input exists"); satisfy_transaction_input( &secp, @@ -1885,9 +1884,8 @@ mod tests { let deposit_txin_sat_cost = deposit_txin.max_sat_weight(); let mut unvault_tx = UnvaultTransaction::new( deposit_txin.clone(), - &unvault_descriptor, - &cpfp_descriptor, - xpub_ctx, + &der_unvault_descriptor, + &der_cpfp_descriptor, 0, )?; @@ -1898,16 +1896,11 @@ mod tests { assert_eq!(unvault_tx.fees(), (548 + deposit_txin_sat_cost as u64) * 6); // Create and sign the cancel transaction - let rev_unvault_txin = unvault_tx.revault_unvault_txin(&unvault_descriptor, xpub_ctx); + let rev_unvault_txin = unvault_tx.revault_unvault_txin(&der_unvault_descriptor); assert_eq!(rev_unvault_txin.txout().txout().value, unvault_value); // We can create it entirely without the feebump input - let mut cancel_tx_without_feebump = CancelTransaction::new( - rev_unvault_txin.clone(), - None, - &deposit_descriptor, - xpub_ctx, - 0, - ); + let mut cancel_tx_without_feebump = + CancelTransaction::new(rev_unvault_txin.clone(), None, &der_deposit_descriptor, 0); assert_eq!(h_cancel, cancel_tx_without_feebump); // Keep track of the fees we computed.. let value_no_feebump = cancel_tx_without_feebump @@ -1945,8 +1938,7 @@ mod tests { let mut cancel_tx = CancelTransaction::new( rev_unvault_txin.clone(), Some(feebump_txin), - &deposit_descriptor, - xpub_ctx, + &der_deposit_descriptor, 0, ); // It really is a belt-and-suspenders check as the sighash would differ too. @@ -1961,11 +1953,7 @@ mod tests { "Base fees when computing with with feebump differ !!" ); let cancel_tx_sighash_feebump = cancel_tx - .signature_hash_feebump_input( - 1, - &feebump_descriptor.script_code(xpub_ctx), - SigHashType::All, - ) + .signature_hash_feebump_input(1, &feebump_descriptor.script_code(), SigHashType::All) .expect("Input exists"); satisfy_transaction_input( &secp, @@ -2049,11 +2037,7 @@ mod tests { } // Now actually satisfy it, libbitcoinconsensus should not yell let unemer_tx_sighash_feebump = unemergency_tx - .signature_hash_feebump_input( - 1, - &feebump_descriptor.script_code(xpub_ctx), - SigHashType::All, - ) + .signature_hash_feebump_input(1, &feebump_descriptor.script_code(), SigHashType::All) .expect("Input exists"); satisfy_transaction_input( &secp, @@ -2079,17 +2063,16 @@ mod tests { Some(child_number), SigHashType::All, )?; + unvault_tx.finalize(&secp)?; // Create and sign a spend transaction - let spend_unvault_txin = - unvault_tx.spend_unvault_txin(&unvault_descriptor, xpub_ctx, csv - 1); // Off-by-one csv + let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor, csv - 1); // Off-by-one csv let dummy_txo = ExternalTxOut::default(); let cpfp_value = SpendTransaction::cpfp_txout( vec![spend_unvault_txin.clone()], vec![SpendTxOut::Destination(dummy_txo.clone())], - &cpfp_descriptor, - xpub_ctx, + &der_cpfp_descriptor, 0, ) .txout() @@ -2104,8 +2087,7 @@ mod tests { let mut spend_tx = SpendTransaction::new( vec![spend_unvault_txin], vec![SpendTxOut::Destination(spend_txo.clone())], - &cpfp_descriptor, - xpub_ctx, + &der_cpfp_descriptor, 0, true, ) @@ -2140,12 +2122,11 @@ mod tests { } // "This time for sure !" - let spend_unvault_txin = unvault_tx.spend_unvault_txin(&unvault_descriptor, xpub_ctx, csv); // Right csv + let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor, csv); // Right csv let mut spend_tx = SpendTransaction::new( vec![spend_unvault_txin], vec![SpendTxOut::Destination(spend_txo.clone())], - &cpfp_descriptor, - xpub_ctx, + &der_cpfp_descriptor, 0, true, ) @@ -2175,7 +2156,7 @@ mod tests { "0ed7dc14fe8d1364b3185fa46e940cb8e858f8de32e63f88353a2bd66eb99e2a:0", ) .unwrap(), - UnvaultTxOut::new(deposit_value, &unvault_descriptor, xpub_ctx), + UnvaultTxOut::new(deposit_value, &der_unvault_descriptor), csv, ), UnvaultTxIn::new( @@ -2183,7 +2164,7 @@ mod tests { "23aacfca328942892bb007a86db0bf5337005f642b3c46aef50c23af03ec333a:1", ) .unwrap(), - UnvaultTxOut::new(deposit_value * 4, &unvault_descriptor, xpub_ctx), + UnvaultTxOut::new(deposit_value * 4, &der_unvault_descriptor), csv, ), UnvaultTxIn::new( @@ -2191,7 +2172,7 @@ mod tests { "fccabf4077b7e44ba02378a97a84611b545c11a1ef2af16cbb6e1032aa059b1d:0", ) .unwrap(), - UnvaultTxOut::new(deposit_value / 2, &unvault_descriptor, xpub_ctx), + UnvaultTxOut::new(deposit_value / 2, &der_unvault_descriptor), csv, ), UnvaultTxIn::new( @@ -2199,7 +2180,7 @@ mod tests { "71dc04303184d54e6cc2f92d843282df2854d6dd66f10081147b84aeed830ae1:0", ) .unwrap(), - UnvaultTxOut::new(deposit_value * 50, &unvault_descriptor, xpub_ctx), + UnvaultTxOut::new(deposit_value * 50, &der_unvault_descriptor), csv, ), ]; @@ -2208,8 +2189,7 @@ mod tests { let cpfp_value = SpendTransaction::cpfp_txout( spend_unvault_txins.clone(), vec![SpendTxOut::Destination(dummy_txo.clone())], - &cpfp_descriptor, - xpub_ctx, + &der_cpfp_descriptor, 0, ) .txout() @@ -2227,8 +2207,7 @@ mod tests { let mut spend_tx = SpendTransaction::new( spend_unvault_txins, vec![SpendTxOut::Destination(spend_txo.clone())], - &cpfp_descriptor, - xpub_ctx, + &der_cpfp_descriptor, 0, true, ) diff --git a/src/txins.rs b/src/txins.rs index d0d2a12b..d3de3b95 100644 --- a/src/txins.rs +++ b/src/txins.rs @@ -4,7 +4,10 @@ use crate::txouts::{CpfpTxOut, DepositTxOut, FeeBumpTxOut, RevaultTxOut, UnvaultTxOut}; -use miniscript::bitcoin::{OutPoint, TxIn}; +use miniscript::{ + bitcoin::{OutPoint, TxIn}, + DescriptorTrait, +}; use std::fmt; @@ -74,7 +77,7 @@ impl DepositTxIn { /// Get the maximum size, in weight units, a satisfaction for this input would cost. pub fn max_sat_weight(&self) -> usize { - miniscript::Descriptor::Wsh( + miniscript::descriptor::Wsh::new( miniscript::Miniscript::parse( self.prev_txout .witness_script() @@ -83,7 +86,8 @@ impl DepositTxIn { ) .expect("DepositTxIn witness_script is created from a Miniscript"), ) - .max_satisfaction_weight(miniscript::NullCtx) + .expect("DepositTxIn witness_script is a witness script hash") + .max_satisfaction_weight() .expect("It's a sane Script, derived from a Miniscript") } } @@ -106,7 +110,7 @@ impl UnvaultTxIn { /// Get the maximum size, in weight units, a satisfaction for this input would cost. pub fn max_sat_weight(&self) -> usize { - miniscript::Descriptor::Wsh( + miniscript::descriptor::Wsh::new( miniscript::Miniscript::parse( self.prev_txout .witness_script() @@ -115,7 +119,8 @@ impl UnvaultTxIn { ) .expect("UnvaultTxIn witness_script is created from a Miniscript"), ) - .max_satisfaction_weight(miniscript::NullCtx) + .expect("UnvaultTxIn is a P2WSH") + .max_satisfaction_weight() .expect("It's a sane Script, derived from a Miniscript") } } diff --git a/src/txouts.rs b/src/txouts.rs index 8c9f7b65..14f826ea 100644 --- a/src/txouts.rs +++ b/src/txouts.rs @@ -4,12 +4,14 @@ use crate::{ error::TxoutCreationError, - scripts::{CpfpDescriptor, DepositDescriptor, EmergencyAddress, UnvaultDescriptor}, + scripts::{ + DerivedCpfpDescriptor, DerivedDepositDescriptor, DerivedUnvaultDescriptor, EmergencyAddress, + }, }; use miniscript::{ bitcoin::{Script, TxOut}, - MiniscriptKey, ToPublicKey, + DescriptorTrait, }; use std::fmt; @@ -61,17 +63,13 @@ implem_revault_txout!( ); impl DepositTxOut { /// Create a new DepositTxOut out of the given Deposit script descriptor - pub fn new>( - value: u64, - script_descriptor: &DepositDescriptor, - to_pk_ctx: ToPkCtx, - ) -> DepositTxOut { + pub fn new(value: u64, script_descriptor: &DerivedDepositDescriptor) -> DepositTxOut { DepositTxOut { txout: TxOut { value, - script_pubkey: script_descriptor.0.script_pubkey(to_pk_ctx), + script_pubkey: script_descriptor.0.script_pubkey(), }, - witness_script: Some(script_descriptor.0.witness_script(to_pk_ctx)), + witness_script: Some(script_descriptor.0.explicit_script()), } } } @@ -79,17 +77,13 @@ impl DepositTxOut { implem_revault_txout!(UnvaultTxOut, doc = "*The* unvault transaction output."); impl UnvaultTxOut { /// Create a new UnvaultTxOut out of the given Unvault script descriptor - pub fn new>( - value: u64, - script_descriptor: &UnvaultDescriptor, - to_pk_ctx: ToPkCtx, - ) -> UnvaultTxOut { + pub fn new(value: u64, script_descriptor: &DerivedUnvaultDescriptor) -> UnvaultTxOut { UnvaultTxOut { txout: TxOut { value, - script_pubkey: script_descriptor.0.script_pubkey(to_pk_ctx), + script_pubkey: script_descriptor.0.script_pubkey(), }, - witness_script: Some(script_descriptor.0.witness_script(to_pk_ctx)), + witness_script: Some(script_descriptor.0.explicit_script()), } } } @@ -117,17 +111,13 @@ implem_revault_txout!( ); impl CpfpTxOut { /// Create a new CpfpTxOut out of the given Cpfp descriptor - pub fn new>( - value: u64, - script_descriptor: &CpfpDescriptor, - to_pk_ctx: ToPkCtx, - ) -> CpfpTxOut { + pub fn new(value: u64, script_descriptor: &DerivedCpfpDescriptor) -> CpfpTxOut { CpfpTxOut { txout: TxOut { value, - script_pubkey: script_descriptor.0.script_pubkey(to_pk_ctx), + script_pubkey: script_descriptor.0.script_pubkey(), }, - witness_script: Some(script_descriptor.0.witness_script(to_pk_ctx)), + witness_script: Some(script_descriptor.0.explicit_script()), } } } From b7ab05aedfe9a6e6df5820d0dc976f6c41f94aa5 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 2 Apr 2021 12:45:36 +0200 Subject: [PATCH 02/18] Cargo: use a smaller alternative to rust-secp's rand Signed-off-by: Antoine Poinsot --- Cargo.toml | 2 +- src/scripts.rs | 22 +++++++--------------- src/transactions.rs | 25 ++++++++++--------------- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f824987d..91fdf010 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,5 +20,5 @@ base64 = { version = "0.13" } serde = { version = "1.0", optional = true } [dev-dependencies] -miniscript = { version = "5.1.0", features = ["compiler", "rand"] } +fastrand = "1.4.0" serde_json = "1.0" diff --git a/src/scripts.rs b/src/scripts.rs index 3925284e..753546c2 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -317,37 +317,29 @@ mod tests { use miniscript::{ bitcoin::{ - secp256k1::{ - self, - rand::{rngs::SmallRng, FromEntropy, RngCore}, - }, + secp256k1::{self}, util::bip32, Network, }, descriptor::{DescriptorPublicKey, DescriptorXKey, Wildcard}, policy::compiler::CompilerError, }; + use std::iter::repeat_with; fn rand_xpub( - rng: &mut SmallRng, + rng: &mut fastrand::Rng, secp: &secp256k1::Secp256k1, ) -> bip32::ExtendedPrivKey { - let mut rand_bytes = [0u8; 64]; - - rng.fill_bytes(&mut rand_bytes); + let rand_bytes: Vec = repeat_with(|| rng.u8(..)).take(64).collect(); bip32::ExtendedPrivKey::new_master(Network::Bitcoin, &rand_bytes) .unwrap_or_else(|_| rand_xpub(rng, secp)) } fn get_random_pubkey( - rng: &mut SmallRng, + rng: &mut fastrand::Rng, secp: &secp256k1::Secp256k1, ) -> DescriptorPublicKey { - let mut rand_bytes = [0u8; 64]; - - rng.fill_bytes(&mut rand_bytes); - DescriptorPublicKey::XPub(DescriptorXKey { origin: None, xkey: bip32::ExtendedPubKey::from_private(&secp, &rand_xpub(rng, secp)), @@ -378,7 +370,7 @@ mod tests { ]; let secp = secp256k1::Secp256k1::signing_only(); - let mut rng = SmallRng::from_entropy(); + let mut rng = fastrand::Rng::new(); for ((thresh, n_managers), n_stakeholders) in configurations.iter() { let managers = (0..*n_managers) .map(|_| get_random_pubkey(&mut rng, &secp)) @@ -422,7 +414,7 @@ mod tests { #[test] fn test_default_configuration_limits() { - let mut rng = SmallRng::from_entropy(); + let mut rng = fastrand::Rng::new(); let secp = secp256k1::Secp256k1::signing_only(); assert_eq!( diff --git a/src/transactions.rs b/src/transactions.rs index 63c4161f..e4db93af 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -1494,23 +1494,19 @@ mod tests { }; use crate::{error::*, scripts::*, txins::*, txouts::*}; - use std::str::FromStr; + use std::{iter::repeat_with, str::FromStr}; use miniscript::{ bitcoin::{ - secp256k1, - secp256k1::rand::{rngs::SmallRng, FromEntropy, RngCore}, - util::bip32, - Address, Network, OutPoint, SigHash, SigHashType, Transaction, TxIn, TxOut, + secp256k1, util::bip32, Address, Network, OutPoint, SigHash, SigHashType, Transaction, + TxIn, TxOut, }, descriptor::{DescriptorPublicKey, DescriptorXKey, Wildcard}, Descriptor, DescriptorTrait, }; - fn get_random_privkey(rng: &mut SmallRng) -> bip32::ExtendedPrivKey { - let mut rand_bytes = [0u8; 64]; - - rng.fill_bytes(&mut rand_bytes); + fn get_random_privkey(rng: &mut fastrand::Rng) -> bip32::ExtendedPrivKey { + let rand_bytes: Vec = repeat_with(|| rng.u8(..)).take(64).collect(); bip32::ExtendedPrivKey::new_master(Network::Bitcoin, &rand_bytes) .unwrap_or_else(|_| get_random_privkey(rng)) @@ -1527,7 +1523,7 @@ mod tests { (Vec, Vec), (Vec, Vec), ) { - let mut rng = SmallRng::from_entropy(); + let mut rng = fastrand::Rng::new(); let managers_priv = (0..n_man) .map(|_| get_random_privkey(&mut rng)) @@ -1638,10 +1634,9 @@ mod tests { #[test] fn transaction_derivation() { let secp = secp256k1::Secp256k1::new(); - let mut rng = SmallRng::from_entropy(); - // FIXME: Miniscript mask for sequence check is bugged in this version. Uncomment when upgrading. - // let csv = rng.next_u32() % (1 << 22); - let csv = rng.next_u32() % (1 << 16); + // FIXME: Miniscript mask for sequence check is bugged in this version. + let csv = fastrand::u32(..1 << 16); + eprintln!("Using a CSV of '{}'", csv); // Test the dust limit assert_eq!( @@ -1759,7 +1754,7 @@ mod tests { // The fee-bumping utxo, used in revaulting transactions inputs to bump their feerate. // We simulate a wallet utxo. - let mut rng = SmallRng::from_entropy(); + let mut rng = fastrand::Rng::new(); let feebump_xpriv = get_random_privkey(&mut rng); let feebump_xpub = bip32::ExtendedPubKey::from_private(&secp, &feebump_xpriv); let feebump_descriptor = Descriptor::new_wpkh( From 3aa53bae433908dbfa151ac4ae0d743304fca448 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 2 Apr 2021 13:05:46 +0200 Subject: [PATCH 03/18] scripts: encapsulation for the descriptors Now that we assume their internal state, it makes sense to prevent modifying them under our feets from the outside. Signed-off-by: Antoine Poinsot --- src/scripts.rs | 287 ++++++++++++++++++++++++-------------------- src/transactions.rs | 14 +-- src/txouts.rs | 12 +- 3 files changed, 169 insertions(+), 144 deletions(-) diff --git a/src/scripts.rs b/src/scripts.rs index 753546c2..bcf8ba81 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -31,13 +31,21 @@ macro_rules! impl_descriptor_newtype { ($struct_name:ident, $derived_struct_name:ident, $doc_comment:meta, $der_doc_comment:meta) => { #[$doc_comment] #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] - pub struct $struct_name(pub Descriptor); + pub struct $struct_name(Descriptor); #[$der_doc_comment] #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] - pub struct $derived_struct_name(pub Descriptor); + pub struct $derived_struct_name(Descriptor); impl $struct_name { + pub fn inner(&self) -> &Descriptor { + &self.0 + } + + pub fn into_inner(self) -> Descriptor { + self.0 + } + /// Derives all wildcard keys in the descriptor using the supplied `child_number` pub fn derive( &self, @@ -52,6 +60,16 @@ macro_rules! impl_descriptor_newtype { ) } } + + impl $derived_struct_name { + pub fn inner(&self) -> &Descriptor { + &self.0 + } + + pub fn into_inner(self) -> Descriptor { + self.0 + } + } }; } @@ -82,52 +100,54 @@ impl_descriptor_newtype!( See the [cpfp_descriptor] function for more information." ); -/// Get the xpub miniscript descriptor for the deposit outputs. -/// -/// The deposit policy is an N-of-N, so `thresh(len(all_pubkeys), all_pubkeys)`. -/// -/// # Examples -/// ```rust -/// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; -/// use std::str::FromStr; -/// -/// let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); -/// let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); -/// -/// let deposit_descriptor = -/// scripts::deposit_descriptor(vec![first_stakeholder, second_stakeholder]).expect("Compiling descriptor"); -/// println!("Deposit descriptor: {}", deposit_descriptor.0); -/// -/// let secp = secp256k1::Secp256k1::verification_only(); -/// println!("Tenth child witness script: {}", deposit_descriptor.derive(bip32::ChildNumber::from(10), &secp).0.explicit_script()); -/// ``` -/// -/// # Errors -/// - If the passed slice contains less than 2 public keys. -/// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a -/// bug. -pub fn deposit_descriptor( - participants: Vec, -) -> Result { - if participants.len() < 2 { - return Err(ScriptCreationError::BadParameters); - } +impl DepositDescriptor { + /// Get the xpub miniscript descriptor for the deposit outputs. + /// + /// The deposit policy is an N-of-N, so `thresh(len(all_pubkeys), all_pubkeys)`. + /// + /// # Examples + /// ```rust + /// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; + /// use std::str::FromStr; + /// + /// let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); + /// let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); + /// + /// let deposit_descriptor = + /// scripts::DepositDescriptor::new(vec![first_stakeholder, second_stakeholder]).expect("Compiling descriptor"); + /// println!("Deposit descriptor: {}", deposit_descriptor.inner()); + /// + /// let secp = secp256k1::Secp256k1::verification_only(); + /// println!("Tenth child witness script: {}", deposit_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); + /// ``` + /// + /// # Errors + /// - If the passed slice contains less than 2 public keys. + /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a + /// bug. + pub fn new( + stakeholders: Vec, + ) -> Result { + if stakeholders.len() < 2 { + return Err(ScriptCreationError::BadParameters); + } - let pubkeys = participants - .into_iter() - .map(Policy::Key) - .collect::>>(); + let pubkeys = stakeholders + .into_iter() + .map(Policy::Key) + .collect::>>(); - let policy = Policy::Threshold(pubkeys.len(), pubkeys); + let policy = Policy::Threshold(pubkeys.len(), pubkeys); - // This handles the non-safe or malleable cases. - let ms = policy.compile::()?; - let desc = Descriptor::new_wsh(ms)?; - if !desc.for_each_key(|k| k.as_key().is_deriveable()) { - return Err(ScriptCreationError::NonWildcardKeys); - } + // This handles the non-safe or malleable cases. + let ms = policy.compile::()?; + let desc = Descriptor::new_wsh(ms)?; + if !desc.for_each_key(|k| k.as_key().is_deriveable()) { + return Err(ScriptCreationError::NonWildcardKeys); + } - Ok(DepositDescriptor(desc)) + Ok(DepositDescriptor(desc)) + } } /// Get the miniscript descriptors for the unvault outputs. @@ -152,7 +172,7 @@ pub fn deposit_descriptor( /// let second_manager = DescriptorPublicKey::from_str("xpub6EWL35hY9uZZs5Ljt6J3G2ZK1Tu4GPVkFdeGvMknG3VmwVRHhtadCaw5hdRDBgrmx1nPVHWjGBb5xeuC1BfbJzjjcic2gNm1aA7ywWjj7G8/*").unwrap(); /// /// -/// let unvault_descriptor = scripts::unvault_descriptor( +/// let unvault_descriptor = scripts::UnvaultDescriptor::new( /// vec![first_stakeholder, second_stakeholder, third_stakeholder], /// vec![first_manager, second_manager], /// 1, @@ -161,10 +181,10 @@ pub fn deposit_descriptor( /// // CSV /// 42 /// ).expect("Compiling descriptor"); -/// println!("Unvault descriptor: {}", unvault_descriptor.0); +/// println!("Unvault descriptor: {}", unvault_descriptor.inner()); /// /// let secp = secp256k1::Secp256k1::verification_only(); -/// println!("Tenth child witness script: {}", unvault_descriptor.derive(bip32::ChildNumber::from(10), &secp).0.explicit_script()); +/// println!("Tenth child witness script: {}", unvault_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); /// ``` /// /// # Errors @@ -172,74 +192,76 @@ pub fn deposit_descriptor( /// not the same as the number of cosigners public key. /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a /// bug. -pub fn unvault_descriptor( - stakeholders: Vec, - managers: Vec, - managers_threshold: usize, - cosigners: Vec, - csv_value: u32, -) -> Result { - if stakeholders.is_empty() || managers.is_empty() || cosigners.len() != stakeholders.len() { - return Err(ScriptCreationError::BadParameters); - } +impl UnvaultDescriptor { + pub fn new( + stakeholders: Vec, + managers: Vec, + managers_threshold: usize, + cosigners: Vec, + csv_value: u32, + ) -> Result { + if stakeholders.is_empty() || managers.is_empty() || cosigners.len() != stakeholders.len() { + return Err(ScriptCreationError::BadParameters); + } - if managers_threshold > managers.len() { - return Err(ScriptCreationError::BadParameters); - } + if managers_threshold > managers.len() { + return Err(ScriptCreationError::BadParameters); + } - // Stakeholders' and managers' must be deriveable xpubs. - for key in stakeholders.iter().chain(managers.iter()) { - match key { - DescriptorPublicKey::XPub(xpub) => { - if matches!(xpub.wildcard, Wildcard::None) { + // Stakeholders' and managers' must be deriveable xpubs. + for key in stakeholders.iter().chain(managers.iter()) { + match key { + DescriptorPublicKey::XPub(xpub) => { + if matches!(xpub.wildcard, Wildcard::None) { + return Err(ScriptCreationError::NonWildcardKeys); + } + } + DescriptorPublicKey::SinglePub(_) => { return Err(ScriptCreationError::NonWildcardKeys); } } - DescriptorPublicKey::SinglePub(_) => { - return Err(ScriptCreationError::NonWildcardKeys); - } } - } - // Cosigners' key may not be. We use DescriptorSinglePub for them downstream with static raw - // keys, but it's not hardcoded into the type system there to allow a more generic usage. + // Cosigners' key may not be. We use DescriptorSinglePub for them downstream with static raw + // keys, but it's not hardcoded into the type system there to allow a more generic usage. - // We require the locktime to be in number of blocks, and of course to not be disabled. - // TODO: use rust-miniscript's constants after upgrading! - if (csv_value & (1 << 31)) != 0 || (csv_value & (1 << 22)) != 0 { - return Err(ScriptCreationError::BadParameters); - } + // We require the locktime to be in number of blocks, and of course to not be disabled. + // TODO: use rust-miniscript's constants after upgrading! + if (csv_value & (1 << 31)) != 0 || (csv_value & (1 << 22)) != 0 { + return Err(ScriptCreationError::BadParameters); + } - let mut pubkeys = managers - .into_iter() - .map(Policy::Key) - .collect::>>(); - let spenders_thres = Policy::Threshold(managers_threshold, pubkeys); + let mut pubkeys = managers + .into_iter() + .map(Policy::Key) + .collect::>>(); + let spenders_thres = Policy::Threshold(managers_threshold, pubkeys); - pubkeys = stakeholders - .into_iter() - .map(Policy::Key) - .collect::>>(); - let stakeholders_thres = Policy::Threshold(pubkeys.len(), pubkeys); + pubkeys = stakeholders + .into_iter() + .map(Policy::Key) + .collect::>>(); + let stakeholders_thres = Policy::Threshold(pubkeys.len(), pubkeys); - pubkeys = cosigners - .into_iter() - .map(Policy::Key) - .collect::>>(); - let cosigners_thres = Policy::Threshold(pubkeys.len(), pubkeys); + pubkeys = cosigners + .into_iter() + .map(Policy::Key) + .collect::>>(); + let cosigners_thres = Policy::Threshold(pubkeys.len(), pubkeys); - let cosigners_and_csv = Policy::And(vec![cosigners_thres, Policy::Older(csv_value)]); + let cosigners_and_csv = Policy::And(vec![cosigners_thres, Policy::Older(csv_value)]); - let managers_and_cosigners_and_csv = Policy::And(vec![spenders_thres, cosigners_and_csv]); + let managers_and_cosigners_and_csv = Policy::And(vec![spenders_thres, cosigners_and_csv]); - let policy = Policy::Or(vec![ - (1, stakeholders_thres), - (9, managers_and_cosigners_and_csv), - ]); + let policy = Policy::Or(vec![ + (1, stakeholders_thres), + (9, managers_and_cosigners_and_csv), + ]); - // This handles the non-safe or malleable cases. - let ms = policy.compile::()?; + // This handles the non-safe or malleable cases. + let ms = policy.compile::()?; - Ok(UnvaultDescriptor(Descriptor::new_wsh(ms)?)) + Ok(UnvaultDescriptor(Descriptor::new_wsh(ms)?)) + } } /// Get the miniscript descriptor for the unvault transaction CPFP output. @@ -249,24 +271,24 @@ pub fn unvault_descriptor( /// # Errors /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a /// bug. -pub fn cpfp_descriptor( - managers: Vec, -) -> Result { - let pubkeys = managers - .into_iter() - .map(Policy::Key) - .collect::>>(); - - let policy = Policy::Threshold(1, pubkeys); - - // This handles the non-safe or malleable cases. - let ms = policy.compile::()?; - let desc = Descriptor::new_wsh(ms)?; - if !desc.for_each_key(|k| k.as_key().is_deriveable()) { - return Err(ScriptCreationError::NonWildcardKeys); - } +impl CpfpDescriptor { + pub fn new(managers: Vec) -> Result { + let pubkeys = managers + .into_iter() + .map(Policy::Key) + .collect::>>(); + + let policy = Policy::Threshold(1, pubkeys); + + // This handles the non-safe or malleable cases. + let ms = policy.compile::()?; + let desc = Descriptor::new_wsh(ms)?; + if !desc.for_each_key(|k| k.as_key().is_deriveable()) { + return Err(ScriptCreationError::NonWildcardKeys); + } - Ok(CpfpDescriptor(desc)) + Ok(CpfpDescriptor(desc)) + } } /// The "emergency address", it's kept obfuscated for the entire duration of the vault and is @@ -313,7 +335,8 @@ impl<'de> de::Deserialize<'de> for EmergencyAddress { #[cfg(test)] mod tests { - use super::{cpfp_descriptor, deposit_descriptor, unvault_descriptor, ScriptCreationError}; + + use super::{CpfpDescriptor, DepositDescriptor, ScriptCreationError, UnvaultDescriptor}; use miniscript::{ bitcoin::{ @@ -382,7 +405,7 @@ mod tests { .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); - unvault_descriptor( + UnvaultDescriptor::new( stakeholders.clone(), managers.clone(), *thresh, @@ -393,7 +416,7 @@ mod tests { "Unvault descriptors creation error with ({}, {})", n_managers, n_stakeholders )); - deposit_descriptor( + DepositDescriptor::new( managers .clone() .iter() @@ -405,7 +428,7 @@ mod tests { "Deposit descriptors creation error with ({}, {})", n_managers, n_stakeholders )); - cpfp_descriptor(managers).expect(&format!( + CpfpDescriptor::new(managers).expect(&format!( "CPFP descriptors creation error with ({}, {})", n_managers, n_stakeholders )); @@ -418,14 +441,14 @@ mod tests { let secp = secp256k1::Secp256k1::signing_only(); assert_eq!( - deposit_descriptor(vec![get_random_pubkey(&mut rng, &secp)]) + DepositDescriptor::new(vec![get_random_pubkey(&mut rng, &secp)]) .unwrap_err() .to_string(), ScriptCreationError::BadParameters.to_string() ); assert_eq!( - unvault_descriptor( + UnvaultDescriptor::new( vec![get_random_pubkey(&mut rng, &secp)], vec![get_random_pubkey(&mut rng, &secp)], 1, @@ -441,7 +464,7 @@ mod tests { ); assert_eq!( - unvault_descriptor( + UnvaultDescriptor::new( vec![get_random_pubkey(&mut rng, &secp)], vec![get_random_pubkey(&mut rng, &secp)], 1, @@ -454,7 +477,7 @@ mod tests { ); assert_eq!( - unvault_descriptor( + UnvaultDescriptor::new( vec![get_random_pubkey(&mut rng, &secp)], vec![get_random_pubkey(&mut rng, &secp)], 2, @@ -470,13 +493,15 @@ mod tests { let participants = (0..99) .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); - deposit_descriptor(participants).expect("Should be OK: max allowed value"); + DepositDescriptor::new(participants).expect("Should be OK: max allowed value"); // Now hit the limit let participants = (0..100) .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); assert_eq!( - deposit_descriptor(participants).unwrap_err().to_string(), + DepositDescriptor::new(participants) + .unwrap_err() + .to_string(), ScriptCreationError::PolicyCompilation(CompilerError::LimitsExceeded).to_string() ); @@ -484,13 +509,13 @@ mod tests { let managers = (0..20) .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); - cpfp_descriptor(managers).expect("Should be OK, that's the maximum allowed value"); + CpfpDescriptor::new(managers).expect("Should be OK, that's the maximum allowed value"); // Hit the limit let managers = (0..21) .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); assert_eq!( - cpfp_descriptor(managers).unwrap_err().to_string(), + CpfpDescriptor::new(managers).unwrap_err().to_string(), ScriptCreationError::PolicyCompilation(CompilerError::LimitsExceeded).to_string() ); @@ -504,7 +529,7 @@ mod tests { let cosigners = (0..38) .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); - unvault_descriptor(stakeholders, managers, 2, cosigners, 145).unwrap(); + UnvaultDescriptor::new(stakeholders, managers, 2, cosigners, 145).unwrap(); // Now hit the limit let stakeholders = (0..39) @@ -517,7 +542,7 @@ mod tests { .map(|_| get_random_pubkey(&mut rng, &secp)) .collect::>(); assert_eq!( - unvault_descriptor(stakeholders, managers, 2, cosigners, 32) + UnvaultDescriptor::new(stakeholders, managers, 2, cosigners, 32) .unwrap_err() .to_string(), ScriptCreationError::PolicyCompilation(CompilerError::LimitsExceeded).to_string() diff --git a/src/transactions.rs b/src/transactions.rs index e4db93af..be174c66 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -698,7 +698,7 @@ impl UnvaultTransaction { unvault_descriptor: &DerivedUnvaultDescriptor, sequence: u32, ) -> UnvaultTxIn { - let spk = unvault_descriptor.0.script_pubkey(); + let spk = unvault_descriptor.inner().script_pubkey(); let index = self .inner_tx() .global @@ -740,7 +740,7 @@ impl UnvaultTransaction { /// Get the CPFP txo to be referenced in a spending transaction pub fn cpfp_txin(&self, cpfp_descriptor: &DerivedCpfpDescriptor) -> CpfpTxIn { - let spk = cpfp_descriptor.0.script_pubkey(); + let spk = cpfp_descriptor.inner().script_pubkey(); let index = self .inner_tx() .global @@ -1685,7 +1685,7 @@ mod tests { ) = get_participants_sets(n_stk, n_man, secp); // Get the script descriptors for the txos we're going to create - let unvault_descriptor = unvault_descriptor( + let unvault_descriptor = UnvaultDescriptor::new( stakeholders.clone(), managers.clone(), managers.len(), @@ -1694,15 +1694,15 @@ mod tests { ) .expect("Unvault descriptor generation error"); let cpfp_descriptor = - cpfp_descriptor(managers).expect("Unvault CPFP descriptor generation error"); + CpfpDescriptor::new(managers).expect("Unvault CPFP descriptor generation error"); let deposit_descriptor = - deposit_descriptor(stakeholders).expect("Deposit descriptor generation error"); + DepositDescriptor::new(stakeholders).expect("Deposit descriptor generation error"); // We reuse the deposit descriptor for the emergency address let emergency_address = EmergencyAddress::from(Address::p2wsh( &deposit_descriptor .derive(child_number, secp) - .0 + .inner() .explicit_script(), Network::Bitcoin, )) @@ -1713,7 +1713,7 @@ mod tests { let der_cpfp_descriptor = cpfp_descriptor.derive(child_number, secp); // The funding transaction does not matter (random txid from my mempool) - let deposit_scriptpubkey = der_deposit_descriptor.0.script_pubkey(); + let deposit_scriptpubkey = der_deposit_descriptor.inner().script_pubkey(); let deposit_raw_tx = Transaction { version: 2, lock_time: 0, diff --git a/src/txouts.rs b/src/txouts.rs index 14f826ea..3b8a1fc9 100644 --- a/src/txouts.rs +++ b/src/txouts.rs @@ -67,9 +67,9 @@ impl DepositTxOut { DepositTxOut { txout: TxOut { value, - script_pubkey: script_descriptor.0.script_pubkey(), + script_pubkey: script_descriptor.inner().script_pubkey(), }, - witness_script: Some(script_descriptor.0.explicit_script()), + witness_script: Some(script_descriptor.inner().explicit_script()), } } } @@ -81,9 +81,9 @@ impl UnvaultTxOut { UnvaultTxOut { txout: TxOut { value, - script_pubkey: script_descriptor.0.script_pubkey(), + script_pubkey: script_descriptor.inner().script_pubkey(), }, - witness_script: Some(script_descriptor.0.explicit_script()), + witness_script: Some(script_descriptor.inner().explicit_script()), } } } @@ -115,9 +115,9 @@ impl CpfpTxOut { CpfpTxOut { txout: TxOut { value, - script_pubkey: script_descriptor.0.script_pubkey(), + script_pubkey: script_descriptor.inner().script_pubkey(), }, - witness_script: Some(script_descriptor.0.explicit_script()), + witness_script: Some(script_descriptor.inner().explicit_script()), } } } From b0be2525a3281d4a4c25d6d826b01d332526a9ca Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sat, 3 Apr 2021 15:33:44 +0200 Subject: [PATCH 04/18] scripts: add constructors for derived descriptors type Quite redundant, so muh meta programming Signed-off-by: Antoine Poinsot --- src/scripts.rs | 566 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 431 insertions(+), 135 deletions(-) diff --git a/src/scripts.rs b/src/scripts.rs index bcf8ba81..4c1a5afb 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -17,7 +17,7 @@ use miniscript::{ bitcoin::{secp256k1, util::bip32, Address, PublicKey}, descriptor::{DescriptorPublicKey, Wildcard}, policy::concrete::Policy, - Descriptor, ForEachKey, Segwitv0, TranslatePk2, + Descriptor, Segwitv0, TranslatePk2, }; use std::fmt; @@ -100,8 +100,108 @@ impl_descriptor_newtype!( See the [cpfp_descriptor] function for more information." ); +macro_rules! deposit_desc_checks { + ($stakeholders:ident) => { + if $stakeholders.len() < 2 { + return Err(ScriptCreationError::BadParameters); + } + }; +} + +macro_rules! deposit_desc { + ($stakeholders:ident) => {{ + let pubkeys = $stakeholders + .into_iter() + .map(Policy::Key) + .collect::>>(); + + let policy = Policy::Threshold(pubkeys.len(), pubkeys); + + // This handles the non-safe or malleable cases. + let ms = policy.compile::()?; + Descriptor::new_wsh(ms)? + }}; +} + +macro_rules! unvault_desc_checks { + ($stakeholders:ident,$managers:ident, $managers_threshold:ident, $cosigners:ident, $csv_value:ident) => { + if $stakeholders.is_empty() + || $managers.is_empty() + || $cosigners.len() != $stakeholders.len() + { + return Err(ScriptCreationError::BadParameters); + } + + if $managers_threshold > $managers.len() { + return Err(ScriptCreationError::BadParameters); + } + + // We require the locktime to be in number of blocks, and of course to not be disabled. + // TODO: use rust-miniscript's constants after upgrading! + if ($csv_value & (1 << 31)) != 0 || ($csv_value & (1 << 22)) != 0 { + return Err(ScriptCreationError::BadParameters); + } + }; +} + +macro_rules! unvault_desc { + ($stakeholders:ident, $managers:ident, $managers_threshold:ident, $cosigners:ident, $csv_value:ident) => {{ + let mut pubkeys = $managers + .into_iter() + .map(Policy::Key) + .collect::>>(); + let spenders_thres = Policy::Threshold($managers_threshold, pubkeys); + + pubkeys = $stakeholders + .into_iter() + .map(Policy::Key) + .collect::>>(); + let stakeholders_thres = Policy::Threshold(pubkeys.len(), pubkeys); + + pubkeys = $cosigners + .into_iter() + .map(Policy::Key) + .collect::>>(); + let cosigners_thres = Policy::Threshold(pubkeys.len(), pubkeys); + + let cosigners_and_csv = Policy::And(vec![cosigners_thres, Policy::Older($csv_value)]); + + let managers_and_cosigners_and_csv = Policy::And(vec![spenders_thres, cosigners_and_csv]); + + let policy = Policy::Or(vec![ + (1, stakeholders_thres), + (9, managers_and_cosigners_and_csv), + ]); + + // This handles the non-safe or malleable cases. + let ms = policy.compile::()?; + + Descriptor::new_wsh(ms)? + }}; +} + +// Check all xpubs contain a wildcard +fn check_deriveable<'a>( + keys: impl Iterator, +) -> Result<(), ScriptCreationError> { + for key in keys { + match key { + DescriptorPublicKey::XPub(xpub) => { + if matches!(xpub.wildcard, Wildcard::None) { + return Err(ScriptCreationError::NonWildcardKeys); + } + } + DescriptorPublicKey::SinglePub(_) => { + return Err(ScriptCreationError::NonWildcardKeys); + } + } + } + + Ok(()) +} + impl DepositDescriptor { - /// Get the xpub miniscript descriptor for the deposit outputs. + /// Get the xpub miniscript descriptor for deposit outputs. /// /// The deposit policy is an N-of-N, so `thresh(len(all_pubkeys), all_pubkeys)`. /// @@ -122,77 +222,95 @@ impl DepositDescriptor { /// ``` /// /// # Errors - /// - If the passed slice contains less than 2 public keys. + /// - If the given `DescriptorPublickKey`s are not wildcards (can be derived from). + /// - If the given vector contains less than 2 public keys. /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a /// bug. pub fn new( stakeholders: Vec, ) -> Result { - if stakeholders.len() < 2 { - return Err(ScriptCreationError::BadParameters); - } - - let pubkeys = stakeholders - .into_iter() - .map(Policy::Key) - .collect::>>(); + deposit_desc_checks!(stakeholders); + check_deriveable(stakeholders.iter())?; - let policy = Policy::Threshold(pubkeys.len(), pubkeys); + Ok(DepositDescriptor(deposit_desc!(stakeholders))) + } +} - // This handles the non-safe or malleable cases. - let ms = policy.compile::()?; - let desc = Descriptor::new_wsh(ms)?; - if !desc.for_each_key(|k| k.as_key().is_deriveable()) { - return Err(ScriptCreationError::NonWildcardKeys); - } +impl DerivedDepositDescriptor { + /// Get the derived miniscript descriptor for deposit outputs. + /// + /// The deposit policy is an N-of-N, so `thresh(len(all_pubkeys), all_pubkeys)`. + /// + /// # Examples + /// ```rust + /// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32, PublicKey}, DescriptorTrait}}; + /// use std::str::FromStr; + /// + /// let first_stakeholder = PublicKey::from_str("02a17786aca5ea2118e9209702454ab432d5b2c656f8ae19447d4ff3e7317d3b41").unwrap(); + /// let second_stakeholder = PublicKey::from_str("036edaec85bb1eee1a19ca9f9fd5620134ec98bc21cc14c4e8e3d0f8f121e1b6d1").unwrap(); + /// + /// let deposit_descriptor = + /// scripts::DerivedDepositDescriptor::new(vec![first_stakeholder, second_stakeholder]).expect("Compiling descriptor"); + /// println!("Concrete deposit descriptor: {}", deposit_descriptor.inner()); + /// ``` + /// + /// # Errors + /// - If the given vector contains less than 2 public keys. + /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a + /// bug. + pub fn new( + stakeholders: Vec, + ) -> Result { + deposit_desc_checks!(stakeholders); - Ok(DepositDescriptor(desc)) + Ok(DerivedDepositDescriptor(deposit_desc!(stakeholders))) } } -/// Get the miniscript descriptors for the unvault outputs. -/// -/// The unvault policy allows either all the stakeholders to spend, or (the fund managers + the cosigners) -/// after a timelock. -/// -/// # Examples -/// ```rust -/// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; -/// use std::str::FromStr; -/// -/// let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); -/// let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); -/// let third_stakeholder = DescriptorPublicKey::from_str("xpub6Br1DUfrzxTVGo1sanuKDCUmSxDfLRrxLQBqpMqygkQLkQWodoyvvGtUV8Rp3r6d6BNYvedBSU8c7whhn2U8haRVxsWwuQiZ9LoFp7jXPQA/*").unwrap(); -/// -/// let first_cosig = DescriptorPublicKey::from_str("02a489e0ea42b56148d212d325b7c67c6460483ff931c303ea311edfef667c8f35").unwrap(); -/// let second_cosig = DescriptorPublicKey::from_str("02767e6dde4877dcbf64de8a45fe1a0575dfc6b0ed06648f1022412c172ebd875c").unwrap(); -/// let third_cosig = DescriptorPublicKey::from_str("0371cdea381b365ea159a3cf4f14029d1bff5b36b4cf12ac9e42be6955d2ed4ecf").unwrap(); -/// -/// let first_manager = DescriptorPublicKey::from_str("xpub6Duq1ob3cQ8Wxees2fTGNK2wTsVjgTPQcKJiPquXY2rQJTDjeCxkXFxTCGhcunFDt26Ddz45KQu7pbLmmUGG2PXTRVx3iDpBPEhdrijJf4U/*").unwrap(); -/// let second_manager = DescriptorPublicKey::from_str("xpub6EWL35hY9uZZs5Ljt6J3G2ZK1Tu4GPVkFdeGvMknG3VmwVRHhtadCaw5hdRDBgrmx1nPVHWjGBb5xeuC1BfbJzjjcic2gNm1aA7ywWjj7G8/*").unwrap(); -/// -/// -/// let unvault_descriptor = scripts::UnvaultDescriptor::new( -/// vec![first_stakeholder, second_stakeholder, third_stakeholder], -/// vec![first_manager, second_manager], -/// 1, -/// // Cosigners -/// vec![first_cosig, second_cosig, third_cosig], -/// // CSV -/// 42 -/// ).expect("Compiling descriptor"); -/// println!("Unvault descriptor: {}", unvault_descriptor.inner()); -/// -/// let secp = secp256k1::Secp256k1::verification_only(); -/// println!("Tenth child witness script: {}", unvault_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); -/// ``` -/// -/// # Errors -/// - If any of the slice contains no public key, or if the number of non_managers public keys is -/// not the same as the number of cosigners public key. -/// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a -/// bug. impl UnvaultDescriptor { + /// Get the miniscript descriptors for Unvault outputs. + /// + /// The Unvault policy allows either all the stakeholders to spend, or (the fund managers + the cosigners) + /// after a timelock. + /// + /// # Examples + /// ```rust + /// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; + /// use std::str::FromStr; + /// + /// let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); + /// let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); + /// let third_stakeholder = DescriptorPublicKey::from_str("xpub6Br1DUfrzxTVGo1sanuKDCUmSxDfLRrxLQBqpMqygkQLkQWodoyvvGtUV8Rp3r6d6BNYvedBSU8c7whhn2U8haRVxsWwuQiZ9LoFp7jXPQA/*").unwrap(); + /// + /// let first_cosig = DescriptorPublicKey::from_str("02a489e0ea42b56148d212d325b7c67c6460483ff931c303ea311edfef667c8f35").unwrap(); + /// let second_cosig = DescriptorPublicKey::from_str("02767e6dde4877dcbf64de8a45fe1a0575dfc6b0ed06648f1022412c172ebd875c").unwrap(); + /// let third_cosig = DescriptorPublicKey::from_str("0371cdea381b365ea159a3cf4f14029d1bff5b36b4cf12ac9e42be6955d2ed4ecf").unwrap(); + /// + /// let first_manager = DescriptorPublicKey::from_str("xpub6Duq1ob3cQ8Wxees2fTGNK2wTsVjgTPQcKJiPquXY2rQJTDjeCxkXFxTCGhcunFDt26Ddz45KQu7pbLmmUGG2PXTRVx3iDpBPEhdrijJf4U/*").unwrap(); + /// let second_manager = DescriptorPublicKey::from_str("xpub6EWL35hY9uZZs5Ljt6J3G2ZK1Tu4GPVkFdeGvMknG3VmwVRHhtadCaw5hdRDBgrmx1nPVHWjGBb5xeuC1BfbJzjjcic2gNm1aA7ywWjj7G8/*").unwrap(); + /// + /// + /// let unvault_descriptor = scripts::UnvaultDescriptor::new( + /// vec![first_stakeholder, second_stakeholder, third_stakeholder], + /// vec![first_manager, second_manager], + /// 1, + /// // Cosigners + /// vec![first_cosig, second_cosig, third_cosig], + /// // CSV + /// 42 + /// ).expect("Compiling descriptor"); + /// println!("Unvault descriptor: {}", unvault_descriptor.inner()); + /// + /// let secp = secp256k1::Secp256k1::verification_only(); + /// println!("Tenth child witness script: {}", unvault_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); + /// ``` + /// + /// # Errors + /// - If the given `DescriptorPublickKey`s are not wildcards (can be derived from). + /// - If any of the slice contains no public key, or if the number of non_managers public keys is + /// not the same as the number of cosigners public key. + /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a + /// bug. pub fn new( stakeholders: Vec, managers: Vec, @@ -200,94 +318,165 @@ impl UnvaultDescriptor { cosigners: Vec, csv_value: u32, ) -> Result { - if stakeholders.is_empty() || managers.is_empty() || cosigners.len() != stakeholders.len() { - return Err(ScriptCreationError::BadParameters); - } - - if managers_threshold > managers.len() { - return Err(ScriptCreationError::BadParameters); - } + unvault_desc_checks!( + stakeholders, + managers, + managers_threshold, + cosigners, + csv_value + ); // Stakeholders' and managers' must be deriveable xpubs. - for key in stakeholders.iter().chain(managers.iter()) { - match key { - DescriptorPublicKey::XPub(xpub) => { - if matches!(xpub.wildcard, Wildcard::None) { - return Err(ScriptCreationError::NonWildcardKeys); - } - } - DescriptorPublicKey::SinglePub(_) => { - return Err(ScriptCreationError::NonWildcardKeys); - } - } - } + check_deriveable(stakeholders.iter().chain(managers.iter()))?; + // Cosigners' key may not be. We use DescriptorSinglePub for them downstream with static raw // keys, but it's not hardcoded into the type system there to allow a more generic usage. - // We require the locktime to be in number of blocks, and of course to not be disabled. - // TODO: use rust-miniscript's constants after upgrading! - if (csv_value & (1 << 31)) != 0 || (csv_value & (1 << 22)) != 0 { - return Err(ScriptCreationError::BadParameters); - } + Ok(UnvaultDescriptor(unvault_desc!( + stakeholders, + managers, + managers_threshold, + cosigners, + csv_value + ))) + } +} - let mut pubkeys = managers - .into_iter() - .map(Policy::Key) - .collect::>>(); - let spenders_thres = Policy::Threshold(managers_threshold, pubkeys); +impl DerivedUnvaultDescriptor { + /// Get the miniscript descriptors for Unvault outputs. + /// + /// The Unvault policy allows either all the stakeholders to spend, or (the fund managers + the cosigners) + /// after a timelock. + /// + /// # Examples + /// ```rust + /// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32, PublicKey}, DescriptorTrait}}; + /// use std::str::FromStr; + /// + /// let first_stakeholder = PublicKey::from_str("0372f4bb19ecf98d7849148b4f40375d2fcef624a1b56fef94489ad012bc11b4df").unwrap(); + /// let second_stakeholder = PublicKey::from_str("036e7ac7a096270f676b53e9917942cf42c6fb9607e3bc09775b5209c908525e80").unwrap(); + /// let third_stakeholder = PublicKey::from_str("03a02e93cf8c47b250075b0af61f96ebd10376c0aaa7635148e889cb2b51c96927").unwrap(); + /// + /// let first_cosig = PublicKey::from_str("02a489e0ea42b56148d212d325b7c67c6460483ff931c303ea311edfef667c8f35").unwrap(); + /// let second_cosig = PublicKey::from_str("02767e6dde4877dcbf64de8a45fe1a0575dfc6b0ed06648f1022412c172ebd875c").unwrap(); + /// let third_cosig = PublicKey::from_str("0371cdea381b365ea159a3cf4f14029d1bff5b36b4cf12ac9e42be6955d2ed4ecf").unwrap(); + /// + /// let first_manager = PublicKey::from_str("03d33a510c0376a3d19ffa0e1ba71d5ee0cbfebbce2df0996b51262142e943c6f0").unwrap(); + /// let second_manager = PublicKey::from_str("030e7d7e1d8014dc17d63057ffc3ef26590bf237ce50054fb4f612be8e0a0dbe2a").unwrap(); + /// + /// + /// let unvault_descriptor = scripts::DerivedUnvaultDescriptor::new( + /// vec![first_stakeholder, second_stakeholder, third_stakeholder], + /// vec![first_manager, second_manager], + /// 1, + /// // Cosigners + /// vec![first_cosig, second_cosig, third_cosig], + /// // CSV + /// 42 + /// ).expect("Compiling descriptor"); + /// println!("Unvault descriptor: {}", unvault_descriptor.inner()); + /// ``` + /// + /// # Errors + /// - If any of the given vectors contains no public key, or if the number of stakeholders public keys + /// is not the same as the number of cosigners public keys. + /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a + /// bug. + pub fn new( + stakeholders: Vec, + managers: Vec, + managers_threshold: usize, + cosigners: Vec, + csv_value: u32, + ) -> Result { + unvault_desc_checks!( + stakeholders, + managers, + managers_threshold, + cosigners, + csv_value + ); - pubkeys = stakeholders - .into_iter() - .map(Policy::Key) - .collect::>>(); - let stakeholders_thres = Policy::Threshold(pubkeys.len(), pubkeys); + Ok(DerivedUnvaultDescriptor(unvault_desc!( + stakeholders, + managers, + managers_threshold, + cosigners, + csv_value + ))) + } +} - pubkeys = cosigners +macro_rules! cpfp_descriptor { + ($managers: ident) => {{ + let pubkeys = $managers .into_iter() .map(Policy::Key) - .collect::>>(); - let cosigners_thres = Policy::Threshold(pubkeys.len(), pubkeys); - - let cosigners_and_csv = Policy::And(vec![cosigners_thres, Policy::Older(csv_value)]); + .collect::>>(); - let managers_and_cosigners_and_csv = Policy::And(vec![spenders_thres, cosigners_and_csv]); - - let policy = Policy::Or(vec![ - (1, stakeholders_thres), - (9, managers_and_cosigners_and_csv), - ]); + let policy = Policy::Threshold(1, pubkeys); // This handles the non-safe or malleable cases. let ms = policy.compile::()?; - - Ok(UnvaultDescriptor(Descriptor::new_wsh(ms)?)) - } + Descriptor::new_wsh(ms)? + }}; } -/// Get the miniscript descriptor for the unvault transaction CPFP output. -/// -/// It's a basic 1-of-N between the fund managers. -/// -/// # Errors -/// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a -/// bug. impl CpfpDescriptor { + /// Get the miniscript descriptor for the Unvault transaction CPFP output. + /// + /// It's a basic 1-of-N between the fund managers. + /// + /// # Examples + /// ```rust + /// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32}, DescriptorPublicKey, DescriptorTrait}}; + /// use std::str::FromStr; + /// + /// let first_manager = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); + /// let second_manager = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); + /// + /// let cpfp_descriptor = + /// scripts::CpfpDescriptor::new(vec![first_manager, second_manager]).expect("Compiling descriptor"); + /// println!("CPFP descriptor: {}", cpfp_descriptor.inner()); + /// + /// let secp = secp256k1::Secp256k1::verification_only(); + /// println!("Tenth child witness script: {}", cpfp_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); + /// ``` + /// + /// # Errors + /// - If the given `DescriptorPublickKey`s are not wildcards (can be derived from). + /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a + /// bug. pub fn new(managers: Vec) -> Result { - let pubkeys = managers - .into_iter() - .map(Policy::Key) - .collect::>>(); + check_deriveable(managers.iter())?; - let policy = Policy::Threshold(1, pubkeys); - - // This handles the non-safe or malleable cases. - let ms = policy.compile::()?; - let desc = Descriptor::new_wsh(ms)?; - if !desc.for_each_key(|k| k.as_key().is_deriveable()) { - return Err(ScriptCreationError::NonWildcardKeys); - } + Ok(CpfpDescriptor(cpfp_descriptor!(managers))) + } +} - Ok(CpfpDescriptor(desc)) +impl DerivedCpfpDescriptor { + /// Get the miniscript descriptor for the Unvault transaction CPFP output. + /// + /// It's a basic 1-of-N between the fund managers. + /// + /// # Examples + /// ```rust + /// use revault_tx::{scripts, miniscript::{bitcoin::{self, secp256k1, util::bip32, PublicKey}, DescriptorTrait}}; + /// use std::str::FromStr; + /// + /// let first_manager = PublicKey::from_str("02a17786aca5ea2118e9209702454ab432d5b2c656f8ae19447d4ff3e7317d3b41").unwrap(); + /// let second_manager = PublicKey::from_str("036edaec85bb1eee1a19ca9f9fd5620134ec98bc21cc14c4e8e3d0f8f121e1b6d1").unwrap(); + /// + /// let cpfp_descriptor = + /// scripts::DerivedCpfpDescriptor::new(vec![first_manager, second_manager]).expect("Compiling descriptor"); + /// println!("Concrete CPFP descriptor: {}", cpfp_descriptor.inner()); + /// ``` + /// + /// # Errors + /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a + /// bug. + pub fn new(managers: Vec) -> Result { + Ok(DerivedCpfpDescriptor(cpfp_descriptor!(managers))) } } @@ -339,15 +528,11 @@ mod tests { use super::{CpfpDescriptor, DepositDescriptor, ScriptCreationError, UnvaultDescriptor}; use miniscript::{ - bitcoin::{ - secp256k1::{self}, - util::bip32, - Network, - }, + bitcoin::{secp256k1, util::bip32, Network}, descriptor::{DescriptorPublicKey, DescriptorXKey, Wildcard}, policy::compiler::CompilerError, }; - use std::iter::repeat_with; + use std::{iter::repeat_with, str::FromStr}; fn rand_xpub( rng: &mut fastrand::Rng, @@ -371,6 +556,117 @@ mod tests { }) } + // Sanity check we error on creating derived descriptors. Non-error cases are in doc comments. + #[test] + fn sanity_check_desc_creation() { + let first_stakeholder = DescriptorPublicKey::from_str("xpub6EHLFGpTTiZgHAHfBJ1LoepGFX5iyLeZ6CVtF9HhzeB1dkxLsEfkiJda78EKhSXuo2m8gQwAs4ZAbqaJixFYHMFWTL9DJX1KsAXS2VY5JJx/*").unwrap(); + let second_stakeholder = DescriptorPublicKey::from_str("xpub6F2U61Uh9FNX94mZE6EgdZ3p5Wg8af6MHzFhskEskkAZ9ns2uvsnHBskU47wYY63yiYv8WufvTuHCePwUjK9zhKT1Cce8JGLBptncpvALw6/*").unwrap(); + let third_stakeholder = DescriptorPublicKey::from_str("xpub6Br1DUfrzxTVGo1sanuKDCUmSxDfLRrxLQBqpMqygkQLkQWodoyvvGtUV8Rp3r6d6BNYvedBSU8c7whhn2U8haRVxsWwuQiZ9LoFp7jXPQA/*").unwrap(); + + let first_cosig = DescriptorPublicKey::from_str( + "02a489e0ea42b56148d212d325b7c67c6460483ff931c303ea311edfef667c8f35", + ) + .unwrap(); + let second_cosig = DescriptorPublicKey::from_str( + "02767e6dde4877dcbf64de8a45fe1a0575dfc6b0ed06648f1022412c172ebd875c", + ) + .unwrap(); + let third_cosig = DescriptorPublicKey::from_str( + "0371cdea381b365ea159a3cf4f14029d1bff5b36b4cf12ac9e42be6955d2ed4ecf", + ) + .unwrap(); + + let first_manager = DescriptorPublicKey::from_str("xpub6Duq1ob3cQ8Wxees2fTGNK2wTsVjgTPQcKJiPquXY2rQJTDjeCxkXFxTCGhcunFDt26Ddz45KQu7pbLmmUGG2PXTRVx3iDpBPEhdrijJf4U/*").unwrap(); + let second_manager = DescriptorPublicKey::from_str("xpub6EWL35hY9uZZs5Ljt6J3G2ZK1Tu4GPVkFdeGvMknG3VmwVRHhtadCaw5hdRDBgrmx1nPVHWjGBb5xeuC1BfbJzjjcic2gNm1aA7ywWjj7G8/*").unwrap(); + + // When a single xpub isn't deriveable + let invalid_stk = DescriptorPublicKey::from_str("xpub6Br1DUfrzxTVGo1sanuKDCUmSxDfLRrxLQBqpMqygkQLkQWodoyvvGtUV8Rp3r6d6BNYvedBSU8c7whhn2U8haRVxsWwuQiZ9LoFp7jXPQA").unwrap(); + DepositDescriptor::new(vec![ + first_stakeholder.clone(), + second_stakeholder.clone(), + invalid_stk.clone(), + ]) + .expect_err("Accepting a non wildcard xpub"); + DepositDescriptor::new(vec![ + first_stakeholder.clone(), + first_cosig.clone(), // A derived key + invalid_stk.clone(), + ]) + .expect_err("Accepting a non wildcard xpub"); + + let invalid_man = DescriptorPublicKey::from_str("xpub6EWL35hY9uZZs5Ljt6J3G2ZK1Tu4GPVkFdeGvMknG3VmwVRHhtadCaw5hdRDBgrmx1nPVHWjGBb5xeuC1BfbJzjjcic2gNm1aA7ywWjj7G8").unwrap(); + CpfpDescriptor::new(vec![ + first_manager.clone(), + second_manager.clone(), + invalid_man.clone(), + ]) + .expect_err("Accepting a non wildcard xpub"); + + UnvaultDescriptor::new( + vec![ + first_stakeholder.clone(), + second_stakeholder.clone(), + invalid_stk.clone(), + ], + vec![first_manager.clone(), second_manager.clone()], + 1, + vec![ + first_cosig.clone(), + second_cosig.clone(), + third_cosig.clone(), + ], + 128, + ) + .expect_err("Accepting a non wildcard stakeholder xpub"); + UnvaultDescriptor::new( + vec![ + first_stakeholder.clone(), + second_stakeholder.clone(), + third_stakeholder, + ], + vec![first_manager.clone(), invalid_man], + 1, + vec![first_cosig, second_cosig, third_cosig], + 128, + ) + .expect_err("Accepting a non wildcard manager xpub"); + + // But for cosigning servers it's fine + let first_cosig = DescriptorPublicKey::from_str( + "xpub6Da8z6vMdBgtfZraAEjruVSyASFbrWqSm724PPbnezQidGH5wVavF6xFKrbpGCC4VtDVnLP5J5NXm8c8do9zC6MRPkgEsxt4oPY7dukETw2", + ) + .unwrap(); + let second_cosig = DescriptorPublicKey::from_str( + "xpub6Cp57dqxsjzveK5XQYJmzRrofaMJLUC3zQjwNNKKWB9kPn1YtUrrPMXxXGQjs9r2RRQ7e9vExWLJinTZmaosezisGG9nTwEVV15iFQYzFfa", + ) + .unwrap(); + UnvaultDescriptor::new( + vec![first_stakeholder.clone(), second_stakeholder.clone()], + vec![first_manager.clone(), second_manager.clone()], + 1, + vec![first_cosig, second_cosig], + 128, + ) + .expect("Refusing a non wildcard cosigning server xpub"); + + let first_cosig = DescriptorPublicKey::from_str( + "xpub6Da8z6vMdBgtfZraAEjruVSyASFbrWqSm724PPbnezQidGH5wVavF6xFKrbpGCC4VtDVnLP5J5NXm8c8do9zC6MRPkgEsxt4oPY7dukETw2/*", + ) + .unwrap(); + let second_cosig = DescriptorPublicKey::from_str( + "xpub6Cp57dqxsjzveK5XQYJmzRrofaMJLUC3zQjwNNKKWB9kPn1YtUrrPMXxXGQjs9r2RRQ7e9vExWLJinTZmaosezisGG9nTwEVV15iFQYzFfa/*", + ) + .unwrap(); + UnvaultDescriptor::new( + vec![first_stakeholder.clone(), second_stakeholder.clone()], + vec![first_manager.clone(), second_manager.clone()], + 1, + vec![first_cosig, second_cosig], + 128, + ) + .expect("Refusing a wildcard cosigning server xpub"); + } + #[test] fn test_possible_default_configurations() { // Policy compilation takes time, so just test some remarkable ones From ca77be204f176e5307d92102e262a8f1fe3f5b73 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sun, 4 Apr 2021 09:24:41 +0200 Subject: [PATCH 05/18] scripts: implement to/from str for descriptors newtypes We can't really check the sanity wrt to our policy of a compiled descriptor, so this is a best effort version in order to avoid mistakes such as eg using `DepositDescriptor::from_str` on a `DerivedDescriptor::to_string` which would result in a crash at derivation. Signed-off-by: Antoine Poinsot --- src/scripts.rs | 244 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 227 insertions(+), 17 deletions(-) diff --git a/src/scripts.rs b/src/scripts.rs index 4c1a5afb..30b6f5a3 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -17,10 +17,13 @@ use miniscript::{ bitcoin::{secp256k1, util::bip32, Address, PublicKey}, descriptor::{DescriptorPublicKey, Wildcard}, policy::concrete::Policy, - Descriptor, Segwitv0, TranslatePk2, + Descriptor, ForEachKey, Segwitv0, TranslatePk2, }; -use std::fmt; +use std::{ + fmt::{self, Display}, + str::FromStr, +}; #[cfg(feature = "use-serde")] use serde::de; @@ -215,7 +218,10 @@ impl DepositDescriptor { /// /// let deposit_descriptor = /// scripts::DepositDescriptor::new(vec![first_stakeholder, second_stakeholder]).expect("Compiling descriptor"); - /// println!("Deposit descriptor: {}", deposit_descriptor.inner()); + /// println!("Deposit descriptor: {}", deposit_descriptor); + /// + /// let desc_str = deposit_descriptor.to_string(); + /// assert_eq!(deposit_descriptor, scripts::DepositDescriptor::from_str(&desc_str).unwrap()); /// /// let secp = secp256k1::Secp256k1::verification_only(); /// println!("Tenth child witness script: {}", deposit_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); @@ -236,6 +242,26 @@ impl DepositDescriptor { } } +impl Display for DepositDescriptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for DepositDescriptor { + type Err = ScriptCreationError; + + fn from_str(s: &str) -> Result { + let desc: Descriptor = FromStr::from_str(s)?; + + if !desc.for_each_key(|k| k.as_key().is_deriveable()) { + return Err(ScriptCreationError::NonWildcardKeys); + } + + Ok(DepositDescriptor(desc)) + } +} + impl DerivedDepositDescriptor { /// Get the derived miniscript descriptor for deposit outputs. /// @@ -251,7 +277,10 @@ impl DerivedDepositDescriptor { /// /// let deposit_descriptor = /// scripts::DerivedDepositDescriptor::new(vec![first_stakeholder, second_stakeholder]).expect("Compiling descriptor"); - /// println!("Concrete deposit descriptor: {}", deposit_descriptor.inner()); + /// println!("Concrete deposit descriptor: {}", deposit_descriptor); + /// + /// let desc_str = deposit_descriptor.to_string(); + /// assert_eq!(deposit_descriptor, scripts::DerivedDepositDescriptor::from_str(&desc_str).unwrap()); /// ``` /// /// # Errors @@ -267,6 +296,22 @@ impl DerivedDepositDescriptor { } } +impl Display for DerivedDepositDescriptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for DerivedDepositDescriptor { + type Err = ScriptCreationError; + + fn from_str(s: &str) -> Result { + let desc: Descriptor = FromStr::from_str(s)?; + + Ok(DerivedDepositDescriptor(desc)) + } +} + impl UnvaultDescriptor { /// Get the miniscript descriptors for Unvault outputs. /// @@ -299,7 +344,10 @@ impl UnvaultDescriptor { /// // CSV /// 42 /// ).expect("Compiling descriptor"); - /// println!("Unvault descriptor: {}", unvault_descriptor.inner()); + /// println!("Unvault descriptor: {}", unvault_descriptor); + /// + /// let desc_str = unvault_descriptor.to_string(); + /// assert_eq!(unvault_descriptor, scripts::UnvaultDescriptor::from_str(&desc_str).unwrap()); /// /// let secp = secp256k1::Secp256k1::verification_only(); /// println!("Tenth child witness script: {}", unvault_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); @@ -342,6 +390,29 @@ impl UnvaultDescriptor { } } +impl Display for UnvaultDescriptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for UnvaultDescriptor { + type Err = ScriptCreationError; + + fn from_str(s: &str) -> Result { + let desc: Descriptor = FromStr::from_str(s)?; + + if !desc.for_each_key(|k| match k.as_key() { + DescriptorPublicKey::SinglePub(_) => true, // For cosigning servers keys + DescriptorPublicKey::XPub(xpub) => xpub.wildcard != Wildcard::None, + }) { + return Err(ScriptCreationError::NonWildcardKeys); + } + + Ok(UnvaultDescriptor(desc)) + } +} + impl DerivedUnvaultDescriptor { /// Get the miniscript descriptors for Unvault outputs. /// @@ -374,7 +445,10 @@ impl DerivedUnvaultDescriptor { /// // CSV /// 42 /// ).expect("Compiling descriptor"); - /// println!("Unvault descriptor: {}", unvault_descriptor.inner()); + /// println!("Unvault descriptor: {}", unvault_descriptor); + /// + /// let desc_str = unvault_descriptor.to_string(); + /// assert_eq!(unvault_descriptor, scripts::DerivedUnvaultDescriptor::from_str(&desc_str).unwrap()); /// ``` /// /// # Errors @@ -407,6 +481,22 @@ impl DerivedUnvaultDescriptor { } } +impl Display for DerivedUnvaultDescriptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for DerivedUnvaultDescriptor { + type Err = ScriptCreationError; + + fn from_str(s: &str) -> Result { + let desc: Descriptor = FromStr::from_str(s)?; + + Ok(DerivedUnvaultDescriptor(desc)) + } +} + macro_rules! cpfp_descriptor { ($managers: ident) => {{ let pubkeys = $managers @@ -437,12 +527,15 @@ impl CpfpDescriptor { /// /// let cpfp_descriptor = /// scripts::CpfpDescriptor::new(vec![first_manager, second_manager]).expect("Compiling descriptor"); - /// println!("CPFP descriptor: {}", cpfp_descriptor.inner()); + /// println!("CPFP descriptor: {}", cpfp_descriptor); /// /// let secp = secp256k1::Secp256k1::verification_only(); /// println!("Tenth child witness script: {}", cpfp_descriptor.derive(bip32::ChildNumber::from(10), &secp).inner().explicit_script()); /// ``` /// + /// let desc_str = cpfp_descriptor.to_string(); + /// assert_eq!(cpfp_descriptor, scripts::CpfpDescriptor::from_str(&desc_str).unwrap()); + /// /// # Errors /// - If the given `DescriptorPublickKey`s are not wildcards (can be derived from). /// - If the policy compilation to miniscript failed, which should not happen (tm) and would be a @@ -454,6 +547,26 @@ impl CpfpDescriptor { } } +impl Display for CpfpDescriptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for CpfpDescriptor { + type Err = ScriptCreationError; + + fn from_str(s: &str) -> Result { + let desc: Descriptor = FromStr::from_str(s)?; + + if !desc.for_each_key(|k| k.as_key().is_deriveable()) { + return Err(ScriptCreationError::NonWildcardKeys); + } + + Ok(CpfpDescriptor(desc)) + } +} + impl DerivedCpfpDescriptor { /// Get the miniscript descriptor for the Unvault transaction CPFP output. /// @@ -469,7 +582,10 @@ impl DerivedCpfpDescriptor { /// /// let cpfp_descriptor = /// scripts::DerivedCpfpDescriptor::new(vec![first_manager, second_manager]).expect("Compiling descriptor"); - /// println!("Concrete CPFP descriptor: {}", cpfp_descriptor.inner()); + /// println!("Concrete CPFP descriptor: {}", cpfp_descriptor); + /// + /// let desc_str = cpfp_descriptor.to_string(); + /// assert_eq!(cpfp_descriptor, scripts::DerivedCpfpDescriptor::from_str(&desc_str).unwrap()); /// ``` /// /// # Errors @@ -480,6 +596,22 @@ impl DerivedCpfpDescriptor { } } +impl Display for DerivedCpfpDescriptor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl FromStr for DerivedCpfpDescriptor { + type Err = ScriptCreationError; + + fn from_str(s: &str) -> Result { + let desc: Descriptor = FromStr::from_str(s)?; + + Ok(DerivedCpfpDescriptor(desc)) + } +} + /// The "emergency address", it's kept obfuscated for the entire duration of the vault and is /// necessarily a v0 P2WSH #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -525,7 +657,10 @@ impl<'de> de::Deserialize<'de> for EmergencyAddress { #[cfg(test)] mod tests { - use super::{CpfpDescriptor, DepositDescriptor, ScriptCreationError, UnvaultDescriptor}; + use super::{ + CpfpDescriptor, DepositDescriptor, DerivedCpfpDescriptor, DerivedDepositDescriptor, + DerivedUnvaultDescriptor, PublicKey, ScriptCreationError, UnvaultDescriptor, + }; use miniscript::{ bitcoin::{secp256k1, util::bip32, Network}, @@ -622,21 +757,21 @@ mod tests { vec![ first_stakeholder.clone(), second_stakeholder.clone(), - third_stakeholder, + third_stakeholder.clone(), ], vec![first_manager.clone(), invalid_man], 1, - vec![first_cosig, second_cosig, third_cosig], + vec![first_cosig.clone(), second_cosig.clone(), third_cosig], 128, ) .expect_err("Accepting a non wildcard manager xpub"); // But for cosigning servers it's fine - let first_cosig = DescriptorPublicKey::from_str( + let xpub_first_cosig = DescriptorPublicKey::from_str( "xpub6Da8z6vMdBgtfZraAEjruVSyASFbrWqSm724PPbnezQidGH5wVavF6xFKrbpGCC4VtDVnLP5J5NXm8c8do9zC6MRPkgEsxt4oPY7dukETw2", ) .unwrap(); - let second_cosig = DescriptorPublicKey::from_str( + let xpub_second_cosig = DescriptorPublicKey::from_str( "xpub6Cp57dqxsjzveK5XQYJmzRrofaMJLUC3zQjwNNKKWB9kPn1YtUrrPMXxXGQjs9r2RRQ7e9vExWLJinTZmaosezisGG9nTwEVV15iFQYzFfa", ) .unwrap(); @@ -644,16 +779,16 @@ mod tests { vec![first_stakeholder.clone(), second_stakeholder.clone()], vec![first_manager.clone(), second_manager.clone()], 1, - vec![first_cosig, second_cosig], + vec![xpub_first_cosig, xpub_second_cosig], 128, ) .expect("Refusing a non wildcard cosigning server xpub"); - let first_cosig = DescriptorPublicKey::from_str( + let xpub_first_cosig = DescriptorPublicKey::from_str( "xpub6Da8z6vMdBgtfZraAEjruVSyASFbrWqSm724PPbnezQidGH5wVavF6xFKrbpGCC4VtDVnLP5J5NXm8c8do9zC6MRPkgEsxt4oPY7dukETw2/*", ) .unwrap(); - let second_cosig = DescriptorPublicKey::from_str( + let xpub_second_cosig = DescriptorPublicKey::from_str( "xpub6Cp57dqxsjzveK5XQYJmzRrofaMJLUC3zQjwNNKKWB9kPn1YtUrrPMXxXGQjs9r2RRQ7e9vExWLJinTZmaosezisGG9nTwEVV15iFQYzFfa/*", ) .unwrap(); @@ -661,10 +796,85 @@ mod tests { vec![first_stakeholder.clone(), second_stakeholder.clone()], vec![first_manager.clone(), second_manager.clone()], 1, - vec![first_cosig, second_cosig], + vec![xpub_first_cosig, xpub_second_cosig], 128, ) .expect("Refusing a wildcard cosigning server xpub"); + + // You can't mess up by from_str a wildcard descriptor from a derived one, and the other + // way around. + let raw_pk_a = PublicKey::from_str( + "02a489e0ea42b56148d212d325b7c67c6460483ff931c303ea311edfef667c8f35", + ) + .unwrap(); + let raw_pk_b = PublicKey::from_str( + "02767e6dde4877dcbf64de8a45fe1a0575dfc6b0ed06648f1022412c172ebd875c", + ) + .unwrap(); + let raw_pk_c = PublicKey::from_str( + "0371cdea381b365ea159a3cf4f14029d1bff5b36b4cf12ac9e42be6955d2ed4ecf", + ) + .unwrap(); + let raw_pk_d = PublicKey::from_str( + "03b330723c5ebc2b6f2b29b5a8429e020c0806eed0bcbbddfe5fcad2bb2d02e946", + ) + .unwrap(); + let raw_pk_e = PublicKey::from_str( + "02c8bd230d2a5cdd0c5716f0ebe774d5a7341e9cbcc87f4f43e39acc43a73d72a9", + ) + .unwrap(); + let raw_pk_f = PublicKey::from_str( + "02d07b4b45f93d161b0846a5dd1691720069d8a27baab2f85022fe78b5f896ba07", + ) + .unwrap(); + + let deposit_desc = DepositDescriptor::new(vec![ + first_stakeholder.clone(), + second_stakeholder.clone(), + third_stakeholder.clone(), + ]) + .expect("Valid wildcard xpubs"); + DerivedDepositDescriptor::from_str(&deposit_desc.to_string()) + .expect_err("FromStr on an xpub descriptor"); + let der_deposit_desc = + DerivedDepositDescriptor::new(vec![raw_pk_a.clone(), raw_pk_b.clone()]) + .expect("Derived pubkeys"); + DepositDescriptor::from_str(&der_deposit_desc.to_string()) + .expect_err("FromStr on a derived descriptor"); + + let unvault_desc = UnvaultDescriptor::new( + vec![first_stakeholder.clone(), second_stakeholder.clone()], + vec![first_manager.clone(), second_manager.clone()], + 1, + vec![first_cosig, second_cosig], + 128, + ) + .expect("Valid, with xpubs"); + DerivedUnvaultDescriptor::from_str(&unvault_desc.to_string()) + .expect_err("FromStr on an xpub descriptor"); + let der_unvault_desc = DerivedUnvaultDescriptor::new( + vec![raw_pk_a.clone(), raw_pk_b.clone()], + vec![raw_pk_c, raw_pk_d], + 2, + vec![raw_pk_e, raw_pk_f], + 1024, + ) + .expect("Derived pubkeys"); + UnvaultDescriptor::from_str(&der_unvault_desc.to_string()) + .expect_err("FromStr on a derived descriptor"); + + let cpfp_desc = CpfpDescriptor::new(vec![ + first_stakeholder.clone(), + second_stakeholder.clone(), + third_stakeholder.clone(), + ]) + .expect("Valid wildcard xpubs"); + DerivedCpfpDescriptor::from_str(&cpfp_desc.to_string()) + .expect_err("FromStr on an xpub descriptor"); + let der_cpfp_desc = + DerivedCpfpDescriptor::new(vec![raw_pk_a, raw_pk_b]).expect("Derived pubkeys"); + CpfpDescriptor::from_str(&der_cpfp_desc.to_string()) + .expect_err("FromStr on a derived descriptor"); } #[test] From bee225421a8c3fb16fb47f38981c1bcbd9de7c6c Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sun, 4 Apr 2021 10:16:24 +0200 Subject: [PATCH 06/18] Tree-wide: misc doc cleanups Signed-off-by: Antoine Poinsot --- src/error.rs | 4 ++-- src/scripts.rs | 37 ++++++++++++++++--------------------- src/transactions.rs | 12 +++++++----- src/txins.rs | 27 +++++++++++++++++---------- src/txouts.rs | 27 ++++++++++++++++----------- 5 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/error.rs b/src/error.rs index 29177c4b..4cacd9a4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,4 @@ -//! Errors related to Revault transactions and Scripts management +//! # Errors related to Revault transactions and Scripts management use crate::transactions::INSANE_FEES; @@ -13,7 +13,7 @@ use miniscript::{ use std::{convert::From, error, fmt}; -/// Error when creating a Revault Bitcoin Script +/// Error when creating a Revault Miniscript Descriptor #[derive(Debug)] pub enum ScriptCreationError { /// Invalid number of keys, threshold, or timelock diff --git a/src/scripts.rs b/src/scripts.rs index 30b6f5a3..153fe77a 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -1,14 +1,15 @@ -//! # Revault scripts +//! # Revault Miniscript descriptors //! -//! Miniscript descriptors for policies specific to the Revault architecture. +//! Miniscript descriptors compilation and handling for policies specific to the Revault +//! architecture. //! -//! We use [miniscript](http://bitcoin.sipa.be/miniscript/) in order to "safely" derive -//! scripts depending on the setup configuration (ie the number of stakeholders, the -//! number of fund managers, and the relative timelock) for all script but the (unknown Emergency -//! one). +//! We use [miniscript](http://bitcoin.sipa.be/miniscript/) in order to "safely" compile, +//! derive, and satisfy Scripts depending on the setup configuration (ie the number of +//! stakeholders, the number of fund managers, and the relative timelock) for all script +//! but the (unknown) Emergency one. //! -//! Note these functions are not safe to reuse after initial set up, as the returned descriptors -//! are non-deterministically compiled from an abstract policy. +//! **NOTE**: the compilation functions are not safe to reuse after initial set up, as the +//! returned descriptors are non-deterministically compiled from an abstract policy. //! Backup the output Miniscript descriptors instead. use crate::error::*; @@ -79,28 +80,22 @@ macro_rules! impl_descriptor_newtype { impl_descriptor_newtype!( DepositDescriptor, DerivedDepositDescriptor, - doc = "A **generalistic** (with wildcard xpubs) vault / deposit miniscript descriptor. \ - See the [deposit_descriptor] function for more information.", - doc = "A **concrete** (with raw public keys) vault / deposit miniscript descriptor. \ - See the [deposit_descriptor] function for more information." + doc = "A **generalistic** (with wildcard xpubs) deposit Miniscript descriptor.", + doc = "A **concrete** (with raw public keys) deposit Miniscript descriptor. " ); impl_descriptor_newtype!( UnvaultDescriptor, DerivedUnvaultDescriptor, - doc = "A **generalistic** (with wildcard xpubs) Unvault miniscript descriptor. \ - See the [unvault_descriptor] function for more information.", - doc = "A **concrete** (with raw public keys) Unvault miniscript descriptor. \ - See the [unvault_descriptor] function for more information." + doc = "A **generalistic** (with wildcard xpubs) Unvault miniscript descriptor.", + doc = "A **concrete** (with raw public keys) Unvault miniscript descriptor." ); impl_descriptor_newtype!( CpfpDescriptor, DerivedCpfpDescriptor, - doc = "A **generalistic** (with wildcard xpubs) CPFP miniscript descriptor. \ - See the [cpfp_descriptor] function for more information.", - doc = "A **concrete** (with raw public keys) CPFP miniscript descriptor. \ - See the [cpfp_descriptor] function for more information." + doc = "A **generalistic** (with wildcard xpubs) CPFP miniscript descriptor.", + doc = "A **concrete** (with raw public keys) CPFP miniscript descriptor." ); macro_rules! deposit_desc_checks { @@ -612,7 +607,7 @@ impl FromStr for DerivedCpfpDescriptor { } } -/// The "emergency address", it's kept obfuscated for the entire duration of the vault and is +/// The "Emergency address", it's kept obfuscated for the entire duration of the vault and is /// necessarily a v0 P2WSH #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct EmergencyAddress(Address); diff --git a/src/transactions.rs b/src/transactions.rs index be174c66..684251b4 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -34,7 +34,7 @@ use { use std::{collections::BTreeMap, convert::TryInto, fmt}; /// The value of the CPFP output in the Unvault transaction. -/// See https://github.com/revault/practical-revault/blob/master/transactions.md#unvault_tx +/// See [practical-revault](https://github.com/revault/practical-revault/blob/master/transactions.md#unvault_tx). pub const UNVAULT_CPFP_VALUE: u64 = 30000; /// The feerate, in sat / W, to create the unvaulting transactions with. @@ -77,9 +77,11 @@ pub trait RevaultTransaction: fmt::Debug + Clone + PartialEq { fn into_psbt(self) -> Psbt; /// Get the sighash for an input spending an internal Revault TXO. - /// **Do not use it for fee bumping inputs, use [signature_hash_feebump_input] instead** + /// **Do not use it for fee bumping inputs, use + /// [RevaultTransaction::signature_hash_feebump_input] instead** /// - /// Returns `None` if the input does not exist. + /// Will error if the input is out of bounds or the PSBT input does not contain a Witness + /// Script (ie was already finalized). fn signature_hash_internal_input( &self, input_index: usize, @@ -232,8 +234,8 @@ pub trait RevaultTransaction: fmt::Debug + Clone + PartialEq { } /// Check the transaction is valid (fully-signed) and can be finalized. - /// Slighty more efficient than calling [finalize] on a clone as it gets rid of the - /// belt-and-suspenders checks. + /// Slighty more efficient than calling [RevaultTransaction::finalize] on a clone as it gets + /// rid of the belt-and-suspenders checks. fn is_finalizable(&self, ctx: &secp256k1::Secp256k1) -> bool { miniscript::psbt::finalize(&mut self.inner_tx().clone(), ctx).is_ok() } diff --git a/src/txins.rs b/src/txins.rs index d3de3b95..587fe5dc 100644 --- a/src/txins.rs +++ b/src/txins.rs @@ -1,6 +1,7 @@ -//! Revault txins -//! Wrappers around bitcoin's OutPoint and previous TxOut to statically check Revault transaction -//! creation and ease PSBT management. +//! # Revault PSBT inputs +//! +//! Wrappers around bitcoin's OutPoint and previous TxOut to statically check Revault +//! transaction creation and ease PSBT management. use crate::txouts::{CpfpTxOut, DepositTxOut, FeeBumpTxOut, RevaultTxOut, UnvaultTxOut}; @@ -63,7 +64,8 @@ macro_rules! implem_revault_txin { implem_revault_txin!( DepositTxIn, DepositTxOut, - doc = "A deposit txo spent by the unvault transaction and the emergency transaction" + doc = "A deposit txo spent by the [Unvault](crate::transactions::UnvaultTransaction) \ + transaction and the [Emergency](crate::transactions::EmergencyTransaction)" ); impl DepositTxIn { /// Instanciate a TxIn referencing a deposit txout which signals for RBF. @@ -95,7 +97,10 @@ impl DepositTxIn { implem_revault_txin!( UnvaultTxIn, UnvaultTxOut, - doc="An unvault txo spent by the cancel transaction, an emergency transaction, and the spend transaction." + doc = "An [Unvault](crate::transactions::UnvaultTransaction) txo spent by the \ + [Cancel](crate::transactions::CancelTransaction), \ + [UnvaultEmergency](crate::transactions::UnvaultEmergencyTransaction), and the \ + [Spend](crate::transactions::SpendTransaction)." ); impl UnvaultTxIn { /// Instanciate a TxIn referencing an unvault txout. We need the sequence to be explicitly @@ -128,12 +133,13 @@ impl UnvaultTxIn { implem_revault_txin!( FeeBumpTxIn, FeeBumpTxOut, - doc = "A wallet txo spent by a revaulting (cancel, emergency) transaction to bump the transaction feerate.\ - This output is often created by a first stage transaction, but may directly be a wallet\ - utxo." + doc = "A wallet txo spent by a revocation ([Cancel](crate::transactions::CancelTransaction), \ + [Emergency](crate::transactions::EmergencyTransaction)) transaction to bump the package feerate. \ + \ + This output is from an external wallet and is often created by a first stage transaction." ); impl FeeBumpTxIn { - /// Instanciate a txin referencing a feebumpt txout which signals for RBF. + /// Instanciate a txin referencing a feebump txout which signals for RBF. pub fn new(outpoint: OutPoint, prev_txout: FeeBumpTxOut) -> FeeBumpTxIn { FeeBumpTxIn { outpoint, @@ -146,7 +152,8 @@ impl FeeBumpTxIn { implem_revault_txin!( CpfpTxIn, CpfpTxOut, - doc = "The unvault CPFP txo spent to accelerate the confirmation of the unvault transaction." + doc = "The [Unvault CPFP txo](crate::txouts::CpfpTxOut) spent to accelerate the confirmation of the \ + [Unvault](crate::transactions::UnvaultTransaction)." ); impl CpfpTxIn { /// Instanciate a TxIn referencing a CPFP txout which signals for RBF. diff --git a/src/txouts.rs b/src/txouts.rs index 3b8a1fc9..ec06af94 100644 --- a/src/txouts.rs +++ b/src/txouts.rs @@ -1,4 +1,5 @@ -//! Revault txouts +//! # Revault PSBT outputs +//! //! Wrappers around bitcoin's TxOut to statically check Revault transactions creation and ease //! their PSBT management. @@ -59,7 +60,9 @@ macro_rules! implem_revault_txout { implem_revault_txout!( DepositTxOut, - doc = "A deposit transaction output. Used by the funding / deposit transactions, the cancel transactions, and the spend transactions (for the change)." + doc = "A deposit transaction output. Used by the [Deposit](crate::transactions::DepositTransaction), \ + the [Cancel](crate::transactions::CancelTransaction), and the \ + [Spend](crate::transactions::SpendTransaction)." ); impl DepositTxOut { /// Create a new DepositTxOut out of the given Deposit script descriptor @@ -74,7 +77,7 @@ impl DepositTxOut { } } -implem_revault_txout!(UnvaultTxOut, doc = "*The* unvault transaction output."); +implem_revault_txout!(UnvaultTxOut, doc = "*The* Unvault transaction output."); impl UnvaultTxOut { /// Create a new UnvaultTxOut out of the given Unvault script descriptor pub fn new(value: u64, script_descriptor: &DerivedUnvaultDescriptor) -> UnvaultTxOut { @@ -90,7 +93,7 @@ impl UnvaultTxOut { implem_revault_txout!( EmergencyTxOut, - doc = "The Emergency Deep Vault, the destination of the emergency transactions fund." + doc = "The Emergency Deep Vault, the destination of the Emergency transactions fund." ); impl EmergencyTxOut { /// Create a new EmergencyTxOut, note that we don't know the witness_script! @@ -107,7 +110,8 @@ impl EmergencyTxOut { implem_revault_txout!( CpfpTxOut, - doc = "The output attached to the unvault transaction so that the fund managers can CPFP." + doc = "The output attached to the [Unvault](crate::transactions::UnvaultTransaction) \ + so that the fund managers can fee-bump it." ); impl CpfpTxOut { /// Create a new CpfpTxOut out of the given Cpfp descriptor @@ -124,7 +128,7 @@ impl CpfpTxOut { implem_revault_txout!( FeeBumpTxOut, - doc = "The output spent by the revaulting transactions to bump their feerate" + doc = "The output spent by the revocation transactions to bump their feerate" ); impl FeeBumpTxOut { /// Create a new FeeBumpTxOut, note that it's managed externally so we don't need a witness @@ -143,11 +147,12 @@ impl FeeBumpTxOut { implem_revault_txout!( ExternalTxOut, - doc = "An untagged external output, as spent by the deposit transaction or created by the spend transaction." + doc = "An untagged external output, as spent / created by the \ + [Deposit](crate::transactions::DepositTransaction) or created by the \ + [Spend](crate::transactions::SpendTransaction)." ); impl ExternalTxOut { - /// Create a new ExternalTxOut, note that it's managed externally so we don't need a witness - /// Script. + /// Create an external txout, hence without a witness script. pub fn new(txout: TxOut) -> ExternalTxOut { ExternalTxOut { txout, @@ -156,8 +161,8 @@ impl ExternalTxOut { } } -/// A spend transaction output can be either a change one (DepositTxOut) or a payee-controlled -/// one (ExternalTxOut). +/// A [Spend](crate::transactions::SpendTransaction) output can be either a change one (DepositTxOut) +/// or a payee-controlled one (ExternalTxOut). #[derive(Debug, Clone)] pub enum SpendTxOut { /// The actual destination of the funds, many such output can be present in a Spend From baebf798d7025cb95335e4787e9d40f479b1515f Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sun, 4 Apr 2021 10:41:56 +0200 Subject: [PATCH 07/18] transactions: fixup CSV tests after Miniscript update Signed-off-by: Antoine Poinsot --- src/transactions.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/transactions.rs b/src/transactions.rs index 684251b4..0fb5d7f9 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -1636,8 +1636,7 @@ mod tests { #[test] fn transaction_derivation() { let secp = secp256k1::Secp256k1::new(); - // FIXME: Miniscript mask for sequence check is bugged in this version. - let csv = fastrand::u32(..1 << 16); + let csv = fastrand::u32(..1 << 22); eprintln!("Using a CSV of '{}'", csv); // Test the dust limit @@ -2106,17 +2105,11 @@ mod tests { Some(child_number), SigHashType::All, )?; - match spend_tx.finalize(&secp) { - Err(e) => assert!( - // FIXME: uncomment when upgrading miniscript - //e.to_string().contains("required relative locktime CSV"), - e.to_string().contains("could not satisfy at index 0"), - "Invalid error: got '{}' \n {:#?}", - e, - spend_tx - ), - Ok(_) => unreachable!(), - } + assert!(spend_tx + .finalize(&secp) + .unwrap_err() + .to_string() + .contains("could not satisfy at index 0")); // "This time for sure !" let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor, csv); // Right csv From b5478f429a1d5b35a4ca5ca834ea0bf1a4db6a86 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sun, 4 Apr 2021 10:46:13 +0200 Subject: [PATCH 08/18] scripts: use rust-miniscript's constants for sequence bit flags They were added in the latest version Signed-off-by: Antoine Poinsot --- src/scripts.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/scripts.rs b/src/scripts.rs index 153fe77a..1152a662 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -17,6 +17,7 @@ use crate::error::*; use miniscript::{ bitcoin::{secp256k1, util::bip32, Address, PublicKey}, descriptor::{DescriptorPublicKey, Wildcard}, + miniscript::limits::{SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG}, policy::concrete::Policy, Descriptor, ForEachKey, Segwitv0, TranslatePk2, }; @@ -135,8 +136,9 @@ macro_rules! unvault_desc_checks { } // We require the locktime to be in number of blocks, and of course to not be disabled. - // TODO: use rust-miniscript's constants after upgrading! - if ($csv_value & (1 << 31)) != 0 || ($csv_value & (1 << 22)) != 0 { + if ($csv_value & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 + || ($csv_value & SEQUENCE_LOCKTIME_TYPE_FLAG) != 0 + { return Err(ScriptCreationError::BadParameters); } }; From d025a868dfbace8fe8b4d212343e5d4d29fe6012 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Sun, 4 Apr 2021 20:36:12 +0200 Subject: [PATCH 09/18] scripts: require minimal CSV We don't use the bits without consensus meaning, so better to not create confusion by always having the numeric value passed to the descriptor represent the minimum number of blocks needed to unlock the coin, no matter what. Signed-off-by: Antoine Poinsot --- src/scripts.rs | 10 +++++++++- src/transactions.rs | 14 +++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/scripts.rs b/src/scripts.rs index 1152a662..e2ba7761 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -30,6 +30,10 @@ use std::{ #[cfg(feature = "use-serde")] use serde::de; +/// Flag applied to the nSequence and CSV value before comparing them. +/// https://github.com/bitcoin/bitcoin/blob/4a540683ec40393d6369da1a9e02e45614db936d/src/primitives/transaction.h#L87-L89 +pub const SEQUENCE_LOCKTIME_MASK: u32 = 0x00_00_ff_ff; + // These are useful to create TxOuts out of the right Script descriptor macro_rules! impl_descriptor_newtype { @@ -135,9 +139,13 @@ macro_rules! unvault_desc_checks { return Err(ScriptCreationError::BadParameters); } - // We require the locktime to be in number of blocks, and of course to not be disabled. + // We require the locktime to: + // - not be disabled + // - be in number of blocks + // - be 'clean' / minimal, ie all bits without consensus meaning should be 0 if ($csv_value & SEQUENCE_LOCKTIME_DISABLE_FLAG) != 0 || ($csv_value & SEQUENCE_LOCKTIME_TYPE_FLAG) != 0 + || ($csv_value & SEQUENCE_LOCKTIME_MASK) != $csv_value { return Err(ScriptCreationError::BadParameters); } diff --git a/src/transactions.rs b/src/transactions.rs index 0fb5d7f9..5368044a 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -724,11 +724,16 @@ impl UnvaultTransaction { } /// Get the Unvault txo to be referenced in a spending transaction + /// + /// # Panic + /// Will panic if passed a csv higher than + /// [SEQUENCE_LOCKTIME_MASK](crate::scripts::SEQUENCE_LOCKTIME_MASK) pub fn spend_unvault_txin( &self, unvault_descriptor: &DerivedUnvaultDescriptor, csv: u32, ) -> UnvaultTxIn { + assert!(csv <= SEQUENCE_LOCKTIME_MASK, "{}", csv); self.unvault_txin(unvault_descriptor, csv) } @@ -1636,7 +1641,7 @@ mod tests { #[test] fn transaction_derivation() { let secp = secp256k1::Secp256k1::new(); - let csv = fastrand::u32(..1 << 22); + let csv = fastrand::u32(..SEQUENCE_LOCKTIME_MASK); eprintln!("Using a CSV of '{}'", csv); // Test the dust limit @@ -1646,6 +1651,10 @@ mod tests { .to_string(), Error::TransactionCreation(TransactionCreationError::Dust).to_string() ); + // Non-minimal CSV + derive_transactions(2, 1, SEQUENCE_LOCKTIME_MASK + 1, 300_000, &secp) + .expect_err("Unclean CSV"); + // Absolute minimum derive_transactions(2, 1, csv, 234_632, &secp).expect(&format!( "Tx chain with 2 stakeholders, 1 manager, {} csv, 235_250 deposit", @@ -1692,8 +1701,7 @@ mod tests { managers.len(), cosigners.clone(), csv, - ) - .expect("Unvault descriptor generation error"); + )?; let cpfp_descriptor = CpfpDescriptor::new(managers).expect("Unvault CPFP descriptor generation error"); let deposit_descriptor = From 80bfdcc01ce221fec9bf3c7fa7c50dae5bf351d2 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 11:58:00 +0200 Subject: [PATCH 10/18] transactions: make sure the Spend transaction size is standard Signed-off-by: Antoine Poinsot --- src/error.rs | 11 +++++++ src/transactions.rs | 70 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/error.rs b/src/error.rs index 4cacd9a4..29bc8780 100644 --- a/src/error.rs +++ b/src/error.rs @@ -77,6 +77,8 @@ pub enum TransactionCreationError { Dust, /// Sends more than it spends NegativeFees, + /// Transaction weight more than 400k weight units. + TooLarge, } impl fmt::Display for TransactionCreationError { @@ -88,6 +90,10 @@ impl fmt::Display for TransactionCreationError { f, "The sum of the inputs value is less than the sum of the outputs value" ), + Self::TooLarge => write!( + f, + "Transaction too large: satisfied it could be >400k weight units" + ), } } } @@ -146,6 +152,7 @@ pub enum PsbtValidationError { InvalidPrevoutType(PsbtInput), PartiallyFinalized, InsaneAmounts, + TransactionTooLarge, } impl fmt::Display for PsbtValidationError { @@ -196,6 +203,10 @@ impl fmt::Display for PsbtValidationError { f, "PSBT contains either overflowing amounts or creates more coins than it spends" ), + Self::TransactionTooLarge => write!( + f, + "Transaction too large: satisfied it could be >400k weight units" + ), } } } diff --git a/src/transactions.rs b/src/transactions.rs index 5368044a..9b70e9df 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -55,6 +55,10 @@ pub const INSANE_FEES: u64 = 20_000_000; /// This enables CSV and is easier to apply to all transactions anyways. pub const TX_VERSION: i32 = 2; +/// Maximum weight of a transaction to be relayed. +/// https://github.com/bitcoin/bitcoin/blob/590e49ccf2af27c6c1f1e0eb8be3a4bf4d92ce8b/src/policy/policy.h#L23-L24 +pub const MAX_STANDARD_TX_WEIGHT: u32 = 400_000; + /// A Revault transaction. /// /// Wraps a rust-bitcoin PSBT and defines some BIP174 roles as methods. @@ -679,6 +683,11 @@ impl UnvaultTransaction { return Err(TransactionCreationError::InsaneFees); } + assert!( + total_weight <= MAX_STANDARD_TX_WEIGHT as u64, + "A single input and two outputs" + ); + // The unvault output value is then equal to the deposit value minus the fees and the CPFP. let deposit_value = deposit_input.txout().txout().value; if fees + UNVAULT_CPFP_VALUE + DUST_LIMIT > deposit_value { @@ -822,6 +831,8 @@ impl UnvaultTransaction { } } + // NOTE: the Unvault transaction cannot get larger than MAX_STANDARD_TX_WEIGHT + Ok(UnvaultTransaction(psbt)) } } @@ -865,6 +876,11 @@ impl CancelTransaction { // Without the feebump input, it should not be reachable. debug_assert!(fees < INSANE_FEES); + assert!( + total_weight <= MAX_STANDARD_TX_WEIGHT as u64, + "At most 2 inputs and single output" + ); + // Now, get the revaulting output value out of it. let unvault_value = unvault_input.txout().txout().value; let revault_value = unvault_value @@ -984,6 +1000,11 @@ impl EmergencyTransaction { // Without the feebump input, it should not be reachable. debug_assert!(fees < INSANE_FEES); + assert!( + total_weight <= MAX_STANDARD_TX_WEIGHT as u64, + "At most 2 inputs and a single output" + ); + // Now, get the emergency output value out of it. let deposit_value = deposit_input.txout().txout().value; let emer_value = deposit_value @@ -1078,6 +1099,11 @@ impl UnvaultEmergencyTransaction { // Without the feebump input, it should not be reachable. debug_assert!(fees < INSANE_FEES); + assert!( + total_weight <= MAX_STANDARD_TX_WEIGHT as u64, + "At most 2 inputs and a single output" + ); + // Now, get the emergency output value out of it. let deposit_value = unvault_input.txout().txout().value; let emer_value = deposit_value @@ -1160,6 +1186,12 @@ impl SpendTransaction { lock_time, ); + // Used later to check the maximum transaction size. + let sat_weight = unvault_inputs + .iter() + .map(|txin| txin.max_sat_weight()) + .sum::(); + // Record the value spent let mut value_in: u64 = 0; @@ -1220,7 +1252,17 @@ impl SpendTransaction { .collect(), }; - let value_out: u64 = psbt.global.unsigned_tx.output.iter().map(|o| o.value).sum(); + // Make sure we didn't create a Monster Tx :tm: .. + let unsigned_tx = &psbt.global.unsigned_tx; + let witstrip_weight = unsigned_tx.get_weight(); + let total_weight = sat_weight + .checked_add(witstrip_weight) + .expect("Weight computation bug: cannot overflow"); + if total_weight > MAX_STANDARD_TX_WEIGHT as usize { + return Err(TransactionCreationError::TooLarge); + } + + let value_out: u64 = unsigned_tx.output.iter().map(|o| o.value).sum(); let fees = value_in .checked_sub(value_out) .ok_or(TransactionCreationError::NegativeFees)?; @@ -1345,6 +1387,7 @@ impl SpendTransaction { return Err(PsbtValidationError::InvalidInputCount(0).into()); } + let mut max_sat_weight = 0; for input in psbt.inputs.iter() { if input.final_script_witness.is_some() { continue; @@ -1363,9 +1406,32 @@ impl SpendTransaction { } else { return Err(PsbtValidationError::MissingInWitnessScript(input.clone()).into()); } + + max_sat_weight += miniscript::descriptor::Wsh::new( + miniscript::Miniscript::parse( + input + .witness_script + .as_ref() + .ok_or_else(|| PsbtValidationError::InvalidInputField(input.clone()))?, + ) + .map_err(|_| PsbtValidationError::InvalidInputField(input.clone()))?, + ) + .map_err(|_| PsbtValidationError::InvalidInputField(input.clone()))? + .max_satisfaction_weight() + .map_err(|_| PsbtValidationError::InvalidInputField(input.clone()))?; } - Ok(SpendTransaction(psbt)) + // Make sure the transaction cannot get out of standardness bounds once finalized + let spend_tx = SpendTransaction(psbt); + let witstrip_weight = spend_tx.inner_tx().global.unsigned_tx.get_weight(); + let total_weight = witstrip_weight + .checked_add(max_sat_weight) + .expect("Weight computation bug"); + if total_weight > MAX_STANDARD_TX_WEIGHT as usize { + return Err(PsbtValidationError::TransactionTooLarge.into()); + } + + Ok(spend_tx) } } From 60a0c8bf7884bbbdd2b03673b5c3efb8991d24a3 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 12:22:11 +0200 Subject: [PATCH 11/18] fuzz: use secp256k1's global context Signed-off-by: Antoine Poinsot --- fuzz/fuzz_targets/parse_cancel.rs | 6 ++---- fuzz/fuzz_targets/parse_emergency.rs | 6 ++---- fuzz/fuzz_targets/parse_spend.rs | 6 ++---- fuzz/fuzz_targets/parse_unvault.rs | 6 ++---- fuzz/fuzz_targets/parse_unvault_emergency.rs | 6 ++---- fuzz/run.sh | 2 +- 6 files changed, 11 insertions(+), 21 deletions(-) diff --git a/fuzz/fuzz_targets/parse_cancel.rs b/fuzz/fuzz_targets/parse_cancel.rs index 37963cf9..38dafd78 100644 --- a/fuzz/fuzz_targets/parse_cancel.rs +++ b/fuzz/fuzz_targets/parse_cancel.rs @@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target; use revault_tx::{ miniscript::bitcoin::{ - secp256k1::{Secp256k1, Signature}, + secp256k1::{Secp256k1, Signature, SECP256K1}, PublicKey, SigHashType, }, transactions::{CancelTransaction, RevaultTransaction}, @@ -91,10 +91,8 @@ fuzz_target!(|data: &[u8]| { #[allow(unused_must_use)] tx.verify_input(0); - // FIXME: find a way to use the global context of secp... - let secp = Secp256k1::new(); // Same for the finalization #[allow(unused_must_use)] - tx.finalize(&secp); + tx.finalize(&SECP256K1); } }); diff --git a/fuzz/fuzz_targets/parse_emergency.rs b/fuzz/fuzz_targets/parse_emergency.rs index e93fbc2f..90b907af 100644 --- a/fuzz/fuzz_targets/parse_emergency.rs +++ b/fuzz/fuzz_targets/parse_emergency.rs @@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target; use revault_tx::{ miniscript::bitcoin::{ - secp256k1::{Secp256k1, Signature}, + secp256k1::{Signature, SECP256K1}, PublicKey, SigHashType, }, transactions::{EmergencyTransaction, RevaultTransaction}, @@ -87,10 +87,8 @@ fuzz_target!(|data: &[u8]| { #[allow(unused_must_use)] tx.verify_input(0); - // FIXME: find a way to use the global context of secp... - let secp = Secp256k1::new(); // Same for the finalization #[allow(unused_must_use)] - tx.finalize(&secp); + tx.finalize(&SECP256K1); } }); diff --git a/fuzz/fuzz_targets/parse_spend.rs b/fuzz/fuzz_targets/parse_spend.rs index 15690b3d..dd5485a3 100644 --- a/fuzz/fuzz_targets/parse_spend.rs +++ b/fuzz/fuzz_targets/parse_spend.rs @@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target; use revault_tx::{ miniscript::bitcoin::{ - secp256k1::{Secp256k1, Signature}, + secp256k1::{Signature, SECP256K1}, PublicKey, SigHashType, }, transactions::{RevaultTransaction, SpendTransaction}, @@ -49,10 +49,8 @@ fuzz_target!(|data: &[u8]| { tx.verify_input(i); } - // FIXME: find a way to use the global context of secp... - let secp = Secp256k1::new(); // Same for the finalization #[allow(unused_must_use)] - tx.finalize(&secp); + tx.finalize(&SECP256K1); } }); diff --git a/fuzz/fuzz_targets/parse_unvault.rs b/fuzz/fuzz_targets/parse_unvault.rs index 9221590c..481117e0 100644 --- a/fuzz/fuzz_targets/parse_unvault.rs +++ b/fuzz/fuzz_targets/parse_unvault.rs @@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target; use revault_tx::{ miniscript::bitcoin::{ - secp256k1::{Secp256k1, Signature}, + secp256k1::{Signature, SECP256K1}, PublicKey, SigHashType, }, transactions::{RevaultTransaction, UnvaultTransaction}, @@ -44,10 +44,8 @@ fuzz_target!(|data: &[u8]| { #[allow(unused_must_use)] tx.verify_input(0); - // FIXME: find a way to use the global context of secp... - let secp = Secp256k1::new(); // Same for the finalization #[allow(unused_must_use)] - tx.finalize(&secp); + tx.finalize(&SECP256K1); } }); diff --git a/fuzz/fuzz_targets/parse_unvault_emergency.rs b/fuzz/fuzz_targets/parse_unvault_emergency.rs index d4509c56..e05b497a 100644 --- a/fuzz/fuzz_targets/parse_unvault_emergency.rs +++ b/fuzz/fuzz_targets/parse_unvault_emergency.rs @@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target; use revault_tx::{ miniscript::bitcoin::{ - secp256k1::{Secp256k1, Signature}, + secp256k1::{Signature, SECP256K1}, PublicKey, SigHashType, }, transactions::{RevaultTransaction, UnvaultEmergencyTransaction}, @@ -94,10 +94,8 @@ fuzz_target!(|data: &[u8]| { #[allow(unused_must_use)] tx.verify_input(0); - // FIXME: find a way to use the global context of secp... - let secp = Secp256k1::new(); // Same for the finalization #[allow(unused_must_use)] - tx.finalize(&secp); + tx.finalize(&SECP256K1); } }); diff --git a/fuzz/run.sh b/fuzz/run.sh index 413157e1..4cc77d4c 100755 --- a/fuzz/run.sh +++ b/fuzz/run.sh @@ -6,5 +6,5 @@ cd corpus && git clone https://github.com/revault/revault_tx_corpus cargo install --force cargo-fuzz for target in $(ls fuzz/fuzz_targets);do - cargo +nightly fuzz run "${target%.*}" -- -runs=0 + cargo +nightly fuzz run -O "${target%.*}" -- -runs=0 done From 6a10e810d9a720ebda1120d67ca7b3b5a5614844 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 14:45:13 +0200 Subject: [PATCH 12/18] scripts: add a CSV getter to the Unvault descriptor We remove the check for invalid csv too, as there is no way to have it wrong anymore (other than creating a whole other descriptor and passing that..) Signed-off-by: Antoine Poinsot --- src/scripts.rs | 33 +++++++++++++++++++++++++-- src/transactions.rs | 55 +++++++++------------------------------------ 2 files changed, 42 insertions(+), 46 deletions(-) diff --git a/src/scripts.rs b/src/scripts.rs index e2ba7761..0c181465 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -16,10 +16,10 @@ use crate::error::*; use miniscript::{ bitcoin::{secp256k1, util::bip32, Address, PublicKey}, - descriptor::{DescriptorPublicKey, Wildcard}, + descriptor::{DescriptorPublicKey, Wildcard, WshInner}, miniscript::limits::{SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG}, policy::concrete::Policy, - Descriptor, ForEachKey, Segwitv0, TranslatePk2, + Descriptor, ForEachKey, MiniscriptKey, Segwitv0, Terminal, TranslatePk2, }; use std::{ @@ -317,6 +317,25 @@ impl FromStr for DerivedDepositDescriptor { } } +fn unvault_descriptor_csv(desc: &Descriptor) -> u32 { + let ms = match desc { + Descriptor::Wsh(ref wsh) => match wsh.as_inner() { + WshInner::Ms(ms) => ms, + WshInner::SortedMulti(_) => unreachable!("Unvault descriptor is not a sorted multi"), + }, + _ => unreachable!("Unvault descriptor is always a P2WSH"), + }; + + let csv_frag = ms + .iter() + .find(|ms| matches!(ms.node, Terminal::Older(_))) + .expect("Unvault Miniscript always contains a CSV fragment"); + match csv_frag.node { + Terminal::Older(csv_value) => csv_value, + _ => unreachable!("Just matched."), + } +} + impl UnvaultDescriptor { /// Get the miniscript descriptors for Unvault outputs. /// @@ -393,6 +412,11 @@ impl UnvaultDescriptor { csv_value ))) } + + /// Get the relative locktime in blocks contained in the Unvault descriptor + pub fn csv_value(&self) -> u32 { + unvault_descriptor_csv(&self.0) + } } impl Display for UnvaultDescriptor { @@ -484,6 +508,11 @@ impl DerivedUnvaultDescriptor { csv_value ))) } + + /// Get the relative locktime in blocks contained in the Unvault descriptor + pub fn csv_value(&self) -> u32 { + unvault_descriptor_csv(&self.0) + } } impl Display for DerivedUnvaultDescriptor { diff --git a/src/transactions.rs b/src/transactions.rs index 9b70e9df..803e67dd 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -737,13 +737,8 @@ impl UnvaultTransaction { /// # Panic /// Will panic if passed a csv higher than /// [SEQUENCE_LOCKTIME_MASK](crate::scripts::SEQUENCE_LOCKTIME_MASK) - pub fn spend_unvault_txin( - &self, - unvault_descriptor: &DerivedUnvaultDescriptor, - csv: u32, - ) -> UnvaultTxIn { - assert!(csv <= SEQUENCE_LOCKTIME_MASK, "{}", csv); - self.unvault_txin(unvault_descriptor, csv) + pub fn spend_unvault_txin(&self, unvault_descriptor: &DerivedUnvaultDescriptor) -> UnvaultTxIn { + self.unvault_txin(unvault_descriptor, unvault_descriptor.csv_value()) } /// Get the Unvault txo to be referenced in a revocation transaction @@ -1536,16 +1531,14 @@ pub fn spend_tx_from_deposits( deposit_txins: Vec<(DepositTxIn, DerivedUnvaultDescriptor, DerivedCpfpDescriptor)>, spend_txos: Vec, cpfp_descriptor: &DerivedCpfpDescriptor, - unvault_csv: u32, lock_time: u32, check_insane_fees: bool, ) -> Result { let unvault_txins = deposit_txins .into_iter() .map(|(txin, unvault_desc, cpfp_desc)| { - UnvaultTransaction::new(txin, &unvault_desc, &cpfp_desc, lock_time).and_then( - |unvault_tx| Ok(unvault_tx.spend_unvault_txin(&unvault_desc, unvault_csv)), - ) + UnvaultTransaction::new(txin, &unvault_desc, &cpfp_desc, lock_time) + .and_then(|unvault_tx| Ok(unvault_tx.spend_unvault_txin(&unvault_desc))) }) .collect::, TransactionCreationError>>()?; @@ -1768,6 +1761,7 @@ mod tests { cosigners.clone(), csv, )?; + assert_eq!(unvault_descriptor.csv_value(), csv); let cpfp_descriptor = CpfpDescriptor::new(managers).expect("Unvault CPFP descriptor generation error"); let deposit_descriptor = @@ -1785,6 +1779,10 @@ mod tests { let der_deposit_descriptor = deposit_descriptor.derive(child_number, secp); let der_unvault_descriptor = unvault_descriptor.derive(child_number, secp); + assert_eq!( + der_unvault_descriptor.csv_value(), + unvault_descriptor.csv_value() + ); let der_cpfp_descriptor = cpfp_descriptor.derive(child_number, secp); // The funding transaction does not matter (random txid from my mempool) @@ -2137,7 +2135,7 @@ mod tests { unvault_tx.finalize(&secp)?; // Create and sign a spend transaction - let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor, csv - 1); // Off-by-one csv + let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor); // Off-by-one csv let dummy_txo = ExternalTxOut::default(); let cpfp_value = SpendTransaction::cpfp_txout( vec![spend_unvault_txin.clone()], @@ -2153,40 +2151,9 @@ mod tests { value: spend_unvault_txin.txout().txout().value - cpfp_value - fees, ..TxOut::default() }); - // Test satisfaction failure with a wrong CSV value - let mut spend_tx = SpendTransaction::new( - vec![spend_unvault_txin], - vec![SpendTxOut::Destination(spend_txo.clone())], - &der_cpfp_descriptor, - 0, - true, - ) - .expect("Fees ok"); - assert_eq!(spend_tx.fees(), fees); - let spend_tx_sighash = spend_tx - .signature_hash_internal_input(0, SigHashType::All) - .expect("Input exists"); - satisfy_transaction_input( - &secp, - &mut spend_tx, - 0, - &spend_tx_sighash, - &managers_priv - .iter() - .chain(cosigners_priv.iter()) - .copied() - .collect::>(), - Some(child_number), - SigHashType::All, - )?; - assert!(spend_tx - .finalize(&secp) - .unwrap_err() - .to_string() - .contains("could not satisfy at index 0")); // "This time for sure !" - let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor, csv); // Right csv + let spend_unvault_txin = unvault_tx.spend_unvault_txin(&der_unvault_descriptor); // Right csv let mut spend_tx = SpendTransaction::new( vec![spend_unvault_txin], vec![SpendTxOut::Destination(spend_txo.clone())], From b8cd7c55e989ed3af127986e9c36b5ec11920211 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 12:46:08 +0200 Subject: [PATCH 13/18] transactions: add txid getters We use them a lot downstream and access inner_tx each time.. Signed-off-by: Antoine Poinsot --- src/transactions.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/transactions.rs b/src/transactions.rs index 803e67dd..04c05aba 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -20,7 +20,7 @@ use miniscript::{ }, }, Address, Network, OutPoint, PublicKey as BitcoinPubKey, Script, SigHash, SigHashType, - Transaction, + Transaction, Txid, Wtxid, }, BitcoinSig, DescriptorTrait, }; @@ -372,6 +372,16 @@ pub trait RevaultTransaction: fmt::Debug + Clone + PartialEq { .checked_sub(value_out) .expect("We never create a transaction with negative fees") } + + /// Get the inner unsigned transaction id + fn txid(&self) -> Txid { + self.inner_tx().global.unsigned_tx.txid() + } + + /// Get the inner unsigned transaction hash with witness data + fn wtxid(&self) -> Wtxid { + self.inner_tx().global.unsigned_tx.wtxid() + } } // Boilerplate for newtype declaration and small trait helpers implementation. From 90f305d50525739553fc4e1eab6c5194175096a7 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 19:21:46 +0200 Subject: [PATCH 14/18] transactions: build transaction chain from deposit outpoint and txo We used DepositTxin for muh typesafety but it's a pain to have to derive the deposit descriptor beforehand downstream while this routine will do it anyways. Signed-off-by: Antoine Poinsot --- src/transactions.rs | 55 +++++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/transactions.rs b/src/transactions.rs index 04c05aba..31d739c3 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -19,8 +19,8 @@ use miniscript::{ PartiallySignedTransaction as Psbt, }, }, - Address, Network, OutPoint, PublicKey as BitcoinPubKey, Script, SigHash, SigHashType, - Transaction, Txid, Wtxid, + Address, Amount, Network, OutPoint, PublicKey as BitcoinPubKey, Script, SigHash, + SigHashType, Transaction, Txid, Wtxid, }, BitcoinSig, DescriptorTrait, }; @@ -1464,7 +1464,8 @@ pub struct FeeBumpTransaction(pub Transaction); /// Get the chain of pre-signed transaction out of a deposit available for a manager. /// No feebump input. pub fn transaction_chain_manager( - deposit_txin: DepositTxIn, + deposit_outpoint: OutPoint, + deposit_amount: Amount, deposit_descriptor: &DepositDescriptor, unvault_descriptor: &UnvaultDescriptor, cpfp_descriptor: &CpfpDescriptor, @@ -1478,12 +1479,17 @@ pub fn transaction_chain_manager( cpfp_descriptor.derive(derivation_index, secp), ); + let deposit_txin = DepositTxIn::new( + deposit_outpoint, + DepositTxOut::new(deposit_amount.as_sat(), &der_deposit_descriptor), + ); let unvault_tx = UnvaultTransaction::new( - deposit_txin.clone(), + deposit_txin, &der_unvault_descriptor, &der_cpfp_descriptor, lock_time, )?; + let cancel_tx = CancelTransaction::new( unvault_tx.revault_unvault_txin(&der_unvault_descriptor), None, @@ -1493,9 +1499,11 @@ pub fn transaction_chain_manager( Ok((unvault_tx, cancel_tx)) } + /// Get the entire chain of pre-signed transaction for this derivation index out of a deposit. No feebump input. pub fn transaction_chain( - deposit_txin: DepositTxIn, + deposit_outpoint: OutPoint, + deposit_amount: Amount, deposit_descriptor: &DepositDescriptor, unvault_descriptor: &UnvaultDescriptor, cpfp_descriptor: &CpfpDescriptor, @@ -1513,7 +1521,8 @@ pub fn transaction_chain( Error, > { let (unvault_tx, cancel_tx) = transaction_chain_manager( - deposit_txin.clone(), + deposit_outpoint, + deposit_amount, deposit_descriptor, unvault_descriptor, cpfp_descriptor, @@ -1522,15 +1531,18 @@ pub fn transaction_chain( secp, )?; + let der_deposit_descriptor = deposit_descriptor.derive(derivation_index, secp); + let deposit_txin = DepositTxIn::new( + deposit_outpoint, + DepositTxOut::new(deposit_amount.as_sat(), &der_deposit_descriptor), + ); let emergency_tx = EmergencyTransaction::new(deposit_txin, None, emer_address.clone(), lock_time)?; + let der_unvault_descriptor = unvault_descriptor.derive(derivation_index, secp); - let unvault_emergency_tx = UnvaultEmergencyTransaction::new( - unvault_tx.revault_unvault_txin(&der_unvault_descriptor), - None, - emer_address, - lock_time, - ); + let unvault_txin = unvault_tx.revault_unvault_txin(&der_unvault_descriptor); + let unvault_emergency_tx = + UnvaultEmergencyTransaction::new(unvault_txin, None, emer_address, lock_time); Ok((unvault_tx, cancel_tx, emergency_tx, unvault_emergency_tx)) } @@ -1574,8 +1586,8 @@ mod tests { use miniscript::{ bitcoin::{ - secp256k1, util::bip32, Address, Network, OutPoint, SigHash, SigHashType, Transaction, - TxIn, TxOut, + secp256k1, util::bip32, Address, Amount, Network, OutPoint, SigHash, SigHashType, + Transaction, TxIn, TxOut, }, descriptor::{DescriptorPublicKey, DescriptorXKey, Wildcard}, Descriptor, DescriptorTrait, @@ -1815,17 +1827,16 @@ mod tests { let deposit_txo = DepositTxOut::new(deposit_raw_tx.output[0].value, &der_deposit_descriptor); let deposit_tx = DepositTransaction(deposit_raw_tx); - let deposit_txin = DepositTxIn::new( - OutPoint { - txid: deposit_tx.0.txid(), - vout: 0, - }, - deposit_txo.clone(), - ); + let deposit_outpoint = OutPoint { + txid: deposit_tx.0.txid(), + vout: 0, + }; + let deposit_txin = DepositTxIn::new(deposit_outpoint, deposit_txo.clone()); // Test that the transaction helper(s) derive the same transactions as we do let (h_unvault, h_cancel, h_emer, h_unemer) = transaction_chain( - deposit_txin.clone(), + deposit_outpoint, + Amount::from_sat(deposit_txo.txout().value), &deposit_descriptor, &unvault_descriptor, &cpfp_descriptor, From 322ac7f0ff8caececa005817125a25b441cb9449 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 20:42:15 +0200 Subject: [PATCH 15/18] transactions: derive Spend transaction from outpoints and amounts For the same reasons as the transaction_chain() taking now an Outpoint and an Amount. Signed-off-by: Antoine Poinsot --- src/transactions.rs | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/transactions.rs b/src/transactions.rs index 31d739c3..322895d8 100644 --- a/src/transactions.rs +++ b/src/transactions.rs @@ -1547,27 +1547,44 @@ pub fn transaction_chain( Ok((unvault_tx, cancel_tx, emergency_tx, unvault_emergency_tx)) } -/// Get a spend transaction out of a list of deposits and derivation indexes. The -/// `unvault_descriptor` will be derived for each and should not be beforehand. -pub fn spend_tx_from_deposits( - deposit_txins: Vec<(DepositTxIn, DerivedUnvaultDescriptor, DerivedCpfpDescriptor)>, +/// Get a spend transaction out of a list of deposits and derivation indexes. +/// The derivation index used for the Spend CPFP is the highest of the deposits one. +pub fn spend_tx_from_deposits( + deposit_txins: Vec<(OutPoint, Amount, ChildNumber)>, spend_txos: Vec, - cpfp_descriptor: &DerivedCpfpDescriptor, + deposit_descriptor: &DepositDescriptor, + unvault_descriptor: &UnvaultDescriptor, + cpfp_descriptor: &CpfpDescriptor, lock_time: u32, check_insane_fees: bool, + secp: &secp256k1::Secp256k1, ) -> Result { + let mut max_deriv_index = ChildNumber::from(0); let unvault_txins = deposit_txins .into_iter() - .map(|(txin, unvault_desc, cpfp_desc)| { - UnvaultTransaction::new(txin, &unvault_desc, &cpfp_desc, lock_time) - .and_then(|unvault_tx| Ok(unvault_tx.spend_unvault_txin(&unvault_desc))) + .map(|(outpoint, amount, deriv_index)| { + let der_deposit_desc = deposit_descriptor.derive(deriv_index, secp); + let der_unvault_desc = unvault_descriptor.derive(deriv_index, secp); + let der_cpfp_desc = cpfp_descriptor.derive(deriv_index, secp); + + let txin = DepositTxIn::new( + outpoint, + DepositTxOut::new(amount.as_sat(), &der_deposit_desc), + ); + if deriv_index > max_deriv_index { + max_deriv_index = deriv_index; + } + + UnvaultTransaction::new(txin, &der_unvault_desc, &der_cpfp_desc, lock_time) + .and_then(|unvault_tx| Ok(unvault_tx.spend_unvault_txin(&der_unvault_desc))) }) .collect::, TransactionCreationError>>()?; + let der_cpfp_descriptor = cpfp_descriptor.derive(max_deriv_index, secp); SpendTransaction::new( unvault_txins, spend_txos, - cpfp_descriptor, + &der_cpfp_descriptor, lock_time, check_insane_fees, ) From 20bc6d6ed632e473cf44b6da9748fd7796b32e5a Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Mon, 5 Apr 2021 14:13:30 +0200 Subject: [PATCH 16/18] ci: fuzz against our generated corpus Signed-off-by: Antoine Poinsot --- .github/workflows/main.yml | 4 ++-- contrib/ci_fuzz.sh | 6 ++++++ fuzz/fuzz_targets/parse_cancel.rs | 2 +- fuzz/run.sh | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) create mode 100755 contrib/ci_fuzz.sh diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fc558551..df48592d 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -1,6 +1,6 @@ name: CI -on: [push, pull_request] +on: [pull_request] jobs: unittesting: @@ -45,7 +45,7 @@ jobs: override: true profile: minimal - name: Run fuzz testing script - run: ./fuzz/run.sh + run: ./contrib/ci_fuzz.sh rustfmt_check: runs-on: ubuntu-latest diff --git a/contrib/ci_fuzz.sh b/contrib/ci_fuzz.sh new file mode 100755 index 00000000..7edf2f87 --- /dev/null +++ b/contrib/ci_fuzz.sh @@ -0,0 +1,6 @@ +set -ex + +git clone https://github.com/revault/revault_tx_corpus fuzz/corpus +. fuzz/run.sh + +set -ex diff --git a/fuzz/fuzz_targets/parse_cancel.rs b/fuzz/fuzz_targets/parse_cancel.rs index 38dafd78..f53fdaa1 100644 --- a/fuzz/fuzz_targets/parse_cancel.rs +++ b/fuzz/fuzz_targets/parse_cancel.rs @@ -3,7 +3,7 @@ use libfuzzer_sys::fuzz_target; use revault_tx::{ miniscript::bitcoin::{ - secp256k1::{Secp256k1, Signature, SECP256K1}, + secp256k1::{Signature, SECP256K1}, PublicKey, SigHashType, }, transactions::{CancelTransaction, RevaultTransaction}, diff --git a/fuzz/run.sh b/fuzz/run.sh index 4cc77d4c..d663525a 100755 --- a/fuzz/run.sh +++ b/fuzz/run.sh @@ -6,5 +6,5 @@ cd corpus && git clone https://github.com/revault/revault_tx_corpus cargo install --force cargo-fuzz for target in $(ls fuzz/fuzz_targets);do - cargo +nightly fuzz run -O "${target%.*}" -- -runs=0 + cargo +nightly fuzz run -O "${target%.*}" -- -runs=0 -maxlen=500000 done From 0076df15bfaf28b5b4ae14ffdeadc53d3d54289e Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 6 Apr 2021 10:19:16 +0200 Subject: [PATCH 17/18] Cargo: never activate rust-bitcoin's "use-serde" feature Fucking esoteric Windows CI build Signed-off-by: Antoine Poinsot --- Cargo.toml | 2 +- src/scripts.rs | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 91fdf010..15676711 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ description = "Bitcoin Script descriptors and transactions creation routines for exclude = [".github/", "fuzz"] [features] -use-serde = ["serde", "miniscript/use-serde"] +use-serde = ["serde"] [dependencies] bitcoinconsensus = "0.19.0-2" diff --git a/src/scripts.rs b/src/scripts.rs index 0c181465..05d1d296 100644 --- a/src/scripts.rs +++ b/src/scripts.rs @@ -683,7 +683,10 @@ impl<'de> de::Deserialize<'de> for EmergencyAddress { where D: de::Deserializer<'de>, { - let addr = Address::deserialize(deserializer)?; + // FIXME: the windows CI build is preventing us from using the 'use-serde' feature of + // rust-bitcoin. + let addr_str = String::deserialize(deserializer)?; + let addr = Address::from_str(&addr_str).map_err(|e| de::Error::custom(e))?; EmergencyAddress::from(addr).map_err(de::Error::custom) } } From 546ee512dd7c625e1ac688d04f2a5b14b95f9c77 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Tue, 6 Apr 2021 10:45:36 +0200 Subject: [PATCH 18/18] Cargo: bump version to '0.2.0' Signed-off-by: Antoine Poinsot --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 15676711..bce0efac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "revault_tx" -version = "0.1.2" +version = "0.2.0" authors = ["Antoine Poinsot "] edition = "2018" repository = "https://github.com/revault/revault_tx"