Commit 6730a05c authored by Mark Tyneway's avatar Mark Tyneway

contracts-bedrock: L2 contracts reinit

Ensures that the L2 contracts cannot be reinitialized by
making their storage value for `_initialze` match the value
that is hardcoded in the contract. Since the call to `initialize()`
never actually runs in the L2 genesis generation, we only have
contract over setting state. Trying to call `reinitialize` again
with the same value will cause it to revert.

Also remove the unnecessary argument to the `L2ERC721Bridge`. This
was noticed by trail of bits during the audit. It adds extra assurance
that the `initialize` functions take no arguments on L2, meaning that
even if they could be reinitialized, nothing horrible could happen.
Note that this is very application specific and would result in nothing
horrible happening based on how the optimism contracts are designed.
We should stil not allow for contracts to be reinitialized.

In the far future, we will move towards removing the concept of
`initialize` completely by breaking storage layout migrations
into their own upgrade step.
parent fe6ce71e
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
...@@ -26,6 +26,11 @@ import ( ...@@ -26,6 +26,11 @@ import (
"github.com/ethereum-optimism/optimism/op-service/eth" "github.com/ethereum-optimism/optimism/op-service/eth"
) )
// initialzedValue represents the `Initializable` contract value. It should be kept in
// sync with the constant in `Constants.sol`.
// https://github.com/ethereum-optimism/optimism/blob/b6104508cb11ac85252484e212c199acf512a874/packages/contracts-bedrock/src/libraries/Constants.sol#L49
const initialzedValue = 3
var ( var (
ErrInvalidDeployConfig = errors.New("invalid deploy config") ErrInvalidDeployConfig = errors.New("invalid deploy config")
ErrInvalidImmutablesConfig = errors.New("invalid immutables config") ErrInvalidImmutablesConfig = errors.New("invalid immutables config")
...@@ -708,13 +713,13 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage ...@@ -708,13 +713,13 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage
"msgNonce": 0, "msgNonce": 0,
} }
storage["L2CrossDomainMessenger"] = state.StorageValues{ storage["L2CrossDomainMessenger"] = state.StorageValues{
"_initialized": 1, "_initialized": initialzedValue,
"_initializing": false, "_initializing": false,
"xDomainMsgSender": "0x000000000000000000000000000000000000dEaD", "xDomainMsgSender": "0x000000000000000000000000000000000000dEaD",
"msgNonce": 0, "msgNonce": 0,
} }
storage["L2StandardBridge"] = state.StorageValues{ storage["L2StandardBridge"] = state.StorageValues{
"_initialized": 2, "_initialized": initialzedValue,
"_initializing": false, "_initializing": false,
"messenger": predeploys.L2CrossDomainMessengerAddr, "messenger": predeploys.L2CrossDomainMessengerAddr,
} }
...@@ -749,12 +754,12 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage ...@@ -749,12 +754,12 @@ func NewL2StorageConfig(config *DeployConfig, block *types.Block) (state.Storage
} }
storage["L2ERC721Bridge"] = state.StorageValues{ storage["L2ERC721Bridge"] = state.StorageValues{
"messenger": predeploys.L2CrossDomainMessengerAddr, "messenger": predeploys.L2CrossDomainMessengerAddr,
"_initialized": 2, "_initialized": initialzedValue,
"_initializing": false, "_initializing": false,
} }
storage["OptimismMintableERC20Factory"] = state.StorageValues{ storage["OptimismMintableERC20Factory"] = state.StorageValues{
"bridge": predeploys.L2StandardBridgeAddr, "bridge": predeploys.L2StandardBridgeAddr,
"_initialized": 2, "_initialized": initialzedValue,
"_initializing": false, "_initializing": false,
} }
return storage, nil return storage, nil
......
...@@ -229,16 +229,16 @@ L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 123957) ...@@ -229,16 +229,16 @@ L2CrossDomainMessenger_Test:test_sendMessage_succeeds() (gas: 123957)
L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 135812) L2CrossDomainMessenger_Test:test_sendMessage_twice_succeeds() (gas: 135812)
L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 48371) L2CrossDomainMessenger_Test:test_xDomainMessageSender_reset_succeeds() (gas: 48371)
L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10590) L2CrossDomainMessenger_Test:test_xDomainSender_senderNotSet_reverts() (gas: 10590)
L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 31431) L2ERC721Bridge_Test:test_bridgeERC721To_localTokenZeroAddress_reverts() (gas: 31453)
L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 26832) L2ERC721Bridge_Test:test_bridgeERC721To_remoteTokenZeroAddress_reverts() (gas: 26854)
L2ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 157134) L2ERC721Bridge_Test:test_bridgeERC721To_succeeds() (gas: 157156)
L2ERC721Bridge_Test:test_bridgeERC721To_wrongOwner_reverts() (gas: 34467) L2ERC721Bridge_Test:test_bridgeERC721To_wrongOwner_reverts() (gas: 34489)
L2ERC721Bridge_Test:test_bridgeERC721_fromContract_reverts() (gas: 27183) L2ERC721Bridge_Test:test_bridgeERC721_fromContract_reverts() (gas: 27183)
L2ERC721Bridge_Test:test_bridgeERC721_localTokenZeroAddress_reverts() (gas: 29305) L2ERC721Bridge_Test:test_bridgeERC721_localTokenZeroAddress_reverts() (gas: 29305)
L2ERC721Bridge_Test:test_bridgeERC721_remoteTokenZeroAddress_reverts() (gas: 24641) L2ERC721Bridge_Test:test_bridgeERC721_remoteTokenZeroAddress_reverts() (gas: 24641)
L2ERC721Bridge_Test:test_bridgeERC721_succeeds() (gas: 154746) L2ERC721Bridge_Test:test_bridgeERC721_succeeds() (gas: 154746)
L2ERC721Bridge_Test:test_bridgeERC721_wrongOwner_reverts() (gas: 34271) L2ERC721Bridge_Test:test_bridgeERC721_wrongOwner_reverts() (gas: 34271)
L2ERC721Bridge_Test:test_constructor_succeeds() (gas: 21027) L2ERC721Bridge_Test:test_constructor_succeeds() (gas: 21004)
L2ERC721Bridge_Test:test_finalizeBridgeERC721_alreadyExists_reverts() (gas: 38661) L2ERC721Bridge_Test:test_finalizeBridgeERC721_alreadyExists_reverts() (gas: 38661)
L2ERC721Bridge_Test:test_finalizeBridgeERC721_interfaceNotCompliant_reverts() (gas: 246925) L2ERC721Bridge_Test:test_finalizeBridgeERC721_interfaceNotCompliant_reverts() (gas: 246925)
L2ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (gas: 27208) L2ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (gas: 27208)
......
...@@ -6,6 +6,7 @@ import { Predeploys } from "src/libraries/Predeploys.sol"; ...@@ -6,6 +6,7 @@ import { Predeploys } from "src/libraries/Predeploys.sol";
import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol"; import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol";
import { ISemver } from "src/universal/ISemver.sol"; import { ISemver } from "src/universal/ISemver.sol";
import { L2ToL1MessagePasser } from "src/L2/L2ToL1MessagePasser.sol"; import { L2ToL1MessagePasser } from "src/L2/L2ToL1MessagePasser.sol";
import { Constants } from "src/libraries/Constants.sol";
/// @custom:proxied /// @custom:proxied
/// @custom:predeploy 0x4200000000000000000000000000000000000007 /// @custom:predeploy 0x4200000000000000000000000000000000000007
...@@ -14,8 +15,8 @@ import { L2ToL1MessagePasser } from "src/L2/L2ToL1MessagePasser.sol"; ...@@ -14,8 +15,8 @@ import { L2ToL1MessagePasser } from "src/L2/L2ToL1MessagePasser.sol";
/// L2 on the L2 side. Users are generally encouraged to use this contract instead of lower /// L2 on the L2 side. Users are generally encouraged to use this contract instead of lower
/// level message passing contracts. /// level message passing contracts.
contract L2CrossDomainMessenger is CrossDomainMessenger, ISemver { contract L2CrossDomainMessenger is CrossDomainMessenger, ISemver {
/// @custom:semver 1.6.1 /// @custom:semver 1.7.0
string public constant version = "1.6.1"; string public constant version = "1.7.0";
/// @notice Constructs the L2CrossDomainMessenger contract. /// @notice Constructs the L2CrossDomainMessenger contract.
/// @param _l1CrossDomainMessenger Address of the L1CrossDomainMessenger contract. /// @param _l1CrossDomainMessenger Address of the L1CrossDomainMessenger contract.
...@@ -24,7 +25,7 @@ contract L2CrossDomainMessenger is CrossDomainMessenger, ISemver { ...@@ -24,7 +25,7 @@ contract L2CrossDomainMessenger is CrossDomainMessenger, ISemver {
} }
/// @notice Initializer. /// @notice Initializer.
function initialize() public reinitializer(2) { function initialize() public reinitializer(Constants.INITIALIZER) {
__CrossDomainMessenger_init(); __CrossDomainMessenger_init();
} }
......
...@@ -7,6 +7,8 @@ import { L1ERC721Bridge } from "src/L1/L1ERC721Bridge.sol"; ...@@ -7,6 +7,8 @@ import { L1ERC721Bridge } from "src/L1/L1ERC721Bridge.sol";
import { IOptimismMintableERC721 } from "src/universal/IOptimismMintableERC721.sol"; import { IOptimismMintableERC721 } from "src/universal/IOptimismMintableERC721.sol";
import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol"; import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol";
import { ISemver } from "src/universal/ISemver.sol"; import { ISemver } from "src/universal/ISemver.sol";
import { Constants } from "src/libraries/Constants.sol";
import { Predeploys } from "src/libraries/Predeploys.sol";
/// @title L2ERC721Bridge /// @title L2ERC721Bridge
/// @notice The L2 ERC721 bridge is a contract which works together with the L1 ERC721 bridge to /// @notice The L2 ERC721 bridge is a contract which works together with the L1 ERC721 bridge to
...@@ -18,19 +20,18 @@ import { ISemver } from "src/universal/ISemver.sol"; ...@@ -18,19 +20,18 @@ import { ISemver } from "src/universal/ISemver.sol";
/// wait for the one-week challenge period to elapse before their Optimism-native NFT /// wait for the one-week challenge period to elapse before their Optimism-native NFT
/// can be refunded on L2. /// can be refunded on L2.
contract L2ERC721Bridge is ERC721Bridge, ISemver { contract L2ERC721Bridge is ERC721Bridge, ISemver {
/// @custom:semver 1.3.1 /// @custom:semver 1.4.0
string public constant version = "1.3.1"; string public constant version = "1.4.0";
/// @notice Constructs the L2ERC721Bridge contract. /// @notice Constructs the L2ERC721Bridge contract.
/// @param _otherBridge Address of the ERC721 bridge on the other network. /// @param _otherBridge Address of the ERC721 bridge on the other network.
constructor(address _otherBridge) ERC721Bridge(_otherBridge) { constructor(address _otherBridge) ERC721Bridge(_otherBridge) {
initialize({ _messenger: CrossDomainMessenger(address(0)) }); initialize();
} }
/// @notice Initializes the contract. /// @notice Initializes the contract.
/// @param _messenger Address of the CrossDomainMessenger on this network. function initialize() public reinitializer(Constants.INITIALIZER) {
function initialize(CrossDomainMessenger _messenger) public reinitializer(2) { __ERC721Bridge_init({ _messenger: CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER) });
__ERC721Bridge_init({ _messenger: _messenger });
} }
/// @notice Completes an ERC721 bridge from the other domain and sends the ERC721 token to the /// @notice Completes an ERC721 bridge from the other domain and sends the ERC721 token to the
......
...@@ -6,6 +6,7 @@ import { StandardBridge } from "src/universal/StandardBridge.sol"; ...@@ -6,6 +6,7 @@ import { StandardBridge } from "src/universal/StandardBridge.sol";
import { ISemver } from "src/universal/ISemver.sol"; import { ISemver } from "src/universal/ISemver.sol";
import { OptimismMintableERC20 } from "src/universal/OptimismMintableERC20.sol"; import { OptimismMintableERC20 } from "src/universal/OptimismMintableERC20.sol";
import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol"; import { CrossDomainMessenger } from "src/universal/CrossDomainMessenger.sol";
import { Constants } from "src/libraries/Constants.sol";
/// @custom:proxied /// @custom:proxied
/// @custom:predeploy 0x4200000000000000000000000000000000000010 /// @custom:predeploy 0x4200000000000000000000000000000000000010
...@@ -51,8 +52,8 @@ contract L2StandardBridge is StandardBridge, ISemver { ...@@ -51,8 +52,8 @@ contract L2StandardBridge is StandardBridge, ISemver {
bytes extraData bytes extraData
); );
/// @custom:semver 1.3.1 /// @custom:semver 1.4.0
string public constant version = "1.3.1"; string public constant version = "1.4.0";
/// @notice Constructs the L2StandardBridge contract. /// @notice Constructs the L2StandardBridge contract.
/// @param _otherBridge Address of the L1StandardBridge. /// @param _otherBridge Address of the L1StandardBridge.
...@@ -61,7 +62,7 @@ contract L2StandardBridge is StandardBridge, ISemver { ...@@ -61,7 +62,7 @@ contract L2StandardBridge is StandardBridge, ISemver {
} }
/// @notice Initializer /// @notice Initializer
function initialize() public reinitializer(2) { function initialize() public reinitializer(Constants.INITIALIZER) {
__StandardBridge_init({ _messenger: CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER) }); __StandardBridge_init({ _messenger: CrossDomainMessenger(Predeploys.L2_CROSS_DOMAIN_MESSENGER) });
} }
......
...@@ -476,7 +476,7 @@ contract ERC721Bridge_Initializer is Bridge_Initializer { ...@@ -476,7 +476,7 @@ contract ERC721Bridge_Initializer is Bridge_Initializer {
vm.prank(multisig); vm.prank(multisig);
Proxy(payable(Predeploys.L2_ERC721_BRIDGE)).upgradeToAndCall( Proxy(payable(Predeploys.L2_ERC721_BRIDGE)).upgradeToAndCall(
address(l2BridgeImpl), abi.encodeCall(L2ERC721Bridge.initialize, (L2Messenger)) address(l2BridgeImpl), abi.encodeCall(L2ERC721Bridge.initialize, ())
); );
// Set up a reference to the L2ERC721Bridge. // Set up a reference to the L2ERC721Bridge.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment