fix reorg upkeep protection to allow bypass by empty blockhash (#9699)

* Fix reorg protection to allow bypass via empty blockhash

* regen wrappers

---------

Co-authored-by: Ryan Hall <RyanRHall@users.noreply.github.com>

contracts/src/v0.8/dev/automation/2_1/KeeperRegistry2_1.sol

@@ -458,10 +458,16 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
       emit StaleUpkeepReport(upkeepId, rawTrigger);
       return false;
     }
-    if (_blockHash(trigger.blockNum) != trigger.blockHash || trigger.blockNum >= block.number) {
-      // Can happen when the block on which report was generated got reorged
-      // We will also revert if checkBlockNumber is older than 256 blocks. In this case we rely on a new transmission
-      // with the latest checkBlockNumber
+    if (
+      (trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash) ||
+      trigger.blockNum >= block.number
+    ) {
+      // There are two cases of reorged report
+      // 1. trigger block number is in future: this is an edge case during extreme deep reorgs of chain
+      // which is always protected against
+      // 2. blockHash at trigger block number was same as trigger time. This is an optional check which is
+      // applied if DON sends non empty trigger.blockHash. Note: It only works for last 256 blocks on chain
+      // when it is sent
       emit ReorgedUpkeepReport(upkeepId, rawTrigger);
       return false;
     }
@@ -470,7 +476,11 @@ contract KeeperRegistry2_1 is KeeperRegistryBase2_1, OCR2Abstract, Chainable, ER
 
   function _validateLogTrigger(uint256 upkeepId, bytes memory rawTrigger) internal returns (bool) {
     LogTrigger memory trigger = abi.decode(rawTrigger, (LogTrigger));
-    if (_blockHash(trigger.blockNum) != trigger.blockHash || trigger.blockNum >= block.number) {
+    if (
+      (trigger.blockHash != bytes32("") && _blockHash(trigger.blockNum) != trigger.blockHash) ||
+      trigger.blockNum >= block.number
+    ) {
+      // Reorg protection is same as block trigger upkeeps
       emit ReorgedUpkeepReport(upkeepId, rawTrigger);
       return false;
     }

contracts/test/v0.8/automation/KeeperRegistry2_1.test.ts

@@ -1290,9 +1290,30 @@ describe('KeeperRegistry2_1', () => {
         }
       })
 
-      it('allows bypassing reorg protection with empty blockhash / bocknumber', async () => {
-        // mine enough blocks so that blockhash(0) is unavailable
-        for (let i = 0; i < 256; i++) {
+      it('allows bypassing reorg protection with empty blockhash', async () => {
+        const tests: [string, BigNumber][] = [
+          ['conditional', upkeepId],
+          ['log-trigger', logUpkeepId],
+        ]
+        for (const [type, id] of tests) {
+          const latestBlock = await ethers.provider.getBlock('latest')
+          const tx = await getTransmitTx(registry, keeper1, [id], {
+            checkBlockNum: latestBlock.number,
+            checkBlockHash: emptyBytes32,
+          })
+          const receipt = await tx.wait()
+          const upkeepPerformedLogs = parseUpkeepPerformedLogs(receipt)
+          assert.equal(
+            upkeepPerformedLogs.length,
+            1,
+            `wrong log count for ${type} upkeep`,
+          )
+        }
+      })
+
+      it('allows very old trigger block numbers when bypassing reorg protection with empty blockhash', async () => {
+        // mine enough blocks so that blockhash(1) is unavailable
+        for (let i = 0; i <= 256; i++) {
           await ethers.provider.send('evm_mine', [])
         }
         const tests: [string, BigNumber][] = [
@@ -1301,7 +1322,7 @@ describe('KeeperRegistry2_1', () => {
         ]
         for (const [type, id] of tests) {
           const tx = await getTransmitTx(registry, keeper1, [id], {
-            checkBlockNum: 0,
+            checkBlockNum: 1,
             checkBlockHash: emptyBytes32,
           })
           const receipt = await tx.wait()
@@ -1314,6 +1335,44 @@ describe('KeeperRegistry2_1', () => {
         }
       })
 
+      it('returns early when future block number is provided as trigger, irrespective of blockhash being present', async () => {
+        const tests: [string, BigNumber][] = [
+          ['conditional', upkeepId],
+          ['log-trigger', logUpkeepId],
+        ]
+        for (const [type, id] of tests) {
+          const latestBlock = await ethers.provider.getBlock('latest')
+
+          // Should fail when blockhash is empty
+          let tx = await getTransmitTx(registry, keeper1, [id], {
+            checkBlockNum: latestBlock.number + 100,
+            checkBlockHash: emptyBytes32,
+          })
+          let receipt = await tx.wait()
+          let reorgedUpkeepReportLogs = parseReorgedUpkeepReportLogs(receipt)
+          // exactly 1 ReorgedUpkeepReportLogs log should be emitted
+          assert.equal(
+            reorgedUpkeepReportLogs.length,
+            1,
+            `wrong log count for ${type} upkeep`,
+          )
+
+          // Should also fail when blockhash is not empty
+          tx = await getTransmitTx(registry, keeper1, [id], {
+            checkBlockNum: latestBlock.number + 100,
+            checkBlockHash: latestBlock.hash,
+          })
+          receipt = await tx.wait()
+          reorgedUpkeepReportLogs = parseReorgedUpkeepReportLogs(receipt)
+          // exactly 1 ReorgedUpkeepReportLogs log should be emitted
+          assert.equal(
+            reorgedUpkeepReportLogs.length,
+            1,
+            `wrong log count for ${type} upkeep`,
+          )
+        }
+      })
+
       it('returns early when upkeep is cancelled and cancellation delay has gone', async () => {
         const latestBlockReport = await makeLatestBlockReport([upkeepId])
         await registry.connect(admin).cancelUpkeep(upkeepId)