From 041d6a80974abb912a309556729f0dbd303f0d08 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 12 Sep 2022 14:50:37 +1000 Subject: [PATCH 1/2] Move and deprecate script_find_and_remove Done as part of flattening the `util` module. We have a function in `util::misc` that operates on scripts, it is an implementation of `FindAndDelete` from Bitcoin Core and is primarily useful for supporting `CODESEPARATOR`, which we do not support. Move the public `script_find_and_remove` function out of `util/misc.rs` and into `util/mod.rs`, delete the testing and deprecate the function. --- bitcoin/src/lib.rs | 1 + bitcoin/src/{util/misc.rs => sign_message.rs} | 91 +------------------ bitcoin/src/util/mod.rs | 51 ++++++++++- 3 files changed, 54 insertions(+), 89 deletions(-) rename bitcoin/src/{util/misc.rs => sign_message.rs} (73%) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 8e320cf8..62e7160d 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -86,6 +86,7 @@ pub mod consensus; pub mod error; pub mod hash_types; pub mod policy; +pub mod sign_message; pub mod util; #[cfg(feature = "std")] diff --git a/bitcoin/src/util/misc.rs b/bitcoin/src/sign_message.rs similarity index 73% rename from bitcoin/src/util/misc.rs rename to bitcoin/src/sign_message.rs index fc1807ca..3b97e9d9 100644 --- a/bitcoin/src/util/misc.rs +++ b/bitcoin/src/sign_message.rs @@ -1,17 +1,14 @@ // Written in 2014 by Andrew Poelstra // SPDX-License-Identifier: CC0-1.0 -//! Miscellaneous functions. +//! Signature //! -//! This module provides various utility functions including secp256k1 signature -//! recovery when library is used with the `secp-recovery` feature. +//! This module provides signature related functions including secp256k1 signature recovery when +//! library is used with the `secp-recovery` feature. //! -use crate::prelude::*; - use crate::hashes::{sha256d, Hash, HashEngine}; -use crate::blockdata::opcodes; use crate::consensus::{encode, Encodable}; #[cfg(feature = "secp-recovery")] @@ -205,47 +202,6 @@ mod message_signing { } } -/// Search for `needle` in the vector `haystack` and remove every -/// instance of it, returning the number of instances removed. -/// Loops through the vector opcode by opcode, skipping pushed data. -pub fn script_find_and_remove(haystack: &mut Vec, needle: &[u8]) -> usize { - if needle.len() > haystack.len() { - return 0; - } - if needle.is_empty() { - return 0; - } - - let mut top = haystack.len() - needle.len(); - let mut n_deleted = 0; - - let mut i = 0; - while i <= top { - if &haystack[i..(i + needle.len())] == needle { - for j in i..top { - haystack.swap(j + needle.len(), j); - } - n_deleted += 1; - // This is ugly but prevents infinite loop in case of overflow - let overflow = top < needle.len(); - top = top.wrapping_sub(needle.len()); - if overflow { - break; - } - } else { - i += match opcodes::All::from((*haystack)[i]).classify(opcodes::ClassifyContext::Legacy) { - opcodes::Class::PushBytes(n) => n as usize + 1, - opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA1) => 2, - opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA2) => 3, - opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) => 5, - _ => 1 - }; - } - } - haystack.truncate(top.wrapping_add(needle.len())); - n_deleted -} - /// Hash message for signature using Bitcoin's message signing format. pub fn signed_msg_hash(msg: &str) -> sha256d::Hash { let mut engine = sha256d::Hash::engine(); @@ -260,47 +216,6 @@ pub fn signed_msg_hash(msg: &str) -> sha256d::Hash { mod tests { use super::*; use crate::hashes::hex::ToHex; - use super::script_find_and_remove; - use super::signed_msg_hash; - - #[test] - fn test_script_find_and_remove() { - let mut v = vec![101u8, 102, 103, 104, 102, 103, 104, 102, 103, 104, 105, 106, 107, 108, 109]; - - assert_eq!(script_find_and_remove(&mut v, &[]), 0); - assert_eq!(script_find_and_remove(&mut v, &[105, 105, 105]), 0); - assert_eq!(v, vec![101, 102, 103, 104, 102, 103, 104, 102, 103, 104, 105, 106, 107, 108, 109]); - - assert_eq!(script_find_and_remove(&mut v, &[105, 106, 107]), 1); - assert_eq!(v, vec![101, 102, 103, 104, 102, 103, 104, 102, 103, 104, 108, 109]); - - assert_eq!(script_find_and_remove(&mut v, &[104, 108, 109]), 1); - assert_eq!(v, vec![101, 102, 103, 104, 102, 103, 104, 102, 103]); - - assert_eq!(script_find_and_remove(&mut v, &[101]), 1); - assert_eq!(v, vec![102, 103, 104, 102, 103, 104, 102, 103]); - - assert_eq!(script_find_and_remove(&mut v, &[102]), 3); - assert_eq!(v, vec![103, 104, 103, 104, 103]); - - assert_eq!(script_find_and_remove(&mut v, &[103, 104]), 2); - assert_eq!(v, vec![103]); - - assert_eq!(script_find_and_remove(&mut v, &[105, 105, 5]), 0); - assert_eq!(script_find_and_remove(&mut v, &[105]), 0); - assert_eq!(script_find_and_remove(&mut v, &[103]), 1); - assert_eq!(v, Vec::::new()); - - assert_eq!(script_find_and_remove(&mut v, &[105, 105, 5]), 0); - assert_eq!(script_find_and_remove(&mut v, &[105]), 0); - } - - #[test] - fn test_script_codesep_remove() { - let mut s = vec![33u8, 3, 132, 121, 160, 250, 153, 140, 211, 82, 89, 162, 239, 10, 122, 92, 104, 102, 44, 20, 116, 248, 140, 203, 109, 8, 167, 103, 123, 190, 199, 242, 32, 65, 173, 171, 33, 3, 132, 121, 160, 250, 153, 140, 211, 82, 89, 162, 239, 10, 122, 92, 104, 102, 44, 20, 116, 248, 140, 203, 109, 8, 167, 103, 123, 190, 199, 242, 32, 65, 173, 171, 81]; - assert_eq!(script_find_and_remove(&mut s, &[171]), 2); - assert_eq!(s, vec![33, 3, 132, 121, 160, 250, 153, 140, 211, 82, 89, 162, 239, 10, 122, 92, 104, 102, 44, 20, 116, 248, 140, 203, 109, 8, 167, 103, 123, 190, 199, 242, 32, 65, 173, 33, 3, 132, 121, 160, 250, 153, 140, 211, 82, 89, 162, 239, 10, 122, 92, 104, 102, 44, 20, 116, 248, 140, 203, 109, 8, 167, 103, 123, 190, 199, 242, 32, 65, 173, 81]); - } #[test] fn test_signed_msg_hash() { diff --git a/bitcoin/src/util/mod.rs b/bitcoin/src/util/mod.rs index 2ff777b7..6d559214 100644 --- a/bitcoin/src/util/mod.rs +++ b/bitcoin/src/util/mod.rs @@ -15,7 +15,6 @@ pub mod bip32; pub mod bip152; pub mod hash; pub mod merkleblock; -pub mod misc; pub mod psbt; pub mod taproot; pub mod uint; @@ -119,3 +118,53 @@ pub mod address { #[deprecated(since = "0.30.0", note = "Please use crate::bip158")] pub use crate::bip158; + +/// The `misc` module was moved and re-named to `sign_message`. +pub mod misc { + use crate::prelude::*; + + /// Search for `needle` in the vector `haystack` and remove every + /// instance of it, returning the number of instances removed. + /// Loops through the vector opcode by opcode, skipping pushed data. + // For why we deprecated see: https://github.com/rust-bitcoin/rust-bitcoin/pull/1259#discussion_r968613736 + #[deprecated(since = "0.30.0", note = "No longer supported")] + pub fn script_find_and_remove(haystack: &mut Vec, needle: &[u8]) -> usize { + use crate::blockdata::opcodes; + + if needle.len() > haystack.len() { + return 0; + } + if needle.is_empty() { + return 0; + } + + let mut top = haystack.len() - needle.len(); + let mut n_deleted = 0; + + let mut i = 0; + while i <= top { + if &haystack[i..(i + needle.len())] == needle { + for j in i..top { + haystack.swap(j + needle.len(), j); + } + n_deleted += 1; + // This is ugly but prevents infinite loop in case of overflow + let overflow = top < needle.len(); + top = top.wrapping_sub(needle.len()); + if overflow { + break; + } + } else { + i += match opcodes::All::from((*haystack)[i]).classify(opcodes::ClassifyContext::Legacy) { + opcodes::Class::PushBytes(n) => n as usize + 1, + opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA1) => 2, + opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA2) => 3, + opcodes::Class::Ordinary(opcodes::Ordinary::OP_PUSHDATA4) => 5, + _ => 1 + }; + } + } + haystack.truncate(top.wrapping_add(needle.len())); + n_deleted + } +} From e24c91e9ca22a9a26605fdd03a996f7ea21ab853 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 12 Sep 2022 15:00:41 +1000 Subject: [PATCH 2/2] sign_message: Run cargo fmt We just renamed and moved the `util::misc` module to `crate::sign_message`, doing so prevents `rustfmt` from ignoring it. run `cargo +nighly fmt`. --- bitcoin/src/sign_message.rs | 79 +++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/bitcoin/src/sign_message.rs b/bitcoin/src/sign_message.rs index 3b97e9d9..f85795a1 100644 --- a/bitcoin/src/sign_message.rs +++ b/bitcoin/src/sign_message.rs @@ -7,30 +7,28 @@ //! library is used with the `secp-recovery` feature. //! -use crate::hashes::{sha256d, Hash, HashEngine}; - -use crate::consensus::{encode, Encodable}; - #[cfg(feature = "secp-recovery")] #[cfg_attr(docsrs, doc(cfg(feature = "secp-recovery")))] pub use self::message_signing::{MessageSignature, MessageSignatureError}; +use crate::consensus::{encode, Encodable}; +use crate::hashes::{sha256d, Hash, HashEngine}; /// The prefix for signed messages using Bitcoin's message signing protocol. pub const BITCOIN_SIGNED_MSG_PREFIX: &[u8] = b"\x18Bitcoin Signed Message:\n"; #[cfg(feature = "secp-recovery")] mod message_signing { - #[cfg(feature = "base64")] use crate::prelude::*; - use core::fmt; use bitcoin_internals::write_err; - use crate::hashes::sha256d; use secp256k1; - use secp256k1::ecdsa::{RecoveryId, RecoverableSignature}; + use secp256k1::ecdsa::{RecoverableSignature, RecoveryId}; - use crate::util::key::PublicKey; use crate::address::{Address, AddressType}; + use crate::hashes::sha256d; + #[cfg(feature = "base64")] + use crate::prelude::*; + use crate::util::key::PublicKey; /// An error used for dealing with Bitcoin Signed Messages. #[cfg_attr(docsrs, doc(cfg(feature = "secp-recovery")))] @@ -51,9 +49,11 @@ mod message_signing { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { MessageSignatureError::InvalidLength => write!(f, "length not 65 bytes"), - MessageSignatureError::InvalidEncoding(ref e) => write_err!(f, "invalid encoding"; e), + MessageSignatureError::InvalidEncoding(ref e) => + write_err!(f, "invalid encoding"; e), MessageSignatureError::InvalidBase64 => write!(f, "invalid base64"), - MessageSignatureError::UnsupportedAddressType(ref address_type) => write!(f, "unsupported address type: {}", address_type), + MessageSignatureError::UnsupportedAddressType(ref address_type) => + write!(f, "unsupported address type: {}", address_type), } } } @@ -95,10 +95,7 @@ mod message_signing { impl MessageSignature { /// Create a new [MessageSignature]. pub fn new(signature: RecoverableSignature, compressed: bool) -> MessageSignature { - MessageSignature { - signature, - compressed, - } + MessageSignature { signature, compressed } } /// Serialize to bytes. @@ -121,7 +118,9 @@ mod message_signing { } // We just check this here so we can safely subtract further. if bytes[0] < 27 { - return Err(MessageSignatureError::InvalidEncoding(secp256k1::Error::InvalidRecoveryId)); + return Err(MessageSignatureError::InvalidEncoding( + secp256k1::Error::InvalidRecoveryId, + )); }; let recid = RecoveryId::from_i32(((bytes[0] - 27) & 0x03) as i32)?; Ok(MessageSignature { @@ -136,14 +135,11 @@ mod message_signing { pub fn recover_pubkey( &self, secp_ctx: &secp256k1::Secp256k1, - msg_hash: sha256d::Hash + msg_hash: sha256d::Hash, ) -> Result { let msg = secp256k1::Message::from(msg_hash); let pubkey = secp_ctx.recover_ecdsa(&msg, &self.signature)?; - Ok(PublicKey { - inner: pubkey, - compressed: self.compressed, - }) + Ok(PublicKey { inner: pubkey, compressed: self.compressed }) } /// Verify that the signature signs the message and was signed by the given address. @@ -153,14 +149,15 @@ mod message_signing { &self, secp_ctx: &secp256k1::Secp256k1, address: &Address, - msg_hash: sha256d::Hash + msg_hash: sha256d::Hash, ) -> Result { match address.address_type() { Some(AddressType::P2pkh) => { let pubkey = self.recover_pubkey(secp_ctx, msg_hash)?; Ok(*address == Address::p2pkh(&pubkey, address.network)) } - Some(address_type) => Err(MessageSignatureError::UnsupportedAddressType(address_type)), + Some(address_type) => + Err(MessageSignatureError::UnsupportedAddressType(address_type)), None => Ok(false), } } @@ -176,9 +173,7 @@ mod message_signing { /// Convert to base64 encoding. #[cfg(feature = "base64")] #[cfg_attr(docsrs, doc(cfg(feature = "base64")))] - pub fn to_base64(self) -> String { - base64::encode(&self.serialize()[..]) - } + pub fn to_base64(self) -> String { base64::encode(&self.serialize()[..]) } } #[cfg(feature = "base64")] @@ -187,8 +182,11 @@ mod message_signing { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let bytes = self.serialize(); // This avoids the allocation of a String. - write!(f, "{}", base64::display::Base64Display::with_config( - &bytes[..], base64::STANDARD)) + write!( + f, + "{}", + base64::display::Base64Display::with_config(&bytes[..], base64::STANDARD) + ) } } @@ -220,28 +218,29 @@ mod tests { #[test] fn test_signed_msg_hash() { let hash = signed_msg_hash("test"); - assert_eq!(hash.to_hex(), "a6f87fe6d58a032c320ff8d1541656f0282c2c7bfcc69d61af4c8e8ed528e49c"); + assert_eq!( + hash.to_hex(), + "a6f87fe6d58a032c320ff8d1541656f0282c2c7bfcc69d61af4c8e8ed528e49c" + ); } #[test] #[cfg(all(feature = "secp-recovery", feature = "base64"))] fn test_message_signature() { use core::str::FromStr; + use secp256k1; - use crate::{Address, Network, AddressType}; + + use crate::{Address, AddressType, Network}; let secp = secp256k1::Secp256k1::new(); let message = "rust-bitcoin MessageSignature test"; let msg_hash = super::signed_msg_hash(message); let msg = secp256k1::Message::from(msg_hash); - let privkey = secp256k1::SecretKey::new(&mut secp256k1::rand::thread_rng()); let secp_sig = secp.sign_ecdsa_recoverable(&msg, &privkey); - let signature = super::MessageSignature { - signature: secp_sig, - compressed: true, - }; + let signature = super::MessageSignature { signature: secp_sig, compressed: true }; assert_eq!(signature.to_base64(), signature.to_string()); let signature2 = super::MessageSignature::from_str(&signature.to_string()).unwrap(); @@ -267,6 +266,7 @@ mod tests { #[cfg(all(feature = "secp-recovery", feature = "base64"))] fn test_incorrect_message_signature() { use secp256k1; + use crate::util::key::PublicKey; use crate::{Address, Network}; @@ -278,11 +278,12 @@ mod tests { // Signed with pk "UuOGDsfLPr4HIMKQX0ipjJeRaj1geCq3yPUF2COP5ME=" let signature_base64 = "IAM2qX24tYx/bdBTIgVLhD8QEAjrPlJpmjB4nZHdRYGIBa4DmVulAcwjPnWe6Q5iEwXH6F0pUCJP/ZeHPWS1h1o="; let pubkey_base64 = "A1FTfMEntPpAty3qkEo0q2Dc1FEycI10a3jmwEFy+Qr6"; - let signature = super::MessageSignature::from_base64(signature_base64).expect("message signature"); + let signature = + super::MessageSignature::from_base64(signature_base64).expect("message signature"); - let pubkey = PublicKey::from_slice( - &::base64::decode(&pubkey_base64).expect("base64 string") - ).expect("pubkey slice"); + let pubkey = + PublicKey::from_slice(&::base64::decode(&pubkey_base64).expect("base64 string")) + .expect("pubkey slice"); let p2pkh = Address::p2pkh(&pubkey, Network::Bitcoin); assert_eq!(signature.is_signed_by_address(&secp, &p2pkh, msg_hash), Ok(false));