fix: prevent unpausing gateways if they're not configured (OZ M-01 from #700)

contracts/gateway/GraphTokenGateway.sol

@@ -45,6 +45,9 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok
      * @param _newPaused New value for the pause state (true means the transfers will be paused)
      */
     function setPaused(bool _newPaused) external onlyGovernorOrGuardian {
+        if (!_newPaused) {
+            _checksBeforeUnpause();
+        }
         _setPaused(_newPaused);
     }
 
@@ -54,4 +57,10 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok
     function paused() external view returns (bool) {
         return _paused;
     }
+
+    /**
+     * @dev Runs state validation before unpausing, reverts if
+     * something is not set properly
+     */
+    function _checksBeforeUnpause() internal view virtual;
 }

contracts/gateway/L1GraphTokenGateway.sol

@@ -356,4 +356,15 @@ contract L1GraphTokenGateway is GraphTokenGateway, L1ArbitrumMessenger {
         }
         return l2GRT;
     }
+
+    /**
+     * @dev Runs state validation before unpausing, reverts if
+     * something is not set properly
+     */
+    function _checksBeforeUnpause() internal view override {
+        require(inbox != address(0), "INBOX_NOT_SET");
+        require(l1Router != address(0), "ROUTER_NOT_SET");
+        require(l2Counterpart != address(0), "L2_COUNTERPART_NOT_SET");
+        require(escrow != address(0), "ESCROW_NOT_SET");
+    }
 }

contracts/l2/gateway/L2GraphTokenGateway.sol

@@ -294,4 +294,14 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran
         }
         return (from, extraData);
     }
+
+    /**
+     * @dev Runs state validation before unpausing, reverts if
+     * something is not set properly
+     */
+    function _checksBeforeUnpause() internal view override {
+        require(l2Router != address(0), "ROUTER_NOT_SET");
+        require(l1Counterpart != address(0), "L1_COUNTERPART_NOT_SET");
+        require(l1GRT != address(0), "L1GRT_NOT_SET");
+    }
 }

e2e/deployment/config/l1/l1GraphTokenGateway.test.ts

@@ -13,9 +13,9 @@ describe('[L1] L1GraphTokenGateway configuration', function () {
     unauthorized = (await graph.getTestAccounts())[0]
   })
 
-  it('bridge should be unpaused', async function () {
+  it('bridge should be paused', async function () {
     const paused = await L1GraphTokenGateway.paused()
-    expect(paused).eq(false)
+    expect(paused).eq(true)
   })
 
   it('should be controlled by Controller', async function () {
@@ -77,7 +77,7 @@ describe('[L1] L1GraphTokenGateway configuration', function () {
         '0x00',
       )
 
-      await expect(tx).revertedWith('INBOX_NOT_SET')
+      await expect(tx).revertedWith('Paused (contract)')
     })
   })
 })

e2e/deployment/config/l2/l2GraphTokenGateway.test.ts

@@ -13,9 +13,9 @@ describe('[L2] L2GraphTokenGateway configuration', function () {
     unauthorized = (await graph.getTestAccounts())[0]
   })
 
-  it('bridge should be unpaused', async function () {
+  it('bridge should be paused', async function () {
     const paused = await L2GraphTokenGateway.paused()
-    expect(paused).eq(false)
+    expect(paused).eq(true)
   })
 
   it('should be controlled by Controller', async function () {
@@ -55,7 +55,7 @@ describe('[L2] L2GraphTokenGateway configuration', function () {
         '0x00',
       )
 
-      await expect(tx).revertedWith('ONLY_COUNTERPART_GATEWAY')
+      await expect(tx).revertedWith('Paused (contract)')
     })
   })
 })

tasks/deployment/unpause.ts

@@ -2,18 +2,29 @@ import { task } from 'hardhat/config'
 import { cliOpts } from '../../cli/defaults'
 import GraphChain from '../../gre/helpers/network'
 
