Skip to content
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.

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

feat(protocol): allow L1 timelock to be the L2 owner #15358

Closed
wants to merge 10 commits into from
Closed
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
117 changes: 117 additions & 0 deletions packages/protocol/contracts/L2/CrossChainOwned.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// SPDX-License-Identifier: MIT
// _____ _ _ _ _
// |_ _|_ _(_) |_____ | | __ _| |__ ___
// | |/ _` | | / / _ \ | |__/ _` | '_ (_-<
// |_|\__,_|_|_\_\___/ |____\__,_|_.__/__/

pragma solidity 0.8.20;

import "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";

import "../common/EssentialContract.sol";
import "../signal/ISignalService.sol";

/// @title CrossChainOwned
/// @notice This contract's owner lives on another chain who uses signal for transaction approval.
abstract contract CrossChainOwned is EssentialContract {
uint64 public ownerChainId; // slot 1
uint64 public nextTxId;
uint256[49] private __gap;

event TransactionExecuted(uint64 indexed txId, bytes32 indexed approvalHash);

error INVALID_PARAMS();
error INVALID_EXECUTOR();
error TX_NOT_APPROVED();
error TX_REVERTED();
error NOT_CALLABLE();

function executeApprovedTransaction(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this function check its own proof instead of doing things similarly to what checkProcessMessageContext where it's called directly by the bridge contract and just the original sender on the source chain needs to be checked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new contract doesn't use the Bridge, instead, it interacts with the SignalService directly on the source chain directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for that? Seems like this type of thing will be a common thing that people have to do for these cross layer transactions. It feels a bit strange that even we wouldn't use the standard bridge to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the SignalService is intuitive to me, but we can also use the Bridge. By using the bridge, maybe we can avoid the introduction of the new CrossChainOwned contract. I'll give it a try and then lets compare the two solutions.

bytes calldata txdata,
bytes calldata proof,
address executor
)
external
{
if (executor != address(0) && executor != msg.sender) {
revert INVALID_EXECUTOR();
}

bytes32 approvalHash = _isTransactionApproved(txdata, proof, executor);
if (approvalHash == 0) revert TX_NOT_APPROVED();

(bool success,) = address(this).call(txdata);
Copy link
Member

@davidtaikocha davidtaikocha Dec 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we let owner() be L1 timelock, shall we modify those onlyOwner methods in TaikoL2 (e.g. function withdraw(address token, address to) external onlyOwner), to make them only executable by the L2 executor here instead of owner()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but we don't override onlyOwner modifier, we overide _checkOwner. It's done in the code.

if (!success) revert TX_REVERTED();
emit TransactionExecuted(nextTxId++, approvalHash);
}

function isTransactionApproved(
bytes calldata txdata,
bytes calldata proof,
address executor
)
public
view
returns (bool)
{
return _isTransactionApproved(txdata, proof, executor) != 0;
}

/// @notice Initializes the contract.
/// @param _addressManager The address of the address manager.
/// @param _ownerChainId The owner's deployment chain ID.
// solhint-disable-next-line func-name-mixedcase
function __CrossChainOwned_init(
address _addressManager,
uint64 _ownerChainId
)
internal
virtual
{
__Essential_init(_addressManager);

if (_ownerChainId == 0 || _ownerChainId == block.chainid) revert INVALID_PARAMS();
ownerChainId = _ownerChainId;
nextTxId = 1;
}

function _isTransactionApproved(
bytes calldata txdata,
bytes calldata proof,
address executor
)
internal
view
returns (bytes32 approvalHash)
{
if (bytes4(txdata) == this.executeApprovedTransaction.selector) revert NOT_CALLABLE();

bytes32 hash = keccak256(
abi.encode("APPROVE_CROSS_CHAIN_TX", block.chainid, nextTxId, executor, txdata)
);

if (_isSignalReceived(hash, proof)) return hash;
else return 0;
}

function _isSignalReceived(
bytes32 signal,
bytes calldata proof
)
internal
view
virtual
returns (bool)
{
return ISignalService(resolve("signal_service", false)).proveSignalReceived({
srcChainId: ownerChainId,
app: owner(),
signal: signal,
proof: proof
});
}

function _checkOwner() internal view virtual override {
if (msg.sender != address(this)) revert NOT_CALLABLE();
}
}
32 changes: 17 additions & 15 deletions packages/protocol/contracts/L2/TaikoL2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ pragma solidity 0.8.20;

import "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";

import "../common/EssentialContract.sol";
import "../common/ICrossChainSync.sol";
import "../libs/LibAddress.sol";
import "../libs/LibMath.sol";
import "../signal/ISignalService.sol";
import "./Lib1559Math.sol";
import "./CrossChainOwned.sol";
import "./TaikoL2Signer.sol";

