From 3861199706b811e891191dd74b7c9600fd7bcbeb Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 10 Sep 2024 03:19:03 +0200 Subject: [PATCH] fix(sol-macro): correctly determine whether event parameters are hashes (#735) --- crates/sol-macro-expander/src/expand/event.rs | 6 +- crates/sol-macro-expander/src/expand/mod.rs | 18 +++++- crates/sol-types/src/types/event/mod.rs | 1 + crates/sol-types/tests/macros/sol/mod.rs | 64 +++++++++++++++++++ crates/syn-solidity/src/item/event.rs | 18 ++---- crates/syn-solidity/src/type/mod.rs | 46 ++++++++----- 6 files changed, 122 insertions(+), 31 deletions(-) diff --git a/crates/sol-macro-expander/src/expand/event.rs b/crates/sol-macro-expander/src/expand/event.rs index a07df2a85..de66728a0 100644 --- a/crates/sol-macro-expander/src/expand/event.rs +++ b/crates/sol-macro-expander/src/expand/event.rs @@ -81,7 +81,7 @@ pub(super) fn expand(cx: &ExpCtxt<'_>, event: &ItemEvent) -> Result let name = anon_name((i, p.name.as_ref())); let ty = expand_type(&p.ty, &cx.crates); - if p.indexed_as_hash() { + if cx.indexed_as_hash(p) { quote! { as alloy_sol_types::EventTopic>::encode_topic(&self.#name) } @@ -225,7 +225,7 @@ pub(super) fn expand(cx: &ExpCtxt<'_>, event: &ItemEvent) -> Result fn expand_event_topic_type(param: &EventParameter, cx: &ExpCtxt<'_>) -> TokenStream { let alloy_sol_types = &cx.crates.sol_types; assert!(param.is_indexed()); - if param.is_abi_dynamic() { + if cx.indexed_as_hash(param) { quote_spanned! {param.ty.span()=> #alloy_sol_types::sol_data::FixedBytes<32> } } else { expand_type(¶m.ty, &cx.crates) @@ -239,7 +239,7 @@ fn expand_event_topic_field( cx: &ExpCtxt<'_>, ) -> TokenStream { let name = anon_name((i, name)); - let ty = if param.indexed_as_hash() { + let ty = if cx.indexed_as_hash(param) { let bytes32 = ast::Type::FixedBytes(name.span(), core::num::NonZeroU16::new(32).unwrap()); ty::expand_rust_type(&bytes32, &cx.crates) } else { diff --git a/crates/sol-macro-expander/src/expand/mod.rs b/crates/sol-macro-expander/src/expand/mod.rs index 85ec5d0e4..f4a85c125 100644 --- a/crates/sol-macro-expander/src/expand/mod.rs +++ b/crates/sol-macro-expander/src/expand/mod.rs @@ -545,7 +545,23 @@ impl<'ast> ExpCtxt<'ast> { } fn try_custom_type(&self, name: &SolPath) -> Option<&Type> { - self.custom_types.resolve(name, &self.current_namespace) + self.custom_types.resolve(name, &self.current_namespace).inspect(|&ty| { + if ty.is_custom() { + abort!( + ty.span(), + "unresolved custom type in map"; + note = name.span() => "name span"; + ); + } + }) + } + + fn indexed_as_hash(&self, param: &EventParameter) -> bool { + param.indexed_as_hash(self.custom_is_value_type()) + } + + fn custom_is_value_type(&self) -> impl Fn(&SolPath) -> bool + '_ { + move |ty| self.custom_type(ty).is_value_type(self.custom_is_value_type()) } /// Returns the name of the function, adjusted for overloads. diff --git a/crates/sol-types/src/types/event/mod.rs b/crates/sol-types/src/types/event/mod.rs index 127665491..8bf5edc3f 100644 --- a/crates/sol-types/src/types/event/mod.rs +++ b/crates/sol-types/src/types/event/mod.rs @@ -47,6 +47,7 @@ pub trait SolEvent: Sized { /// /// For non-anonymous events, this will be the first topic (`topic0`). /// For anonymous events, this is unused, but is still present. + #[doc(alias = "SELECTOR")] const SIGNATURE_HASH: FixedBytes<32>; /// Whether the event is anonymous. diff --git a/crates/sol-types/tests/macros/sol/mod.rs b/crates/sol-types/tests/macros/sol/mod.rs index 4bae1c3e9..b4e5e5bda 100644 --- a/crates/sol-types/tests/macros/sol/mod.rs +++ b/crates/sol-types/tests/macros/sol/mod.rs @@ -1084,3 +1084,67 @@ fn regression_nested_namespaced_structs() { assert_eq!(inner::C::libCNested1Call::SIGNATURE, format!("libCNested1({c_nested1})")); assert_eq!(inner::C::libCNested2Call::SIGNATURE, format!("libCNested2({c_nested2})")); } + +// https://github.com/alloy-rs/core/issues/734 +#[test] +fn event_indexed_udvt() { + use alloy_primitives::aliases::*; + + sol! { + type Currency is address; + type PoolId is bytes32; + + event Initialize( + PoolId indexed id, + Currency indexed currency0, + Currency indexed currency1, + uint24 fee, + int24 tickSpacing, + address hooks, + uint160 sqrtPriceX96, + int24 tick + ); + } + + assert_eq!( + Initialize::SIGNATURE, + "Initialize(bytes32,address,address,uint24,int24,address,uint160,int24)", + ); + assert_eq!( + Initialize::SIGNATURE_HASH, + b256!("dd466e674ea557f56295e2d0218a125ea4b4f0f6f3307b95f85e6110838d6438"), + ); + + let _ = Initialize { + id: B256::ZERO, + currency0: Address::ZERO, + currency1: Address::ZERO, + fee: U24::ZERO, + tickSpacing: I24::ZERO, + hooks: Address::ZERO, + sqrtPriceX96: U160::ZERO, + tick: I24::ZERO, + }; +} + +#[test] +fn event_indexed_elementary_arrays() { + sol! { + event AddrArray(address[1] indexed x); + event AddrDynArray(address[] indexed x); + + type MyAddress is address; + event AddrUdvtArray(MyAddress[1] indexed y); + event AddrUdvtDynArray(MyAddress[] indexed y); + } + + assert_eq!(AddrArray::SIGNATURE, "AddrArray(address[1])"); + let _ = AddrArray { x: B256::ZERO }; + assert_eq!(AddrDynArray::SIGNATURE, "AddrDynArray(address[])"); + let _ = AddrDynArray { x: B256::ZERO }; + + assert_eq!(AddrUdvtArray::SIGNATURE, "AddrUdvtArray(address[1])"); + let _ = AddrUdvtArray { y: B256::ZERO }; + assert_eq!(AddrUdvtDynArray::SIGNATURE, "AddrUdvtDynArray(address[])"); + let _ = AddrUdvtDynArray { y: B256::ZERO }; +} diff --git a/crates/syn-solidity/src/item/event.rs b/crates/syn-solidity/src/item/event.rs index ba8312eb5..c5d18073b 100644 --- a/crates/syn-solidity/src/item/event.rs +++ b/crates/syn-solidity/src/item/event.rs @@ -1,5 +1,6 @@ use crate::{ - kw, utils::DebugPunctuated, ParameterList, SolIdent, Spanned, Type, VariableDeclaration, + kw, utils::DebugPunctuated, ParameterList, SolIdent, SolPath, Spanned, Type, + VariableDeclaration, }; use proc_macro2::Span; use std::fmt; @@ -153,10 +154,6 @@ impl ItemEvent { self.parameters.iter().filter(|p| p.is_indexed()) } - pub fn dynamic_params(&self) -> impl Iterator { - self.parameters.iter().filter(|p| p.is_abi_dynamic()) - } - pub fn as_type(&self) -> Type { let mut ty = Type::Tuple(self.parameters.iter().map(|arg| arg.ty.clone()).collect()); ty.set_span(self.span()); @@ -252,17 +249,14 @@ impl EventParameter { self.indexed.is_none() } - /// Returns true if the event parameter is a dynamically sized type. - pub fn is_abi_dynamic(&self) -> bool { - self.ty.is_abi_dynamic() - } - /// Returns `true` if the event parameter is indexed and dynamically sized. /// These types are hashed, and then stored in the topics as specified in /// [the Solidity spec][ref]. /// + /// `custom_is_value_type` accounts for custom value types. + /// /// [ref]: https://docs.soliditylang.org/en/latest/abi-spec.html#events - pub fn indexed_as_hash(&self) -> bool { - self.is_indexed() && self.is_abi_dynamic() + pub fn indexed_as_hash(&self, custom_is_value_type: impl Fn(&SolPath) -> bool) -> bool { + self.is_indexed() && !self.ty.is_value_type(custom_is_value_type) } } diff --git a/crates/syn-solidity/src/type/mod.rs b/crates/syn-solidity/src/type/mod.rs index 9049b1912..64f997ce0 100644 --- a/crates/syn-solidity/src/type/mod.rs +++ b/crates/syn-solidity/src/type/mod.rs @@ -278,21 +278,17 @@ impl Type { Ok(self) } - /// Returns whether this type is ABI-encoded as a single EVM word (32 - /// bytes). - pub const fn is_one_word(&self) -> bool { - matches!( - self, - Self::Bool(_) - | Self::Int(..) - | Self::Uint(..) - | Self::FixedBytes(..) - | Self::Address(..) - | Self::Function(_) - ) + /// Returns whether this type is ABI-encoded as a single EVM word (32 bytes). + /// + /// This is the same as [`is_value_type`](Self::is_value_type). + #[deprecated = "use `is_value_type` instead"] + pub fn is_one_word(&self, custom_is_value_type: impl Fn(&SolPath) -> bool) -> bool { + self.is_value_type(custom_is_value_type) } /// Returns whether this type is dynamic according to ABI rules. + /// + /// Note that this does not account for custom types, such as UDVTs. pub fn is_abi_dynamic(&self) -> bool { match self { Self::Bool(_) @@ -308,7 +304,7 @@ impl Type { Self::Tuple(tuple) => tuple.is_abi_dynamic(), // not applicable - Self::Mapping(_) => false, + Self::Mapping(_) => true, } } @@ -316,9 +312,29 @@ impl Type { /// /// These types' variables are always passed by value. /// + /// `custom_is_value_type` accounts for custom value types. + /// /// See the [Solidity docs](https://docs.soliditylang.org/en/latest/types.html#value-types) for more information. - pub const fn is_value_type(&self) -> bool { - self.is_one_word() + pub fn is_value_type(&self, custom_is_value_type: impl Fn(&SolPath) -> bool) -> bool { + match self { + Self::Custom(custom) => custom_is_value_type(custom), + _ => self.is_value_type_simple(), + } + } + + /// Returns whether this type is a simple value type. + /// + /// See [`is_value_type`](Self::is_value_type) for more information. + pub fn is_value_type_simple(&self) -> bool { + matches!( + self, + Self::Bool(_) + | Self::Int(..) + | Self::Uint(..) + | Self::FixedBytes(..) + | Self::Address(..) + | Self::Function(_) + ) } pub const fn is_array(&self) -> bool {