From 0e7f46b03d1af02e9a79a327a9d9089210e9c35a Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 7 Sep 2022 18:14:13 +0200 Subject: [PATCH] feat(abigen): subsitute structs in event bindings (#1674) * fix(abigen): handle event defaults * feat(abigen): subsitute structs in event bindings * update changelog * chore: rustfmt * fix broken tests * chore(clippy): make clippy happy --- CHANGELOG.md | 1 + .../ethers-contract-abigen/src/contract.rs | 1 + .../src/contract/events.rs | 111 +++++++++++------- .../src/contract/structs.rs | 36 +++++- .../ethers-contract-abigen/src/multi.rs | 32 ++--- ethers-contract/tests/it/abigen.rs | 16 +++ .../solidity-contracts/EventWithStruct.json | 33 ++++++ ethers-core/src/abi/human_readable/mod.rs | 33 +++++- 8 files changed, 196 insertions(+), 67 deletions(-) create mode 100644 ethers-contract/tests/solidity-contracts/EventWithStruct.json diff --git a/CHANGELOG.md b/CHANGELOG.md index ec588d09..241c9ae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,7 @@ ### Unreleased +- Use corresponding rust structs for event fields if they're solidity structs [#1674](https://github.com/gakonst/ethers-rs/pull/1674) - Add `ContractFilter` to filter contracts in `MultiAbigen` [#1564](https://github.com/gakonst/ethers-rs/pull/1564) - generate error bindings for custom errors [#1549](https://github.com/gakonst/ethers-rs/pull/1549) - Support overloaded events diff --git a/ethers-contract/ethers-contract-abigen/src/contract.rs b/ethers-contract/ethers-contract-abigen/src/contract.rs index 885b348a..2d55698b 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract.rs @@ -207,6 +207,7 @@ impl Context { .rust_type_names .extend(abi_parser.function_params.values().map(|ty| (ty.clone(), ty.clone()))); internal_structs.function_params = abi_parser.function_params.clone(); + internal_structs.event_params = abi_parser.event_params.clone(); internal_structs.outputs = abi_parser.outputs.clone(); internal_structs diff --git a/ethers-contract/ethers-contract-abigen/src/contract/events.rs b/ethers-contract/ethers-contract-abigen/src/contract/events.rs index 9c098635..90727439 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract/events.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract/events.rs @@ -1,6 +1,7 @@ use super::{types, util, Context}; +use crate::util::can_derive_defaults; use ethers_core::{ - abi::{Event, EventExt, EventParam, ParamType, SolStruct}, + abi::{Event, EventExt, EventParam, Param, ParamType}, macros::{ethers_contract_crate, ethers_core_crate}, }; use eyre::Result; @@ -140,55 +141,60 @@ impl Context { /// Note that this is slightly different from expanding a Solidity type as /// complex types like arrays and strings get emitted as hashes when they are /// indexed. - /// If a complex types matches with a struct previously parsed by the AbiParser, + /// If a complex types matches with a struct previously parsed by the internal structs, /// we can replace it - fn expand_input_type(&self, input: &EventParam) -> Result { + fn expand_input_type( + &self, + event: &Event, + input: &EventParam, + idx: usize, + ) -> Result { let ethers_core = ethers_core_crate(); Ok(match (&input.kind, input.indexed) { - (ParamType::Array(ty), true) => { - if let ParamType::Tuple(..) = **ty { - // represents an array of a struct - if let Some(ty) = self - .abi_parser - .structs - .get(&input.name) - .map(SolStruct::name) - .map(util::ident) - { - return Ok(quote! {::std::vec::Vec<#ty>}) - } - } + (ParamType::Array(_), true) => { quote! { #ethers_core::types::H256 } } - (ParamType::FixedArray(ty, size), true) => { - if let ParamType::Tuple(..) = **ty { - // represents a fixed array of a struct - if let Some(ty) = self - .abi_parser - .structs - .get(&input.name) - .map(SolStruct::name) - .map(util::ident) - { - let size = Literal::usize_unsuffixed(*size); - return Ok(quote! {[#ty; #size]}) - } - } + (ParamType::FixedArray(_, _), true) => { quote! { #ethers_core::types::H256 } } (ParamType::Tuple(..), true) => { - // represents a struct - if let Some(ty) = - self.abi_parser.structs.get(&input.name).map(SolStruct::name).map(util::ident) - { - quote! {#ty} - } else { - quote! { #ethers_core::types::H256 } - } + quote! { #ethers_core::types::H256 } } (ParamType::Bytes, true) | (ParamType::String, true) => { quote! { #ethers_core::types::H256 } } + (ParamType::Tuple(_), false) => { + let ty = if let Some(rust_struct_name) = + self.internal_structs.get_event_input_struct_type(&event.name, idx) + { + let ident = util::ident(rust_struct_name); + quote! {#ident} + } else { + types::expand(&input.kind)? + }; + ty + } + (ParamType::Array(_), _) => { + // represents an array of a struct + if let Some(rust_struct_name) = + self.internal_structs.get_event_input_struct_type(&event.name, idx) + { + let ty = util::ident(rust_struct_name); + return Ok(quote! {::std::vec::Vec<#ty>}) + } + types::expand(&input.kind)? + } + (ParamType::FixedArray(_, size), _) => { + // represents a fixed array of a struct + if let Some(rust_struct_name) = + self.internal_structs.get_event_input_struct_type(&event.name, idx) + { + let ty = util::ident(rust_struct_name); + let size = Literal::usize_unsuffixed(*size); + return Ok(quote! {[#ty; #size]}) + } + types::expand(&input.kind)? + } (kind, _) => types::expand(kind)?, }) } @@ -199,10 +205,10 @@ impl Context { .inputs .iter() .enumerate() - .map(|(i, input)| { + .map(|(idx, input)| { // NOTE: Events can contain nameless values. - let name = util::expand_input_name(i, &input.name); - let ty = self.expand_input_type(input)?; + let name = util::expand_input_name(idx, &input.name); + let ty = self.expand_input_type(event, input, idx)?; Ok((name, ty, input.indexed)) }) @@ -255,10 +261,31 @@ impl Context { let derives = util::expand_derives(&self.event_derives); + // rust-std only derives default automatically for arrays len <= 32 + // for large array types we skip derive(Default) + let derive_default = if can_derive_defaults( + &event + .inputs + .iter() + .map(|param| Param { + name: param.name.clone(), + kind: param.kind.clone(), + internal_type: None, + }) + .collect::>(), + ) { + quote! { + #[derive(Default)] + } + } else { + quote! {} + }; + let ethers_contract = ethers_contract_crate(); Ok(quote! { - #[derive(Clone, Debug, Default, Eq, PartialEq, #ethers_contract::EthEvent, #ethers_contract::EthDisplay, #derives)] + #[derive(Clone, Debug, Eq, PartialEq, #ethers_contract::EthEvent, #ethers_contract::EthDisplay, #derives)] + #derive_default #[ethevent( name = #event_abi_name, abi = #abi_signature )] pub #data_type_definition }) diff --git a/ethers-contract/ethers-contract-abigen/src/contract/structs.rs b/ethers-contract/ethers-contract-abigen/src/contract/structs.rs index fc82f34f..c99f8447 100644 --- a/ethers-contract/ethers-contract-abigen/src/contract/structs.rs +++ b/ethers-contract/ethers-contract-abigen/src/contract/structs.rs @@ -229,6 +229,12 @@ pub struct InternalStructs { /// (function name) -> Vec all structs the function returns pub(crate) outputs: HashMap>, + /// (event name, idx) -> struct which are the identifying properties we get the name + /// from ethabi. + /// + /// Note: we need to map the index of the event here because events can contain nameless inputs + pub(crate) event_params: HashMap<(String, usize), String>, + /// All the structs extracted from the abi with their identifier as key pub(crate) structs: HashMap, @@ -245,23 +251,37 @@ impl InternalStructs { let mut top_level_internal_types = HashMap::new(); let mut function_params = HashMap::new(); let mut outputs = HashMap::new(); + let mut event_params = HashMap::new(); let mut structs = HashMap::new(); for item in abi .into_iter() - .filter(|item| item.type_field == "constructor" || item.type_field == "function") + .filter(|item| matches!(item.type_field.as_str(), "constructor" | "function" | "event")) { + let is_event = item.type_field == "event"; + if let Some(name) = item.name { - for input in item.inputs { + for (idx, input) in item.inputs.into_iter().enumerate() { if let Some(ty) = input .internal_type .as_deref() .filter(|ty| ty.starts_with("struct ")) .map(struct_type_identifier) { - function_params.insert((name.clone(), input.name.clone()), ty.to_string()); + if is_event { + event_params.insert((name.clone(), idx), ty.to_string()); + } else { + function_params + .insert((name.clone(), input.name.clone()), ty.to_string()); + } top_level_internal_types.insert(ty.to_string(), input); } } + + if is_event { + // no outputs in an event + continue + } + let mut output_structs = Vec::new(); for output in item.outputs { if let Some(ty) = output @@ -300,6 +320,7 @@ impl InternalStructs { function_params, outputs, structs, + event_params, struct_tuples, rust_type_names: type_names .into_iter() @@ -318,6 +339,15 @@ impl InternalStructs { .map(String::as_str) } + /// Returns the name of the rust type that will be generated if the given input is a struct + /// This takes the index of event's parameter instead of the parameter's name like + /// [`Self::get_function_input_struct_type`] does because we can't rely on the name since events + /// support nameless parameters NOTE: this does not account for arrays or fixed arrays + pub fn get_event_input_struct_type(&self, event: &str, idx: usize) -> Option<&str> { + let key = (event.to_string(), idx); + self.event_params.get(&key).and_then(|id| self.rust_type_names.get(id)).map(String::as_str) + } + /// Returns the name of the rust type that will be generated if the given output is a struct /// NOTE: this does not account for arrays or fixed arrays pub fn get_function_output_struct_type( diff --git a/ethers-contract/ethers-contract-abigen/src/multi.rs b/ethers-contract/ethers-contract-abigen/src/multi.rs index 32396b9c..9c687eba 100644 --- a/ethers-contract/ethers-contract-abigen/src/multi.rs +++ b/ethers-contract/ethers-contract-abigen/src/multi.rs @@ -828,12 +828,12 @@ mod tests { let single_file = false; - multi_gen.clone().build().unwrap().write_to_module(&mod_root, single_file).unwrap(); + multi_gen.clone().build().unwrap().write_to_module(mod_root, single_file).unwrap(); multi_gen .clone() .build() .unwrap() - .ensure_consistent_module(&mod_root, single_file) + .ensure_consistent_module(mod_root, single_file) .expect("Inconsistent bindings"); }) } @@ -845,12 +845,12 @@ mod tests { let single_file = true; - multi_gen.clone().build().unwrap().write_to_module(&mod_root, single_file).unwrap(); + multi_gen.clone().build().unwrap().write_to_module(mod_root, single_file).unwrap(); multi_gen .clone() .build() .unwrap() - .ensure_consistent_module(&mod_root, single_file) + .ensure_consistent_module(mod_root, single_file) .expect("Inconsistent bindings"); }) } @@ -868,13 +868,13 @@ mod tests { .clone() .build() .unwrap() - .write_to_crate(name, version, &mod_root, single_file) + .write_to_crate(name, version, mod_root, single_file) .unwrap(); multi_gen .clone() .build() .unwrap() - .ensure_consistent_crate(name, version, &mod_root, single_file, true) + .ensure_consistent_crate(name, version, mod_root, single_file, true) .expect("Inconsistent bindings"); }) } @@ -892,13 +892,13 @@ mod tests { .clone() .build() .unwrap() - .write_to_crate(name, version, &mod_root, single_file) + .write_to_crate(name, version, mod_root, single_file) .unwrap(); multi_gen .clone() .build() .unwrap() - .ensure_consistent_crate(name, version, &mod_root, single_file, true) + .ensure_consistent_crate(name, version, mod_root, single_file, true) .expect("Inconsistent bindings"); }) } @@ -910,7 +910,7 @@ mod tests { let single_file = false; - multi_gen.clone().build().unwrap().write_to_module(&mod_root, single_file).unwrap(); + multi_gen.clone().build().unwrap().write_to_module(mod_root, single_file).unwrap(); let mut cloned = multi_gen.clone(); cloned.push( @@ -924,7 +924,7 @@ mod tests { ); let result = - cloned.build().unwrap().ensure_consistent_module(&mod_root, single_file).is_err(); + cloned.build().unwrap().ensure_consistent_module(mod_root, single_file).is_err(); // ensure inconsistent bindings are detected assert!(result, "Inconsistent bindings wrongly approved"); @@ -938,7 +938,7 @@ mod tests { let single_file = true; - multi_gen.clone().build().unwrap().write_to_module(&mod_root, single_file).unwrap(); + multi_gen.clone().build().unwrap().write_to_module(mod_root, single_file).unwrap(); let mut cloned = multi_gen.clone(); cloned.push( @@ -952,7 +952,7 @@ mod tests { ); let result = - cloned.build().unwrap().ensure_consistent_module(&mod_root, single_file).is_err(); + cloned.build().unwrap().ensure_consistent_module(mod_root, single_file).is_err(); // ensure inconsistent bindings are detected assert!(result, "Inconsistent bindings wrongly approved"); @@ -972,7 +972,7 @@ mod tests { .clone() .build() .unwrap() - .write_to_crate(name, version, &mod_root, single_file) + .write_to_crate(name, version, mod_root, single_file) .unwrap(); let mut cloned = multi_gen.clone(); @@ -989,7 +989,7 @@ mod tests { let result = cloned .build() .unwrap() - .ensure_consistent_crate(name, version, &mod_root, single_file, true) + .ensure_consistent_crate(name, version, mod_root, single_file, true) .is_err(); // ensure inconsistent bindings are detected @@ -1010,7 +1010,7 @@ mod tests { .clone() .build() .unwrap() - .write_to_crate(name, version, &mod_root, single_file) + .write_to_crate(name, version, mod_root, single_file) .unwrap(); let mut cloned = multi_gen.clone(); @@ -1027,7 +1027,7 @@ mod tests { let result = cloned .build() .unwrap() - .ensure_consistent_crate(name, version, &mod_root, single_file, true) + .ensure_consistent_crate(name, version, mod_root, single_file, true) .is_err(); // ensure inconsistent bindings are detected diff --git a/ethers-contract/tests/it/abigen.rs b/ethers-contract/tests/it/abigen.rs index 2d924be3..d09c2b96 100644 --- a/ethers-contract/tests/it/abigen.rs +++ b/ethers-contract/tests/it/abigen.rs @@ -19,6 +19,7 @@ use std::{ fn assert_codec() {} fn assert_tokenizeable() {} fn assert_call() {} +fn assert_event() {} #[test] fn can_gen_human_readable() { @@ -720,3 +721,18 @@ fn can_gen_large_tuple_array() { let _call = CallWithLongArrayCall::default(); assert_call::(); } + +#[test] +fn can_generate_event_with_structs() { + /* + contract MyContract { + struct MyStruct {uint256 a; uint256 b; } + event MyEvent(MyStruct, uint256); + } + */ + abigen!(MyContract, "ethers-contract/tests/solidity-contracts/EventWithStruct.json"); + + let _filter = MyEventFilter { p0: MyStruct::default(), c: U256::zero() }; + assert_eq!("MyEvent((uint256,uint256),uint256)", MyEventFilter::abi_signature()); + assert_event::(); +} diff --git a/ethers-contract/tests/solidity-contracts/EventWithStruct.json b/ethers-contract/tests/solidity-contracts/EventWithStruct.json new file mode 100644 index 00000000..1bbb1f86 --- /dev/null +++ b/ethers-contract/tests/solidity-contracts/EventWithStruct.json @@ -0,0 +1,33 @@ +[ + { + "anonymous": false, + "inputs": [ + { + "components": [ + { + "internalType": "uint256", + "name": "a", + "type": "uint256" + }, + { + "internalType": "uint256", + "name": "b", + "type": "uint256" + } + ], + "indexed": false, + "internalType": "struct MyContract.MyStruct", + "name": "", + "type": "tuple" + }, + { + "indexed": false, + "internalType": "uint256", + "name": "c", + "type": "uint256" + } + ], + "name": "MyEvent", + "type": "event" + } +] \ No newline at end of file diff --git a/ethers-core/src/abi/human_readable/mod.rs b/ethers-core/src/abi/human_readable/mod.rs index ca832e8f..4b5125ff 100644 --- a/ethers-core/src/abi/human_readable/mod.rs +++ b/ethers-core/src/abi/human_readable/mod.rs @@ -18,6 +18,11 @@ pub struct AbiParser { /// (function name, param name) -> struct which are the identifying properties we get the name /// from ethabi. pub function_params: HashMap<(String, String), String>, + /// (event name, idx) -> struct which are the identifying properties we get the name + /// from ethabi. + /// + /// Note: we need to map the index of the event here because events can contain nameless inputs + pub event_params: HashMap<(String, usize), String>, /// (function name) -> Vec all structs the function returns pub outputs: HashMap>, } @@ -170,12 +175,13 @@ impl AbiParser { structs: structs.into_iter().map(|s| (s.name().to_string(), s)).collect(), struct_tuples: HashMap::new(), function_params: Default::default(), + event_params: Default::default(), outputs: Default::default(), } } /// Parses a solidity event declaration from `event (args*) anonymous?` - pub fn parse_event(&self, s: &str) -> Result { + pub fn parse_event(&mut self, s: &str) -> Result { let mut event = s.trim(); if !event.starts_with("event ") { bail!("Not an event `{}`", s) @@ -208,8 +214,20 @@ impl AbiParser { .split(',') .map(|e| self.parse_event_arg(e)) .collect::, _>>()? + .into_iter() + .enumerate() + .map(|(idx, (input, struct_name))| { + if let Some(struct_name) = struct_name { + // keep track of the user defined struct of that param + self.event_params.insert((name.clone(), idx), struct_name); + } + input + }) + .collect() }; - return Ok(Event { name, inputs, anonymous }) + + let event = Event { name, inputs, anonymous }; + return Ok(event) } Some(' ') | Some('\t') => continue, Some(c) => { @@ -220,7 +238,9 @@ impl AbiParser { } /// Parse a single event param - fn parse_event_arg(&self, input: &str) -> Result { + /// + /// See [`Self::parse_type`] + fn parse_event_arg(&self, input: &str) -> Result<(EventParam, Option)> { let mut iter = input.trim().rsplitn(3, is_whitespace); let mut indexed = false; let mut name = @@ -246,7 +266,8 @@ impl AbiParser { name = ""; } - Ok(EventParam { name: name.to_string(), indexed, kind: self.parse_type(type_str)?.0 }) + let (kind, user_ty) = self.parse_type(type_str)?; + Ok((EventParam { name: name.to_string(), indexed, kind }, user_ty)) } /// Returns the parsed function from the input string @@ -665,12 +686,12 @@ mod tests { #[test] fn parse_event_input() { assert_eq!( - AbiParser::default().parse_event_arg("address indexed x").unwrap(), + AbiParser::default().parse_event_arg("address indexed x").unwrap().0, EventParam { name: "x".to_string(), kind: ParamType::Address, indexed: true } ); assert_eq!( - AbiParser::default().parse_event_arg("address x").unwrap(), + AbiParser::default().parse_event_arg("address x").unwrap().0, EventParam { name: "x".to_string(), kind: ParamType::Address, indexed: false } ); }