From 1b7dc51ccbf116affcada27c2b75da0a2fc8ed8e Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 27 Jul 2023 07:24:02 +1000 Subject: [PATCH 1/2] Remove deprecated code We only keep deprecated code around for one release so we can now remove code deprecated in v0.30.0 Done in preparation as we gear up for v0.31.0 release. --- bitcoin/src/address.rs | 7 -- bitcoin/src/base58.rs | 26 ------- bitcoin/src/blockdata/script/mod.rs | 18 ----- bitcoin/src/blockdata/transaction.rs | 100 +-------------------------- bitcoin/src/blockdata/witness.rs | 4 -- bitcoin/src/lib.rs | 1 - bitcoin/src/taproot.rs | 8 --- bitcoin/src/util/mod.rs | 58 ---------------- 8 files changed, 2 insertions(+), 220 deletions(-) delete mode 100644 bitcoin/src/util/mod.rs diff --git a/bitcoin/src/address.rs b/bitcoin/src/address.rs index 28ef44a4..41b881d4 100644 --- a/bitcoin/src/address.rs +++ b/bitcoin/src/address.rs @@ -718,13 +718,6 @@ impl Address { /// pub fn is_spend_standard(&self) -> bool { self.address_type().is_some() } - /// Checks whether or not the address is following Bitcoin standardness rules. - /// - /// SegWit addresses with unassigned witness versions or non-standard program sizes are - /// considered non-standard. - #[deprecated(since = "0.30.0", note = "Use Address::is_spend_standard instead")] - pub fn is_standard(&self) -> bool { self.address_type().is_some() } - /// Constructs an [`Address`] from an output script (`scriptPubkey`). pub fn from_script(script: &Script, network: Network) -> Result { Ok(Address::new(network, Payload::from_script(script)?)) diff --git a/bitcoin/src/base58.rs b/bitcoin/src/base58.rs index 98fea7cf..3ba51660 100644 --- a/bitcoin/src/base58.rs +++ b/bitcoin/src/base58.rs @@ -35,10 +35,6 @@ static BASE58_DIGITS: [Option; 128] = [ Some(55), Some(56), Some(57), None, None, None, None, None, // 120-127 ]; -/// Decodes a base58-encoded string into a byte vector. -#[deprecated(since = "0.30.0", note = "Use base58::decode() instead")] -pub fn from(data: &str) -> Result, Error> { decode(data) } - /// Decodes a base58-encoded string into a byte vector. pub fn decode(data: &str) -> Result, Error> { // 11/15 is just over log_256(58) @@ -70,10 +66,6 @@ pub fn decode(data: &str) -> Result, Error> { Ok(ret) } -/// Decodes a base58check-encoded string into a byte vector verifying the checksum. -#[deprecated(since = "0.30.0", note = "Use base58::decode_check() instead")] -pub fn from_check(data: &str) -> Result, Error> { decode_check(data) } - /// Decodes a base58check-encoded string into a byte vector verifying the checksum. pub fn decode_check(data: &str) -> Result, Error> { let mut ret: Vec = decode(data)?; @@ -97,19 +89,9 @@ pub fn decode_check(data: &str) -> Result, Error> { Ok(ret) } -/// Encodes `data` as a base58 string. -#[deprecated(since = "0.30.0", note = "Use base58::encode() instead")] -pub fn encode_slice(data: &[u8]) -> String { encode(data) } - /// Encodes `data` as a base58 string (see also `base58::encode_check()`). pub fn encode(data: &[u8]) -> String { encode_iter(data.iter().cloned()) } -/// Encodes `data` as a base58 string including the checksum. -/// -/// The checksum is the first four bytes of the sha256d of the data, concatenated onto the end. -#[deprecated(since = "0.30.0", note = "Use base58::encode_check() instead")] -pub fn check_encode_slice(data: &[u8]) -> String { encode_check(data) } - /// Encodes `data` as a base58 string including the checksum. /// /// The checksum is the first four bytes of the sha256d of the data, concatenated onto the end. @@ -118,14 +100,6 @@ pub fn encode_check(data: &[u8]) -> String { encode_iter(data.iter().cloned().chain(checksum[0..4].iter().cloned())) } -/// Encodes `data` as base58, including the checksum, into a formatter. -/// -/// The checksum is the first four bytes of the sha256d of the data, concatenated onto the end. -#[deprecated(since = "0.30.0", note = "Use base58::encode_check_to_fmt() instead")] -pub fn check_encode_slice_to_fmt(fmt: &mut fmt::Formatter, data: &[u8]) -> fmt::Result { - encode_check_to_fmt(fmt, data) -} - /// Encodes a slice as base58, including the checksum, into a formatter. /// /// The checksum is the first four bytes of the sha256d of the data, concatenated onto the end. diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index ecd68547..4db52561 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -182,24 +182,6 @@ pub fn read_scriptbool(v: &[u8]) -> bool { } } -/// Decodes a script-encoded unsigned integer. -/// -/// ## Errors -/// -/// This function returns an error in these cases: -/// -/// * `data` is shorter than `size` => `EarlyEndOfScript` -/// * `size` is greater than `u16::MAX / 8` (8191) => `NumericOverflow` -/// * The number being read overflows `usize` => `NumericOverflow` -/// -/// Note that this does **not** return an error for `size` between `core::size_of::()` -/// and `u16::MAX / 8` if there's no overflow. -#[inline] -#[deprecated(since = "0.30.0", note = "bitcoin integers are signed 32 bits, use read_scriptint")] -pub fn read_uint(data: &[u8], size: usize) -> Result { - read_uint_iter(&mut data.iter(), size).map_err(Into::into) -} - // We internally use implementation based on iterator so that it automatically advances as needed // Errors are same as above, just different type. fn read_uint_iter(data: &mut core::slice::Iter<'_, u8>, size: usize) -> Result { diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 6a1a2108..18a0cb4f 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -21,12 +21,11 @@ use internals::write_err; use super::Weight; use crate::blockdata::locktime::absolute::{self, Height, Time}; use crate::blockdata::locktime::relative; -use crate::blockdata::script::{Script, ScriptBuf}; +use crate::blockdata::script::ScriptBuf; use crate::blockdata::witness::Witness; #[cfg(feature = "bitcoinconsensus")] pub use crate::consensus::validation::TxVerifyError; use crate::consensus::{encode, Decodable, Encodable}; -use crate::crypto::sighash::LegacySighash; use crate::hash_types::{Txid, Wtxid}; use crate::internal_macros::impl_consensus_encoding; use crate::parse::impl_parse_str_from_int_infallible; @@ -521,7 +520,7 @@ impl TxOut { } } -/// Result of [`Transaction::encode_signing_data_to`]. +/// Result of [`crate::sighash::SighashCache::legacy_encode_signing_data_to`]. /// /// This type forces the caller to handle SIGHASH_SINGLE bug case. /// @@ -725,101 +724,6 @@ impl Transaction { Wtxid::from_engine(enc) } - /// Encodes the signing data from which a signature hash for a given input index with a given - /// sighash flag can be computed. - /// - /// To actually produce a scriptSig, this hash needs to be run through an ECDSA signer, the - /// [`EcdsaSighashType`] appended to the resulting sig, and a script written around this, but - /// this is the general (and hard) part. - /// - /// The `sighash_type` supports an arbitrary `u32` value, instead of just [`EcdsaSighashType`], - /// because internally 4 bytes are being hashed, even though only the lowest byte is appended to - /// signature in a transaction. - /// - /// # Warning - /// - /// - Does NOT attempt to support OP_CODESEPARATOR. In general this would require evaluating - /// `script_pubkey` to determine which separators get evaluated and which don't, which we don't - /// have the information to determine. - /// - Does NOT handle the sighash single bug (see "Returns" section) - /// - /// # Returns - /// - /// This function can't handle the SIGHASH_SINGLE bug internally, so it returns [`EncodeSigningDataResult`] - /// that must be handled by the caller (see [`EncodeSigningDataResult::is_sighash_single_bug`]). - /// - /// # Panics - /// - /// If `input_index` is out of bounds (greater than or equal to `self.input.len()`). - #[deprecated( - since = "0.30.0", - note = "Use SighashCache::legacy_encode_signing_data_to instead" - )] - pub fn encode_signing_data_to>( - &self, - writer: Write, - input_index: usize, - script_pubkey: &Script, - sighash_type: U, - ) -> EncodeSigningDataResult { - use EncodeSigningDataResult::*; - - use crate::sighash::{self, SighashCache}; - - assert!(input_index < self.input.len()); // Panic on OOB - - let cache = SighashCache::new(self); - match cache.legacy_encode_signing_data_to(writer, input_index, script_pubkey, sighash_type) - { - SighashSingleBug => SighashSingleBug, - WriteResult(res) => match res { - Ok(()) => WriteResult(Ok(())), - Err(e) => match e { - sighash::Error::Io(e) => WriteResult(Err(e.into())), - _ => unreachable!("we check input_index above"), - }, - }, - } - } - - /// Computes a signature hash for a given input index with a given sighash flag. - /// - /// To actually produce a scriptSig, this hash needs to be run through an ECDSA signer, the - /// [`EcdsaSighashType`] appended to the resulting sig, and a script written around this, but - /// this is the general (and hard) part. - /// - /// The `sighash_type` supports an arbitrary `u32` value, instead of just [`EcdsaSighashType`], - /// because internally 4 bytes are being hashed, even though only the lowest byte is appended to - /// signature in a transaction. - /// - /// This function correctly handles the sighash single bug by returning the 'one array'. The - /// sighash single bug becomes exploitable when one tries to sign a transaction with - /// `SIGHASH_SINGLE` and there is not a corresponding output with the same index as the input. - /// - /// # Warning - /// - /// Does NOT attempt to support OP_CODESEPARATOR. In general this would require evaluating - /// `script_pubkey` to determine which separators get evaluated and which don't, which we don't - /// have the information to determine. - /// - /// # Panics - /// - /// If `input_index` is out of bounds (greater than or equal to `self.input.len()`). - #[deprecated(since = "0.30.0", note = "Use SighashCache::legacy_signature_hash instead")] - pub fn signature_hash( - &self, - input_index: usize, - script_pubkey: &Script, - sighash_u32: u32, - ) -> LegacySighash { - assert!(input_index < self.input.len()); // Panic on OOB, enables expect below. - - let cache = crate::sighash::SighashCache::new(self); - cache - .legacy_signature_hash(input_index, script_pubkey, sighash_u32) - .expect("cache method doesn't error") - } - /// Returns the "weight" of this transaction, as defined by BIP141. /// /// For transactions with an empty witness, this is simply the consensus-serialized size times diff --git a/bitcoin/src/blockdata/witness.rs b/bitcoin/src/blockdata/witness.rs index d0b7408c..272637a8 100644 --- a/bitcoin/src/blockdata/witness.rs +++ b/bitcoin/src/blockdata/witness.rs @@ -227,10 +227,6 @@ impl Witness { /// Creates a new empty [`Witness`]. pub fn new() -> Self { Witness::default() } - /// Creates [`Witness`] object from an array of byte-arrays - #[deprecated(since = "0.30.0", note = "use `Witness::from_slice()` instead")] - pub fn from_vec(vec: Vec>) -> Self { Witness::from_slice(&vec) } - /// Creates a [`Witness`] object from a slice of bytes slices where each slice is a witness item. pub fn from_slice>(slice: &[T]) -> Self { let witness_elements = slice.len(); diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 5b3ef180..b3c6ec39 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -106,7 +106,6 @@ pub mod psbt; pub mod sign_message; pub mod string; pub mod taproot; -pub mod util; // May depend on crate features and we don't want to bother with it #[allow(unused)] diff --git a/bitcoin/src/taproot.rs b/bitcoin/src/taproot.rs index ae1b3f45..f3d5953b 100644 --- a/bitcoin/src/taproot.rs +++ b/bitcoin/src/taproot.rs @@ -1060,10 +1060,6 @@ impl TaprootMerkleBranch { /// Checks if this merkle proof is empty. pub fn is_empty(&self) -> bool { self.0.is_empty() } - /// Decodes bytes from control block. - #[deprecated(since = "0.30.0", note = "Use decode instead")] - pub fn from_slice(sl: &[u8]) -> Result { Self::decode(sl) } - /// Decodes bytes from control block. /// /// This reads the branch as encoded in the control block: the concatenated 32B byte chunks - @@ -1199,10 +1195,6 @@ pub struct ControlBlock { } impl ControlBlock { - /// Constructs a `ControlBlock` from slice. - #[deprecated(since = "0.30.0", note = "Use decode instead")] - pub fn from_slice(sl: &[u8]) -> Result { Self::decode(sl) } - /// Decodes bytes representing a `ControlBlock`. /// /// This is an extra witness element that provides the proof that taproot script pubkey is diff --git a/bitcoin/src/util/mod.rs b/bitcoin/src/util/mod.rs deleted file mode 100644 index 537e35ac..00000000 --- a/bitcoin/src/util/mod.rs +++ /dev/null @@ -1,58 +0,0 @@ -// SPDX-License-Identifier: CC0-1.0 - -//! Utility functions. -//! -//! Functions needed by all parts of the Bitcoin library. -//! - -/// 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 50ada8298f14d4ee35e3f14284119c2321205d0a Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Thu, 27 Jul 2023 09:21:34 +1000 Subject: [PATCH 2/2] Move EncodeSigningDataResult to sighash module This type was defined in the `transaction` module because it was originally used in a function that had been deprecated in favour of moving the logic to the `sighash` module. We just removed the deprecated code so we can now move this type to the `sighash` module where it is used. --- bitcoin/src/blockdata/transaction.rs | 69 --------------------------- bitcoin/src/crypto/sighash.rs | 70 +++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 70 deletions(-) diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 18a0cb4f..a02aa661 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -520,75 +520,6 @@ impl TxOut { } } -/// Result of [`crate::sighash::SighashCache::legacy_encode_signing_data_to`]. -/// -/// This type forces the caller to handle SIGHASH_SINGLE bug case. -/// -/// This corner case can't be expressed using standard `Result`, -/// in a way that is both convenient and not-prone to accidental -/// mistakes (like calling `.expect("writer never fails")`). -#[must_use] -pub enum EncodeSigningDataResult { - /// Input data is an instance of `SIGHASH_SINGLE` bug - SighashSingleBug, - /// Operation performed normally. - WriteResult(Result<(), E>), -} - -impl EncodeSigningDataResult { - /// Checks for SIGHASH_SINGLE bug returning error if the writer failed. - /// - /// This method is provided for easy and correct handling of the result because - /// SIGHASH_SINGLE bug is a special case that must not be ignored nor cause panicking. - /// Since the data is usually written directly into a hasher which never fails, - /// the recommended pattern to handle this is: - /// - /// ```rust - /// # use bitcoin::consensus::deserialize; - /// # use bitcoin::hashes::{Hash, hex::FromHex}; - /// # use bitcoin::sighash::{LegacySighash, SighashCache}; - /// # use bitcoin::Transaction; - /// # let mut writer = LegacySighash::engine(); - /// # let input_index = 0; - /// # let script_pubkey = bitcoin::ScriptBuf::new(); - /// # let sighash_u32 = 0u32; - /// # const SOME_TX: &'static str = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000"; - /// # let raw_tx = Vec::from_hex(SOME_TX).unwrap(); - /// # let tx: Transaction = deserialize(&raw_tx).unwrap(); - /// let cache = SighashCache::new(&tx); - /// if cache.legacy_encode_signing_data_to(&mut writer, input_index, &script_pubkey, sighash_u32) - /// .is_sighash_single_bug() - /// .expect("writer can't fail") { - /// // use a hash value of "1", instead of computing the actual hash due to SIGHASH_SINGLE bug - /// } - /// ``` - #[allow(clippy::wrong_self_convention)] // E is not Copy so we consume self. - pub fn is_sighash_single_bug(self) -> Result { - match self { - EncodeSigningDataResult::SighashSingleBug => Ok(true), - EncodeSigningDataResult::WriteResult(Ok(())) => Ok(false), - EncodeSigningDataResult::WriteResult(Err(e)) => Err(e), - } - } - - /// Maps a `Result` to `Result` by applying a function to a - /// contained [`Err`] value, leaving an [`Ok`] value untouched. - /// - /// Like [`Result::map_err`]. - pub fn map_err(self, f: F) -> EncodeSigningDataResult - where - F: FnOnce(E) -> E2, - { - match self { - EncodeSigningDataResult::SighashSingleBug => EncodeSigningDataResult::SighashSingleBug, - EncodeSigningDataResult::WriteResult(Err(e)) => - EncodeSigningDataResult::WriteResult(Err(f(e))), - EncodeSigningDataResult::WriteResult(Ok(o)) => - EncodeSigningDataResult::WriteResult(Ok(o)), - } - } -} - /// Bitcoin transaction. /// /// An authenticated movement of coins. diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index fe4f757d..657257a2 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -16,7 +16,6 @@ use core::{fmt, str}; use hashes::{hash_newtype, sha256, sha256d, sha256t_hash_newtype, Hash}; -use crate::blockdata::transaction::EncodeSigningDataResult; use crate::blockdata::witness::Witness; use crate::consensus::{encode, Encodable}; use crate::error::impl_std_error; @@ -1106,6 +1105,75 @@ fn is_invalid_use_of_sighash_single(sighash: u32, input_index: usize, output_len ty == EcdsaSighashType::Single && input_index >= output_len } +/// Result of [`SighashCache::legacy_encode_signing_data_to`]. +/// +/// This type forces the caller to handle SIGHASH_SINGLE bug case. +/// +/// This corner case can't be expressed using standard `Result`, +/// in a way that is both convenient and not-prone to accidental +/// mistakes (like calling `.expect("writer never fails")`). +#[must_use] +pub enum EncodeSigningDataResult { + /// Input data is an instance of `SIGHASH_SINGLE` bug + SighashSingleBug, + /// Operation performed normally. + WriteResult(Result<(), E>), +} + +impl EncodeSigningDataResult { + /// Checks for SIGHASH_SINGLE bug returning error if the writer failed. + /// + /// This method is provided for easy and correct handling of the result because + /// SIGHASH_SINGLE bug is a special case that must not be ignored nor cause panicking. + /// Since the data is usually written directly into a hasher which never fails, + /// the recommended pattern to handle this is: + /// + /// ```rust + /// # use bitcoin::consensus::deserialize; + /// # use bitcoin::hashes::{Hash, hex::FromHex}; + /// # use bitcoin::sighash::{LegacySighash, SighashCache}; + /// # use bitcoin::Transaction; + /// # let mut writer = LegacySighash::engine(); + /// # let input_index = 0; + /// # let script_pubkey = bitcoin::ScriptBuf::new(); + /// # let sighash_u32 = 0u32; + /// # const SOME_TX: &'static str = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000"; + /// # let raw_tx = Vec::from_hex(SOME_TX).unwrap(); + /// # let tx: Transaction = deserialize(&raw_tx).unwrap(); + /// let cache = SighashCache::new(&tx); + /// if cache.legacy_encode_signing_data_to(&mut writer, input_index, &script_pubkey, sighash_u32) + /// .is_sighash_single_bug() + /// .expect("writer can't fail") { + /// // use a hash value of "1", instead of computing the actual hash due to SIGHASH_SINGLE bug + /// } + /// ``` + #[allow(clippy::wrong_self_convention)] // E is not Copy so we consume self. + pub fn is_sighash_single_bug(self) -> Result { + match self { + EncodeSigningDataResult::SighashSingleBug => Ok(true), + EncodeSigningDataResult::WriteResult(Ok(())) => Ok(false), + EncodeSigningDataResult::WriteResult(Err(e)) => Err(e), + } + } + + /// Maps a `Result` to `Result` by applying a function to a + /// contained [`Err`] value, leaving an [`Ok`] value untouched. + /// + /// Like [`Result::map_err`]. + pub fn map_err(self, f: F) -> EncodeSigningDataResult + where + F: FnOnce(E) -> E2, + { + match self { + EncodeSigningDataResult::SighashSingleBug => EncodeSigningDataResult::SighashSingleBug, + EncodeSigningDataResult::WriteResult(Err(e)) => + EncodeSigningDataResult::WriteResult(Err(f(e))), + EncodeSigningDataResult::WriteResult(Ok(o)) => + EncodeSigningDataResult::WriteResult(Ok(o)), + } + } +} + #[cfg(test)] mod tests { use std::str::FromStr;