fix(TryMulticallClient): Use simulated gasLimit when sending multical…

…l txns (#1761)

* fix(TryMulticallClient): Use simulated gasLimit when sending multicall transactions

Signed-off-by: bennett <bennett@umaproject.org>

---------

Signed-off-by: bennett <bennett@umaproject.org>

contracts/MockMulticallReceiver.sol

@@ -0,0 +1,30 @@
+//SPDX-License-Identifier: Unlicense
+pragma solidity ^0.8.0;
+
+/**
+ * @title MockMulticallReceiver
+ */
+contract MockMulticallReceiver {
+    struct Result {
+        bool success;
+        bytes returnData;
+    }
+
+    /// @custom:oz-upgrades-unsafe-allow constructor
+    constructor() {} // solhint-disable-line no-empty-blocks
+
+    // Entrypoint for tryMulticall tests. For simplicity, a call will succeed if its MSB is nonzero.
+    function tryMulticall(bytes[] calldata calls) external pure returns (Result[] memory) {
+        Result[] memory results = new Result[](calls.length);
+        for (uint256 i = 0; i < calls.length; ++i) {
+            bool success = uint256(bytes32(calls[i])) != 0;
+            results[i] = Result(success, calls[i]);
+        }
+        return results;
+    }
+
+    // In the MultiCallerClient, we require the contract we are calling to contain
+    // multicall in order for the transaction to not be marked as unsendable.
+    // https://github.com/across-protocol/relayer/blob/e56a6874419eeded58d64e77a1628b6541861b65/src/clients/MultiCallerClient.ts#L374
+    function multicall() external {}
+}

src/clients/MultiCallerClient.ts

@@ -16,7 +16,7 @@ import {
   getProvider,
   assert,
 } from "../utils";
-import { AugmentedTransaction, TransactionClient, RawTransaction } from "./TransactionClient";
+import { AugmentedTransaction, TransactionClient } from "./TransactionClient";
 import lodash from "lodash";
 
 // @todo: MultiCallerClient should be generic. For future, permit the class instantiator to supply their own
@@ -65,6 +65,16 @@ export const unknownRevertReasonMethodsToIgnore = new Set([
 //      See also https://community.optimism.io/docs/developers/bedrock/differences/
 const MULTICALL3_AGGREGATE_GAS_MULTIPLIER = 1.5;
 
+// The below interface is used by the TryMulticallClient to store information about successfully simulated transactions.
+// A tryMulticall call returns information about whether or not each individual transaction within the bundle succeeds.
+// If some but not all individual transactions in the bundle succeed, then we store that information in this interface
+// so that we may rebuild a tryMulticall bundle without having to perform further simulations.
+export interface TryMulticallTransaction {
+  contract: Contract;
+  calldata: string[];
+  gasLimit: BigNumber;
+}
+
 export class MultiCallerClient {
   protected txnClient: TransactionClient;
   protected txns: { [chainId: number]: AugmentedTransaction[] } = {};
@@ -540,16 +550,24 @@ export class TryMulticallClient extends MultiCallerClient {
       message: `${simulate ? "Simulating" : "Executing"} ${nTxns} transaction(s) on ${networkName}.`,
     });
 
-    const buildRawTransaction = (contract: Contract, data: string): RawTransaction => ({ contract, data });
+    const buildTryMulticallTransaction = (
+      contract: Contract,
+      calldata: string[],
+      gasLimit: BigNumber
+    ): TryMulticallTransaction => ({
+      contract,
+      calldata,
+      gasLimit,
+    });
 
     const txnRequestsToSubmit: AugmentedTransaction[] = [];
-    const txnCalldataToRebuild: RawTransaction[] = [];
+    const txnCalldataToRebuild: TryMulticallTransaction[] = [];
 
     // The goal is to simulate the transactions as a batch and pick out those which succeed.
     const bundledTxns = await this.buildMultiCallBundles(txns, this.chunkSize[chainId]);
     const bundledSimResults = await this.txnClient.simulate(bundledTxns);
 
-    bundledSimResults.forEach(({ succeed, transaction, reason, data }, idx) => {
+    bundledSimResults.forEach(({ succeed, transaction, reason, data }) => {
       // TryMulticall _should_ always succeed, but either way, we need the return data to be defined so that we can properly
       // filter out the transactions which failed.
       if (!succeed || !isDefined(data?.length)) {
@@ -572,7 +590,7 @@ export class TryMulticallClient extends MultiCallerClient {
       // some txns in the bundle must have failed. We take note only of the ones which succeeded.
       if (succeededTxnCalldata.length !== data.length) {
         txnCalldataToRebuild.push(
-          ...succeededTxnCalldata.map((calldata) => buildRawTransaction(transaction.contract, calldata))
+          buildTryMulticallTransaction(transaction.contract, succeededTxnCalldata, transaction.gasLimit)
         );
         this.logger.debug({
           at: "tryMulticallClient#executeChainTxnQueue",
@@ -582,8 +600,7 @@ export class TryMulticallClient extends MultiCallerClient {
         });
       } else {
         // Otherwise, none of the transactions failed, so we can add the bundle to txnRequestsToSubmit.
-        bundledTxns[idx].gasLimit = transaction.gasLimit;
-        txnRequestsToSubmit.push(bundledTxns[idx]);
+        txnRequestsToSubmit.push(transaction);
 
         // If txn succeeded or the revert reason is known to be benign, then log at debug level.
         this.logger.debug({
@@ -599,25 +616,24 @@ export class TryMulticallClient extends MultiCallerClient {
       // chunk size. Every transaction should be aimed at the same spoke pool since 1. The tryMulticall client is only
       // instantiated for the relayer, which uses this client only for interfacing with the spoke pool, and 2. This function
       // is called after filtering transactions by chainId, so each individual transaction is a call to a chainId's spoke pool.
-      const txnChunks = lodash.chunk(txnCalldataToRebuild, this.chunkSize[chainId] ?? DEFAULT_MULTICALL_CHUNK_SIZE);
-      const rebuildTryMulticall = (txns: RawTransaction[]) => {
+      const rebuildTryMulticall = (txn: TryMulticallTransaction) => {
         const mrkdwn: string[] = [];
-        const contract = txns[0].contract;
-        txns.forEach((txn, idx) => {
-          mrkdwn.push(`\n *txn. ${idx + 1}:* ${txn.data ?? "No calldata"}`);
-          assert(contract === txn.contract);
+        const contract = txn.contract;
+        const gasLimit = txn.gasLimit;
+        txn.calldata.forEach((data, idx) => {
+          mrkdwn.push(`\n *txn. ${idx + 1}:* ${data}`);
         });
-        const callData = txns.map((txn) => txn.data);
         return {
           chainId,
           contract,
+          gasLimit,
           method: "tryMulticall",
-          args: [callData],
+          args: [txn.calldata],
           message: "Across tryMulticall transaction",
           mrkdwn: mrkdwn.join(""),
-        } as AugmentedTransaction;
+        };
       };
-      txnRequestsToSubmit.push(...txnChunks.map(rebuildTryMulticall));
+      txnRequestsToSubmit.push(...txnCalldataToRebuild.map(rebuildTryMulticall));
     }
 
     if (simulate) {

src/clients/TransactionClient.ts

@@ -30,11 +30,6 @@ export interface AugmentedTransaction {
   groupId?: string;
 }
 
-export interface RawTransaction {
-  contract: Contract;
-  data: string;
-}
-
 const { fixedPointAdjustment: fixedPoint } = sdkUtils;
 const { isError } = typeguards;