fix(contract): multicall decode error (#1907)

* wip

* use empty bytes for reverts

* minor improvements

* fix test

* use is_empty

* docs

* clippy

* fix: rust-analyzer bug

* revert rename

Author: DaniPopes

Diff for: ethers-contract/src/lib.rs

@@ -31,8 +31,8 @@ mod multicall;
 #[cfg(any(test, feature = "abigen"))]
 #[cfg_attr(docsrs, doc(cfg(feature = "abigen")))]
 pub use multicall::{
-    Multicall, MulticallContract, MulticallError, MulticallVersion, MULTICALL_ADDRESS,
-    MULTICALL_SUPPORTED_CHAIN_IDS,
+    multicall_contract, Call, Multicall, MulticallContract, MulticallError, MulticallVersion,
+    MULTICALL_ADDRESS, MULTICALL_SUPPORTED_CHAIN_IDS,
 };
 
 /// This module exposes low lever builder structures which are only consumed by the

Diff for: ethers-contract/src/multicall/mod.rs

@@ -1,17 +1,15 @@
+use crate::{
+    call::{ContractCall, ContractError},
+    Lazy,
+};
 use ethers_core::{
     abi::{AbiDecode, Detokenize, Function, Token},
     types::{Address, BlockNumber, Bytes, Chain, NameOrAddress, TxHash, H160, U256},
 };
 use ethers_providers::Middleware;
-
 use std::{convert::TryFrom, sync::Arc};
 
-use crate::{
-    call::{ContractCall, ContractError},
-    Lazy,
-};
-
-mod multicall_contract;
+pub mod multicall_contract;
 use multicall_contract::multicall_3::{
     Call as Multicall1Call, Call3 as Multicall3Call, Call3Value as Multicall3CallValue,
     Result as MulticallResult,
@@ -263,11 +261,12 @@ impl TryFrom<u8> for MulticallVersion {
 #[derive(Clone)]
 #[must_use = "Multicall does nothing unless you use `call` or `send`"]
 pub struct Multicall<M> {
+    /// The Multicall contract interface.
+    pub contract: MulticallContract<M>,
     version: MulticallVersion,
     legacy: bool,
     block: Option<BlockNumber>,
     calls: Vec<Call>,
-    contract: MulticallContract<M>,
 }
 
 // Manually implement Debug due to Middleware trait bounds.
@@ -655,15 +654,20 @@ impl<M: Middleware> Multicall<M> {
                     .iter()
                     .zip(&return_data)
                     .map(|(call, bytes)| {
-                        let mut tokens: Vec<Token> = call
-                            .function
-                            .decode_output(bytes.as_ref())
-                            .map_err(ContractError::DecodingError)?;
-                        Ok(match tokens.len() {
-                            0 => Token::Tuple(vec![]),
-                            1 => tokens.remove(0),
-                            _ => Token::Tuple(tokens),
-                        })
+                        // Always return an empty Bytes token for calls that return no data
+                        if bytes.is_empty() {
+                            Ok(Token::Bytes(Default::default()))
+                        } else {
+                            let mut tokens = call
+                                .function
+                                .decode_output(bytes)
+                                .map_err(ContractError::DecodingError)?;
+                            Ok(match tokens.len() {
+                                0 => Token::Tuple(vec![]),
+                                1 => tokens.remove(0),
+                                _ => Token::Tuple(tokens),
+                            })
+                        }
                     })
                     .collect::<Result<Vec<Token>, M>>()?
             }
@@ -674,14 +678,17 @@ impl<M: Middleware> Multicall<M> {
                 let return_data = call.call().await?;
                 self.calls
                     .iter()
-                    .zip(&return_data)
+                    .zip(return_data.into_iter())
                     .map(|(call, res)| {
-                        let ret = &res.return_data;
-                        let res_token: Token = if res.success {
+                        let bytes = &res.return_data;
+                        // Always return an empty Bytes token for calls that return no data
+                        let res_token: Token = if bytes.is_empty() {
+                            Token::Bytes(Default::default())
+                        } else if res.success {
                             // Decode using call.function
                             let mut res_tokens = call
                                 .function
-                                .decode_output(ret)
+                                .decode_output(bytes)
                                 .map_err(ContractError::DecodingError)?;
                             match res_tokens.len() {
                                 0 => Token::Tuple(vec![]),
@@ -702,16 +709,15 @@ impl<M: Middleware> Multicall<M> {
                             }
 
                             // Decode with "Error(string)" (0x08c379a0)
-                            if ret.len() >= 4 && ret[..4] == [0x08, 0xc3, 0x79, 0xa0] {
+                            if bytes.len() >= 4 && bytes[..4] == [0x08, 0xc3, 0x79, 0xa0] {
                                 Token::String(
-                                    String::decode(&ret[4..]).map_err(ContractError::AbiError)?,
+                                    String::decode(&bytes[4..]).map_err(ContractError::AbiError)?,
                                 )
-                            } else if ret.is_empty() {
-                                Token::String(String::new())
                             } else {
-                                Token::Bytes(ret.to_vec())
+                                Token::Bytes(bytes.to_vec())
                             }
                         };
+
                         // (bool, (...))
                         Ok(Token::Tuple(vec![Token::Bool(res.success), res_token]))
                     })
@@ -778,22 +784,15 @@ impl<M: Middleware> Multicall<M> {
         // Map the calls vector into appropriate types for `aggregate` function
         let calls: Vec<Multicall1Call> = self
             .calls
-            .iter()
-            .map(|call| Multicall1Call { target: call.target, call_data: call.data.clone() })
+            .clone()
+            .into_iter()
+            .map(|call| Multicall1Call { target: call.target, call_data: call.data })
             .collect();
 
         // Construct the ContractCall for `aggregate` function to broadcast the transaction
-        let mut contract_call = self.contract.aggregate(calls);
-
-        if let Some(block) = self.block {
-            contract_call = contract_call.block(block)
-        };
-
-        if self.legacy {
-            contract_call = contract_call.legacy();
-        };
+        let contract_call = self.contract.aggregate(calls);
 
-        contract_call
+        self.set_call_flags(contract_call)
     }
 
     /// v2
@@ -802,55 +801,41 @@ impl<M: Middleware> Multicall<M> {
         // Map the calls vector into appropriate types for `try_aggregate` function
         let calls: Vec<Multicall1Call> = self
             .calls
-            .iter()
+            .clone()
+            .into_iter()
             .map(|call| {
                 // Allow entire call failure if at least one call is allowed to fail.
                 // To avoid iterating multiple times, equivalent of:
                 // self.calls.iter().any(|call| call.allow_failure)
-                allow_failure = allow_failure || call.allow_failure;
-                Multicall1Call { target: call.target, call_data: call.data.clone() }
+                allow_failure |= call.allow_failure;
+                Multicall1Call { target: call.target, call_data: call.data }
             })
             .collect();
 
         // Construct the ContractCall for `try_aggregate` function to broadcast the transaction
-        let mut contract_call = self.contract.try_aggregate(!allow_failure, calls);
-
-        if let Some(block) = self.block {
-            contract_call = contract_call.block(block)
-        };
+        let contract_call = self.contract.try_aggregate(!allow_failure, calls);
 
-        if self.legacy {
-            contract_call = contract_call.legacy();
-        };
-
-        contract_call
+        self.set_call_flags(contract_call)
     }
 
     /// v3
     fn as_aggregate_3(&self) -> ContractCall<M, Vec<MulticallResult>> {
         // Map the calls vector into appropriate types for `aggregate_3` function
         let calls: Vec<Multicall3Call> = self
             .calls
-            .iter()
+            .clone()
+            .into_iter()
             .map(|call| Multicall3Call {
                 target: call.target,
-                call_data: call.data.clone(),
+                call_data: call.data,
                 allow_failure: call.allow_failure,
             })
             .collect();
 
         // Construct the ContractCall for `aggregate_3` function to broadcast the transaction
-        let mut contract_call = self.contract.aggregate_3(calls);
-
-        if let Some(block) = self.block {
-            contract_call = contract_call.block(block)
-        };
+        let contract_call = self.contract.aggregate_3(calls);
 
-        if self.legacy {
-            contract_call = contract_call.legacy();
-        };
-
-        contract_call
+        self.set_call_flags(contract_call)
     }
 
     /// v3 + values (only .send())
@@ -859,12 +844,13 @@ impl<M: Middleware> Multicall<M> {
         let mut total_value = U256::zero();
         let calls: Vec<Multicall3CallValue> = self
             .calls
-            .iter()
+            .clone()
+            .into_iter()
             .map(|call| {
                 total_value += call.value;
                 Multicall3CallValue {
                     target: call.target,
-                    call_data: call.data.clone(),
+                    call_data: call.data,
                     allow_failure: call.allow_failure,
                     value: call.value,
                 }
@@ -877,17 +863,22 @@ impl<M: Middleware> Multicall<M> {
         } else {
             // Construct the ContractCall for `aggregate_3_value` function to broadcast the
             // transaction
-            let mut contract_call = self.contract.aggregate_3_value(calls);
+            let contract_call = self.contract.aggregate_3_value(calls);
 
-            if let Some(block) = self.block {
-                contract_call = contract_call.block(block)
-            };
+            self.set_call_flags(contract_call).value(total_value)
+        }
+    }
 
-            if self.legacy {
-                contract_call = contract_call.legacy();
-            };
+    /// Sets the block and legacy flags on a [ContractCall] if they were set on Multicall.
+    fn set_call_flags<D: Detokenize>(&self, mut call: ContractCall<M, D>) -> ContractCall<M, D> {
+        if let Some(block) = self.block {
+            call = call.block(block);
+        }
 
-            contract_call.value(total_value)
+        if self.legacy {
+            call = call.legacy();
         }
+
+        call
     }
 }

Diff for: ethers-contract/src/multicall/multicall_contract.rs

@@ -4,24 +4,27 @@ pub mod multicall_3 {
     #![allow(dead_code)]
     #![allow(clippy::type_complexity)]
     #![allow(unused_imports)]
-    /// Some macros may expand into using `ethers_contract` instead of `crate`.
-    mod ethers_contract {
-        pub use crate::*;
-    }
 
     // This is a hack to guarantee all ethers-derive macros can find the types.
-    // See [`ethers_core::macros::determine_ethers_crates`]
+    // See [`ethers_core::macros::determine_ethers_crates`].
     #[doc(hidden)]
     mod ethers {
-        pub mod core {
-            pub use ethers_core::*;
-        }
         pub mod contract {
             pub use crate::*;
         }
+        pub mod core {
+            pub use ethers_core::*;
+        }
         pub mod providers {
             pub use ethers_providers::*;
         }
+        pub mod types {
+            pub use ethers_core::types::*;
+        }
+    }
+    #[doc(hidden)]
+    mod ethers_contract {
+        pub use crate::*;
     }
 
     use self::ethers_contract::{
@@ -30,12 +33,12 @@ pub mod multicall_3 {
     };
     use ethers_core::{
         abi::{Abi, Detokenize, InvalidOutputType, Token, Tokenizable},
-        types::*,
+        types::{Address, Bytes, U256},
     };
     use ethers_providers::Middleware;
+    use std::sync::Arc;
 
     #[doc = "Multicall3 was auto-generated with ethers-rs Abigen. More information at: https://github.com/gakonst/ethers-rs"]
-    use std::sync::Arc;
     # [rustfmt :: skip] const __ABI : & str = "[{\"type\":\"function\",\"name\":\"aggregate\",\"inputs\":[{\"internalType\":\"struct Multicall3.Call[]\",\"name\":\"calls\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"address\"},{\"type\":\"bytes\"}]}],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"blockNumber\",\"type\":\"uint256\"},{\"internalType\":\"bytes[]\",\"name\":\"returnData\",\"type\":\"bytes[]\"}],\"stateMutability\":\"payable\"},{\"type\":\"function\",\"name\":\"aggregate3\",\"inputs\":[{\"internalType\":\"struct Multicall3.Call3[]\",\"name\":\"calls\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"address\"},{\"type\":\"bool\"},{\"type\":\"bytes\"}]}],\"outputs\":[{\"internalType\":\"struct Multicall3.Result[]\",\"name\":\"returnData\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"bool\"},{\"type\":\"bytes\"}]}],\"stateMutability\":\"payable\"},{\"type\":\"function\",\"name\":\"aggregate3Value\",\"inputs\":[{\"internalType\":\"struct Multicall3.Call3Value[]\",\"name\":\"calls\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"address\"},{\"type\":\"bool\"},{\"type\":\"uint256\"},{\"type\":\"bytes\"}]}],\"outputs\":[{\"internalType\":\"struct Multicall3.Result[]\",\"name\":\"returnData\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"bool\"},{\"type\":\"bytes\"}]}],\"stateMutability\":\"payable\"},{\"type\":\"function\",\"name\":\"blockAndAggregate\",\"inputs\":[{\"internalType\":\"struct Multicall3.Call[]\",\"name\":\"calls\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"address\"},{\"type\":\"bytes\"}]}],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"blockNumber\",\"type\":\"uint256\"},{\"internalType\":\"bytes32\",\"name\":\"blockHash\",\"type\":\"bytes32\"},{\"internalType\":\"struct Multicall3.Result[]\",\"name\":\"returnData\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"bool\"},{\"type\":\"bytes\"}]}],\"stateMutability\":\"payable\"},{\"type\":\"function\",\"name\":\"getBasefee\",\"inputs\":[],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"basefee\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getBlockHash\",\"inputs\":[{\"internalType\":\"uint256\",\"name\":\"blockNumber\",\"type\":\"uint256\"}],\"outputs\":[{\"internalType\":\"bytes32\",\"name\":\"blockHash\",\"type\":\"bytes32\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getBlockNumber\",\"inputs\":[],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"blockNumber\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getChainId\",\"inputs\":[],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"chainid\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getCurrentBlockCoinbase\",\"inputs\":[],\"outputs\":[{\"internalType\":\"address\",\"name\":\"coinbase\",\"type\":\"address\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getCurrentBlockDifficulty\",\"inputs\":[],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"difficulty\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getCurrentBlockGasLimit\",\"inputs\":[],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"gaslimit\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getCurrentBlockTimestamp\",\"inputs\":[],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"timestamp\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getEthBalance\",\"inputs\":[{\"internalType\":\"address\",\"name\":\"addr\",\"type\":\"address\"}],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"balance\",\"type\":\"uint256\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"getLastBlockHash\",\"inputs\":[],\"outputs\":[{\"internalType\":\"bytes32\",\"name\":\"blockHash\",\"type\":\"bytes32\"}],\"stateMutability\":\"view\"},{\"type\":\"function\",\"name\":\"tryAggregate\",\"inputs\":[{\"internalType\":\"bool\",\"name\":\"requireSuccess\",\"type\":\"bool\"},{\"internalType\":\"struct Multicall3.Call[]\",\"name\":\"calls\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"address\"},{\"type\":\"bytes\"}]}],\"outputs\":[{\"internalType\":\"struct Multicall3.Result[]\",\"name\":\"returnData\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"bool\"},{\"type\":\"bytes\"}]}],\"stateMutability\":\"payable\"},{\"type\":\"function\",\"name\":\"tryBlockAndAggregate\",\"inputs\":[{\"internalType\":\"bool\",\"name\":\"requireSuccess\",\"type\":\"bool\"},{\"internalType\":\"struct Multicall3.Call[]\",\"name\":\"calls\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"address\"},{\"type\":\"bytes\"}]}],\"outputs\":[{\"internalType\":\"uint256\",\"name\":\"blockNumber\",\"type\":\"uint256\"},{\"internalType\":\"bytes32\",\"name\":\"blockHash\",\"type\":\"bytes32\"},{\"internalType\":\"struct Multicall3.Result[]\",\"name\":\"returnData\",\"type\":\"tuple[]\",\"components\":[{\"type\":\"bool\"},{\"type\":\"bytes\"}]}],\"stateMutability\":\"payable\"}]" ;
     #[doc = r" The parsed JSON-ABI of the contract."]
     pub static MULTICALL3_ABI: ethers_contract::Lazy<ethers_core::abi::Abi> =

Diff for: ethers-contract/tests/it/contract.rs

@@ -694,22 +694,22 @@ mod eth_tests {
             .clear_calls()
             .add_call(empty_revert.clone(), true)
             .add_call(empty_revert.clone(), true);
-        let res: ((bool, String), (bool, String)) = multicall.call().await.unwrap();
+        let res: ((bool, Bytes), (bool, Bytes)) = multicall.call().await.unwrap();
         assert!(!res.0 .0);
-        assert_eq!(res.0 .1, "");
+        assert_eq!(res.0 .1, Bytes::default());
 
         // string revert
         let string_revert =
             reverting_contract.method::<_, H256>("stringRevert", ("String".to_string())).unwrap();
         multicall.clear_calls().add_call(string_revert, true).add_call(empty_revert.clone(), true);
-        let res: ((bool, String), (bool, String)) = multicall.call().await.unwrap();
+        let res: ((bool, String), (bool, Bytes)) = multicall.call().await.unwrap();
         assert!(!res.0 .0);
         assert_eq!(res.0 .1, "String");
 
         // custom error revert
         let custom_error = reverting_contract.method::<_, H256>("customError", ()).unwrap();
         multicall.clear_calls().add_call(custom_error, true).add_call(empty_revert.clone(), true);
-        let res: ((bool, Bytes), (bool, String)) = multicall.call().await.unwrap();
+        let res: ((bool, Bytes), (bool, Bytes)) = multicall.call().await.unwrap();
         let selector = &keccak256("CustomError()")[..4];
         assert!(!res.0 .0);
         assert_eq!(res.0 .1.len(), 4);
@@ -723,7 +723,7 @@ mod eth_tests {
             .clear_calls()
             .add_call(custom_error_with_data, true)
             .add_call(empty_revert.clone(), true);
-        let res: ((bool, Bytes), (bool, String)) = multicall.call().await.unwrap();
+        let res: ((bool, Bytes), (bool, Bytes)) = multicall.call().await.unwrap();
         let selector = &keccak256("CustomErrorWithData(string)")[..4];
         assert!(!res.0 .0);
         assert_eq!(&res.0 .1[..4], selector);