Skip to content

nft: L-13 and M-08 #3457

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 5 commits into from
Closed

nft: L-13 and M-08 #3457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8603864
nft: L-13 Duplicated code
sam-goldman Aug 15, 2022
844ec04
nft: M-08 Upgradeability inconsistencies
sam-goldman Aug 26, 2022
cd12a58
fix(ctp): change `CrossDomainEnabled` to `CrossDomainMessenger`
sam-goldman Sep 2, 2022
eb94d11
Merge branch 'develop' into ctp/refactor-erc721-bridge
sam-goldman Sep 13, 2022
9da66aa
Merge branch 'develop' into ctp/refactor-erc721-bridge
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
150 changes: 12 additions & 138 deletions packages/contracts-periphery/contracts/L1/L1ERC721Bridge.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;

import {
CrossDomainEnabled
} from "@eth-optimism/contracts/libraries/bridge/CrossDomainEnabled.sol";
import { ERC721Bridge } from "../universal/op-erc721/ERC721Bridge.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";

Expand All @@ -18,51 +15,7 @@ import { Semver } from "@eth-optimism/contracts-bedrock/contracts/universal/Semv
* 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 {
/**
* @notice Emitted when an ERC721 bridge to the other network is initiated.
*
* @param localToken Address of the token on this domain.
* @param remoteToken Address of the token on the remote domain.
* @param from Address that initiated bridging action.
* @param to Address to receive the token.
* @param tokenId ID of the specific token deposited.
* @param extraData Extra data for use on the client-side.
*/
event ERC721BridgeInitiated(
address indexed localToken,
address indexed remoteToken,
address indexed from,
address to,
uint256 tokenId,
bytes extraData
);

/**
* @notice Emitted when an ERC721 bridge from the other network is finalized.
*
* @param localToken Address of the token on this domain.
* @param remoteToken Address of the token on the remote domain.
* @param from Address that initiated bridging action.
* @param to Address to receive the token.
* @param tokenId ID of the specific token deposited.
* @param extraData Extra data for use on the client-side.
*/
event ERC721BridgeFinalized(
address indexed localToken,
address indexed remoteToken,
address indexed from,
address to,
uint256 tokenId,
bytes extraData
);

/**
* @notice Address of the bridge on the other network.
*/
address public otherBridge;

// Maps L1 token to L2 token to token ID to a boolean indicating if the token is deposited
contract L1ERC721Bridge is ERC721Bridge, Semver, OwnableUpgradeable {
/**
* @notice Mapping of L1 token to L2 token to ID to boolean, indicating if the given L1 token
* by ID was deposited for a given L2 token.
Expand All @@ -75,87 +28,18 @@ contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
* @param _messenger Address of the CrossDomainMessenger on this network.
* @param _otherBridge Address of the ERC721 bridge on the other network.
*/
constructor(address _messenger, address _otherBridge)
Semver(0, 0, 1)
CrossDomainEnabled(address(0))
{
constructor(address _messenger, address _otherBridge) Semver(0, 0, 1) {
initialize(_messenger, _otherBridge);
}

/**
* @param _messenger Address of the CrossDomainMessenger on this network.
* @param _otherBridge Address of the ERC721 bridge on the other network.
*/
function initialize(address _messenger, address _otherBridge) public initializer {
messenger = _messenger;
otherBridge = _otherBridge;

// Initialize upgradable OZ contracts
__Ownable_init();
}

/**
* @notice Initiates a bridge of an NFT to the caller's account on L2.
* @notice Initializer.
*
* @param _localToken Address of the ERC721 on this domain.
* @param _remoteToken Address of the ERC721 on the remote domain.
* @param _tokenId Token ID to bridge.
* @param _minGasLimit Minimum gas limit for the bridge message on the other domain.
* @param _extraData Optional data to forward to L2. Data supplied here will not be used to
* execute any code on L2 and is only emitted as extra data for the
* convenience of off-chain tooling.
* @param _messenger Address of the L1CrossDomainMessenger.
* @param _otherBridge Address of the L2ERC721Bridge.
*/
function bridgeERC721(
address _localToken,
address _remoteToken,
uint256 _tokenId,
uint32 _minGasLimit,
bytes calldata _extraData
) external {
// Modifier requiring sender to be EOA. This check could be bypassed by a malicious
// contract via initcode, but it takes care of the user error we want to avoid.
require(!Address.isContract(msg.sender), "L1ERC721Bridge: account is not externally owned");

_initiateBridgeERC721(
_localToken,
_remoteToken,
msg.sender,
msg.sender,
_tokenId,
_minGasLimit,
_extraData
);
}

/**
* @notice Initiates a bridge of an NFT to some recipient's account on L2.
*
* @param _localToken Address of the ERC721 on this domain.
* @param _remoteToken Address of the ERC721 on the remote domain.
* @param _to Address to receive the token on the other domain.
* @param _tokenId Token ID to bridge.
* @param _minGasLimit Minimum gas limit for the bridge message on the other domain.
* @param _extraData Optional data to forward to L2. Data supplied here will not be used to
* execute any code on L2 and is only emitted as extra data for the
* convenience of off-chain tooling.
*/
function bridgeERC721To(
address _localToken,
address _remoteToken,
address _to,
uint256 _tokenId,
uint32 _minGasLimit,
bytes calldata _extraData
) external {
_initiateBridgeERC721(
_localToken,
_remoteToken,
msg.sender,
_to,
_tokenId,
_minGasLimit,
_extraData
);
function initialize(address _messenger, address _otherBridge) public initializer {
__ERC721Bridge_init(_messenger, _otherBridge);
}

/*************************
Expand All @@ -182,7 +66,7 @@ contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
address _to,
uint256 _tokenId,
bytes calldata _extraData
) external onlyFromCrossDomainAccount(otherBridge) {
) external onlyOtherBridge {
// Checks that the L1/L2 token pair has a token ID that is escrowed in the L1 Bridge
require(
deposits[_localToken][_remoteToken][_tokenId] == true,
Expand All @@ -200,17 +84,7 @@ contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
}

/**
* @notice Internal function for initiating a token bridge to the other domain.
*
* @param _localToken Address of the ERC721 on this domain.
* @param _remoteToken Address of the ERC721 on the remote domain.
* @param _from Address of the sender on this domain.
* @param _to Address to receive the token on the other domain.
* @param _tokenId Token ID to bridge.
* @param _minGasLimit Minimum gas limit for the bridge message on the other domain.
* @param _extraData Optional data to forward to L2. Data supplied here will not be used to
* execute any code on L2 and is only emitted as extra data for the
* convenience of off-chain tooling.
* @inheritdoc ERC721Bridge
*/
function _initiateBridgeERC721(
address _localToken,
Expand All @@ -220,7 +94,7 @@ contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
uint256 _tokenId,
uint32 _minGasLimit,
bytes calldata _extraData
) internal {
) internal override {
// Construct calldata for _l2Token.finalizeBridgeERC721(_to, _tokenId)
bytes memory message = abi.encodeWithSelector(
L2ERC721Bridge.finalizeBridgeERC721.selector,
Expand All @@ -237,7 +111,7 @@ contract L1ERC721Bridge is Semver, CrossDomainEnabled, OwnableUpgradeable {
IERC721(_localToken).transferFrom(_from, address(this), _tokenId);

// Send calldata into L2
sendCrossDomainMessage(otherBridge, _minGasLimit, message);
messenger.sendMessage(otherBridge, message, _minGasLimit);
emit ERC721BridgeInitiated(_localToken, _remoteToken, _from, _to, _tokenId, _extraData);
}
}
Loading