devsvcs-958: fix automation v2.3 batching bug (#15897)

* devsvcs-958: fix automation v2.3 batching bug * Update gethwrappers * update * fix changeset * update logic and tests * update --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
smartcontractkit · Jan 16, 2025 · 47a0c42 · 47a0c42
1 parent 46ef625
commit 47a0c42
Show file tree

Hide file tree

Showing 13 changed files with 383 additions and 22 deletions.
diff --git a/.changeset/long-apples-fold.md b/.changeset/long-apples-fold.md
@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+DEVSVCS-958: fix automation v2.3 batching bug #bugfix
diff --git a/contracts/.changeset/poor-turtles-give.md b/contracts/.changeset/poor-turtles-give.md
@@ -0,0 +1,5 @@
+---
+'@chainlink/contracts': patch
+---
+
+DEVSVCS-958: fix automation v2.3 batching bug #bugfix
diff --git a/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol b/contracts/src/v0.8/automation/test/v2_3/AutomationRegistry2_3.t.sol
@@ -1653,6 +1653,140 @@ contract Transmit is SetUp {
     vm.stopPrank();
   }
 
+  struct PaymentReceipt {
+    uint96 gasChargeInBillingToken;
+    uint96 premiumInBillingToken;
+    uint96 gasReimbursementInJuels;
+    uint96 premiumInJuels;
+    IERC20 billingToken;
+    uint96 linkUSD;
+    uint96 nativeUSD;
+    uint96 billingUSD;
+  }
+  event ReorgedUpkeepReport(uint256 indexed id, bytes trigger);
+  event UpkeepCharged(uint256 indexed id, PaymentReceipt receipt);
+  event UpkeepPerformed(
+    uint256 indexed id,
+    bool indexed success,
+    uint96 totalPayment,
+    uint256 gasUsed,
+    uint256 gasOverhead,
+    bytes trigger
+  );
+  function test_whenFirstUpkeepFails_subsequentUpkeepsPerform() external {
+    // the first and second upkeeps use LINK as billing token and the third uses WETH
+    // the first upkeep fails the pre perform check due to incorrect block number in its trigger
+    // the second and third upkeep perform
+    // the first upkeep should not be charged and the second and third upkeep should be charged
+    uint256[] memory prevUpkeepBalances = new uint256[](3);
+    prevUpkeepBalances[0] = registry.getBalance(linkUpkeepID);
+    prevUpkeepBalances[1] = registry.getBalance(linkUpkeepID2);
+    prevUpkeepBalances[2] = registry.getBalance(nativeUpkeepID);
+    uint256[] memory prevTokenBalances = new uint256[](2);
+    prevTokenBalances[0] = linkToken.balanceOf(address(registry));
+    prevTokenBalances[1] = weth.balanceOf(address(registry));
+    uint256[] memory prevReserveBalances = new uint256[](2);
+    prevReserveBalances[0] = registry.getReserveAmount(address(linkToken));
+    prevReserveBalances[1] = registry.getReserveAmount(address(weth));
+    uint256[] memory upkeepIDs = new uint256[](3);
+    upkeepIDs[0] = linkUpkeepID;
+    upkeepIDs[1] = linkUpkeepID2;
+    upkeepIDs[2] = nativeUpkeepID;
+
+    // do the transmit
+    vm.expectEmit();
+    emit ReorgedUpkeepReport(linkUpkeepID, abi.encode(block.number, blockhash(block.number)));
+    vm.expectEmit(true, false, false, false);
+    emit UpkeepCharged(linkUpkeepID2, PaymentReceipt(0, 0, 0, 0, linkToken, 0, 0, 0));
+    vm.expectEmit(true, true, false, false);
+    emit UpkeepPerformed(linkUpkeepID2, true, 0, 0, 0, bytes(""));
+    vm.expectEmit(true, false, false, false);
+    emit UpkeepCharged(nativeUpkeepID, PaymentReceipt(0, 0, 0, 0, linkToken, 0, 0, 0));
+    vm.expectEmit(true, true, false, false);
+    emit UpkeepPerformed(nativeUpkeepID, true, 0, 0, 0, bytes(""));
+    bool[] memory goodTriggers = new bool[](3);
+    goodTriggers[0] = false;
+    goodTriggers[1] = true;
+    goodTriggers[2] = true;
+    _batchTransmitWithBadTriggers(upkeepIDs, goodTriggers, registry);
+
+    // assert upkeep balances
+    // the first upkeep fails and the second and third upkeep succeeds
+    require(prevUpkeepBalances[0] == registry.getBalance(linkUpkeepID), "link upkeep balance should remain the same");
+    require(prevUpkeepBalances[1] > registry.getBalance(linkUpkeepID2), "link upkeep 2 balance should have decreased");
+    require(prevUpkeepBalances[2] > registry.getBalance(nativeUpkeepID), "native upkeep balance should have decreased");
+    // assert token balances have not changed
+    assertEq(prevTokenBalances[0], linkToken.balanceOf(address(registry)));
+    assertEq(prevTokenBalances[1], weth.balanceOf(address(registry)));
+    // assert reserve amounts have adjusted accordingly
+    require(
+      prevReserveBalances[0] < registry.getReserveAmount(address(linkToken)),
+      "link reserve amount should have increased"
+    ); // link reserve amount increases in value equal to the decrease of the other reserve amounts
+    require(
+      prevReserveBalances[1] > registry.getReserveAmount(address(weth)),
+      "native reserve amount should have decreased"
+    );
+  }
+
+  function test_whenSecondUpkeepFails_FirstAndThirdUpkeepsPerform() external {
+    // the first upkeep uses WETH as billing token and the second and third use LINK
+    // the first upkeep succeeds
+    // the second upkeep fails pre perform check due to incorrect block number in its trigger
+    // the third upkeep succeeds
+    // the second upkeep should not be charged and the first and third upkeep should be charged
+    uint256[] memory prevUpkeepBalances = new uint256[](3);
+    prevUpkeepBalances[0] = registry.getBalance(nativeUpkeepID);
+    prevUpkeepBalances[1] = registry.getBalance(linkUpkeepID);
+    prevUpkeepBalances[2] = registry.getBalance(linkUpkeepID2);
+    uint256[] memory prevTokenBalances = new uint256[](2);
+    prevTokenBalances[0] = weth.balanceOf(address(registry));
+    prevTokenBalances[1] = linkToken.balanceOf(address(registry));
+    uint256[] memory prevReserveBalances = new uint256[](2);
+    prevReserveBalances[0] = registry.getReserveAmount(address(weth));
+    prevReserveBalances[1] = registry.getReserveAmount(address(linkToken));
+    uint256[] memory upkeepIDs = new uint256[](3);
+    upkeepIDs[0] = nativeUpkeepID;
+    upkeepIDs[1] = linkUpkeepID;
+    upkeepIDs[2] = linkUpkeepID2;
+
+    // do the transmit
+    // expect this event first because all the upkeeps will be checked first before they are performed and charged
+    vm.expectEmit();
+    emit ReorgedUpkeepReport(linkUpkeepID, abi.encode(block.number, blockhash(block.number)));
+    vm.expectEmit(true, false, false, false);
+    emit UpkeepCharged(nativeUpkeepID, PaymentReceipt(0, 0, 0, 0, linkToken, 0, 0, 0));
+    vm.expectEmit(true, true, false, false);
+    emit UpkeepPerformed(nativeUpkeepID, true, 0, 0, 0, bytes(""));
+    vm.expectEmit(true, false, false, false);
+    emit UpkeepCharged(linkUpkeepID2, PaymentReceipt(0, 0, 0, 0, linkToken, 0, 0, 0));
+    vm.expectEmit(true, true, false, false);
+    emit UpkeepPerformed(linkUpkeepID2, true, 0, 0, 0, bytes(""));
+    bool[] memory goodTriggers = new bool[](3);
+    goodTriggers[0] = true;
+    goodTriggers[1] = false;
+    goodTriggers[2] = true;
+    _batchTransmitWithBadTriggers(upkeepIDs, goodTriggers, registry);
+
+    // assert upkeep balances
+    // the first upkeep fails and the second and third upkeep succeeds
+    require(prevUpkeepBalances[0] > registry.getBalance(nativeUpkeepID), "native upkeep balance should have decreased");
+    require(prevUpkeepBalances[1] == registry.getBalance(linkUpkeepID), "link upkeep balance should remain the same");
+    require(prevUpkeepBalances[2] > registry.getBalance(linkUpkeepID2), "link upkeep 2 balance should have decreased");
+    // assert token balances have not changed
+    assertEq(prevTokenBalances[0], weth.balanceOf(address(registry)));
+    assertEq(prevTokenBalances[1], linkToken.balanceOf(address(registry)));
+    // assert reserve amounts have adjusted accordingly
+    require(
+      prevReserveBalances[0] > registry.getReserveAmount(address(weth)),
+      "native reserve amount should have decreased"
+    ); // link reserve amount increases in value equal to the decrease of the other reserve amounts
+    require(
+      prevReserveBalances[1] < registry.getReserveAmount(address(linkToken)),
+      "link reserve amount should have increased"
+    );
+  }
+
   function test_handlesMixedBatchOfBillingTokens() external {
     uint256[] memory prevUpkeepBalances = new uint256[](3);
     prevUpkeepBalances[0] = registry.getBalance(linkUpkeepID);
@@ -1696,7 +1830,7 @@ contract Transmit is SetUp {
     // assert reserve amounts have adjusted accordingly
     require(
       prevReserveBalances[0] < registry.getReserveAmount(address(linkToken)),
-      "usd reserve amount should have increased"
+      "link reserve amount should have increased"
     ); // link reserve amount increases in value equal to the decrease of the other reserve amounts
     require(
       prevReserveBalances[1] > registry.getReserveAmount(address(usdToken18)),

diff --git a/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol b/contracts/src/v0.8/automation/test/v2_3/BaseTest.t.sol
@@ -367,6 +367,54 @@ contract BaseTest is Test {
     _handleTransmit(ids, registry, bytes4(0));
   }
 
+  function _batchTransmitWithBadTriggers(uint256[] memory ids, bool[] memory goodTriggers, Registry registry) internal {
+    bytes memory reportBytes;
+    {
+      uint256[] memory upkeepIds = new uint256[](ids.length);
+      uint256[] memory gasLimits = new uint256[](ids.length);
+      bytes[] memory performDatas = new bytes[](ids.length);
+      bytes[] memory triggers = new bytes[](ids.length);
+      for (uint256 i = 0; i < ids.length; i++) {
+        upkeepIds[i] = ids[i];
+        gasLimits[i] = registry.getUpkeep(ids[i]).performGas;
+        performDatas[i] = new bytes(0);
+        uint8 triggerType = registry.getTriggerType(ids[i]);
+        if (triggerType == 0) {
+          uint256 j = 0;
+          if (goodTriggers[i]) {
+            j = 1;
+          }
+          triggers[i] = _encodeConditionalTrigger(
+            AutoBase.ConditionalTrigger(uint32(block.number - j), blockhash(block.number - j))
+          );
+        } else {
+          revert("not implemented");
+        }
+      }
+
+      AutoBase.Report memory report = AutoBase.Report(
+        uint256(1000000000),
+        uint256(2000000000),
+        upkeepIds,
+        gasLimits,
+        triggers,
+        performDatas
+      );
+
+      reportBytes = _encodeReport(report);
+    }
+    (, , bytes32 configDigest) = registry.latestConfigDetails();
+    bytes32[3] memory reportContext = [configDigest, configDigest, configDigest];
+    uint256[] memory signerPKs = new uint256[](2);
+    signerPKs[0] = SIGNING_KEY0;
+    signerPKs[1] = SIGNING_KEY1;
+    (bytes32[] memory rs, bytes32[] memory ss, bytes32 vs) = _signReport(reportBytes, reportContext, signerPKs);
+
+    vm.startPrank(TRANSMITTERS[0]);
+    registry.transmit(reportContext, reportBytes, rs, ss, vs);
+    vm.stopPrank();
+  }
+
   // tests single upkeep, expects revert
   function _transmitAndExpectRevert(uint256 id, Registry registry, bytes4 selector) internal {
     uint256[] memory ids = new uint256[](1);

diff --git a/contracts/src/v0.8/automation/test/v2_3_zksync/BaseTest.t.sol b/contracts/src/v0.8/automation/test/v2_3_zksync/BaseTest.t.sol
@@ -419,6 +419,54 @@ contract BaseTest is Test {
     vm.stopPrank();
   }
 
+  function _batchTransmitWithBadTriggers(uint256[] memory ids, bool[] memory goodTriggers, Registry registry) internal {
+    bytes memory reportBytes;
+    {
+      uint256[] memory upkeepIds = new uint256[](ids.length);
+      uint256[] memory gasLimits = new uint256[](ids.length);
+      bytes[] memory performDatas = new bytes[](ids.length);
+      bytes[] memory triggers = new bytes[](ids.length);
+      for (uint256 i = 0; i < ids.length; i++) {
+        upkeepIds[i] = ids[i];
+        gasLimits[i] = registry.getUpkeep(ids[i]).performGas;
+        performDatas[i] = new bytes(0);
+        uint8 triggerType = registry.getTriggerType(ids[i]);
+        if (triggerType == 0) {
+          uint256 j = 0;
+          if (goodTriggers[i]) {
+            j = 1;
+          }
+          triggers[i] = _encodeConditionalTrigger(
+            ZKSyncAutoBase.ConditionalTrigger(uint32(block.number - j), blockhash(block.number - j))
+          );
+        } else {
+          revert("not implemented");
+        }
+      }
+
+      ZKSyncAutoBase.Report memory report = ZKSyncAutoBase.Report(
+        uint256(1000000000),
+        uint256(2000000000),
+        upkeepIds,
+        gasLimits,
+        triggers,
+        performDatas
+      );
+
+      reportBytes = _encodeReport(report);
+    }
+    (, , bytes32 configDigest) = registry.latestConfigDetails();
+    bytes32[3] memory reportContext = [configDigest, configDigest, configDigest];
+    uint256[] memory signerPKs = new uint256[](2);
+    signerPKs[0] = SIGNING_KEY0;
+    signerPKs[1] = SIGNING_KEY1;
+    (bytes32[] memory rs, bytes32[] memory ss, bytes32 vs) = _signReport(reportBytes, reportContext, signerPKs);
+
+    vm.startPrank(TRANSMITTERS[0]);
+    registry.transmit(reportContext, reportBytes, rs, ss, vs);
+    vm.stopPrank();
+  }
+
   /// @notice Gather signatures on report data
   /// @param report - Report bytes generated from `_buildReport`
   /// @param reportContext - Report context bytes32 generated from `_buildReport`