Auto 8888 properly account for l 1 gas overhead for l 2 chains op scr…

…oll arb (smartcontractkit#11983) * AUTO-8804: added a chain specific module for automation * add modules * create specific modules for different chains * implement modules * addressed some feedbacks * update tests * generate wrappers * fix foundry * run yarn prettier:write * remove unnecessary import * remove unnecessary checks * update gas overheads to pass tests * regen wrappers * fix sonarcube issues * address some comments * adjust gas overheads * prettier * remove only * adjust gas overhead again * dont use const * rebase to latest and add chainmodule getter * refactor gas overhead usage * Cleanup and rename variables * add chain module overhead calculation * inline max overhead function * remove underfunded upkeep check * simplify max link payment * minor improvements * cleanup maxLinkPayment * Revert "cleanup maxLinkPayment" This reverts commit 1048b7c. * fixc small issues * fix some tests, adjust overheads, regen wrappers * run prettier * fix tests * add some todos * update comments * regen master interface * regen wrappers and update tests * improve tests * add per signer transmit overhead * fix conditional overhead test * fix overhead tests * Fix batching tests * refactor linkForGas in tests * Divide l1 fee according to performdata weight * format tests * add test for l1 split * adjust comment * update comment * format tests * iformat tests * add tests for reorg protection flag * update go wrappers * remove i keeper registry 2.2 wrapper * Polish minBalance test * formatting * refine constants * update * udpate wrappers --------- Co-authored-by: FelixFan1992 <[email protected]> Co-authored-by: lei shi <[email protected]> Co-authored-by: Akshay Aggarwal <[email protected]>
faretdinovfi · Feb 16, 2024 · 7685116 · 7685116
1 parent 60de607
commit 7685116
Show file tree

Hide file tree

Showing 18 changed files with 757 additions and 493 deletions.
diff --git a/contracts/src/v0.8/automation/dev/chains/ArbitrumModule.sol b/contracts/src/v0.8/automation/dev/chains/ArbitrumModule.sol
@@ -34,6 +34,17 @@ contract ArbitrumModule is ChainModuleBase {
 
   function getMaxL1Fee(uint256 dataSize) external view override returns (uint256) {
     (, uint256 perL1CalldataUnit, , , , ) = ARB_GAS.getPricesInWei();
+    // TODO: Verify this is an accurate estimate
     return perL1CalldataUnit * dataSize * 16;
   }
+
+  function getGasOverhead()
+    external
+    view
+    override
+    returns (uint256 chainModuleFixedOverhead, uint256 chainModulePerByteOverhead)
+  {
+    // TODO: Calculate
+    return (0, 0);
+  }
 }
diff --git a/contracts/src/v0.8/automation/dev/chains/ChainModuleBase.sol b/contracts/src/v0.8/automation/dev/chains/ChainModuleBase.sol
@@ -22,4 +22,13 @@ contract ChainModuleBase is IChainModule {
   function getMaxL1Fee(uint256) external view virtual returns (uint256) {
     return 0;
   }
+
+  function getGasOverhead()
+    external
+    view
+    virtual
+    returns (uint256 chainModuleFixedOverhead, uint256 chainModulePerByteOverhead)
+  {
+    return (0, 0);
+  }
 }
diff --git a/contracts/src/v0.8/automation/dev/chains/OptimismModule.sol b/contracts/src/v0.8/automation/dev/chains/OptimismModule.sol
@@ -14,13 +14,25 @@ contract OptimismModule is ChainModuleBase {
   OVM_GasPriceOracle private constant OVM_GASPRICEORACLE = OVM_GasPriceOracle(OVM_GASPRICEORACLE_ADDR);
 
   function getCurrentL1Fee() external view override returns (uint256) {
+    // TODO: Verify this is accurate calculation with appropriate padding
     return OVM_GASPRICEORACLE.getL1Fee(bytes.concat(msg.data, OP_L1_DATA_FEE_PADDING));
   }
 
   function getMaxL1Fee(uint256 dataSize) external view override returns (uint256) {
     // fee is 4 per 0 byte, 16 per non-zero byte. Worst case we can have all non zero-bytes.
     // Instead of setting bytes to non-zero, we initialize 'new bytes' of length 4*dataSize to cover for zero bytes.
     bytes memory txCallData = new bytes(4 * dataSize);
+    // TODO: Verify this is an accurate estimate
     return OVM_GASPRICEORACLE.getL1Fee(bytes.concat(txCallData, OP_L1_DATA_FEE_PADDING));
   }
+
+  function getGasOverhead()
+    external
+    view
+    override
+    returns (uint256 chainModuleFixedOverhead, uint256 chainModulePerByteOverhead)
+  {
+    // TODO: Calculate
+    return (0, 0);
+  }
 }
diff --git a/contracts/src/v0.8/automation/dev/chains/ScrollModule.sol b/contracts/src/v0.8/automation/dev/chains/ScrollModule.sol
@@ -16,14 +16,26 @@ contract ScrollModule is ChainModuleBase {
   IScrollL1GasPriceOracle private constant SCROLL_ORACLE = IScrollL1GasPriceOracle(SCROLL_ORACLE_ADDR);
 
   function getCurrentL1Fee() external view override returns (uint256) {
+    // TODO: Verify this is accurate calculation with appropriate padding
     return SCROLL_ORACLE.getL1Fee(bytes.concat(msg.data, SCROLL_L1_FEE_DATA_PADDING));
   }
 
   function getMaxL1Fee(uint256 dataSize) external view override returns (uint256) {
     // fee is 4 per 0 byte, 16 per non-zero byte. Worst case we can have all non zero-bytes.
     // Instead of setting bytes to non-zero, we initialize 'new bytes' of length 4*dataSize to cover for zero bytes.
     // this is the same as OP.
+    // TODO: Verify this is an accurate estimate
     bytes memory txCallData = new bytes(4 * dataSize);
     return SCROLL_ORACLE.getL1Fee(bytes.concat(txCallData, SCROLL_L1_FEE_DATA_PADDING));
   }
+
+  function getGasOverhead()
+    external
+    view
+    override
+    returns (uint256 chainModuleFixedOverhead, uint256 chainModulePerByteOverhead)
+  {
+    // TODO: Calculate
+    return (0, 0);
+  }
 }
diff --git a/contracts/src/v0.8/automation/dev/interfaces/v2_2/IAutomationRegistryMaster.sol b/contracts/src/v0.8/automation/dev/interfaces/v2_2/IAutomationRegistryMaster.sol
diff --git a/contracts/src/v0.8/automation/dev/interfaces/v2_2/IChainModule.sol b/contracts/src/v0.8/automation/dev/interfaces/v2_2/IChainModule.sol
@@ -10,10 +10,20 @@ interface IChainModule {
 
   // retrieve the L1 data fee for a L2 transaction. it should return 0 for L1 chains and
   // L2 chains which don't have L1 fee component. it uses msg.data to estimate L1 data so
-  // it must be used with a transaction.
+  // it must be used with a transaction. Return value in wei.
   function getCurrentL1Fee() external view returns (uint256);
 
   // retrieve the L1 data fee for a L2 simulation. it should return 0 for L1 chains and
-  // L2 chains which don't have L1 fee component.
+  // L2 chains which don't have L1 fee component. Return value in wei.
   function getMaxL1Fee(uint256 dataSize) external view returns (uint256);
+
+  // Returns an upper bound on execution gas cost for one invocation of blockNumber(),
+  // one invocation of blockHash() and one invocation of getCurrentL1Fee().
+  // Returns two values, first value indicates a fixed cost and the second value is
+  // the cost per msg.data byte (As some chain module's getCurrentL1Fee execution cost
+  // scales with calldata size)
+  function getGasOverhead()
+    external
+    view
+    returns (uint256 chainModuleFixedOverhead, uint256 chainModulePerByteOverhead);
 }
diff --git a/contracts/src/v0.8/automation/dev/v2_2/AutomationRegistry2_2.sol b/contracts/src/v0.8/automation/dev/v2_2/AutomationRegistry2_2.sol
@@ -57,6 +57,16 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
     Chainable(address(logicA))
   {}
 
+  /**
+   * @notice holds the variables used in the transmit function, necessary to avoid stack too deep errors
+   */
+  struct TransmitVars {
+    uint16 numUpkeepsPassedChecks;
+    uint256 totalCalldataWeight;
+    uint96 totalReimbursement;
+    uint96 totalPremium;
+  }
+
   // ================================================================
   // |                           ACTIONS                            |
   // ================================================================
@@ -96,21 +106,20 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
 
   function _handleReport(HotVars memory hotVars, Report memory report, uint256 gasOverhead) private {
     UpkeepTransmitInfo[] memory upkeepTransmitInfo = new UpkeepTransmitInfo[](report.upkeepIds.length);
-    uint16 numUpkeepsPassedChecks;
+    TransmitVars memory transmitVars = TransmitVars({
+      numUpkeepsPassedChecks: 0,
+      totalCalldataWeight: 0,
+      totalReimbursement: 0,
+      totalPremium: 0
+    });
 
     uint256 blocknumber = hotVars.chainModule.blockNumber();
+    uint256 l1Fee = hotVars.chainModule.getCurrentL1Fee();
+
     for (uint256 i = 0; i < report.upkeepIds.length; i++) {
       upkeepTransmitInfo[i].upkeep = s_upkeep[report.upkeepIds[i]];
       upkeepTransmitInfo[i].triggerType = _getTriggerType(report.upkeepIds[i]);
-      upkeepTransmitInfo[i].maxLinkPayment = _getMaxLinkPayment(
-        hotVars,
-        upkeepTransmitInfo[i].triggerType,
-        uint32(report.gasLimits[i]),
-        uint32(report.performDatas[i].length),
-        report.fastGasWei,
-        report.linkNative,
-        true
-      );
+
       (upkeepTransmitInfo[i].earlyChecksPassed, upkeepTransmitInfo[i].dedupID) = _prePerformChecks(
         report.upkeepIds[i],
         blocknumber,
@@ -120,7 +129,7 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
       );
 
       if (upkeepTransmitInfo[i].earlyChecksPassed) {
-        numUpkeepsPassedChecks += 1;
+        transmitVars.numUpkeepsPassedChecks += 1;
       } else {
         continue;
       }
@@ -132,66 +141,61 @@ contract AutomationRegistry2_2 is AutomationRegistryBase2_2, OCR2Abstract, Chain
         report.performDatas[i]
       );
 
+      // To split L1 fee across the upkeeps, assign a weight to this upkeep based on the length
+      // of the perform data and calldata overhead
+      upkeepTransmitInfo[i].calldataWeight =
+        report.performDatas[i].length +
+        TRANSMIT_CALLDATA_FIXED_BYTES_OVERHEAD +
+        (TRANSMIT_CALLDATA_PER_SIGNER_BYTES_OVERHEAD * (hotVars.f + 1));
+      transmitVars.totalCalldataWeight += upkeepTransmitInfo[i].calldataWeight;
+
       // Deduct that gasUsed by upkeep from our running counter
       gasOverhead -= upkeepTransmitInfo[i].gasUsed;
 
       // Store last perform block number / deduping key for upkeep
       _updateTriggerMarker(report.upkeepIds[i], blocknumber, upkeepTransmitInfo[i]);
     }
     // No upkeeps to be performed in this report
-    if (numUpkeepsPassedChecks == 0) {
+    if (transmitVars.numUpkeepsPassedChecks == 0) {
       return;
     }
 
     // This is the overall gas overhead that will be split across performed upkeeps
-    // Take upper bound of 16 gas per callData bytes, which is approximated to be reportLength
-    // Rest of msg.data is accounted for in accounting overheads
-    // NOTE in process of changing accounting, so pre-emptively changed reportLength to msg.data.length
-    gasOverhead =
-      (gasOverhead - gasleft() + 16 * msg.data.length) +
-      ACCOUNTING_FIXED_GAS_OVERHEAD +
-      (ACCOUNTING_PER_SIGNER_GAS_OVERHEAD * (hotVars.f + 1));
-    gasOverhead = gasOverhead / numUpkeepsPassedChecks + ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD;
+    // Take upper bound of 16 gas per callData bytes
+    gasOverhead = (gasOverhead - gasleft()) + (16 * msg.data.length) + ACCOUNTING_FIXED_GAS_OVERHEAD;
+    gasOverhead = gasOverhead / transmitVars.numUpkeepsPassedChecks + ACCOUNTING_PER_UPKEEP_GAS_OVERHEAD;
 
-    uint96 totalReimbursement;
-    uint96 totalPremium;
     {
       uint96 reimbursement;
       uint96 premium;
       for (uint256 i = 0; i < report.upkeepIds.length; i++) {
         if (upkeepTransmitInfo[i].earlyChecksPassed) {
-          upkeepTransmitInfo[i].gasOverhead = _getCappedGasOverhead(
-            gasOverhead,
-            upkeepTransmitInfo[i].triggerType,
-            uint32(report.performDatas[i].length),
-            hotVars.f
-          );
-
           (reimbursement, premium) = _postPerformPayment(
             hotVars,
             report.upkeepIds[i],
-            upkeepTransmitInfo[i],
+            upkeepTransmitInfo[i].gasUsed,
             report.fastGasWei,
             report.linkNative,
-            numUpkeepsPassedChecks
+            gasOverhead,
+            (l1Fee * upkeepTransmitInfo[i].calldataWeight) / transmitVars.totalCalldataWeight
           );
-          totalPremium += premium;
-          totalReimbursement += reimbursement;
+          transmitVars.totalPremium += premium;
+          transmitVars.totalReimbursement += reimbursement;
 
           emit UpkeepPerformed(
             report.upkeepIds[i],
             upkeepTransmitInfo[i].performSuccess,
             reimbursement + premium,
             upkeepTransmitInfo[i].gasUsed,
-            upkeepTransmitInfo[i].gasOverhead,
+            gasOverhead,
             report.triggers[i]
           );
         }
       }
     }
     // record payments
-    s_transmitters[msg.sender].balance += totalReimbursement;
-    s_hotVars.totalPremium += totalPremium;
+    s_transmitters[msg.sender].balance += transmitVars.totalReimbursement;
+    s_hotVars.totalPremium += transmitVars.totalPremium;
   }
 
   /**