/// @title TaikoL2
Expand All @@ -22,7 +21,7 @@ import "./TaikoL2Signer.sol";
/// It is used to anchor the latest L1 block details to L2 for cross-layer
/// communication, manage EIP-1559 parameters for gas pricing, and store
/// verified L1 block information.
contract TaikoL2 is EssentialContract, TaikoL2Signer, ICrossChainSync {
contract TaikoL2 is CrossChainOwned, TaikoL2Signer, ICrossChainSync {
using LibAddress for address;
using LibMath for uint256;

Expand All @@ -37,13 +36,11 @@ contract TaikoL2 is EssentialContract, TaikoL2Signer, ICrossChainSync {
mapping(uint256 l1height => ICrossChainSync.Snippet) public snippets;

// A hash to check the integrity of public inputs.
address public signalService; // slot 3
bytes32 public publicInputHash; // slot 4

uint64 public gasExcess; // slot 5
bytes32 public publicInputHash; // slot 3
uint64 public gasExcess; // slot 4
uint64 public latestSyncedL1Height;

uint256[145] private __gap;
uint256[146] private __gap;

event Anchored(bytes32 parentHash, uint64 gasExcess);

Expand All @@ -55,13 +52,18 @@ contract TaikoL2 is EssentialContract, TaikoL2Signer, ICrossChainSync {
error L2_TOO_LATE();

/// @notice Initializes the TaikoL2 contract.
/// @param _signalService Address of the {ISignalService} contract.
/// @param _addressManager Address of the AddressManager contract.
/// @param _l1ChainId The ID of the base layer.
/// @param _gasExcess The initial gasExcess.
function init(address _signalService, uint64 _gasExcess) external initializer {
__Essential_init();

if (_signalService == address(0)) revert L2_INVALID_PARAM();
signalService = _signalService;
function init(
address _addressManager,
uint64 _l1ChainId,
uint64 _gasExcess
)
external
initializer
{
__CrossChainOwned_init(_addressManager, _l1ChainId);

if (block.chainid <= 1 || block.chainid >= type(uint64).max) {
revert L2_INVALID_CHAIN_ID();
Expand Down Expand Up @@ -125,7 +127,7 @@ contract TaikoL2 is EssentialContract, TaikoL2Signer, ICrossChainSync {

// Store the L1's signal root as a signal to the local signal service to
// allow for multi-hop bridging.
ISignalService(signalService).sendSignal(l1SignalRoot);
ISignalService(resolve("signal_service", false)).sendSignal(l1SignalRoot);
emit CrossChainSynced(uint64(block.number), l1Height, l1BlockHash, l1SignalRoot);

// Update state variables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,7 @@ contract TaikoL2EIP1559Configurable is TaikoL2 {
/// @notice Sets EIP1559 configuration and gas excess.
/// @param config The new EIP1559 config.
/// @param newGasExcess The new gas excess
function setConfigAndExcess(
Config memory config,
uint64 newGasExcess
)
external
virtual
onlyOwner
{
function setConfigAndExcess(Config memory config, uint64 newGasExcess) external {
if (config.gasTargetPerL1Block == 0) revert L2_INVALID_CONFIG();
if (config.basefeeAdjustmentQuotient == 0) revert L2_INVALID_CONFIG();

Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/test/L1/Guardians.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ contract TestSignalService is TaikoTest {
deployProxy({
name: "guardians",
impl: address(new DummyGuardians()),
data: bytes.concat(DummyGuardians.init.selector)
data: abi.encodeCall(DummyGuardians.init, ())
})
);
}
Expand Down
103 changes: 103 additions & 0 deletions packages/protocol/test/L2/CrossChainOwned.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;

import "../TaikoTest.sol";

contract CrossChainOwnedContract is CrossChainOwned {
uint256 public counter;

function increment() external virtual onlyOwner {
counter += 1;
}

function init(address addressManager) external initializer {
__CrossChainOwned_init(addressManager, 12_345);
}

function _isSignalReceived(
bytes32, /*signal*/
bytes calldata /*proof*/
)
internal
pure
override
returns (bool)
{
return true;
}
}

contract CrossChainOwnedContract2 is CrossChainOwnedContract {
function increment() external override onlyOwner {
counter -= 1;
}
}

contract TestCrossChainOwned is TaikoTest {
CrossChainOwnedContract public xchainowned;

function setUp() public {
address addressManager = deployProxy({
name: "address_manager",
impl: address(new AddressManager()),
data: abi.encodeCall(AddressManager.init, ())
});
xchainowned = CrossChainOwnedContract(
deployProxy({
name: "contract",
impl: address(new CrossChainOwnedContract()),
data: abi.encodeCall(CrossChainOwnedContract.init, (addressManager))
})
);
}

function test_xchainowned_ower_cannot_be_msgsender() public {
vm.startPrank(xchainowned.owner());
vm.expectRevert(CrossChainOwned.NOT_CALLABLE.selector);
xchainowned.increment();
vm.stopPrank();
}

function test_xchainowned_exec_tx() public {
bytes memory proof = "";
bytes memory data = abi.encodeCall(xchainowned.increment, ());

assertEq(xchainowned.counter(), 0);
xchainowned.executeApprovedTransaction(data, proof, address(0));
xchainowned.executeApprovedTransaction(data, proof, address(0));
assertEq(xchainowned.counter(), 2);

vm.expectRevert(CrossChainOwned.INVALID_EXECUTOR.selector);
xchainowned.executeApprovedTransaction(data, proof, Alice);

vm.prank(Alice);
xchainowned.executeApprovedTransaction(data, proof, Alice);
assertEq(xchainowned.counter(), 3);
}

function test_xchainowned_exec_executeApprovedTransaction_revert() public {
bytes memory proof = "";
bytes memory data =
abi.encodeCall(xchainowned.executeApprovedTransaction, ("", "", address(1)));
vm.expectRevert(CrossChainOwned.NOT_CALLABLE.selector);
xchainowned.executeApprovedTransaction(data, proof, address(0));
}

function test_xchainowned_exec_upgrade() public {
bytes memory proof = "";

bytes memory incrementCall = abi.encodeCall(xchainowned.increment, ());
bytes memory upgradetoCall =
abi.encodeCall(xchainowned.upgradeTo, (address(new CrossChainOwnedContract2())));

assertEq(xchainowned.counter(), 0);
xchainowned.executeApprovedTransaction(incrementCall, proof, address(0));
assertEq(xchainowned.counter(), 1);

xchainowned.executeApprovedTransaction(upgradetoCall, proof, address(0));
assertEq(xchainowned.counter(), 1);

xchainowned.executeApprovedTransaction(incrementCall, proof, address(0));
assertEq(xchainowned.counter(), 0);
}
}
43 changes: 17 additions & 26 deletions packages/protocol/test/L2/TaikoL2.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,36 @@ contract TestTaikoL2 is TaikoTest {
// same as `block_gas_limit` in foundry.toml
uint32 public constant BLOCK_GAS_LIMIT = 30_000_000;

AddressManager public addressManager;
SignalService public ss;
address public addressManager;
TaikoL2EIP1559Configurable public L2;
SkipBasefeeCheckL2 public L2skip;

function setUp() public {
addressManager = AddressManager(
deployProxy({
name: "address_manager",
impl: address(new AddressManager()),
data: bytes.concat(AddressManager.init.selector)
})
);

ss = SignalService(
deployProxy({
name: "signal_service",
impl: address(new SignalService()),
data: bytes.concat(SignalService.init.selector),
registerTo: address(addressManager),
owner: address(0)
})
);
addressManager = deployProxy({
name: "address_manager",
impl: address(new AddressManager()),
data: abi.encodeCall(AddressManager.init, ())
});

deployProxy({
name: "signal_service",
impl: address(new SignalService()),
data: abi.encodeCall(SignalService.init, ()),
registerTo: addressManager,
owner: address(0)
});

uint64 gasExcess = 0;
uint8 quotient = 8;
uint32 gasTarget = 60_000_000;
uint64 l1ChainId = 12_345;

L2 = TaikoL2EIP1559Configurable(
payable(
deployProxy({
name: "taiko_l2",
impl: address(new TaikoL2EIP1559Configurable()),
data: bytes.concat(TaikoL2.init.selector, abi.encode(address(ss), gasExcess))
data: abi.encodeCall(TaikoL2.init, (addressManager, l1ChainId, gasExcess))
})
)
);
Expand All @@ -63,7 +59,7 @@ contract TestTaikoL2 is TaikoTest {
deployProxy({
name: "taiko_l2",
impl: address(new SkipBasefeeCheckL2()),
data: bytes.concat(TaikoL2.init.selector, abi.encode(address(ss), gasExcess))
data: abi.encodeCall(TaikoL2.init, (addressManager, l1ChainId, gasExcess))
})
)
);
Expand Down Expand Up @@ -258,11 +254,6 @@ contract TestTaikoL2 is TaikoTest {
L2skip.anchor(l1Hash, l1SignalRoot, l1Height, parentGasLimit);
}

function registerAddress(bytes32 nameHash, address addr) internal {
addressManager.setAddress(uint64(block.chainid), nameHash, addr);
console2.log(block.chainid, uint256(nameHash), unicode"→", addr);
}

// Semi-random number generator
function pickRandomNumber(
uint256 randomNum,
Expand Down
Loading