From eeeb65f255db2fe7183645736a576cb2815b430e Mon Sep 17 00:00:00 2001 From: Maurelian Date: Sun, 25 Apr 2021 11:44:30 +0300 Subject: [PATCH 01/10] feat(contracts): add pausable.sol This is identical to Pausable, except for replacing Context._msgSender() with just msg.sender --- .../libraries/utils/Lib_Pausable.sol | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol new file mode 100644 index 0000000000000..6182e90c840cb --- /dev/null +++ b/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: MIT +pragma solidity >0.5.0 <0.8.0; + +/** + * @title Pausable + * @dev Adapted from https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol + * Contract module which allows children to implement an emergency stop + * mechanism that can be triggered by an authorized account. + * + * This module is used through inheritance. It will make available the + * modifiers `whenNotPaused` and `whenPaused`, which can be applied to + * the functions of your contract. Note that they will not be pausable by + * simply including this module, only once the modifiers are put in place. + */ +abstract contract Pausable { + /** + * @dev Emitted when the pause is triggered by `account`. + */ + event Paused(address account); + + /** + * @dev Emitted when the pause is lifted by `account`. + */ + event Unpaused(address account); + + bool private _paused; + + /** + * @dev Initializes the contract in unpaused state. + */ + constructor () { + _paused = false; + } + + /** + * @dev Returns true if the contract is paused, and false otherwise. + */ + function paused() public view virtual returns (bool) { + return _paused; + } + + /** + * @dev Modifier to make a function callable only when the contract is not paused. + * + * Requirements: + * + * - The contract must not be paused. + */ + modifier whenNotPaused() { + require(!paused(), "Pausable: paused"); + _; + } + + /** + * @dev Modifier to make a function callable only when the contract is paused. + * + * Requirements: + * + * - The contract must be paused. + */ + modifier whenPaused() { + require(paused(), "Pausable: not paused"); + _; + } + + /** + * @dev Triggers stopped state. + * + * Requirements: + * + * - The contract must not be paused. + */ + function _pause() internal virtual whenNotPaused { + _paused = true; + emit Paused(msg.sender); + } + + /** + * @dev Returns to normal state. + * + * Requirements: + * + * - The contract must be paused. + */ + function _unpause() internal virtual whenPaused { + _paused = false; + emit Unpaused(msg.sender); + } +} From 5f995e6c56b2e9c5b0c57cd0ddfa2df2e5ac3e06 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Sun, 25 Apr 2021 11:49:16 +0300 Subject: [PATCH 02/10] feat: add pause, block and allow to L1 messenger --- .../messaging/OVM_L1CrossDomainMessenger.sol | 78 +++++++++++++++++-- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol index 9d2a69c336ccf..25e31d3834089 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol @@ -16,6 +16,11 @@ import { iOVM_StateCommitmentChain } from "../../../iOVM/chain/iOVM_StateCommitm /* Contract Imports */ import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol"; +/* External Imports */ +import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +import { Ownable } from "@openzeppelin/contracts/utils/Ownable.sol"; +import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol"; + /** * @title OVM_L1CrossDomainMessenger * @dev The L1 Cross Domain Messenger contract sends messages from L1 to L2, and relays messages @@ -25,20 +30,39 @@ import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol * Compiler used: solc * Runtime target: EVM */ -contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCrossDomainMessenger, Lib_AddressResolver { +contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCrossDomainMessenger, Lib_AddressResolver, Ownable, Pausable { + + /********** + * Events * + **********/ + + event MessageBlocked( + bytes32 indexed _xDomainCalldataHash + ); + + event MessageAllowed( + bytes32 indexed _xDomainCalldataHash + ); + + /********************** + * Contract Variables * + **********************/ + + mapping (bytes32 => bool) public blockedMessages; /*************** * Constructor * ***************/ /** - * Pass a default zero address to the address resolver. This will be updated when initialized. + * This contract is intended to be behind a delegate proxy. + * We pass the zero address to the address resolver just to satisfy the constructor. + * We still need to set this value in initialize(). */ constructor() Lib_AddressResolver(address(0)) {} - /********************** * Function Modifiers * **********************/ @@ -75,9 +99,47 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros address(libAddressManager) == address(0), "L1CrossDomainMessenger already intialized." ); - libAddressManager = Lib_AddressManager(_libAddressManager); xDomainMsgSender = DEFAULT_XDOMAIN_SENDER; + owner = msg.sender; + } + + /** + * Pause relaying. + */ + function pause() + external + onlyOwner + { + _pause(); + } + + /** + * Block a message. + * @param _xDomainCalldataHash Hash of the message to block. + */ + function blockMessage( + bytes32 _xDomainCalldataHash + ) + external + onlyOwner + { + blockedMessages[_xDomainCalldataHash] = true; + emit MessageBlocked(_xDomainCalldataHash); + } + + /** + * Allow a message. + * @param _xDomainCalldataHash Hash of the message to block. + */ + function allowMessage( + bytes32 _xDomainCalldataHash + ) + external + onlyOwner + { + blockedMessages[_xDomainCalldataHash] = false; + emit MessageAllowed(_xDomainCalldataHash); } /** @@ -94,7 +156,8 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros override public nonReentrant - onlyRelayer() + onlyRelayer + whenNotPaused { bytes memory xDomainCalldata = _getXDomainCalldata( _target, @@ -118,6 +181,11 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros "Provided message has already been received." ); + require( + blockedMessages[xDomainCalldataHash] == false, + "Provided message has been blocked." + ); + xDomainMsgSender = _sender; (bool success, ) = _target.call(_message); xDomainMsgSender = DEFAULT_XDOMAIN_SENDER; From c5a60975dfb48f8b7d7127801e69d45ea0d3cd9d Mon Sep 17 00:00:00 2001 From: Maurelian Date: Sun, 25 Apr 2021 11:52:48 +0300 Subject: [PATCH 03/10] test(l1-messenger): access control, block, allow --- .../base/OVM_L1CrossDomainMessenger.spec.ts | 114 ++++++++++++++++-- 1 file changed, 102 insertions(+), 12 deletions(-) diff --git a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts index 613ccfb2d6444..342c680d673ca 100644 --- a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts @@ -36,8 +36,9 @@ const deployProxyXDomainMessenger = async ( describe('OVM_L1CrossDomainMessenger', () => { let signer: Signer + let signer2: Signer before(async () => { - ;[signer] = await ethers.getSigners() + ;[signer, signer2] = await ethers.getSigners() }) let AddressManager: Contract @@ -89,15 +90,33 @@ describe('OVM_L1CrossDomainMessenger', () => { let OVM_L1CrossDomainMessenger: Contract beforeEach(async () => { - const xDomainMessenerImpl = await Factory__OVM_L1CrossDomainMessenger.deploy() + const xDomainMessengerImpl = await Factory__OVM_L1CrossDomainMessenger.deploy() // We use an upgradable proxy for the XDomainMessenger--deploy & set up the proxy. OVM_L1CrossDomainMessenger = await deployProxyXDomainMessenger( AddressManager, - xDomainMessenerImpl + xDomainMessengerImpl ) await OVM_L1CrossDomainMessenger.initialize(AddressManager.address) }) + describe('pause', () => { + describe('when called by the current owner', () => { + it('should pause the contract', async () => { + await OVM_L1CrossDomainMessenger.pause() + + expect(await OVM_L1CrossDomainMessenger.paused()).to.be.true + }) + }) + + describe('when called by account other than the owner', () => { + it('should not pause the contract', async () => { + await expect(OVM_L1CrossDomainMessenger.connect(signer2).pause()).to.be.revertedWith( + 'Ownable: caller is not the owner' + ) + }) + }) + }) + describe('sendMessage', () => { const target = NON_ZERO_ADDRESS const message = NON_NULL_BYTES32 @@ -355,12 +374,8 @@ describe('OVM_L1CrossDomainMessenger', () => { ).to.be.revertedWith('Provided message has already been received.') }) - it('when the OVM_L2MessageRelayer address is set, should revert if called by a different account', async () => { - // set to a random NON-ZERO address - await AddressManager.setAddress( - 'OVM_L2MessageRelayer', - '0x1234123412341234123412341234123412341234' - ) + it('should revert if paused', async () => { + await OVM_L1CrossDomainMessenger.pause() await expect( OVM_L1CrossDomainMessenger.relayMessage( @@ -370,9 +385,84 @@ describe('OVM_L1CrossDomainMessenger', () => { 0, proof ) - ).to.be.revertedWith( - 'Only OVM_L2MessageRelayer can relay L2-to-L1 messages.' - ) + ).to.be.revertedWith('Pausable: paused') + }) + + describe('blockMessage and allowMessage', () => { + it('should revert if called by an account other than the owner', async () => { + const OVM_L1CrossDomainMessenger2 = OVM_L1CrossDomainMessenger.connect( + signer2 + ) + await expect( + OVM_L1CrossDomainMessenger2.blockMessage(keccak256(calldata)) + ).to.be.revertedWith('Ownable: caller is not the owner') + + await expect( + OVM_L1CrossDomainMessenger2.allowMessage(keccak256(calldata)) + ).to.be.revertedWith('Ownable: caller is not the owner') + }) + + it('should revert if the message is blocked', async () => { + await OVM_L1CrossDomainMessenger.blockMessage(keccak256(calldata)) + + await expect( + OVM_L1CrossDomainMessenger.relayMessage( + target, + sender, + message, + 0, + proof + ) + ).to.be.revertedWith('Provided message has been blocked.') + }) + + it('should succeed if the message is blocked, then unblocked', async () => { + await OVM_L1CrossDomainMessenger.blockMessage(keccak256(calldata)) + + await expect( + OVM_L1CrossDomainMessenger.relayMessage( + target, + sender, + message, + 0, + proof + ) + ).to.be.revertedWith('Provided message has been blocked.') + + await OVM_L1CrossDomainMessenger.allowMessage(keccak256(calldata)) + + await expect( + OVM_L1CrossDomainMessenger.relayMessage( + target, + sender, + message, + 0, + proof + ) + ).to.not.be.reverted + }) + }) + + describe('onlyRelayer', () => { + it('when the OVM_L2MessageRelayer address is set, should revert if called by a different account', async () => { + // set to a random NON-ZERO address + await AddressManager.setAddress( + 'OVM_L2MessageRelayer', + '0x1234123412341234123412341234123412341234' + ) + + await expect( + OVM_L1CrossDomainMessenger.relayMessage( + target, + sender, + message, + 0, + proof + ) + ).to.be.revertedWith( + 'Only OVM_L2MessageRelayer can relay L2-to-L1 messages.' + ) + }) }) }) }) From a95094fcb8147c1189ecf05df5653ec3a85f9ffa Mon Sep 17 00:00:00 2001 From: Maurelian Date: Sun, 25 Apr 2021 12:05:44 +0300 Subject: [PATCH 04/10] chore: add changeset --- .changeset/wild-cycles-care.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/wild-cycles-care.md diff --git a/.changeset/wild-cycles-care.md b/.changeset/wild-cycles-care.md new file mode 100644 index 0000000000000..a7927a9ecdc08 --- /dev/null +++ b/.changeset/wild-cycles-care.md @@ -0,0 +1,5 @@ +--- +"@eth-optimism/contracts": patch +--- + +Add pause(), blockMessage() and allowMessage() to L1 messenger From 90299b562f7483d0d36c1480bcb4c8eaff5169cd Mon Sep 17 00:00:00 2001 From: Georgios Konstantopoulos Date: Sun, 25 Apr 2021 12:15:26 +0300 Subject: [PATCH 05/10] chore: yarn lint --- .../OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts index 342c680d673ca..7ed33d551d4e9 100644 --- a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts @@ -110,9 +110,9 @@ describe('OVM_L1CrossDomainMessenger', () => { describe('when called by account other than the owner', () => { it('should not pause the contract', async () => { - await expect(OVM_L1CrossDomainMessenger.connect(signer2).pause()).to.be.revertedWith( - 'Ownable: caller is not the owner' - ) + await expect( + OVM_L1CrossDomainMessenger.connect(signer2).pause() + ).to.be.revertedWith('Ownable: caller is not the owner') }) }) }) From bb7e87ad0118517a570b68077f9b8829db7360b1 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 26 Apr 2021 21:35:09 -0400 Subject: [PATCH 06/10] test[contracts]: maintain previous test order --- .../base/OVM_L1CrossDomainMessenger.spec.ts | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts index 7ed33d551d4e9..9e37c231677e3 100644 --- a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts @@ -388,6 +388,28 @@ describe('OVM_L1CrossDomainMessenger', () => { ).to.be.revertedWith('Pausable: paused') }) + describe('onlyRelayer', () => { + it('when the OVM_L2MessageRelayer address is set, should revert if called by a different account', async () => { + // set to a random NON-ZERO address + await AddressManager.setAddress( + 'OVM_L2MessageRelayer', + '0x1234123412341234123412341234123412341234' + ) + + await expect( + OVM_L1CrossDomainMessenger.relayMessage( + target, + sender, + message, + 0, + proof + ) + ).to.be.revertedWith( + 'Only OVM_L2MessageRelayer can relay L2-to-L1 messages.' + ) + }) + }) + describe('blockMessage and allowMessage', () => { it('should revert if called by an account other than the owner', async () => { const OVM_L1CrossDomainMessenger2 = OVM_L1CrossDomainMessenger.connect( @@ -442,27 +464,5 @@ describe('OVM_L1CrossDomainMessenger', () => { ).to.not.be.reverted }) }) - - describe('onlyRelayer', () => { - it('when the OVM_L2MessageRelayer address is set, should revert if called by a different account', async () => { - // set to a random NON-ZERO address - await AddressManager.setAddress( - 'OVM_L2MessageRelayer', - '0x1234123412341234123412341234123412341234' - ) - - await expect( - OVM_L1CrossDomainMessenger.relayMessage( - target, - sender, - message, - 0, - proof - ) - ).to.be.revertedWith( - 'Only OVM_L2MessageRelayer can relay L2-to-L1 messages.' - ) - }) - }) }) }) From e9e29cc2a5538a6eb514713ebb5bb400b3bf3a72 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Mon, 26 Apr 2021 23:28:22 -0400 Subject: [PATCH 07/10] fix[contracts]: add upgradable OZ contracts --- .../Abs_BaseCrossDomainMessenger.sol | 7 ++----- .../messaging/OVM_L1CrossDomainMessenger.sol | 20 ++++++++++++++----- .../messaging/OVM_L2CrossDomainMessenger.sol | 11 +++++++++- packages/contracts/package.json | 1 + yarn.lock | 5 +++++ 5 files changed, 33 insertions(+), 11 deletions(-) diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/Abs_BaseCrossDomainMessenger.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/Abs_BaseCrossDomainMessenger.sol index fae425de27aa8..b4b468b5d9d1c 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/Abs_BaseCrossDomainMessenger.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/Abs_BaseCrossDomainMessenger.sol @@ -5,9 +5,6 @@ pragma experimental ABIEncoderV2; /* Interface Imports */ import { iAbs_BaseCrossDomainMessenger } from "../../../iOVM/bridge/messaging/iAbs_BaseCrossDomainMessenger.sol"; -/* External Imports */ -import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; - /** * @title Abs_BaseCrossDomainMessenger * @dev The Base Cross Domain Messenger is an abstract contract providing the interface and common @@ -17,7 +14,7 @@ import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.s * Compiler used: defined by child contract * Runtime target: defined by child contract */ -abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, ReentrancyGuard { +abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger { /************* * Constants * @@ -44,7 +41,7 @@ abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, * Constructor * ***************/ - constructor() ReentrancyGuard() {} + constructor() {} /******************** diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol index 25e31d3834089..8e77ff7903fb5 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol @@ -17,9 +17,9 @@ import { iOVM_StateCommitmentChain } from "../../../iOVM/chain/iOVM_StateCommitm import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol"; /* External Imports */ -import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; -import { Ownable } from "@openzeppelin/contracts/utils/Ownable.sol"; -import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol"; +import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; +import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol"; +import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol"; /** * @title OVM_L1CrossDomainMessenger @@ -30,7 +30,14 @@ import { Pausable } from "@openzeppelin/contracts/utils/Pausable.sol"; * Compiler used: solc * Runtime target: EVM */ -contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCrossDomainMessenger, Lib_AddressResolver, Ownable, Pausable { +contract OVM_L1CrossDomainMessenger is + iOVM_L1CrossDomainMessenger, + Abs_BaseCrossDomainMessenger, + Lib_AddressResolver, + OwnableUpgradeable, + PausableUpgradeable, + ReentrancyGuardUpgradeable +{ /********** * Events * @@ -101,7 +108,10 @@ contract OVM_L1CrossDomainMessenger is iOVM_L1CrossDomainMessenger, Abs_BaseCros ); libAddressManager = Lib_AddressManager(_libAddressManager); xDomainMsgSender = DEFAULT_XDOMAIN_SENDER; - owner = msg.sender; + __Context_init_unchained(); + __Ownable_init_unchained(); + __Pausable_init_unchained(); + __ReentrancyGuard_init_unchained(); } /** diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L2CrossDomainMessenger.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L2CrossDomainMessenger.sol index 292ae6eff89c3..dfd3d56d8be6a 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L2CrossDomainMessenger.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L2CrossDomainMessenger.sol @@ -13,6 +13,9 @@ import { iOVM_L2ToL1MessagePasser } from "../../../iOVM/predeploys/iOVM_L2ToL1Me /* Contract Imports */ import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol"; +/* External Imports */ +import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; + /** * @title OVM_L2CrossDomainMessenger * @dev The L2 Cross Domain Messenger contract sends messages from L2 to L1, and is the entry point @@ -21,7 +24,12 @@ import { Abs_BaseCrossDomainMessenger } from "./Abs_BaseCrossDomainMessenger.sol * Compiler used: optimistic-solc * Runtime target: OVM */ -contract OVM_L2CrossDomainMessenger is iOVM_L2CrossDomainMessenger, Abs_BaseCrossDomainMessenger, Lib_AddressResolver { +contract OVM_L2CrossDomainMessenger is + iOVM_L2CrossDomainMessenger, + Abs_BaseCrossDomainMessenger, + Lib_AddressResolver, + ReentrancyGuard +{ /*************** * Constructor * @@ -34,6 +42,7 @@ contract OVM_L2CrossDomainMessenger is iOVM_L2CrossDomainMessenger, Abs_BaseCros address _libAddressManager ) Lib_AddressResolver(_libAddressManager) + ReentrancyGuard() {} diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 9284c1ff94960..18d155a11567b 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -48,6 +48,7 @@ "@ethersproject/abstract-provider": "^5.0.8", "@ethersproject/contracts": "^5.0.5", "@openzeppelin/contracts": "^3.3.0", + "@openzeppelin/contracts-upgradeable": "^3.3.0", "@typechain/hardhat": "^1.0.1", "ganache-core": "^2.13.2", "glob": "^7.1.6" diff --git a/yarn.lock b/yarn.lock index 4451ecc8e3ef0..3f13f957a4b48 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1830,6 +1830,11 @@ dependencies: "@octokit/openapi-types" "^6.0.0" +"@openzeppelin/contracts-upgradeable@^3.3.0": + version "3.4.1" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts-upgradeable/-/contracts-upgradeable-3.4.1.tgz#38dfdfa86fda0a088c6fcdebe6870cfaf897b471" + integrity sha512-wBGlUzEkOxcj/ghtcF2yKc8ZYh+PTUtm1mK38zoENulJ6aplij7eH8quo3lMugfzPJy+V6V5qI8QhdQmCn7hkQ== + "@openzeppelin/contracts@^3.3.0": version "3.4.1" resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-3.4.1.tgz#03c891fec7f93be0ae44ed74e57a122a38732ce7" From b167e620db489782afe6f70b1afc0954de3f6f55 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Tue, 27 Apr 2021 08:28:04 -0400 Subject: [PATCH 08/10] Revert "test[contracts]: maintain previous test order" This reverts commit 4bbfe60f32e5785debf3cfbe2b3b125f54ed6798. --- .../base/OVM_L1CrossDomainMessenger.spec.ts | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts index 9e37c231677e3..7ed33d551d4e9 100644 --- a/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts +++ b/packages/contracts/test/contracts/OVM/bridge/base/OVM_L1CrossDomainMessenger.spec.ts @@ -388,28 +388,6 @@ describe('OVM_L1CrossDomainMessenger', () => { ).to.be.revertedWith('Pausable: paused') }) - describe('onlyRelayer', () => { - it('when the OVM_L2MessageRelayer address is set, should revert if called by a different account', async () => { - // set to a random NON-ZERO address - await AddressManager.setAddress( - 'OVM_L2MessageRelayer', - '0x1234123412341234123412341234123412341234' - ) - - await expect( - OVM_L1CrossDomainMessenger.relayMessage( - target, - sender, - message, - 0, - proof - ) - ).to.be.revertedWith( - 'Only OVM_L2MessageRelayer can relay L2-to-L1 messages.' - ) - }) - }) - describe('blockMessage and allowMessage', () => { it('should revert if called by an account other than the owner', async () => { const OVM_L1CrossDomainMessenger2 = OVM_L1CrossDomainMessenger.connect( @@ -464,5 +442,27 @@ describe('OVM_L1CrossDomainMessenger', () => { ).to.not.be.reverted }) }) + + describe('onlyRelayer', () => { + it('when the OVM_L2MessageRelayer address is set, should revert if called by a different account', async () => { + // set to a random NON-ZERO address + await AddressManager.setAddress( + 'OVM_L2MessageRelayer', + '0x1234123412341234123412341234123412341234' + ) + + await expect( + OVM_L1CrossDomainMessenger.relayMessage( + target, + sender, + message, + 0, + proof + ) + ).to.be.revertedWith( + 'Only OVM_L2MessageRelayer can relay L2-to-L1 messages.' + ) + }) + }) }) }) From 6084267aa10055ad8ac3804372d3271bc59e9c03 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Tue, 27 Apr 2021 09:11:07 -0400 Subject: [PATCH 09/10] test[contracts]: add initializer modifier --- .../OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol index 8e77ff7903fb5..445afd808509b 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/bridge/messaging/OVM_L1CrossDomainMessenger.sol @@ -101,6 +101,7 @@ contract OVM_L1CrossDomainMessenger is address _libAddressManager ) public + initializer { require( address(libAddressManager) == address(0), @@ -108,7 +109,9 @@ contract OVM_L1CrossDomainMessenger is ); libAddressManager = Lib_AddressManager(_libAddressManager); xDomainMsgSender = DEFAULT_XDOMAIN_SENDER; - __Context_init_unchained(); + + // Initialize upgradable OZ contracts + __Context_init_unchained(); // Context is a dependency for both Ownable and Pausable __Ownable_init_unchained(); __Pausable_init_unchained(); __ReentrancyGuard_init_unchained(); From 78246e1225ea5fd43c5f3f5d2e9449748819e3e1 Mon Sep 17 00:00:00 2001 From: Maurelian Date: Wed, 28 Apr 2021 20:05:42 -0400 Subject: [PATCH 10/10] fix(contracts): delete unused file --- .../libraries/utils/Lib_Pausable.sol | 89 ------------------- 1 file changed, 89 deletions(-) delete mode 100644 packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol deleted file mode 100644 index 6182e90c840cb..0000000000000 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Pausable.sol +++ /dev/null @@ -1,89 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >0.5.0 <0.8.0; - -/** - * @title Pausable - * @dev Adapted from https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol - * Contract module which allows children to implement an emergency stop - * mechanism that can be triggered by an authorized account. - * - * This module is used through inheritance. It will make available the - * modifiers `whenNotPaused` and `whenPaused`, which can be applied to - * the functions of your contract. Note that they will not be pausable by - * simply including this module, only once the modifiers are put in place. - */ -abstract contract Pausable { - /** - * @dev Emitted when the pause is triggered by `account`. - */ - event Paused(address account); - - /** - * @dev Emitted when the pause is lifted by `account`. - */ - event Unpaused(address account); - - bool private _paused; - - /** - * @dev Initializes the contract in unpaused state. - */ - constructor () { - _paused = false; - } - - /** - * @dev Returns true if the contract is paused, and false otherwise. - */ - function paused() public view virtual returns (bool) { - return _paused; - } - - /** - * @dev Modifier to make a function callable only when the contract is not paused. - * - * Requirements: - * - * - The contract must not be paused. - */ - modifier whenNotPaused() { - require(!paused(), "Pausable: paused"); - _; - } - - /** - * @dev Modifier to make a function callable only when the contract is paused. - * - * Requirements: - * - * - The contract must be paused. - */ - modifier whenPaused() { - require(paused(), "Pausable: not paused"); - _; - } - - /** - * @dev Triggers stopped state. - * - * Requirements: - * - * - The contract must not be paused. - */ - function _pause() internal virtual whenNotPaused { - _paused = true; - emit Paused(msg.sender); - } - - /** - * @dev Returns to normal state. - * - * Requirements: - * - * - The contract must be paused. - */ - function _unpause() internal virtual whenPaused { - _paused = false; - emit Unpaused(msg.sender); - } -}