mds1 / multicall

Multicall: Aggregate multiple constant function call results into one
https://multicall3.com
MIT License
955 stars 166 forks source link

meta: improvements for a potential Multicall4 #146

Open mds1 opened 1 year ago

mds1 commented 1 year ago

There are currently no active plans for Multicall4, but there are definitely some things that can be improved, so tracking these for a future version:

  1. When reverting, include original error data. We currently just revert with a string which makes it hard to determine the source of errors. Instead, revert with error MulticallFailed(uint256 callIndex, bytes revertData). This error can be easily decoded to get back both the revert data and the specific call that triggered it
  2. Include members for all data that can be retrieved in solidity. For example, getters for an address' code, code size, and code hash are currently not present, but could be. There are likely other getters missing also, need to come up with a comprehensive list here
dbeal-eth commented 1 year ago

another improvement to consider for Multicall4, you could have the contract be deployed with CREATE2 using arachnid https://github.com/Arachnid/deterministic-deployment-proxy

this way its very easy to deploy Multicall4 to their own chain using any address with gas token if for whatever reason the contract hasnt been deployed already. no need to open an issue on the repo.

mds1 commented 1 year ago

There is a presigned transaction in the README so users can deploy on EVM-compatible chains without opening an issue. I don't want to deploy using that deployer (or similar ones) because the way that deployer is deployed means that deployer can only be deployed on chains that (1) have the same gas metering as the EVM, and (2) don't require EIP-155. Arbitrum chains don't follow 1 (it was administratively placed there) and Celo doesn't follow 2. (Those are just examples, there are other chains like that also). Therefore Multicall would be limited to being deployed on chains with that deployer, or where you can convince the chain to place that deployer.

Given that, having a known private key + sharing a presigned transaction feels like the best compromise.

I'm also working on a new create2/create3 deployer that will be deployed with the same "known private key + sharing a presigned transaction" approach, then future contracts (like Multicall4) can just be deployed through that deployer

nontrivial44 commented 1 year ago

Is it possible to make it such that if one of the calls fails it doesn't make the whole multicall fail?

mds1 commented 1 year ago

Yes, there are multiple ways to do this, such as aggregate3 method with allowFailure set to false. Please see the readme and Multicall3 contract itself for more info

AdemKao commented 1 year ago

Hi @mds1 : Just wondering if MultiCall supports calling multiple write method which only return static state? For now I use single call to get uniswap 'collect' method. And I want to get multiple collects by using multicall. But I got the error which show 'not approved'

address='0xc36442b4a4522e871399cd717abdd847ab11fe88'
abi=[{
                "inputs": [
                    {
                        "components": [
                            {
                                "internalType": "uint256",
                                "name": "tokenId",
                                "type": "uint256"
                            },
                            {
                                "internalType": "address",
                                "name": "recipient",
                                "type": "address"
                            },
                            {
                                "internalType": "uint128",
                                "name": "amount0Max",
                                "type": "uint128"
                            },
                            {
                                "internalType": "uint128",
                                "name": "amount1Max",
                                "type": "uint128"
                            }
                        ],
                        "internalType": "struct INonfungiblePositionManager.CollectParams",
                        "name": "params",
                        "type": "tuple"
                    }
                ],
                "name": "collect",
                "outputs": [
                    {
                        "internalType": "uint256",
                        "name": "amount0",
                        "type": "uint256"
                    },
                    {
                        "internalType": "uint256",
                        "name": "amount1",
                        "type": "uint256"
                    }
                ],
                "stateMutability": "payable",
                "type": "function"
            }]
contract =w3.eth.contract(abi=abi, address=address)
params = (id, user_addr, self.MAX, self.MAX)
data = contract.functions.collect(params).call()
# it is successed

for id in ids:
            params = (id, user_addr, self.MAX, self.MAX)
            # print(params)
            # print('=================')
            encode_data.append((address,contract.encodeABI(fn_name="collect", args=[params])))

data = multicall_contract.functions.tryAggregate(False,encode_data).call()
# it shows Not approved
mds1 commented 1 year ago

not approved sounds like an error with your specific calls, since multicalls do work with any method type. I'm not familiar with the collect method, but if you need more help please open a separate issue as this is not related to a Multicall4 :)

joyjune970 commented 1 year ago

Is it possible to communicate with the contract through python's web3.py and use the Multicall3 function?

mds1 commented 1 year ago

Yes you can interact it with it just like you would with any other contract. If you have specific questions about web3.py I'd suggest opening issues in the web3.py repo as I'm not familiar with web3.py. If you have multicall-related issues please open a separate issue as this is not related to a Multicall4 :)

jaydenwindle commented 1 year ago

Would love to see an authenticated version of the aggregate function which appends msg.sender to the calldata for each call in the style of ERC-2771. This would allow recipient contracts to verify that the caller is authorized to perform actions on the contract within the context of a multicall. One use case for this is executing admin operations across multiple smart contracts in a single transaction.

I've implemented this behavior in a fork (multicall-authenticated), but would love to see it integrated into the main multicall contracts.

klepto commented 7 months ago

Similarly to allowFailure, it would be cool if it was possible to specify gas limit for each individual call. As it is right now, one malicious call can still make entire request fail, which is exactly what happened in my use-case.

rmcmk commented 7 months ago

The above is greatly requested. Basically any multicall is susceptible to denial of service due to a malicious or poorly implemented contracts. Gas limit per call/per request would be a very welcome addition.

I am willing to implement this if there is interest. We already maintain our own version of batched reads that implements a similar feature.

@mds1

geekberryonekey commented 3 months ago

Oh! it is the same propose with klepto commented on Feb 27


how about new Call struct with gas limit.

We often use multicall to query balances in bulk, such as obtaining the balance of a user's address across multiple contract addresses. However, due to reasons like contract issues, some calls may exhaust gas, causing the entire multicall to fail. If the caller can set a reasonable gas limit for each individual call, and any call exceeding this gas limit is considered failed, it would greatly improve the issues caused by the aforementioned scenario.

fake code:

pragma solidity ^0.8.0;

contract Caller {
 struct Call3Gas {
    address target;
    bool allowFailure;
    uint16 gas;
    bytes callData;
  }

  function aggregate3Gas(Call3Gas call) public returns (bool, bytes memory) {
         ...
         (result.success, result.returnData) = calli.target.call{gas: call.gas}(calli.callData);
         ...
    }
}
mds1 commented 2 months ago

Even when specifying an amount of gas to forward, you'd still be susceptible to DoS attacks because of return data bombs that can still forcibly use the rest of your gas. This Call3Gas struct would also need to specify a cap on the amount of return data to copy to memory to fully mitigate the issue. See https://github.com/nomad-xyz/ExcessivelySafeCall to learn more