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 @@ -276,6 +276,11 @@
"name": "ChainAlreadyAdded",
"type": "error"
},
{
"inputs": [],
"name": "ChainAlreadyHasDependencies",
"type": "error"
},
{
"inputs": [],
"name": "Unauthorized",
Expand Down
4 changes: 2 additions & 2 deletions packages/contracts-bedrock/snapshots/semver-lock.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
"sourceCodeHash": "0x26b7450f991fe197a94861cf618df04feb695dbbba884ffe93f96defa63e5bd7"
},
"src/L1/SuperchainConfig.sol": {
"initCodeHash": "0x14e3ce78307d6709b29199de4380620003cbfe311a56b201ab2767c17e9e2091",
"sourceCodeHash": "0xff091439483e0669cf39c27d16ad0ef6c624d28829d079fe2c5c2c836a2db748"
"initCodeHash": "0x5cd6b119f03beea59e2ba29ea92694e6e5cbcbb1a83a02557ffbe332ce687ab6",
"sourceCodeHash": "0x1e191c7746b936427b8391c68a42b542eab0821c005a4a6a517bbe855278cced"
},
"src/L1/SystemConfig.sol": {
"initCodeHash": "0x0eda38e2fb2687a324289f04ec8ad0d2afe51f45219d074740fb4a0e24ea6569",
Expand Down
6 changes: 5 additions & 1 deletion packages/contracts-bedrock/src/L1/SuperchainConfig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ contract SuperchainConfig is Initializable, ISemver {
/// @param portal The address of the OptimismPortal contract.
event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal);

/// @notice Thrown when the input chain's system config already contains dependencies on its set.
error ChainAlreadyHasDependencies();

/// @notice Thrown when the input chain is already added to the dependency set.
error ChainAlreadyAdded();

Expand Down Expand Up @@ -122,10 +125,11 @@ contract SuperchainConfig is Initializable, ISemver {
/// Adds the new chain as a dependency for all existing chains in the dependency set, and vice versa. It
/// also stores the SystemConfig address of it, and authorizes the OptimismPortal on the SharedLockbox.
/// @param _chainId The chain ID.
/// @param _systemConfig The address of the SystemConfig contract.
/// @param _systemConfig The SystemConfig contract address of the chain to add.
function addChain(uint256 _chainId, address _systemConfig) external {
// TODO: Updater role TBD, using guardian for now.
if (msg.sender != guardian()) revert Unauthorized();
if (ISystemConfigInterop(_systemConfig).dependencyCounter() != 0) revert ChainAlreadyHasDependencies();
// Add to the dependency set and check it is not already added (`add()` returns false if it already exists)
if (!_dependencySet.add(_chainId)) revert ChainAlreadyAdded();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ interface ISuperchainConfig is IDependencySet {
event Unpaused();
event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal);

error ChainAlreadyAdded();
error Unauthorized();
error ChainAlreadyHasDependencies();
error ChainAlreadyAdded();

function GUARDIAN_SLOT() external view returns (bytes32);
function PAUSED_SLOT() external view returns (bytes32);
Expand Down
1 change: 1 addition & 0 deletions packages/contracts-bedrock/test/L1/LiquidityMigrator.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ contract LiquidityMigratorTest is CommonTest {
LiquidityMigrator public migrator;

function setUp() public virtual override {
super.enableInterop();
super.setUp();
migrator = new LiquidityMigrator(address(sharedLockbox));
}
Expand Down
5 changes: 5 additions & 0 deletions packages/contracts-bedrock/test/L1/SharedLockbox.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ contract SharedLockboxTest is CommonTest {

event PortalAuthorized(address indexed portal);

function setUp() public virtual override {
super.enableInterop();
super.setUp();
}

/// @notice Tests it reverts when the caller is not an authorized portal.
function test_lockETH_unauthorizedPortal_reverts(address _caller) public {
vm.assume(!sharedLockbox.authorizedPortals(_caller));
Expand Down
36 changes: 33 additions & 3 deletions packages/contracts-bedrock/test/L1/SuperchainConfig.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ contract SuperchainConfig_Unpause_Test is CommonTest {
contract SuperchainConfig_AddChain_Test is CommonTest {
event ChainAdded(uint256 indexed chainId, address indexed systemConfig, address indexed portal);

function setUp() public virtual override {
super.enableInterop();
super.setUp();
}

function _mockAndExpect(address _target, bytes memory _calldata, bytes memory _returnData) internal {
vm.mockCall(_target, _calldata, _returnData);
vm.expectCall(_target, _calldata);
Expand All @@ -151,11 +156,31 @@ contract SuperchainConfig_AddChain_Test is CommonTest {
superchainConfig.addChain(_chainId, _systemConfig);
}

/// @notice Tests that `addChain` reverts when the input chain already contains dependencies on its set.
function test_addChain_alreadyHasDependencies_reverts(uint256 _chainId, address _systemConfig) external {
// Mock the number of dependencies to be greater than 0.
uint256 numberOfDependencies = 1;
_mockAndExpect(
_systemConfig,
abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector),
abi.encode(numberOfDependencies)
);

vm.startPrank(superchainConfig.guardian());
vm.expectRevert(ISuperchainConfig.ChainAlreadyHasDependencies.selector);
superchainConfig.addChain(_chainId, _systemConfig);
}

/// @notice Tests that `addChain` reverts when the chain is already in the dependency set.
function test_addChain_chainAlreadyExists_reverts(uint256 _chainId, address _systemConfig) external {
SuperchainConfigForTest superchainConfig = new SuperchainConfigForTest(address(sharedLockbox));
superchainConfig.forTest_addChainOnDependencySet(_chainId);

// Mock the call over `dependencyCounter` to return 0 and avoid a previous revert
_mockAndExpect(
_systemConfig, abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector), abi.encode(0)
);

vm.startPrank(superchainConfig.guardian());
vm.expectRevert(SuperchainConfig.ChainAlreadyAdded.selector);
superchainConfig.addChain(_chainId, _systemConfig);
Expand All @@ -175,9 +200,7 @@ contract SuperchainConfig_AddChain_Test is CommonTest {
vm.expectCall(address(systemConfig), abi.encodeWithSelector(ISystemConfigInterop.optimismPortal.selector));

// Mock and expect the call to authorize the portal on the SharedLockbox with the `_portal` address
_mockAndExpect(
address(sharedLockbox), abi.encodeWithSelector(ISharedLockbox.authorizePortal.selector, _portal), ""
);
vm.expectCall(address(sharedLockbox), abi.encodeWithSelector(ISharedLockbox.authorizePortal.selector, _portal));

// Expect the `addDependency` function call to not be called since the dependency set is empty
uint64 zeroCalls = 0;
Expand Down Expand Up @@ -219,6 +242,13 @@ contract SuperchainConfig_AddChain_Test is CommonTest {
superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdTwo, systemConfigTwo);
superchainConfigForTest.forTest_addChainAndSystemConfig(chainIdThree, systemConfigThree);

// Mock and expect the call to `dependencyCounter` to return 0 for the new chain
_mockAndExpect(
address(systemConfig),
abi.encodeWithSelector(ISystemConfigInterop.dependencyCounter.selector),
abi.encode(0)
);

// Mock and expect the calls when looping through the first chain of the dependency set
_mockAndExpect(
systemConfigOne, abi.encodeWithSelector(ISystemConfigInterop.addDependency.selector, _chainId), ""
Expand Down