From 73356ca13dee14ba07aadf251205d4a748f1adea Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Mon, 23 Sep 2024 16:46:06 +0200 Subject: [PATCH] feat: pausable (#1433) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Time spent on this PR: ## Pull request type Please check the type of change your PR introduces: - [ ] Bugfix - [ ] Feature - [ ] Code style update (formatting, renaming) - [ ] Refactoring (no functional changes, no api changes) - [ ] Build related changes - [ ] Documentation content changes - [ ] Other (please describe): ## What is the current behavior? Resolves #1426 ## What is the new behavior? - Add a Pausable feature that prevents from using public, state-modifying entrypoint into Kakarot. - - - - - This change is [Reviewable](https://reviewable.io/reviews/kkrt-labs/kakarot/1433) --------- Co-authored-by: Oba --- src/kakarot/eth_rpc.cairo | 2 + src/kakarot/kakarot.cairo | 68 ++++++++++++++++++---------- tests/src/kakarot/test_kakarot.cairo | 50 ++++++++++++++++++++ tests/src/kakarot/test_kakarot.py | 60 ++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 23 deletions(-) diff --git a/src/kakarot/eth_rpc.cairo b/src/kakarot/eth_rpc.cairo index 9f54a1b01..f82ee104b 100644 --- a/src/kakarot/eth_rpc.cairo +++ b/src/kakarot/eth_rpc.cairo @@ -1,6 +1,7 @@ %lang starknet from openzeppelin.access.ownable.library import Ownable_owner +from openzeppelin.security.pausable.library import Pausable from starkware.cairo.common.bool import FALSE, TRUE from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin from starkware.cairo.common.math import assert_le, assert_nn, split_felt @@ -241,6 +242,7 @@ func eth_send_raw_unsigned_tx{ return_data_len: felt, return_data: felt*, success: felt, gas_used: felt ) { alloc_locals; + Pausable.assert_not_paused(); let tx = EthTransaction.decode(tx_data_len, tx_data); // Validate chain_id for post eip155 diff --git a/src/kakarot/kakarot.cairo b/src/kakarot/kakarot.cairo index cff7495bf..001a81d1a 100644 --- a/src/kakarot/kakarot.cairo +++ b/src/kakarot/kakarot.cairo @@ -9,6 +9,7 @@ from starkware.cairo.common.cairo_builtins import HashBuiltin, BitwiseBuiltin from starkware.cairo.common.uint256 import Uint256 from starkware.starknet.common.syscalls import replace_class from openzeppelin.access.ownable.library import Ownable, Ownable_owner +from openzeppelin.security.pausable.library import Pausable // Local dependencies from backend.starknet import Starknet @@ -28,6 +29,47 @@ from kakarot.eth_rpc import ( eth_send_raw_unsigned_tx, ) +// / ADMIN /// + +// @notice Upgrade the contract +// @dev Use the replace_hash syscall to upgrade the contract +// @param new_class_hash The new class hash +@external +func upgrade{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( + new_class_hash: felt +) { + Ownable.assert_only_owner(); + replace_class(new_class_hash); + kakarot_upgraded.emit(new_class_hash); + return (); +} + +// @notice Transfer the ownership of the contract +// @param new_owner The new owner +@external +func transfer_ownership{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( + new_owner: felt +) { + Ownable.transfer_ownership(new_owner); + return (); +} + +// @notice Pause the contract +@external +func pause{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() { + Ownable.assert_only_owner(); + Pausable._pause(); + return (); +} + +// @notice Unpause the contract +@external +func unpause{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() { + Ownable.assert_only_owner(); + Pausable._unpause(); + return (); +} + // Constructor @constructor func constructor{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( @@ -50,35 +92,12 @@ func constructor{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr ); } -// @notive Upgrade the contract -// @dev Use the replace_hash syscall to upgrade the contract -// @param new_class_hash The new class hash -@external -func upgrade{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( - new_class_hash: felt -) { - Ownable.assert_only_owner(); - replace_class(new_class_hash); - kakarot_upgraded.emit(new_class_hash); - return (); -} - // @notive Returns the owner of the contract @external func get_owner{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() -> (owner: felt) { return Ownable_owner.read(); } -// @notive Transfer the ownership of the contract -// @param new_owner The new owner -@external -func transfer_ownership{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( - new_owner: felt -) { - Ownable.transfer_ownership(new_owner); - return (); -} - // @notice Set the native token used by kakarot // @dev Set the native token which will emulate the role of ETH on Ethereum // @param native_token_address The address of the native token @@ -269,6 +288,7 @@ func get_starknet_address{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_ func deploy_externally_owned_account{ syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr }(evm_address: felt) -> (starknet_contract_address: felt) { + Pausable.assert_not_paused(); return Kakarot.deploy_externally_owned_account(evm_address); } @@ -279,6 +299,7 @@ func deploy_externally_owned_account{ func register_account{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( evm_address: felt ) { + Pausable.assert_not_paused(); return Kakarot.register_account(evm_address); } @@ -349,6 +370,7 @@ func handle_l1_message{ syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* }(from_address: felt, l1_sender: felt, to_address: felt, value: felt, data_len: felt, data: felt*) { alloc_locals; + Pausable.assert_not_paused(); let l1_messaging_contract_address = Kakarot.get_l1_messaging_contract_address(); if (from_address != l1_messaging_contract_address) { return (); diff --git a/tests/src/kakarot/test_kakarot.cairo b/tests/src/kakarot/test_kakarot.cairo index 9e14797ed..fcb8b3ac1 100644 --- a/tests/src/kakarot/test_kakarot.cairo +++ b/tests/src/kakarot/test_kakarot.cairo @@ -19,6 +19,10 @@ from kakarot.kakarot import ( set_cairo1_helpers_class_hash, transfer_ownership, upgrade_account, + deploy_externally_owned_account, + handle_l1_message, + pause, + unpause, ) from kakarot.model import model from kakarot.account import Account @@ -98,6 +102,18 @@ func compute_starknet_address{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, ra return starknet_address; } +func test__deploy_externally_owned_account{ + syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr +}() { + tempvar evm_address; + + %{ ids.evm_address = program_input["evm_address"] %} + + deploy_externally_owned_account(evm_address=evm_address); + + return (); +} + func test__register_account{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() { tempvar evm_address; @@ -230,3 +246,37 @@ func test__upgrade_account{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range return (); } + +func test__handle_l1_message{ + syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin* +}() { + tempvar from_address; + tempvar l1_sender; + tempvar to_address; + tempvar value; + tempvar data_len; + let (data) = alloc(); + + %{ + ids.from_address = program_input["from_address"] + ids.l1_sender = program_input["l1_sender"] + ids.to_address = program_input["to_address"] + ids.value = program_input["value"] + ids.data_len = len(program_input["data"]) + segments.write_arg(ids.data, list(program_input["data"])) + %} + + handle_l1_message(from_address, l1_sender, to_address, value, data_len, data); + + return (); +} + +func test__pause{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() { + pause(); + return (); +} + +func test__unpause{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}() { + unpause(); + return (); +} diff --git a/tests/src/kakarot/test_kakarot.py b/tests/src/kakarot/test_kakarot.py index c97e059a9..0ba9d54c6 100644 --- a/tests/src/kakarot/test_kakarot.py +++ b/tests/src/kakarot/test_kakarot.py @@ -77,6 +77,35 @@ def _wrapper(self, *args, **kwargs): class TestKakarot: + class TestPause: + @SyscallHandler.patch("Ownable_owner", 0xDEAD) + def test_should_assert_only_owner(self, cairo_run): + with cairo_error(message="Ownable: caller is not the owner"): + cairo_run("test__pause") + + @SyscallHandler.patch("Ownable_owner", SyscallHandler.caller_address) + @SyscallHandler.patch("Pausable_paused", 0) + def test_should_pause(self, cairo_run): + cairo_run("test__pause") + SyscallHandler.mock_storage.assert_called_with( + address=get_storage_var_address("Pausable_paused"), value=1 + ) + + class TestUnpause: + + @SyscallHandler.patch("Ownable_owner", 0xDEAD) + def test_should_assert_only_owner(self, cairo_run): + with cairo_error(message="Ownable: caller is not the owner"): + cairo_run("test__unpause") + + @SyscallHandler.patch("Ownable_owner", SyscallHandler.caller_address) + @SyscallHandler.patch("Pausable_paused", 1) + def test_should_unpause(self, cairo_run): + cairo_run("test__unpause") + SyscallHandler.mock_storage.assert_called_with( + address=get_storage_var_address("Pausable_paused"), value=0 + ) + class TestNativeToken: @pytest.mark.slow @SyscallHandler.patch("Ownable_owner", 0xDEAD) @@ -237,7 +266,20 @@ def test_should_assert_only_owner(self, cairo_run): with cairo_error(message="Ownable: caller is not the owner"): cairo_run("test__set_cairo1_helpers_class_hash", class_hash=0xABC) + class TestDeployEOA: + @SyscallHandler.patch("Pausable_paused", 1) + def test_should_assert_unpaused(self, cairo_run): + with cairo_error(message="Pausable: paused"): + cairo_run( + "test__deploy_externally_owned_account", evm_address=EVM_ADDRESS + ) + class TestRegisterAccount: + @SyscallHandler.patch("Pausable_paused", 1) + def test_should_assert_unpaused(self, cairo_run): + with cairo_error(message="Pausable: paused"): + cairo_run("test__register_account", evm_address=EVM_ADDRESS) + @SyscallHandler.patch("Kakarot_evm_to_starknet_address", EVM_ADDRESS, 0) @patch( "tests.utils.syscall_handler.SyscallHandler.caller_address", @@ -322,6 +364,19 @@ def test_upgrade_account_should_replace_class(self, cairo_run): calldata=[0x1234], ) + class TestL1Handler: + @SyscallHandler.patch("Pausable_paused", 1) + def test_should_assert_unpaused(self, cairo_run): + with cairo_error(message="Pausable: paused"): + cairo_run( + "test__handle_l1_message", + from_address=0xABC, + l1_sender=0xABC, + to_address=0xABC, + value=0xABC, + data=[], + ) + class TestEthCall: @pytest.mark.slow @pytest.mark.SolmateERC20 @@ -445,6 +500,11 @@ def test_should_return_chain_id_modulo_53(self, cairo_run, chain_id): assert res == chain_id % 2**53 class TestEthSendRawTransactionEntrypoint: + @SyscallHandler.patch("Pausable_paused", 1) + def test_should_assert_unpaused(self, cairo_run): + with cairo_error(message="Pausable: paused"): + cairo_run("test__eth_send_raw_unsigned_tx", tx_data_len=0, tx_data=[]) + def test_should_raise_invalid_chain_id_tx_type_different_from_0( self, cairo_run ):