From 2a518d62e6e310d2242dbbbe7176f1ea75759e43 Mon Sep 17 00:00:00 2001 From: Erick Cestari Date: Mon, 28 Apr 2025 10:23:33 -0300 Subject: [PATCH 1/2] Wrap secp256k1::XOnlyPublicKey to improve error handling This commit creates a wrapper type for XOnlyPublicKey instead of directly re-exporting it from the secp256k1 library. --- bitcoin/examples/sign-tx-taproot.rs | 4 +- bitcoin/examples/taproot-psbt-simple.rs | 10 +- bitcoin/examples/taproot-psbt.rs | 10 +- bitcoin/src/address/mod.rs | 3 +- bitcoin/src/bip32.rs | 3 +- bitcoin/src/crypto/key.rs | 155 ++++++++++++++++++++++-- bitcoin/src/crypto/sighash.rs | 3 +- bitcoin/src/psbt/map/input.rs | 2 +- bitcoin/src/psbt/map/output.rs | 3 +- bitcoin/src/psbt/mod.rs | 6 +- bitcoin/src/psbt/serialize.rs | 2 +- bitcoin/src/taproot/mod.rs | 3 +- bitcoin/tests/psbt-sign-taproot.rs | 10 +- 13 files changed, 179 insertions(+), 35 deletions(-) diff --git a/bitcoin/examples/sign-tx-taproot.rs b/bitcoin/examples/sign-tx-taproot.rs index 06a3fab16..616d4c3a9 100644 --- a/bitcoin/examples/sign-tx-taproot.rs +++ b/bitcoin/examples/sign-tx-taproot.rs @@ -26,7 +26,7 @@ fn main() { // Get an unspent output that is locked to the key above that we control. // In a real application these would come from the chain. - let (dummy_out_point, dummy_utxo) = dummy_unspent_transaction_output(&secp, internal_key); + let (dummy_out_point, dummy_utxo) = dummy_unspent_transaction_output(&secp, internal_key.into()); // Get an address to send to. let address = receivers_address(); @@ -45,7 +45,7 @@ fn main() { // The change output is locked to a key controlled by us. let change = TxOut { value: CHANGE_AMOUNT, - script_pubkey: ScriptBuf::new_p2tr(&secp, internal_key, None), // Change comes back to us. + script_pubkey: ScriptBuf::new_p2tr(&secp, internal_key.into(), None), // Change comes back to us. }; // The transaction we want to sign and broadcast. diff --git a/bitcoin/examples/taproot-psbt-simple.rs b/bitcoin/examples/taproot-psbt-simple.rs index 8181573a2..425ffdc20 100644 --- a/bitcoin/examples/taproot-psbt-simple.rs +++ b/bitcoin/examples/taproot-psbt-simple.rs @@ -151,12 +151,12 @@ fn main() { // Get the Tap Key Origins // Map of tap root X-only keys to origin info and leaf hashes contained in it. let origin_input_1 = get_tap_key_origin( - pk_input_1, + pk_input_1.into(), MASTER_FINGERPRINT.parse::().unwrap(), "m/86'/0'/0'/0/0".parse::().unwrap(), ); let origin_input_2 = get_tap_key_origin( - pk_input_2, + pk_input_2.into(), MASTER_FINGERPRINT.parse::().unwrap(), "m/86'/0'/0'/1/0".parse::().unwrap(), ); @@ -187,7 +187,7 @@ fn main() { // The change output is locked to a key controlled by us. let change = TxOut { value: CHANGE_AMOUNT, - script_pubkey: ScriptBuf::new_p2tr(&secp, pk_change, None), // Change comes back to us. + script_pubkey: ScriptBuf::new_p2tr(&secp, pk_change.into(), None), // Change comes back to us. }; // The transaction we want to sign and broadcast. @@ -210,14 +210,14 @@ fn main() { Input { witness_utxo: Some(utxos[0].clone()), tap_key_origins: origins[0].clone(), - tap_internal_key: Some(pk_input_1), + tap_internal_key: Some(pk_input_1.into()), sighash_type: Some(ty), ..Default::default() }, Input { witness_utxo: Some(utxos[1].clone()), tap_key_origins: origins[1].clone(), - tap_internal_key: Some(pk_input_2), + tap_internal_key: Some(pk_input_2.into()), sighash_type: Some(ty), ..Default::default() }, diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs index 92af85156..86df3114e 100644 --- a/bitcoin/examples/taproot-psbt.rs +++ b/bitcoin/examples/taproot-psbt.rs @@ -404,7 +404,7 @@ impl BenefactorWallet { let taproot_spend_info = TaprootBuilder::new() .add_leaf(0, script.clone())? - .finalize(&self.secp, internal_keypair.x_only_public_key().0) + .finalize(&self.secp, internal_keypair.x_only_public_key().0.into()) .expect("should be finalizable"); self.current_spend_info = Some(taproot_spend_info.clone()); let script_pubkey = ScriptBuf::new_p2tr( @@ -442,7 +442,7 @@ impl BenefactorWallet { (vec![leaf_hash], (self.beneficiary_xpub.fingerprint(), derivation_path.clone())), ); origins.insert( - internal_keypair.x_only_public_key().0, + internal_keypair.x_only_public_key().0.into(), (vec![], (self.master_xpriv.fingerprint(&self.secp), derivation_path)), ); let ty = "SIGHASH_ALL".parse::()?; @@ -457,7 +457,7 @@ impl BenefactorWallet { tap_key_origins: origins, tap_merkle_root: taproot_spend_info.merkle_root(), sighash_type: Some(ty), - tap_internal_key: Some(internal_keypair.x_only_public_key().0), + tap_internal_key: Some(internal_keypair.x_only_public_key().0.into()), tap_scripts, ..Default::default() }; @@ -502,7 +502,7 @@ impl BenefactorWallet { let taproot_spend_info = TaprootBuilder::new() .add_leaf(0, script.clone())? - .finalize(&self.secp, new_internal_keypair.x_only_public_key().0) + .finalize(&self.secp, new_internal_keypair.x_only_public_key().0.into()) .expect("should be finalizable"); self.current_spend_info = Some(taproot_spend_info.clone()); let prevout_script_pubkey = input.witness_utxo.as_ref().unwrap().script_pubkey.clone(); @@ -608,7 +608,7 @@ impl BenefactorWallet { tap_key_origins: origins, tap_merkle_root: taproot_spend_info.merkle_root(), sighash_type: Some(ty), - tap_internal_key: Some(new_internal_keypair.x_only_public_key().0), + tap_internal_key: Some(new_internal_keypair.x_only_public_key().0.into()), tap_scripts, ..Default::default() }; diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index 167de2292..6950cbb4a 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -50,7 +50,7 @@ use bech32::primitives::gf32::Fe32; use bech32::primitives::hrp::Hrp; use hashes::{hash160, HashEngine}; use internals::array::ArrayExt; -use secp256k1::{Secp256k1, Verification, XOnlyPublicKey}; +use secp256k1::{Secp256k1, Verification}; use crate::address::script_pubkey::ScriptBufExt as _; use crate::constants::{ @@ -69,6 +69,7 @@ use crate::script::{ WitnessScriptSizeError, }; use crate::taproot::TapNodeHash; +use crate::XOnlyPublicKey; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] diff --git a/bitcoin/src/bip32.rs b/bitcoin/src/bip32.rs index 7d20f58ac..07d42a8b0 100644 --- a/bitcoin/src/bip32.rs +++ b/bitcoin/src/bip32.rs @@ -13,12 +13,13 @@ use core::{fmt, slice}; use hashes::{hash160, hash_newtype, sha512, Hash, HashEngine, Hmac, HmacEngine}; use internals::array::ArrayExt; use internals::write_err; -use secp256k1::{Secp256k1, XOnlyPublicKey}; +use secp256k1::Secp256k1; use crate::crypto::key::{CompressedPublicKey, Keypair, PrivateKey}; use crate::internal_macros::{impl_array_newtype, impl_array_newtype_stringify}; use crate::network::NetworkKind; use crate::prelude::{String, Vec}; +use crate::XOnlyPublicKey; /// Version bytes for extended public keys on the Bitcoin network. const VERSION_BYTES_MAINNET_PUBLIC: [u8; 4] = [0x04, 0x88, 0xB2, 0x1E]; diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 45a232941..816e0f513 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -25,11 +25,110 @@ use crate::script::{self, ScriptBuf}; use crate::taproot::{TapNodeHash, TapTweakHash}; #[rustfmt::skip] // Keep public re-exports separate. -pub use secp256k1::{constants, Keypair, Parity, Secp256k1, Verification, XOnlyPublicKey}; +pub use secp256k1::{constants, Keypair, Parity, Secp256k1, Verification}; #[cfg(feature = "rand-std")] pub use secp256k1::rand; pub use serialized_x_only::SerializedXOnlyPublicKey; +/// A Bitcoin Schnorr X-only public key used for BIP340 signatures. +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +pub struct XOnlyPublicKey(secp256k1::XOnlyPublicKey); + +impl XOnlyPublicKey { + /// Constructs a new x-only public key from the provided generic Secp256k1 x-only public key. + pub fn new(key: impl Into) -> XOnlyPublicKey { + XOnlyPublicKey(key.into()) + } + + /// Creates an x-only public key from a keypair. + /// + /// Returns the x-only public key and the parity of the full public key. + #[inline] + pub fn from_keypair(keypair: &Keypair) -> (XOnlyPublicKey, Parity) { + let (xonly, parity) = secp256k1::XOnlyPublicKey::from_keypair(keypair); + (XOnlyPublicKey::new(xonly), parity) + } + + /// Creates an x-only public key from a 32-byte x-coordinate. + /// + /// Returns an error if the provided bytes don't represent a valid secp256k1 point x-coordinate. + #[inline] + pub fn from_byte_array( + data: &[u8; constants::SCHNORR_PUBLIC_KEY_SIZE], + ) -> Result { + secp256k1::XOnlyPublicKey::from_byte_array(data) + .map(XOnlyPublicKey::new) + .map_err(|_| ParseXOnlyPublicKeyError::InvalidXCoordinate) + } + + /// Serializes the x-only public key as a byte-encoded x coordinate value (32 bytes). + #[inline] + pub fn serialize(&self) -> [u8; constants::SCHNORR_PUBLIC_KEY_SIZE] { self.0.serialize() } + + /// Converts this x-only public key to a full public key given the parity. + #[inline] + pub fn public_key(&self, parity: Parity) -> PublicKey { self.0.public_key(parity).into() } + + /// Verifies that a tweak produced by [`XOnlyPublicKey::add_tweak`] was computed correctly. + /// + /// Should be called on the original untweaked key. Takes the tweaked key and output parity from + /// [`XOnlyPublicKey::add_tweak`] as input. + #[inline] + pub fn tweak_add_check( + &self, + secp: &Secp256k1, + tweaked_key: &Self, + tweaked_parity: Parity, + tweak: secp256k1::Scalar, + ) -> bool { + self.0.tweak_add_check(secp, &tweaked_key.0, tweaked_parity, tweak) + } + + /// Tweaks an [`XOnlyPublicKey`] by adding the generator multiplied with the given tweak to it. + /// + /// # Returns + /// + /// The newly tweaked key plus an opaque type representing the parity of the tweaked key, this + /// should be provided to `tweak_add_check` which can be used to verify a tweak more efficiently + /// than regenerating it and checking equality. + /// + /// # Errors + /// + /// If the resulting key would be invalid. + #[inline] + pub fn add_tweak( + &self, + secp: &Secp256k1, + tweak: &secp256k1::Scalar, + ) -> Result<(XOnlyPublicKey, Parity), TweakXOnlyPublicKeyError> { + match self.0.add_tweak(secp, tweak) { + Ok((xonly, parity)) => Ok((XOnlyPublicKey(xonly), parity)), + Err(secp256k1::Error::InvalidTweak) => Err(TweakXOnlyPublicKeyError::BadTweak), + Err(secp256k1::Error::InvalidParityValue(_)) => + Err(TweakXOnlyPublicKeyError::ParityError), + Err(_) => Err(TweakXOnlyPublicKeyError::ResultKeyInvalid), + } + } +} + +impl FromStr for XOnlyPublicKey { + type Err = ParseXOnlyPublicKeyError; + fn from_str(s: &str) -> Result { + secp256k1::XOnlyPublicKey::from_str(s) + .map(XOnlyPublicKey::from) + .map_err(|_| ParseXOnlyPublicKeyError::InvalidXCoordinate) + } +} + +impl From for XOnlyPublicKey { + fn from(pk: secp256k1::XOnlyPublicKey) -> XOnlyPublicKey { XOnlyPublicKey::new(pk) } +} + +impl From for XOnlyPublicKey { + fn from(pk: secp256k1::PublicKey) -> XOnlyPublicKey { XOnlyPublicKey::new(pk) } +} + /// A Bitcoin ECDSA public key. #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct PublicKey { @@ -222,7 +321,7 @@ impl From for PublicKey { } impl From for XOnlyPublicKey { - fn from(pk: PublicKey) -> XOnlyPublicKey { pk.inner.into() } + fn from(pk: PublicKey) -> XOnlyPublicKey { XOnlyPublicKey::new(pk.inner) } } /// An opaque return type for PublicKey::to_sort_key. @@ -743,13 +842,13 @@ pub type UntweakedPublicKey = XOnlyPublicKey; pub struct TweakedPublicKey(XOnlyPublicKey); impl fmt::LowerHex for TweakedPublicKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0, f) } + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0 .0, f) } } // Allocate for serialized size impl_to_hex_from_lower_hex!(TweakedPublicKey, |_| constants::SCHNORR_PUBLIC_KEY_SIZE * 2); impl fmt::Display for TweakedPublicKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0 .0, f) } } /// Untweaked BIP-340 key pair. @@ -874,7 +973,7 @@ impl TweakedPublicKey { #[inline] pub fn from_keypair(keypair: TweakedKeypair) -> Self { let (xonly, _parity) = keypair.0.x_only_public_key(); - TweakedPublicKey(xonly) + TweakedPublicKey(xonly.into()) } /// Constructs a new [`TweakedPublicKey`] from a [`XOnlyPublicKey`]. No tweak is applied, consider @@ -914,7 +1013,7 @@ impl TweakedKeypair { #[inline] pub fn public_parts(&self) -> (TweakedPublicKey, Parity) { let (xonly, parity) = self.0.x_only_public_key(); - (TweakedPublicKey(xonly), parity) + (TweakedPublicKey(xonly.into()), parity) } } @@ -1246,7 +1345,7 @@ mod serialized_x_only { impl SerializedXOnlyPublicKey { /// Returns `XOnlyPublicKey` if the bytes are valid. - pub fn to_validated(self) -> Result { + pub fn to_validated(self) -> Result { XOnlyPublicKey::from_byte_array(self.as_byte_array()) } } @@ -1265,6 +1364,48 @@ impl fmt::Debug for SerializedXOnlyPublicKey { } } +/// Error that can occur when parsing an [`XOnlyPublicKey`] from bytes. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ParseXOnlyPublicKeyError { + /// The provided bytes do not represent a valid secp256k1 point x-coordinate. + InvalidXCoordinate, +} + +impl fmt::Display for ParseXOnlyPublicKeyError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::InvalidXCoordinate => write!(f, "Invalid X coordinate for secp256k1 point"), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ParseXOnlyPublicKeyError {} + +/// Error that can occur when tweaking an [`XOnlyPublicKey`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TweakXOnlyPublicKeyError { + /// The tweak value was invalid. + BadTweak, + /// The resulting public key would be invalid. + ResultKeyInvalid, + /// Invalid parity value encountered during the operation. + ParityError, +} + +impl fmt::Display for TweakXOnlyPublicKeyError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::BadTweak => write!(f, "Invalid tweak value"), + Self::ResultKeyInvalid => write!(f, "Resulting public key would be invalid"), + Self::ParityError => write!(f, "Invalid parity value encountered"), + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for TweakXOnlyPublicKeyError {} + #[cfg(test)] mod tests { use super::*; diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index b91816768..6e88d9417 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -1878,10 +1878,11 @@ mod tests { }) } - use secp256k1::{SecretKey, XOnlyPublicKey}; + use secp256k1::SecretKey; use crate::consensus::serde as con_serde; use crate::taproot::{TapNodeHash, TapTweakHash}; + use crate::XOnlyPublicKey; #[derive(serde::Deserialize)] struct UtxoSpent { diff --git a/bitcoin/src/psbt/map/input.rs b/bitcoin/src/psbt/map/input.rs index 8f1f27537..1d021f3a6 100644 --- a/bitcoin/src/psbt/map/input.rs +++ b/bitcoin/src/psbt/map/input.rs @@ -4,7 +4,6 @@ use core::fmt; use core::str::FromStr; use hashes::{hash160, ripemd160, sha256, sha256d}; -use secp256k1::XOnlyPublicKey; use crate::bip32::KeySource; use crate::crypto::key::PublicKey; @@ -21,6 +20,7 @@ use crate::sighash::{ use crate::taproot::{ControlBlock, LeafVersion, TapLeafHash, TapNodeHash}; use crate::transaction::{Transaction, TxOut}; use crate::witness::Witness; +use crate::XOnlyPublicKey; /// Type: Non-Witness UTXO PSBT_IN_NON_WITNESS_UTXO = 0x00 const PSBT_IN_NON_WITNESS_UTXO: u64 = 0x00; diff --git a/bitcoin/src/psbt/map/output.rs b/bitcoin/src/psbt/map/output.rs index 4829e39f5..cd0f1dbc5 100644 --- a/bitcoin/src/psbt/map/output.rs +++ b/bitcoin/src/psbt/map/output.rs @@ -1,13 +1,12 @@ // SPDX-License-Identifier: CC0-1.0 -use secp256k1::XOnlyPublicKey; - use crate::bip32::KeySource; use crate::prelude::{btree_map, BTreeMap, Vec}; use crate::psbt::map::Map; use crate::psbt::{raw, Error}; use crate::script::ScriptBuf; use crate::taproot::{TapLeafHash, TapTree}; +use crate::XOnlyPublicKey; /// Type: Redeem ScriptBuf PSBT_OUT_REDEEM_SCRIPT = 0x00 const PSBT_OUT_REDEEM_SCRIPT: u64 = 0x00; diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index 9b3dfad59..13fa546cf 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -850,14 +850,14 @@ impl GetKey for $map { match key_request { KeyRequest::Pubkey(pk) => Ok(self.get(&pk).cloned()), KeyRequest::XOnlyPubkey(xonly) => { - let pubkey_even = PublicKey::new(xonly.public_key(secp256k1::Parity::Even)); + let pubkey_even = xonly.public_key(secp256k1::Parity::Even); let key = self.get(&pubkey_even).cloned(); if key.is_some() { return Ok(key); } - let pubkey_odd = PublicKey::new(xonly.public_key(secp256k1::Parity::Odd)); + let pubkey_odd = xonly.public_key(secp256k1::Parity::Odd); if let Some(priv_key) = self.get(&pubkey_odd).copied() { let negated_priv_key = priv_key.negate(); return Ok(Some(negated_priv_key)); @@ -2257,7 +2257,7 @@ mod tests { pubkey_map.insert(pk, priv_key); - let req_result = pubkey_map.get_key(&KeyRequest::XOnlyPubkey(xonly), &secp).unwrap(); + let req_result = pubkey_map.get_key(&KeyRequest::XOnlyPubkey(xonly.into()), &secp).unwrap(); let retrieved_key = req_result.unwrap(); diff --git a/bitcoin/src/psbt/serialize.rs b/bitcoin/src/psbt/serialize.rs index 25eef99c6..ed3c682d1 100644 --- a/bitcoin/src/psbt/serialize.rs +++ b/bitcoin/src/psbt/serialize.rs @@ -9,7 +9,6 @@ use hashes::{hash160, ripemd160, sha256, sha256d}; use internals::compact_size; #[allow(unused)] // MSRV polyfill use internals::slice::SliceExt; -use secp256k1::XOnlyPublicKey; use super::map::{Input, Map, Output, PsbtSighashType}; use crate::bip32::{ChildNumber, Fingerprint, KeySource}; @@ -25,6 +24,7 @@ use crate::taproot::{ }; use crate::transaction::{Transaction, TxOut}; use crate::witness::Witness; +use crate::XOnlyPublicKey; /// A trait for serializing a value as raw data for insertion into PSBT /// key-value maps. diff --git a/bitcoin/src/taproot/mod.rs b/bitcoin/src/taproot/mod.rs index e4a60a35b..397002c1a 100644 --- a/bitcoin/src/taproot/mod.rs +++ b/bitcoin/src/taproot/mod.rs @@ -25,6 +25,7 @@ use crate::consensus::Encodable; use crate::crypto::key::{ SerializedXOnlyPublicKey, TapTweak, TweakedPublicKey, UntweakedPublicKey, }; +use crate::key::ParseXOnlyPublicKeyError; use crate::prelude::{BTreeMap, BTreeSet, BinaryHeap, Vec}; use crate::{Script, ScriptBuf}; @@ -1523,7 +1524,7 @@ pub enum TaprootError { /// Invalid control block size. InvalidControlBlockSize(InvalidControlBlockSizeError), /// Invalid Taproot internal key. - InvalidInternalKey(secp256k1::Error), + InvalidInternalKey(ParseXOnlyPublicKeyError), /// Invalid control block hex InvalidControlBlockHex(HexToBytesError), } diff --git a/bitcoin/tests/psbt-sign-taproot.rs b/bitcoin/tests/psbt-sign-taproot.rs index 78f3a77b1..623386924 100644 --- a/bitcoin/tests/psbt-sign-taproot.rs +++ b/bitcoin/tests/psbt-sign-taproot.rs @@ -11,9 +11,9 @@ use bitcoin::taproot::{LeafVersion, TaprootBuilder, TaprootSpendInfo}; use bitcoin::transaction::Version; use bitcoin::{ absolute, script, Address, Network, OutPoint, PrivateKey, Psbt, ScriptBuf, Sequence, - Transaction, TxIn, TxOut, Witness, + Transaction, TxIn, TxOut, Witness, XOnlyPublicKey, }; -use secp256k1::{Keypair, Secp256k1, Signing, XOnlyPublicKey}; +use secp256k1::{Keypair, Secp256k1, Signing}; use units::Amount; #[test] @@ -66,7 +66,7 @@ fn psbt_sign_taproot() { let internal_key = kp.x_only_public_key().0; // Ignore the parity. let tree = - create_taproot_tree(secp, script1.clone(), script2.clone(), script3.clone(), internal_key); + create_taproot_tree(secp, script1.clone(), script2.clone(), script3.clone(), internal_key.into()); let address = create_p2tr_address(tree.clone()); assert_eq!( @@ -131,7 +131,7 @@ fn psbt_sign_taproot() { address, to_address, tree.clone(), - x_only_pubkey, + x_only_pubkey.into(), signing_key_path, script2.clone(), ); @@ -146,7 +146,7 @@ fn psbt_sign_taproot() { sig, psbt_script_path_spend.inputs[0] .tap_script_sigs - .get(&(x_only_pubkey, script2.clone().tapscript_leaf_hash())) + .get(&(x_only_pubkey.into(), script2.clone().tapscript_leaf_hash())) .unwrap() .signature .to_string() From c11772a768eefd89dcc0e3b1a369d535c191f94a Mon Sep 17 00:00:00 2001 From: Erick Cestari Date: Mon, 28 Apr 2025 14:28:11 -0300 Subject: [PATCH 2/2] Accept flexible input types for Taproot-related functions Refactor Taproot functions to accept any type implementing `Into`, instead of requiring `XOnlyPublicKey` directly. This improves ergonomics when working with compatible types, avoiding unnecessary `.into()` conversions at call sites. --- bitcoin/examples/sign-tx-taproot.rs | 9 +++---- bitcoin/examples/taproot-psbt-simple.rs | 11 +++++---- bitcoin/examples/taproot-psbt.rs | 4 ++-- bitcoin/src/address/mod.rs | 7 +++--- bitcoin/src/address/script_pubkey.rs | 10 ++++---- bitcoin/src/bip32.rs | 3 +-- .../src/blockdata/script/witness_program.rs | 5 ++-- bitcoin/src/crypto/key.rs | 14 +++++++++-- bitcoin/src/crypto/sighash.rs | 2 +- bitcoin/src/psbt/map/input.rs | 3 +-- bitcoin/src/psbt/map/output.rs | 2 +- bitcoin/src/psbt/serialize.rs | 3 +-- bitcoin/src/taproot/mod.rs | 24 +++++++++++-------- bitcoin/tests/psbt-sign-taproot.rs | 14 ++++++----- 14 files changed, 65 insertions(+), 46 deletions(-) diff --git a/bitcoin/examples/sign-tx-taproot.rs b/bitcoin/examples/sign-tx-taproot.rs index 616d4c3a9..80a1005ec 100644 --- a/bitcoin/examples/sign-tx-taproot.rs +++ b/bitcoin/examples/sign-tx-taproot.rs @@ -26,7 +26,7 @@ fn main() { // Get an unspent output that is locked to the key above that we control. // In a real application these would come from the chain. - let (dummy_out_point, dummy_utxo) = dummy_unspent_transaction_output(&secp, internal_key.into()); + let (dummy_out_point, dummy_utxo) = dummy_unspent_transaction_output(&secp, internal_key); // Get an address to send to. let address = receivers_address(); @@ -45,7 +45,7 @@ fn main() { // The change output is locked to a key controlled by us. let change = TxOut { value: CHANGE_AMOUNT, - script_pubkey: ScriptBuf::new_p2tr(&secp, internal_key.into(), None), // Change comes back to us. + script_pubkey: ScriptBuf::new_p2tr(&secp, internal_key, None), // Change comes back to us. }; // The transaction we want to sign and broadcast. @@ -113,10 +113,11 @@ fn receivers_address() -> Address { /// /// This output is locked to keys that we control, in a real application this would be a valid /// output taken from a transaction that appears in the chain. -fn dummy_unspent_transaction_output( +fn dummy_unspent_transaction_output>( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, ) -> (OutPoint, TxOut) { + let internal_key = internal_key.into(); let script_pubkey = ScriptBuf::new_p2tr(secp, internal_key, None); let out_point = OutPoint { diff --git a/bitcoin/examples/taproot-psbt-simple.rs b/bitcoin/examples/taproot-psbt-simple.rs index 425ffdc20..409a819f7 100644 --- a/bitcoin/examples/taproot-psbt-simple.rs +++ b/bitcoin/examples/taproot-psbt-simple.rs @@ -83,11 +83,12 @@ fn get_internal_address_xpriv( } // Get the Taproot Key Origin. -fn get_tap_key_origin( - x_only_key: UntweakedPublicKey, +fn get_tap_key_origin + std::cmp::Ord>( + x_only_key: K, master_fingerprint: Fingerprint, path: DerivationPath, ) -> BTreeMap, (Fingerprint, DerivationPath))> { + let x_only_key = x_only_key.into(); let mut map = BTreeMap::new(); map.insert(x_only_key, (vec![], (master_fingerprint, path))); map @@ -151,12 +152,12 @@ fn main() { // Get the Tap Key Origins // Map of tap root X-only keys to origin info and leaf hashes contained in it. let origin_input_1 = get_tap_key_origin( - pk_input_1.into(), + pk_input_1, MASTER_FINGERPRINT.parse::().unwrap(), "m/86'/0'/0'/0/0".parse::().unwrap(), ); let origin_input_2 = get_tap_key_origin( - pk_input_2.into(), + pk_input_2, MASTER_FINGERPRINT.parse::().unwrap(), "m/86'/0'/0'/1/0".parse::().unwrap(), ); @@ -187,7 +188,7 @@ fn main() { // The change output is locked to a key controlled by us. let change = TxOut { value: CHANGE_AMOUNT, - script_pubkey: ScriptBuf::new_p2tr(&secp, pk_change.into(), None), // Change comes back to us. + script_pubkey: ScriptBuf::new_p2tr(&secp, pk_change, None), // Change comes back to us. }; // The transaction we want to sign and broadcast. diff --git a/bitcoin/examples/taproot-psbt.rs b/bitcoin/examples/taproot-psbt.rs index 86df3114e..2b6207413 100644 --- a/bitcoin/examples/taproot-psbt.rs +++ b/bitcoin/examples/taproot-psbt.rs @@ -404,7 +404,7 @@ impl BenefactorWallet { let taproot_spend_info = TaprootBuilder::new() .add_leaf(0, script.clone())? - .finalize(&self.secp, internal_keypair.x_only_public_key().0.into()) + .finalize(&self.secp, internal_keypair.x_only_public_key().0) .expect("should be finalizable"); self.current_spend_info = Some(taproot_spend_info.clone()); let script_pubkey = ScriptBuf::new_p2tr( @@ -502,7 +502,7 @@ impl BenefactorWallet { let taproot_spend_info = TaprootBuilder::new() .add_leaf(0, script.clone())? - .finalize(&self.secp, new_internal_keypair.x_only_public_key().0.into()) + .finalize(&self.secp, new_internal_keypair.x_only_public_key().0) .expect("should be finalizable"); self.current_spend_info = Some(taproot_spend_info.clone()); let prevout_script_pubkey = input.witness_utxo.as_ref().unwrap().script_pubkey.clone(); diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index 6950cbb4a..919de6c04 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -59,6 +59,7 @@ use crate::constants::{ }; use crate::crypto::key::{ CompressedPublicKey, PubkeyHash, PublicKey, TweakedPublicKey, UntweakedPublicKey, + XOnlyPublicKey, }; use crate::network::{Network, NetworkKind, Params}; use crate::prelude::{String, ToOwned}; @@ -69,7 +70,6 @@ use crate::script::{ WitnessScriptSizeError, }; use crate::taproot::TapNodeHash; -use crate::XOnlyPublicKey; #[rustfmt::skip] // Keep public re-exports separate. #[doc(inline)] @@ -573,12 +573,13 @@ impl Address { } /// Constructs a new pay-to-Taproot (P2TR) [`Address`] from an untweaked key. - pub fn p2tr( + pub fn p2tr>( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, merkle_root: Option, hrp: impl Into, ) -> Address { + let internal_key = internal_key.into(); let program = WitnessProgram::p2tr(secp, internal_key, merkle_root); Address::from_witness_program(program, hrp) } diff --git a/bitcoin/src/address/script_pubkey.rs b/bitcoin/src/address/script_pubkey.rs index a8f902667..bc3637de3 100644 --- a/bitcoin/src/address/script_pubkey.rs +++ b/bitcoin/src/address/script_pubkey.rs @@ -49,11 +49,12 @@ define_extension_trait! { /// Computes P2TR output with a given internal key and a single script spending path equal to /// the current script, assuming that the script is a Tapscript. - fn to_p2tr( + fn to_p2tr>( &self, secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, ) -> ScriptBuf { + let internal_key = internal_key.into(); let leaf_hash = self.tapscript_leaf_hash(); let merkle_root = TapNodeHash::from(leaf_hash); ScriptBuf::new_p2tr(secp, internal_key, Some(merkle_root)) @@ -157,11 +158,12 @@ define_extension_trait! { /// Generates P2TR for script spending path using an internal public key and some optional /// script tree Merkle root. - fn new_p2tr( + fn new_p2tr>( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, merkle_root: Option, ) -> Self { + let internal_key = internal_key.into(); let (output_key, _) = internal_key.tap_tweak(secp, merkle_root); // output key is 32 bytes long, so it's safe to use `new_witness_program_unchecked` (Segwitv1) new_witness_program_unchecked(WitnessVersion::V1, output_key.serialize()) diff --git a/bitcoin/src/bip32.rs b/bitcoin/src/bip32.rs index 07d42a8b0..ebb59064b 100644 --- a/bitcoin/src/bip32.rs +++ b/bitcoin/src/bip32.rs @@ -15,11 +15,10 @@ use internals::array::ArrayExt; use internals::write_err; use secp256k1::Secp256k1; -use crate::crypto::key::{CompressedPublicKey, Keypair, PrivateKey}; +use crate::crypto::key::{CompressedPublicKey, Keypair, PrivateKey, XOnlyPublicKey}; use crate::internal_macros::{impl_array_newtype, impl_array_newtype_stringify}; use crate::network::NetworkKind; use crate::prelude::{String, Vec}; -use crate::XOnlyPublicKey; /// Version bytes for extended public keys on the Bitcoin network. const VERSION_BYTES_MAINNET_PUBLIC: [u8; 4] = [0x04, 0x88, 0xB2, 0x1E]; diff --git a/bitcoin/src/blockdata/script/witness_program.rs b/bitcoin/src/blockdata/script/witness_program.rs index f2780f85b..e43602627 100644 --- a/bitcoin/src/blockdata/script/witness_program.rs +++ b/bitcoin/src/blockdata/script/witness_program.rs @@ -95,11 +95,12 @@ impl WitnessProgram { /// /// This function applies BIP341 key-tweaking to the untweaked /// key using the merkle root, if it's present. - pub fn p2tr( + pub fn p2tr>( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, merkle_root: Option, ) -> Self { + let internal_key = internal_key.into(); let (output_key, _parity) = internal_key.tap_tweak(secp, merkle_root); let pubkey = output_key.to_inner().serialize(); WitnessProgram::new_p2tr(pubkey) diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 816e0f513..c54365bfe 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -129,6 +129,16 @@ impl From for XOnlyPublicKey { fn from(pk: secp256k1::PublicKey) -> XOnlyPublicKey { XOnlyPublicKey::new(pk) } } +impl fmt::LowerHex for XOnlyPublicKey { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0, f) } +} +// Allocate for serialized size +impl_to_hex_from_lower_hex!(XOnlyPublicKey, |_| constants::SCHNORR_PUBLIC_KEY_SIZE * 2); + +impl fmt::Display for XOnlyPublicKey { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } +} + /// A Bitcoin ECDSA public key. #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct PublicKey { @@ -842,13 +852,13 @@ pub type UntweakedPublicKey = XOnlyPublicKey; pub struct TweakedPublicKey(XOnlyPublicKey); impl fmt::LowerHex for TweakedPublicKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0 .0, f) } + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::LowerHex::fmt(&self.0, f) } } // Allocate for serialized size impl_to_hex_from_lower_hex!(TweakedPublicKey, |_| constants::SCHNORR_PUBLIC_KEY_SIZE * 2); impl fmt::Display for TweakedPublicKey { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0 .0, f) } + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } } /// Untweaked BIP-340 key pair. diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 6e88d9417..612f51c6a 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -1881,8 +1881,8 @@ mod tests { use secp256k1::SecretKey; use crate::consensus::serde as con_serde; + use crate::crypto::key::XOnlyPublicKey; use crate::taproot::{TapNodeHash, TapTweakHash}; - use crate::XOnlyPublicKey; #[derive(serde::Deserialize)] struct UtxoSpent { diff --git a/bitcoin/src/psbt/map/input.rs b/bitcoin/src/psbt/map/input.rs index 1d021f3a6..d33be651b 100644 --- a/bitcoin/src/psbt/map/input.rs +++ b/bitcoin/src/psbt/map/input.rs @@ -6,7 +6,7 @@ use core::str::FromStr; use hashes::{hash160, ripemd160, sha256, sha256d}; use crate::bip32::KeySource; -use crate::crypto::key::PublicKey; +use crate::crypto::key::{PublicKey, XOnlyPublicKey}; use crate::crypto::{ecdsa, taproot}; use crate::prelude::{btree_map, BTreeMap, Borrow, Box, ToOwned, Vec}; use crate::psbt::map::Map; @@ -20,7 +20,6 @@ use crate::sighash::{ use crate::taproot::{ControlBlock, LeafVersion, TapLeafHash, TapNodeHash}; use crate::transaction::{Transaction, TxOut}; use crate::witness::Witness; -use crate::XOnlyPublicKey; /// Type: Non-Witness UTXO PSBT_IN_NON_WITNESS_UTXO = 0x00 const PSBT_IN_NON_WITNESS_UTXO: u64 = 0x00; diff --git a/bitcoin/src/psbt/map/output.rs b/bitcoin/src/psbt/map/output.rs index cd0f1dbc5..22e28abc6 100644 --- a/bitcoin/src/psbt/map/output.rs +++ b/bitcoin/src/psbt/map/output.rs @@ -1,12 +1,12 @@ // SPDX-License-Identifier: CC0-1.0 use crate::bip32::KeySource; +use crate::crypto::key::XOnlyPublicKey; use crate::prelude::{btree_map, BTreeMap, Vec}; use crate::psbt::map::Map; use crate::psbt::{raw, Error}; use crate::script::ScriptBuf; use crate::taproot::{TapLeafHash, TapTree}; -use crate::XOnlyPublicKey; /// Type: Redeem ScriptBuf PSBT_OUT_REDEEM_SCRIPT = 0x00 const PSBT_OUT_REDEEM_SCRIPT: u64 = 0x00; diff --git a/bitcoin/src/psbt/serialize.rs b/bitcoin/src/psbt/serialize.rs index ed3c682d1..f16c514b5 100644 --- a/bitcoin/src/psbt/serialize.rs +++ b/bitcoin/src/psbt/serialize.rs @@ -13,7 +13,7 @@ use internals::slice::SliceExt; use super::map::{Input, Map, Output, PsbtSighashType}; use crate::bip32::{ChildNumber, Fingerprint, KeySource}; use crate::consensus::encode::{self, deserialize_partial, serialize, Decodable, Encodable}; -use crate::crypto::key::PublicKey; +use crate::crypto::key::{PublicKey, XOnlyPublicKey}; use crate::crypto::{ecdsa, taproot}; use crate::io::Write; use crate::prelude::{DisplayHex, String, Vec}; @@ -24,7 +24,6 @@ use crate::taproot::{ }; use crate::transaction::{Transaction, TxOut}; use crate::witness::Witness; -use crate::XOnlyPublicKey; /// A trait for serializing a value as raw data for insertion into PSBT /// key-value maps. diff --git a/bitcoin/src/taproot/mod.rs b/bitcoin/src/taproot/mod.rs index 397002c1a..481fe1904 100644 --- a/bitcoin/src/taproot/mod.rs +++ b/bitcoin/src/taproot/mod.rs @@ -97,10 +97,11 @@ impl From for TapNodeHash { impl TapTweakHash { /// Constructs a new BIP341 [`TapTweakHash`] from key and tweak. Produces `H_taptweak(P||R)` where /// `P` is the internal key and `R` is the Merkle root. - pub fn from_key_and_tweak( - internal_key: UntweakedPublicKey, + pub fn from_key_and_tweak>( + internal_key: K, merkle_root: Option, ) -> TapTweakHash { + let internal_key = internal_key.into(); let mut eng = sha256t::Hash::::engine(); // always hash the key eng.input(&internal_key.serialize()); @@ -248,14 +249,15 @@ impl TaprootSpendInfo { /// weights of satisfaction for that script. /// /// See [`TaprootBuilder::with_huffman_tree`] for more detailed documentation. - pub fn with_huffman_tree( + pub fn with_huffman_tree( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, script_weights: I, ) -> Result where I: IntoIterator, C: secp256k1::Verification, + K: Into, { let builder = TaprootBuilder::with_huffman_tree(script_weights)?; Ok(builder.finalize(secp, internal_key).expect("Huffman tree is always complete")) @@ -272,11 +274,12 @@ impl TaprootSpendInfo { /// /// Refer to BIP 341 footnote ('Why should the output key always have a Taproot commitment, even /// if there is no script path?') for more details. - pub fn new_key_spend( + pub fn new_key_spend>( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, merkle_root: Option, ) -> Self { + let internal_key = internal_key.into(); let (output_key, parity) = internal_key.tap_tweak(secp, merkle_root); Self { internal_key, @@ -312,9 +315,9 @@ impl TaprootSpendInfo { /// /// This is useful when you want to manually build a Taproot tree without using /// [`TaprootBuilder`]. - pub fn from_node_info( + pub fn from_node_info>( secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, node: NodeInfo, ) -> TaprootSpendInfo { // Create as if it is a key spend path with the given Merkle root @@ -583,11 +586,12 @@ impl TaprootBuilder { /// /// Returns the unmodified builder as Err if the builder is not finalizable. /// See also [`TaprootBuilder::is_finalizable`] - pub fn finalize( + pub fn finalize>( mut self, secp: &Secp256k1, - internal_key: UntweakedPublicKey, + internal_key: K, ) -> Result { + let internal_key = internal_key.into(); match self.branch.len() { 0 => Ok(TaprootSpendInfo::new_key_spend(secp, internal_key, None)), 1 => diff --git a/bitcoin/tests/psbt-sign-taproot.rs b/bitcoin/tests/psbt-sign-taproot.rs index 623386924..e692cb1b5 100644 --- a/bitcoin/tests/psbt-sign-taproot.rs +++ b/bitcoin/tests/psbt-sign-taproot.rs @@ -66,7 +66,7 @@ fn psbt_sign_taproot() { let internal_key = kp.x_only_public_key().0; // Ignore the parity. let tree = - create_taproot_tree(secp, script1.clone(), script2.clone(), script3.clone(), internal_key.into()); + create_taproot_tree(secp, script1.clone(), script2.clone(), script3.clone(), internal_key); let address = create_p2tr_address(tree.clone()); assert_eq!( @@ -131,7 +131,7 @@ fn psbt_sign_taproot() { address, to_address, tree.clone(), - x_only_pubkey.into(), + x_only_pubkey, signing_key_path, script2.clone(), ); @@ -176,13 +176,14 @@ fn create_basic_single_sig_script(secp: &Secp256k1, sk: &str) -> .into_script() } -fn create_taproot_tree( +fn create_taproot_tree>( secp: &Secp256k1, script1: ScriptBuf, script2: ScriptBuf, script3: ScriptBuf, - internal_key: XOnlyPublicKey, + internal_key: K, ) -> TaprootSpendInfo { + let internal_key = internal_key.into(); let builder = TaprootBuilder::new(); let builder = builder.add_leaf(2, script1).unwrap(); let builder = builder.add_leaf(2, script2).unwrap(); @@ -267,14 +268,15 @@ fn finalize_psbt_for_key_path_spend(mut psbt: Psbt) -> Psbt { psbt } -fn create_psbt_for_taproot_script_path_spend( +fn create_psbt_for_taproot_script_path_spend>( from_address: Address, to_address: Address, tree: TaprootSpendInfo, - x_only_pubkey_of_signing_key: XOnlyPublicKey, + x_only_pubkey_of_signing_key: K, signing_key_path: &str, use_script: ScriptBuf, ) -> Psbt { + let x_only_pubkey_of_signing_key = x_only_pubkey_of_signing_key.into(); let utxo_value = 6280; let send_value = 6000; let mfp = "73c5da0a";