Reuse single shared allocation for ABI data (#970)

Currently whenever you create a new instance of a contract the entire
`Abi` gets cloned (which can be really huge) plus a bunch of signatures
get computed (a lot of hashing) and cached. This is really costly
considering that the `Abi` of a contract never changes so it would be an
easy win to share the same allocation of those immutable pieces of data
(`Abi` and computed hashes) across all instances of the same contract.
This would make the creation of subsequent instances of the same
contract mostly free.

This can already be achieved with ugly workarounds discussed and
implemented [here](cowprotocol/services#2628)
but if this issue gets resolved in the library itself manual allocation
optimization will not be needed and the performance improvement for any
code that creates a lot of contract instances would be huge.

The actual change is overall pretty small. I created a new type
`Interface` that contains all the immutable data derived from the `Abi`.
Then I gave it custom `(De)Serialize` implementations which mostly just
do the same thing it did before (defer to `Abi`'s implementation). The
`Deserialize` implementation of course also computes the important hash
and stores it's values together with the `Abi`.
Because the typesafe rust binding code generated by `ethcontract-rs`
already stores the raw contract in a static allocation (which now
contains the relevant immutable state `Arc`ed) we now get cheap contract
instantiations.

Thanks @nlordell for the idea of fixing the performance issue directly
in the library. 👍

The code also has a few unrelated changes to make the `nightly` CI build
pass.

### Breaking changes
* `Artifact::insert` can now run into annoying borrow checker issues
(that can be worked around, though)
* `Contract:abi` is now accessible with `Contract::interface::abi`
* `Contract::interface::abi` is harder to mutate than `Contract::abi`
due to the introduction of the `Arc`

Overall I don't know how many people actually rely on this crate and
whether or not these breaking changes would be a big deal for them. But
even if this should not get merged into the main release of the crate I
think this patch is still useful for anybody who needs to optimize
performance across the board.

### Test Plan
existing tests
ran cow protocol backend using the patched library and didn't see any
obvious issues

ethcontract-common/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ethcontract-common"
-version = "0.25.5"
+version = "0.25.6"
 authors = ["Gnosis developers <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
@@ -14,7 +14,7 @@ Common types for ethcontract-rs runtime and proc macro.
 [dependencies]
 ethabi = "18.0"
 hex = "0.4"
-serde = "1.0"
+serde= { version = "1.0", features = ["rc"] }
 serde_derive = "1.0"
 serde_json = "1.0"
 thiserror = "1.0"

ethcontract-common/src/artifact.rs

@@ -8,11 +8,12 @@
 //! artifact models. It also provides tools to load artifacts from different
 //! sources, and parse them using different formats.
 
-use crate::contract::{Documentation, Network};
+use crate::contract::{Documentation, Interface, Network};
 use crate::{Abi, Bytecode, Contract};
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
 use std::ops::Deref;
+use std::sync::Arc;
 
 pub mod hardhat;
 pub mod truffle;
@@ -151,7 +152,7 @@ pub struct ContractMut<'a>(&'a mut Contract);
 impl<'a> ContractMut<'a> {
     /// Returns mutable reference to contract's abi.
     pub fn abi_mut(&mut self) -> &mut Abi {
-        &mut self.0.abi
+        &mut Arc::make_mut(&mut self.0.interface).abi
     }
 
     /// Returns mutable reference to contract's bytecode.
@@ -188,6 +189,18 @@ impl Deref for ContractMut<'_> {
     }
 }
 
+impl Drop for ContractMut<'_> {
+    fn drop(&mut self) {
+        // The ABI might have gotten mutated while this guard was alive.
+        // Since we compute pre-compute and cache a few values based on the ABI
+        // as a performance optimization we need to recompute those cached values
+        // with the new ABI once the user is done updating the mutable contract.
+        let abi = self.0.interface.abi.clone();
+        let interface = Interface::from(abi);
+        *Arc::make_mut(&mut self.0.interface) = interface;
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -204,26 +217,32 @@ mod test {
 
         assert_eq!(artifact.len(), 0);
 
-        let insert_res = artifact.insert(make_contract("C1"));
+        {
+            let insert_res = artifact.insert(make_contract("C1"));
 
-        assert_eq!(insert_res.inserted_contract.name, "C1");
-        assert!(insert_res.old_contract.is_none());
+            assert_eq!(insert_res.inserted_contract.name, "C1");
+            assert!(insert_res.old_contract.is_none());
+        }
 
         assert_eq!(artifact.len(), 1);
         assert!(artifact.contains("C1"));
 
-        let insert_res = artifact.insert(make_contract("C2"));
+        {
+            let insert_res = artifact.insert(make_contract("C2"));
 
-        assert_eq!(insert_res.inserted_contract.name, "C2");
-        assert!(insert_res.old_contract.is_none());
+            assert_eq!(insert_res.inserted_contract.name, "C2");
+            assert!(insert_res.old_contract.is_none());
+        }
 
         assert_eq!(artifact.len(), 2);
         assert!(artifact.contains("C2"));
 
-        let insert_res = artifact.insert(make_contract("C1"));
+        {
+            let insert_res = artifact.insert(make_contract("C1"));
 
-        assert_eq!(insert_res.inserted_contract.name, "C1");
-        assert!(insert_res.old_contract.is_some());
+            assert_eq!(insert_res.inserted_contract.name, "C1");
+            assert!(insert_res.old_contract.is_some());
+        }
 
         assert_eq!(artifact.len(), 2);
     }

ethcontract-common/src/artifact/hardhat.rs

@@ -426,15 +426,18 @@ impl HardHatLoader {
         address: Address,
         transaction_hash: Option<TransactionHash>,
     ) -> Result<(), ArtifactError> {
-        let mut contract = match artifact.get_mut(&contract.name) {
-            Some(existing_contract) => {
-                if existing_contract.abi != contract.abi {
-                    return Err(ArtifactError::AbiMismatch(contract.name));
-                }
-
-                existing_contract
+        let contract_guard = artifact.get_mut(&contract.name);
+        let mut contract = if let Some(existing_contract) = contract_guard {
+            if existing_contract.interface != contract.interface {
+                return Err(ArtifactError::AbiMismatch(contract.name));
             }
-            None => artifact.insert(contract).inserted_contract,
+
+            existing_contract
+        } else {
+            // `Drop` of the contract guard can update the underlying contract which can lead
+            // to borrowing issues. To work around those we manually drop the guard here.
+            drop(contract_guard);
+            artifact.insert(contract).inserted_contract
         };
 
         let deployment_information = transaction_hash.map(DeploymentInformation::TransactionHash);

ethcontract-common/src/artifact/truffle.rs

@@ -141,7 +141,7 @@ impl TruffleLoader {
         let mut contract: Contract = loader(source)?;
 
         if let Some(name) = &self.name {
-            contract.name = name.clone();
+            contract.name.clone_from(name);
         }
 
         Ok(contract)

ethcontract-common/src/contract.rs

@@ -1,9 +1,16 @@
 //! Module for reading and examining data produced by truffle.
 
+use crate::abiext::FunctionExt;
+use crate::hash::H32;
 use crate::Abi;
 use crate::{bytecode::Bytecode, DeploymentInformation};
+use ethabi::ethereum_types::H256;
+use serde::Deserializer;
+use serde::Serializer;
 use serde::{Deserialize, Serialize};
-use std::collections::HashMap;
+use std::collections::{BTreeMap, HashMap};
+use std::hash::Hash;
+use std::sync::Arc;
 use web3::types::Address;
 
 /// Represents a contract data.
@@ -13,8 +20,9 @@ pub struct Contract {
     /// The contract name. Unnamed contracts have an empty string as their name.
     #[serde(rename = "contractName")]
     pub name: String,
-    /// The contract ABI
-    pub abi: Abi,
+    /// The contract interface.
+    #[serde(rename = "abi")]
+    pub interface: Arc<Interface>,
     /// The contract deployment bytecode.
     pub bytecode: Bytecode,
     /// The contract's expected deployed bytecode.
@@ -28,6 +36,71 @@ pub struct Contract {
     pub userdoc: Documentation,
 }
 
+/// Struct representing publicly accessible interface of a smart contract.
+#[derive(Clone, Debug, Default, PartialEq)]
+pub struct Interface {
+    /// The contract ABI
+    pub abi: Abi,
+    /// A mapping from method signature to a name-index pair for accessing
+    /// functions in the contract ABI. This is used to avoid allocation when
+    /// searching for matching functions by signature.
+    pub methods: HashMap<H32, (String, usize)>,
+    /// A mapping from event signature to a name-index pair for resolving
+    /// events in the contract ABI.
+    pub events: HashMap<H256, (String, usize)>,
+}
+
+impl<'de> Deserialize<'de> for Interface {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let abi = Abi::deserialize(deserializer)?;
+        Ok(abi.into())
+    }
+}
+
+impl Serialize for Interface {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        self.abi.serialize(serializer)
+    }
+}
+
+impl From<Abi> for Interface {
+    fn from(abi: Abi) -> Self {
+        Self {
+            methods: create_mapping(&abi.functions, |function| function.selector()),
+            events: create_mapping(&abi.events, |event| event.signature()),
+            abi,
+        }
+    }
+}
+
+/// Utility function for creating a mapping between a unique signature and a
+/// name-index pair for accessing contract ABI items.
+fn create_mapping<T, S, F>(
+    elements: &BTreeMap<String, Vec<T>>,
+    signature: F,
+) -> HashMap<S, (String, usize)>
+where
+    S: Hash + Eq + Ord,
+    F: Fn(&T) -> S,
+{
+    let signature = &signature;
+    elements
+        .iter()
+        .flat_map(|(name, sub_elements)| {
+            sub_elements
+                .iter()
+                .enumerate()
+                .map(move |(index, element)| (signature(element), (name.to_owned(), index)))
+        })
+        .collect()
+}
+
 impl Contract {
     /// Creates an empty contract instance.
     pub fn empty() -> Self {
@@ -38,7 +111,7 @@ impl Contract {
     pub fn with_name(name: impl Into<String>) -> Self {
         Contract {
             name: name.into(),
-            abi: Default::default(),
+            interface: Default::default(),
             bytecode: Default::default(),
             deployed_bytecode: Default::default(),
             networks: HashMap::new(),

ethcontract-derive/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ethcontract-derive"
-version = "0.25.5"
+version = "0.25.6"
 authors = ["Gnosis developers <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
@@ -20,8 +20,8 @@ proc-macro = true
 
 [dependencies]
 anyhow = "1.0"
-ethcontract-common = { version = "0.25.5", path = "../ethcontract-common" }
-ethcontract-generate = { version = "0.25.5", path = "../ethcontract-generate", default-features = false }
+ethcontract-common = { version = "0.25.6", path = "../ethcontract-common" }
+ethcontract-generate = { version = "0.25.6", path = "../ethcontract-generate", default-features = false }
 proc-macro2 = "1.0"
 quote = "1.0"
 syn = "2.0"

ethcontract-generate/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ethcontract-generate"
-version = "0.25.5"
+version = "0.25.6"
 authors = ["Gnosis developers <[email protected]>"]
 edition = "2021"
 license = "MIT OR Apache-2.0"
@@ -18,7 +18,7 @@ http = ["curl"]
 [dependencies]
 anyhow = "1.0"
 curl = { version = "0.4", optional = true }
-ethcontract-common = { version = "0.25.5", path = "../ethcontract-common" }
+ethcontract-common = { version = "0.25.6", path = "../ethcontract-common" }
 Inflector = "0.11"
 proc-macro2 = "1.0"
 quote = "1.0"

ethcontract-generate/src/generate/common.rs

@@ -52,7 +52,7 @@ pub(crate) fn expand(cx: &Context) -> TokenStream {
                 lazy_static! {
                     pub static ref CONTRACT: Contract = {
                         #[allow(unused_mut)]
-                        let mut contract = TruffleLoader::new()
+                        let mut contract: Contract = TruffleLoader::new()
                             .load_contract_from_str(#contract_json)
                             .expect("valid contract JSON");
                         #( #deployments )*
@@ -146,8 +146,8 @@ pub(crate) fn expand(cx: &Context) -> TokenStream {
 
                 let transport = DynTransport::new(web3.transport().clone());
                 let web3 = Web3::new(transport);
-                let abi = Self::raw_contract().abi.clone();
-                let instance = Instance::with_deployment_info(web3, abi, address, deployment_information);
+                let interface = Self::raw_contract().interface.clone();
+                let instance = Instance::with_deployment_info(web3, interface, address, deployment_information);
 
                 Contract::from_raw(instance)
             }

ethcontract-generate/src/generate/deployment.rs

@@ -80,7 +80,7 @@ fn expand_deploy(cx: &Context) -> Result<TokenStream> {
     //   can't seem to get truffle to output it
     let doc = util::expand_doc("Generated by `ethcontract`");
 
-    let (input, arg) = match cx.contract.abi.constructor() {
+    let (input, arg) = match cx.contract.interface.abi.constructor() {
         Some(constructor) => (
             methods::expand_inputs(&constructor.inputs)?,
             methods::expand_inputs_call_arg(&constructor.inputs),
@@ -190,7 +190,7 @@ fn expand_deploy(cx: &Context) -> Result<TokenStream> {
             }
 
             fn abi(_: &Self::Context) -> &self::ethcontract::common::Abi {
-                &Self::raw_contract().abi
+                &Self::raw_contract().interface.abi
             }
 
             fn from_deployment(