Merge branch 'master' into pablo/acx-3907-fill-test-with-lookup-table

Author: md0x

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);

scripts/svm/fakeFillWithRandomDistribution.ts

@@ -20,7 +20,7 @@ import {
   calculateRelayHashUint8Array,
   getSpokePoolProgram,
   loadFillRelayParams,
-  sendTransactionWithLookupTable,
+  sendTransactionWithLookupTableV1,
 } from "../../src/svm/web3-v1";
 import { FillDataParams, FillDataValues } from "../../src/types/svm";
 
@@ -219,7 +219,7 @@ async function fillV3RelayToRandom(): Promise<void> {
     .instruction();
 
   // Fill using the ALT.
-  const { txSignature } = await sendTransactionWithLookupTable(
+  const { txSignature } = await sendTransactionWithLookupTableV1(
     provider.connection,
     [approveInstruction, fillInstruction],
     signer

src/svm/web3-v1/transactionUtils.ts

@@ -12,7 +12,7 @@ import {
 /**
  * Sends a transaction using an Address Lookup Table for large numbers of accounts.
  */
-export async function sendTransactionWithLookupTable(
+export async function sendTransactionWithLookupTableV1(
   connection: Connection,
   instructions: TransactionInstruction[],
   sender: Keypair

src/svm/web3-v2/index.ts

@@ -1,2 +1,3 @@
 export * from "./solanaProgramUtils";
 export * from "./transactionUtils";
+export * from "./types";

src/svm/web3-v2/solanaProgramUtils.ts

@@ -1,35 +1,6 @@
 import { BorshEventCoder, Idl, utils } from "@coral-xyz/anchor";
-import web3, {
-  Address,
-  AddressesByLookupTableAddress,
-  appendTransactionMessageInstruction,
-  appendTransactionMessageInstructions,
-  Commitment,
-  compressTransactionMessageUsingAddressLookupTables as compressTxWithAlt,
-  getSignatureFromTransaction,
-  GetSignaturesForAddressApi,
-  GetTransactionApi,
-  IInstruction,
-  KeyPairSigner,
-  pipe,
-  Rpc,
-  RpcSubscriptions,
-  RpcTransport,
-  sendAndConfirmTransactionFactory,
-  Signature,
-  SignatureNotificationsApi,
-  signTransactionMessageWithSigners,
-  SlotNotificationsApi,
-  SolanaRpcApiFromTransport,
-} from "@solana/kit";
-
-import {
-  fetchAddressLookupTable,
-  findAddressLookupTablePda,
-  getCreateLookupTableInstructionAsync,
-  getExtendLookupTableInstruction,
-} from "@solana-program/address-lookup-table";
-import { createDefaultTransaction, signAndSendTransaction } from "../../../test/svm/utils";
+import web3, { Address, Commitment, GetSignaturesForAddressApi, GetTransactionApi, Signature } from "@solana/kit";
+import { RpcClient } from "./types";
 
 type GetTransactionReturnType = ReturnType<GetTransactionApi["getTransaction"]>;
 
@@ -170,76 +141,3 @@ export async function readFillEventFromFillStatusPda(
   const events = await readEvents(client, signatures[signatures.length - 1].signature, programId, programIdl);
   return { event: events[0], slot: Number(signatures[signatures.length - 1].slot) };
 }
-
-export async function createAlt(client: RpcClient, authority: KeyPairSigner): Promise<Address> {
-  const recentSlot = await client.rpc.getSlot({ commitment: "finalized" }).send();
-
-  const [alt] = await findAddressLookupTablePda({
-    authority: authority.address,
-    recentSlot,
-  });
-
-  const createAltIx = await getCreateLookupTableInstructionAsync({
-    authority,
-    recentSlot,
-  });
-
-  await pipe(
-    await createDefaultTransaction(client, authority),
-    (tx) => appendTransactionMessageInstruction(createAltIx, tx),
-    (tx) => signAndSendTransaction(client, tx)
-  );
-
-  return alt;
-}
-
-export async function extendAlt(client: RpcClient, authority: KeyPairSigner, alt: Address, addresses: Address[]) {
-  const extendAltIx = getExtendLookupTableInstruction({
-    address: alt,
-    authority,
-    payer: authority,
-    addresses,
-  });
-
-  await pipe(
-    await createDefaultTransaction(client, authority),
-    (tx) => appendTransactionMessageInstruction(extendAltIx, tx),
-    (tx) => signAndSendTransaction(client, tx)
-  );
-
-  const altAccount = await fetchAddressLookupTable(client.rpc, alt);
-
-  const addressesByLookupTableAddress: AddressesByLookupTableAddress = {};
-  addressesByLookupTableAddress[alt] = altAccount.data.addresses;
-
-  // Delay a second here to let lookup table warm up
-  await sleep(1000);
-
-  return addressesByLookupTableAddress;
-}
-
-async function sleep(ms: number) {
-  return new Promise((resolve) => setTimeout(resolve, ms));
-}
-
-export async function sendTransactionWithLookupTable(
-  client: RpcClient,
-  payer: KeyPairSigner,
-  instructions: IInstruction[],
-  addressesByLookupTableAddress: AddressesByLookupTableAddress
-) {
-  return pipe(
-    await createDefaultTransaction(client, payer),
-    (tx) => appendTransactionMessageInstructions(instructions, tx),
-    (tx) => compressTxWithAlt(tx, addressesByLookupTableAddress),
-    (tx) => signTransactionMessageWithSigners(tx),
-    async (tx) => {
-      const signedTx = await tx;
-      await sendAndConfirmTransactionFactory(client)(signedTx, {
-        commitment: "confirmed",
-        skipPreflight: false,
-      });
-      return getSignatureFromTransaction(signedTx);
-    }
-  );
-}

src/svm/web3-v2/transactionUtils.ts

@@ -1,17 +1,53 @@
 import {
+  Address,
   AddressesByLookupTableAddress,
+  appendTransactionMessageInstruction,
   appendTransactionMessageInstructions,
+  Commitment,
+  CompilableTransactionMessage,
   compressTransactionMessageUsingAddressLookupTables as compressTxWithAlt,
+  createTransactionMessage,
   getSignatureFromTransaction,
   IInstruction,
   KeyPairSigner,
   pipe,
   sendAndConfirmTransactionFactory,
+  setTransactionMessageFeePayerSigner,
+  setTransactionMessageLifetimeUsingBlockhash,
   signTransactionMessageWithSigners,
+  TransactionMessageWithBlockhashLifetime,
+  TransactionSigner,
 } from "@solana/kit";
 
-import { createDefaultTransaction } from "../../../test/svm/utils";
+import {
+  fetchAddressLookupTable,
+  findAddressLookupTablePda,
+  getCreateLookupTableInstructionAsync,
+  getExtendLookupTableInstruction,
+} from "@solana-program/address-lookup-table";
+import { RpcClient } from "./types";
+
+export const signAndSendTransaction = async (
+  rpcClient: RpcClient,
+  transactionMessage: CompilableTransactionMessage & TransactionMessageWithBlockhashLifetime,
+  commitment: Commitment = "confirmed"
+) => {
+  const signedTransaction = await signTransactionMessageWithSigners(transactionMessage);
+  const signature = getSignatureFromTransaction(signedTransaction);
+  await sendAndConfirmTransactionFactory(rpcClient)(signedTransaction, {
+    commitment,
+  });
+  return signature;
+};
 
+export const createDefaultTransaction = async (rpcClient: RpcClient, signer: TransactionSigner) => {
+  const { value: latestBlockhash } = await rpcClient.rpc.getLatestBlockhash().send();
+  return pipe(
+    createTransactionMessage({ version: 0 }),
+    (tx) => setTransactionMessageFeePayerSigner(signer, tx),
+    (tx) => setTransactionMessageLifetimeUsingBlockhash(latestBlockhash, tx)
+  );
+};
 export async function sendTransactionWithLookupTable(
   client: RpcClient,
   payer: KeyPairSigner,
@@ -33,3 +69,55 @@ export async function sendTransactionWithLookupTable(
     }
   );
 }
+
+export async function createLookupTable(client: RpcClient, authority: KeyPairSigner): Promise<Address> {
+  const recentSlot = await client.rpc.getSlot({ commitment: "finalized" }).send();
+
+  const [alt] = await findAddressLookupTablePda({
+    authority: authority.address,
+    recentSlot,
+  });
+
+  const createAltIx = await getCreateLookupTableInstructionAsync({
+    authority,
+    recentSlot,
+  });
+
+  await pipe(
+    await createDefaultTransaction(client, authority),
+    (tx) => appendTransactionMessageInstruction(createAltIx, tx),
+    (tx) => signAndSendTransaction(client, tx)
+  );
+
+  return alt;
+}
+
+export async function extendLookupTable(
+  client: RpcClient,
+  authority: KeyPairSigner,
+  alt: Address,
+  addresses: Address[]
+) {
+  const extendAltIx = getExtendLookupTableInstruction({
+    address: alt,
+    authority,
+    payer: authority,
+    addresses,
+  });
+
+  await pipe(
+    await createDefaultTransaction(client, authority),
+    (tx) => appendTransactionMessageInstruction(extendAltIx, tx),
+    (tx) => signAndSendTransaction(client, tx)
+  );
+
+  const altAccount = await fetchAddressLookupTable(client.rpc, alt);
+
+  const addressesByLookupTableAddress: AddressesByLookupTableAddress = {};
+  addressesByLookupTableAddress[alt] = altAccount.data.addresses;
+
+  // Delay a second here to let lookup table warm up
+  await new Promise((resolve) => setTimeout(resolve, 1000));
+
+  return addressesByLookupTableAddress;
+}

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();