From a4bdb1efb79b8b87ba6131dea96798766e1fbcf5 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 20 Oct 2022 15:20:18 +0200 Subject: [PATCH] tree wide: upgrade to Miniscript 8.0.0 This also updates the indirect rust-bitcoin dependency to 0.29. Major changes are in the descriptors management: - The rust-miniscript descriptors don't support raw hashes within the pk_h() fragments, so we don't need all the boutique management for this. - The key translator API changed. - We now take a u16 for the timelock instead of our previous checks. This was inspired by the new rust-bitcoin Sequence type. - We now take a path instead of just a derivation index in DerivedKey. We might use paths after all... As for rust-bitcoin it's just a few nits: - No more 'global' field on PSBTs (yay) - Couple more trait derivation on types - Some APIs were renamed. --- Cargo.lock | 25 ++--- Cargo.toml | 2 +- src/commands/mod.rs | 88 +++++++++------- src/commands/utils.rs | 4 +- src/config.rs | 9 +- src/database/sqlite/mod.rs | 4 +- src/database/sqlite/schema.rs | 4 +- src/descriptors.rs | 193 +++++++++++++++++++--------------- src/testutils.rs | 2 +- 9 files changed, 180 insertions(+), 151 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b25b4b15..cc44a7a1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,15 +60,15 @@ dependencies = [ [[package]] name = "bech32" -version = "0.8.1" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf9ff0bbfd639f15c74af777d81383cf53efb7c93613f6cab67c6c11e05bbf8b" +checksum = "d86b93f97252c47b41663388e6d155714a9d0c398b99f1005cbc5f978b29f445" [[package]] name = "bitcoin" -version = "0.27.1" +version = "0.29.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a41df6ad9642c5c15ae312dd3d074de38fd3eb7cc87ad4ce10f90292a83fe4d" +checksum = "9cb36de3b18ad25f396f9168302e36fb7e1e8923298ab3127da252d288d5af9d" dependencies = [ "bech32", "bitcoin_hashes", @@ -78,9 +78,9 @@ dependencies = [ [[package]] name = "bitcoin_hashes" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "006cc91e1a1d99819bc5b8214be3555c1f0611b169f527a1fdc54ed1f2b745b0" +checksum = "90064b8dee6815a6470d60bad07bbbaee885c0e12d04177138fa3291a01b7bc4" dependencies = [ "serde", ] @@ -255,9 +255,9 @@ dependencies = [ [[package]] name = "miniscript" -version = "6.1.0" +version = "8.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e292b58407dfbf1384e5aca8428d3b0f2eaa09d24cb17088f6db0b7ca31194a" +checksum = "7f4975078076f0b7b914a3044ad7432d2a7fcec38edb855afdc672e24ca35b69" dependencies = [ "bitcoin", "serde", @@ -360,19 +360,20 @@ checksum = "f3f6f92acf49d1b98f7a81226834412ada05458b7364277387724a237f062695" [[package]] name = "secp256k1" -version = "0.20.3" +version = "0.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97d03ceae636d0fed5bae6a7f4f664354c5f4fcedf6eef053fef17e49f837d0a" +checksum = "b7649a0b3ffb32636e60c7ce0d70511eda9c52c658cd0634e194d5a19943aeff" dependencies = [ + "bitcoin_hashes", "secp256k1-sys", "serde", ] [[package]] name = "secp256k1-sys" -version = "0.4.2" +version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "957da2573cde917463ece3570eab4a0b3f19de6f1646cde62e6fd3868f566036" +checksum = "83080e2c2fc1006e625be82e5d1eb6a43b7fd9578b617fcc55814daf286bba4b" dependencies = [ "cc", ] diff --git a/Cargo.toml b/Cargo.toml index 8ae85c26..35a61ff4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ jsonrpc_server = [] [dependencies] # For managing transactions (it re-exports the bitcoin crate) -miniscript = { version = "6.0.0", features = ["use-serde"] } +miniscript = { version = "8.0", features = ["serde"] } # Don't reinvent the wheel dirs = "3.0" diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 040382d0..db563993 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -17,10 +17,13 @@ use std::{ fmt, }; -use miniscript::bitcoin::{ - self, - util::bip32, - util::psbt::{self, Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt}, +use miniscript::{ + bitcoin::{ + self, + util::bip32, + util::psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt}, + }, + psbt::PsbtExt, }; use serde::{Deserialize, Serialize}; @@ -36,7 +39,7 @@ const MAX_FEE: u64 = bitcoin::blockdata::constants::COIN_VALUE; // Assume that paying more than 1000sat/vb in feerate is a bug. const MAX_FEERATE: u64 = bitcoin::blockdata::constants::COIN_VALUE; -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum CommandError { NoOutpoint, NoDestination, @@ -89,8 +92,8 @@ impl std::error::Error for CommandError {} // Sanity check the value of a transaction output. fn check_output_value(value: bitcoin::Amount) -> Result<(), CommandError> { // NOTE: the network parameter isn't used upstream - if value.as_sat() > bitcoin::blockdata::constants::max_money(bitcoin::Network::Bitcoin) - || value.as_sat() < DUST_OUTPUT_SATS + if value.to_sat() > bitcoin::blockdata::constants::max_money(bitcoin::Network::Bitcoin) + || value.to_sat() < DUST_OUTPUT_SATS { Err(CommandError::InvalidOutputValue(value)) } else { @@ -101,7 +104,7 @@ fn check_output_value(value: bitcoin::Amount) -> Result<(), CommandError> { // Apply some sanity checks on a created transaction's PSBT. // TODO: add more sanity checks from revault_tx fn sanity_check_psbt(psbt: &Psbt) -> Result<(), CommandError> { - let tx = &psbt.global.unsigned_tx; + let tx = &psbt.unsigned_tx; // Must have as many in/out in the PSBT and Bitcoin tx. if psbt.inputs.len() != tx.input.len() || psbt.outputs.len() != tx.output.len() { @@ -154,7 +157,7 @@ fn desc_sat_vb(desc: &descriptors::DerivedInheritanceDescriptor) -> u64 { // Get the virtual size of this transaction fn tx_vbytes(tx: &bitcoin::Transaction) -> u64 { - tx.get_weight() + tx.weight() .checked_div(WITNESS_FACTOR) .unwrap() .try_into() @@ -279,7 +282,7 @@ impl DaemonControl { sat_vb += desc_sat_vb(&coin_desc); let witness_script = Some(coin_desc.witness_script()); let witness_utxo = Some(bitcoin::TxOut { - value: coin.amount.as_sat(), + value: coin.amount.to_sat(), script_pubkey: coin_desc.script_pubkey(), }); let bip32_derivation = coin_desc.bip32_derivations(); @@ -302,7 +305,7 @@ impl DaemonControl { out_value = out_value.checked_add(amount).unwrap(); txouts.push(bitcoin::TxOut { - value: amount.as_sat(), + value: amount.to_sat(), script_pubkey: address.script_pubkey(), }); // TODO: if it's an address of ours, signal it as change to signing devices by adding @@ -314,7 +317,7 @@ impl DaemonControl { // isn't much less than what was asked (and obviously that fees aren't negative). let mut tx = bitcoin::Transaction { version: 2, - lock_time: 0, // TODO: randomized anti fee sniping + lock_time: bitcoin::PackedLockTime(0), // TODO: randomized anti fee sniping input: txins, output: txouts, }; @@ -325,7 +328,7 @@ impl DaemonControl { .ok_or(CommandError::InsufficientFunds( in_value, out_value, feerate_vb, ))?; - let nochange_feerate_vb = absolute_fee.as_sat().checked_div(nochange_vb).unwrap(); + let nochange_feerate_vb = absolute_fee.to_sat().checked_div(nochange_vb).unwrap(); if nochange_feerate_vb.checked_mul(10).unwrap() < feerate_vb.checked_mul(9).unwrap() { return Err(CommandError::InsufficientFunds( in_value, out_value, feerate_vb, @@ -351,18 +354,18 @@ impl DaemonControl { // We assume the added output does not increase the size of the varint for // the output count. let with_change_vb = nochange_vb.checked_add(change_vb).unwrap(); - let with_change_feerate_vb = absolute_fee.as_sat().checked_div(with_change_vb).unwrap(); + let with_change_feerate_vb = absolute_fee.to_sat().checked_div(with_change_vb).unwrap(); if with_change_feerate_vb > feerate_vb { let target_fee = with_change_vb.checked_mul(feerate_vb).unwrap(); let change_amount = absolute_fee .checked_sub(bitcoin::Amount::from_sat(target_fee)) .unwrap(); - if change_amount.as_sat() >= DUST_OUTPUT_SATS { + if change_amount.to_sat() >= DUST_OUTPUT_SATS { check_output_value(change_amount)?; // TODO: shuffle once we have Taproot - change_txo.value = change_amount.as_sat(); + change_txo.value = change_amount.to_sat(); tx.output.push(change_txo); psbt_outs.push(PsbtOut::default()); } @@ -370,13 +373,11 @@ impl DaemonControl { } let psbt = Psbt { - global: psbt::Global { - unsigned_tx: tx, - version: 0, - xpub: BTreeMap::new(), - proprietary: BTreeMap::new(), - unknown: BTreeMap::new(), - }, + unsigned_tx: tx, + version: 0, + xpub: BTreeMap::new(), + proprietary: BTreeMap::new(), + unknown: BTreeMap::new(), inputs: psbt_ins, outputs: psbt_outs, }; @@ -388,14 +389,14 @@ impl DaemonControl { pub fn update_spend(&self, mut psbt: Psbt) -> Result<(), CommandError> { let mut db_conn = self.db.connection(); - let tx = &psbt.global.unsigned_tx; + let tx = &psbt.unsigned_tx; // If the transaction already exists in DB, merge the signatures for each input on a best // effort basis. // We work on the newly provided PSBT, in case its content was updated. let txid = tx.txid(); if let Some(db_psbt) = db_conn.spend_tx(&txid) { - let db_tx = db_psbt.global.unsigned_tx; + let db_tx = db_psbt.unsigned_tx; for i in 0..db_tx.input.len() { if tx .input @@ -465,9 +466,14 @@ impl DaemonControl { let mut spend_psbt = db_conn .spend_tx(txid) .ok_or(CommandError::UnknownSpend(*txid))?; - log::debug!("B"); - miniscript::psbt::finalize(&mut spend_psbt, &self.secp) - .map_err(|e| CommandError::SpendFinalization(e.to_string()))?; + spend_psbt.finalize_mut(&self.secp).map_err(|e| { + CommandError::SpendFinalization( + e.into_iter() + .next() + .map(|e| e.to_string()) + .unwrap_or_default(), + ) + })?; // Then, broadcast it (or try to, we never know if we are not going to hit an // error at broadcast time). @@ -524,7 +530,7 @@ pub struct ListCoinsResult { pub coins: Vec, } -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct CreateSpendResult { #[serde(serialize_with = "ser_base64", deserialize_with = "deser_psbt_base64")] pub psbt: Psbt, @@ -546,7 +552,6 @@ pub struct ListSpendResult { mod tests { use super::*; use crate::testutils::*; - use bitcoin::hashes::hex::FromHex; use std::str::FromStr; #[test] @@ -625,7 +630,7 @@ mod tests { spend_block: None, }]); let res = control.create_spend(&[dummy_op], &destinations, 1).unwrap(); - let tx = res.psbt.global.unsigned_tx; + let tx = res.psbt.unsigned_tx; assert_eq!(tx.input.len(), 1); assert_eq!(tx.input[0].previous_output, dummy_op); assert_eq!(tx.output.len(), 2); @@ -636,7 +641,7 @@ mod tests { // At 2sats/vb, it's twice that. assert_eq!(tx.output[1].value, 89_830); let res = control.create_spend(&[dummy_op], &destinations, 2).unwrap(); - let tx = res.psbt.global.unsigned_tx; + let tx = res.psbt.unsigned_tx; assert_eq!(tx.output[1].value, 89_660); // If we ask for a too high feerate, or a too large/too small output, it'll fail. @@ -669,7 +674,7 @@ mod tests { // won't create an output lower than 5k sats. *destinations.get_mut(&dummy_addr).unwrap() = 95_000; let res = control.create_spend(&[dummy_op], &destinations, 1).unwrap(); - let tx = res.psbt.global.unsigned_tx; + let tx = res.psbt.unsigned_tx; assert_eq!(tx.input.len(), 1); assert_eq!(tx.input[0].previous_output, dummy_op); assert_eq!(tx.output.len(), 1); @@ -755,17 +760,17 @@ mod tests { .create_spend(&[dummy_op_a], &destinations_a, 1) .unwrap() .psbt; - let txid_a = psbt_a.global.unsigned_tx.txid(); + let txid_a = psbt_a.unsigned_tx.txid(); let psbt_b = control .create_spend(&[dummy_op_b], &destinations_b, 10) .unwrap() .psbt; - let txid_b = psbt_b.global.unsigned_tx.txid(); + let txid_b = psbt_b.unsigned_tx.txid(); let psbt_c = control .create_spend(&[dummy_op_a, dummy_op_b], &destinations_c, 100) .unwrap() .psbt; - let txid_c = psbt_c.global.unsigned_tx.txid(); + let txid_c = psbt_c.unsigned_tx.txid(); // We can store and query them all control.update_spend(psbt_a.clone()).unwrap(); @@ -776,7 +781,14 @@ mod tests { assert_eq!(db_conn.spend_tx(&txid_c).unwrap(), psbt_c); // As well as update them, with or without new signatures - psbt_a.inputs[0].partial_sigs.insert(bitcoin::PublicKey::from_str("023a664c5617412f0b292665b1fd9d766456a7a3b1614c7e7c5f411200ff1958ef").unwrap(), Vec::::from_hex("304402204004fcdbb9c0d0cbf585f58cee34dccb012efbd8fc2b0d5e97760045ae35803802201a0bd7ec2383e0b93748abc9946c8e17a8312e314dab85982aeba650e738cbf401").unwrap()); + let sig = bitcoin::EcdsaSig::from_str("304402204004fcdbb9c0d0cbf585f58cee34dccb012efbd8fc2b0d5e97760045ae35803802201a0bd7ec2383e0b93748abc9946c8e17a8312e314dab85982aeba650e738cbf401").unwrap(); + psbt_a.inputs[0].partial_sigs.insert( + bitcoin::PublicKey::from_str( + "023a664c5617412f0b292665b1fd9d766456a7a3b1614c7e7c5f411200ff1958ef", + ) + .unwrap(), + sig, + ); control.update_spend(psbt_a.clone()).unwrap(); assert_eq!(db_conn.spend_tx(&txid_a).unwrap(), psbt_a); control.update_spend(psbt_b.clone()).unwrap(); @@ -789,7 +801,7 @@ mod tests { "8753a1d74c0af8dd0a0f3b763c14faf3bd9ed03cbdf33337a074fb0e9f6c7810:2", ) .unwrap(); - psbt_a.global.unsigned_tx.input[0].previous_output = external_op; + psbt_a.unsigned_tx.input[0].previous_output = external_op; assert_eq!( control.update_spend(psbt_a), Err(CommandError::UnknownOutpoint(external_op)) diff --git a/src/commands/utils.rs b/src/commands/utils.rs index a736ebf5..2e3a7f3d 100644 --- a/src/commands/utils.rs +++ b/src/commands/utils.rs @@ -3,7 +3,7 @@ use serde::{de, Deserialize, Deserializer, Serializer}; /// Serialize an amount as sats pub fn ser_amount(amount: &bitcoin::Amount, s: S) -> Result { - s.serialize_u64(amount.as_sat()) + s.serialize_u64(amount.to_sat()) } /// Deserialize an amount from sats @@ -47,7 +47,7 @@ pub fn change_index(psbt: &Psbt) -> Option { None => return None, }; - let tx = &psbt.global.unsigned_tx; + let tx = &psbt.unsigned_tx; (0..tx.output.len()) .rev() .find(|&i| &tx.output[i].script_pubkey == first_coin_spk) diff --git a/src/config.rs b/src/config.rs index 8e8a0dd2..f4ef1ca3 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,7 +2,7 @@ use crate::descriptors::InheritanceDescriptor; use std::{net::SocketAddr, path::PathBuf, str::FromStr, time::Duration}; -use miniscript::{bitcoin::Network, DescriptorPublicKey, ForEach, ForEachKey}; +use miniscript::{bitcoin::Network, DescriptorPublicKey, ForEachKey}; use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; @@ -203,12 +203,7 @@ impl Config { Network::Bitcoin => Network::Bitcoin, _ => Network::Testnet, }; - let unexpected_net = self.main_descriptor.as_inner().for_each_key(|pkpkh| { - let xpub = match pkpkh { - // For DescriptorPublicKey, Pk::Hash == Self. - ForEach::Key(xpub) => xpub, - ForEach::Hash(xpub) => xpub, - }; + let unexpected_net = self.main_descriptor.as_inner().for_each_key(|xpub| { if let DescriptorPublicKey::XPub(xpub) = xpub { xpub.xkey.network != expected_network } else { diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 1f29842d..d9489d59 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -288,7 +288,7 @@ impl SqliteConn { WALLET_ID, coin.outpoint.txid.to_vec(), coin.outpoint.vout, - coin.amount.as_sat(), + coin.amount.to_sat(), deriv_index, ], )?; @@ -405,7 +405,7 @@ impl SqliteConn { /// Insert a new Spend transaction or replace an existing one. pub fn store_spend(&mut self, psbt: &Psbt) { - let txid = psbt.global.unsigned_tx.txid().to_vec(); + let txid = psbt.unsigned_tx.txid().to_vec(); let psbt = encode::serialize(psbt); db_exec(&mut self.conn, |db_tx| { diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 58b5f8b5..b3b0f617 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -218,7 +218,7 @@ impl TryFrom<&rusqlite::Row<'_>> for DbAddress { } /// A row in the "spend_transactions" table -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct DbSpendTransaction { pub id: i64, pub psbt: Psbt, @@ -236,7 +236,7 @@ impl TryFrom<&rusqlite::Row<'_>> for DbSpendTransaction { let txid: Vec = row.get(2)?; let txid: bitcoin::Txid = encode::deserialize(&txid).expect("We only store valid txids"); - assert_eq!(txid, psbt.global.unsigned_tx.txid()); + assert_eq!(txid, psbt.unsigned_tx.txid()); Ok(DbSpendTransaction { id, psbt, txid }) } diff --git a/src/descriptors.rs b/src/descriptors.rs index 0ddb7ac2..4336903c 100644 --- a/src/descriptors.rs +++ b/src/descriptors.rs @@ -1,25 +1,21 @@ use miniscript::{ - bitcoin::{self, hashes::hash160, hashes::Hash, secp256k1, util::bip32}, - descriptor::{self, DescriptorTrait}, - miniscript::{ - decode::Terminal, - iter::PkPkh, - limits::{SEQUENCE_LOCKTIME_DISABLE_FLAG, SEQUENCE_LOCKTIME_TYPE_FLAG}, - Miniscript, + bitcoin::{ + self, + blockdata::transaction::Sequence, + hashes::{hash160, ripemd160, sha256}, + secp256k1, + util::bip32, }, + descriptor, hash256, + miniscript::{decode::Terminal, Miniscript}, policy::{Liftable, Semantic as SemanticPolicy}, - MiniscriptKey, ScriptContext, ToPublicKey, TranslatePk2, + translate_hash_clone, MiniscriptKey, ScriptContext, ToPublicKey, TranslatePk, Translator, }; -use std::{collections::BTreeMap, error, fmt, io::Write, str, sync}; +use std::{collections::BTreeMap, convert::TryFrom, error, fmt, str, sync}; use serde::{Deserialize, Serialize}; -// Flag applied to the nSequence and CSV value before comparing them. -// -// -pub const SEQUENCE_LOCKTIME_MASK: u32 = 0x00_00_ff_ff; - #[derive(Debug)] pub enum DescCreationError { InsaneTimelock(u32), @@ -50,20 +46,20 @@ impl error::Error for DescCreationError {} pub struct DerivedPublicKey { /// Fingerprint of the master xpub and the derivation index used. We don't use a path /// since we never derive at more than one depth. - pub origin: (bip32::Fingerprint, bip32::ChildNumber), + pub origin: (bip32::Fingerprint, bip32::DerivationPath), /// The actual key pub key: bitcoin::PublicKey, } impl fmt::Display for DerivedPublicKey { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let (fingerprint, deriv_index) = &self.origin; + let (fingerprint, deriv_path) = &self.origin; write!(f, "[")?; for byte in fingerprint.as_bytes().iter() { write!(f, "{:02x}", byte)?; } - write!(f, "/{}", deriv_index)?; + write!(f, "/{}", deriv_path)?; write!(f, "]{}", self.key) } } @@ -100,9 +96,9 @@ impl str::FromStr for DerivedPublicKey { } let fingerprint = bip32::Fingerprint::from_str(&fg_deriv[..8]) .map_err(|_| DescCreationError::DerivedKeyParsing)?; - let deriv_index = bip32::ChildNumber::from_str(&fg_deriv[9..]) + let deriv_path = bip32::DerivationPath::from_str(&fg_deriv[9..]) .map_err(|_| DescCreationError::DerivedKeyParsing)?; - if deriv_index.is_hardened() { + if deriv_path.into_iter().any(bip32::ChildNumber::is_hardened) { return Err(DescCreationError::DerivedKeyParsing); } @@ -111,21 +107,23 @@ impl str::FromStr for DerivedPublicKey { Ok(DerivedPublicKey { key, - origin: (fingerprint, deriv_index), + origin: (fingerprint, deriv_path), }) } } impl MiniscriptKey for DerivedPublicKey { - // This allows us to be able to derive keys and key source even for PkH s - type Hash = Self; + type Sha256 = sha256::Hash; + type Hash256 = hash256::Hash; + type Ripemd160 = ripemd160::Hash; + type Hash160 = hash160::Hash; fn is_uncompressed(&self) -> bool { self.key.is_uncompressed() } - fn to_pubkeyhash(&self) -> Self::Hash { - self.clone() + fn is_x_only_key(&self) -> bool { + false } } @@ -134,12 +132,20 @@ impl ToPublicKey for DerivedPublicKey { self.key } - fn hash_to_hash160(derived_key: &Self) -> hash160::Hash { - let mut engine = hash160::Hash::engine(); - engine - .write_all(&derived_key.key.key.serialize()) - .expect("engines don't error"); - hash160::Hash::from_engine(engine) + fn to_sha256(hash: &sha256::Hash) -> sha256::Hash { + *hash + } + + fn to_hash256(hash: &hash256::Hash) -> hash256::Hash { + *hash + } + + fn to_ripemd160(hash: &ripemd160::Hash) -> ripemd160::Hash { + *hash + } + + fn to_hash160(hash: &hash160::Hash) -> hash160::Hash { + *hash } } @@ -147,20 +153,18 @@ impl ToPublicKey for DerivedPublicKey { // - not be disabled // - be in number of blocks // - be 'clean' / minimal, ie all bits without consensus meaning should be 0 -fn csv_check(csv: u32) -> Result<(), DescCreationError> { - if (csv & SEQUENCE_LOCKTIME_DISABLE_FLAG) == 0 - && (csv & SEQUENCE_LOCKTIME_TYPE_FLAG) == 0 - && (csv & SEQUENCE_LOCKTIME_MASK) == csv - { - Ok(()) - } else { - Err(DescCreationError::InsaneTimelock(csv)) - } +// +// All this is achieved simply through asking for a 16-bit integer, since all the +// above are signaled in leftmost bits. +fn csv_check(csv_value: u32) -> Result<(), DescCreationError> { + u16::try_from(csv_value) + .map(|_| ()) + .map_err(|_| DescCreationError::InsaneTimelock(csv_value)) } fn is_unhardened_deriv(key: &descriptor::DescriptorPublicKey) -> bool { match *key { - descriptor::DescriptorPublicKey::SinglePub(..) => false, + descriptor::DescriptorPublicKey::Single(..) => false, descriptor::DescriptorPublicKey::XPub(ref xpub) => { xpub.wildcard == descriptor::Wildcard::Unhardened } @@ -192,11 +196,7 @@ impl str::FromStr for InheritanceDescriptor { descriptor::WshInner::Ms(ms) => ms, _ => return Err(DescCreationError::IncompatibleDesc), }; - let invalid_key = ms.iter_pk_pkh().find_map(|pk_pkh| { - let pk = match pk_pkh { - PkPkh::PlainPubkey(pk) => pk, - PkPkh::HashedPubkey(pk) => pk, - }; + let invalid_key = ms.iter_pk().find_map(|pk| { if is_unhardened_deriv(&pk) { None } else { @@ -224,7 +224,7 @@ impl str::FromStr for InheritanceDescriptor { // Owner branch subs.iter() - .find(|s| matches!(s, SemanticPolicy::KeyHash(_))) + .find(|s| matches!(s, SemanticPolicy::Key(_))) .ok_or(DescCreationError::IncompatibleDesc)?; // Heir branch @@ -239,18 +239,18 @@ impl str::FromStr for InheritanceDescriptor { return Err(DescCreationError::IncompatibleDesc); } // Must be timelocked - let csv = heir_subs + let csv_value = heir_subs .iter() .find_map(|s| match s { SemanticPolicy::Older(csv) => Some(csv), _ => None, }) .ok_or(DescCreationError::IncompatibleDesc)?; - csv_check(*csv)?; + csv_check(csv_value.to_consensus_u32())?; // And key locked heir_subs .iter() - .find(|s| matches!(s, SemanticPolicy::KeyHash(_))) + .find(|s| matches!(s, SemanticPolicy::Key(_))) .ok_or(DescCreationError::IncompatibleDesc)?; Ok(InheritanceDescriptor(descriptor::Descriptor::Wsh(wsh_desc))) @@ -261,9 +261,15 @@ impl InheritanceDescriptor { pub fn new( owner_key: descriptor::DescriptorPublicKey, heir_key: descriptor::DescriptorPublicKey, - timelock: u32, + timelock: u16, ) -> Result { - csv_check(timelock)?; + // 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 + // + // All this is achieved through asking for a 16-bit integer. + let timelock = Sequence::from_height(timelock); if let Some(key) = vec![&owner_key, &heir_key] .iter() @@ -309,30 +315,52 @@ impl InheritanceDescriptor { } /// Derive this descriptor at a given index. + /// + /// # Panics + /// - If the given index is hardened. pub fn derive( &self, index: bip32::ChildNumber, secp: &secp256k1::Secp256k1, ) -> DerivedInheritanceDescriptor { assert!(index.is_normal()); + + // Unfortunately we can't just use `self.0.at_derivation_index().derived_descriptor()` + // since it would return a raw public key, but we need the origin too. + // TODO: upstream our DerivedPublicKey stuff to rust-miniscript. + // + // So we roll our own translation. + struct Derivator<'a, C: secp256k1::Verification>(u32, &'a secp256k1::Secp256k1); + impl<'a, C: secp256k1::Verification> + Translator< + descriptor::DescriptorPublicKey, + DerivedPublicKey, + descriptor::ConversionError, + > for Derivator<'a, C> + { + fn pk( + &mut self, + pk: &descriptor::DescriptorPublicKey, + ) -> Result { + let definite_key = pk.clone().at_derivation_index(self.0); + let origin = ( + definite_key.master_fingerprint(), + definite_key.full_derivation_path(), + ); + let key = definite_key.derive_public_key(self.1)?; + Ok(DerivedPublicKey { origin, key }) + } + translate_hash_clone!( + descriptor::DescriptorPublicKey, + DerivedPublicKey, + descriptor::ConversionError + ); + } + let desc = self .0 - .derive(index.into()) - .translate_pk2(|xpk| { - xpk.derive_public_key(secp).map(|key| { - // FIXME: rust-miniscript will panic if we call - // xpk.master_fingerprint() on a key without origin - let origin = match xpk { - descriptor::DescriptorPublicKey::XPub(..) => { - (xpk.master_fingerprint(), index) - } - _ => unreachable!("All keys are always xpubs"), - }; - - DerivedPublicKey { key, origin } - }) - }) - .expect("All pubkeys are derived, no wildcard."); + .translate_pk(&mut Derivator(index.into(), secp)) + .expect("May only fail on hardened derivation indexes, but we ruled out this case."); DerivedInheritanceDescriptor(desc) } @@ -370,12 +398,13 @@ impl InheritanceDescriptor { }) .expect("Always present"); - *csv + assert!(csv.is_height_locked()); + csv.to_consensus_u32() } } /// Map of a raw public key to the xpub used to derive it and its derivation path -pub type Bip32Deriv = BTreeMap; +pub type Bip32Deriv = BTreeMap; impl DerivedInheritanceDescriptor { pub fn address(&self, network: bitcoin::Network) -> bitcoin::Address { @@ -389,7 +418,7 @@ impl DerivedInheritanceDescriptor { } pub fn witness_script(&self) -> bitcoin::Script { - self.0.explicit_script() + self.0.explicit_script().expect("Not a Taproot descriptor") } pub fn bip32_derivations(&self) -> Bip32Deriv { @@ -404,17 +433,8 @@ impl DerivedInheritanceDescriptor { }; // For DerivedPublicKey, Pk::Hash == Self. - ms.iter_pk_pkh() - .map(|pkpkh| match pkpkh { - PkPkh::PlainPubkey(pk) => pk, - PkPkh::HashedPubkey(pkh) => pkh, - }) - .map(|k| { - ( - k.key, - (k.origin.0, bip32::DerivationPath::from(&[k.origin.1][..])), - ) - }) + ms.iter_pk() + .map(|k| (k.key.inner, (k.origin.0, k.origin.1))) .collect() } @@ -437,12 +457,13 @@ mod tests { let owner_key = descriptor::DescriptorPublicKey::from_str("xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/*").unwrap(); let heir_key = descriptor::DescriptorPublicKey::from_str("xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/*").unwrap(); let timelock = 52560; - assert_eq!(InheritanceDescriptor::new(owner_key.clone(), heir_key.clone(), timelock).unwrap().to_string(), "wsh(or_d(pk(xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/*),and_v(v:pkh(xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/*),older(52560))))#eeyujkt7"); + assert_eq!(InheritanceDescriptor::new(owner_key, heir_key, timelock).unwrap().to_string(), "wsh(or_d(pk(xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/*),and_v(v:pkh(xpub688Hn4wScQAAiYJLPg9yH27hUpfZAUnmJejRQBCiwfP5PEDzjWMNW1wChcninxr5gyavFqbbDjdV1aK5USJz8NDVjUy7FRQaaqqXHh5SbXe/*),older(52560))))#eeyujkt7"); - // We prevent footguns with timelocks - InheritanceDescriptor::new(owner_key.clone(), heir_key.clone(), 0x00_01_0f_00).unwrap_err(); - InheritanceDescriptor::new(owner_key.clone(), heir_key.clone(), (1 << 31) + 1).unwrap_err(); - InheritanceDescriptor::new(owner_key, heir_key, (1 << 22) + 1).unwrap_err(); + // We prevent footguns with timelocks by requiring a u16. Note how the following wouldn't + // compile: + //InheritanceDescriptor::new(owner_key.clone(), heir_key.clone(), 0x00_01_0f_00).unwrap_err(); + //InheritanceDescriptor::new(owner_key.clone(), heir_key.clone(), (1 << 31) + 1).unwrap_err(); + //InheritanceDescriptor::new(owner_key, heir_key, (1 << 22) + 1).unwrap_err(); let owner_key = descriptor::DescriptorPublicKey::from_str("[aabb0011/10/4893]xpub661MyMwAqRbcFG59fiikD8UV762quhruT8K8bdjqy6N2o3LG7yohoCdLg1m2HAY1W6rfBrtauHkBhbfA4AQ3iazaJj5wVPhwgaRCHBW2DBg/*").unwrap(); let heir_key = descriptor::DescriptorPublicKey::from_str("xpub661MyMwAqRbcFfxf71L4Dx4w5TmyNXrBicTEAM7vLzumxangwATWWgdJPb6xH1JHcJH9S3jNZx3fCnkkB1WyqrqGgavj1rehHcbythmruvZ/24/32/*").unwrap(); diff --git a/src/testutils.rs b/src/testutils.rs index 3f5b1e72..f738d503 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -203,7 +203,7 @@ impl DatabaseConnection for DummyDbConn { } fn store_spend(&mut self, psbt: &Psbt) { - let txid = psbt.global.unsigned_tx.txid(); + let txid = psbt.unsigned_tx.txid(); self.db .write() .unwrap()