Skip to content

Commit

Permalink
feat(protocol): improve DelegateOwner to have an optional L2 admin (#…
Browse files Browse the repository at this point in the history
…17445)

Co-authored-by: dantaik <[email protected]>
Co-authored-by: David <[email protected]>
  • Loading branch information
3 people authored Jun 18, 2024
1 parent 0a8a886 commit 1c59e8c
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 13 deletions.
5 changes: 3 additions & 2 deletions packages/protocol/contract_layout.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@
| lastUnpausedAt | uint64 | 201 | 2 | 8 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| __gap | uint256[49] | 202 | 0 | 1568 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| l1ChainId | uint64 | 251 | 0 | 8 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| nextTxId | uint64 | 251 | 8 | 8 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| realOwner | address | 252 | 0 | 20 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| admin | address | 251 | 8 | 20 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| nextTxId | uint64 | 252 | 0 | 8 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| realOwner | address | 252 | 8 | 20 | contracts/L2/DelegateOwner.sol:DelegateOwner |
| __gap | uint256[48] | 253 | 0 | 1536 | contracts/L2/DelegateOwner.sol:DelegateOwner |

## GuardianProver
Expand Down
40 changes: 34 additions & 6 deletions packages/protocol/contracts/L2/DelegateOwner.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ import "../bridge/IBridge.sol";
/// @custom:security-contact [email protected]
contract DelegateOwner is EssentialContract, IMessageInvocable {
/// @notice The owner chain ID.
uint64 public l1ChainId;
uint64 public l1ChainId; // slot 1

/// @notice The admin who can directly call `invokeCall`.
address public admin;

/// @notice The next transaction ID.
uint64 public nextTxId;
uint64 public nextTxId; // slot 2

/// @notice The real owner on L1, supposedly the DAO.
address public realOwner;
Expand All @@ -40,6 +43,11 @@ contract DelegateOwner is EssentialContract, IMessageInvocable {
uint64 indexed txId, address indexed target, bool isDelegateCall, bytes4 indexed selector
);

/// @notice Emitted when the admin has been changed.
/// @param oldAdmin The old admin address.
/// @param newAdmin The new admin address.
event AdminUpdated(address indexed oldAdmin, address indexed newAdmin);

error DO_DRYRUN_SUCCEEDED();
error DO_INVALID_PARAM();
error DO_INVALID_TARGET();
Expand All @@ -49,12 +57,14 @@ contract DelegateOwner is EssentialContract, IMessageInvocable {
/// @notice Initializes the contract.
/// @param _realOwner The real owner on L1 that can send a cross-chain message to invoke
/// `onMessageInvocation`.
/// @param _addressManager The address of the {AddressManager} contract.
/// @param _l1ChainId The L1 chain's ID.
/// @param _addressManager The address of the {AddressManager} contract.
/// @param _admin The admin address.
function init(
address _realOwner,
address _addressManager,
uint64 _l1ChainId
uint64 _l1ChainId,
address _admin
)
external
initializer
Expand All @@ -66,8 +76,9 @@ contract DelegateOwner is EssentialContract, IMessageInvocable {
revert DO_INVALID_PARAM();
}

realOwner = _realOwner;
l1ChainId = _l1ChainId;
realOwner = _realOwner;
admin = _admin;
}

/// @inheritdoc IMessageInvocable
Expand All @@ -83,11 +94,28 @@ contract DelegateOwner is EssentialContract, IMessageInvocable {
_invokeCall(_data, true);
}

/// @dev Invokes a call by the admin
/// @param _data The data for this contract to interpret.
function invokeCall(bytes calldata _data) external {
if (msg.sender != admin) revert DO_PERMISSION_DENIED();
_invokeCall(_data, true);
}

/// @dev Updates the admin address.
/// @param _admin The new admin address.
function setAdmin(address _admin) external {
if (msg.sender != owner() && msg.sender != admin) revert DO_PERMISSION_DENIED();
if (admin == _admin) revert DO_INVALID_PARAM();

emit AdminUpdated(admin, _admin);
admin = _admin;
}

/// @notice Dryruns a message invocation but always revert.
/// If this tx is reverted with DO_TRY_RUN_SUCCEEDED, the try run is successful.
/// Note that this function shall not be used in transaction and is designed for offchain
/// simulation only.
function dryrunMessageInvocation(bytes calldata _data) external payable {
function dryrunCall(bytes calldata _data) external payable {
_invokeCall(_data, false);
revert DO_DRYRUN_SUCCEEDED();
}
Expand Down
35 changes: 35 additions & 0 deletions packages/protocol/script/DeployL2DelegateOwner.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

import "../test/DeployCapability.sol";
import "../contracts/L2/DelegateOwner.sol";

// forge script --rpc-url https://rpc.mainnet.taiko.xyz script/DeployL2DelegateOwner.s.sol
contract DeployL2DelegateOwner is DeployCapability {
address public l2Sam = 0x1670000000000000000000000000000000000006;
address public l1Owner = 0x8F13E3a9dFf52e282884aA70eAe93F57DD601298; // Daniel's EOA
address public l2Admin = 0x8F13E3a9dFf52e282884aA70eAe93F57DD601298; // same

modifier broadcast() {
vm.startBroadcast();
_;
vm.stopBroadcast();
}

function run() external broadcast {
// Deploy the QuotaManager contract on Ethereum
// 0x08c82ab90f86bf8d98440b96754a67411d656130
address delegateowner = deployProxy({
name: "delegate_owner",
impl: address(new DelegateOwner()), // 0xdaf15cfa36c2188e3e0f4fb15a80e476e5e2ceb9
data: abi.encodeCall(DelegateOwner.init, (l1Owner, l2Sam, 1, l2Admin))
});

// 0xf4707c2821b3067bdf9c4d48eb133851ff3e7ea7
deployProxy({
name: "test_address_am",
impl: address(new AddressManager()), // 0x66489c2932a906ea7971eeb0a7379593ea32eb79
data: abi.encodeCall(AddressManager.init, (delegateowner))
});
}
}
68 changes: 68 additions & 0 deletions packages/protocol/script/L1DelegateOwnerBatching.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;

import "../test/DeployCapability.sol";
import "../contracts/L2/DelegateOwner.sol";
import "../contracts/bridge/Bridge.sol";
import "../test/common/TestMulticall3.sol";

// forge script \
// --rpc-url https://mainnet.infura.io/v3/... \
// --private-key ... \
// --legacy \
// --broadcast \
// script/DeployL2DelegateOwner.s.sol
contract L2DelegateOwnerBatching is DeployCapability {
address public l2Admin = 0x8F13E3a9dFf52e282884aA70eAe93F57DD601298; // same
address public l2DelegateOwner = 0x08c82ab90f86BF8d98440b96754a67411D656130;
address public constant l2Multicall3 = 0xcA11bde05977b3631167028862bE2a173976CA11;

modifier broadcast() {
vm.startBroadcast();
_;
vm.stopBroadcast();
}

function run() external broadcast {
TestMulticall3.Call3[] memory calls = new TestMulticall3.Call3[](2);
calls[0].target = 0x08c82ab90f86BF8d98440b96754a67411D656130;
calls[0].allowFailure = false;
calls[0].callData =
abi.encodeCall(DelegateOwner.setAdmin, (0x4757D97449acA795510b9f3152C6a9019A3545c3));

calls[1].target = 0xf4707c2821b3067bdF9c4D48eB133851FF3e7ea7;
calls[1].allowFailure = false;
calls[1].callData =
abi.encodeCall(UUPSUpgradeable.upgradeTo, (0x0167000000000000000000000000000000010002));

sendMessage(0, calls);
}

function sendMessage(uint64 txId, TestMulticall3.Call3[] memory calls) internal {
IBridge.Message memory message;
message.fee = 20_000_000_000_000;
message.gasLimit = 2_000_000;
message.destChainId = 167_000;

// TODO: What if l2Admin becomes 0x0?
message.srcOwner = l2Admin;
message.destOwner = l2Admin;
message.to = l2DelegateOwner;

message.data = abi.encodeCall(
DelegateOwner.onMessageInvocation,
abi.encode(
DelegateOwner.Call(
txId,
l2Multicall3,
true, // DELEGATECALL
abi.encodeCall(TestMulticall3.aggregate3, (calls))
)
)
);

Bridge(0xd60247c6848B7Ca29eDdF63AA924E53dB6Ddd8EC).sendMessage{ value: message.fee }(
message
);
}
}
65 changes: 61 additions & 4 deletions packages/protocol/test/L2/DelegateOwner.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ contract TestDelegateOwner is TaikoTest {
name: "delegate_owner",
impl: address(new DelegateOwner()),
data: abi.encodeCall(
DelegateOwner.init, (remoteOwner, address(addressManager), remoteChainId)
DelegateOwner.init,
(remoteOwner, address(addressManager), remoteChainId, address(0))
),
registerTo: address(addressManager)
})
Expand Down Expand Up @@ -94,7 +95,7 @@ contract TestDelegateOwner is TaikoTest {
);

vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector);
delegateOwner.dryrunMessageInvocation(data);
delegateOwner.dryrunCall(data);

