Skip to content

Commit 1d5dde2

Browse files
pcarranzavpcarranzav
committed
fix: prevent unpausing gateways if they're not configured (OZ M-01 from #700)
1 parent 7b38314 commit 1d5dde2

File tree

5 files changed

+102
-16
lines changed

5 files changed

+102
-16
lines changed

‎contracts/gateway/GraphTokenGateway.sol

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok
4545
* @param _newPaused New value for the pause state (true means the transfers will be paused)
4646
*/
4747
function setPaused(bool _newPaused) external onlyGovernorOrGuardian {
48+
if (!_newPaused) {
49+
_checksBeforeUnpause();
50+
}
4851
_setPaused(_newPaused);
4952
}
5053

@@ -54,4 +57,10 @@ abstract contract GraphTokenGateway is GraphUpgradeable, Pausable, Managed, ITok
5457
function paused() external view returns (bool) {
5558
return _paused;
5659
}
60+
61+
/**
62+
* @dev Runs state validation before unpausing, reverts if
63+
* something is not set properly
64+
*/
65+
function _checksBeforeUnpause() internal view virtual;
5766
}

‎contracts/gateway/L1GraphTokenGateway.sol

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,4 +356,15 @@ contract L1GraphTokenGateway is GraphTokenGateway, L1ArbitrumMessenger {
356356
}
357357
return l2GRT;
358358
}
359+
360+
/**
361+
* @dev Runs state validation before unpausing, reverts if
362+
* something is not set properly
363+
*/
364+
function _checksBeforeUnpause() internal view override {
365+
require(inbox != address(0), "INBOX_NOT_SET");
366+
require(l1Router != address(0), "ROUTER_NOT_SET");
367+
require(l2Counterpart != address(0), "L2_COUNTERPART_NOT_SET");
368+
require(escrow != address(0), "ESCROW_NOT_SET");
369+
}
359370
}

‎contracts/l2/gateway/L2GraphTokenGateway.sol

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,4 +294,14 @@ contract L2GraphTokenGateway is GraphTokenGateway, L2ArbitrumMessenger, Reentran
294294
}
295295
return (from, extraData);
296296
}
297+
298+
/**
299+
* @dev Runs state validation before unpausing, reverts if
300+
* something is not set properly
301+
*/
302+
function _checksBeforeUnpause() internal view override {
303+
require(l2Router != address(0), "ROUTER_NOT_SET");
304+
require(l1Counterpart != address(0), "L1_COUNTERPART_NOT_SET");
305+
require(l1GRT != address(0), "L1GRT_NOT_SET");
306+
}
297307
}

