multicall: bubble up revert reason

.forge-snapshots/PositionManager_burn_empty.snap

@@ -1 +1 @@
-46767
+46764

.forge-snapshots/PositionManager_burn_empty_native.snap

@@ -1 +1 @@
-46584
+46582

.forge-snapshots/PositionManager_burn_nonEmpty.snap

@@ -1 +1 @@
-129412
+129410

.forge-snapshots/PositionManager_burn_nonEmpty_native.snap

@@ -1 +1 @@
-122334
+122332

.forge-snapshots/PositionManager_collect.snap

@@ -1 +1 @@
-149447
+149444

.forge-snapshots/PositionManager_collect_native.snap

@@ -1 +1 @@
-140599
+140596

.forge-snapshots/PositionManager_collect_sameRange.snap

@@ -1 +1 @@
-149447
+149444

.forge-snapshots/PositionManager_decreaseLiquidity.snap

@@ -1 +1 @@
-115032
+115030

.forge-snapshots/PositionManager_decreaseLiquidity_native.snap

@@ -1 +1 @@
-107954
+107952

.forge-snapshots/PositionManager_decrease_burnEmpty.snap

@@ -1 +1 @@
-133172
+133169

.forge-snapshots/PositionManager_decrease_burnEmpty_native.snap

@@ -1 +1 @@
-125911
+125908

.forge-snapshots/PositionManager_decrease_sameRange_allLiquidity.snap

@@ -1 +1 @@
-127706
+127703

.forge-snapshots/PositionManager_increaseLiquidity_erc20.snap

@@ -1 +1 @@
-150926
+150923

.forge-snapshots/PositionManager_increaseLiquidity_native.snap

@@ -1 +1 @@
-132726
+132723

.forge-snapshots/PositionManager_increase_autocompoundExactUnclaimedFees.snap

@@ -1 +1 @@
-133653
+133650

.forge-snapshots/PositionManager_increase_autocompoundExcessFeesCredit.snap

@@ -1 +1 @@
-169809
+169806

.forge-snapshots/PositionManager_mint.snap

@@ -1 +1 @@
-370961
+370958

.forge-snapshots/PositionManager_mint_native.snap

@@ -1 +1 @@
-335661
+335658

.forge-snapshots/PositionManager_mint_nativeWithSweep.snap

@@ -1 +1 @@
-344170
+344167

.forge-snapshots/PositionManager_mint_onSameTickLower.snap

@@ -1 +1 @@
-313643
+313640

.forge-snapshots/PositionManager_mint_onSameTickUpper.snap

@@ -1 +1 @@
-314285
+314282

.forge-snapshots/PositionManager_mint_sameRange.snap

@@ -1 +1 @@
-239867
+239864

.forge-snapshots/PositionManager_mint_settleWithBalance_sweep.snap

@@ -1 +1 @@
-368863
+368860

.forge-snapshots/PositionManager_mint_warmedPool_differentRange.snap

@@ -1 +1 @@
-319661
+319658

.forge-snapshots/PositionManager_multicall_initialize_mint.snap

@@ -1 +1 @@
-415324
+415321

src/base/Multicall.sol

@@ -1,30 +1,28 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 pragma solidity ^0.8.19;
 
+import {CustomRevert} from "@uniswap/v4-core/src/libraries/CustomRevert.sol";
+
 import {IMulticall} from "../interfaces/IMulticall.sol";
 
+import "forge-std/console2.sol";
+
 /// @title Multicall
 /// @notice Enables calling multiple methods in a single call to the contract
 abstract contract Multicall is IMulticall {
+    using CustomRevert for bytes4;
+
     /// @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) {
-                // handle custom errors
-                if (result.length == 4) {
-                    assembly {
-                        revert(add(result, 0x20), mload(result))
-                    }
-                }
-                // Next 5 lines from https://ethereum.stackexchange.com/a/83577
-                if (result.length < 68) revert();
+                // bubble up the revert reason
                 assembly {
-                    result := add(result, 0x04)
+                    revert(add(result, 0x20), mload(result))
                 }
-                revert(abi.decode(result, (string)));
             }
 
             results[i] = result;

