Commit 85232179 authored by Mark Tyneway's avatar Mark Tyneway

contracts-bedrock: CrossDomainOwnable

Adds 2 new contracts to the `L2` directory. Both contracts
extend OZ `Ownable` such that the `onlyOwner` modifer logic is
overwritten.

`CrossDomainOwnable` is designed for ownership of contracts on L2
when the communication goes directly through the `OptimismPortal`.
This is not safe for when the communication goes through the
`CrossDomainMessenger` system, as `msg.sender` would be the
address of the `L2CrossDomainMessenger` and if that is the owner
of the L2 contract then anybody going through the messenger system
would have owner permissions on the contract.

`CrossDomainOwnable2` is designed for ownership of contracts on L2
when the communication goes through the messenger system. It checks
for the L1 message sender value that is set in the
`L2CrossDomainMessenger` contract.

Both of these contracts include test coverage and are relatively simple.

contracts-bedrock: review fixes

contracts-bedrock: make the messenger value constant

contracts: fix

contracts-bedrock: update snapshot

lint

fix test
parent 59051957
---
'@eth-optimism/contracts-bedrock': patch
---
Add CrossDomainOwnable contracts
...@@ -8,6 +8,13 @@ GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (g ...@@ -8,6 +8,13 @@ GasBenchMark_L1StandardBridge_Finalize:test_finalizeETHWithdrawal_benchmark() (g
GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 68671) GasBenchMark_L2OutputOracle:test_proposeL2Output_benchmark() (gas: 68671)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74964) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark() (gas: 74964)
GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 35777) GasBenchMark_OptimismPortal:test_depositTransaction_benchmark_1() (gas: 35777)
CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner() (gas: 61850)
CrossDomainOwnable_Test:test_onlyOwner() (gas: 34945)
CrossDomainOwnable_Test:test_revertOnlyOwner() (gas: 10619)
CrossDomainOwnable2_Test:test_onlyOwner() (gas: 82135)
CrossDomainOwnable2_Test:test_revertNotSetOnlyOwner() (gas: 10564)
CrossDomainOwnable2_Test:test_revertNotSetOnlyOwner2() (gas: 16859)
CrossDomainOwnable2_Test:test_revertOnlyOwner() (gas: 67943)
DeployerWhitelist_Test:test_owner() (gas: 7591) DeployerWhitelist_Test:test_owner() (gas: 7591)
DeployerWhitelist_Test:test_storageSlots() (gas: 33427) DeployerWhitelist_Test:test_storageSlots() (gas: 33427)
GasPriceOracle_Test:test_baseFee() (gas: 8392) GasPriceOracle_Test:test_baseFee() (gas: 8392)
......
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
/**
* @title CrossDomainOwnable
* @notice This contract extends the OpenZeppelin `Ownable` contract
* for L2 contracts to be owned by contracts on L1. Note that
* this contract is only safe to be used if the CrossDomainMessenger
* system is bypassed and the caller on L1 is calling the
* OptimismPortal directly.
*/
abstract contract CrossDomainOwnable is Ownable {
/**
* @notice Overrides the implementation of the `onlyOwner` modifier
* to check that the unaliased `msg.sender` is the owner
* of the contract.
*/
function _checkOwner() internal view override {
require(
owner() == AddressAliasHelper.undoL1ToL2Alias(msg.sender),
"CrossDomainOwnable: caller is not the owner"
);
}
}
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import { Predeploys } from "../libraries/Predeploys.sol";
import { L2CrossDomainMessenger } from "./L2CrossDomainMessenger.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
/**
* @title CrossDomainOwnable2
* @notice This contract extends the OpenZeppelin `Ownable` contract
* for L2 contracts to be owned by contracts on L1. Note that
* this contract is meant to be used with systems that use the
* CrossDomainMessenger system. It will not work if the OptimismPortal
* is used directly.
*/
abstract contract CrossDomainOwnable2 is Ownable {
/**
* @notice Overrides the implementation of the `onlyOwner` modifier
* to check that the unaliased `xDomainMessageSender`
* is the owner of the contract. This value is set
* to the caller of the L1CrossDomainMessenger.
*/
function _checkOwner() internal view override {
L2CrossDomainMessenger messenger = L2CrossDomainMessenger(
Predeploys.L2_CROSS_DOMAIN_MESSENGER
);
require(
msg.sender == address(messenger),
"CrossDomainOwnable2: caller is not the messenger"
);
require(
owner() == messenger.xDomainMessageSender(),
"CrossDomainOwnable2: caller is not the owner"
);
}
}
...@@ -200,6 +200,7 @@ contract Messenger_Initializer is L2OutputOracle_Initializer { ...@@ -200,6 +200,7 @@ contract Messenger_Initializer is L2OutputOracle_Initializer {
); );
event RelayedMessage(bytes32 indexed msgHash); event RelayedMessage(bytes32 indexed msgHash);
event FailedRelayedMessage(bytes32 indexed msgHash);
event TransactionDeposited( event TransactionDeposited(
address indexed from, address indexed from,
......
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import { CommonTest, Portal_Initializer } from "./CommonTest.t.sol";
import { CrossDomainOwnable } from "../L2/CrossDomainOwnable.sol";
import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
import { Vm } from "forge-std/Vm.sol";
import { Bytes32AddressLib } from "@rari-capital/solmate/src/utils/Bytes32AddressLib.sol";
contract XDomainSetter is CrossDomainOwnable {
uint256 public value;
function set(uint256 _value) external onlyOwner {
value = _value;
}
}
contract CrossDomainOwnable_Test is CommonTest {
XDomainSetter setter;
function setUp() external {
setter = new XDomainSetter();
}
// Check that the revert message is correct
function test_revertOnlyOwner() external {
vm.expectRevert("CrossDomainOwnable: caller is not the owner");
setter.set(1);
}
// Check that making a call can set the value properly
function test_onlyOwner() external {
assertEq(
setter.value(),
0
);
vm.prank(AddressAliasHelper.applyL1ToL2Alias(setter.owner()));
setter.set(1);
assertEq(
setter.value(),
1
);
}
}
contract CrossDomainOwnableThroughPortal_Test is Portal_Initializer {
XDomainSetter setter;
function setUp() public override {
super.setUp();
vm.prank(alice);
setter = new XDomainSetter();
}
function test_depositTransaction_crossDomainOwner() external {
vm.recordLogs();
vm.prank(alice);
op.depositTransaction(
address(setter),
0,
10000,
false,
abi.encodeWithSelector(
XDomainSetter.set.selector,
1
)
);
// Simulate the operation of the `op-node` by parsing data
// from logs
Vm.Log[] memory logs = vm.getRecordedLogs();
// Only 1 log emitted
assertEq(logs.length, 1);
Vm.Log memory log = logs[0];
// It is the expected topic
bytes32 topic = log.topics[0];
assertEq(
topic,
keccak256("TransactionDeposited(address,address,uint256,bytes)")
);
// from is indexed and the first argument to the event.
bytes32 _from = log.topics[1];
address from = Bytes32AddressLib.fromLast20Bytes(_from);
assertEq(
AddressAliasHelper.undoL1ToL2Alias(from),
alice
);
// Make a call from the "from" value received from the log.
// In theory the opaque data could be parsed from the log
// and passed to a low level call to "to", but calling set
// directly on the setter is good enough.
vm.prank(from);
setter.set(1);
assertEq(
setter.value(),
1
);
}
}
// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
import { Bytes32AddressLib } from "@rari-capital/solmate/src/utils/Bytes32AddressLib.sol";
import { CommonTest, Messenger_Initializer } from "./CommonTest.t.sol";
import { CrossDomainOwnable2 } from "../L2/CrossDomainOwnable2.sol";
import { AddressAliasHelper } from "../vendor/AddressAliasHelper.sol";
import { Hashing } from "../libraries/Hashing.sol";
contract XDomainSetter2 is CrossDomainOwnable2 {
uint256 public value;
function set(uint256 _value) external onlyOwner {
value = _value;
}
}
contract CrossDomainOwnable2_Test is Messenger_Initializer {
XDomainSetter2 setter;
function setUp() override public {
super.setUp();
vm.prank(alice);
setter = new XDomainSetter2();
}
function test_revertNotSetOnlyOwner() external {
vm.expectRevert("CrossDomainOwnable2: caller is not the messenger");
setter.set(1);
}
function test_revertNotSetOnlyOwner2() external {
// set the xdomain messenger storage slot
bytes32 key = bytes32(uint256(204));
bytes32 value = Bytes32AddressLib.fillLast12Bytes(address(setter));
vm.store(address(L2Messenger), key, value);
vm.prank(address(L2Messenger));
vm.expectRevert("CrossDomainOwnable2: caller is not the owner");
setter.set(1);
}
function test_revertOnlyOwner() external {
uint256 nonce = 0;
address sender = bob;
address target = address(setter);
uint256 value = 0;
uint256 minGasLimit = 0;
bytes memory message = abi.encodeWithSelector(
XDomainSetter2.set.selector,
1
);
bytes32 hash = Hashing.hashCrossDomainMessage(
nonce,
sender,
target,
value,
minGasLimit,
message
);
// It should be a failed message. The revert is caught,
// so we cannot expectRevert here.
vm.expectEmit(true, true, true, true);
emit FailedRelayedMessage(hash);
vm.prank(AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger)));
L2Messenger.relayMessage(
nonce,
sender,
target,
value,
minGasLimit,
message
);
assertEq(
setter.value(),
0
);
}
function test_onlyOwner() external {
address owner = setter.owner();
// Simulate the L2 execution where the call is coming from
// the L1CrossDomainMessenger
vm.prank(AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger)));
L2Messenger.relayMessage(
1,
owner,
address(setter),
0,
0,
abi.encodeWithSelector(
XDomainSetter2.set.selector,
2
)
);
assertEq(
setter.value(),
2
);
}
}
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