-
Notifications
You must be signed in to change notification settings - Fork 22
Open
Copy link
Labels
refactorImprovements to code structure, readability, or performance that don't add new features or fix bugs.Improvements to code structure, readability, or performance that don't add new features or fix bugs.
Description
Background
We currently have two hash_newtype!-style macros with the same purpose but different derive sets:
mpc-primitives(crates/primitives/src/hash.rs) — generates hash newtypes withborsh,schemars, andserdesupport (for on-chain / contract types):
mpc/crates/primitives/src/hash.rs
Lines 14 to 130 in 500bcfb
/// Generates a hash newtype wrapping `[u8; $n]` with hex serde, borsh, `FromStr`, /// `Deref`, `AsRef`, `Into`, and (when the `abi` feature is active) BorshSchema / JsonSchema. macro_rules! hash_newtype { ($(#[$meta:meta])* $name:ident, $n:literal) => { #[serde_with::serde_as] #[derive( Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, serde::Serialize, serde::Deserialize, borsh::BorshSerialize, borsh::BorshDeserialize, derive_more::Deref, derive_more::AsRef, derive_more::Into, )] $(#[$meta])* #[serde(transparent)] pub struct $name { #[deref] #[as_ref] #[into] #[serde_as(as = "serde_with::hex::Hex")] bytes: [u8; $n], } impl From<[u8; $n]> for $name { fn from(bytes: [u8; $n]) -> Self { Self::new(bytes) } } impl $name { /// Converts the hash to a hexadecimal string representation. pub fn as_hex(&self) -> String { hex::encode(self.as_ref()) } pub fn as_bytes(&self) -> [u8; $n] { self.bytes } pub const fn new(bytes: [u8; $n]) -> Self { Self { bytes } } } impl FromStr for $name { type Err = HashParseError; fn from_str(s: &str) -> Result<Self, Self::Err> { let decoded_hex_bytes = hex::decode(s)?; let hash_bytes: [u8; $n] = decoded_hex_bytes .try_into() .map_err(|v: Vec<u8>| HashParseError::InvalidLength { expected: $n, got: v.len(), })?; Ok(hash_bytes.into()) } } #[cfg(all(feature = "abi", not(target_arch = "wasm32")))] impl borsh::BorshSchema for $name { fn declaration() -> borsh::schema::Declaration { alloc::format!(stringify!($name)) } fn add_definitions_recursively( definitions: &mut alloc::collections::BTreeMap< borsh::schema::Declaration, borsh::schema::Definition, >, ) { let byte_array_decl = alloc::format!("[u8; {}]", $n); definitions.insert( Self::declaration(), borsh::schema::Definition::Struct { fields: borsh::schema::Fields::NamedFields(alloc::vec![ ("bytes".into(), byte_array_decl), ]), }, ); } } #[cfg(all(feature = "abi", not(target_arch = "wasm32")))] impl schemars::JsonSchema for $name { fn schema_name() -> String { alloc::format!(stringify!($name)) } fn json_schema( _generator: &mut schemars::r#gen::SchemaGenerator, ) -> schemars::schema::Schema { let hex_len = ($n * 2) as u32; schemars::schema::Schema::Object(schemars::schema::SchemaObject { instance_type: Some(schemars::schema::SingleOrVec::Single(Box::new( schemars::schema::InstanceType::String, ))), string: Some(Box::new(schemars::schema::StringValidation { min_length: Some(hex_len), max_length: Some(hex_len), pattern: Some("^[0-9a-fA-F]+$".to_string()), })), ..Default::default() }) } } }; } foreign-chain-inspector(crates/foreign-chain-inspector/src/hash.rs) — lightweightserde-only hash newtypes for RPC / off-chain types:
mpc/crates/foreign-chain-inspector/src/hash.rs
Lines 15 to 72 in 500bcfb
/// Generates a 32-byte hash newtype with hex JSON serialization and `FromStr`. /// Unlike the primitives crate's `hash_newtype!`, this does not include borsh or schema support. macro_rules! hash_newtype { ($(#[$meta:meta])* $name:ident) => { #[derive( Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, derive_more::Deref, derive_more::AsRef, derive_more::Into, )] $(#[$meta])* pub struct $name { #[deref] #[as_ref] #[into] bytes: [u8; 32], } impl serde::Serialize for $name { fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> { serializer.serialize_str(&hex::encode(&self.bytes)) } } impl<'de> serde::Deserialize<'de> for $name { fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> { let s = <String as serde::Deserialize>::deserialize(deserializer)?; crate::hash::parse_hex_hash::<32, Self>(&s).map_err(serde::de::Error::custom) } } impl From<[u8; 32]> for $name { fn from(bytes: [u8; 32]) -> Self { Self { bytes } } } impl core::str::FromStr for $name { type Err = mpc_primitives::hash::HashParseError; fn from_str(s: &str) -> Result<Self, Self::Err> { crate::hash::parse_hex_hash::<32, Self>(s) } } impl $name { pub fn as_hex(&self) -> String { hex::encode(self.as_ref()) } } }; }
Having two separate macros with identical names is confusing and duplicates logic.
User Story
As a developer working on this codebase, I want a single source of truth for hash newtype generation so that I don't have to maintain two nearly-identical macros and remember which one to use.
Acceptance Criteria
- A single
hash_newtype!(or equivalent genericHash<T, N>) macro/type exists, usable from both on-chain (mpc-contract) and off-chain (foreign-chain-inspector) crates. - Crates that don't need
borshare not forced to depend on it (e.g., via feature flags or by keeping borsh derives behind a feature gate). - The duplicate macro in
foreign-chain-inspectoris removed. - All existing tests pass.
Resources & Additional Notes
- Discussion originated in PR refactor: generalize
Hash32<T>toHash<T, N>and replaceSha384Digest#2564: comment and follow-up. - PR refactor: exported macro tradeoff #2596 was a spike showing what
#[macro_export]unification would look like — it requires a_macro_depsre-export module ($crate::_macro_deps::*) so the macro can referenceborsh,serde,hex, etc. from the caller's context. This added non-trivial boilerplate. - A feature-flag approach (e.g.,
features = ["borsh"]onmpc-primitives) may be cleaner than the re-export trick, but needs investigation.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
refactorImprovements to code structure, readability, or performance that don't add new features or fix bugs.Improvements to code structure, readability, or performance that don't add new features or fix bugs.