src/interfaces/IMulticall.sol

@@ -4,6 +4,8 @@ pragma solidity ^0.8.19;
 /// @title Multicall interface
 /// @notice Enables calling multiple methods in a single call to the contract
 interface IMulticall {
+    error CallFailed(bytes revertReason);
+
     /// @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

test/Multicall.t.sol

@@ -2,7 +2,7 @@
 pragma solidity ^0.8.20;
 
 import "forge-std/Test.sol";
-import {MockMulticall} from "./mocks/MockMulticall.sol";
+import {MockMulticall, RevertContract} from "./mocks/MockMulticall.sol";
 
 contract MulticallTest is Test {
     MockMulticall multicall;
@@ -30,7 +30,7 @@ contract MulticallTest is Test {
     function test_multicall_firstRevert() public {
         bytes[] memory calls = new bytes[](2);
         calls[0] =
-            abi.encodeWithSelector(MockMulticall(multicall).functionThatRevertsWithError.selector, "First call failed");
+            abi.encodeWithSelector(MockMulticall(multicall).functionThatRevertsWithString.selector, "First call failed");
         calls[1] = abi.encodeWithSelector(MockMulticall(multicall).functionThatReturnsTuple.selector, 1, 2);
 
         vm.expectRevert("First call failed");
@@ -40,8 +40,9 @@ contract MulticallTest is Test {
     function test_multicall_secondRevert() public {
         bytes[] memory calls = new bytes[](2);
         calls[0] = abi.encodeWithSelector(MockMulticall(multicall).functionThatReturnsTuple.selector, 1, 2);
-        calls[1] =
-            abi.encodeWithSelector(MockMulticall(multicall).functionThatRevertsWithError.selector, "Second call failed");
+        calls[1] = abi.encodeWithSelector(
+            MockMulticall(multicall).functionThatRevertsWithString.selector, "Second call failed"
+        );
 
         vm.expectRevert("Second call failed");
         multicall.multicall(calls);
@@ -106,4 +107,55 @@ contract MulticallTest is Test {
         assertEq(multicall.msgValue(), 100);
         assertEq(multicall.msgValueDouble(), 200);
     }
+
+    // revert bubbling
+    function test_multicall_bubbleRevert_string() public {
+        bytes[] memory calls = new bytes[](1);
+        calls[0] =
+            abi.encodeWithSelector(MockMulticall(multicall).functionThatRevertsWithString.selector, "errorString");
+
+        vm.expectRevert("errorString");
+        multicall.multicall(calls);
+    }
+
+    function test_multicall_bubbleRevert_simpleError() public {
+        bytes[] memory calls = new bytes[](1);
+        calls[0] = abi.encodeWithSelector(MockMulticall(multicall).functionThatRevertsWithSimpleError.selector);
+
+        vm.expectRevert(MockMulticall.SimpleError.selector);
+        multicall.multicall(calls);
+    }
+
+    function test_multicall_bubbleRevert_errorWithParams(uint256 a, uint256 b) public {
+        bytes[] memory calls = new bytes[](1);
+        calls[0] =
+            abi.encodeWithSelector(MockMulticall(multicall).functionThatRevertsWithErrorWithParams.selector, a, b);
+
+        vm.expectRevert(abi.encodeWithSelector(MockMulticall.ErrorWithParams.selector, a, b));
+        multicall.multicall(calls);
+    }
+
+    function test_multicall_bubbleRevert_externalRevertString() public {
+        bytes[] memory calls = new bytes[](2);
+        calls[0] = abi.encodeWithSelector(MockMulticall(multicall).externalRevertString.selector, "errorString");
+
+        vm.expectRevert("errorString");
+        multicall.multicall(calls);
+    }
+
+    function test_multicall_bubbleRevert_externalRevertSimple() public {
+        bytes[] memory calls = new bytes[](1);
+        calls[0] = abi.encodeWithSelector(MockMulticall(multicall).externalRevertError1.selector);
+
+        vm.expectRevert(RevertContract.Error1.selector);
+        multicall.multicall(calls);
+    }
+
+    function test_multicall_bubbleRevert_externalRevertWithParams(uint256 a, uint256 b) public {
+        bytes[] memory calls = new bytes[](1);
+        calls[0] = abi.encodeWithSelector(MockMulticall(multicall).externalRevertError2.selector, a, b);
+
+        vm.expectRevert(abi.encodeWithSelector(RevertContract.Error2.selector, a, b));
+        multicall.multicall(calls);
+    }
 }

test/mocks/MockMulticall.sol

@@ -3,7 +3,28 @@ pragma solidity ^0.8.20;
 
 import "../../src/base/Multicall.sol";
 
+/// @dev If MockMulticall is to PositionManager, then RevertContract is to PoolManager
+contract RevertContract {
+    error Error1();
+    error Error2(uint256 a, uint256 b);
+
+    function revertWithString(string memory error) external pure {
+        revert(error);
+    }
+
+    function revertWithError1() external pure {
+        revert Error1();
+    }
+
+    function revertWithError2(uint256 a, uint256 b) external pure {
+        revert Error2(a, b);
+    }
+}
+
 contract MockMulticall is Multicall {
+    error SimpleError();
+    error ErrorWithParams(uint256 a, uint256 b);
+
     struct Tuple {
         uint256 a;
         uint256 b;
@@ -12,10 +33,20 @@ contract MockMulticall is Multicall {
     uint256 public msgValue;
     uint256 public msgValueDouble;
 
-    function functionThatRevertsWithError(string memory error) external pure {
+    RevertContract public revertContract = new RevertContract();
+
+    function functionThatRevertsWithString(string memory error) external pure {
         revert(error);
     }
 
+    function functionThatRevertsWithSimpleError() external pure {
+        revert SimpleError();
+    }
+
+    function functionThatRevertsWithErrorWithParams(uint256 a, uint256 b) external pure {
+        revert ErrorWithParams(a, b);
+    }
+
     function functionThatReturnsTuple(uint256 a, uint256 b) external pure returns (Tuple memory tuple) {
         tuple = Tuple({a: a, b: b});
     }
@@ -31,4 +62,16 @@ contract MockMulticall is Multicall {
     function returnSender() external view returns (address) {
         return msg.sender;
     }
+
+    function externalRevertString(string memory error) external view {
+        revertContract.revertWithString(error);
+    }
+
+    function externalRevertError1() external view {
+        revertContract.revertWithError1();
+    }
+
+    function externalRevertError2(uint256 a, uint256 b) external view {
+        revertContract.revertWithError2(a, b);
+    }
 }

test/position-managers/PositionManager.multicall.t.sol

@@ -68,4 +68,31 @@ contract PositionManagerMulticallTest is Test, PosmTestSetup, LiquidityFuzzers {
         assertEq(result.amount0(), amountSpecified);
         assertGt(result.amount1(), 0);
     }
+
+    function test_multicall_bubbleRevert() public {
+        // charlie will attempt to decrease liquidity without approval
+        // posm's NotApproved(charlie) should bubble up through Multicall
+
+        PositionConfig memory config = PositionConfig({
+            poolKey: key,
+            tickLower: TickMath.minUsableTick(key.tickSpacing),
+            tickUpper: TickMath.maxUsableTick(key.tickSpacing)
+        });
+        uint256 tokenId = lpm.nextTokenId();
+        mint(config, 100e18, address(this), ZERO_BYTES);
+
+        Plan memory planner = Planner.init();
+        planner.add(Actions.DECREASE_LIQUIDITY, abi.encode(tokenId, config, 100e18, ZERO_BYTES));
+        bytes memory actions = planner.finalizeModifyLiquidity(config.poolKey);
+
+        // Use multicall to decrease liquidity
+        bytes[] memory calls = new bytes[](1);
+        calls[0] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, actions, _deadline);
+
+        address charlie = makeAddr("CHARLIE");
+        vm.startPrank(charlie);
+        vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, charlie));
+        lpm.multicall(calls);
+        vm.stopPrank();
+    }
 }