Skip to content

nft: N-11 Unused inherited contract #3459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sam-goldman wants to merge 3 commits into from
Closed

nft: N-11 Unused inherited contract #3459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
09ac4b0
nft: N-11 Unused inherited contract
sam-goldman Aug 11, 2022
c081519
Merge branch 'develop' into ctp/unused-ownable-import
sam-goldman Sep 13, 2022
57f8c45
Merge branch 'develop' into ctp/unused-ownable-import
sam-goldman Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions packages/contracts-periphery/contracts/L1/L1ERC721Bridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,19 @@ pragma solidity 0.8.15;
import {
CrossDomainEnabled
} from "@eth-optimism/contracts/libraries/bridge/CrossDomainEnabled.sol";
import {
OwnableUpgradeable
} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
import { Address } from "@openzeppelin/contracts/utils/Address.sol";
import { L2ERC721Bridge } from "../L2/L2ERC721Bridge.sol";
import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semver.sol";
import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";

/**
* @title L1ERC721Bridge
* @notice The L1 ERC721 bridge is a contract which works together with the L2 ERC721 bridge to
* make it possible to transfer ERC721 tokens between Optimism and Ethereum. This contract
* acts as an escrow for ERC721 tokens deposted into L2.
*/
contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
contract L1ERC721Bridge is Semver, CrossDomainEnabled, Initializable {
/**
* @notice Emitted when an ERC721 bridge to the other network is initiated.
*
Expand Down Expand Up @@ -89,9 +87,6 @@ contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
function initialize(address _messenger, address _otherBridge) public initializer {
messenger = _messenger;
otherBridge = _otherBridge;
Copy link
Contributor

@maurelian maurelian Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be cheaper and more consistent with the direction of our StandardBridge to make this an immutable variable.

Copy link
Contributor Author

@sam-goldman sam-goldman Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring to messenger, otherBridge, or both?

Copy link
Contributor

@maurelian maurelian Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both

Copy link
Contributor Author

@sam-goldman sam-goldman Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this in this PR, where it seemed more in scope.


// Initialize upgradable OZ contracts
__Ownable_init();
}

/**
Expand Down
9 changes: 2 additions & 7 deletions packages/contracts-periphery/contracts/L2/L2ERC721Bridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ pragma solidity 0.8.15;
import {
CrossDomainEnabled
} from "@eth-optimism/contracts/libraries/bridge/CrossDomainEnabled.sol";
import {
OwnableUpgradeable
} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import { Address } from "@openzeppelin/contracts/utils/Address.sol";
import { L1ERC721Bridge } from "../L1/L1ERC721Bridge.sol";
import { IOptimismMintableERC721 } from "../universal/op-erc721/IOptimismMintableERC721.sol";
import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semver.sol";
import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";

/**
* @title L2ERC721Bridge
Expand All @@ -20,7 +18,7 @@ import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semv
* acts as a minter for new tokens when it hears about deposits into the L1 ERC721 bridge.
* This contract also acts as a burner for tokens being withdrawn.
*/
contract L2ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
contract L2ERC721Bridge is Semver, CrossDomainEnabled, Initializable {
/**
* @notice Emitted when an ERC721 bridge to the other network is initiated.
*
Expand Down Expand Up @@ -103,9 +101,6 @@ contract L2ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
function initialize(address _messenger, address _otherBridge) public initializer {
messenger = _messenger;
otherBridge = _otherBridge;

// Initialize upgradable OZ contracts
__Ownable_init();
}

/**
Expand Down
9 changes: 2 additions & 7 deletions packages/contracts-periphery/contracts/universal/op-erc721/OptimismMintableERC721Factory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

import {
OwnableUpgradeable
} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
import { OptimismMintableERC721 } from "./OptimismMintableERC721.sol";
import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semver.sol";

/**
* @title OptimismMintableERC721Factory
* @notice Factory contract for creating OptimismMintableERC721 contracts.
*/
contract OptimismMintableERC721Factory is Semver, OwnableUpgradeable {
contract OptimismMintableERC721Factory is Semver, Initializable {
/**
* @notice Emitted whenever a new OptimismMintableERC721 contract is created.
*
Expand Down Expand Up @@ -52,9 +50,6 @@ contract OptimismMintableERC721Factory is Semver, OwnableUpgradeable {
function initialize(address _bridge, uint256 _remoteChainId) public initializer {
bridge = _bridge;
remoteChainId = _remoteChainId;
Copy link
Contributor

@maurelian maurelian Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, what's this used for?

Copy link
Contributor Author

@sam-goldman sam-goldman Sep 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We pass it in as an argument when we create an OptimismMintableERC721 contract here. It's used in the OptimismMintableERC721 here to create the EIP-681 URI, which is used to query the L1 token URI.


// Initialize upgradable OZ contracts
__Ownable_init();
}

/**
Expand Down