diff --git a/src/Governance.sol b/src/Governance.sol index a424a7a9..9f3c5e79 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -14,13 +14,13 @@ import {UserProxyFactory} from "./UserProxyFactory.sol"; import {add, max} from "./utils/Math.sol"; import {_requireNoDuplicates, _requireNoNegatives} from "./utils/UniqueArray.sol"; -import {Multicall} from "./utils/Multicall.sol"; +import {MultiDelegateCall} from "./utils/MultiDelegateCall.sol"; import {WAD, PermitParams} from "./utils/Types.sol"; import {safeCallWithMinGas} from "./utils/SafeCallMinGas.sol"; import {Ownable} from "./utils/Ownable.sol"; /// @title Governance: Modular Initiative based Governance -contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, Ownable, IGovernance { +contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Ownable, IGovernance { using SafeERC20 for IERC20; uint256 constant MIN_GAS_TO_HOOK = 350_000; diff --git a/src/interfaces/IMultiDelegateCall.sol b/src/interfaces/IMultiDelegateCall.sol new file mode 100644 index 00000000..6c042453 --- /dev/null +++ b/src/interfaces/IMultiDelegateCall.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +interface IMultiDelegateCall { + /// @notice Call multiple functions of the contract while preserving `msg.sender` + /// @param inputs Function calls to perform, encoded using `abi.encodeCall()` or equivalent + /// @return returnValues Raw data returned by each call + function multiDelegateCall(bytes[] calldata inputs) external returns (bytes[] memory returnValues); +} diff --git a/src/interfaces/IMulticall.sol b/src/interfaces/IMulticall.sol deleted file mode 100644 index 8227fdc9..00000000 --- a/src/interfaces/IMulticall.sol +++ /dev/null @@ -1,13 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.24; - -/// Copied from: https://github.com/Uniswap/v3-periphery/blob/main/contracts/interfaces/IMulticall.sol -/// @title Multicall interface -/// @notice Enables calling multiple methods in a single call to the contract -interface IMulticall { - /// @notice Call multiple functions in the current contract and return the data from all of them if they all succeed - /// @dev The `msg.value` should not be trusted for any method callable from multicall. - /// @param data The encoded function data for each of the calls to make to this contract - /// @return results The results from each of the calls passed in via data - function multicall(bytes[] calldata data) external payable returns (bytes[] memory results); -} diff --git a/src/utils/MultiDelegateCall.sol b/src/utils/MultiDelegateCall.sol new file mode 100644 index 00000000..cd6b701b --- /dev/null +++ b/src/utils/MultiDelegateCall.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import {IMultiDelegateCall} from "../interfaces/IMultiDelegateCall.sol"; + +contract MultiDelegateCall is IMultiDelegateCall { + /// @inheritdoc IMultiDelegateCall + function multiDelegateCall(bytes[] calldata inputs) external returns (bytes[] memory returnValues) { + returnValues = new bytes[](inputs.length); + + for (uint256 i; i < inputs.length; ++i) { + (bool success, bytes memory returnData) = address(this).delegatecall(inputs[i]); + + if (!success) { + // Bubble up the revert + assembly { + revert( + add(32, returnData), // offset (skip first 32 bytes, where the size of the array is stored) + mload(returnData) // size + ) + } + } + + returnValues[i] = returnData; + } + } +} diff --git a/src/utils/Multicall.sol b/src/utils/Multicall.sol deleted file mode 100644 index f5752815..00000000 --- a/src/utils/Multicall.sol +++ /dev/null @@ -1,28 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-or-later -pragma solidity ^0.8.24; - -import {IMulticall} from "../interfaces/IMulticall.sol"; - -/// Copied from: https://github.com/Uniswap/v3-periphery/blob/main/contracts/base/Multicall.sol -/// @title Multicall -/// @notice Enables calling multiple methods in a single call to the contract -abstract contract Multicall is IMulticall { - /// @inheritdoc IMulticall - function multicall(bytes[] calldata data) public payable override returns (bytes[] memory results) { - results = new bytes[](data.length); - for (uint256 i = 0; i < data.length; i++) { - (bool success, bytes memory result) = address(this).delegatecall(data[i]); - - if (!success) { - // Next 5 lines from https://ethereum.stackexchange.com/a/83577 - if (result.length < 68) revert(); - assembly { - result := add(result, 0x04) - } - revert(abi.decode(result, (string))); - } - - results[i] = result; - } - } -} diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 94c12386..7c31f19d 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -1446,7 +1446,7 @@ abstract contract GovernanceTest is Test { ); data[6] = abi.encodeWithSignature("resetAllocations(address[],bool)", initiatives, true); data[7] = abi.encodeWithSignature("withdrawLQTY(uint88)", lqtyAmount); - bytes[] memory response = governance.multicall(data); + bytes[] memory response = governance.multiDelegateCall(data); (uint88 allocatedLQTY,) = abi.decode(response[3], (uint88, uint120)); assertEq(allocatedLQTY, lqtyAmount); diff --git a/test/MultiDelegateCall.t.sol b/test/MultiDelegateCall.t.sol new file mode 100644 index 00000000..b09e359a --- /dev/null +++ b/test/MultiDelegateCall.t.sol @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.24; + +import {Test} from "forge-std/Test.sol"; +import {stdError} from "forge-std/StdError.sol"; +import {MultiDelegateCall} from "../src/utils/MultiDelegateCall.sol"; + +contract Target is MultiDelegateCall { + error CustomError(string); + + function id(bytes calldata x) external pure returns (bytes calldata) { + return x; + } + + function revertWithMessage(string calldata message) external pure { + revert(message); + } + + function revertWithCustomError(string calldata message) external pure { + revert CustomError(message); + } + + function panicWithArithmeticError() external pure returns (int256) { + return -type(int256).min; + } +} + +contract MultiDelegateCallTest is Test { + function test_CallsAllInputsAndAggregatesResults() external { + Target target = new Target(); + + bytes[] memory inputValues = new bytes[](3); + inputValues[0] = abi.encode("asd", 123); + inputValues[1] = abi.encode("fgh", 456); + inputValues[2] = abi.encode("jkl", 789); + + bytes[] memory inputs = new bytes[](3); + inputs[0] = abi.encodeCall(target.id, (inputValues[0])); + inputs[1] = abi.encodeCall(target.id, (inputValues[1])); + inputs[2] = abi.encodeCall(target.id, (inputValues[2])); + + bytes[] memory returnValues = target.multiDelegateCall(inputs); + assertEq(returnValues.length, inputs.length, "returnValues.length != inputs.length"); + + assertEq(abi.decode(returnValues[0], (bytes)), inputValues[0], "returnValues[0]"); + assertEq(abi.decode(returnValues[1], (bytes)), inputValues[1], "returnValues[1]"); + assertEq(abi.decode(returnValues[2], (bytes)), inputValues[2], "returnValues[2]"); + } + + function test_StopsAtFirstRevertAndBubblesItUp() external { + Target target = new Target(); + + bytes[] memory inputs = new bytes[](3); + inputs[0] = abi.encodeCall(target.id, ("asd")); + inputs[1] = abi.encodeCall(target.revertWithMessage, ("fgh")); + inputs[2] = abi.encodeCall(target.revertWithMessage, ("jkl")); + + vm.expectRevert(bytes("fgh")); + target.multiDelegateCall(inputs); + } + + function test_CanBubbleCustomError() external { + Target target = new Target(); + + bytes[] memory inputs = new bytes[](3); + inputs[0] = abi.encodeCall(target.id, ("asd")); + inputs[1] = abi.encodeCall(target.revertWithCustomError, ("fgh")); + inputs[2] = abi.encodeCall(target.revertWithMessage, ("jkl")); + + vm.expectRevert(abi.encodeWithSelector(Target.CustomError.selector, "fgh")); + target.multiDelegateCall(inputs); + } + + function test_CanBubblePanic() external { + Target target = new Target(); + + bytes[] memory inputs = new bytes[](3); + inputs[0] = abi.encodeCall(target.id, ("asd")); + inputs[1] = abi.encodeCall(target.panicWithArithmeticError, ()); + inputs[2] = abi.encodeCall(target.revertWithMessage, ("jkl")); + + vm.expectRevert(stdError.arithmeticError); + target.multiDelegateCall(inputs); + } +}