From 9ec3ec01eaaff25e87a5e3e4ce84854d9747e751 Mon Sep 17 00:00:00 2001 From: smartcontracts <kelvinfichter@gmail.com> Date: Mon, 26 Apr 2021 12:06:01 -0400 Subject: [PATCH] refactor[contracts]: Import OZ contracts directly (#609) * refactor[contracts]: Import contracts directly from OZ * chore[contracts]: Add changeset * fix[integration-tests]: Update gas estimate --- .changeset/odd-cows-fry.md | 5 ++ integration-tests/test/native-eth.spec.ts | 2 +- .../Abs_BaseCrossDomainMessenger.sol | 8 +- .../messaging/OVM_L1CrossDomainMessenger.sol | 1 - .../messaging/OVM_L2CrossDomainMessenger.sol | 1 - .../chain/OVM_CanonicalTransactionChain.sol | 6 +- .../libraries/resolver/Lib_AddressManager.sol | 4 +- .../libraries/resolver/Lib_Ownable.sol | 87 ------------------- .../libraries/utils/Lib_Math.sol | 35 -------- .../libraries/utils/Lib_ReentrancyGuard.sol | 61 ------------- 10 files changed, 16 insertions(+), 194 deletions(-) create mode 100644 .changeset/odd-cows-fry.md delete mode 100644 packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_Ownable.sol delete mode 100644 packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Math.sol delete mode 100644 packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_ReentrancyGuard.sol diff --git a/.changeset/odd-cows-fry.md b/.changeset/odd-cows-fry.md new file mode 100644 index 000000000..087031653 --- /dev/null +++ b/.changeset/odd-cows-fry.md @@ -0,0 +1,5 @@ +--- +"@eth-optimism/contracts": patch +--- + +Removes copies of OZ contracts in favor of importing from OZ directly diff --git a/integration-tests/test/native-eth.spec.ts b/integration-tests/test/native-eth.spec.ts index 2adeed976..96ec0fd5f 100644 --- a/integration-tests/test/native-eth.spec.ts +++ b/integration-tests/test/native-eth.spec.ts @@ -51,7 +51,7 @@ describe('Native ETH Integration Tests', async () => { it('Should estimate gas for ETH withdraw', async () => { const amount = utils.parseEther('0.5') const gas = await env.ovmEth.estimateGas.withdraw(amount) - expect(gas).to.be.deep.eq(BigNumber.from(503749)) + expect(gas).to.be.deep.eq(BigNumber.from(503789)) }) }) 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 62254f225..fae425de2 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,8 +5,8 @@ pragma experimental ABIEncoderV2; /* Interface Imports */ import { iAbs_BaseCrossDomainMessenger } from "../../../iOVM/bridge/messaging/iAbs_BaseCrossDomainMessenger.sol"; -/* Library Imports */ -import { Lib_ReentrancyGuard } from "../../../libraries/utils/Lib_ReentrancyGuard.sol"; +/* External Imports */ +import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; /** * @title Abs_BaseCrossDomainMessenger @@ -17,7 +17,7 @@ import { Lib_ReentrancyGuard } from "../../../libraries/utils/Lib_ReentrancyGuar * Compiler used: defined by child contract * Runtime target: defined by child contract */ -abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, Lib_ReentrancyGuard { +abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, ReentrancyGuard { /************* * Constants * @@ -44,7 +44,7 @@ abstract contract Abs_BaseCrossDomainMessenger is iAbs_BaseCrossDomainMessenger, * Constructor * ***************/ - constructor() Lib_ReentrancyGuard() {} + constructor() ReentrancyGuard() {} /******************** 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 5f32a475a..9d2a69c33 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 @@ -7,7 +7,6 @@ import { Lib_OVMCodec } from "../../../libraries/codec/Lib_OVMCodec.sol"; import { Lib_AddressResolver } from "../../../libraries/resolver/Lib_AddressResolver.sol"; import { Lib_AddressManager } from "../../../libraries/resolver/Lib_AddressManager.sol"; import { Lib_SecureMerkleTrie } from "../../../libraries/trie/Lib_SecureMerkleTrie.sol"; -import { Lib_ReentrancyGuard } from "../../../libraries/utils/Lib_ReentrancyGuard.sol"; /* Interface Imports */ import { iOVM_L1CrossDomainMessenger } from "../../../iOVM/bridge/messaging/iOVM_L1CrossDomainMessenger.sol"; 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 7f126df2a..292ae6eff 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 @@ -4,7 +4,6 @@ pragma experimental ABIEncoderV2; /* Library Imports */ import { Lib_AddressResolver } from "../../../libraries/resolver/Lib_AddressResolver.sol"; -import { Lib_ReentrancyGuard } from "../../../libraries/utils/Lib_ReentrancyGuard.sol"; /* Interface Imports */ import { iOVM_L2CrossDomainMessenger } from "../../../iOVM/bridge/messaging/iOVM_L2CrossDomainMessenger.sol"; diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/chain/OVM_CanonicalTransactionChain.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/chain/OVM_CanonicalTransactionChain.sol index 1986492e4..5b97806e7 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/chain/OVM_CanonicalTransactionChain.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/chain/OVM_CanonicalTransactionChain.sol @@ -7,7 +7,6 @@ pragma experimental ABIEncoderV2; import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol"; import { Lib_AddressResolver } from "../../libraries/resolver/Lib_AddressResolver.sol"; import { Lib_MerkleTree } from "../../libraries/utils/Lib_MerkleTree.sol"; -import { Lib_Math } from "../../libraries/utils/Lib_Math.sol"; /* Interface Imports */ import { iOVM_CanonicalTransactionChain } from "../../iOVM/chain/iOVM_CanonicalTransactionChain.sol"; @@ -16,6 +15,9 @@ import { iOVM_ChainStorageContainer } from "../../iOVM/chain/iOVM_ChainStorageCo /* Contract Imports */ import { OVM_ExecutionManager } from "../execution/OVM_ExecutionManager.sol"; +/* External Imports */ +import { Math } from "@openzeppelin/contracts/math/Math.sol"; + /** * @title OVM_CanonicalTransactionChain * @dev The Canonical Transaction Chain (CTC) contract is an append-only log of transactions @@ -343,7 +345,7 @@ contract OVM_CanonicalTransactionChain is iOVM_CanonicalTransactionChain, Lib_Ad // TEMPORARY: Disable `appendQueueBatch` for minnet revert("appendQueueBatch is currently disabled."); - // _numQueuedTransactions = Lib_Math.min(_numQueuedTransactions, getNumPendingQueueElements()); + // _numQueuedTransactions = Math.min(_numQueuedTransactions, getNumPendingQueueElements()); // require( // _numQueuedTransactions > 0, // "Must append more than zero transactions." diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_AddressManager.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_AddressManager.sol index 35b612d5a..606f2cef3 100644 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_AddressManager.sol +++ b/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_AddressManager.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity >0.5.0 <0.8.0; -/* Contract Imports */ -import { Ownable } from "./Lib_Ownable.sol"; +/* External Imports */ +import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; /** * @title Lib_AddressManager diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_Ownable.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_Ownable.sol deleted file mode 100644 index e3320c4f5..000000000 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/resolver/Lib_Ownable.sol +++ /dev/null @@ -1,87 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >0.5.0 <0.8.0; - -/** - * @title Ownable - * @dev Adapted from https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable.sol - */ -abstract contract Ownable { - - /************* - * Variables * - *************/ - - address public owner; - - - /********** - * Events * - **********/ - - event OwnershipTransferred( - address indexed previousOwner, - address indexed newOwner - ); - - - /*************** - * Constructor * - ***************/ - - constructor() { - owner = msg.sender; - emit OwnershipTransferred(address(0), owner); - } - - - /********************** - * Function Modifiers * - **********************/ - - modifier onlyOwner() { - require( - owner == msg.sender, - "Ownable: caller is not the owner" - ); - _; - } - - - /******************** - * Public Functions * - ********************/ - - /** - * Sets the owner of this contract to the zero address, effectively renouncing ownership - * completely. Can only be called by the current owner of this contract. - */ - function renounceOwnership() - public - virtual - onlyOwner - { - emit OwnershipTransferred(owner, address(0)); - owner = address(0); - } - - /** - * Transfers ownership of this contract to a new address. Can only be called by the current - * owner of this contract. - * @param _newOwner Address of the new contract owner. - */ - function transferOwnership( - address _newOwner - ) - public - virtual - onlyOwner - { - require( - _newOwner != address(0), - "Ownable: new owner cannot be the zero address" - ); - - emit OwnershipTransferred(owner, _newOwner); - owner = _newOwner; - } -} diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Math.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Math.sol deleted file mode 100644 index 2227cc544..000000000 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_Math.sol +++ /dev/null @@ -1,35 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >0.5.0 <0.8.0; - -/** - * @title Lib_Math - */ -library Lib_Math { - - /********************** - * Internal Functions * - **********************/ - - /** - * Calculates the minumum of two numbers. - * @param _x First number to compare. - * @param _y Second number to compare. - * @return Lesser of the two numbers. - */ - function min( - uint256 _x, - uint256 _y - ) - internal - pure - returns ( - uint256 - ) - { - if (_x < _y) { - return _x; - } - - return _y; - } -} diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_ReentrancyGuard.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_ReentrancyGuard.sol deleted file mode 100644 index a2aaa4d0e..000000000 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/utils/Lib_ReentrancyGuard.sol +++ /dev/null @@ -1,61 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity >0.5.0 <0.8.0; - -/** - * @dev Contract module that helps prevent reentrant calls to a function. - * - * Inheriting from `ReentrancyGuard` will make the {nonReentrant} modifier - * available, which can be applied to functions to make sure there are no nested - * (reentrant) calls to them. - * - * Note that because there is a single `nonReentrant` guard, functions marked as - * `nonReentrant` may not call one another. This can be worked around by making - * those functions `private`, and then adding `external` `nonReentrant` entry - * points to them. - * - * TIP: If you would like to learn more about reentrancy and alternative ways - * to protect against it, check out our blog post - * https://blog.openzeppelin.com/reentrancy-after-istanbul/[Reentrancy After Istanbul]. - */ -abstract contract Lib_ReentrancyGuard { - // Booleans are more expensive than uint256 or any type that takes up a full - // word because each write operation emits an extra SLOAD to first read the - // slot's contents, replace the bits taken up by the boolean, and then write - // back. This is the compiler's defense against contract upgrades and - // pointer aliasing, and it cannot be disabled. - - // The values being non-zero value makes deployment a bit more expensive, - // but in exchange the refund on every call to nonReentrant will be lower in - // amount. Since refunds are capped to a percentage of the total - // transaction's gas, it is best to keep them low in cases like this one, to - // increase the likelihood of the full refund coming into effect. - uint256 private constant _NOT_ENTERED = 1; - uint256 private constant _ENTERED = 2; - - uint256 private _status; - - constructor () { - _status = _NOT_ENTERED; - } - - /** - * @dev Prevents a contract from calling itself, directly or indirectly. - * Calling a `nonReentrant` function from another `nonReentrant` - * function is not supported. It is possible to prevent this from happening - * by making the `nonReentrant` function external, and make it call a - * `private` function that does the actual work. - */ - modifier nonReentrant() { - // On the first call to nonReentrant, _notEntered will be true - require(_status != _ENTERED, "ReentrancyGuard: reentrant call"); - - // Any calls to nonReentrant after this point will fail - _status = _ENTERED; - - _; - - // By storing the original value once again, a refund is triggered (see - // https://eips.ethereum.org/EIPS/eip-2200) - _status = _NOT_ENTERED; - } -} -- 2.23.0