Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta: improvements for a potential Multicall4 #146

Open
mds1 opened this issue Aug 10, 2023 · 13 comments
Open

meta: improvements for a potential Multicall4 #146

mds1 opened this issue Aug 10, 2023 · 13 comments

Comments

@mds1
Copy link
Owner

mds1 commented Aug 10, 2023

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
@mds1 mds1 mentioned this issue Aug 10, 2023
@dbeal-eth
Copy link

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
Copy link
Owner Author

mds1 commented Aug 10, 2023

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
Copy link

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

@mds1
Copy link
Owner Author

mds1 commented Aug 14, 2023

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
Copy link

AdemKao commented Aug 17, 2023

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
Copy link
Owner Author

mds1 commented Aug 23, 2023

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
Copy link

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

@mds1
Copy link
Owner Author

mds1 commented Oct 6, 2023

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
Copy link

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
Copy link

klepto commented Feb 26, 2024

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
Copy link

rmcmk commented Feb 26, 2024

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

@mds1 mds1 mentioned this issue May 8, 2024
@geekberryonekey
Copy link

geekberryonekey commented Jul 5, 2024

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
Copy link
Owner Author

mds1 commented Jul 28, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants