From ec4635904bf0533ce4f33b5594c5a7aa1f35c558 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 1 Oct 2024 12:52:58 +1000 Subject: [PATCH 1/2] Inline bytes_to_asm_fmt into Script::Display impl This function does not need to exist anymore because it is exactly the same as what is produced by the `Display` impl. --- bitcoin/src/blockdata/script/borrowed.rs | 14 +-- bitcoin/src/blockdata/script/mod.rs | 146 +++++++++++------------ 2 files changed, 76 insertions(+), 84 deletions(-) diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs index fdecffc85..0c91715c1 100644 --- a/bitcoin/src/blockdata/script/borrowed.rs +++ b/bitcoin/src/blockdata/script/borrowed.rs @@ -9,14 +9,14 @@ use internals::ToU64 as _; use super::witness_version::WitnessVersion; use super::{ - bytes_to_asm_fmt, Builder, Instruction, InstructionIndices, Instructions, PushBytes, - RedeemScriptSizeError, ScriptBuf, ScriptHash, WScriptHash, WitnessScriptSizeError, + Builder, Instruction, InstructionIndices, Instructions, PushBytes, RedeemScriptSizeError, + ScriptBuf, ScriptHash, WScriptHash, WitnessScriptSizeError, }; use crate::consensus::Encodable; use crate::opcodes::all::*; use crate::opcodes::{self, Opcode}; use crate::policy::DUST_RELAY_TX_FEE; -use crate::prelude::{sink, Box, DisplayHex, String, ToOwned, Vec}; +use crate::prelude::{sink, Box, DisplayHex, String, ToOwned, ToString, Vec}; use crate::taproot::{LeafVersion, TapLeafHash}; use crate::FeeRate; @@ -483,16 +483,12 @@ crate::internal_macros::define_extension_trait! { /// Writes the human-readable assembly representation of the script to the formatter. #[deprecated(since = "TBD", note = "use the script's Display impl instead")] fn fmt_asm(&self, f: &mut dyn fmt::Write) -> fmt::Result { - bytes_to_asm_fmt(self.as_ref(), f) + write!(f, "{}", self) } /// Returns the human-readable assembly representation of the script. #[deprecated(since = "TBD", note = "use `to_string()` instead")] - fn to_asm_string(&self) -> String { - let mut buf = String::new(); - bytes_to_asm_fmt(self.as_ref(), &mut buf).expect("in-memory writers don't fail"); - buf - } + fn to_asm_string(&self) -> String { self.to_string() } /// Formats the script as lower-case hex. /// diff --git a/bitcoin/src/blockdata/script/mod.rs b/bitcoin/src/blockdata/script/mod.rs index b6c07dbc5..7be207b83 100644 --- a/bitcoin/src/blockdata/script/mod.rs +++ b/bitcoin/src/blockdata/script/mod.rs @@ -415,7 +415,7 @@ impl AsMut<[u8]> for ScriptBuf { impl fmt::Debug for Script { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("Script(")?; - bytes_to_asm_fmt(self.as_ref(), f)?; + fmt::Display::fmt(self, f)?; f.write_str(")") } } @@ -425,8 +425,76 @@ impl fmt::Debug for ScriptBuf { } impl fmt::Display for Script { - #[inline] - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { bytes_to_asm_fmt(self.as_ref(), f) } + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // This has to be a macro because it needs to break the loop + macro_rules! read_push_data_len { + ($iter:expr, $size:path, $formatter:expr) => { + match script::read_push_data_len($iter, $size) { + Ok(n) => n, + Err(_) => { + $formatter.write_str("")?; + break; + } + } + }; + } + + let mut iter = self.as_bytes().iter(); + // Was at least one opcode emitted? + let mut at_least_one = false; + // `iter` needs to be borrowed in `read_push_data_len`, so we have to use `while let` instead + // of `for`. + while let Some(byte) = iter.next() { + let opcode = Opcode::from(*byte); + + let data_len = if let opcodes::Class::PushBytes(n) = + opcode.classify(opcodes::ClassifyContext::Legacy) + { + n as usize + } else { + match opcode { + OP_PUSHDATA1 => { + // side effects: may write and break from the loop + read_push_data_len!(&mut iter, PushDataLenLen::One, f) + } + OP_PUSHDATA2 => { + // side effects: may write and break from the loop + read_push_data_len!(&mut iter, PushDataLenLen::Two, f) + } + OP_PUSHDATA4 => { + // side effects: may write and break from the loop + read_push_data_len!(&mut iter, PushDataLenLen::Four, f) + } + _ => 0, + } + }; + + if at_least_one { + f.write_str(" ")?; + } else { + at_least_one = true; + } + // Write the opcode + if opcode == OP_PUSHBYTES_0 { + f.write_str("OP_0")?; + } else { + write!(f, "{:?}", opcode)?; + } + // Write any pushdata + if data_len > 0 { + f.write_str(" ")?; + if data_len <= iter.len() { + for ch in iter.by_ref().take(data_len) { + write!(f, "{:02x}", ch)?; + } + } else { + f.write_str("")?; + break; + } + } + } + Ok(()) + } } impl fmt::Display for ScriptBuf { @@ -637,78 +705,6 @@ impl Decodable for ScriptBuf { } } -/// Writes the assembly decoding of the script bytes to the formatter. -pub(super) fn bytes_to_asm_fmt(script: &[u8], f: &mut dyn fmt::Write) -> fmt::Result { - // This has to be a macro because it needs to break the loop - macro_rules! read_push_data_len { - ($iter:expr, $size:path, $formatter:expr) => { - match script::read_push_data_len($iter, $size) { - Ok(n) => n, - Err(_) => { - $formatter.write_str("")?; - break; - } - } - }; - } - - let mut iter = script.iter(); - // Was at least one opcode emitted? - let mut at_least_one = false; - // `iter` needs to be borrowed in `read_push_data_len`, so we have to use `while let` instead - // of `for`. - while let Some(byte) = iter.next() { - let opcode = Opcode::from(*byte); - - let data_len = if let opcodes::Class::PushBytes(n) = - opcode.classify(opcodes::ClassifyContext::Legacy) - { - n as usize - } else { - match opcode { - OP_PUSHDATA1 => { - // side effects: may write and break from the loop - read_push_data_len!(&mut iter, PushDataLenLen::One, f) - } - OP_PUSHDATA2 => { - // side effects: may write and break from the loop - read_push_data_len!(&mut iter, PushDataLenLen::Two, f) - } - OP_PUSHDATA4 => { - // side effects: may write and break from the loop - read_push_data_len!(&mut iter, PushDataLenLen::Four, f) - } - _ => 0, - } - }; - - if at_least_one { - f.write_str(" ")?; - } else { - at_least_one = true; - } - // Write the opcode - if opcode == OP_PUSHBYTES_0 { - f.write_str("OP_0")?; - } else { - write!(f, "{:?}", opcode)?; - } - // Write any pushdata - if data_len > 0 { - f.write_str(" ")?; - if data_len <= iter.len() { - for ch in iter.by_ref().take(data_len) { - write!(f, "{:02x}", ch)?; - } - } else { - f.write_str("")?; - break; - } - } - } - Ok(()) -} - /// Ways that a script might fail. Not everything is split up as /// much as it could be; patches welcome if more detailed errors /// would help you. From d649c062385d28385d99c950ba30f67bff7670dd Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 1 Oct 2024 13:31:18 +1000 Subject: [PATCH 2/2] Move script types to primitives Move the `Script` and `ScriptBuf` types to `primitives`, nothing else. --- bitcoin/src/blockdata/script/borrowed.rs | 150 +-------- bitcoin/src/blockdata/script/mod.rs | 398 +--------------------- bitcoin/src/blockdata/script/owned.rs | 94 +----- bitcoin/src/blockdata/script/tests.rs | 1 + primitives/Cargo.toml | 2 +- primitives/src/lib.rs | 10 +- primitives/src/script/borrowed.rs | 149 +++++++++ primitives/src/script/mod.rs | 406 +++++++++++++++++++++++ primitives/src/script/owned.rs | 96 ++++++ 9 files changed, 682 insertions(+), 624 deletions(-) create mode 100644 primitives/src/script/borrowed.rs create mode 100644 primitives/src/script/mod.rs create mode 100644 primitives/src/script/owned.rs diff --git a/bitcoin/src/blockdata/script/borrowed.rs b/bitcoin/src/blockdata/script/borrowed.rs index 0c91715c1..cfc0f16be 100644 --- a/bitcoin/src/blockdata/script/borrowed.rs +++ b/bitcoin/src/blockdata/script/borrowed.rs @@ -1,139 +1,25 @@ // SPDX-License-Identifier: CC0-1.0 use core::fmt; -use core::ops::{ - Bound, Index, Range, RangeFrom, RangeFull, RangeInclusive, RangeTo, RangeToInclusive, -}; use internals::ToU64 as _; use super::witness_version::WitnessVersion; use super::{ Builder, Instruction, InstructionIndices, Instructions, PushBytes, RedeemScriptSizeError, - ScriptBuf, ScriptHash, WScriptHash, WitnessScriptSizeError, + ScriptHash, WScriptHash, WitnessScriptSizeError, }; use crate::consensus::Encodable; use crate::opcodes::all::*; use crate::opcodes::{self, Opcode}; use crate::policy::DUST_RELAY_TX_FEE; -use crate::prelude::{sink, Box, DisplayHex, String, ToOwned, ToString, Vec}; +use crate::prelude::{sink, DisplayHex, String, ToString}; use crate::taproot::{LeafVersion, TapLeafHash}; use crate::FeeRate; -/// Bitcoin script slice. -/// -/// *[See also the `bitcoin::script` module](super).* -/// -/// `Script` is a script slice, the most primitive script type. It's usually seen in its borrowed -/// form `&Script`. It is always encoded as a series of bytes representing the opcodes and data -/// pushes. -/// -/// ## Validity -/// -/// `Script` does not have any validity invariants - it's essentially just a marked slice of -/// bytes. This is similar to [`Path`](std::path::Path) vs [`OsStr`](std::ffi::OsStr) where they -/// are trivially cast-able to each-other and `Path` doesn't guarantee being a usable FS path but -/// having a newtype still has value because of added methods, readability and basic type checking. -/// -/// Although at least data pushes could be checked not to overflow the script, bad scripts are -/// allowed to be in a transaction (outputs just become unspendable) and there even are such -/// transactions in the chain. Thus we must allow such scripts to be placed in the transaction. -/// -/// ## Slicing safety -/// -/// Slicing is similar to how `str` works: some ranges may be incorrect and indexing by -/// `usize` is not supported. However, as opposed to `std`, we have no way of checking -/// correctness without causing linear complexity so there are **no panics on invalid -/// ranges!** If you supply an invalid range, you'll get a garbled script. -/// -/// The range is considered valid if it's at a boundary of instruction. Care must be taken -/// especially with push operations because you could get a reference to arbitrary -/// attacker-supplied bytes that look like a valid script. -/// -/// It is recommended to use `.instructions()` method to get an iterator over script -/// instructions and work with that instead. -/// -/// ## Memory safety -/// -/// The type is `#[repr(transparent)]` for internal purposes only! -/// No consumer crate may rely on the representation of the struct! -/// -/// ## References -/// -/// -/// ### Bitcoin Core References -/// -/// * [CScript definition](https://github.com/bitcoin/bitcoin/blob/d492dc1cdaabdc52b0766bf4cba4bd73178325d0/src/script/script.h#L410) -/// -#[derive(PartialOrd, Ord, PartialEq, Eq, Hash)] -#[repr(transparent)] -pub struct Script(pub(in crate::blockdata::script) [u8]); - -impl ToOwned for Script { - type Owned = ScriptBuf; - - fn to_owned(&self) -> Self::Owned { ScriptBuf(self.0.to_owned()) } -} - -impl Script { - /// Creates a new empty script. - #[inline] - pub fn new() -> &'static Script { Script::from_bytes(&[]) } - - /// Treat byte slice as `Script` - #[inline] - pub fn from_bytes(bytes: &[u8]) -> &Script { - // SAFETY: copied from `std` - // The pointer was just created from a reference which is still alive. - // Casting slice pointer to a transparent struct wrapping that slice is sound (same - // layout). - unsafe { &*(bytes as *const [u8] as *const Script) } - } - - /// Treat mutable byte slice as `Script` - #[inline] - pub fn from_bytes_mut(bytes: &mut [u8]) -> &mut Script { - // SAFETY: copied from `std` - // The pointer was just created from a reference which is still alive. - // Casting slice pointer to a transparent struct wrapping that slice is sound (same - // layout). - // Function signature prevents callers from accessing `bytes` while the returned reference - // is alive. - unsafe { &mut *(bytes as *mut [u8] as *mut Script) } - } - - /// Returns the script data as a byte slice. - #[inline] - pub fn as_bytes(&self) -> &[u8] { &self.0 } - - /// Returns the script data as a mutable byte slice. - #[inline] - pub fn as_mut_bytes(&mut self) -> &mut [u8] { &mut self.0 } - - /// Returns the length in bytes of the script. - #[inline] - pub fn len(&self) -> usize { self.0.len() } - - /// Returns whether the script is the empty script. - #[inline] - pub fn is_empty(&self) -> bool { self.0.is_empty() } - - /// Returns a copy of the script data. - #[inline] - pub fn to_bytes(&self) -> Vec { self.0.to_owned() } - - /// Converts a [`Box