Multicall module

[frangio]

This is a very smart piece of code found in the ENS repository that we think could be a useful feature for people to plug into their contracts. I suppose this would be the utils/Multicall module, unless we come up with another name!

    function multicall(bytes[] calldata data) external returns(bytes[] memory results) {
        results = new bytes[](data.length);
        for(uint i = 0; i < data.length; i++) {
            (bool success, bytes memory result) = address(this).delegatecall(data[i]);
            require(success);
            results[i] = result;
        }
        return results;
    }

[martriay]

Interesting, will tackle it!

[Amxx]

could be interresting to use Address.functionDelegateCall so revert reasons are bubbled

[Amxx]

FYI: There is a proposal that multicall like design could be used to implement callbacks mechanism that are missing to ERC20.
The idea being to delegate call to self in to do some operation (such as approve) and then do a call to a recipent (which could do a subsequent transferFrom)

interface IERCXXXXReceiver {
    function receiveControl(address sender, bytes calldata thisCall) external returns(bytes4);
}

abstract contract Multicall is Context {
    function chainMulticall(bytes[] calldata localCalls, address receiver,  bytes calldata remoteCall) external {
        for(uint i = 0; i < localCalls.length; ++i) {
            Address.functionDelegateCall(address(this), localCalls[i]);
        }
        require(
            receiver == address(0)
            ||
            IERCXXXXReceiver(receiver).receiveControl(_msgSender(), remoteCall)
            ==
            IERCXXXXReceiver(receiver).receiveControl.selector,
            "Remote call failed"
        );
    }
}

[frangio]

Everyone seems to easily confuse this contract with a MultiSend contract. I'm thinking maybe we should call this module something related to "batch" or "batching", like Batch with a batch(...) function?

[alcueca]

There is Batchable.sol from @boringcrypto which I quite like: https://github.com/boringcrypto/BoringSolidity/blob/master/contracts/BoringBatchable.sol

The variation with a callback at the end was my idea, but after tinkering with this and with EIP2271 in the horizon I don't think it would be worth it. Too rigid and complex of a pattern.

[zemse]

The BoringBatchable seems to have a better multicall. Actually, I'm planning to work on a multicall provider (ethers-io/ethers.js#788). I think it would be helpful if the multicall function allows to not entirely revert if a single call fails.

[frangio]

IMO neither option is better than the other, they offer different semantics that will be more appropriate in different scenarios. It seems to me that calling all functions regardless of reverts is more appropriate for queries.

Given that the multicall we're adding is not view, it will be used to batch mutating function calls. As far as I can tell making the entire batch atomic will be the generally expected behavior and is likely to avoid bugs.

For querying it's also not necessary to have multicall embedded in the target contract, a separate "multisend" contract can be used as a forwarding proxy.

[mds1]

Given that the multicall we're adding is not view, it will be used to batch mutating function calls. As far as I can tell making the entire batch atomic will be the generally expected behavior and is likely to avoid bugs.

One counter here is that there are times when you'd want to batch mutating function calls, but don't want everything to revert if one fails. For example, say this was integrated into a system like Compound and I want to batch these three transactions:

1. Repay my DAI debt on Compound
2. Redeem my supplied ETH collateral
3. Claim my COMP

But if claiming COMP fails (e.g. the Comptroller has insufficient balance to give me the COMP I'm owed), I still want transactions 1 and 2 to succeed.

Given that, another option I wanted to throw out is a pattern like the below so you can explicitly specify which transaction can be safely ignored on failure:

/// @notice Required parameters for each call
struct Call {
  bytes data; // calldata to use
  bool requireSuccess; // true if whole tx should revert on failure, false if we ignore when this call fails
}

/// @notice When a call fails, but has requireSuccess as false, emit a failure log with the call's index
event CallFailed(uint256 callIndex);

/// @notice Batch multiple calls to this contract
function multicall(Call[] memory calls) external payable returns (bytes[] memory) {
  bytes[] memory returnData = new bytes[](calls.length);

  for (uint256 i = 0; i < calls.length; i++) {
    (bool success, bytes memory response) = address(this).delegatecall(calls[i].data);

    if (!success && calls[i].requireSuccess) {
      revert(getRevertMessage(response)); 
    } else if (!success) {
      emit CallFailed(i); // don't revert, but emit failure log to make it easier to identify failed calls
    }
    returnData[i] = response;
  }
  return returnData;
}

[boringcrypto]

There are clearly cases in which you want to revert on ANY failure and cases where you want to ignore reverts. BoringBatchable has a revertOnFail flag for this. Although I also like the per call approach. Overhead is minimal, so I'd include this. Also the CallFailed event is a nice one. I'll steal these ideas ;)

[frangio]

Thanks everyone for the input.

We understand that there will be use cases that would benefit from ignoring reverts, but we still think that the default of failing on reverts will be what's needed in most cases, and it is very clearly to us the safest option in general. We evaluated adding a boolean as suggested, but we think this will fall short in many cases, and anything more complex should probably be coded as a smart contract to be used directly or delegating to it from a smart wallet.

There is also the fact that this same function is already present in ENS and Uniswap v3 Periphery, and being compatible with those is very appealing to us because this is the kind of contract that we think will benefit from being a widespread standard.