diff --git a/integration_test/src/test_desc.rs b/integration_test/src/test_desc.rs index de8054121..8a6e1811d 100644 --- a/integration_test/src/test_desc.rs +++ b/integration_test/src/test_desc.rs @@ -13,7 +13,7 @@ use bitcoin::util::{psbt, sighash}; use bitcoin::{self, Amount, OutPoint, SchnorrSig, Script, Transaction, TxIn, TxOut, Txid}; use bitcoincore_rpc::{json, Client, RpcApi}; use miniscript::miniscript::iter; -use miniscript::psbt::PsbtExt; +use miniscript::psbt::{PsbtInputExt, PsbtExt}; use miniscript::{Descriptor, DescriptorTrait, Miniscript, ToPublicKey}; use miniscript::{MiniscriptKey, ScriptContext}; use std::collections::BTreeMap; @@ -109,10 +109,10 @@ pub fn test_desc_satisfy(cl: &Client, testdata: &TestData, desc: &str) -> Witnes script_pubkey: addr.script_pubkey(), }); let mut input = psbt::Input::default(); + input.update_with_descriptor_unchecked(&desc).unwrap(); input.witness_utxo = Some(witness_utxo.clone()); psbt.inputs.push(input); psbt.outputs.push(psbt::Output::default()); - psbt.update_desc(0, &desc, None).unwrap(); // -------------------------------------------- // Sign the transactions with all keys diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index 36f2449fa..f844900cb 100644 --- a/src/descriptor/mod.rs +++ b/src/descriptor/mod.rs @@ -23,6 +23,7 @@ //! these with BIP32 paths, pay-to-contract instructions, etc. //! +use std::ops::Range; use std::{collections::HashMap, sync::Arc}; use std::{ fmt, @@ -30,6 +31,7 @@ use std::{ }; use bitcoin::blockdata::witness::Witness; +use bitcoin::util::address::WitnessVersion; use bitcoin::{self, secp256k1, Script}; use self::checksum::verify_checksum; @@ -259,6 +261,22 @@ pub enum DescriptorType { Tr, } +impl DescriptorType { + /// Returns the segwit version implied by the descriptor type. + /// + /// This will return `Some(WitnessVersion::V0)` whether it is "native" segwitv0 or "wrapped" p2sh segwit. + pub fn segwit_version(&self) -> Option { + use self::DescriptorType::*; + match self { + Tr => Some(WitnessVersion::V1), + Wpkh | ShWpkh | Wsh | ShWsh | ShWshSortedMulti | WshSortedMulti => { + Some(WitnessVersion::V0) + } + Bare | Sh | Pkh | ShSortedMulti => None, + } + } +} + impl Descriptor { // Keys @@ -749,6 +767,31 @@ impl Descriptor { descriptor.to_string() } + + /// Utility method for deriving the descriptor at each index in a range to find one matching + /// `script_pubkey`. + /// + /// If it finds a match then it returns the index it was derived at and the concrete + /// descriptor at that index. If the descriptor is non-derivable then it will simply check the + /// script pubkey against the descriptor and return it if it matches (in this case the index + /// returned will be meaningless). + pub fn find_derivation_index_for_spk( + &self, + secp: &secp256k1::Secp256k1, + script_pubkey: &Script, + range: Range, + ) -> Result)>, ConversionError> { + let range = if self.is_deriveable() { range } else { 0..1 }; + + for i in range { + let concrete = self.derived_descriptor(&secp, i)?; + if &concrete.script_pubkey() == script_pubkey { + return Ok(Some((i, concrete))); + } + } + + Ok(None) + } } impl expression::FromTree for Descriptor @@ -1736,4 +1779,31 @@ pk(03f28773c2d975288bc7d1d205c3748651b075fbc6610e58cddeeddf8f19405aa8))"; Descriptor::::from_str(&format!("wsh(pk({}))", x_only_key)) .unwrap_err(); } + + #[test] + fn test_find_derivation_index_for_spk() { + let secp = secp256k1::Secp256k1::verification_only(); + let descriptor = Descriptor::from_str("tr([73c5da0a/86'/0'/0']xpub6BgBgsespWvERF3LHQu6CnqdvfEvtMcQjYrcRzx53QJjSxarj2afYWcLteoGVky7D3UKDP9QyrLprQ3VCECoY49yfdDEHGCtMMj92pReUsQ/0/*)").unwrap(); + let script_at_0_1 = Script::from_str( + "5120a82f29944d65b86ae6b5e5cc75e294ead6c59391a1edc5e016e3498c67fc7bbb", + ) + .unwrap(); + let expected_concrete = Descriptor::from_str( + "tr(0283dfe85a3151d2517290da461fe2815591ef69f2b18a2ce63f01697a8b313145)", + ) + .unwrap(); + + assert_eq!( + descriptor.find_derivation_index_for_spk(&secp, &script_at_0_1, 0..1), + Ok(None) + ); + assert_eq!( + descriptor.find_derivation_index_for_spk(&secp, &script_at_0_1, 0..2), + Ok(Some((1, expected_concrete.clone()))) + ); + assert_eq!( + descriptor.find_derivation_index_for_spk(&secp, &script_at_0_1, 0..10), + Ok(Some((1, expected_concrete))) + ); + } } diff --git a/src/psbt/mod.rs b/src/psbt/mod.rs index 9f2f2396f..8d6c1e93f 100644 --- a/src/psbt/mod.rs +++ b/src/psbt/mod.rs @@ -20,12 +20,12 @@ //! use std::collections::BTreeMap; -use std::ops::{Deref, Range}; +use std::ops::Deref; use std::{error, fmt}; use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d}; use bitcoin::secp256k1::{self, Secp256k1}; -use bitcoin::util::psbt::PartiallySignedTransaction as Psbt; +use bitcoin::util::psbt::{self, PartiallySignedTransaction as Psbt}; use bitcoin::util::sighash::SighashCache; use bitcoin::{self, SchnorrSighashType}; use bitcoin::{EcdsaSighashType, Script}; @@ -33,11 +33,15 @@ use bitcoin::{EcdsaSighashType, Script}; use bitcoin::util::taproot::{self, ControlBlock, LeafVersion, TapLeafHash}; use descriptor; use interpreter; +use miniscript::iter::PkPkh; use miniscript::limits::SEQUENCE_LOCKTIME_DISABLE_FLAG; use miniscript::satisfy::{After, Older}; use Preimage32; use Satisfier; -use {Descriptor, DescriptorPublicKey, DescriptorTrait, MiniscriptKey, ToPublicKey}; +use { + Descriptor, DescriptorPublicKey, DescriptorTrait, MiniscriptKey, ToPublicKey, TranslatePk, + TranslatePk2, +}; mod finalizer; @@ -516,25 +520,30 @@ pub trait PsbtExt { secp: &Secp256k1, ) -> Result; - /// Update an psbt with the derived descriptor information. If the descriptor is - /// - Sh: Update the redeem script - /// - Wsh: Update the witness script - /// - ShWsh/ShWpkh: Updates redeem script and witness script(in nested wsh case) - /// - Tr: Update the control block maps, internal key and merkle root + /// Update PSBT input with a descriptor and check consistency of `*_utxo` fields. /// - /// # Errors: + /// This is the checked version of [`update_with_descriptor_unchecked`]. It checks that the + /// `witness_utxo` and `non_witness_utxo` are sane and have a `script_pubkey` that matches the + /// descriptor. In particular, it makes sure segwit descriptors always have `witness_utxo` + /// present and pre-segwit descriptors always have `non_witness_utxo` present (and the txid + /// matches). If both `witness_utxo` and `non_witness_utxo` are present then it also checks they + /// are consistent with each other. + /// + /// Hint: because of the *[segwit bug]* some PSBT signers require that `non_witness_utxo` is + /// present on segwitv0 inputs regardless but this function doesn't enforce this so you will + /// have to do this check its presence manually (if it is present this *will* check its + /// validity). /// - /// - If the input Index out of bounds - /// - If there is [`descriptor::ConversionError`] while deriving keys - /// - Psbt does not have corresponding witness/non-witness utxo - /// - Ranged descriptor not supplied with a range to call at - /// - If the given descriptor cannot derive the output key. - fn update_desc( + /// The `descriptor` **must not have any wildcards** in it + /// otherwise an error will be returned however it can (and should) have extended keys in it. + /// + /// [`update_with_descriptor_unchecked`]: PsbtInputExt::update_with_descriptor_unchecked + /// [segwit bug]: https://bitcoinhackers.org/@lukedashjr/104287698361196952 + fn update_input_with_descriptor( &mut self, input_index: usize, - desc: &Descriptor, - range: Option>, - ) -> Result<(), UxtoUpdateError>; + descriptor: &Descriptor, + ) -> Result<(), UtxoUpdateError>; /// Get the sighash message(data to sign) at input index `idx` based on the sighash /// flag specified in the [`Psbt`] sighash field. If the input sighash flag psbt field is `None` @@ -695,77 +704,75 @@ impl PsbtExt for Psbt { Ok(ret) } - fn update_desc( + fn update_input_with_descriptor( &mut self, input_index: usize, desc: &Descriptor, - range: Option>, - ) -> Result<(), UxtoUpdateError> { - if input_index >= self.inputs.len() { - return Err(UxtoUpdateError::IndexOutOfBounds( - input_index, - self.inputs.len(), - )); - } - let mut derived_desc = None; - // NLL block - { - let inp_spk = finalizer::get_scriptpubkey(self, input_index) - .map_err(|_e| UxtoUpdateError::MissingInputUxto)?; - let secp = secp256k1::Secp256k1::verification_only(); + ) -> Result<(), UtxoUpdateError> { + let n_inputs = self.inputs.len(); + let input = self + .inputs + .get_mut(input_index) + .ok_or(UtxoUpdateError::IndexOutOfBounds(input_index, n_inputs))?; + let txin = self + .unsigned_tx + .input + .get(input_index) + .ok_or(UtxoUpdateError::MissingInputUtxo)?; + + let desc_type = desc.desc_type(); + + if let Some(non_witness_utxo) = &input.non_witness_utxo { + if txin.previous_output.txid != non_witness_utxo.txid() { + return Err(UtxoUpdateError::UtxoCheck); + } + } - match range { - None => { - if desc.is_deriveable() { - return Err(UxtoUpdateError::RangeMissingWildCardDescriptor); + let expected_spk = { + match (&input.witness_utxo, &input.non_witness_utxo) { + (Some(witness_utxo), None) => { + if desc_type.segwit_version().is_some() { + witness_utxo.script_pubkey.clone() } else { - derived_desc = desc - .derived_descriptor(&secp, 0) - .map_err(|e| UxtoUpdateError::DerivationError(e)) - .ok(); + return Err(UtxoUpdateError::UtxoCheck); } } - Some(range) => { - for i in range { - let derived = desc - .derived_descriptor(&secp, i) - .map_err(|e| UxtoUpdateError::DerivationError(e))?; - if &derived.script_pubkey() == inp_spk { - derived_desc = Some(derived); - break; - } + (None, Some(non_witness_utxo)) => { + if desc_type.segwit_version().is_some() { + return Err(UtxoUpdateError::UtxoCheck); } + + non_witness_utxo + .output + .get(txin.previous_output.vout as usize) + .ok_or(UtxoUpdateError::UtxoCheck)? + .script_pubkey + .clone() } - } - } - let inp = &mut self.inputs[input_index]; - let derived_desc = derived_desc.ok_or(UxtoUpdateError::IncorrectDescriptor)?; - match derived_desc { - Descriptor::Bare(_) | Descriptor::Pkh(_) | Descriptor::Wpkh(_) => {} - Descriptor::Sh(sh) => match sh.as_inner() { - descriptor::ShInner::Wsh(wsh) => { - inp.witness_script = Some(wsh.inner_script()); - inp.redeem_script = Some(wsh.inner_script().to_v0_p2wsh()); - } - descriptor::ShInner::Wpkh(..) => inp.redeem_script = Some(sh.inner_script()), - descriptor::ShInner::SortedMulti(_) | descriptor::ShInner::Ms(_) => { - inp.redeem_script = Some(sh.inner_script()) - } - }, - Descriptor::Wsh(wsh) => inp.witness_script = Some(wsh.inner_script()), - Descriptor::Tr(tr) => { - let spend_info = tr.spend_info(); - inp.tap_internal_key = Some(spend_info.internal_key()); - inp.tap_merkle_root = spend_info.merkle_root(); - for (_depth, ms) in tr.iter_scripts() { - let leaf_script = (ms.encode(), LeafVersion::TapScript); - let control_block = spend_info - .control_block(&leaf_script) - .expect("Control block must exist in script map for every known leaf"); - inp.tap_scripts.insert(control_block, leaf_script); + (Some(witness_utxo), Some(non_witness_utxo)) => { + if witness_utxo + != non_witness_utxo + .output + .get(txin.previous_output.vout as usize) + .ok_or(UtxoUpdateError::UtxoCheck)? + { + return Err(UtxoUpdateError::UtxoCheck); + } + + witness_utxo.script_pubkey.clone() } + (None, None) => return Err(UtxoUpdateError::UtxoCheck), } }; + + let (_, spk_check_passed) = + update_input_with_descriptor_helper(input, &desc, Some(expected_spk)) + .map_err(UtxoUpdateError::DerivationError)?; + + if !spk_check_passed { + return Err(UtxoUpdateError::MismatchedScriptPubkey); + } + Ok(()) } @@ -785,7 +792,7 @@ impl PsbtExt for Psbt { // Even if the transaction does not require SighashAll, we create `Prevouts::All` for code simplicity let prevouts = bitcoin::util::sighash::Prevouts::All(&prevouts); let inp_spk = - finalizer::get_scriptpubkey(self, idx).map_err(|_e| SighashError::MissingInputUxto)?; + finalizer::get_scriptpubkey(self, idx).map_err(|_e| SighashError::MissingInputUtxo)?; if inp_spk.is_v1_p2tr() { let hash_ty = inp .sighash_type @@ -811,7 +818,7 @@ impl PsbtExt for Psbt { .unwrap_or(Ok(EcdsaSighashType::All)) .map_err(|_e| SighashError::InvalidSighashType)?; let amt = finalizer::get_utxo(self, idx) - .map_err(|_e| SighashError::MissingInputUxto)? + .map_err(|_e| SighashError::MissingInputUtxo)? .value; let is_nested_wpkh = inp_spk.is_p2sh() && inp @@ -861,6 +868,171 @@ impl PsbtExt for Psbt { } } +/// Extension trait for PSBT inputs +pub trait PsbtInputExt { + /// Given the descriptor for a utxo being spent populate the PSBT input's fields so it can be signed. + /// + /// If the descriptor contains wildcards or otherwise cannot be transformed into a concrete + /// descriptor an error will be returned. The descriptor *can* (and should) have extended keys in + /// it so PSBT fields like `bip32_derivation` and `tap_key_origins` can be populated. + /// + /// Note that his method doesn't check that the `witness_utxo` or `non_witness_utxo` is + /// consistent with the descriptor. To do that see [`update_input_with_descriptor`]. + /// + /// ## Return value + /// + /// For convenience, this returns the concrete descriptor that is computed internally to fill + /// out the PSBT input fields. This can be used to manually check that the `script_pubkey` in + /// `witness_utxo` and/or `non_witness_utxo` is consistent with the descriptor. + /// + /// [`update_input_with_descriptor`]: PsbtExt::update_input_with_descriptor + fn update_with_descriptor_unchecked( + &mut self, + descriptor: &Descriptor, + ) -> Result, descriptor::ConversionError>; +} + +impl PsbtInputExt for psbt::Input { + fn update_with_descriptor_unchecked( + &mut self, + descriptor: &Descriptor, + ) -> Result, descriptor::ConversionError> { + let (derived, _) = update_input_with_descriptor_helper(self, descriptor, None)?; + Ok(derived) + } +} + +fn update_input_with_descriptor_helper( + input: &mut psbt::Input, + descriptor: &Descriptor, + check_script: Option