Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ interface IL2ToL2CrossDomainMessenger {
/// @notice Thrown when a call to the target contract during message relay fails.
error TargetCallFailed();

/// @notice Thrown when attempting to use a chain ID that is not in the dependency set.
error InvalidChainId();

/// @notice Emitted whenever a message is sent to a destination
/// @param destination Chain ID of the destination chain.
/// @param target Target contract or wallet address.
Expand Down
1 change: 0 additions & 1 deletion packages/contracts-bedrock/scripts/L2Genesis.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ contract L2Genesis is Deployer {
setOptimismSuperchainERC20Factory(); // 26
setOptimismSuperchainERC20Beacon(); // 27
setSuperchainTokenBridge(); // 28
setDependencyManager(); // 29
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,6 @@
"name": "IdOriginNotL2ToL2CrossDomainMessenger",
"type": "error"
},
{
"inputs": [],
"name": "InvalidChainId",
"type": "error"
},
{
"inputs": [],
"name": "MessageAlreadyRelayed",
Expand Down
8 changes: 4 additions & 4 deletions packages/contracts-bedrock/snapshots/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
"sourceCodeHash": "0xa98dfaa25f6594f702ca868a3bf54c6b00694d6a3d5601c04ac5db788d2b6e3c"
},
"src/L1/SuperchainConfigInterop.sol": {
"initCodeHash": "0x061ace7ed28a2e9653085bb13802d58f8f80a047f88b2fe548721ff180930a09",
"sourceCodeHash": "0x28179045dd1c6c01846f3ec351102c47d7bf54e6ed87c929d67cdfce5426a130"
"initCodeHash": "0xc478f3a55eccdecf008bd3171eb0d80dca2f5b1ee2540594f264b62621a96cb1",
"sourceCodeHash": "0x3db38a6f7743e63cbeabc1ea598c70d29bfce67daef6871b486aea90691c7cd6"
},
"src/L1/SystemConfig.sol": {
"initCodeHash": "0x98c1049952199f55ae63e34ec61a839d43bde52b0892c482ae4246d0c088e826",
Expand Down Expand Up @@ -100,8 +100,8 @@
"sourceCodeHash": "0xaef8ea36c5b78cd12e0e62811d51db627ccf0dfd2cc5479fb707a10ef0d42048"
},
"src/L2/L2ToL2CrossDomainMessenger.sol": {
"initCodeHash": "0x86f1ff98c5ffe19a400c65a112b3e4f16242dcbcb635f636d63f25dbd3732d58",
"sourceCodeHash": "0x00237042c23a463b0aaa05fa95c819d246ca8fb8deb6a63510d85bd2a988fce1"
"initCodeHash": "0x115a00cfd2432d8b230302ec9a22f611ec23420aecd4b35d3d1d42041db820a9",
"sourceCodeHash": "0x067e879cd1acf2348e0729dd02395c064d24cf4571fdeade8bc2eefd8accec87"
},
"src/L2/OptimismMintableERC721.sol": {
"initCodeHash": "0xcfa6ad9997a422aef5a19a490a0a535bc870ee34b1f5258c2949eb3680f71e8a",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { SuperchainConfig } from "src/L1/SuperchainConfig.sol";
import { Storage } from "src/libraries/Storage.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { Unauthorized } from "src/libraries/errors/CommonErrors.sol";
import { Predeploys } from "src/libraries/Predeploys.sol";

// Interfaces
import { ISystemConfig } from "interfaces/L1/ISystemConfig.sol";
Expand Down Expand Up @@ -120,16 +119,10 @@ contract SuperchainConfigInterop is SuperchainConfig {
/// @param _systemConfig The SystemConfig contract address of the chain to add.
function addDependency(uint256 _chainId, address _systemConfig) external {
if (paused()) revert SuperchainPaused();
if (msg.sender != clusterManager()) revert Unauthorized();

SuperchainConfigDependencies storage dependenciesStorage = _dependenciesStorage();

if (msg.sender != clusterManager()) {
if (!dependenciesStorage.authorizedPortals[msg.sender]) revert Unauthorized();
if (IOptimismPortalInterop(payable(msg.sender)).l2Sender() != Predeploys.DEPENDENCY_MANAGER) {
revert Unauthorized();
}
}

if (dependenciesStorage.dependencySet.length() == type(uint8).max) revert DependencySetTooLarge();

// Add to the dependency set and check it is not already added (`add()` returns false if it already exists)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { TransientReentrancyAware } from "src/libraries/TransientContext.sol";

// Interfaces
import { ISemver } from "interfaces/universal/ISemver.sol";
import { IDependencySet } from "interfaces/L2/IDependencySet.sol";
import { ICrossL2Inbox, Identifier } from "interfaces/L2/ICrossL2Inbox.sol";

/// @notice Thrown when a non-written slot in transient storage is attempted to be read from.
Expand Down Expand Up @@ -42,9 +41,6 @@ error ReentrantCall();
/// @notice Thrown when a call to the target contract during message relay fails.
error TargetCallFailed();

/// @notice Thrown when attempting to use a chain ID that is not in the dependency set.
error InvalidChainId();

/// @custom:proxied true
/// @custom:predeploy 0x4200000000000000000000000000000000000023
/// @title L2ToL2CrossDomainMessenger
Expand Down Expand Up @@ -136,7 +132,6 @@ contract L2ToL2CrossDomainMessenger is ISemver, TransientReentrancyAware {
if (_destination == block.chainid) revert MessageDestinationSameChain();
if (_target == Predeploys.CROSS_L2_INBOX) revert MessageTargetCrossL2Inbox();
if (_target == Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER) revert MessageTargetL2ToL2CrossDomainMessenger();
if (!IDependencySet(Predeploys.DEPENDENCY_MANAGER).isInDependencySet(_destination)) revert InvalidChainId();

uint256 nonce = messageNonce();
emit SentMessage(_destination, _target, nonce, msg.sender, _message);
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/src/libraries/Predeploys.sol
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ library Predeploys {
|| (_useInterop && _addr == SUPERCHAIN_WETH) || (_useInterop && _addr == ETH_LIQUIDITY)
|| (_useInterop && _addr == OPTIMISM_SUPERCHAIN_ERC20_FACTORY)
|| (_useInterop && _addr == OPTIMISM_SUPERCHAIN_ERC20_BEACON)
|| (_useInterop && _addr == SUPERCHAIN_TOKEN_BRIDGE) || (_useInterop && _addr == DEPENDENCY_MANAGER);
|| (_useInterop && _addr == SUPERCHAIN_TOKEN_BRIDGE);
}

function isPredeployNamespace(address _addr) internal pure returns (bool) {
Expand Down
68 changes: 8 additions & 60 deletions packages/contracts-bedrock/test/L1/SuperchainConfigInterop.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -173,71 +173,15 @@ contract SuperchainConfigInterop_AddDependency_Test is SuperchainConfigInterop_B
assertTrue(_superchainConfigInterop().authorizedPortals(portal));
}

/// @notice Tests that `addDependency` successfully adds a chain to the dependency set through a portal call.
function test_addDependencyFromPortal_succeeds(uint256 _chainId, uint256 _chainId2) external {
vm.assume(!_superchainConfigInterop().isInDependencySet(_chainId));
vm.assume(!_superchainConfigInterop().isInDependencySet(_chainId2));
vm.assume(_chainId != _chainId2);
uint256 currentSize = _superchainConfigInterop().dependencySetSize();

// Add first an authorized portal
address authorizedPortal = _setUpPortal(_chainId);
vm.prank(_superchainConfigInterop().clusterManager());
_superchainConfigInterop().addDependency(_chainId, address(systemConfig));

address portal2 = _setUpPortal(_chainId2);

// Expect the DependencyAdded event to be emitted
vm.expectEmit(address(superchainConfig));
emit DependencyAdded(_chainId2, address(systemConfig), portal2);

// Mock the `authorizedPortal` to return the dependency manager predeploy as l2Sender
_mockAndExpect(
authorizedPortal,
abi.encodeCall(IOptimismPortalInterop.l2Sender, ()),
abi.encode(Predeploys.DEPENDENCY_MANAGER)
);

// Add the new chain to the dependency set from the `authorizedPortal`
vm.prank(authorizedPortal);
_superchainConfigInterop().addDependency(_chainId2, address(systemConfig));

// Check that the new chain is in the dependency set
assertTrue(_superchainConfigInterop().isInDependencySet(_chainId2));
assertEq(_superchainConfigInterop().dependencySetSize(), currentSize + 2);
assertTrue(_superchainConfigInterop().authorizedPortals(portal2));
}

/// @notice Tests that `addDependency` reverts when the caller is not the cluster manager or an authorized portal.
function test_addDependency_notClusterManagerOrPortal_reverts(address _caller, uint256 _chainId) external {
function test_addDependency_notClusterManager_reverts(address _caller, uint256 _chainId) external {
vm.assume(_caller != _superchainConfigInterop().clusterManager());
vm.assume(_caller != address(optimismPortal2));

vm.expectRevert(Unauthorized.selector);
vm.prank(_caller);
_superchainConfigInterop().addDependency(_chainId, address(systemConfig));
}

/// @notice Tests that `addDependency` reverts when the caller is an authorized portal but not the correct L2
/// sender.
function test_addDependency_notCorrectL2Sender_reverts(uint256 _chainId, address _l2sender) external {
vm.assume(_chainId != block.chainid);
vm.assume(_l2sender != Predeploys.DEPENDENCY_MANAGER);

address portal = _setUpPortal(_chainId);

// Mock the `authorizedPortal` to return the dependency manager predeploy as l2Sender
_mockAndExpect(portal, abi.encodeCall(IOptimismPortalInterop.l2Sender, ()), abi.encode(_l2sender));

// Add first an authorized portal
vm.prank(_superchainConfigInterop().clusterManager());
_superchainConfigInterop().addDependency(_chainId, address(systemConfig));

vm.expectRevert(Unauthorized.selector);
vm.prank(portal);
_superchainConfigInterop().addDependency(_chainId, address(systemConfig));
}

/// @notice Tests that `addDependency` reverts when the portal has an invalid superchain config address.
function test_addDependency_portalInvalidSuperchainConfig_reverts(
uint256 _chainId,
Expand Down Expand Up @@ -269,9 +213,13 @@ contract SuperchainConfigInterop_AddDependency_Test is SuperchainConfigInterop_B
}

/// @notice Tests that `addDependency` reverts when the portal is already authorized.
function test_addDependency_portalAlreadyAuthorized_reverts(uint256 _chainId) external {
function test_addDependency_portalAlreadyAuthorized_reverts(uint256 _chainId, uint256 _otherChainId) external {
// Bound chainId to be within uint128 range but not equal to block.chainid
_chainId = bound(_chainId, 1, type(uint128).max);
_otherChainId = bound(_otherChainId, 1, type(uint128).max);
vm.assume(_chainId != block.chainid);
vm.assume(_chainId <= type(uint128).max);
vm.assume(_otherChainId != block.chainid);
vm.assume(_chainId != _otherChainId);

_setUpPortal(_chainId);

Expand All @@ -281,7 +229,7 @@ contract SuperchainConfigInterop_AddDependency_Test is SuperchainConfigInterop_B

vm.prank(_superchainConfigInterop().clusterManager());
vm.expectRevert(SuperchainConfigInterop.PortalAlreadyAuthorized.selector);
_superchainConfigInterop().addDependency(_chainId + 1, address(systemConfig));
_superchainConfigInterop().addDependency(_otherChainId, address(systemConfig));
}

/// @notice Tests that `addDependency` reverts when the superchain is paused.
Expand Down
80 changes: 54 additions & 26 deletions packages/contracts-bedrock/test/L2/DependencyManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ contract DependencyManager_Base_Test is CommonTest {
event DependencyAdded(uint256 indexed chainId, address indexed systemConfig, address indexed superchainConfig);

function setUp() public virtual override {
// Skip the test until DependencyManager is integrated again
vm.skip(true);

super.enableInterop();
super.setUp();
}
Expand Down Expand Up @@ -103,7 +106,7 @@ contract DependencyManager_AddDependency_Test is DependencyManager_Base_Test {
contract DependencyManager_IsInDependencySet_Test is DependencyManager_Base_Test {
/// @dev Tests that current chain is always in dependency set
function testFuzz_isInDependencySet_currentChain_succeeds(uint256 _chainId) public {
vm.assume(_chainId <= type(uint8).max);
_chainId = bound(_chainId, 1, type(uint128).max);

vm.chainId(_chainId);
assertTrue(_dependencyManager().isInDependencySet(_chainId));
Expand All @@ -127,54 +130,79 @@ contract DependencyManager_IsInDependencySet_Test is DependencyManager_Base_Test
}

contract DependencyManager_DependencySet_Test is DependencyManager_Base_Test {
// Create a mapping to track used chainIds
mapping(uint256 => bool) usedChainIds;

/// @dev Tests that dependencySet returns correct values
function testFuzz_dependencySet_succeeds(uint256[] memory _chainIds) public {
vm.assume(_chainIds.length <= type(uint8).max);

// Ensure chain IDs are unique and valid
for (uint256 i = 0; i < _chainIds.length; i++) {
_chainIds[i] = bound(_chainIds[i], 1, type(uint8).max);
vm.assume(_chainIds[i] != block.chainid);
for (uint256 j = 0; j < i; j++) {
vm.assume(_chainIds[i] != _chainIds[j]);
function testFuzz_dependencySet_succeeds(uint256[32] memory _chainIdsValues) public {
// Limit array size to prevent too many rejections
uint256[] memory chainIds = new uint256[](bound(_chainIdsValues.length, 1, 32));

// Loop over the values and add them to the array
for (uint256 i = 0; i < chainIds.length; i++) {
chainIds[i] = _chainIdsValues[i];
}

// Generate unique chain IDs more efficiently
for (uint256 i = 0; i < chainIds.length; i++) {
// Start with a bounded random value
uint256 chainId = bound(chainIds[i], 1, type(uint8).max);

// If this chainId is already used or is the current chainId,
// increment until we find an unused one
while (usedChainIds[chainId] || chainId == block.chainid) {
chainId = (chainId % type(uint8).max) + 1;
}
usedChainIds[chainId] = true;
chainIds[i] = chainId;
}

vm.startPrank(Constants.DEPOSITOR_ACCOUNT);

// Add dependencies
for (uint256 i = 0; i < _chainIds.length; i++) {
_dependencyManager().addDependency(address(superchainConfig), _chainIds[i], address(systemConfig));
for (uint256 i = 0; i < chainIds.length; i++) {
_dependencyManager().addDependency(address(superchainConfig), chainIds[i], address(systemConfig));
}

uint256[] memory deps = _dependencyManager().dependencySet();
assertEq(deps.length, _chainIds.length);
assertEq(deps.length, chainIds.length);

// Verify each chain ID is in the dependency set
for (uint256 i = 0; i < _chainIds.length; i++) {
assertTrue(_dependencyManager().isInDependencySet(_chainIds[i]));
for (uint256 i = 0; i < chainIds.length; i++) {
assertTrue(_dependencyManager().isInDependencySet(chainIds[i]));
}

vm.stopPrank();
}

/// @dev Tests that dependencySetSize returns correct value
function testFuzz_dependencySetSize_succeeds(uint256[] memory _chainIds) public {
vm.assume(_chainIds.length <= type(uint8).max);

// Ensure chain IDs are unique and valid
for (uint256 i = 0; i < _chainIds.length; i++) {
_chainIds[i] = bound(_chainIds[i], 1, type(uint8).max);
vm.assume(_chainIds[i] != block.chainid);
for (uint256 j = 0; j < i; j++) {
vm.assume(_chainIds[i] != _chainIds[j]);
function testFuzz_dependencySetSize_succeeds(uint256[32] memory _chainIdsValues) public {
// Limit array size to prevent too many rejections
uint256[] memory chainIds = new uint256[](bound(_chainIdsValues.length, 1, 32));

// Loop over the values and add them to the array
for (uint256 i = 0; i < chainIds.length; i++) {
chainIds[i] = _chainIdsValues[i];
}

// Generate unique chain IDs more efficiently
for (uint256 i = 0; i < chainIds.length; i++) {
// Start with a bounded random value
uint256 chainId = bound(chainIds[i], 1, type(uint8).max);

// If this chainId is already used or is the current chainId,
// increment until we find an unused one
while (usedChainIds[chainId] || chainId == block.chainid) {
chainId = (chainId % type(uint8).max) + 1;
}
usedChainIds[chainId] = true;
chainIds[i] = chainId;
}

vm.startPrank(Constants.DEPOSITOR_ACCOUNT);

for (uint256 i = 0; i < _chainIds.length; i++) {
_dependencyManager().addDependency(address(superchainConfig), _chainIds[i], address(systemConfig));
for (uint256 i = 0; i < chainIds.length; i++) {
_dependencyManager().addDependency(address(superchainConfig), chainIds[i], address(systemConfig));
assertEq(_dependencyManager().dependencySetSize(), i + 1);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/contracts-bedrock/test/L2/L2Genesis.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ contract L2GenesisTest is Test {
assertEq(getCodeCount(_path, "Proxy.sol:Proxy"), Predeploys.PREDEPLOY_COUNT - 2);

// 24 proxies have the implementation set if useInterop is true and 17 if useInterop is false
assertEq(getPredeployCountWithSlotSet(_path, Constants.PROXY_IMPLEMENTATION_ADDRESS), _useInterop ? 25 : 17);
assertEq(getPredeployCountWithSlotSet(_path, Constants.PROXY_IMPLEMENTATION_ADDRESS), _useInterop ? 24 : 17);

// All proxies except 2 have the proxy 1967 admin slot set to the proxy admin
assertEq(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import {

// Interfaces
import { ICrossL2Inbox, Identifier } from "interfaces/L2/ICrossL2Inbox.sol";
import { IDependencySet } from "interfaces/L2/IDependencySet.sol";

/// @title L2ToL2CrossDomainMessengerWithModifiableTransientStorage
/// @dev L2ToL2CrossDomainMessenger contract with methods to modify the transient storage.
Expand Down Expand Up @@ -90,13 +89,6 @@ contract L2ToL2CrossDomainMessengerTest is Test {
// Ensure that the target contract is not CrossL2Inbox or L2ToL2CrossDomainMessenger
vm.assume(_target != Predeploys.CROSS_L2_INBOX && _target != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER);

// Mock the call over the `isInDependencySet` function to return true
vm.mockCall(
Predeploys.DEPENDENCY_MANAGER,
abi.encodeCall(IDependencySet.isInDependencySet, (_destination)),
abi.encode(true)
);

// Get the current message nonce
uint256 messageNonce = l2ToL2CrossDomainMessenger.messageNonce();

Expand Down