From 2b83eb9cd3e8d218dcc6d460d43ba1602b7feb53 Mon Sep 17 00:00:00 2001 From: /alex/ Date: Tue, 17 Oct 2023 18:21:02 +0200 Subject: [PATCH] refactor(types): `Hrp` wraps `bech32::Hrp` (#1413) * Wrap bech32::Hrp * finish impl * nit * Fix no_std compilation * Use Result * fix bech32 test * wrap bech32 error * improve unpack + test * impl From * Add TODO --------- Co-authored-by: Thibault Martinez --- Cargo.lock | 6 +- sdk/Cargo.toml | 4 +- sdk/src/types/block/address/bech32.rs | 93 ++++++++------------------- sdk/src/types/block/error.rs | 11 +++- sdk/tests/types/address/bech32.rs | 12 ++-- 5 files changed, 49 insertions(+), 77 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a92603b205..c099d23da8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -242,9 +242,9 @@ checksum = "2dabbe35f96fb9507f7330793dc490461b2962659ac5d427181e451a623751d1" [[package]] name = "bech32" -version = "0.9.1" +version = "0.10.0-beta" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d86b93f97252c47b41663388e6d155714a9d0c398b99f1005cbc5f978b29f445" +checksum = "98f7eed2b2781a6f0b5c903471d48e15f56fb4e1165df8a9a2337fd1a59d45ea" [[package]] name = "bincode" @@ -1592,7 +1592,7 @@ version = "1.0.3" dependencies = [ "anymap", "async-trait", - "bech32 0.9.1", + "bech32 0.10.0-beta", "bitflags 2.4.0", "bs58", "derive_more", diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index eda2106ca9..7261ec6f52 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -18,7 +18,9 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] # Mandatory dependencies -bech32 = { version = "0.9.1", default-features = false } +bech32 = { version = "0.10.0-beta", default-features = false, features = [ + "alloc", +] } bitflags = { version = "2.4.0", default-features = false } derive_more = { version = "0.99.17", default-features = false, features = [ "from", diff --git a/sdk/src/types/block/address/bech32.rs b/sdk/src/types/block/address/bech32.rs index 2c5b55e141..fbb01066f2 100644 --- a/sdk/src/types/block/address/bech32.rs +++ b/sdk/src/types/block/address/bech32.rs @@ -7,7 +7,6 @@ use alloc::{ }; use core::str::FromStr; -use bech32::{FromBase32, ToBase32, Variant}; use derive_more::{AsRef, Deref}; use packable::{ error::{UnpackError, UnpackErrorExt}, @@ -18,20 +17,16 @@ use packable::{ use crate::types::block::{address::Address, ConvertTo, Error}; -const HRP_MAX: u8 = 83; - -#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] -pub struct Hrp { - inner: [u8; HRP_MAX as usize], - len: u8, -} +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Deref)] +#[repr(transparent)] +pub struct Hrp(bech32::Hrp); impl core::fmt::Debug for Hrp { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("Hrp") - .field("display", &self.to_string()) - .field("inner", &prefix_hex::encode(&self.inner[..self.len as usize])) - .field("len", &self.len) + .field("display", &self.0.to_string()) + .field("bytes", &prefix_hex::encode(self.0.byte_iter().collect::>())) + .field("len", &self.0.len()) .finish() } } @@ -39,18 +34,7 @@ impl core::fmt::Debug for Hrp { impl Hrp { /// Convert a string to an Hrp without checking validity. pub const fn from_str_unchecked(hrp: &str) -> Self { - let len = hrp.len(); - let mut bytes = [0; HRP_MAX as usize]; - let hrp = hrp.as_bytes(); - let mut i = 0; - while i < len { - bytes[i] = hrp[i]; - i += 1; - } - Self { - inner: bytes, - len: len as _, - } + Self(bech32::Hrp::parse_unchecked(hrp)) } } @@ -58,27 +42,13 @@ impl FromStr for Hrp { type Err = Error; fn from_str(hrp: &str) -> Result { - let len = hrp.len(); - if hrp.is_ascii() && len <= HRP_MAX as usize { - let mut bytes = [0; HRP_MAX as usize]; - bytes[..len].copy_from_slice(hrp.as_bytes()); - Ok(Self { - inner: bytes, - len: len as _, - }) - } else { - Err(Error::InvalidBech32Hrp(hrp.to_string())) - } + Ok(Self(bech32::Hrp::parse(hrp)?)) } } impl core::fmt::Display for Hrp { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - let hrp_str = self.inner[..self.len as usize] - .iter() - .map(|b| *b as char) - .collect::(); - f.write_str(&hrp_str) + self.0.fmt(f) } } @@ -88,8 +58,9 @@ impl Packable for Hrp { #[inline] fn pack(&self, packer: &mut P) -> Result<(), P::Error> { - self.len.pack(packer)?; - packer.pack_bytes(&self.inner[..self.len as usize])?; + (self.0.len() as u8).pack(packer)?; + // TODO revisit when/if bech32 adds a way to get the bytes without iteration to avoid collecting + packer.pack_bytes(&self.0.byte_iter().collect::>())?; Ok(()) } @@ -99,21 +70,15 @@ impl Packable for Hrp { unpacker: &mut U, visitor: &Self::UnpackVisitor, ) -> Result> { - let len = u8::unpack::<_, VERIFY>(unpacker, visitor).coerce()?; - - if len > HRP_MAX { - return Err(UnpackError::Packable(Error::InvalidBech32Hrp( - "hrp len above 83".to_string(), - ))); - } + let len = u8::unpack::<_, VERIFY>(unpacker, visitor).coerce()? as usize; - let mut bytes = alloc::vec![0u8; len as usize]; + let mut bytes = alloc::vec![0u8; len]; unpacker.unpack_bytes(&mut bytes)?; - let mut inner = [0; 83]; - inner[..len as usize].copy_from_slice(&bytes); - - Ok(Self { inner, len }) + Ok(Self( + bech32::Hrp::parse(&String::from_utf8_lossy(&bytes)) + .map_err(|e| UnpackError::Packable(Error::InvalidBech32Hrp(e)))?, + )) } } @@ -161,14 +126,13 @@ impl FromStr for Bech32Address { type Err = Error; fn from_str(address: &str) -> Result { - match ::bech32::decode(address) { - Ok((hrp, data, _)) => { - let hrp = hrp.parse()?; - let bytes = Vec::::from_base32(&data).map_err(|_| Error::InvalidAddress)?; - Address::unpack_verified(bytes.as_slice(), &()) - .map_err(|_| Error::InvalidAddress) - .map(|address| Self { hrp, inner: address }) - } + match bech32::decode(address) { + Ok((hrp, bytes)) => Address::unpack_verified(bytes.as_slice(), &()) + .map_err(|_| Error::InvalidAddress) + .map(|address| Self { + hrp: Hrp(hrp), + inner: address, + }), Err(_) => Err(Error::InvalidAddress), } } @@ -217,12 +181,7 @@ impl core::fmt::Display for Bech32Address { write!( f, "{}", - ::bech32::encode( - &self.hrp.to_string(), - self.inner.pack_to_vec().to_base32(), - Variant::Bech32 - ) - .unwrap() + bech32::encode::(self.hrp.0, &self.inner.pack_to_vec(),).unwrap() ) } } diff --git a/sdk/src/types/block/error.rs b/sdk/src/types/block/error.rs index a960abf307..c79904ebef 100644 --- a/sdk/src/types/block/error.rs +++ b/sdk/src/types/block/error.rs @@ -4,6 +4,7 @@ use alloc::string::{FromUtf8Error, String}; use core::{convert::Infallible, fmt}; +use bech32::primitives::hrp::Error as Bech32HrpError; use crypto::Error as CryptoError; use prefix_hex::Error as HexError; use primitive_types::U256; @@ -79,7 +80,7 @@ pub enum Error { InvalidInputKind(u8), InvalidInputCount(>::Error), InvalidInputOutputIndex(>::Error), - InvalidBech32Hrp(String), + InvalidBech32Hrp(Bech32HrpError), InvalidAddressCapabilitiesCount(>::Error), InvalidBlockWrapperLength(usize), InvalidStateMetadataLength(>::Error), @@ -216,7 +217,7 @@ impl fmt::Display for Error { Self::InvalidAddress => write!(f, "invalid address provided"), Self::InvalidAddressKind(k) => write!(f, "invalid address kind: {k}"), Self::InvalidAccountIndex(index) => write!(f, "invalid account index: {index}"), - Self::InvalidBech32Hrp(err) => write!(f, "invalid bech32 hrp: {err}"), + Self::InvalidBech32Hrp(e) => write!(f, "invalid bech32 hrp: {e}"), Self::InvalidAddressCapabilitiesCount(e) => write!(f, "invalid capabilities count: {e}"), Self::InvalidBlockKind(k) => write!(f, "invalid block kind: {k}"), Self::InvalidRewardInputIndex(idx) => write!(f, "invalid reward input index: {idx}"), @@ -386,6 +387,12 @@ impl fmt::Display for Error { } } +impl From for Error { + fn from(error: Bech32HrpError) -> Self { + Self::InvalidBech32Hrp(error) + } +} + impl From for Error { fn from(error: CryptoError) -> Self { Self::Crypto(error) diff --git a/sdk/tests/types/address/bech32.rs b/sdk/tests/types/address/bech32.rs index 96e1e109b6..bb3910cf98 100644 --- a/sdk/tests/types/address/bech32.rs +++ b/sdk/tests/types/address/bech32.rs @@ -39,10 +39,7 @@ fn ctors() { fn hrp_from_str() { Hrp::from_str("rms").unwrap(); - assert!(matches!( - Hrp::from_str("中國"), - Err(Error::InvalidBech32Hrp(hrp)) if hrp == "中國" - )); + assert!(matches!(Hrp::from_str("中國"), Err(Error::InvalidBech32Hrp(_)))); } #[test] @@ -61,6 +58,13 @@ fn hrp_pack_unpack() { assert_eq!(hrp, Hrp::unpack_verified(packed_hrp.as_slice(), &()).unwrap()); } +#[test] +fn invalid_hrp_unpack() { + let packed_hrp = vec![32, 32, 32]; // invalid HRP: " " + + assert!(Hrp::unpack_verified(packed_hrp.as_slice(), &()).is_err()); +} + #[test] fn bech32_into_inner() { let address = Address::try_from_bech32(ED25519_BECH32).unwrap();