‎test/gateway/l1GraphTokenGateway.test.ts

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -237,13 +237,35 @@ describe('L1GraphTokenGateway', () => {
237237
tx = l1GraphTokenGateway.connect(tokenSender.signer).setPaused(true)
238238
await expect(tx).revertedWith('Only Governor or Guardian can call')
239239
})
240-
it('can be paused and unpaused by the governor', async function () {
240+
it('cannot be unpaused if some state variables are not set', async function () {
241241
let tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
242-
await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
243-
await expect(await l1GraphTokenGateway.paused()).eq(false)
244-
tx = l1GraphTokenGateway.connect(governor.signer).setPaused(true)
242+
await expect(tx).revertedWith('INBOX_NOT_SET')
243+
await l1GraphTokenGateway
244+
.connect(governor.signer)
245+
.setArbitrumAddresses(arbitrumMocks.inboxMock.address, mockRouter.address)
246+
tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
247+
await expect(tx).revertedWith('L2_COUNTERPART_NOT_SET')
248+
await l1GraphTokenGateway
249+
.connect(governor.signer)
250+
.setL2CounterpartAddress(mockL2Gateway.address)
251+
tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
252+
await expect(tx).revertedWith('ESCROW_NOT_SET')
253+
})
254+
it('can be paused and unpaused by the governor', async function () {
255+
await fixture.configureL1Bridge(
256+
governor.signer,
257+
arbitrumMocks,
258+
fixtureContracts,
259+
mockRouter.address,
260+
mockL2GRT.address,
261+
mockL2Gateway.address,
262+
)
263+
let tx = l1GraphTokenGateway.connect(governor.signer).setPaused(true)
245264
await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(true)
246265
await expect(await l1GraphTokenGateway.paused()).eq(true)
266+
tx = l1GraphTokenGateway.connect(governor.signer).setPaused(false)
267+
await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
268+
await expect(await l1GraphTokenGateway.paused()).eq(false)
247269
})
248270
describe('setPauseGuardian', function () {
249271
it('cannot be called by someone other than governor', async function () {
@@ -261,13 +283,21 @@ describe('L1GraphTokenGateway', () => {
261283
.withArgs(AddressZero, pauseGuardian.address)
262284
})
263285
it('allows a pause guardian to pause and unpause', async function () {
286+
await fixture.configureL1Bridge(
287+
governor.signer,
288+
arbitrumMocks,
289+
fixtureContracts,
290+
mockRouter.address,
291+
mockL2GRT.address,
292+
mockL2Gateway.address,
293+
)
264294
await l1GraphTokenGateway.connect(governor.signer).setPauseGuardian(pauseGuardian.address)
265-
let tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
266-
await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
267-
await expect(await l1GraphTokenGateway.paused()).eq(false)
268-
tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
295+
let tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
269296
await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(true)
270297
await expect(await l1GraphTokenGateway.paused()).eq(true)
298+
tx = l1GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
299+
await expect(tx).emit(l1GraphTokenGateway, 'PauseChanged').withArgs(false)
300+
await expect(await l1GraphTokenGateway.paused()).eq(false)
271301
})
272302
})
273303
})

‎test/l2/l2GraphTokenGateway.test.ts

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,13 +173,32 @@ describe('L2GraphTokenGateway', () => {
173173
tx = l2GraphTokenGateway.connect(tokenSender.signer).setPaused(true)
174174
await expect(tx).revertedWith('Only Governor or Guardian can call')
175175
})
176-
it('can be paused and unpaused by the governor', async function () {
176+
it('cannot be paused if some state variables are not set', async function () {
177177
let tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
178-
await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
179-
await expect(await l2GraphTokenGateway.paused()).eq(false)
180-
tx = l2GraphTokenGateway.connect(governor.signer).setPaused(true)
178+
await expect(tx).revertedWith('ROUTER_NOT_SET')
179+
await l2GraphTokenGateway.connect(governor.signer).setL2Router(mockRouter.address)
180+
tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
181+
await expect(tx).revertedWith('L1_COUNTERPART_NOT_SET')
182+
await l2GraphTokenGateway
183+
.connect(governor.signer)
184+
.setL1CounterpartAddress(mockL1Gateway.address)
185+
tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
186+
await expect(tx).revertedWith('L1GRT_NOT_SET')
187+
})
188+
it('can be paused and unpaused by the governor', async function () {
189+
await fixture.configureL2Bridge(
190+
governor.signer,
191+
fixtureContracts,
192+
mockRouter.address,
193+
mockL1GRT.address,
194+
mockL1Gateway.address,
195+
)
196+
let tx = l2GraphTokenGateway.connect(governor.signer).setPaused(true)
181197
await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(true)
182198
await expect(await l2GraphTokenGateway.paused()).eq(true)
199+
tx = l2GraphTokenGateway.connect(governor.signer).setPaused(false)
200+
await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
201+
await expect(await l2GraphTokenGateway.paused()).eq(false)
183202
})
184203
describe('setPauseGuardian', function () {
185204
it('cannot be called by someone other than governor', async function () {
@@ -197,13 +216,20 @@ describe('L2GraphTokenGateway', () => {
197216
.withArgs(AddressZero, pauseGuardian.address)
198217
})
199218
it('allows a pause guardian to pause and unpause', async function () {
219+
await fixture.configureL2Bridge(
220+
governor.signer,
221+
fixtureContracts,
222+
mockRouter.address,
223+
mockL1GRT.address,
224+
mockL1Gateway.address,
225+
)
200226
await l2GraphTokenGateway.connect(governor.signer).setPauseGuardian(pauseGuardian.address)
201-
let tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
202-
await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
203-
await expect(await l2GraphTokenGateway.paused()).eq(false)
204-
tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
227+
let tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(true)
205228
await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(true)
206229
await expect(await l2GraphTokenGateway.paused()).eq(true)
230+
tx = l2GraphTokenGateway.connect(pauseGuardian.signer).setPaused(false)
231+
await expect(tx).emit(l2GraphTokenGateway, 'PauseChanged').withArgs(false)
232+
await expect(await l2GraphTokenGateway.paused()).eq(false)
207233
})
208234
})
209235
})

0 commit comments

Comments
 (0)