Repository: gakonst/ethers-rs
Pull request #1498 (2022-07-21 19:25:40)

fix(abigen): contract names can be reserve words
Some contract names [(such as Enum.sol in gnosis)](https://github.com/safe-global/safe-contracts/blob/main/contracts/common/Enum.sol) are rust reserve words that cause invalid bindings to be generated.

## PR Checklist

- [x] Added Tests
- [x] Added Documentation
- [x] Updated the changelog


clifton: re #989 
clifton: ok @mattsse i made that change and fixed it up. should be good to go.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 13d57870..84da1688 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -90,6 +90,10 @@
   [#1030](https://github.com/gakonst/ethers-rs/pull/1030).
 - Generate correct bindings of struct's field names that are reserved words
   [#989](https://github.com/gakonst/ethers-rs/pull/989).
+- Generate correct binding module names that are reserved words
+  [#1498](https://github.com/gakonst/ethers-rs/pull/1498). Note: this changes
+  generated module names to snake case. For example, `MyContract` is now
+  `my_contract` rather than `mycontract_mod`.
 
 ### 0.6.0
 
diff --git a/ethers-contract/ethers-contract-abigen/Cargo.toml b/ethers-contract/ethers-contract-abigen/Cargo.toml
index d011b1e3..8cf9b72e 100644
--- a/ethers-contract/ethers-contract-abigen/Cargo.toml
+++ b/ethers-contract/ethers-contract-abigen/Cargo.toml
@@ -41,4 +41,4 @@ rustls = ["reqwest/rustls-tls"]
 
 [dev-dependencies]
 tempfile = "3.2.0"
-ethers-solc = { version = "^0.13.0", path = "../../ethers-solc", default-features = false, features = ["project-util"] }
+ethers-solc = { version = "^0.13.0", path = "../../ethers-solc", default-features = false, features = ["project-util", "svm-solc"] }
diff --git a/ethers-contract/ethers-contract-abigen/src/contract.rs b/ethers-contract/ethers-contract-abigen/src/contract.rs
index 1b33ff86..df31a3ec 100644
--- a/ethers-contract/ethers-contract-abigen/src/contract.rs
+++ b/ethers-contract/ethers-contract-abigen/src/contract.rs
@@ -101,8 +101,7 @@ impl Context {
     /// Expands the whole rust contract
     pub fn expand(&self) -> Result<ExpandedContract> {
         let name = &self.contract_ident;
-        let name_mod =
-            util::ident(&format!("{}_mod", self.contract_ident.to_string().to_lowercase()));
+        let name_mod = util::ident(&util::safe_module_name(&self.contract_name));
         let abi_name = self.inline_abi_ident();
 
         // 0. Imports
diff --git a/ethers-contract/ethers-contract-abigen/src/lib.rs b/ethers-contract/ethers-contract-abigen/src/lib.rs
index a9b7e76e..a538df71 100644
--- a/ethers-contract/ethers-contract-abigen/src/lib.rs
+++ b/ethers-contract/ethers-contract-abigen/src/lib.rs
@@ -29,7 +29,6 @@ pub use util::parse_address;
 
 use crate::contract::ExpandedContract;
 use eyre::Result;
-use inflector::Inflector;
 use proc_macro2::TokenStream;
 use std::{collections::HashMap, fs::File, io::Write, path::Path};
 
@@ -233,7 +232,7 @@ impl ContractBindings {
 
     /// Generate the default module name (snake case of the contract name)
     pub fn module_name(&self) -> String {
-        self.name.to_snake_case()
+        util::safe_module_name(&self.name)
     }
 
     /// Generate the default filename of the module
diff --git a/ethers-contract/ethers-contract-abigen/src/multi.rs b/ethers-contract/ethers-contract-abigen/src/multi.rs
index 594a5797..0023f583 100644
--- a/ethers-contract/ethers-contract-abigen/src/multi.rs
+++ b/ethers-contract/ethers-contract-abigen/src/multi.rs
@@ -559,7 +559,7 @@ serde_json = "1.0.79"
     /// Append module declarations to the `lib.rs` or `mod.rs`
     fn append_module_names(&self, mut buf: impl Write) -> Result<()> {
         let mut mod_names: BTreeSet<_> =
-            self.bindings.keys().map(|name| name.to_snake_case()).collect();
+            self.bindings.keys().map(|name| util::safe_module_name(name)).collect();
         if let Some(ref shared) = self.shared_types {
             mod_names.insert(shared.name.to_snake_case());
         }
@@ -1093,4 +1093,63 @@ contract Greeter2 {
         assert!(content.contains("pub struct Inner"));
         assert!(content.contains("pub struct Stuff"));
     }
+
+    #[test]
+    fn can_sanitize_reserved_words() {
+        let tmp = TempProject::dapptools().unwrap();
+
+        tmp.add_source(
+            "ReservedWords",
+            r#"
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.0;
+
+contract Mod {
+    function greet() public pure returns (uint256) {
+        return 1;
+    }
+}
+
+// from a gnosis contract
+contract Enum {
+    enum Operation {Call, DelegateCall}
+}
+"#,
+        )
+        .unwrap();
+
+        let _ = tmp.compile().unwrap();
+
+        let gen = MultiAbigen::from_json_files(tmp.artifacts_path()).unwrap();
+        let bindings = gen.build().unwrap();
+        let single_file_dir = tmp.root().join("single_bindings");
+        bindings.write_to_module(&single_file_dir, true).unwrap();
+
+        let single_file_mod = single_file_dir.join("mod.rs");
+        assert!(single_file_mod.exists());
+        let content = fs::read_to_string(&single_file_mod).unwrap();
+        assert!(content.contains("pub mod mod_ {"));
+        assert!(content.contains("pub mod enum_ {"));
+
+        // multiple files
+        let gen = MultiAbigen::from_json_files(tmp.artifacts_path()).unwrap();
+        let bindings = gen.build().unwrap();
+        let multi_file_dir = tmp.root().join("multi_bindings");
+        bindings.write_to_module(&multi_file_dir, false).unwrap();
+        let multi_file_mod = multi_file_dir.join("mod.rs");
+        assert!(multi_file_mod.exists());
+        let content = fs::read_to_string(&multi_file_mod).unwrap();
+        assert!(content.contains("pub mod enum_;"));
+        assert!(content.contains("pub mod mod_;"));
+
+        let enum_ = multi_file_dir.join("enum_.rs");
+        assert!(enum_.exists());
+        let content = fs::read_to_string(&enum_).unwrap();
+        assert!(content.contains("pub mod enum_ {"));
+
+        let mod_ = multi_file_dir.join("mod_.rs");
+        assert!(mod_.exists());
+        let content = fs::read_to_string(&mod_).unwrap();
+        assert!(content.contains("pub mod mod_ {"));
+    }
 }
diff --git a/ethers-contract/ethers-contract-abigen/src/util.rs b/ethers-contract/ethers-contract-abigen/src/util.rs
index d9c812e0..33936e09 100644
--- a/ethers-contract/ethers-contract-abigen/src/util.rs
+++ b/ethers-contract/ethers-contract-abigen/src/util.rs
@@ -40,6 +40,12 @@ fn safe_identifier_name(name: String) -> String {
     }
 }
 
+/// converts invalid rust module names to valid ones
+pub fn safe_module_name(name: &str) -> String {
+    // handle reserve words used in contracts (eg Enum is a gnosis contract)
+    safe_ident(&safe_snake_case(name)).to_string()
+}
+
 /// Expands an identifier as snakecase and preserve any leading or trailing underscores
 pub fn safe_snake_case_ident(name: &str) -> Ident {
     let i = name.to_snake_case();
@@ -228,4 +234,12 @@ mod tests {
             Address::from([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19]);
         assert_eq!(parse_address("0x000102030405060708090a0b0c0d0e0f10111213").unwrap(), expected);
     }
+
+    #[test]
+    fn test_safe_module_name() {
+        assert_eq!(safe_module_name("Valid"), "valid");
+        assert_eq!(safe_module_name("Enum"), "enum_");
+        assert_eq!(safe_module_name("Mod"), "mod_");
+        assert_eq!(safe_module_name("2Two"), "_2_two");
+    }
 }
diff --git a/ethers-middleware/Cargo.toml b/ethers-middleware/Cargo.toml
index ca8cdd37..16e37f76 100644
--- a/ethers-middleware/Cargo.toml
+++ b/ethers-middleware/Cargo.toml
@@ -14,7 +14,7 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
-ethers-contract = { version = "^0.13.0", path = "../ethers-contract", default-features = false, features = ["abigen"] }
+ethers-contract = { version = "^0.13.0", path = "../ethers-contract", default-features = false }
 ethers-core = { version = "^0.13.0", path = "../ethers-core", default-features = false }
 ethers-etherscan = { version = "^0.13.0", path = "../ethers-etherscan", default-features = false }
 ethers-providers = { version = "^0.13.0", path = "../ethers-providers", default-features = false }
@@ -43,7 +43,7 @@ hex = { version = "0.4.3", default-features = false, features = ["std"] }
 rand = { version = "0.8.5", default-features = false }
 ethers-providers = { version = "^0.13.0", path = "../ethers-providers", default-features = false, features = ["ws", "rustls"] }
 once_cell = "1.13.0"
-ethers-solc = { version = "^0.13.0", path = "../ethers-solc", default-features = false }
+ethers-solc = { version = "^0.13.0", path = "../ethers-solc" }
 serial_test = "0.8.0"
 
 [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
diff --git a/ethers-middleware/src/transformer/ds_proxy/factory.rs b/ethers-middleware/src/transformer/ds_proxy/factory.rs
index 8de0c88f..c6f0561d 100644
--- a/ethers-middleware/src/transformer/ds_proxy/factory.rs
+++ b/ethers-middleware/src/transformer/ds_proxy/factory.rs
@@ -37,12 +37,16 @@ mod dsproxyfactory_mod {
         Contract, EthEvent, Lazy,
     };
     use ethers_core::{
-        abi::{parse_abi, Abi, Detokenize, InvalidOutputType, Token, Tokenizable},
+        abi::{
+            parse_abi, Abi, Detokenize, InvalidOutputType, ParamType, Token, Tokenizable,
+            TokenizableItem,
+        },
         types::*,
     };
     use ethers_providers::Middleware;
     #[doc = "DsProxyFactory was auto-generated with ethers-rs Abigen. More information at: https://github.com/gakonst/ethers-rs"]
     use std::sync::Arc;
+
     pub static DSPROXYFACTORY_ABI: Lazy<Abi> = Lazy::new(|| {
         serde_json :: from_str ("[{\"constant\":true,\"inputs\":[{\"name\":\"\",\"type\":\"address\"}],\"name\":\"isProxy\",\"outputs\":[{\"name\":\"\",\"type\":\"bool\"}],\"payable\":false,\"stateMutability\":\"view\",\"type\":\"function\"},{\"constant\":true,\"inputs\":[],\"name\":\"cache\",\"outputs\":[{\"name\":\"\",\"type\":\"address\"}],\"payable\":false,\"stateMutability\":\"view\",\"type\":\"function\"},{\"constant\":false,\"inputs\":[],\"name\":\"build\",\"outputs\":[{\"name\":\"proxy\",\"type\":\"address\"}],\"payable\":false,\"stateMutability\":\"nonpayable\",\"type\":\"function\"},{\"constant\":false,\"inputs\":[{\"name\":\"owner\",\"type\":\"address\"}],\"name\":\"build\",\"outputs\":[{\"name\":\"proxy\",\"type\":\"address\"}],\"payable\":false,\"stateMutability\":\"nonpayable\",\"type\":\"function\"},{\"anonymous\":false,\"inputs\":[{\"indexed\":true,\"name\":\"sender\",\"type\":\"address\"},{\"indexed\":true,\"name\":\"owner\",\"type\":\"address\"},{\"indexed\":false,\"name\":\"proxy\",\"type\":\"address\"},{\"indexed\":false,\"name\":\"cache\",\"type\":\"address\"}],\"name\":\"Created\",\"type\":\"event\"}]\n") . expect ("invalid abi")
     });
@@ -78,18 +82,13 @@ mod dsproxyfactory_mod {
                 .expect("method not found (this should never happen)")
         }
         ///Calls the contract's `build` (0x8e1a55fc) function
-        pub fn build_with_sender(
-            &self,
-        ) -> ethers_contract::builders::ContractCall<M, ethers_core::types::Address> {
+        pub fn build_with_sender(&self) -> ContractCall<M, Address> {
             self.0
                 .method_hash([142, 26, 85, 252], ())
                 .expect("method not found (this should never happen)")
         }
         ///Calls the contract's `build` (0xf3701da2) function
-        pub fn build(
-            &self,
-            owner: ethers_core::types::Address,
-        ) -> ethers_contract::builders::ContractCall<M, ethers_core::types::Address> {
+        pub fn build(&self, owner: Address) -> ContractCall<M, Address> {
             self.0
                 .method_hash([243, 112, 29, 162], owner)
                 .expect("method not found (this should never happen)")
@@ -101,24 +100,107 @@ mod dsproxyfactory_mod {
                 .expect("method not found (this should never happen)")
         }
         ///Gets the contract's `Created` event
-        pub fn created_filter(&self) -> ethers_contract::builders::Event<M, CreatedFilter> {
+        pub fn created_filter(&self) -> Event<M, CreatedFilter> {
             self.0.event()
         }
 
         /// Returns an [`Event`](ethers_contract::builders::Event) builder for all events of this
         /// contract
-        pub fn events(&self) -> ethers_contract::builders::Event<M, CreatedFilter> {
+        pub fn events(&self) -> Event<M, CreatedFilter> {
             self.0.event_with_filter(Default::default())
         }
     }
-    #[derive(Clone, Debug, Default, Eq, PartialEq, EthEvent)]
-    #[ethevent(name = "Created")]
+
+    #[derive(Clone, Debug, Default, Eq, PartialEq)]
     pub struct CreatedFilter {
-        #[ethevent(indexed)]
         pub sender: Address,
-        #[ethevent(indexed)]
         pub owner: Address,
         pub proxy: Address,
         pub cache: Address,
     }
+
+    impl ethers_core::abi::AbiType for CreatedFilter {
+        fn param_type() -> ParamType {
+            ParamType::Tuple(::std::vec![
+                ParamType::Address; 4
+            ])
+        }
+    }
+    impl ethers_core::abi::AbiArrayType for CreatedFilter {}
+
+    impl Tokenizable for CreatedFilter {
+        fn from_token(token: Token) -> Result<Self, InvalidOutputType>
+        where
+            Self: Sized,
+        {
+            if let Token::Tuple(tokens) = token {
+                if tokens.len() != 4usize {
+                    return Err(InvalidOutputType(format!(
+                        "Expected {} tokens, got {}: {:?}",
+                        4usize,
+                        tokens.len(),
+                        tokens
+                    )))
+                }
+                let mut iter = tokens.into_iter();
+                Ok(Self {
+                    sender: Tokenizable::from_token(iter.next().unwrap())?,
+                    owner: Tokenizable::from_token(iter.next().unwrap())?,
+                    proxy: Tokenizable::from_token(iter.next().unwrap())?,
+                    cache: Tokenizable::from_token(iter.next().unwrap())?,
+                })
+            } else {
+                Err(InvalidOutputType(format!("Expected Tuple, got {:?}", token)))
+            }
+        }
+        fn into_token(self) -> Token {
+            Token::Tuple(::std::vec![
+                self.sender.into_token(),
+                self.owner.into_token(),
+                self.proxy.into_token(),
+                self.cache.into_token(),
+            ])
+        }
+    }
+    impl TokenizableItem for CreatedFilter {}
+
+    impl ethers_contract::EthEvent for CreatedFilter {
+        fn name() -> std::borrow::Cow<'static, str> {
+            "Created".into()
+        }
+        fn signature() -> H256 {
+            H256([
+                37, 155, 48, 202, 57, 136, 92, 109, 128, 26, 11, 93, 188, 152, 134, 64, 243, 194,
+                94, 47, 55, 83, 31, 225, 56, 197, 197, 175, 137, 85, 212, 27,
+            ])
+        }
+        fn abi_signature() -> std::borrow::Cow<'static, str> {
+            "Created(address,address,address,address)".into()
+        }
+        fn decode_log(log: &ethers_core::abi::RawLog) -> Result<Self, ethers_core::abi::Error>
+        where
+            Self: Sized,
+        {
+            let ethers_core::abi::RawLog { data, topics } = log;
+            let event_signature = topics.get(0).ok_or(ethers_core::abi::Error::InvalidData)?;
+            if event_signature != &Self::signature() {
+                return Err(ethers_core::abi::Error::InvalidData)
+            }
+            let topic_types = ::std::vec![ParamType::Address, ParamType::Address];
+            let data_types = [ParamType::Address, ParamType::Address];
+            let flat_topics =
+                topics.iter().skip(1).flat_map(|t| t.as_ref().to_vec()).collect::<Vec<u8>>();
+            let topic_tokens = ethers_core::abi::decode(&topic_types, &flat_topics)?;
+            if topic_tokens.len() != topics.len() - 1 {
+                return Err(ethers_core::abi::Error::InvalidData)
+            }
+            let data_tokens = ethers_core::abi::decode(&data_types, data)?;
+            let tokens: Vec<_> = topic_tokens.into_iter().chain(data_tokens.into_iter()).collect();
+            Tokenizable::from_token(Token::Tuple(tokens))
+                .map_err(|_| ethers_core::abi::Error::InvalidData)
+        }
+        fn is_anonymous() -> bool {
+            false
+        }
+    }
 }
diff --git a/ethers-solc/src/artifact_output/configurable.rs b/ethers-solc/src/artifact_output/configurable.rs
index 7958a211..7d4fb06c 100644
--- a/ethers-solc/src/artifact_output/configurable.rs
+++ b/ethers-solc/src/artifact_output/configurable.rs
@@ -5,8 +5,8 @@
 //! `ConfigurableArtifacts` populates a single `Artifact`, the `ConfigurableArtifact`, by default
 //! with essential entries only, such as `abi`, `bytecode`,..., but may include additional values
 //! based on its `ExtraOutputValues` that maps to various objects in the solc contract output, see
-//! also: [`OutputSelection`](crate::artifacts::OutputSelection). In addition to that some output
-//! values can also be emitted as standalone files.
+//! also: [`OutputSelection`](crate::artifacts::output_selection::OutputSelection). In addition to
+//! that some output values can also be emitted as standalone files.
 
 use crate::{
     artifacts::{
diff --git a/ethers-solc/src/compile/mod.rs b/ethers-solc/src/compile/mod.rs
index e1b6217a..4fe6bf5a 100644
--- a/ethers-solc/src/compile/mod.rs
+++ b/ethers-solc/src/compile/mod.rs
@@ -431,7 +431,7 @@ impl Solc {
     }
 
     /// Verify that the checksum for this version of solc is correct. We check against the SHA256
-    /// checksum from the build information published by binaries.soliditylang
+    /// checksum from the build information published by [binaries.soliditylang.org](https://binaries.soliditylang.org/)
     #[cfg(all(feature = "svm-solc"))]
     pub fn verify_checksum(&self) -> Result<()> {
         let version = self.version_short()?;
diff --git a/ethers-solc/src/lib.rs b/ethers-solc/src/lib.rs
index 8d98713b..55238466 100644
--- a/ethers-solc/src/lib.rs
+++ b/ethers-solc/src/lib.rs
@@ -1,4 +1,6 @@
 #![deny(rustdoc::broken_intra_doc_links)]
+#![allow(rustdoc::private_intra_doc_links)]
+
 pub mod artifacts;
 pub mod sourcemap;
 
@@ -279,7 +281,7 @@ impl<T: ArtifactOutput> Project<T> {
     }
 
     /// Convenience function to compile a series of solidity files with the project's settings.
-    /// Same as [`Self::svm_compile()`] but with the given `files` as input.
+    /// Same as [`Self::compile()`] but with the given `files` as input.
     ///
     /// # Example
     ///
@@ -310,7 +312,7 @@ impl<T: ArtifactOutput> Project<T> {
     }
 
     /// Convenience function to compile only (re)compile files that match the provided [FileFilter].
-    /// Same as [`Self::svm_compile()`] but with only with those files as input that match
+    /// Same as [`Self::compile()`] but with only with those files as input that match
     /// [FileFilter::is_match()].
     ///
     /// # Example - Only compile Test files


Test patch:
diff --git a/ethers-contract/tests/abigen.rs b/ethers-contract/tests/abigen.rs
index 3271c551..ecf805f9 100644
--- a/ethers-contract/tests/abigen.rs
+++ b/ethers-contract/tests/abigen.rs
@@ -366,7 +366,7 @@ async fn can_handle_underscore_functions() {
     // Manual call construction
     use ethers_providers::Middleware;
     // TODO: How do we handle underscores for calls here?
-    let data = simplestorage_mod::HashPuzzleCall.encode();
+    let data = simple_storage::HashPuzzleCall.encode();
     let tx = Eip1559TransactionRequest::new().data(data).to(addr);
     let tx = TypedTransaction::Eip1559(tx);
     let res5 = client.call(&tx, None).await.unwrap();
diff --git a/ethers-contract/tests/contract.rs b/ethers-contract/tests/contract.rs
index acf63042..4d119048 100644
--- a/ethers-contract/tests/contract.rs
+++ b/ethers-contract/tests/contract.rs
@@ -601,7 +601,7 @@ mod eth_tests {
             out: Address::from([0; 20]),
         };
 
-        let derived_foo_bar = deriveeip712test_mod::FooBar {
+        let derived_foo_bar = derive_eip_712_test::FooBar {
             foo: foo_bar.foo,
             bar: foo_bar.bar,
             fizz: foo_bar.fizz.clone(),