From 3e520f9094ec0ff93d4287bf4e6040449aa6c945 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Wed, 7 Dec 2022 00:50:50 +0100 Subject: [PATCH] Use hex from internals rather than hashes `bitcoin-internals` contains a more performant implementation of hex encoding than what `bitcoin_hashes` uses internally. This switches the implementations for formatting trait implementations as a step towards moving over completely. The public macros are also changed to delegate to inner type which is technically a breaking change but we will break the API anyway and the consuers should only call the macro on the actual hash newtypes where the inner types already have the appropriate implementations. Apart from removing reliance on internal hex from public API this reduces duplicated code generated and compiled. E.g. if you created 10 hash newtypes of SHA256 the formatting implementation would be instantiated 11 times despite being the same. To do all this some other changes were required to the hex infrastructure. Mainly modifying `put_bytes` to accept iterator (so that `iter().rev()` can be used) and adding a new `DisplayArray` type. The iterator idea was invented by Tobin C. Harding, this commit just adds a bound check and generalizes over `u8` and `&u8` returning iterators. While it may seem that `DisplayByteSlice` would suffice it'd create and initialize a large array even for small arrays wasting performance. Knowing the exact length `DisplayArray` fixes this. Another part of refactoring is changing from returning `impl Display` to return `impl LowerHex + UpperHex`. This makes selecting casing less annoying since the consumer no longer needs to import `Case` without cluttering the API with convenience methods. --- hashes/Cargo.toml | 2 + hashes/src/internal_macros.rs | 67 +++++++++++++++++++++- hashes/src/sha256.rs | 2 +- hashes/src/util.rs | 36 +++++++----- internals/src/hex/buf_encoder.rs | 46 +++++++++++++-- internals/src/hex/display.rs | 97 ++++++++++++++++++++++++-------- 6 files changed, 206 insertions(+), 44 deletions(-) diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 419d2c94..a4b6879a 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -26,6 +26,8 @@ serde-std = ["serde/std"] serde = { version = "1.0", default-features = false, optional = true } # Only enable this if you explicitly do not want to use an allocator, otherwise enable "alloc". core2 = { version = "0.3.0", optional = true, default_features = false } +# TODO: change to proper version before release +internals = { path = "../internals", package = "bitcoin-internals" } # Do NOT use this as a feature! Use the `schemars` feature instead. Can only be used with "std" enabled. actual-schemars = { package = "schemars", version = "<=0.8.3", optional = true } diff --git a/hashes/src/internal_macros.rs b/hashes/src/internal_macros.rs index 897917a1..48ebd608 100644 --- a/hashes/src/internal_macros.rs +++ b/hashes/src/internal_macros.rs @@ -1,5 +1,52 @@ //! Non-public macros +macro_rules! arr_newtype_fmt_impl { + ($ty:ident, $bytes:expr $(, $gen:ident: $gent:ident)*) => { + impl<$($gen: $gent),*> $crate::_export::_core::fmt::LowerHex for $ty<$($gen),*> { + #[inline] + fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { + #[allow(unused)] + use crate::Hash as _; + let case = internals::hex::Case::Lower; + if <$ty<$($gen),*>>::DISPLAY_BACKWARD { + internals::hex::display::fmt_hex_exact!(f, $bytes, self.0.iter().rev(), case) + } else { + internals::hex::display::fmt_hex_exact!(f, $bytes, self.0.iter(), case) + } + } + } + + impl<$($gen: $gent),*> $crate::_export::_core::fmt::UpperHex for $ty<$($gen),*> { + #[inline] + fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { + #[allow(unused)] + use crate::Hash as _; + let case = internals::hex::Case::Upper; + if <$ty<$($gen),*>>::DISPLAY_BACKWARD { + internals::hex::display::fmt_hex_exact!(f, $bytes, self.0.iter().rev(), case) + } else { + internals::hex::display::fmt_hex_exact!(f, $bytes, self.0.iter(), case) + } + } + } + + impl<$($gen: $gent),*> $crate::_export::_core::fmt::Display for $ty<$($gen),*> { + #[inline] + fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { + $crate::_export::_core::fmt::LowerHex::fmt(self, f) + } + } + + impl<$($gen: $gent),*> $crate::_export::_core::fmt::Debug for $ty<$($gen),*> { + #[inline] + fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { + write!(f, "{:#}", self) + } + } + } +} +pub(crate) use arr_newtype_fmt_impl; + /// Adds trait impls to the type called `Hash` in the current scope. /// /// Implpements various conversion traits as well as the [`crate::Hash`] trait. @@ -21,6 +68,24 @@ /// `internal_engine` is required to initialize the engine for given hash type. macro_rules! hash_trait_impls { ($bits:expr, $reversed:expr $(, $gen:ident: $gent:ident)*) => { + impl<$($gen: $gent),*> Hash<$($gen),*> { + /// Displays hex forwards, regardless of how this type would display it naturally. + /// + /// This is mainly intended as an internal method and you shouldn't need it unless + /// you're doing something special. + pub fn forward_hex(&self) -> impl '_ + core::fmt::LowerHex + core::fmt::UpperHex { + internals::hex::display::DisplayHex::as_hex(&self.0) + } + + /// Displays hex backwards, regardless of how this type would display it naturally. + /// + /// This is mainly intended as an internal method and you shouldn't need it unless + /// you're doing something special. + pub fn backward_hex(&self) -> impl '_ + core::fmt::LowerHex + core::fmt::UpperHex { + internals::hex::display::DisplayArray::<_, [u8; $bits / 8 * 2]>::new(self.0.iter().rev()) + } + } + impl<$($gen: $gent),*> str::FromStr for Hash<$($gen),*> { type Err = hex::Error; fn from_str(s: &str) -> Result { @@ -28,7 +93,7 @@ macro_rules! hash_trait_impls { } } - hex_fmt_impl!(Hash $(, $gen: $gent)*); + $crate::internal_macros::arr_newtype_fmt_impl!(Hash, $bits / 8 $(, $gen: $gent)*); serde_impl!(Hash, $bits / 8 $(, $gen: $gent)*); borrow_slice_impl!(Hash $(, $gen: $gent)*); diff --git a/hashes/src/sha256.rs b/hashes/src/sha256.rs index b6336fa6..31479b50 100644 --- a/hashes/src/sha256.rs +++ b/hashes/src/sha256.rs @@ -119,7 +119,7 @@ impl Hash { #[derive(Copy, Clone, PartialEq, Eq, Default, PartialOrd, Ord, Hash)] pub struct Midstate(pub [u8; 32]); -hex_fmt_impl!(Midstate); +crate::internal_macros::arr_newtype_fmt_impl!(Midstate, 32); serde_impl!(Midstate, 32); borrow_slice_impl!(Midstate); diff --git a/hashes/src/util.rs b/hashes/src/util.rs index 95353474..1a083279 100644 --- a/hashes/src/util.rs +++ b/hashes/src/util.rs @@ -15,33 +15,41 @@ #[macro_export] /// Adds hexadecimal formatting implementation of a trait `$imp` to a given type `$ty`. macro_rules! hex_fmt_impl( - ($ty:ident) => ( - $crate::hex_fmt_impl!($ty, ); + ($reverse:expr, $ty:ident) => ( + $crate::hex_fmt_impl!($reverse, $ty, ); ); - ($ty:ident, $($gen:ident: $gent:ident),*) => ( + ($reverse:expr, $ty:ident, $($gen:ident: $gent:ident),*) => ( impl<$($gen: $gent),*> $crate::_export::_core::fmt::LowerHex for $ty<$($gen),*> { + #[inline] fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { - #[allow(unused_imports)] - use $crate::{Hash as _, HashEngine as _, hex}; - - if f.alternate() { - write!(f, "0x")?; - } - if $ty::<$($gen),*>::DISPLAY_BACKWARD { - hex::format_hex_reverse(self.as_ref(), f) + if $reverse { + $crate::_export::_core::fmt::LowerHex::fmt(&self.0.backward_hex(), f) } else { - hex::format_hex(self.as_ref(), f) + $crate::_export::_core::fmt::LowerHex::fmt(&self.0.forward_hex(), f) + } + } + } + + impl<$($gen: $gent),*> $crate::_export::_core::fmt::UpperHex for $ty<$($gen),*> { + #[inline] + fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { + if $reverse { + $crate::_export::_core::fmt::UpperHex::fmt(&self.0.backward_hex(), f) + } else { + $crate::_export::_core::fmt::UpperHex::fmt(&self.0.forward_hex(), f) } } } impl<$($gen: $gent),*> $crate::_export::_core::fmt::Display for $ty<$($gen),*> { + #[inline] fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { - $crate::_export::_core::fmt::LowerHex::fmt(self, f) + $crate::_export::_core::fmt::LowerHex::fmt(&self, f) } } impl<$($gen: $gent),*> $crate::_export::_core::fmt::Debug for $ty<$($gen),*> { + #[inline] fn fmt(&self, f: &mut $crate::_export::_core::fmt::Formatter) -> $crate::_export::_core::fmt::Result { write!(f, "{:#}", self) } @@ -113,7 +121,7 @@ macro_rules! hash_newtype { #[repr(transparent)] pub struct $newtype($hash); - $crate::hex_fmt_impl!($newtype); + $crate::hex_fmt_impl!($reverse, $newtype); $crate::serde_impl!($newtype, $len); $crate::borrow_slice_impl!($newtype); diff --git a/internals/src/hex/buf_encoder.rs b/internals/src/hex/buf_encoder.rs index 5997b50b..8130af64 100644 --- a/internals/src/hex/buf_encoder.rs +++ b/internals/src/hex/buf_encoder.rs @@ -6,6 +6,7 @@ pub use out_bytes::OutBytes; +use core::borrow::Borrow; use super::Case; /// Trait for types that can be soundly converted to `OutBytes`. @@ -28,6 +29,17 @@ pub trait AsOutBytes: out_bytes::Sealed { fn as_mut_out_bytes(&mut self) -> &mut OutBytes; } +/// A buffer with compile-time-known length. +/// +/// This is essentially `Default + AsOutBytes` but supports lengths 1.41 doesn't. +pub trait FixedLenBuf: Sized + AsOutBytes { + /// Creates an uninitialized buffer. + /// + /// The current implementtions initialize the buffer with zeroes but it should be treated a + /// uninitialized anyway. + fn uninit() -> Self; +} + /// Implements `OutBytes` /// /// This prevents the rest of the crate from accessing the field of `OutBytes`. @@ -97,6 +109,12 @@ mod out_bytes { macro_rules! impl_from_array { ($($len:expr),* $(,)?) => { $( + impl super::FixedLenBuf for [u8; $len] { + fn uninit() -> Self { + [0u8; $len] + } + } + impl AsOutBytes for [u8; $len] { fn as_out_bytes(&self) -> &OutBytes { OutBytes::from_bytes(self) @@ -108,6 +126,17 @@ mod out_bytes { } impl Sealed for [u8; $len] {} + + impl<'a> super::super::display::DisplayHex for &'a [u8; $len / 2] { + type Display = super::super::display::DisplayArray, [u8; $len]>; + fn as_hex(self) -> Self::Display { + super::super::display::DisplayArray::new(self.iter()) + } + + fn hex_reserve_suggestion(self) -> usize { + $len + } + } )* } } @@ -131,7 +160,7 @@ mod out_bytes { // As a sanity check we only provide conversions for even, non-empty arrays. // Weird lengths 66 and 130 are provided for serialized public keys. impl_from_array!( - 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32, 64, 66, 128, 130, 256, 512, + 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32, 40, 64, 66, 128, 130, 256, 512, 1024, 2048, 4096, 8192 ); @@ -176,10 +205,19 @@ impl BufEncoder { /// The method panics if the bytes wouldn't fit the buffer. #[inline] #[cfg_attr(rust_v_1_46, track_caller)] - pub fn put_bytes(&mut self, bytes: &[u8], case: Case) { - assert!(bytes.len() <= self.space_remaining()); + pub fn put_bytes(&mut self, bytes: I, case: Case) where I: IntoIterator, I::Item: Borrow { + self.put_bytes_inner(bytes.into_iter(), case) + } + + #[inline] + #[cfg_attr(rust_v_1_46, track_caller)] + fn put_bytes_inner(&mut self, bytes: I, case: Case) where I: Iterator, I::Item: Borrow { + // May give the compiler better optimization opportunity + if let Some(max) = bytes.size_hint().1 { + assert!(max <= self.space_remaining()); + } for byte in bytes { - self.put_byte(*byte, case); + self.put_byte(*byte.borrow(), case); } } diff --git a/internals/src/hex/display.rs b/internals/src/hex/display.rs index a2dbabf1..1cdb980c 100644 --- a/internals/src/hex/display.rs +++ b/internals/src/hex/display.rs @@ -3,12 +3,14 @@ //! This module provides a trait for displaying things as hex as well as an implementation for //! `&[u8]`. +use core::borrow::Borrow; use core::fmt; use super::buf_encoder::{BufEncoder, OutBytes}; use super::Case; #[cfg(feature = "alloc")] use crate::prelude::*; +use crate::hex::buf_encoder::FixedLenBuf; /// Extension trait for types that can be displayed as hex. /// @@ -22,20 +24,10 @@ pub trait DisplayHex: Copy + sealed::IsRef { /// The type providing [`fmt::Display`] implementation. /// /// This is usually a wrapper type holding a reference to `Self`. - type Display: fmt::Display; + type Display: fmt::LowerHex + fmt::UpperHex; /// Display `Self` as a continuous sequence of ASCII hex chars. - fn display_hex(self, case: Case) -> Self::Display; - - /// Shorthand for `display_hex(Case::Lower)`. - /// - /// Avoids the requirement to import the `Case` type. - fn display_lower_hex(self) -> Self::Display { self.display_hex(Case::Lower) } - - /// Shorthand for `display_hex(Case::Upper)`. - /// - /// Avoids the requirement to import the `Case` type. - fn display_upper_hex(self) -> Self::Display { self.display_hex(Case::Upper) } + fn as_hex(self) -> Self::Display; /// Create a lower-hex-encoded string. /// @@ -68,7 +60,7 @@ pub trait DisplayHex: Copy + sealed::IsRef { /// Appends hex-encoded content to an existing `String`. /// - /// This may be faster than `write!(string, "{}", self.display_hex())` because it uses + /// This may be faster than `write!(string, "{:x}", self.display_hex())` because it uses /// `reserve_sugggestion`. #[cfg(feature = "alloc")] #[cfg_attr(docsrs, doc(cfg(feature = "alloc")))] @@ -76,7 +68,11 @@ pub trait DisplayHex: Copy + sealed::IsRef { use fmt::Write; string.reserve(self.hex_reserve_suggestion()); - write!(string, "{}", self.display_hex(case)).unwrap_or_else(|_| { + match case { + Case::Lower => write!(string, "{:x}", self.as_hex()), + Case::Upper => write!(string, "{:X}", self.as_hex()), + } + .unwrap_or_else(|_| { let name = core::any::type_name::(); // We don't expect `std` to ever be buggy, so the bug is most likely in the `Display` // impl of `Self::Display`. @@ -104,7 +100,7 @@ impl<'a> DisplayHex for &'a [u8] { type Display = DisplayByteSlice<'a>; #[inline] - fn display_hex(self, case: Case) -> Self::Display { DisplayByteSlice { bytes: self, case } } + fn as_hex(self) -> Self::Display { DisplayByteSlice { bytes: self } } #[inline] fn hex_reserve_suggestion(self) -> usize { @@ -119,26 +115,76 @@ impl<'a> DisplayHex for &'a [u8] { /// /// Created by [`<&[u8] as DisplayHex>::display_hex`](DisplayHex::display_hex). pub struct DisplayByteSlice<'a> { - bytes: &'a [u8], - case: Case, + // pub because we want to keep lengths in sync + pub(crate) bytes: &'a [u8], } -impl<'a> fmt::Display for DisplayByteSlice<'a> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl<'a> DisplayByteSlice<'a> { + fn display(&self, f: &mut fmt::Formatter, case: Case) -> fmt::Result { let mut buf = [0u8; 1024]; let mut encoder = super::BufEncoder::new(&mut buf); let mut chunks = self.bytes.chunks_exact(512); for chunk in &mut chunks { - encoder.put_bytes(chunk, self.case); + encoder.put_bytes(chunk, case); f.write_str(encoder.as_str())?; encoder.clear(); } - encoder.put_bytes(chunks.remainder(), self.case); + encoder.put_bytes(chunks.remainder(), case); f.write_str(encoder.as_str()) } } +impl<'a> fmt::LowerHex for DisplayByteSlice<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.display(f, Case::Lower) + } +} + +impl<'a> fmt::UpperHex for DisplayByteSlice<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.display(f, Case::Upper) + } +} + +/// Displays byte array as hex. +/// +/// Created by [`<&[u8; LEN] as DisplayHex>::display_hex`](DisplayHex::display_hex). +pub struct DisplayArray where A::Item: Borrow { + array: A, + _buffer_marker: core::marker::PhantomData, +} + + +impl DisplayArray where A::Item: Borrow { + /// Creates the wrapper. + pub fn new(array: A) -> Self { + DisplayArray { + array, + _buffer_marker: Default::default(), + } + } + + fn display(&self, f: &mut fmt::Formatter, case: Case) -> fmt::Result { + let mut buf = B::uninit(); + let mut encoder = super::BufEncoder::new(&mut buf); + encoder.put_bytes(self.array.clone(), case); + f.pad_integral(true, "0x", encoder.as_str()) + } +} + +impl fmt::LowerHex for DisplayArray where A::Item: Borrow { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.display(f, Case::Lower) + } +} + +impl fmt::UpperHex for DisplayArray where A::Item: Borrow { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.display(f, Case::Upper) + } +} + /// Format known-length array as hex. /// /// This supports all formatting options of formatter and may be faster than calling @@ -169,16 +215,19 @@ macro_rules! fmt_hex_exact { $crate::hex::display::fmt_hex_exact_fn($formatter, buf, $bytes, $case) }}; } +pub use fmt_hex_exact; // Implementation detail of `write_hex_exact` macro to de-duplicate the code #[doc(hidden)] #[inline] -pub fn fmt_hex_exact_fn( +pub fn fmt_hex_exact_fn( f: &mut fmt::Formatter, buf: &mut OutBytes, - bytes: &[u8], + bytes: I, case: Case, -) -> fmt::Result { +) -> fmt::Result + where I: IntoIterator, I::Item: Borrow +{ let mut encoder = BufEncoder::new(buf); encoder.put_bytes(bytes, case); f.pad_integral(true, "0x", encoder.as_str())