IBridge.Message memory message;
message.from = remoteOwner;
Expand Down Expand Up @@ -127,7 +128,7 @@ contract TestDelegateOwner is TaikoTest {
);

vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector);
delegateOwner.dryrunMessageInvocation(data);
delegateOwner.dryrunCall(data);

IBridge.Message memory message;
message.from = remoteOwner;
Expand Down Expand Up @@ -191,7 +192,7 @@ contract TestDelegateOwner is TaikoTest {
);

vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector);
delegateOwner.dryrunMessageInvocation(data);
delegateOwner.dryrunCall(data);

IBridge.Message memory message;
message.from = remoteOwner;
Expand All @@ -212,4 +213,60 @@ contract TestDelegateOwner is TaikoTest {
assertEq(target2.impl(), impl2);
assertEq(delegateOwner.impl(), delegateOwnerImpl2);
}

function test_delegate_owner_update_admin() public {
assertEq(delegateOwner.admin(), address(0));
bytes memory data = abi.encode(
DelegateOwner.Call(
uint64(0),
address(delegateOwner),
false, // CALL
abi.encodeCall(DelegateOwner.setAdmin, (David))
)
);

vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector);
delegateOwner.dryrunCall(data);

IBridge.Message memory message;
message.from = remoteOwner;
message.destChainId = uint64(block.chainid);
message.srcChainId = remoteChainId;
message.destOwner = Bob;
message.data = abi.encodeCall(DelegateOwner.onMessageInvocation, (data));
message.to = address(delegateOwner);

