Fix error handling in multicall and move fn to own contract

contracts/interfaces/IStakingPool.sol

@@ -84,8 +84,6 @@ interface IStakingPool {
     string memory ipfsDescriptionHash
   ) external;
 
-  function multicall(bytes[] calldata data) external returns (bytes[] memory results);
-
   function processExpirations(bool updateUntilCurrentTimestamp) external;
 
   function requestAllocation(

contracts/mocks/MulticallMock.sol

@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-3.0-only
+pragma solidity ^0.8.0;
+
+import "../utils/MultiCallable.sol";
+
+contract MulticallMock is MultiCallable {
+    error EmptyCustomError();
+    error UintCustomError(uint errCode);
+    uint zero = 0;
+
+    function panicError() public view {
+       // use storage to trick compiler into thinking this makes sense
+       uint(100) / zero;
+    }
+
+    function emptyRevert() public pure {
+        revert();
+    }
+
+    function emptyRequire() public pure {
+        require(false);
+    }
+
+    function emptyCustomError() public pure {
+        revert EmptyCustomError();
+    }
+
+    function uintCustomError(uint errCode) public pure {
+        revert UintCustomError(errCode);
+    }
+
+    function stringRevert32() public pure {
+       require(false, "String revert");
+    }
+
+    function stringRevert64() public pure {
+       require(false, "012345678901234567890123456789012345678901234567890123456789001234567890");
+    }
+
+    function success() public pure returns (bool) {
+      return true;
+    }
+}

contracts/modules/staking/StakingPool.sol

@@ -2,6 +2,7 @@
 
 pragma solidity ^0.8.9;
 
+import "../../utils/MultiCallable.sol";
 import "../../interfaces/IStakingPool.sol";
 import "../../interfaces/IGovernance.sol";
 import "../../interfaces/ICover.sol";
@@ -21,7 +22,7 @@ import "./StakingTypesLib.sol";
 // on cover buys we allocate the available product capacity
 // on cover expiration we deallocate the capacity and it becomes available again
 
-contract StakingPool is IStakingPool {
+contract StakingPool is IStakingPool, MultiCallable {
   using StakingTypesLib for TrancheAllocationGroup;
   using StakingTypesLib for TrancheGroupBucket;
   using SafeUintCast for uint;
@@ -1646,23 +1647,4 @@ contract StakingPool is IStakingPool {
     return surgePremium / ALLOCATION_UNITS_PER_NXM;
   }
 
-  function multicall(bytes[] calldata data) external returns (bytes[] memory results) {
-
-    uint callCount = data.length;
-    results = new bytes[](callCount);
-
-    for (uint i = 0; i < callCount; i++) {
-      (bool ok, bytes memory result) = address(this).delegatecall(data[i]);
-
-      if (!ok) {
-        // https://ethereum.stackexchange.com/a/83577
-        if (result.length < 68) revert();
-        assembly { result := add(result, 0x04) }
-        revert(abi.decode(result, (string)));
-      }
-
-      results[i] = result;
-    }
-  }
-
 }

contracts/utils/MultiCallable.sol

@@ -0,0 +1,34 @@
+// SPDX-License-Identifier: GPL-3.0-only
+pragma solidity ^0.8.0;
+
+contract MultiCallable {
+  error RevertedWithoutReason(uint index);
+
+  // WARNING: Do not set this function as payable
+  function multicall(bytes[] calldata data) external returns (bytes[] memory results) {
+
+    uint callCount = data.length;
+    results = new bytes[](callCount);
+
+    for (uint i = 0; i < callCount; i++) {
+      (bool ok, bytes memory result) = address(this).delegatecall(data[i]);
+
+      uint length = result.length;
+
+      if (!ok) {
+
+        // 0 length returned from empty revert() / require(false)
+        if (length == 0) {
+          revert RevertedWithoutReason(i);
+        }
+
+        assembly {
+          result := add(result, 0x20)
+          revert(result, add(result, length))
+        }
+      }
+
+      results[i] = result;
+    }
+  }
+}

test/unit/StakingPool/depositTo.js

@@ -604,6 +604,24 @@ describe('depositTo', function () {
     ).to.be.revertedWith('NOT_MINTED');
   });
 
+  it('multicall should bubble up string revert', async function () {
+    const { stakingPool } = this;
+    const [user] = this.accounts.members;
+    const { amount, destination } = depositToFixture;
+
+    const invalidTokenId = 127;
+    const { firstActiveTrancheId } = await getTranches(DEFAULT_PERIOD, DEFAULT_GRACE_PERIOD);
+
+    const depositToData = stakingPool.interface.encodeFunctionData('depositTo', [
+      amount,
+      firstActiveTrancheId,
+      invalidTokenId,
+      destination,
+    ]);
+
+    await expect(stakingPool.connect(user).multicall([depositToData])).to.be.revertedWith('NOT_MINTED');
+  });
+
   it('should revert if trying to deposit, while nxm is locked for governance vote', async function () {
     const { stakingPool, nxm } = this;
     const [user] = this.accounts.members;
@@ -614,8 +632,16 @@ describe('depositTo', function () {
     // Simulate member vote lock
     await nxm.setLock(user.address, 1e6);
 
-    await expect(
-      stakingPool.connect(user).depositTo(amount, firstActiveTrancheId, tokenId, destination),
-    ).to.be.revertedWithCustomError(stakingPool, 'NxmIsLockedForGovernanceVote');
+    const depositToData = stakingPool.interface.encodeFunctionData('depositTo', [
+      amount,
+      firstActiveTrancheId,
+      tokenId,
+      destination,
+    ]);
+
+    await expect(stakingPool.connect(user).multicall([depositToData])).to.be.revertedWithCustomError(
+      stakingPool,
+      'NxmIsLockedForGovernanceVote',
+    );
   });
 });

test/unit/StakingPool/index.js

@@ -24,4 +24,5 @@ describe('StakingPool unit tests', function () {
   require('./setPoolPrivacy');
   require('./setProducts');
   require('./withdraw');
+  require('./multicall');
 });

test/unit/StakingPool/multicall.js

@@ -0,0 +1,60 @@
+const { ethers } = require('hardhat');
+const { expect } = require('chai');
+const { parseEther } = ethers.utils;
+const { DIVIDE_BY_ZERO } = require('../utils').errors;
+
+describe('Multicall unit tests', function () {
+  it('should bubble up empty custom error signatures', async function () {
+    const { multicall } = this;
+    const calldata = multicall.interface.encodeFunctionData('emptyCustomError');
+    await expect(multicall.multicall([calldata])).to.be.revertedWithCustomError(multicall, 'EmptyCustomError');
+  });
+
+  it('should bubble up custom error with 32 byte uint value', async function () {
+    const { multicall } = this;
+    const calldata = multicall.interface.encodeFunctionData('uintCustomError', [parseEther('3459802')]);
+    await expect(multicall.multicall([calldata])).to.be.revertedWithCustomError(multicall, 'UintCustomError', [
+      calldata,
+    ]);
+  });
+
+  it('should bubble up 32 byte string revert messages', async function () {
+    const { multicall } = this;
+    const calldata = multicall.interface.encodeFunctionData('stringRevert32');
+    await expect(multicall.multicall([calldata])).to.be.revertedWith('String revert');
+  });
+
+  it('should bubble up 64 byte string revert messages', async function () {
+    const { multicall } = this;
+    const calldata = multicall.interface.encodeFunctionData('stringRevert64');
+    await expect(multicall.multicall([calldata])).to.be.revertedWith(
+      '012345678901234567890123456789012345678901234567890123456789001234567890',
+    );
+  });
+
+  it('should bubble up panic error codes', async function () {
+    const { multicall } = this;
+    const calldata = multicall.interface.encodeFunctionData('panicError');
+    await expect(multicall.multicall([calldata])).to.be.revertedWithPanic(DIVIDE_BY_ZERO);
+  });
+
+  it('should bubble up first empty revert messages', async function () {
+    const { multicall } = this;
+    const calldata = [
+      multicall.interface.encodeFunctionData('emptyRevert'),
+      multicall.interface.encodeFunctionData('success'),
+      multicall.interface.encodeFunctionData('emptyRevert'),
+    ];
+    await expect(multicall.multicall(calldata)).to.be.revertedWithCustomError(multicall, 'RevertedWithoutReason', 0);
+  });
+
+  it('should bubble up empty require messages with correct index', async function () {
+    const { multicall } = this;
+    const calldata = [
+      multicall.interface.encodeFunctionData('success'), // 0
+      multicall.interface.encodeFunctionData('success'), // 1
+      multicall.interface.encodeFunctionData('emptyRequire'), // 2
+    ];
+    await expect(multicall.multicall(calldata)).to.be.revertedWithCustomError(multicall, 'RevertedWithoutReason', 2);
+  });
+});

test/unit/StakingPool/setup.js

@@ -14,6 +14,8 @@ async function setup() {
   const MCR = await ethers.getContractFactory('CoverMockMCR');
   const StakingPool = await ethers.getContractFactory('StakingPool');
 
+  const multicallMock = await ethers.deployContract('MulticallMock');
+
   const master = await MasterMock.deploy();
   await master.deployed();
 
@@ -105,6 +107,7 @@ async function setup() {
   const coverSigner = await ethers.getImpersonatedSigner(cover.address);
   await setEtherBalance(coverSigner.address, ethers.utils.parseEther('1'));
 
+  this.multicall = multicallMock;
   this.tokenController = tokenController;
   this.master = master;
   this.nxm = nxm;