Squashed commit of the following: (#980)

Author: nicholaspai

contracts/Polygon_SpokePool.sol

@@ -178,17 +178,22 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
 
     /**
      * @notice Override multicall so that it cannot include executeRelayerRefundLeaf
-     * as one of the calls combined with other public function calls.
+     * as one of the calls combined with other public function calls and blocks nested multicalls in general, which
+     * don't have any practical use case. We also block nested multicalls which could be used to bypass
+     * this check and there are no practical use cases for nested multicalls.
      * @dev Multicalling a single transaction will always succeed.
      * @dev Multicalling execute functions without combining other public function calls will succeed.
+     * @dev Nested multicalls will always fail.
      * @dev Multicalling public function calls without combining execute functions will succeed.
      */
     function _validateMulticallData(bytes[] calldata data) internal pure override {
         bool hasOtherPublicFunctionCall = false;
         bool hasExecutedLeafCall = false;
         for (uint256 i = 0; i < data.length; i++) {
             bytes4 selector = bytes4(data[i][:4]);
-            if (selector == SpokePoolInterface.executeRelayerRefundLeaf.selector) {
+            if (selector == MultiCallerUpgradeable.multicall.selector) {
+                revert MulticallExecuteLeaf();
+            } else if (selector == SpokePoolInterface.executeRelayerRefundLeaf.selector) {
                 if (hasOtherPublicFunctionCall) revert MulticallExecuteLeaf();
                 hasExecutedLeafCall = true;
             } else {
@@ -211,9 +216,11 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
     ) public payable override {
         // AddressLibUpgradeable.isContract isn't a sufficient check because it checks the contract code size of
         // msg.sender which is 0 if called from a constructor function on msg.sender. This is why we check if
-        // msg.sender is equal to tx.origin which is fine as long as Polygon supports the tx.origin opcode.
+        // msg.sender is equal to tx.origin which is fine as long as Polygon supports the tx.origin opcode. We also
+        // check if the msg.sender has delegated their code to a contract via EIP7702.
         // solhint-disable-next-line avoid-tx-origin
-        if (relayerRefundLeaf.amountToReturn > 0 && msg.sender != tx.origin) revert NotEOA();
+        if (relayerRefundLeaf.amountToReturn > 0 && (msg.sender != tx.origin || msg.sender.code.length > 0))
+            revert NotEOA();
         super.executeRelayerRefundLeaf(rootBundleId, relayerRefundLeaf, proof);
     }
 
@@ -238,6 +245,11 @@ contract Polygon_SpokePool is IFxMessageProcessor, SpokePool, CircleCCTPAdapter
     }
 
     function _bridgeTokensToHubPool(uint256 amountToReturn, address l2TokenAddress) internal override {
+        // WARNING: Withdrawing MATIC can result in the L1 PolygonTokenBridger.startExitWithBurntTokens() failing
+        // due to a MAX_LOGS constraint imposed by the ERC20Predicate, so if this SpokePool will be used to withdraw
+        // MATIC then additional constraints need to be imposed to limit the # of logs produed by the L2 withdrawal
+        // transaction. Currently, MATIC is not a supported token in Across for this SpokePool.
+
         // If the token is USDC, we need to use the CCTP bridge to transfer it to the hub pool.
         if (_isCCTPEnabled() && l2TokenAddress == address(usdcToken)) {
             _transferUsdc(withdrawalRecipient, amountToReturn);

test/evm/hardhat/chain-specific-spokepools/Polygon_SpokePool.ts

@@ -377,6 +377,79 @@ describe("Polygon Spoke Pool", function () {
       polygonSpokePool.connect(relayer).multicall([executeLeafData[0], fillData[0], executeLeafData[1], fillData[1]])
     ).to.be.reverted;
   });
+  it("Cannot use nested multicalls", async function () {
+    // In this test we attempt to stuff the `executeRelayerRefundLeaf` call inside a nested multicall to bypass
+    // the _validateMulticallData check in PolygonSpokePool.sol. This should not be possible.
+    const l2ChainId = await owner.getChainId();
+    const leaves = buildRelayerRefundLeaves(
+      [l2ChainId, l2ChainId], // Destination chain ID.
+      [ethers.constants.Zero, ethers.constants.Zero], // amountToReturn.
+      [dai.address, dai.address], // l2Token.
+      [[], []], // refundAddresses.
+      [[], []] // refundAmounts.
+    );
+    const tree = await buildRelayerRefundTree(leaves);
+
+    // Relay leaves to Spoke
+    const relayRootBundleData = polygonSpokePool.interface.encodeFunctionData("relayRootBundle", [
+      tree.getHexRoot(),
+      mockTreeRoot,
+    ]);
+    await polygonSpokePool.connect(fxChild).processMessageFromRoot(0, owner.address, relayRootBundleData);
+
+    // Deploy message handler and create fill with message that should succeed in isolation:
+    const acrossMessageHandler = await createFake("AcrossMessageHandlerMock");
+    await seedWallet(relayer, [dai], weth, toWei("2"));
+    await dai.connect(relayer).approve(polygonSpokePool.address, toWei("2"));
+
+    const executeLeafData = [
+      polygonSpokePool.interface.encodeFunctionData("executeRelayerRefundLeaf", [
+        0,
+        leaves[0],
+        tree.getHexProof(leaves[0]),
+      ]),
+      polygonSpokePool.interface.encodeFunctionData("executeRelayerRefundLeaf", [
+        0,
+        leaves[1],
+        tree.getHexProof(leaves[1]),
+      ]),
+    ];
+    const currentTime = (await polygonSpokePool.getCurrentTime()).toNumber();
+    const relayData: V3RelayData = {
+      depositor: addressToBytes(owner.address),
+      recipient: addressToBytes(acrossMessageHandler.address),
+      exclusiveRelayer: addressToBytes(zeroAddress),
+      inputToken: addressToBytes(dai.address),
+      outputToken: addressToBytes(dai.address),
+      inputAmount: toWei("1"),
+      outputAmount: toWei("1"),
+      originChainId: originChainId,
+      depositId: toBN(0),
+      fillDeadline: currentTime + 7200,
+      exclusivityDeadline: 0,
+      message: "0x1234",
+    };
+    const fillData = [
+      polygonSpokePool.interface.encodeFunctionData("fillRelay", [
+        relayData,
+        repaymentChainId,
+        addressToBytes(relayer.address),
+      ]),
+      polygonSpokePool.interface.encodeFunctionData("fillRelay", [
+        { ...relayData, depositId: 1 },
+        repaymentChainId,
+        addressToBytes(relayer.address),
+      ]),
+    ];
+
+    // Fills and execute leaf should succeed in isolation:
+    await expect(polygonSpokePool.connect(relayer).estimateGas.multicall([...fillData])).to.not.be.reverted;
+    await expect(polygonSpokePool.connect(relayer).estimateGas.multicall([...executeLeafData])).to.not.be.reverted;
+
+    const nestedMulticallData = [polygonSpokePool.interface.encodeFunctionData("multicall", [executeLeafData])];
+    await expect(polygonSpokePool.connect(relayer).estimateGas.multicall([...fillData, ...nestedMulticallData])).to.be
+      .reverted;
+  });
   it("PolygonTokenBridger retrieves and unwraps tokens correctly", async function () {
     const l1ChainId = await owner.getChainId();