vm.prank(Bob);
bridge.processMessage(message, "");

bytes32 hash = bridge.hashMessage(message);
assertTrue(bridge.messageStatus(hash) == IBridge.Status.DONE);

assertEq(delegateOwner.nextTxId(), 1);
assertEq(delegateOwner.admin(), David);

// David is now the security council
data = abi.encode(
DelegateOwner.Call(
uint64(1),
address(delegateOwner),
false, // CALL
abi.encodeCall(DelegateOwner.setAdmin, (Emma))
)
);

vm.prank(Bob);
vm.expectRevert(DelegateOwner.DO_PERMISSION_DENIED.selector);
delegateOwner.invokeCall(data);

vm.prank(David);
delegateOwner.invokeCall(data);
assertEq(delegateOwner.nextTxId(), 2);
assertEq(delegateOwner.admin(), Emma);

vm.prank(Emma);
delegateOwner.setAdmin(Bob);
assertEq(delegateOwner.nextTxId(), 2);
assertEq(delegateOwner.admin(), Bob);
}
}
2 changes: 1 addition & 1 deletion packages/protocol/test/bridge/Bridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ contract BridgeTest is TaikoTest {
name: "delegate_owner",
impl: address(new DelegateOwner()),
data: abi.encodeCall(
DelegateOwner.init, (mockDAO, address(addressManager), l1ChainId)
DelegateOwner.init, (mockDAO, address(addressManager), l1ChainId, address(0))
)
})
)
Expand Down

0 comments on commit 1c59e8c

Please sign in to comment.