-task('migrate:unpause', 'Unpause protocol and bridge')
+task('migrate:unpause', 'Unpause protocol (except bridge)')
   .addOptionalParam('addressBook', cliOpts.addressBook.description)
   .addOptionalParam('graphConfig', cliOpts.graphConfig.description)
   .setAction(async (taskArgs, hre) => {
     const graph = hre.graph(taskArgs)
     const { governor } = await graph.getNamedAccounts()
-    const { Controller, L1GraphTokenGateway, L2GraphTokenGateway } = graph.contracts
+    const { Controller } = graph.contracts
 
     console.log('> Unpausing protocol')
     const tx = await Controller.connect(governor).setPaused(false)
     await tx.wait()
 
+    console.log('Done!')
+  })
+
+task('migrate:unpause-bridge', 'Unpause bridge')
+  .addOptionalParam('addressBook', cliOpts.addressBook.description)
+  .addOptionalParam('graphConfig', cliOpts.graphConfig.description)
+  .setAction(async (taskArgs, hre) => {
+    const graph = hre.graph(taskArgs)
+    const { governor } = await graph.getNamedAccounts()
+    const { L1GraphTokenGateway, L2GraphTokenGateway } = graph.contracts
+
     console.log('> Unpausing bridge')
     const GraphTokenGateway = GraphChain.isL2(graph.chainId)
       ? L2GraphTokenGateway

test/gateway/l1GraphTokenGateway.test.ts

@@ -237,13 +237,35 @@ describe('L1GraphTokenGateway', () => {
         tx = l1GraphTokenGateway.connect(tokenSender.signer).setPaused(true)
         await expect(tx).revertedWith('Only Governor or Guardian can call')
       })
-      it('can be paused and unpaused by the governor', async function () {
+      it('cannot be unpaused if some state variables are not set', async function () {
         let tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
-        await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
-        await expect(await l1GraphTokenGateway.paused()).eq(false)
-        tx = l1GraphTokenGateway.connect(governor.signer).setPaused(true)
+        await expect(tx).revertedWith('INBOX_NOT_SET')
+        await l1GraphTokenGateway
+          .connect(governor.signer)
+          .setArbitrumAddresses(arbitrumMocks.inboxMock.address, mockRouter.address)
+        tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
+        await expect(tx).revertedWith('L2_COUNTERPART_NOT_SET')
+        await l1GraphTokenGateway
+          .connect(governor.signer)
+          .setL2CounterpartAddress(mockL2Gateway.address)
+        tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
+        await expect(tx).revertedWith('ESCROW_NOT_SET')
+      })
+      it('can be paused and unpaused by the governor', async function () {
+        await fixture.configureL1Bridge(
+          governor.signer,
+          arbitrumMocks,
+          fixtureContracts,
+          mockRouter.address,
+          mockL2GRT.address,
+          mockL2Gateway.address,
+        )
+        let tx = l1GraphTokenGateway.connect(governor.signer).setPaused(true)
         await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(true)
         await expect(await l1GraphTokenGateway.paused()).eq(true)
+        tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
+        await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
+        await expect(await l1GraphTokenGateway.paused()).eq(false)
       })
       describe('setPauseGuardian', function () {
         it('cannot be called by someone other than governor', async function () {
@@ -261,13 +283,21 @@ describe('L1GraphTokenGateway', () => {
             .withArgs(AddressZero, pauseGuardian.address)
         })
         it('allows a pause guardian to pause and unpause', async function () {
+          await fixture.configureL1Bridge(
+            governor.signer,
+            arbitrumMocks,
+            fixtureContracts,
+            mockRouter.address,
+            mockL2GRT.address,
+            mockL2Gateway.address,
+          )
           await l1GraphTokenGateway.connect(governor.signer).setPauseGuardian(pauseGuardian.address)
-          let tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
-          await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
-          await expect(await l1GraphTokenGateway.paused()).eq(false)
-          tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
+          let tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
           await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(true)
           await expect(await l1GraphTokenGateway.paused()).eq(true)
+          tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
+          await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
+          await expect(await l1GraphTokenGateway.paused()).eq(false)
         })
       })
     })

test/l2/l2GraphTokenGateway.test.ts

@@ -173,13 +173,32 @@ describe('L2GraphTokenGateway', () => {
         tx = l2GraphTokenGateway.connect(tokenSender.signer).setPaused(true)
         await expect(tx).revertedWith('Only Governor or Guardian can call')
       })
-      it('can be paused and unpaused by the governor', async function () {
+      it('cannot be paused if some state variables are not set', async function () {
         let tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
-        await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
-        await expect(await l2GraphTokenGateway.paused()).eq(false)
-        tx = l2GraphTokenGateway.connect(governor.signer).setPaused(true)
+        await expect(tx).revertedWith('ROUTER_NOT_SET')
+        await l2GraphTokenGateway.connect(governor.signer).setL2Router(mockRouter.address)
+        tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
+        await expect(tx).revertedWith('L1_COUNTERPART_NOT_SET')
+        await l2GraphTokenGateway
+          .connect(governor.signer)
+          .setL1CounterpartAddress(mockL1Gateway.address)
+        tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
+        await expect(tx).revertedWith('L1GRT_NOT_SET')
+      })
+      it('can be paused and unpaused by the governor', async function () {
+        await fixture.configureL2Bridge(
+          governor.signer,
+          fixtureContracts,
+          mockRouter.address,
+          mockL1GRT.address,
+          mockL1Gateway.address,
+        )
+        let tx = l2GraphTokenGateway.connect(governor.signer).setPaused(true)
         await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(true)
         await expect(await l2GraphTokenGateway.paused()).eq(true)
+        tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
+        await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
+        await expect(await l2GraphTokenGateway.paused()).eq(false)
       })
       describe('setPauseGuardian', function () {
         it('cannot be called by someone other than governor', async function () {
@@ -197,13 +216,20 @@ describe('L2GraphTokenGateway', () => {
             .withArgs(AddressZero, pauseGuardian.address)
         })
         it('allows a pause guardian to pause and unpause', async function () {
+          await fixture.configureL2Bridge(
+            governor.signer,
+            fixtureContracts,
+            mockRouter.address,
+            mockL1GRT.address,
+            mockL1Gateway.address,
+          )
           await l2GraphTokenGateway.connect(governor.signer).setPauseGuardian(pauseGuardian.address)
-          let tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
-          await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
-          await expect(await l2GraphTokenGateway.paused()).eq(false)
-          tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
+          let tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
           await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(true)
           await expect(await l2GraphTokenGateway.paused()).eq(true)
+          tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
+          await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
+          await expect(await l2GraphTokenGateway.paused()).eq(false)
         })
       })
     })