From 36d45bf360b4fff069203a86e3429831babe5e5d Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Wed, 19 Feb 2025 10:45:42 -0800 Subject: [PATCH 001/278] chacha20_poly1305: remove mod operator * Swaps out the mod operator for a switch statement for a 5% performance boost. --- chacha20_poly1305/src/chacha20.rs | 40 +++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs index bf0013192f..cde412d0a0 100644 --- a/chacha20_poly1305/src/chacha20.rs +++ b/chacha20_poly1305/src/chacha20.rs @@ -31,6 +31,14 @@ impl Nonce { pub const fn new(nonce: [u8; 12]) -> Self { Nonce(nonce) } } +// Const validation trait for compile time check with max of 3. +trait UpTo3 {} + +impl UpTo3<0> for () {} +impl UpTo3<1> for () {} +impl UpTo3<2> for () {} +impl UpTo3<3> for () {} + /// A SIMD-friendly structure which holds 25% of the cipher state. /// /// The cipher's quarter round function is the bulk of its work @@ -81,21 +89,29 @@ impl U32x4 { } #[inline(always)] - fn rotate_elements_left(self) -> Self { - let mut result = [0u32; 4]; - (0..4).for_each(|i| { - result[i] = self.0[(i + N as usize) % 4]; - }); - U32x4(result) + fn rotate_elements_left(self) -> Self + where + (): UpTo3, + { + match N { + 1 => U32x4([self.0[1], self.0[2], self.0[3], self.0[0]]), + 2 => U32x4([self.0[2], self.0[3], self.0[0], self.0[1]]), + 3 => U32x4([self.0[3], self.0[0], self.0[1], self.0[2]]), + _ => self, // Rotate by 0 is a no-op. + } } #[inline(always)] - fn rotate_elements_right(self) -> Self { - let mut result = [0u32; 4]; - (0..4).for_each(|i| { - result[i] = self.0[(i + 4 - N as usize) % 4]; - }); - U32x4(result) + fn rotate_elements_right(self) -> Self + where + (): UpTo3, + { + match N { + 1 => U32x4([self.0[3], self.0[0], self.0[1], self.0[2]]), + 2 => U32x4([self.0[2], self.0[3], self.0[0], self.0[1]]), + 3 => U32x4([self.0[1], self.0[2], self.0[3], self.0[0]]), + _ => self, // Rotate by 0 is a no-op. + } } #[inline(always)] From dadd1d72248015450fe6f898cd999ac54ff34aec Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Wed, 19 Feb 2025 10:48:17 -0800 Subject: [PATCH 002/278] chacha20_poly1305: remove alignment * Benchmarks showed that on recent versions of the rust compiler, alignment settings could hurt and never helped. --- chacha20_poly1305/src/chacha20.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs index cde412d0a0..48eb310bf5 100644 --- a/chacha20_poly1305/src/chacha20.rs +++ b/chacha20_poly1305/src/chacha20.rs @@ -60,12 +60,10 @@ impl UpTo3<3> for () {} /// * For-each loops are easy for the compiler to recognize as vectorizable. /// * The type is a based on an array instead of tuple since the heterogeneous /// nature of tuples can confuse the compiler into thinking it is not vectorizable. -/// * Memory alignment lines up with SIMD size. /// /// In the future, a "blacklist" for the alignment option might be useful to /// disable it on architectures which definitely do not support SIMD in order to avoid /// needless memory inefficientcies. -#[repr(align(16))] #[derive(Clone, Copy, PartialEq)] struct U32x4([u32; 4]); From 33dc1b95fa282ec1fe9706525f1a38ee611b7915 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Wed, 19 Feb 2025 10:55:25 -0800 Subject: [PATCH 003/278] chacha20_poly1305: swap tuple for array * While perhaps a small performance gain, < 1%, this conforms to the style used in the rest of the module. --- chacha20_poly1305/src/chacha20.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs index 48eb310bf5..c611ec64e9 100644 --- a/chacha20_poly1305/src/chacha20.rs +++ b/chacha20_poly1305/src/chacha20.rs @@ -177,7 +177,7 @@ impl State { /// Four quarter rounds performed on the entire state of the cipher in a vectorized SIMD friendly fashion. #[inline(always)] - fn quarter_round(a: U32x4, b: U32x4, c: U32x4, d: U32x4) -> (U32x4, U32x4, U32x4, U32x4) { + fn quarter_round(a: U32x4, b: U32x4, c: U32x4, d: U32x4) -> [U32x4; 4] { let a = a.wrapping_add(b); let d = d.bitxor(a).rotate_left(16); @@ -190,7 +190,7 @@ impl State { let c = c.wrapping_add(d); let b = b.bitxor(c).rotate_left(7); - (a, b, c, d) + [a, b, c, d] } /// Perform a round on "columns" and then "diagonals" of the state. @@ -207,13 +207,13 @@ impl State { let [mut a, mut b, mut c, mut d] = state; // Column round. - (a, b, c, d) = Self::quarter_round(a, b, c, d); + [a, b, c, d] = Self::quarter_round(a, b, c, d); // Diagonal round (with rotations). b = b.rotate_elements_left::<1>(); c = c.rotate_elements_left::<2>(); d = d.rotate_elements_left::<3>(); - (a, b, c, d) = Self::quarter_round(a, b, c, d); + [a, b, c, d] = Self::quarter_round(a, b, c, d); // Rotate the words back into their normal positions. b = b.rotate_elements_right::<1>(); c = c.rotate_elements_right::<2>(); From 415945cd2b3f4bd40f6c15e359ea241ed7b4f2d4 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Wed, 19 Feb 2025 11:38:19 -0800 Subject: [PATCH 004/278] chacha20_poly1305: avoid duplicate block work * The keystream function is creating two state block on every call, but just to handle a corner case. Break up the function into separate methods so that the corner case is handled by itself, avoiding unnecessary work most of the time. * Handle offset state internally. While not strictly necessary due to the cipher's use in BIP324, it makes the library much easier to work with (the bug above would probably have been avoided) if the cipher handles the offset state. --- chacha20_poly1305/src/chacha20.rs | 113 ++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 36 deletions(-) diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs index c611ec64e9..34f0413040 100644 --- a/chacha20_poly1305/src/chacha20.rs +++ b/chacha20_poly1305/src/chacha20.rs @@ -274,37 +274,64 @@ impl ChaCha20 { ChaCha20 { key, nonce, block_count: block, seek_offset_bytes: 0 } } - /// Apply the keystream to a buffer. + /// Get the keystream for a specific block. + fn keystream_at_block(&self, block: u32) -> [u8; 64] { + let mut state = State::new(self.key, self.nonce, block); + state.chacha_block(); + state.keystream() + } + + /// Apply the keystream to a buffer updating the cipher block state as necessary. pub fn apply_keystream(&mut self, buffer: &mut [u8]) { - let num_full_blocks = buffer.len() / CHACHA_BLOCKSIZE; - for block in 0..num_full_blocks { - let keystream = - keystream_at_slice(self.key, self.nonce, self.block_count, self.seek_offset_bytes); - for (buffer_byte, keystream_byte) in buffer - [block * CHACHA_BLOCKSIZE..(block + 1) * CHACHA_BLOCKSIZE] - .iter_mut() - .zip(keystream.iter()) + // If we have an initial offset, handle the first partial block to get back to alignment. + let remaining_buffer = if self.seek_offset_bytes != 0 { + let bytes_until_aligned = 64 - self.seek_offset_bytes; + let bytes_to_process = buffer.len().min(bytes_until_aligned); + + let keystream = self.keystream_at_block(self.block_count); + for (buffer_byte, keystream_byte) in + buffer[..bytes_to_process].iter_mut().zip(&keystream[self.seek_offset_bytes..]) { - *buffer_byte ^= *keystream_byte + *buffer_byte ^= *keystream_byte; } + + if bytes_to_process < bytes_until_aligned { + self.seek_offset_bytes += bytes_to_process; + return; + } + self.block_count += 1; - } - if buffer.len() % 64 > 0 { - let keystream = - keystream_at_slice(self.key, self.nonce, self.block_count, self.seek_offset_bytes); - for (buffer_byte, keystream_byte) in - buffer[num_full_blocks * CHACHA_BLOCKSIZE..].iter_mut().zip(keystream.iter()) - { - *buffer_byte ^= *keystream_byte + self.seek_offset_bytes = 0; + &mut buffer[bytes_to_process..] + } else { + buffer + }; + + // Process full blocks. + let mut chunks = remaining_buffer.chunks_exact_mut(CHACHA_BLOCKSIZE); + for chunk in &mut chunks { + let keystream = self.keystream_at_block(self.block_count); + for (buffer_byte, keystream_byte) in chunk.iter_mut().zip(keystream.iter()) { + *buffer_byte ^= *keystream_byte; } self.block_count += 1; } + + // Handle any remaining bytes as partial block. + let remainder = chunks.into_remainder(); + if !remainder.is_empty() { + let keystream = self.keystream_at_block(self.block_count); + for (buffer_byte, keystream_byte) in remainder.iter_mut().zip(keystream.iter()) { + *buffer_byte ^= *keystream_byte; + } + self.seek_offset_bytes = remainder.len(); + } } /// Get the keystream block at a specified block. pub fn get_keystream(&mut self, block: u32) -> [u8; 64] { self.block(block); - keystream_at_slice(self.key, self.nonce, self.block_count, self.seek_offset_bytes) + self.keystream_at_block(self.block_count) } /// Update the index of the keystream to the given byte. @@ -320,23 +347,6 @@ impl ChaCha20 { } } -fn keystream_at_slice(key: Key, nonce: Nonce, count: u32, seek: usize) -> [u8; 64] { - let mut keystream: [u8; 128] = [0; 128]; - let (first_half, second_half) = keystream.split_at_mut(64); - - let mut state = State::new(key, nonce, count); - state.chacha_block(); - first_half.copy_from_slice(&state.keystream()); - - let mut state = State::new(key, nonce, count + 1); - state.chacha_block(); - second_half.copy_from_slice(&state.keystream()); - - let seeked_keystream: [u8; 64] = - keystream[seek..seek + 64].try_into().expect("slicing produces 64-byte slice"); - seeked_keystream -} - #[cfg(test)] #[cfg(feature = "alloc")] mod tests { @@ -460,4 +470,35 @@ mod tests { let binding = *b"Ladies and Gentlemen of the class of '99: If I could offer you only one tip for the future, sunscreen would be it."; assert_eq!(binding, to); } + + #[test] + fn multiple_partial_applies() { + let key = + Key(Vec::from_hex("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f") + .unwrap() + .try_into() + .unwrap()); + let nonce = Nonce(Vec::from_hex("000000000000004a00000000").unwrap().try_into().unwrap()); + + // Create two instances, one for a full single pass and one for chunked partial calls. + let mut chacha_full = ChaCha20::new(key, nonce, 0); + let mut chacha_chunked = ChaCha20::new(key, nonce, 0); + + // Test data that crosses block boundaries. + let mut full_buffer = [0u8; 100]; + let mut chunked_buffer = [0u8; 100]; + for (i, byte) in full_buffer.iter_mut().enumerate() { + *byte = i as u8; + } + chunked_buffer.copy_from_slice(&full_buffer); + + // Apply keystream to full buffer. + chacha_full.apply_keystream(&mut full_buffer); + // Apply keystream in multiple calls to chunked buffer. + chacha_chunked.apply_keystream(&mut chunked_buffer[..30]); // Partial block + chacha_chunked.apply_keystream(&mut chunked_buffer[30..82]); // Cross block boundary + chacha_chunked.apply_keystream(&mut chunked_buffer[82..]); // End with partial block + + assert_eq!(full_buffer, chunked_buffer); + } } From 30920c4d849cf25ae3847b51380b90aa331f903f Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Wed, 19 Feb 2025 12:12:28 -0800 Subject: [PATCH 005/278] chacha20_poly1305: drop mutable requirement * The get_keystream method exposes the keystream for a block for special case scenarios. Generally the cipher state should only be updated with teh apply_keystream method. --- chacha20_poly1305/src/chacha20.rs | 7 ++----- chacha20_poly1305/src/lib.rs | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs index 34f0413040..a93e0f1e65 100644 --- a/chacha20_poly1305/src/chacha20.rs +++ b/chacha20_poly1305/src/chacha20.rs @@ -328,11 +328,8 @@ impl ChaCha20 { } } - /// Get the keystream block at a specified block. - pub fn get_keystream(&mut self, block: u32) -> [u8; 64] { - self.block(block); - self.keystream_at_block(self.block_count) - } + /// Get the keystream for specified block. + pub fn get_keystream(&self, block: u32) -> [u8; 64] { self.keystream_at_block(block) } /// Update the index of the keystream to the given byte. pub fn seek(&mut self, seek: u32) { diff --git a/chacha20_poly1305/src/lib.rs b/chacha20_poly1305/src/lib.rs index 42b2df3afe..1627b511e8 100644 --- a/chacha20_poly1305/src/lib.rs +++ b/chacha20_poly1305/src/lib.rs @@ -118,7 +118,7 @@ impl ChaCha20Poly1305 { tag: [u8; 16], aad: Option<&[u8]>, ) -> Result<(), Error> { - let mut chacha = ChaCha20::new_from_block(self.key, self.nonce, 0); + let chacha = ChaCha20::new_from_block(self.key, self.nonce, 0); let keystream = chacha.get_keystream(0); let mut poly = Poly1305::new(keystream[..32].try_into().expect("slicing produces 32-byte slice")); From 1ca55ac77db698f3816d8b7ed4051ddb5a579a29 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Wed, 19 Feb 2025 13:04:20 -0800 Subject: [PATCH 006/278] chacha20_poly1305: inline simd functions * Inline all the block operations to give the compiler the best chance to optimize the SIMD instruction usage. This squeezed out another percent or two on the benchmarks when comparing target-cpu=native builds to standard. --- chacha20_poly1305/src/chacha20.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs index a93e0f1e65..b9080b40ec 100644 --- a/chacha20_poly1305/src/chacha20.rs +++ b/chacha20_poly1305/src/chacha20.rs @@ -223,6 +223,7 @@ impl State { } /// Transform the state by performing the ChaCha block function. + #[inline(always)] fn chacha_block(&mut self) { let mut working_state = self.matrix; @@ -237,6 +238,7 @@ impl State { } /// Expose the 512-bit state as a byte stream. + #[inline(always)] fn keystream(&self) -> [u8; 64] { let mut keystream = [0u8; 64]; for i in 0..4 { @@ -275,6 +277,7 @@ impl ChaCha20 { } /// Get the keystream for a specific block. + #[inline(always)] fn keystream_at_block(&self, block: u32) -> [u8; 64] { let mut state = State::new(self.key, self.nonce, block); state.chacha_block(); From 8ee088df740189cd13dd78ce2356453d0002b455 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 24 Mar 2025 13:17:49 +0100 Subject: [PATCH 007/278] Make `sha256t` obey sanity rules The comparison functions were accessing the fields directly, so this changes them to use methods instead. --- hashes/src/sha256t/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hashes/src/sha256t/mod.rs b/hashes/src/sha256t/mod.rs index d5acd5edfd..d8eb327938 100644 --- a/hashes/src/sha256t/mod.rs +++ b/hashes/src/sha256t/mod.rs @@ -128,7 +128,7 @@ impl Clone for Hash { fn clone(&self) -> Self { *self } } impl PartialEq for Hash { - fn eq(&self, other: &Hash) -> bool { self.0 == other.0 } + fn eq(&self, other: &Hash) -> bool { self.as_byte_array() == other.as_byte_array() } } impl Eq for Hash {} impl PartialOrd for Hash { @@ -137,10 +137,10 @@ impl PartialOrd for Hash { } } impl Ord for Hash { - fn cmp(&self, other: &Hash) -> cmp::Ordering { cmp::Ord::cmp(&self.0, &other.0) } + fn cmp(&self, other: &Hash) -> cmp::Ordering { cmp::Ord::cmp(&self.as_byte_array(), &other.as_byte_array()) } } impl core::hash::Hash for Hash { - fn hash(&self, h: &mut H) { self.0.hash(h) } + fn hash(&self, h: &mut H) { self.as_byte_array().hash(h) } } crate::internal_macros::hash_trait_impls!(256, false, T: Tag); From bc6da1fe07937051ada4a60c78f5f5128484a91a Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 24 Mar 2025 13:19:59 +0100 Subject: [PATCH 008/278] Swap around the fields in `sha256t::Hash` There's a restriction that for structs containing unsized types the unsized type has to be the last field. `sha256t::Hash` is not an unsized type but we are going to introduce a macro that will assume this order to work equally well with both sized and unsized types. Thus we swap it upfront here. --- hashes/src/sha256t/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hashes/src/sha256t/mod.rs b/hashes/src/sha256t/mod.rs index d8eb327938..59f73bf0f1 100644 --- a/hashes/src/sha256t/mod.rs +++ b/hashes/src/sha256t/mod.rs @@ -45,13 +45,13 @@ pub trait Tag { /// Output of the SHA256t hash function. #[repr(transparent)] -pub struct Hash([u8; 32], PhantomData); +pub struct Hash(PhantomData, [u8; 32]); impl Hash where T: Tag, { - const fn internal_new(arr: [u8; 32]) -> Self { Hash(arr, PhantomData) } + const fn internal_new(arr: [u8; 32]) -> Self { Hash(PhantomData, arr) } /// Constructs a new hash from the underlying byte array. pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self::internal_new(bytes) } @@ -117,10 +117,10 @@ where } /// Returns the underlying byte array. - pub const fn to_byte_array(self) -> [u8; 32] { self.0 } + pub const fn to_byte_array(self) -> [u8; 32] { self.1 } /// Returns a reference to the underlying byte array. - pub const fn as_byte_array(&self) -> &[u8; 32] { &self.0 } + pub const fn as_byte_array(&self) -> &[u8; 32] { &self.1 } } impl Copy for Hash {} From 7a115e3cf13f5cde24b957efd888b0c0c6840e2c Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 24 Mar 2025 14:21:04 +0100 Subject: [PATCH 009/278] Make `Address` obey sanity rules `Address` was directly accessing its internals in multiple places. This makes maintenance harder, so change it to use methods instead. --- bitcoin/src/address/error.rs | 2 +- bitcoin/src/address/mod.rs | 60 +++++++++++++++++++++--------------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/bitcoin/src/address/error.rs b/bitcoin/src/address/error.rs index c174710994..f1479425c1 100644 --- a/bitcoin/src/address/error.rs +++ b/bitcoin/src/address/error.rs @@ -158,7 +158,7 @@ pub struct NetworkValidationError { impl fmt::Display for NetworkValidationError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "address ")?; - fmt::Display::fmt(&self.address.0, f)?; + fmt::Display::fmt(&self.address.inner(), f)?; write!(f, " is not valid on {}", self.required) } } diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index 93bc290d87..0200b108be 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -404,7 +404,7 @@ struct DisplayUnchecked<'a, N: NetworkValidation>(&'a Address); #[cfg(feature = "serde")] impl fmt::Display for DisplayUnchecked<'_, N> { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0 .0, fmt) } + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0.inner(), fmt) } } #[cfg(feature = "serde")] @@ -433,7 +433,7 @@ impl<'de, U: NetworkValidationUnchecked> serde::Deserialize<'de> for Address { // We know that `U` is only ever `NetworkUnchecked` but the compiler does not. let address = v.parse::>().map_err(E::custom)?; - Ok(Address(address.0, PhantomData::)) + Ok(Address::from_inner(address.into_inner())) } } @@ -454,18 +454,30 @@ impl serde::Serialize for Address { /// Methods on [`Address`] that can be called on both `Address` and /// `Address`. impl Address { + fn from_inner(inner: AddressInner) -> Self { + Address(inner, PhantomData) + } + + fn into_inner(self) -> AddressInner { + self.0 + } + + fn inner(&self) -> &AddressInner { + &self.0 + } + /// Returns a reference to the address as if it was unchecked. pub fn as_unchecked(&self) -> &Address { unsafe { &*(self as *const Address as *const Address) } } /// Marks the network of this address as unchecked. - pub fn into_unchecked(self) -> Address { Address(self.0, PhantomData) } + pub fn into_unchecked(self) -> Address { Address::from_inner(self.into_inner()) } /// Returns the [`NetworkKind`] of this address. pub fn network_kind(&self) -> NetworkKind { use AddressInner::*; - match self.0 { + match *self.inner() { P2pkh { hash: _, ref network } => *network, P2sh { hash: _, ref network } => *network, Segwit { program: _, ref hrp } => NetworkKind::from(*hrp), @@ -481,7 +493,7 @@ impl Address { #[inline] pub fn p2pkh(pk: impl Into, network: impl Into) -> Address { let hash = pk.into(); - Self(AddressInner::P2pkh { hash, network: network.into() }, PhantomData) + Self::from_inner(AddressInner::P2pkh { hash, network: network.into() }) } /// Constructs a new pay-to-script-hash (P2SH) [`Address`] from a script. @@ -504,7 +516,7 @@ impl Address { /// The `hash` pre-image (redeem script) must not exceed 520 bytes in length /// otherwise outputs created from the returned address will be un-spendable. pub fn p2sh_from_hash(hash: ScriptHash, network: impl Into) -> Address { - Self(AddressInner::P2sh { hash, network: network.into() }, PhantomData) + Self::from_inner(AddressInner::P2sh { hash, network: network.into() }) } /// Constructs a new pay-to-witness-public-key-hash (P2WPKH) [`Address`] from a public key. @@ -577,7 +589,7 @@ impl Address { /// then you likely do not need this constructor. pub fn from_witness_program(program: WitnessProgram, hrp: impl Into) -> Address { let inner = AddressInner::Segwit { program, hrp: hrp.into() }; - Address(inner, PhantomData) + Address::from_inner(inner) } /// Gets the address type of the [`Address`]. @@ -587,7 +599,7 @@ impl Address { /// None if unknown, non-standard or related to the future witness version. #[inline] pub fn address_type(&self) -> Option { - match self.0 { + match *self.inner() { AddressInner::P2pkh { .. } => Some(AddressType::P2pkh), AddressInner::P2sh { .. } => Some(AddressType::P2sh), AddressInner::Segwit { ref program, hrp: _ } => @@ -609,7 +621,7 @@ impl Address { pub fn to_address_data(&self) -> AddressData { use AddressData::*; - match self.0 { + match *self.inner() { AddressInner::P2pkh { hash, network: _ } => P2pkh { pubkey_hash: hash }, AddressInner::P2sh { hash, network: _ } => P2sh { script_hash: hash }, AddressInner::Segwit { program, hrp: _ } => Segwit { witness_program: program }, @@ -620,7 +632,7 @@ impl Address { pub fn pubkey_hash(&self) -> Option { use AddressInner::*; - match self.0 { + match *self.inner() { P2pkh { ref hash, network: _ } => Some(*hash), _ => None, } @@ -630,7 +642,7 @@ impl Address { pub fn script_hash(&self) -> Option { use AddressInner::*; - match self.0 { + match *self.inner() { P2sh { ref hash, network: _ } => Some(*hash), _ => None, } @@ -640,7 +652,7 @@ impl Address { pub fn witness_program(&self) -> Option { use AddressInner::*; - match self.0 { + match *self.inner() { Segwit { ref program, hrp: _ } => Some(*program), _ => None, } @@ -689,7 +701,7 @@ impl Address { /// Generates a script pubkey spending to this address. pub fn script_pubkey(&self) -> ScriptBuf { use AddressInner::*; - match self.0 { + match *self.inner() { P2pkh { hash, network: _ } => ScriptBuf::new_p2pkh(hash), P2sh { hash, network: _ } => ScriptBuf::new_p2sh(hash), Segwit { ref program, hrp: _ } => { @@ -756,7 +768,7 @@ impl Address { /// This function doesn't make any allocations. pub fn matches_script_pubkey(&self, script: &Script) -> bool { use AddressInner::*; - match self.0 { + match *self.inner() { P2pkh { ref hash, network: _ } if script.is_p2pkh() => &script.as_bytes()[3..23] == >::as_ref(hash), P2sh { ref hash, network: _ } if script.is_p2sh() => @@ -777,7 +789,7 @@ impl Address { /// - For SegWit addresses, the payload is the witness program. fn payload_as_bytes(&self) -> &[u8] { use AddressInner::*; - match self.0 { + match *self.inner() { P2sh { ref hash, network: _ } => hash.as_ref(), P2pkh { ref hash, network: _ } => hash.as_ref(), Segwit { ref program, hrp: _ } => program.program().as_bytes(), @@ -817,7 +829,7 @@ impl Address { /// ``` pub fn is_valid_for_network(&self, n: Network) -> bool { use AddressInner::*; - match self.0 { + match *self.inner() { P2pkh { hash: _, ref network } => *network == NetworkKind::from(n), P2sh { hash: _, ref network } => *network == NetworkKind::from(n), Segwit { program: _, ref hrp } => *hrp == KnownHrp::from_network(n), @@ -882,7 +894,7 @@ impl Address { /// For details about this mechanism, see section [*Parsing addresses*](Address#parsing-addresses) /// on [`Address`]. #[inline] - pub fn assume_checked(self) -> Address { Address(self.0, PhantomData) } + pub fn assume_checked(self) -> Address { Address::from_inner(self.into_inner()) } /// Parse a bech32 Address string pub fn from_bech32_str(s: &str) -> Result, Bech32Error> { @@ -894,7 +906,7 @@ impl Address { let hrp = KnownHrp::from_hrp(hrp)?; let inner = AddressInner::Segwit { program, hrp }; - Ok(Address(inner, PhantomData)) + Ok(Address::from_inner(inner)) } /// Parse a base58 Address string @@ -927,7 +939,7 @@ impl Address { invalid => return Err(InvalidLegacyPrefixError { invalid }.into()), }; - Ok(Address(inner, PhantomData)) + Ok(Address::from_inner(inner)) } } @@ -938,16 +950,16 @@ impl From
for ScriptBuf { // Alternate formatting `{:#}` is used to return uppercase version of bech32 addresses which should // be used in QR codes, see [`Address::to_qr_uri`]. impl fmt::Display for Address { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, fmt) } + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.inner(), fmt) } } impl fmt::Debug for Address { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { if V::IS_CHECKED { - fmt::Display::fmt(&self.0, f) + fmt::Display::fmt(&self.inner(), f) } else { write!(f, "Address(")?; - fmt::Display::fmt(&self.0, f)?; + fmt::Display::fmt(&self.inner(), f)?; write!(f, ")") } } @@ -975,10 +987,10 @@ impl FromStr for Address { if ["bc1", "bcrt1", "tb1"].iter().any(|&prefix| s.to_lowercase().starts_with(prefix)) { let address = Address::from_bech32_str(s)?; // We know that `U` is only ever `NetworkUnchecked` but the compiler does not. - Ok(Address(address.0, PhantomData::)) + Ok(Address::from_inner(address.into_inner())) } else if ["1", "2", "3", "m", "n"].iter().any(|&prefix| s.starts_with(prefix)) { let address = Address::from_base58_str(s)?; - Ok(Address(address.0, PhantomData::)) + Ok(Address::from_inner(address.into_inner())) } else { let hrp = match s.rfind('1') { Some(pos) => &s[..pos], From d42364bd9d39ce4d509cf2d13336bc3bc619e3f1 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 24 Mar 2025 14:30:56 +0100 Subject: [PATCH 010/278] Swap around the fields in `Address` There's a restriction that for structs containing unsized types the unsized type has to be the last field. `Address` is not an unsize type but we are going to introduce a macro that will assume this order to work equally well with both sized and unsized types. Thus we swap it upfront here. --- bitcoin/src/address/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index 0200b108be..48cf871fd7 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -395,7 +395,7 @@ pub enum AddressData { // The `#[repr(transparent)]` attribute is used to guarantee the layout of the `Address` struct. It // is an implementation detail and users should not rely on it in their code. #[repr(transparent)] -pub struct Address(AddressInner, PhantomData) +pub struct Address(PhantomData, AddressInner) where V: NetworkValidation; @@ -455,15 +455,15 @@ impl serde::Serialize for Address { /// `Address`. impl Address { fn from_inner(inner: AddressInner) -> Self { - Address(inner, PhantomData) + Address(PhantomData, inner) } fn into_inner(self) -> AddressInner { - self.0 + self.1 } fn inner(&self) -> &AddressInner { - &self.0 + &self.1 } /// Returns a reference to the address as if it was unchecked. From 1d1cf00b1e8eaff89eba86d4fa6f0dc3e395f2ca Mon Sep 17 00:00:00 2001 From: "Jamil Lambert, PhD" Date: Wed, 26 Mar 2025 09:36:42 +0000 Subject: [PATCH 011/278] Add test to BlockTime to kill mutants Mutants found in weekly mutation testing. Add a test to kill them. --- units/src/time.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/units/src/time.rs b/units/src/time.rs index 41faec706e..bd0cdb5565 100644 --- a/units/src/time.rs +++ b/units/src/time.rs @@ -56,3 +56,14 @@ impl<'a> Arbitrary<'a> for BlockTime { Ok(BlockTime::from(t)) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn block_time_round_trip() { + let t = BlockTime::from(1_742_979_600); // 26 Mar 2025 9:00 UTC + assert_eq!(u32::from(t), 1_742_979_600); + } +} From 9a2b56f38174ced77ecd139b1c1019dc86f8f062 Mon Sep 17 00:00:00 2001 From: "Jamil Lambert, PhD" Date: Wed, 26 Mar 2025 09:48:21 +0000 Subject: [PATCH 012/278] Modify test in Amount to kill mutants Mutants found in weekly mutation testing. Add an assert for both signed and unsigned to an existing test to kill them. --- units/src/amount/tests.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs index 127e7566c4..f8aba133f9 100644 --- a/units/src/amount/tests.rs +++ b/units/src/amount/tests.rs @@ -104,8 +104,13 @@ fn from_str_zero_without_denomination() { fn from_int_btc() { let amt = Amount::from_btc_u16(2); assert_eq!(sat(200_000_000), amt); + let amt = Amount::from_int_btc(2_u16); + assert_eq!(sat(200_000_000), amt); + let amt = SignedAmount::from_btc_i16(-2); assert_eq!(ssat(-200_000_000), amt); + let amt = SignedAmount::from_int_btc(-2_i16); + assert_eq!(ssat(-200_000_000), amt); } #[test] From f15e461baf463da0bf9d89018180c1d5032106c1 Mon Sep 17 00:00:00 2001 From: "Jamil Lambert, PhD" Date: Wed, 26 Mar 2025 09:50:50 +0000 Subject: [PATCH 013/278] Add an exclusion for a mutant in a deprecated fn Weekly mutation testing found a mutant in a deprecated function in SignedAmount. Exclude the function from mutation testing. --- .cargo/mutants.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/.cargo/mutants.toml b/.cargo/mutants.toml index 96f3735504..f4c71f5633 100644 --- a/.cargo/mutants.toml +++ b/.cargo/mutants.toml @@ -21,6 +21,7 @@ exclude_re = [ "Time::to_consensus_u32", # Mutant from replacing | with ^, this returns the same value since the XOR is taken against the u16 with an all-zero bitmask "FeeRate::fee_vb", # Deprecated "FeeRate::fee_wu", # Deprecated + "SignedAmount::checked_abs", # Deprecated # primitives "Sequence::from_512_second_intervals", # Mutant from replacing | with ^, this returns the same value since the XOR is taken against the u16 with an all-zero bitmask From 2867a7074ffef9c77d0a660d788b30d9afaf7494 Mon Sep 17 00:00:00 2001 From: todaymoon Date: Wed, 26 Mar 2025 21:03:03 +0800 Subject: [PATCH 014/278] chore: fix some typos in comment Signed-off-by: todaymoon --- hashes/src/sha256/crypto.rs | 6 +++--- internals/src/macros.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hashes/src/sha256/crypto.rs b/hashes/src/sha256/crypto.rs index eea9ff7a04..ef4b0f25ce 100644 --- a/hashes/src/sha256/crypto.rs +++ b/hashes/src/sha256/crypto.rs @@ -80,7 +80,7 @@ mod fast_hash { } impl Midstate { - #[allow(clippy::identity_op)] // more readble + #[allow(clippy::identity_op)] // more readable const fn read_u32(bytes: &[u8], index: usize) -> u32 { ((bytes[index + 0] as u32) << 24) | ((bytes[index + 1] as u32) << 16) @@ -130,7 +130,7 @@ impl Midstate { if (bytes.len() % 64 <= 64 - 9) || (chunk + 2 == num_chunks) { buf[i] = 0x80; } - #[allow(clippy::identity_op)] // more readble + #[allow(clippy::identity_op)] // more readable #[allow(clippy::erasing_op)] if chunk + 1 == num_chunks { let bit_len = bytes.len() as u64 * 8; @@ -235,7 +235,7 @@ impl Midstate { } let mut output = [0u8; 32]; let mut i = 0; - #[allow(clippy::identity_op)] // more readble + #[allow(clippy::identity_op)] // more readable while i < 8 { output[i * 4 + 0] = (state[i + 0] >> 24) as u8; output[i * 4 + 1] = (state[i + 0] >> 16) as u8; diff --git a/internals/src/macros.rs b/internals/src/macros.rs index 69725e232a..b4c95f2758 100644 --- a/internals/src/macros.rs +++ b/internals/src/macros.rs @@ -8,7 +8,7 @@ macro_rules! const_assert { ($x:expr $(; $message:expr)?) => { const _: () = { if !$x { - // We can't use formatting in const, only concating literals. + // We can't use formatting in const, only concatenating literals. panic!(concat!("assertion ", stringify!($x), " failed" $(, ": ", $message)?)) } }; From 9a572dabdeb077f96b2ab66be1a80fcec3e805e3 Mon Sep 17 00:00:00 2001 From: Eval EXEC Date: Tue, 25 Mar 2025 17:48:58 +0800 Subject: [PATCH 015/278] refactor: use path dependencies for workspace members in bitcoin/Cargo.toml Signed-off-by: Eval EXEC --- Cargo.toml | 24 +++--------------------- base58/Cargo.toml | 4 ++-- bitcoin/Cargo.toml | 14 +++++++------- bitcoin/embedded/Cargo.toml | 14 -------------- hashes/Cargo.toml | 2 +- hashes/embedded/Cargo.toml | 9 --------- io/Cargo.toml | 6 +++--- primitives/Cargo.toml | 6 +++--- units/Cargo.toml | 4 ++-- 9 files changed, 21 insertions(+), 62 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8f8a3c163c..7d9a5d5264 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,26 +2,8 @@ members = ["addresses", "base58", "bitcoin", "chacha20_poly1305", "fuzz", "hashes", "internals", "io", "primitives", "units"] resolver = "2" -[patch.crates-io.bitcoin-addresses] -path = "addresses" - -[patch.crates-io.base58ck] -path = "base58" - -[patch.crates-io.bitcoin] -path = "bitcoin" - +# Keep this patch for hashes because secp256k1 depends on bitcoin-hashes via crates.io +# This allows testing changes to hashes with secp256k1 +# See https://github.com/rust-bitcoin/rust-bitcoin/pull/4284#pullrequestreview-2714442229 [patch.crates-io.bitcoin_hashes] path = "hashes" - -[patch.crates-io.bitcoin-internals] -path = "internals" - -[patch.crates-io.bitcoin-io] -path = "io" - -[patch.crates-io.bitcoin-primitives] -path = "primitives" - -[patch.crates-io.bitcoin-units] -path = "units" diff --git a/base58/Cargo.toml b/base58/Cargo.toml index 06000bcbf9..7a64cf5abb 100644 --- a/base58/Cargo.toml +++ b/base58/Cargo.toml @@ -18,8 +18,8 @@ std = ["alloc", "hashes/std", "internals/std"] alloc = ["hashes/alloc", "internals/alloc"] [dependencies] -hashes = { package = "bitcoin_hashes", version = "0.16.0", default-features = false } -internals = { package = "bitcoin-internals", version = "0.4.0" } +hashes = { package = "bitcoin_hashes", path = "../hashes", default-features = false } +internals = { package = "bitcoin-internals", path = "../internals" } [dev-dependencies] hex = { package = "hex-conservative", version = "0.3.0", default-features = false, features = ["alloc"] } diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index 7784efff71..85b7291f66 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -25,15 +25,15 @@ secp-recovery = ["secp256k1/recovery"] arbitrary = ["dep:arbitrary", "units/arbitrary", "primitives/arbitrary"] [dependencies] -base58 = { package = "base58ck", version = "0.2.0", default-features = false, features = ["alloc"] } +base58 = { package = "base58ck", path = "../base58", default-features = false, features = ["alloc"] } bech32 = { version = "0.11.0", default-features = false, features = ["alloc"] } -hashes = { package = "bitcoin_hashes", version = "0.16.0", default-features = false, features = ["alloc", "hex"] } +hashes = { package = "bitcoin_hashes", path = "../hashes", default-features = false, features = ["alloc", "hex"] } hex = { package = "hex-conservative", version = "0.3.0", default-features = false, features = ["alloc"] } -internals = { package = "bitcoin-internals", version = "0.4.0", features = ["alloc", "hex"] } -io = { package = "bitcoin-io", version = "0.2.0", default-features = false, features = ["alloc", "hashes"] } -primitives = { package = "bitcoin-primitives", version = "0.101.0", default-features = false, features = ["alloc"] } +internals = { package = "bitcoin-internals", path = "../internals", features = ["alloc", "hex"] } +io = { package = "bitcoin-io", path = "../io", default-features = false, features = ["alloc", "hashes"] } +primitives = { package = "bitcoin-primitives", path = "../primitives", default-features = false, features = ["alloc"] } secp256k1 = { version = "0.30.0", default-features = false, features = ["hashes", "alloc", "rand"] } -units = { package = "bitcoin-units", version = "0.2.0", default-features = false, features = ["alloc"] } +units = { package = "bitcoin-units", path = "../units", default-features = false, features = ["alloc"] } arbitrary = { version = "1.4", optional = true } base64 = { version = "0.22.0", optional = true } @@ -42,7 +42,7 @@ bitcoinconsensus = { version = "0.106.0", default-features = false, optional = t serde = { version = "1.0.103", default-features = false, features = [ "derive", "alloc" ], optional = true } [dev-dependencies] -internals = { package = "bitcoin-internals", version = "0.4.0", features = ["test-serde"] } +internals = { package = "bitcoin-internals", path = "../internals", features = ["test-serde"] } serde_json = "1.0.0" serde_test = "1.0.19" bincode = "1.3.1" diff --git a/bitcoin/embedded/Cargo.toml b/bitcoin/embedded/Cargo.toml index e754e76ff3..01fcd890ca 100644 --- a/bitcoin/embedded/Cargo.toml +++ b/bitcoin/embedded/Cargo.toml @@ -27,20 +27,6 @@ codegen-units = 1 # better optimizations debug = true # symbols are nice and they don't increase the size on Flash lto = true # better optimizations -[patch.crates-io.base58ck] -path = "../../base58" [patch.crates-io.bitcoin_hashes] path = "../../hashes" - -[patch.crates-io.bitcoin-internals] -path = "../../internals" - -[patch.crates-io.bitcoin-io] -path = "../../io" - -[patch.crates-io.bitcoin-primitives] -path = "../../primitives" - -[patch.crates-io.bitcoin-units] -path = "../../units" diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 3e020d4b9a..9a0f8d1a7a 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -22,7 +22,7 @@ serde = ["dep:serde", "hex"] small-hash = [] [dependencies] -internals = { package = "bitcoin-internals", version = "0.4.0" } +internals = { package = "bitcoin-internals", path = "../internals" } hex = { package = "hex-conservative", version = "0.3.0", default-features = false, optional = true } serde = { version = "1.0", default-features = false, optional = true } diff --git a/hashes/embedded/Cargo.toml b/hashes/embedded/Cargo.toml index dcf5a7ecbc..f6779e42df 100644 --- a/hashes/embedded/Cargo.toml +++ b/hashes/embedded/Cargo.toml @@ -34,12 +34,3 @@ lto = true # better optimizations [patch.crates-io.bitcoin_hashes] path = "../../hashes" - -[patch.crates-io.bitcoin-internals] -path = "../../internals" - -[patch.crates-io.bitcoin-io] -path = "../../io" - -[patch.crates-io.bitcoin-units] -path = "../../units" diff --git a/io/Cargo.toml b/io/Cargo.toml index f59c35e00c..07b38baa61 100644 --- a/io/Cargo.toml +++ b/io/Cargo.toml @@ -19,12 +19,12 @@ std = ["alloc", "hashes?/std"] alloc = ["hashes?/alloc"] [dependencies] -internals = { package = "bitcoin-internals", version = "0.4.0" } +internals = { package = "bitcoin-internals", path = "../internals" } -hashes = { package = "bitcoin_hashes", version = "0.16.0", default-features = false, optional = true } +hashes = { package = "bitcoin_hashes", path = "../hashes", default-features = false, optional = true } [dev-dependencies] -hashes = { package = "bitcoin_hashes", version = "0.16.0", default-features = false, features = ["hex"] } +hashes = { package = "bitcoin_hashes", path = "../hashes", default-features = false, features = ["hex"] } [package.metadata.docs.rs] all-features = true diff --git a/primitives/Cargo.toml b/primitives/Cargo.toml index b8b0bdd4c5..6a5659547f 100644 --- a/primitives/Cargo.toml +++ b/primitives/Cargo.toml @@ -22,10 +22,10 @@ serde = ["dep:serde", "hashes/serde", "internals/serde", "units/serde", "alloc"] arbitrary = ["dep:arbitrary", "units/arbitrary"] [dependencies] -hashes = { package = "bitcoin_hashes", version = "0.16.0", default-features = false, features = ["hex"] } +hashes = { package = "bitcoin_hashes", path = "../hashes", default-features = false, features = ["hex"] } hex = { package = "hex-conservative", version = "0.3.0", default-features = false } -internals = { package = "bitcoin-internals", version = "0.4.0", features = ["hex"] } -units = { package = "bitcoin-units", version = "0.2.0", default-features = false } +internals = { package = "bitcoin-internals", path = "../internals", features = ["hex"] } +units = { package = "bitcoin-units", path = "../units", default-features = false } arbitrary = { version = "1.4", optional = true } serde = { version = "1.0.103", default-features = false, features = ["derive", "alloc"], optional = true } diff --git a/units/Cargo.toml b/units/Cargo.toml index f2a73f3093..5134003cdd 100644 --- a/units/Cargo.toml +++ b/units/Cargo.toml @@ -18,13 +18,13 @@ std = ["alloc", "internals/std"] alloc = ["internals/alloc","serde?/alloc"] [dependencies] -internals = { package = "bitcoin-internals", version = "0.4.0" } +internals = { package = "bitcoin-internals", path = "../internals" } serde = { version = "1.0.103", default-features = false, features = ["derive"], optional = true } arbitrary = { version = "1.4", optional = true } [dev-dependencies] -internals = { package = "bitcoin-internals", version = "0.4.0", features = ["test-serde"] } +internals = { package = "bitcoin-internals", path = "../internals", features = ["test-serde"] } bincode = "1.3.1" serde_test = "1.0" serde_json = "1.0" From 6ebdf61e769903da1ad21fd6ab9bf22ff3c04bab Mon Sep 17 00:00:00 2001 From: VolodymyrBg Date: Thu, 27 Mar 2025 20:17:18 +0200 Subject: [PATCH 016/278] Fix grammatical typos Fix grammar and typos in documentation and README --- base58/src/error.rs | 2 +- bitcoin/CHANGELOG.md | 2 +- units/src/amount/error.rs | 2 +- units/src/fee_rate/mod.rs | 2 +- units/tests/api.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/base58/src/error.rs b/base58/src/error.rs index 678c22d930..6b7b21f6c3 100644 --- a/base58/src/error.rs +++ b/base58/src/error.rs @@ -141,7 +141,7 @@ impl fmt::Display for TooShortError { #[cfg(feature = "std")] impl std::error::Error for TooShortError {} -/// Found a invalid ASCII byte while decoding base58 string. +/// Found an invalid ASCII byte while decoding base58 string. #[derive(Debug, Clone, PartialEq, Eq)] pub struct InvalidCharacterError(pub(super) InvalidCharacterErrorInner); diff --git a/bitcoin/CHANGELOG.md b/bitcoin/CHANGELOG.md index 0972ca23b7..b91b3bad2f 100644 --- a/bitcoin/CHANGELOG.md +++ b/bitcoin/CHANGELOG.md @@ -311,7 +311,7 @@ In particular consider having some type that implements `AsRef`, we have - Rename `ExtendedPubKey` to `Xpub` [#2019](https://github.com/rust-bitcoin/rust-bitcoin/pull/2019) - Rename `ExtendedPrivKey` to `Xpriv` [#2019](https://github.com/rust-bitcoin/rust-bitcoin/pull/2019) - Remove `_v0` from various function names (eg, `new_v0_p2wpkh`) [#1994](https://github.com/rust-bitcoin/rust-bitcoin/pull/1994) - - Remove `SighashCache::segwit_signature_hash` (add `p2wpkh_signiture_hash` and `p2wsh_signature_hash`) [#1995](https://github.com/rust-bitcoin/rust-bitcoin/pull/1995) + - Remove `SighashCache::segwit_signature_hash` (add `p2wpkh_signature_hash` and `p2wsh_signature_hash`) [#1995](https://github.com/rust-bitcoin/rust-bitcoin/pull/1995) - Reexport all the hash types from the crate root [#1998](https://github.com/rust-bitcoin/rust-bitcoin/pull/1998) - Rename `opcodes::All` to `Opcode` [#1995](https://github.com/rust-bitcoin/rust-bitcoin/pull/1995) - Removed `TxOut::default()`, the same logic now exists as `TxOut::NULL` [#1811](https://github.com/rust-bitcoin/rust-bitcoin/pull/1811) and [#1838](https://github.com/rust-bitcoin/rust-bitcoin/pull/1838) diff --git a/units/src/amount/error.rs b/units/src/amount/error.rs index 992d7e543f..2c91392b4b 100644 --- a/units/src/amount/error.rs +++ b/units/src/amount/error.rs @@ -176,7 +176,7 @@ impl OutOfRangeError { } } - /// Returns true if the input value was large than the maximum allowed value. + /// Returns true if the input value was larger than the maximum allowed value. pub fn is_above_max(self) -> bool { self.is_greater_than_max } /// Returns true if the input value was smaller than the minimum allowed value. diff --git a/units/src/fee_rate/mod.rs b/units/src/fee_rate/mod.rs index f3ec8fd7d9..238d21a734 100644 --- a/units/src/fee_rate/mod.rs +++ b/units/src/fee_rate/mod.rs @@ -1,6 +1,6 @@ // SPDX-License-Identifier: CC0-1.0 -//! Implements `FeeRate` and assoctiated features. +//! Implements `FeeRate` and associated features. #[cfg(feature = "serde")] pub mod serde; diff --git a/units/tests/api.rs b/units/tests/api.rs index e3b89fed0f..46a16e422a 100644 --- a/units/tests/api.rs +++ b/units/tests/api.rs @@ -2,7 +2,7 @@ //! Test the API surface of `units`. //! -//! The point of these tests are to check the API surface as opposed to test the API functionality. +//! The point of these tests is to check the API surface as opposed to test the API functionality. //! //! ref: From e8a42d5851000d210ea9b886478d83f3e7c077a6 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 24 Mar 2025 14:37:13 +0100 Subject: [PATCH 017/278] Unify/reduce usage of `unsafe` Since the introduction of `Script` `unsafe` started slowly creeping in as more types with similar semantics were added. The `unsafe` in these cases is just for trivial conversions between various pointer-like types. As such, it's possible to move these into a single macro that takes care of the conversions at one place and avoid repeating the same `unsafe` code in the codebase. This decreases the cost of audits which now only need to happen in `internals`, focuses any changes to happen in that single macro and decreases the chance that we will mess up similarly to the recent `try_into().expect()` issue (but this time with UB rather than panic). The new macro accepts syntax very similar to the already-existing struct declarations with these differences: * The struct MUST NOT have `#[repr(transparent)]` - it's added by the macro * If the struct uses `PhantomData` it must be the first field and the real data must be the second field (to allow unsized types). * The struct must be immediately followed by an impl block containing at least on conversion function. * If the struct has generics the impl block has to use the same names of generics. * The conversion functions don't have bodies (similarly to required trait methods) and have a fixed set of allowed signatures. * Underscore (`_`) must be used in place of the inner type in the conversion function parameters. The existing code can simply call the macro with simple changes and get the same behavior without any direct use of `unsafe`. This change already calls the macro for all relevant existing types. There are still some usages left unrelated to the macro, except one additional conversion in reverse direction on `Script`. It could be moved as well but since it's on a single place so far it's not really required. --- bitcoin/src/address/mod.rs | 189 ++++++++-------- bitcoin/src/blockdata/script/push_bytes.rs | 44 ++-- bitcoin/src/taproot/merkle_branch/borrowed.rs | 26 +-- hashes/src/internal_macros.rs | 33 ++- hashes/src/sha256t/mod.rs | 31 ++- internals/src/lib.rs | 6 + internals/src/macros.rs | 208 ++++++++++++++++++ io/src/bridge.rs | 67 +++--- primitives/src/script/borrowed.rs | 131 +++++------ primitives/src/script/mod.rs | 14 +- primitives/src/script/owned.rs | 4 +- 11 files changed, 460 insertions(+), 293 deletions(-) diff --git a/bitcoin/src/address/mod.rs b/bitcoin/src/address/mod.rs index 48cf871fd7..df8e04067d 100644 --- a/bitcoin/src/address/mod.rs +++ b/bitcoin/src/address/mod.rs @@ -308,96 +308,101 @@ pub enum AddressData { }, } -/// A Bitcoin address. -/// -/// ### Parsing addresses -/// -/// When parsing string as an address, one has to pay attention to the network, on which the parsed -/// address is supposed to be valid. For the purpose of this validation, `Address` has -/// [`is_valid_for_network`](Address::is_valid_for_network) method. In order to provide more safety, -/// enforced by compiler, `Address` also contains a special marker type, which indicates whether network of the parsed -/// address has been checked. This marker type will prevent from calling certain functions unless the network -/// verification has been successfully completed. -/// -/// The result of parsing an address is `Address` suggesting that network of the parsed address -/// has not yet been verified. To perform this verification, method [`require_network`](Address::require_network) -/// can be called, providing network on which the address is supposed to be valid. If the verification succeeds, -/// `Address` is returned. -/// -/// The types `Address` and `Address` are synonymous, i. e. they can be used interchangeably. -/// -/// ```rust -/// use std::str::FromStr; -/// use bitcoin::{Address, Network}; -/// use bitcoin::address::{NetworkUnchecked, NetworkChecked}; -/// -/// // variant 1 -/// let address: Address = "32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf".parse().unwrap(); -/// let _address: Address = address.require_network(Network::Bitcoin).unwrap(); -/// -/// // variant 2 -/// let _address: Address = Address::from_str("32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf").unwrap() -/// .require_network(Network::Bitcoin).unwrap(); -/// -/// // variant 3 -/// let _address: Address = "32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf".parse::>() -/// .unwrap().require_network(Network::Bitcoin).unwrap(); -/// ``` -/// -/// ### Formatting addresses -/// -/// To format address into its textual representation, both `Debug` (for usage in programmer-facing, -/// debugging context) and `Display` (for user-facing output) can be used, with the following caveats: -/// -/// 1. `Display` is implemented only for `Address`: -/// -/// ``` -/// # use bitcoin::address::{Address, NetworkChecked}; -/// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() -/// .unwrap().assume_checked(); -/// assert_eq!(address.to_string(), "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM"); -/// ``` -/// -/// ```ignore -/// # use bitcoin::address::{Address, NetworkChecked}; -/// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() -/// .unwrap(); -/// let s = address.to_string(); // does not compile -/// ``` -/// -/// 2. `Debug` on `Address` does not produce clean address but address wrapped by -/// an indicator that its network has not been checked. This is to encourage programmer to properly -/// check the network and use `Display` in user-facing context. -/// -/// ``` -/// # use bitcoin::address::{Address, NetworkUnchecked}; -/// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() -/// .unwrap(); -/// assert_eq!(format!("{:?}", address), "Address(132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM)"); -/// ``` -/// -/// ``` -/// # use bitcoin::address::{Address, NetworkChecked}; -/// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() -/// .unwrap().assume_checked(); -/// assert_eq!(format!("{:?}", address), "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM"); -/// ``` -/// -/// ### Relevant BIPs -/// -/// * [BIP13 - Address Format for pay-to-script-hash](https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki) -/// * [BIP16 - Pay to Script Hash](https://github.com/bitcoin/bips/blob/master/bip-0016.mediawiki) -/// * [BIP141 - Segregated Witness (Consensus layer)](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki) -/// * [BIP142 - Address Format for Segregated Witness](https://github.com/bitcoin/bips/blob/master/bip-0142.mediawiki) -/// * [BIP341 - Taproot: SegWit version 1 spending rules](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki) -/// * [BIP350 - Bech32m format for v1+ witness addresses](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki) -#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -// The `#[repr(transparent)]` attribute is used to guarantee the layout of the `Address` struct. It -// is an implementation detail and users should not rely on it in their code. -#[repr(transparent)] -pub struct Address(PhantomData, AddressInner) -where - V: NetworkValidation; +internals::transparent_newtype! { + /// A Bitcoin address. + /// + /// ### Parsing addresses + /// + /// When parsing string as an address, one has to pay attention to the network, on which the parsed + /// address is supposed to be valid. For the purpose of this validation, `Address` has + /// [`is_valid_for_network`](Address::is_valid_for_network) method. In order to provide more safety, + /// enforced by compiler, `Address` also contains a special marker type, which indicates whether network of the parsed + /// address has been checked. This marker type will prevent from calling certain functions unless the network + /// verification has been successfully completed. + /// + /// The result of parsing an address is `Address` suggesting that network of the parsed address + /// has not yet been verified. To perform this verification, method [`require_network`](Address::require_network) + /// can be called, providing network on which the address is supposed to be valid. If the verification succeeds, + /// `Address` is returned. + /// + /// The types `Address` and `Address` are synonymous, i. e. they can be used interchangeably. + /// + /// ```rust + /// use std::str::FromStr; + /// use bitcoin::{Address, Network}; + /// use bitcoin::address::{NetworkUnchecked, NetworkChecked}; + /// + /// // variant 1 + /// let address: Address = "32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf".parse().unwrap(); + /// let _address: Address = address.require_network(Network::Bitcoin).unwrap(); + /// + /// // variant 2 + /// let _address: Address = Address::from_str("32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf").unwrap() + /// .require_network(Network::Bitcoin).unwrap(); + /// + /// // variant 3 + /// let _address: Address = "32iVBEu4dxkUQk9dJbZUiBiQdmypcEyJRf".parse::>() + /// .unwrap().require_network(Network::Bitcoin).unwrap(); + /// ``` + /// + /// ### Formatting addresses + /// + /// To format address into its textual representation, both `Debug` (for usage in programmer-facing, + /// debugging context) and `Display` (for user-facing output) can be used, with the following caveats: + /// + /// 1. `Display` is implemented only for `Address`: + /// + /// ``` + /// # use bitcoin::address::{Address, NetworkChecked}; + /// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() + /// .unwrap().assume_checked(); + /// assert_eq!(address.to_string(), "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM"); + /// ``` + /// + /// ```ignore + /// # use bitcoin::address::{Address, NetworkChecked}; + /// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() + /// .unwrap(); + /// let s = address.to_string(); // does not compile + /// ``` + /// + /// 2. `Debug` on `Address` does not produce clean address but address wrapped by + /// an indicator that its network has not been checked. This is to encourage programmer to properly + /// check the network and use `Display` in user-facing context. + /// + /// ``` + /// # use bitcoin::address::{Address, NetworkUnchecked}; + /// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() + /// .unwrap(); + /// assert_eq!(format!("{:?}", address), "Address(132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM)"); + /// ``` + /// + /// ``` + /// # use bitcoin::address::{Address, NetworkChecked}; + /// let address: Address = "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM".parse::>() + /// .unwrap().assume_checked(); + /// assert_eq!(format!("{:?}", address), "132F25rTsvBdp9JzLLBHP5mvGY66i1xdiM"); + /// ``` + /// + /// ### Relevant BIPs + /// + /// * [BIP13 - Address Format for pay-to-script-hash](https://github.com/bitcoin/bips/blob/master/bip-0013.mediawiki) + /// * [BIP16 - Pay to Script Hash](https://github.com/bitcoin/bips/blob/master/bip-0016.mediawiki) + /// * [BIP141 - Segregated Witness (Consensus layer)](https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki) + /// * [BIP142 - Address Format for Segregated Witness](https://github.com/bitcoin/bips/blob/master/bip-0142.mediawiki) + /// * [BIP341 - Taproot: SegWit version 1 spending rules](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki) + /// * [BIP350 - Bech32m format for v1+ witness addresses](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki) + #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] + // The `#[repr(transparent)]` attribute is used to guarantee the layout of the `Address` struct. It + // is an implementation detail and users should not rely on it in their code. + pub struct Address(PhantomData, AddressInner) + where + V: NetworkValidation; + + impl Address { + fn from_inner_ref(inner: &_) -> &Self; + } +} #[cfg(feature = "serde")] struct DisplayUnchecked<'a, N: NetworkValidation>(&'a Address); @@ -468,7 +473,7 @@ impl Address { /// Returns a reference to the address as if it was unchecked. pub fn as_unchecked(&self) -> &Address { - unsafe { &*(self as *const Address as *const Address) } + Address::from_inner_ref(self.inner()) } /// Marks the network of this address as unchecked. @@ -803,7 +808,7 @@ impl Address { /// /// This function is dangerous in case the address is not a valid checked address. pub fn assume_checked_ref(&self) -> &Address { - unsafe { &*(self as *const Address as *const Address) } + Address::from_inner_ref(self.inner()) } /// Parsed addresses do not always have *one* network. The problem is that legacy testnet, diff --git a/bitcoin/src/blockdata/script/push_bytes.rs b/bitcoin/src/blockdata/script/push_bytes.rs index bae7902b3b..f469290f04 100644 --- a/bitcoin/src/blockdata/script/push_bytes.rs +++ b/bitcoin/src/blockdata/script/push_bytes.rs @@ -35,33 +35,29 @@ mod primitive { } } - /// Byte slices that can be in Bitcoin script. - /// - /// The encoding of Bitcoin script restricts data pushes to be less than 2^32 bytes long. - /// This type represents slices that are guaranteed to be within the limit so they can be put in - /// the script safely. - #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] - #[repr(transparent)] - pub struct PushBytes([u8]); - - impl PushBytes { - /// Constructs a new `&PushBytes` without checking the length. + internals::transparent_newtype! { + /// Byte slices that can be in Bitcoin script. /// - /// The caller is responsible for checking that the length is less than the 2^32. - fn from_slice_unchecked(bytes: &[u8]) -> &Self { - // SAFETY: The conversion is sound because &[u8] and &PushBytes - // have the same layout (because of #[repr(transparent)] on PushBytes). - unsafe { &*(bytes as *const [u8] as *const PushBytes) } + /// The encoding of Bitcoin script restricts data pushes to be less than 2^32 bytes long. + /// This type represents slices that are guaranteed to be within the limit so they can be put in + /// the script safely. + #[derive(Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] + pub struct PushBytes([u8]); + + impl PushBytes { + /// Constructs a new `&PushBytes` without checking the length. + /// + /// The caller is responsible for checking that the length is less than the 2^32. + fn from_slice_unchecked(bytes: &_) -> &Self; + + /// Constructs a new `&mut PushBytes` without checking the length. + /// + /// The caller is responsible for checking that the length is less than the 2^32. + fn from_mut_slice_unchecked(bytes: &mut _) -> &mut Self; } + } - /// Constructs a new `&mut PushBytes` without checking the length. - /// - /// The caller is responsible for checking that the length is less than the 2^32. - fn from_mut_slice_unchecked(bytes: &mut [u8]) -> &mut Self { - // SAFETY: The conversion is sound because &mut [u8] and &mut PushBytes - // have the same layout (because of #[repr(transparent)] on PushBytes). - unsafe { &mut *(bytes as *mut [u8] as *mut PushBytes) } - } + impl PushBytes { /// Constructs an empty `&PushBytes`. pub fn empty() -> &'static Self { Self::from_slice_unchecked(&[]) } diff --git a/bitcoin/src/taproot/merkle_branch/borrowed.rs b/bitcoin/src/taproot/merkle_branch/borrowed.rs index 58538f09d2..6ca681e259 100644 --- a/bitcoin/src/taproot/merkle_branch/borrowed.rs +++ b/bitcoin/src/taproot/merkle_branch/borrowed.rs @@ -9,10 +9,16 @@ pub use privacy_boundary::TaprootMerkleBranch; mod privacy_boundary { use super::*; - /// The Merkle proof for inclusion of a tree in a Taproot tree hash. - #[repr(transparent)] - #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] - pub struct TaprootMerkleBranch([TapNodeHash]); + internals::transparent_newtype! { + /// The Merkle proof for inclusion of a tree in a Taproot tree hash. + #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct TaprootMerkleBranch([TapNodeHash]); + + impl TaprootMerkleBranch { + pub(super) const fn from_hashes_unchecked(hashes: &_) -> &Self; + pub(super) fn from_mut_hashes_unchecked(hashes: &mut _) -> &mut Self; + } + } impl TaprootMerkleBranch { /// Returns a reference to the slice of hashes. @@ -22,18 +28,6 @@ mod privacy_boundary { /// Returns a reference to the mutable slice of hashes. #[inline] pub fn as_mut_slice(&mut self) -> &mut [TapNodeHash] { &mut self.0 } - - pub(super) const fn from_hashes_unchecked(hashes: &[TapNodeHash]) -> &Self { - unsafe { - &*(hashes as *const _ as *const Self) - } - } - - pub(super) fn from_mut_hashes_unchecked(hashes: &mut [TapNodeHash]) -> &mut Self { - unsafe { - &mut *(hashes as *mut _ as *mut Self) - } - } } } diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index 5e9599322a..19945b46cf 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -117,10 +117,21 @@ pub(crate) use general_hash_type; macro_rules! hash_type_no_default { ($bits:expr, $reverse:expr, $doc:literal) => { - #[doc = $doc] - #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] - #[repr(transparent)] - pub struct Hash([u8; $bits / 8]); + internals::transparent_newtype! { + #[doc = $doc] + #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub struct Hash([u8; $bits / 8]); + + impl Hash { + /// Zero cost conversion between a fixed length byte array shared reference and + /// a shared reference to this Hash type. + pub fn from_bytes_ref(bytes: &_) -> &Self; + + /// Zero cost conversion between a fixed length byte array exclusive reference and + /// an exclusive reference to this Hash type. + pub fn from_bytes_mut(bytes: &mut _) -> &mut Self; + } + } impl Hash { const fn internal_new(arr: [u8; $bits / 8]) -> Self { Hash(arr) } @@ -130,20 +141,6 @@ macro_rules! hash_type_no_default { Self::internal_new(bytes) } - /// Zero cost conversion between a fixed length byte array shared reference and - /// a shared reference to this Hash type. - pub fn from_bytes_ref(bytes: &[u8; $bits / 8]) -> &Self { - // Safety: Sound because Self is #[repr(transparent)] containing [u8; $bits / 8] - unsafe { &*(bytes as *const _ as *const Self) } - } - - /// Zero cost conversion between a fixed length byte array exclusive reference and - /// an exclusive reference to this Hash type. - pub fn from_bytes_mut(bytes: &mut [u8; $bits / 8]) -> &mut Self { - // Safety: Sound because Self is #[repr(transparent)] containing [u8; $bits / 8] - unsafe { &mut *(bytes as *mut _ as *mut Self) } - } - /// Copies a byte slice into a hash object. #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")] #[allow(deprecated_in_future)] // Because of `FromSliceError`. diff --git a/hashes/src/sha256t/mod.rs b/hashes/src/sha256t/mod.rs index 59f73bf0f1..526892db23 100644 --- a/hashes/src/sha256t/mod.rs +++ b/hashes/src/sha256t/mod.rs @@ -43,9 +43,20 @@ pub trait Tag { const MIDSTATE: sha256::Midstate; } -/// Output of the SHA256t hash function. -#[repr(transparent)] -pub struct Hash(PhantomData, [u8; 32]); +internals::transparent_newtype! { + /// Output of the SHA256t hash function. + pub struct Hash(PhantomData, [u8; 32]); + + impl Hash { + /// Zero cost conversion between a fixed length byte array shared reference and + /// a shared reference to this Hash type. + pub fn from_bytes_ref(bytes: &_) -> &Self; + + /// Zero cost conversion between a fixed length byte array exclusive reference and + /// an exclusive reference to this Hash type. + pub fn from_bytes_mut(bytes: &mut _) -> &mut Self; + } +} impl Hash where @@ -56,20 +67,6 @@ where /// Constructs a new hash from the underlying byte array. pub const fn from_byte_array(bytes: [u8; 32]) -> Self { Self::internal_new(bytes) } - /// Zero cost conversion between a fixed length byte array shared reference and - /// a shared reference to this Hash type. - pub fn from_bytes_ref(bytes: &[u8; 32]) -> &Self { - // Safety: Sound because Self is #[repr(transparent)] containing [u8; 32] - unsafe { &*(bytes as *const _ as *const Self) } - } - - /// Zero cost conversion between a fixed length byte array exclusive reference and - /// an exclusive reference to this Hash type. - pub fn from_bytes_mut(bytes: &mut [u8; 32]) -> &mut Self { - // Safety: Sound because Self is #[repr(transparent)] containing [u8; 32] - unsafe { &mut *(bytes as *mut _ as *mut Self) } - } - /// Copies a byte slice into a hash object. #[deprecated(since = "0.15.0", note = "use `from_byte_array` instead")] #[allow(deprecated_in_future)] // Because of `FromSliceError`. diff --git a/internals/src/lib.rs b/internals/src/lib.rs index c2e2ce7080..35c1791423 100644 --- a/internals/src/lib.rs +++ b/internals/src/lib.rs @@ -35,6 +35,12 @@ pub mod rust_version { include!(concat!(env!("OUT_DIR"), "/rust_version.rs")); } +#[doc(hidden)] +pub mod _export { + #[cfg(feature = "alloc")] + pub extern crate alloc; +} + pub mod array; pub mod array_vec; pub mod compact_size; diff --git a/internals/src/macros.rs b/internals/src/macros.rs index 69725e232a..aa5ed1b62f 100644 --- a/internals/src/macros.rs +++ b/internals/src/macros.rs @@ -37,3 +37,211 @@ macro_rules! impl_to_hex_from_lower_hex { } }; } + +/// Creates a transparent wrapper around an inner type and soundly implements reference casts. +/// +/// This macro takes care of several issues related to newtypes that need to allow casting their +/// inner types to themselves: +/// +/// * It makes sure to put repr(transparent) on the type +/// * It optionally implements conversions from `&`, `&mut`, `Box`, `Rc`, `Arc` +/// * It makes sure to put `#[inline]` on all of these conversions since they are trivial +/// * It makes sure the reference cast is const +/// * It makes sure the `Arc` conversion is conditioned on `target_has_atomic = "ptr"` +/// +/// Usage: just type the struct inside the macro as you would implementing it manually except leave +/// `#[repr(transparent)]` out. Then add an impl block for the just-defined type containing function +/// declarations that take a reference/smart pointer to `_` (use literal underscore; e.g. `&_` for +/// shared references) and return `Self` behind the appropriate "pointer" type. Do not write the +/// body, just semicolon. +/// +/// The `alloc` types MUST NOT have import paths and don't need imports. +#[macro_export] +macro_rules! transparent_newtype { + ( + $(#[$($struct_attr:tt)*])* + $vis:vis struct $newtype:tt$(<$gen:ident $(= $default:ty)?>)?($($fields:tt)+) $(where $($where_ty:ty: $bound:path),* $(,)?)?; + + impl$(<$gen2:tt>)? $newtype2:ident$(<$gen3:tt>)? { + $( + $(#[$($fn_attr:tt)*])* + $fn_vis:vis $(const)? fn $fn:ident($fn_arg_name:ident: $($fn_arg_ty:tt)+) -> $fn_ret_ty:ty; + )* + } + ) => { + $crate::_check_tts_eq!($newtype2, $newtype, "the type name in the impl block doesn't match the struct name"); + $( + // WARNING: renaming has to be disabled for soundness! + // If it weren't it'd be possible to make the type inside struct not match the one passed + // to functions. In principle we could also omit the generics but that'd be confusing for + // readers. + $crate::_check_tts_eq!($gen2, $gen, "the name of the left generic parameter in impl block doesn't match the one on struct"); + $crate::_check_tts_eq!($gen3, $gen, "the name of the right generic parameter in impl block doesn't match the one on struct"); + )? + $(#[$($struct_attr)*])* + #[repr(transparent)] + $vis struct $newtype$(<$gen $(= $default)?>)?($($fields)+) $(where $($where_ty: $bound),*)?; + + impl$(<$gen2>)? $newtype$(<$gen3>)? $(where $($where_ty: $bound),*)? { + $crate::_transparent_ref_conversions! { + $crate::_transparent_newtype_inner_type!($($fields)+); + $( + $(#[$($fn_attr)*])* + $fn_vis fn $fn($fn_arg_name: $($fn_arg_ty)+) -> $fn_ret_ty; + )+ + } + } + }; +} + +#[doc(hidden)] +#[macro_export] +macro_rules! _transparent_ref_conversions { + ( + $inner:ty; + $( + $(#[$($fn_attr:tt)*])* + $fn_vis:vis $(const)? fn $fn:ident($fn_arg_name:ident: $($fn_arg_ty:tt)+) -> $fn_ret_ty:ty; + )+ + ) => { + $( + $crate::_transparent_ref_conversion! { + $inner; + $(#[$($fn_attr)*])* + $fn_vis fn $fn($fn_arg_name: $($fn_arg_ty)+) -> $fn_ret_ty; + } + )+ + } +} + +#[doc(hidden)] +#[macro_export] +macro_rules! _transparent_ref_conversion { + ( + $inner:ty; + $(#[$($from_ref_attr:tt)*])* + $from_ref_vis:vis fn $from_ref:ident($from_ref_arg_name:ident: &_) -> $fn_ret_ty:ty; + ) => { + #[inline] + $(#[$($from_ref_attr)*])* + $from_ref_vis const fn $from_ref($from_ref_arg_name: &$inner) -> &Self { + // SAFETY: the pointer is created by casting a pointer that is pointing to an object + // with the same layout and validity invariants and the previous pointer was created + // directly from a reference. (Notice repr(transparent).) + // The lifetime of the input reference matches the lifetime of the returned reference. + unsafe { &*($from_ref_arg_name as *const $inner as *const Self) } + } + }; + ( + $inner:ty; + $(#[$($from_mut_attr:tt)*])* + $from_mut_vis:vis fn $from_mut:ident($from_mut_arg_name:ident: &mut _) -> $fn_ret_ty:ty; + ) => { + #[inline] + $(#[$($from_mut_attr)*])* + $from_mut_vis fn $from_mut($from_mut_arg_name: &mut $inner) -> &mut Self { + // SAFETY: the pointer is created by casting a pointer that is pointing to an object + // with the same layout and validity invariants and the previous pointer was created + // directly from a reference. (Notice repr(transparent).) + // The lifetime of the input reference matches the lifetime of the returned reference. + unsafe { &mut *($from_mut_arg_name as *mut $inner as *mut Self) } + } + }; + ( + $inner:ty; + $(#[$($from_box_attr:tt)*])* + $from_box_vis:vis fn $from_box:ident($from_box_arg_name:ident: Box<_>) -> $fn_ret_ty:ty; + ) => { + $crate::_emit_alloc! { + $(#[$($from_box_attr)*])* + #[inline] + $from_box_vis fn $from_box($from_box_arg_name: $crate::_export::alloc::boxed::Box<$inner>) -> $crate::_export::alloc::boxed::Box { + let ptr = $crate::_export::alloc::boxed::Box::into_raw($from_box_arg_name); + // SAFETY: the pointer is created by casting a pointer that is pointing to an object + // with the same layout and validity invariants and the previous pointer was created + // directly from box. (Notice repr(transparent).) + unsafe { $crate::_export::alloc::boxed::Box::from_raw(ptr as *mut Self) } + } + } + }; + + ( + $inner:ty; + $(#[$($from_rc_attr:tt)*])* + $from_rc_vis:vis fn $from_rc:ident($from_rc_arg_name:ident: Rc<_>) -> $fn_ret_ty:ty; + ) => { + $crate::_emit_alloc! { + $(#[$($from_rc_attr)*])* + #[inline] + $from_rc_vis fn $from_rc($from_rc_arg_name: $crate::_export::alloc::rc::Rc<$inner>) -> $crate::_export::alloc::rc::Rc { + let ptr = $crate::_export::alloc::rc::Rc::into_raw($from_rc_arg_name); + // SAFETY: the pointer is created by casting a pointer that is pointing to an object + // with the same layout and validity invariants and the previous pointer was created + // directly from box. (Notice repr(transparent).) + unsafe { $crate::_export::alloc::rc::Rc::from_raw(ptr as *mut Self) } + } + } + }; + + ( + $inner:ty; + $(#[$($from_arc_attr:tt)*])* + $from_arc_vis:vis fn $from_arc:ident($from_arc_arg_name:ident: Arc<_>) -> $fn_ret_ty:ty; + ) => { + $crate::_emit_alloc! { + $(#[$($from_arc_attr)*])* + #[cfg(target_has_atomic = "ptr")] + #[inline] + $from_arc_vis fn $from_arc($from_arc_arg_name: $crate::_export::alloc::sync::Arc<$inner>) -> $crate::_export::alloc::sync::Arc { + let ptr = $crate::_export::alloc::sync::Arc::into_raw($from_arc_arg_name); + // SAFETY: the pointer is created by casting a pointer that is pointing to an object + // with the same layout and validity invariants and the previous pointer was created + // directly from box. (Notice repr(transparent).) + unsafe { $crate::_export::alloc::sync::Arc::from_raw(ptr as *mut Self) } + } + } + } +} + +#[doc(hidden)] +#[macro_export] +macro_rules! _check_tts_eq { + ($left:tt, $right:tt, $message:literal) => { + macro_rules! token_eq { + ($right) => {}; + ($any:tt) => { compile_error!($message) }; + } + token_eq!($left); + } +} + +#[doc(hidden)] +#[macro_export] +macro_rules! _transparent_newtype_inner_type { + ($(#[$($field_attr:tt)*])* $inner:ty) => { + $inner + }; + ($(#[$($phantom_attr:tt)*])* PhantomData<$phantom:ty>, $(#[$($field_attr:tt)*])* $inner:ty) => { + $inner + }; +} + +/// Emits given tokens only if the `alloc` feature **in this crate** is enabled. +/// +/// (The feature is currently enabled.) +#[cfg(feature = "alloc")] +#[doc(hidden)] +#[macro_export] +macro_rules! _emit_alloc { + ($($tokens:tt)*) => { $($tokens)* }; +} + +/// Emits given tokens only if the `alloc` feature **in this crate** is enabled. +/// +/// (The feature is currently disabled.) +#[cfg(not(feature = "alloc"))] +#[doc(hidden)] +#[macro_export] +macro_rules! _emit_alloc { + ($($tokens:tt)*) => {}; +} diff --git a/io/src/bridge.rs b/io/src/bridge.rs index fc49a07719..816820a2a5 100644 --- a/io/src/bridge.rs +++ b/io/src/bridge.rs @@ -1,34 +1,25 @@ // SPDX-License-Identifier: CC0-1.0 -#[cfg(feature = "alloc")] -use alloc::boxed::Box; - use internals::rust_version; -/// A bridging wrapper providing the I/O traits for types that already implement `std` I/O traits. -#[repr(transparent)] -#[derive(Debug)] -pub struct FromStd(T); +internals::transparent_newtype! { + /// A bridging wrapper providing the I/O traits for types that already implement `std` I/O traits. + #[derive(Debug)] + pub struct FromStd(T); -impl FromStd { - /// Wraps an I/O type. - #[inline] - pub const fn new(inner: T) -> Self { Self(inner) } + impl FromStd { + /// Wraps a mutable reference to I/O type. + pub fn new_mut(inner: &mut _) -> &mut Self; - /// Wraps a mutable reference to I/O type. - #[inline] - pub fn new_mut(inner: &mut T) -> &mut Self { - // SAFETY: the type is repr(transparent) and the lifetimes match - unsafe { &mut *(inner as *mut _ as *mut Self) } + /// Wraps a boxed I/O type. + pub fn new_boxed(inner: Box<_>) -> Box; } +} - /// Wraps a boxed I/O type. - #[cfg(feature = "alloc")] +impl FromStd { + /// Wraps an I/O type. #[inline] - pub fn new_boxed(inner: Box) -> Box { - // SAFETY: the type is repr(transparent) and the pointer is created from Box - unsafe { Box::from_raw(Box::into_raw(inner) as *mut Self) } - } + pub const fn new(inner: T) -> Self { Self(inner) } /// Returns the wrapped value. #[inline] @@ -117,30 +108,24 @@ impl std::io::Write for FromStd { fn write_all(&mut self, buf: &[u8]) -> std::io::Result<()> { self.0.write_all(buf) } } -/// A bridging wrapper providing the std traits for types that already implement our traits. -#[repr(transparent)] -#[derive(Debug)] -pub struct ToStd(T); +internals::transparent_newtype! { + /// A bridging wrapper providing the std traits for types that already implement our traits. + #[derive(Debug)] + pub struct ToStd(T); -impl ToStd { - /// Wraps an I/O type. - #[inline] - pub const fn new(inner: T) -> Self { Self(inner) } + impl ToStd { + /// Wraps a mutable reference to I/O type. + pub fn new_mut(inner: &mut _) -> &mut Self; - /// Wraps a mutable reference to I/O type. - #[inline] - pub fn new_mut(inner: &mut T) -> &mut Self { - // SAFETY: the type is repr(transparent) and the lifetimes match - unsafe { &mut *(inner as *mut _ as *mut Self) } + /// Wraps a boxed I/O type. + pub fn new_boxed(inner: Box<_>) -> Box; } +} - /// Wraps a boxed I/O type. - #[cfg(feature = "alloc")] +impl ToStd { + /// Wraps an I/O type. #[inline] - pub fn new_boxed(inner: Box) -> Box { - // SAFETY: the type is repr(transparent) and the pointer is created from Box - unsafe { Box::from_raw(Box::into_raw(inner) as *mut Self) } - } + pub const fn new(inner: T) -> Self { Self(inner) } /// Returns the wrapped value. #[inline] diff --git a/primitives/src/script/borrowed.rs b/primitives/src/script/borrowed.rs index ece38f6ee6..4f3bca28cd 100644 --- a/primitives/src/script/borrowed.rs +++ b/primitives/src/script/borrowed.rs @@ -10,54 +10,67 @@ use arbitrary::{Arbitrary, Unstructured}; use super::ScriptBuf; use crate::prelude::{Box, ToOwned, Vec}; -/// 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([u8]); +internals::transparent_newtype! { + /// 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)] + pub struct Script([u8]); + + impl Script { + /// Treat byte slice as `Script` + pub const fn from_bytes(bytes: &_) -> &Self; + + /// Treat mutable byte slice as `Script` + pub fn from_bytes_mut(bytes: &mut _) -> &mut Self; + + pub(crate) fn from_boxed_bytes(bytes: Box<_>) -> Box; + pub(crate) fn from_rc_bytes(bytes: Rc<_>) -> Rc; + pub(crate) fn from_arc_bytes(bytes: Arc<_>) -> Arc; + } +} impl Default for &Script { #[inline] @@ -76,28 +89,6 @@ impl Script { #[inline] pub const fn new() -> &'static Self { Self::from_bytes(&[]) } - /// Treat byte slice as `Script` - #[inline] - pub const fn from_bytes(bytes: &[u8]) -> &Self { - // 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 Self) } - } - - /// Treat mutable byte slice as `Script` - #[inline] - pub fn from_bytes_mut(bytes: &mut [u8]) -> &mut Self { - // 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 Self) } - } - /// Returns the script data as a byte slice. #[inline] pub const fn as_bytes(&self) -> &[u8] { &self.0 } diff --git a/primitives/src/script/mod.rs b/primitives/src/script/mod.rs index c263b4d688..2ad34017cc 100644 --- a/primitives/src/script/mod.rs +++ b/primitives/src/script/mod.rs @@ -277,24 +277,14 @@ impl<'a> From<&'a Script> for Cow<'a, Script> { impl<'a> From<&'a Script> for Arc