diff --git a/packages/protocol/contracts/L2/DelegateOwner.sol b/packages/protocol/contracts/L2/DelegateOwner.sol index b6d7fb6906e..06d8d2e9799 100644 --- a/packages/protocol/contracts/L2/DelegateOwner.sol +++ b/packages/protocol/contracts/L2/DelegateOwner.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import "../common/EssentialContract.sol"; import "../common/LibStrings.sol"; +import "../libs/LibBytes.sol"; import "../bridge/IBridge.sol"; /// @title DelegateOwner @@ -22,20 +23,27 @@ contract DelegateOwner is EssentialContract, IMessageInvocable { uint256[48] private __gap; - /// @notice Emitted when a transaction is executed. + struct Call { + uint64 txId; + address target; + bool isDelegateCall; + bytes txdata; + } + + /// @notice Emitted when a message is invoked. /// @param txId The transaction ID. /// @param target The target address. + /// @param isDelegateCall True if the call is a `delegatecall`. /// @param selector The function selector. - event TransactionExecuted(uint64 indexed txId, address indexed target, bytes4 indexed selector); - - /// @notice Emitted when this contract accepted the ownership of a target contract. - /// @param target The target address. - event OwnershipAccepted(address indexed target); + event MessageInvoked( + uint64 indexed txId, address indexed target, bool isDelegateCall, bytes4 indexed selector + ); + error DO_DRYRUN_SUCCEEDED(); error DO_INVALID_PARAM(); error DO_INVALID_TX_ID(); error DO_PERMISSION_DENIED(); - error DO_TX_REVERTED(); + error DO_TARGET_CALL_REVERTED(); /// @notice Initializes the contract. /// @param _realOwner The real owner on L1 that can send a cross-chain message to invoke @@ -69,28 +77,40 @@ contract DelegateOwner is EssentialContract, IMessageInvocable { payable onlyFromNamed(LibStrings.B_BRIDGE) { - (uint64 txId, address target, bytes memory txdata) = - abi.decode(_data, (uint64, address, bytes)); - - if (txId != nextTxId) revert DO_INVALID_TX_ID(); - IBridge.Context memory ctx = IBridge(msg.sender).context(); if (ctx.srcChainId != l1ChainId || ctx.from != realOwner) { revert DO_PERMISSION_DENIED(); } - nextTxId++; - // Sending ether along with the function call. Although this is sending Ether from this - // contract back to itself, txData's function can now be payable. - (bool success,) = target.call{ value: msg.value }(txdata); - if (!success) revert DO_TX_REVERTED(); + _invokeCall(_data, true); + } - emit TransactionExecuted(txId, target, bytes4(txdata)); + /// @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 { + _invokeCall(_data, false); + revert DO_DRYRUN_SUCCEEDED(); } function acceptOwnership(address target) external { Ownable2StepUpgradeable(target).acceptOwnership(); - emit OwnershipAccepted(target); } + function transferOwnership(address) public pure override notImplemented { } + function _authorizePause(address, bool) internal pure override notImplemented { } + + function _invokeCall(bytes calldata _data, bool _verifyTxId) internal { + Call memory call = abi.decode(_data, (Call)); + + if (_verifyTxId && call.txId != nextTxId++) revert DO_INVALID_TX_ID(); + + (bool success, bytes memory result) = call.isDelegateCall // + ? call.target.delegatecall(call.txdata) + : call.target.call{ value: msg.value }(call.txdata); + + if (!success) LibBytes.revertWithExtractedError(result); + emit MessageInvoked(call.txId, call.target, call.isDelegateCall, bytes4(call.txdata)); + } } diff --git a/packages/protocol/contracts/common/EssentialContract.sol b/packages/protocol/contracts/common/EssentialContract.sol index 0206fb585ad..b11a4ea5c5a 100644 --- a/packages/protocol/contracts/common/EssentialContract.sol +++ b/packages/protocol/contracts/common/EssentialContract.sol @@ -91,6 +91,10 @@ abstract contract EssentialContract is UUPSUpgradeable, Ownable2StepUpgradeable, _authorizePause(msg.sender, false); } + function impl() public view returns (address) { + return _getImplementation(); + } + /// @notice Returns true if the contract is paused, and false otherwise. /// @return true if paused, false otherwise. function paused() public view returns (bool) { diff --git a/packages/protocol/contracts/libs/LibBytes.sol b/packages/protocol/contracts/libs/LibBytes.sol index 283cdb6b52f..0a35d5c7a64 100644 --- a/packages/protocol/contracts/libs/LibBytes.sol +++ b/packages/protocol/contracts/libs/LibBytes.sol @@ -2,6 +2,8 @@ pragma solidity 0.8.24; library LibBytes { + error INNER_ERROR(bytes innerError); + // Taken from: // https://github.com/0xPolygonHermez/zkevm-contracts/blob/main/contracts/PolygonZkEVMBridge.sol#L835-L860 /// @notice Function to convert returned data to string @@ -29,4 +31,21 @@ library LibBytes { return ""; } } + + // Taken from: + // https://github.com/boringcrypto/BoringSolidity/blob/master/contracts/BoringBatchable.sol + /// @dev Helper function to extract a useful revert message from a failed call. + /// If the returned data is malformed or not correctly abi encoded then this call can fail + /// itself. + function revertWithExtractedError(bytes memory _returnData) internal pure { + // If the _res length is less than 68, then + // the transaction failed with custom error or silently (without a revert message) + if (_returnData.length < 68) revert INNER_ERROR(_returnData); + + assembly { + // Slice the sighash. + _returnData := add(_returnData, 0x04) + } + revert(abi.decode(_returnData, (string))); // All that remains is the revert string + } } diff --git a/packages/protocol/test/L2/DelegateOwner.t.sol b/packages/protocol/test/L2/DelegateOwner.t.sol new file mode 100644 index 00000000000..96bf4bc9e37 --- /dev/null +++ b/packages/protocol/test/L2/DelegateOwner.t.sol @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import "../common/TestMulticall3.sol"; +import "../TaikoTest.sol"; + +contract Target is EssentialContract { + function init(address _owner) external initializer { + __Essential_init(_owner); + } +} + +contract TestDelegateOwner is TaikoTest { + address public owner; + address public remoteOwner; + Bridge public bridge; + SignalService public signalService; + AddressManager public addressManager; + DelegateOwner public delegateOwner; + TestMulticall3 public multicall; + + uint64 remoteChainId = uint64(block.chainid + 1); + address remoteBridge = vm.addr(0x2000); + + function setUp() public { + owner = vm.addr(0x1000); + vm.deal(owner, 100 ether); + + remoteOwner = vm.addr(0x2000); + + vm.startPrank(owner); + + multicall = new TestMulticall3(); + + addressManager = AddressManager( + deployProxy({ + name: "address_manager", + impl: address(new AddressManager()), + data: abi.encodeCall(AddressManager.init, (address(0))) + }) + ); + + delegateOwner = DelegateOwner( + deployProxy({ + name: "delegate_owner", + impl: address(new DelegateOwner()), + data: abi.encodeCall( + DelegateOwner.init, (remoteOwner, address(addressManager), remoteChainId) + ), + registerTo: address(addressManager) + }) + ); + + signalService = SkipProofCheckSignal( + deployProxy({ + name: "signal_service", + impl: address(new SkipProofCheckSignal()), + data: abi.encodeCall(SignalService.init, (address(0), address(addressManager))), + registerTo: address(addressManager) + }) + ); + + bridge = Bridge( + payable( + deployProxy({ + name: "bridge", + impl: address(new Bridge()), + data: abi.encodeCall(Bridge.init, (address(0), address(addressManager))), + registerTo: address(addressManager) + }) + ) + ); + + addressManager.setAddress(remoteChainId, "bridge", remoteBridge); + vm.stopPrank(); + } + + function test_delegate_owner_single_non_delegatecall() public { + Target target1 = Target( + deployProxy({ + name: "target1", + impl: address(new Target()), + data: abi.encodeCall(Target.init, (address(delegateOwner))) + }) + ); + + bytes memory data = abi.encode( + DelegateOwner.Call( + uint64(0), + address(target1), + false, // CALL + abi.encodeCall(EssentialContract.pause, ()) + ) + ); + + vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector); + delegateOwner.dryrunMessageInvocation(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); + assertTrue(target1.paused()); + } + + function test_delegate_owner_single_non_delegatecall_self() public { + address delegateOwnerImpl2 = address(new DelegateOwner()); + + bytes memory data = abi.encode( + DelegateOwner.Call( + uint64(0), + address(delegateOwner), + false, // CALL + abi.encodeCall(UUPSUpgradeable.upgradeTo, (delegateOwnerImpl2)) + ) + ); + + vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector); + delegateOwner.dryrunMessageInvocation(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.impl(), delegateOwnerImpl2); + } + + function test_delegate_owner_delegate_multicall() public { + address impl1 = address(new Target()); + address impl2 = address(new Target()); + + address delegateOwnerImpl2 = address(new DelegateOwner()); + + Target target1 = Target( + deployProxy({ + name: "target1", + impl: impl1, + data: abi.encodeCall(Target.init, (address(delegateOwner))) + }) + ); + Target target2 = Target( + deployProxy({ + name: "target2", + impl: impl1, + data: abi.encodeCall(Target.init, (address(delegateOwner))) + }) + ); + + TestMulticall3.Call3[] memory calls = new TestMulticall3.Call3[](3); + calls[0].target = address(target1); + calls[0].allowFailure = false; + calls[0].callData = abi.encodeCall(EssentialContract.pause, ()); + + calls[1].target = address(target2); + calls[1].allowFailure = false; + calls[1].callData = abi.encodeCall(UUPSUpgradeable.upgradeTo, (impl2)); + + calls[2].target = address(delegateOwner); + calls[2].allowFailure = false; + calls[2].callData = abi.encodeCall(UUPSUpgradeable.upgradeTo, (delegateOwnerImpl2)); + + bytes memory data = abi.encode( + DelegateOwner.Call( + uint64(0), + address(multicall), + true, // DELEGATECALL + abi.encodeCall(TestMulticall3.aggregate3, (calls)) + ) + ); + + vm.expectRevert(DelegateOwner.DO_DRYRUN_SUCCEEDED.selector); + delegateOwner.dryrunMessageInvocation(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); + assertTrue(target1.paused()); + assertEq(target2.impl(), impl2); + assertEq(delegateOwner.impl(), delegateOwnerImpl2); + } +} diff --git a/packages/protocol/test/bridge/Bridge.t.sol b/packages/protocol/test/bridge/Bridge.t.sol index 0cf597e4579..4a6cd593ee9 100644 --- a/packages/protocol/test/bridge/Bridge.t.sol +++ b/packages/protocol/test/bridge/Bridge.t.sol @@ -250,94 +250,6 @@ contract BridgeTest is TaikoTest { assertEq(Carol.balance, 500); } - function test_Bridge_pause_bridge_via_delegate_owner() public { - bytes memory pauseCall = abi.encodeCall(EssentialContract.pause, ()); - - IBridge.Message memory message = getDelegateOwnerMessage( - address(mockDAO), - abi.encodeCall( - DelegateOwner.onMessageInvocation, - abi.encode(0, address(destChainBridge), pauseCall) - ) - ); - - // Mocking proof - but obviously it needs to be created in prod - // corresponding to the message - bytes memory proof = hex"00"; - - bytes32 msgHash = destChainBridge.hashMessage(message); - - vm.chainId(destChainId); - - vm.prank(Bob, Bob); - destChainBridge.processMessage(message, proof); - - IBridge.Status status = destChainBridge.messageStatus(msgHash); - assertEq(status == IBridge.Status.DONE, true); - - assertEq(destChainBridge.paused(), true); - } - - function test_Bridge_authorize_signal_service_via_delegate_owner() public { - assertEq(mockProofSignalService.isAuthorized(Alice), false); - - bytes memory authorizeCall = abi.encodeCall(SignalService.authorize, (Alice, true)); - - IBridge.Message memory message = getDelegateOwnerMessage( - address(mockDAO), - abi.encodeCall( - DelegateOwner.onMessageInvocation, - abi.encode(0, address(mockProofSignalService), authorizeCall) - ) - ); - - // Mocking proof - but obviously it needs to be created in prod - // corresponding to the message - bytes memory proof = hex"00"; - - bytes32 msgHash = destChainBridge.hashMessage(message); - - vm.chainId(destChainId); - - vm.prank(Bob, Bob); - destChainBridge.processMessage(message, proof); - - //Status is DONE, proper call - IBridge.Status status = destChainBridge.messageStatus(msgHash); - assertEq(status == IBridge.Status.DONE, true); - - assertEq(mockProofSignalService.isAuthorized(Alice), true); - } - - function test_Bridge_upgrade_delegate_owner() public { - // Needs a compatible impl. contract - address newDelegateOwnerImp = address(new DelegateOwner()); - bytes memory upgradeCall = abi.encodeCall(UUPSUpgradeable.upgradeTo, (newDelegateOwnerImp)); - - IBridge.Message memory message = getDelegateOwnerMessage( - address(mockDAO), - abi.encodeCall( - DelegateOwner.onMessageInvocation, - abi.encode(0, address(delegateOwner), upgradeCall) - ) - ); - - // Mocking proof - but obviously it needs to be created in prod - // corresponding to the message - bytes memory proof = hex"00"; - - bytes32 msgHash = destChainBridge.hashMessage(message); - - vm.chainId(destChainId); - - vm.prank(Bob, Bob); - destChainBridge.processMessage(message, proof); - - //Status is DONE,means a proper call - IBridge.Status status = destChainBridge.messageStatus(msgHash); - assertEq(status == IBridge.Status.DONE, true); - } - function test_Bridge_send_message_ether_reverts_if_value_doesnt_match_expected() public { // uint256 amount = 1 wei; IBridge.Message memory message = newMessage({ @@ -727,28 +639,4 @@ contract BridgeTest is TaikoTest { data: "" }); } - - function getDelegateOwnerMessage( - address from, - bytes memory encodedCall - ) - internal - view - returns (IBridge.Message memory message) - { - message = IBridge.Message({ - id: 0, - from: from, - srcChainId: uint64(block.chainid), - destChainId: destChainId, - srcOwner: Alice, //Does not matter who is the src/dest owner actually - except if we - // want to send ether - destOwner: Alice, - to: address(delegateOwner), - value: 0, - fee: 0, - gasLimit: 1_000_000, - data: encodedCall - }); - } } diff --git a/packages/protocol/test/bridge/Bridge2.t.sol b/packages/protocol/test/bridge/Bridge2.t.sol index 95d01427a80..621bf1fa484 100644 --- a/packages/protocol/test/bridge/Bridge2.t.sol +++ b/packages/protocol/test/bridge/Bridge2.t.sol @@ -35,8 +35,10 @@ contract BridgeTest2 is TaikoTest { _; } - function setUp() public dealEther(owner) { + function setUp() public { owner = vm.addr(0x1000); + vm.deal(owner, 100 ether); + remoteChainId = uint64(block.chainid + 1); remoteBridge = vm.addr(0x2000); diff --git a/packages/protocol/test/common/TestMulticall3.sol b/packages/protocol/test/common/TestMulticall3.sol new file mode 100644 index 00000000000..bc0620169b5 --- /dev/null +++ b/packages/protocol/test/common/TestMulticall3.sol @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +/// @title Multicall3 +/// @notice Aggregate results from multiple function calls +/// @dev Multicall & Multicall2 backwards-compatible +/// @dev Aggregate methods are marked `payable` to save 24 gas per call +/// @author Michael Elliot +/// @author Joshua Levine +/// @author Nick Johnson +/// @author Andreas Bigger +/// @author Matt Solomon +contract TestMulticall3 { + struct Call3 { + address target; + bool allowFailure; + bytes callData; + } + + struct Result { + bool success; + bytes returnData; + } + + /// @notice Aggregate calls, ensuring each returns success if required + /// @param calls An array of Call3 structs + /// @return returnData An array of Result structs + function aggregate3(Call3[] calldata calls) + public + payable + returns (Result[] memory returnData) + { + uint256 length = calls.length; + returnData = new Result[](length); + Call3 calldata calli; + for (uint256 i = 0; i < length; ++i) { + Result memory result = returnData[i]; + calli = calls[i]; + (result.success, result.returnData) = calli.target.call(calli.callData); + assembly { + // Revert if the call fails and failure is not allowed + // `allowFailure := calldataload(add(calli, 0x20))` and `success := mload(result)` + if iszero(or(calldataload(add(calli, 0x20)), mload(result))) { + // set "Error(string)" signature: bytes32(bytes4(keccak256("Error(string)"))) + mstore(0x00, 0x08c379a000000000000000000000000000000000000000000000000000000000) + // set data offset + mstore(0x04, 0x0000000000000000000000000000000000000000000000000000000000000020) + // set length of revert string + mstore(0x24, 0x0000000000000000000000000000000000000000000000000000000000000017) + // set revert string: bytes32(abi.encodePacked("Multicall3: call failed")) + mstore(0x44, 0x4d756c746963616c6c333a2063616c6c206661696c6564000000000000000000) + revert(0x00, 0x64) + } + } + } + } +}