From 8629583e5ad0fbb19087cb369dbae74cb730f87d Mon Sep 17 00:00:00 2001 From: Oba Date: Fri, 25 Oct 2024 15:10:00 +0200 Subject: [PATCH] feat: execute_call (#2) --- .../src/kakarot_interface.cairo | 26 +-- cairo/protocol_handler/src/lib.cairo | 3 +- .../src/protocol_handler.cairo | 103 ++++++++- .../tests/test_protocol_handler.cairo | 195 +++++++++++++++++- 4 files changed, 294 insertions(+), 33 deletions(-) diff --git a/cairo/protocol_handler/src/kakarot_interface.cairo b/cairo/protocol_handler/src/kakarot_interface.cairo index c4ab74dfa..1e66bdee6 100644 --- a/cairo/protocol_handler/src/kakarot_interface.cairo +++ b/cairo/protocol_handler/src/kakarot_interface.cairo @@ -2,33 +2,9 @@ use starknet::{ContractAddress, ClassHash}; #[starknet::interface] pub trait IKakarot { - //* ------------------------------------------------------------------------ *// - //* ADMIN FUNCTIONS *// - //* ------------------------------------------------------------------------ *// fn upgrade(ref self: TContractState, new_class_hash: ClassHash); fn transfer_ownership(ref self: TContractState, new_owner: ContractAddress); fn pause(ref self: TContractState); fn unpause(ref self: TContractState); - - //* ------------------------------------------------------------------------ *// - //* STORAGE SETTING FUNCTIONS *// - //* ------------------------------------------------------------------------ *// - fn set_native_token(ref self: TContractState, native_token: ContractAddress); - fn set_base_fee(ref self: TContractState, base_fee: u64); - fn set_coinbase(ref self: TContractState, new_coinbase: ContractAddress); - fn set_prev_randao(ref self: TContractState, pre_randao: felt252); - fn set_block_gas_limit(ref self: TContractState, new_block_gas_limit: felt252); - fn set_account_contract_class_hash(ref self: TContractState, new_class_hash: felt252); - fn set_uninitialized_account_class_hash(ref self: TContractState, new_class_hash: felt252); - fn set_authorized_cairo_precompile_caller( - ref self: TContractState, evm_address: ContractAddress, authorized: bool - ); - fn set_cairo1_helpers_class_hash(ref self: TContractState, new_class_hash: felt252); - fn upgrade_account(ref self: TContractState, evm_address: ContractAddress, new_class: felt252); - fn set_authorized_pre_eip155_tx( - ref self: TContractState, sender_address: ContractAddress, msg_hash: felt252 - ); - fn set_l1_messaging_contract_address( - ref self: TContractState, l1_messaging_contract_address: ContractAddress - ); + fn set_base_fee(ref self: TContractState, new_base_fee: felt252); } diff --git a/cairo/protocol_handler/src/lib.cairo b/cairo/protocol_handler/src/lib.cairo index f721ac330..2403b5483 100644 --- a/cairo/protocol_handler/src/lib.cairo +++ b/cairo/protocol_handler/src/lib.cairo @@ -1,5 +1,6 @@ mod protocol_handler; pub use protocol_handler::{ - ProtocolHandler, IProtocolHandler, IProtocolHandlerDispatcher, IProtocolHandlerDispatcherTrait + ProtocolHandler, IProtocolHandlerDispatcher, IProtocolHandlerDispatcherTrait, + IProtocolHandlerSafeDispatcher, IProtocolHandlerSafeDispatcherTrait }; mod kakarot_interface; diff --git a/cairo/protocol_handler/src/protocol_handler.cairo b/cairo/protocol_handler/src/protocol_handler.cairo index 57002a88a..aea87296f 100644 --- a/cairo/protocol_handler/src/protocol_handler.cairo +++ b/cairo/protocol_handler/src/protocol_handler.cairo @@ -50,6 +50,26 @@ pub trait IProtocolHandler { /// # Panics /// * `Caller is missing role` in case the caller is not the security council fn unpause(ref self: TContractState); + + /// Set the base fee of the Kakarot contract. + /// Only the gas price admin can call this function. + /// # Arguments + /// * `base_fee` - The new base fee + /// + /// # Panics + /// * `Caller is missing role` in case the caller is not the gas price admin + fn set_base_fee(ref self: TContractState, new_base_fee: felt252); + + /// Execute a call to the Kakarot contract + /// Only the operator can call this function. + /// # Arguments + /// * `call` - The call to be executed + /// + /// # Panics + /// * `Caller is missing role` in case the caller is not the operator + /// * `UNAUTHORIZED_SELECTOR` in case the selector is not authorized + /// * `ONLY_KAKAROT_CAN_BE_CALLED` in case the call is not to the Kakarot contract + fn execute_call(ref self: TContractState, call: Call); } #[starknet::contract] @@ -57,12 +77,14 @@ pub mod ProtocolHandler { use starknet::event::EventEmitter; use starknet::account::Call; use starknet::{ContractAddress, ClassHash, get_block_timestamp, SyscallResultTrait}; - use starknet::storage::{Map, StoragePointerReadAccess, StoragePointerWriteAccess}; + use starknet::storage::{ + Map, StoragePointerReadAccess, StoragePointerWriteAccess, StorageMapReadAccess, + StorageMapWriteAccess + }; use openzeppelin_access::accesscontrol::AccessControlComponent; use openzeppelin_introspection::src5::SRC5Component; use crate::kakarot_interface::{IKakarotDispatcher, IKakarotDispatcherTrait}; - //* ------------------------------------------------------------------------ *// //* COMPONENTS *// //* ------------------------------------------------------------------------ *// @@ -83,8 +105,9 @@ pub mod ProtocolHandler { // Access controls roles const SECURITY_COUNCIL_ROLE: felt252 = selector!("SECURITY_COUNCIL_ROLE"); - const GUARDIAN_ROLE: felt252 = selector!("GUARDIAN_ROLE"); const OPERATOR_ROLE: felt252 = selector!("OPERATOR_ROLE"); + const GUARDIAN_ROLE: felt252 = selector!("GUARDIAN_ROLE"); + const GAS_PRICE_ADMIN_ROLE: felt252 = selector!("GAS_PRICE_ADMIN_ROLE"); // Pause delay pub const SOFT_PAUSE_DELAY: u64 = 12 * 60 * 60; // 12 hours pub const HARD_PAUSE_DELAY: u64 = 7 * 24 * 60 * 60; // 7 days @@ -97,6 +120,7 @@ pub mod ProtocolHandler { pub mod errors { pub const ONLY_KAKAROT_CAN_BE_CALLED: felt252 = 'ONLY_KAKAROT_CAN_BE_CALLED'; pub const PROTOCOL_ALREADY_PAUSED: felt252 = 'PROTOCOL_ALREADY_PAUSED'; + pub const UNAUTHORIZED_SELECTOR: felt252 = 'UNAUTHORIZED_SELECTOR'; } //* ------------------------------------------------------------------------ *// @@ -108,7 +132,9 @@ pub mod ProtocolHandler { pub kakarot: IKakarotDispatcher, pub operator: ContractAddress, pub guardians: Map, + pub gas_price_admin: ContractAddress, pub protocol_frozen_until: u64, + pub authorized_operator_selector: Map, #[substorage(v0)] accesscontrol: AccessControlComponent::Storage, #[substorage(v0)] @@ -128,6 +154,8 @@ pub mod ProtocolHandler { SoftPause: SoftPause, HardPause: HardPause, Unpause: Unpause, + BaseFeeChanged: BaseFeeChanged, + Execution: Execution, #[flat] AccessControlEvent: AccessControlComponent::Event, #[flat] @@ -162,6 +190,16 @@ pub mod ProtocolHandler { #[derive(Drop, starknet::Event)] pub struct Unpause {} + #[derive(Drop, starknet::Event)] + pub struct BaseFeeChanged { + pub new_base_fee: felt252 + } + + #[derive(Drop, starknet::Event)] + pub struct Execution { + pub call: Call + } + //* ------------------------------------------------------------------------ *// //* CONSTRUCTOR *// //* ------------------------------------------------------------------------ *// @@ -172,7 +210,8 @@ pub mod ProtocolHandler { kakarot: ContractAddress, security_council: ContractAddress, operator: ContractAddress, - mut guardians: Span, + gas_price_admin: ContractAddress, + mut guardians: Span ) { // Store the Kakarot address self.kakarot.write(IKakarotDispatcher { contract_address: kakarot }); @@ -186,6 +225,26 @@ pub mod ProtocolHandler { for guardian in guardians { self.accesscontrol._grant_role(GUARDIAN_ROLE, *guardian); }; + self.accesscontrol._grant_role(GAS_PRICE_ADMIN_ROLE, gas_price_admin); + + // Store the authorized selectors for the operator + self.authorized_operator_selector.write(selector!("set_native_token"), true); + self.authorized_operator_selector.write(selector!("set_coinbase"), true); + self.authorized_operator_selector.write(selector!("set_prev_randao"), true); + self.authorized_operator_selector.write(selector!("set_block_gas_limit"), true); + self.authorized_operator_selector.write(selector!("set_account_contract_class_hash"), true); + self + .authorized_operator_selector + .write(selector!("set_uninitialized_account_class_hash"), true); + self + .authorized_operator_selector + .write(selector!("set_authorized_cairo_precompile_caller"), true); + self.authorized_operator_selector.write(selector!("set_cairo1_helpers_class_hash"), true); + self.authorized_operator_selector.write(selector!("upgrade_account"), true); + self.authorized_operator_selector.write(selector!("set_authorized_pre_eip155_tx"), true); + self + .authorized_operator_selector + .write(selector!("set_l1_messaging_contract_address"), true); } #[abi(embed_v0)] @@ -291,5 +350,41 @@ pub mod ProtocolHandler { // Emit Unpause event self.emit(Unpause {}); } + + fn set_base_fee(ref self: ContractState, new_base_fee: felt252) { + // Check only gas price admin can call + self.accesscontrol.assert_only_role(GAS_PRICE_ADMIN_ROLE); + + // Call the Kakarot set_base_fee function + let kakarot = self.kakarot.read(); + kakarot.set_base_fee(new_base_fee); + + // Emit BaseFeeChanged + self.emit(BaseFeeChanged { new_base_fee }); + } + + //* ------------------------------------------------------------------------ *// + //* EXECUTE OPERATOR CALL *// + //* ------------------------------------------------------------------------ *// + + fn execute_call(ref self: ContractState, call: Call) { + // Check only operator can call + self.accesscontrol.assert_only_role(OPERATOR_ROLE); + + // Ensure the selector to call is part of the authorized selectors + let authorized = self.authorized_operator_selector.read(call.selector); + assert(authorized, errors::UNAUTHORIZED_SELECTOR); + + // Ensure the call is to the Kakarot + let kakarot = self.kakarot.read(); + assert(call.to == kakarot.contract_address, errors::ONLY_KAKAROT_CAN_BE_CALLED); + + // Call Kakarot with syscall + starknet::syscalls::call_contract_syscall(call.to, call.selector, call.calldata) + .unwrap_syscall(); + + // Emit Event Execution event + self.emit(Execution { call }); + } } } diff --git a/cairo/protocol_handler/tests/test_protocol_handler.cairo b/cairo/protocol_handler/tests/test_protocol_handler.cairo index 100121293..412e5f3f7 100644 --- a/cairo/protocol_handler/tests/test_protocol_handler.cairo +++ b/cairo/protocol_handler/tests/test_protocol_handler.cairo @@ -6,9 +6,9 @@ use starknet::{ContractAddress, contract_address_const, get_block_timestamp}; use starknet::account::Call; use starknet::class_hash::ClassHash; use protocol_handler::{ - IProtocolHandlerDispatcher, IProtocolHandlerDispatcherTrait, ProtocolHandler + IProtocolHandlerDispatcher, IProtocolHandlerDispatcherTrait, ProtocolHandler, + IProtocolHandlerSafeDispatcher, IProtocolHandlerSafeDispatcherTrait }; - use snforge_utils::snforge_utils::{ EventsFilterBuilderTrait, ContractEventsTrait, assert_called_with }; @@ -32,18 +32,27 @@ fn guardians_mock() -> Span { .span() } +fn gas_price_admin_mock() -> ContractAddress { + contract_address_const::<'gas_price_admin_mock'>() +} + fn setup_contracts_for_testing() -> (IProtocolHandlerDispatcher, ContractClass) { // Mock Kakarot, security council, operator and guardians let kakarot_mock: ContractAddress = kakarot_mock(); let security_council_mock: ContractAddress = security_council_mock(); let operator_mock: ContractAddress = operator_mock(); + let gas_price_admin_mock: ContractAddress = gas_price_admin_mock(); let guardians: Span = guardians_mock(); // Construct the calldata for the ProtocolHandler contrustor let mut constructor_calldata: Array:: = array![ - kakarot_mock.into(), security_council_mock.into(), operator_mock.into() + kakarot_mock.into(), + security_council_mock.into(), + operator_mock.into(), + gas_price_admin_mock.into() ]; Serde::serialize(@guardians, ref constructor_calldata); + let contract = declare("ProtocolHandler").unwrap().contract_class(); let (contract_address, _) = contract.deploy(@constructor_calldata).unwrap(); @@ -426,3 +435,183 @@ fn test_protocol_handler_unpause_should_pass_after_delay() { .build(); contract_events.assert_emitted(@expected); } + +#[test] +#[should_panic(expected: 'Caller is missing role')] +fn test_protocol_handler_execute_call_should_fail_wrong_caller() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + // Change caller to random caller address + let random_caller = contract_address_const::<'random_caller'>(); + start_cheat_caller_address(protocol_handler.contract_address, random_caller); + + // Call the protocol handler execute_call, should fail as caller is not operator + let call = Call { to: kakarot_mock(), selector: 0, calldata: [].span() }; + protocol_handler.execute_call(call); +} + +#[test] +#[should_panic(expected: 'UNAUTHORIZED_SELECTOR')] +fn test_protocol_handler_execute_call_should_fail_unauthorized_selector() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + // Change caller to operator + start_cheat_caller_address(protocol_handler.contract_address, operator_mock()); + + // Construct the Call to a random address + let random_called_address = contract_address_const::<'random_called_address'>(); + let call = Call { to: random_called_address, selector: 0, calldata: [].span() }; + + // Call the protocol handler execute_call, should fail as the selector is not authorized + protocol_handler.execute_call(call); +} + +#[test] +#[should_panic(expected: 'ONLY_KAKAROT_CAN_BE_CALLED')] +fn test_protocol_handler_execute_call_should_fail_wrong_destination() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + // Change caller to operator + start_cheat_caller_address(protocol_handler.contract_address, operator_mock()); + + // Construct the Call to kakarot + let random_called_address = contract_address_const::<'random_called_address'>(); + let call = Call { + to: random_called_address, selector: selector!("set_native_token"), calldata: [].span() + }; + + // Call the protocol handler execute_call, should fail as the call is not to Kakarot + protocol_handler.execute_call(call); +} + +#[test] +#[should_panic(expected: 'Caller is missing role')] +fn test_protocol_handler_set_base_fee_wrong_caller() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + // Change caller to random caller address + let random_caller = contract_address_const::<'random_caller'>(); + start_cheat_caller_address(protocol_handler.contract_address, random_caller); + + // Call the protocol handler set_base_fee, should fail as caller is not operator + protocol_handler.set_base_fee(0); +} + +#[test] +fn test_protocol_handler_set_base_fee_should_pass() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + // Change caller to gas price admin + start_cheat_caller_address(protocol_handler.contract_address, gas_price_admin_mock()); + + // Mock the call to Kakarot set_base_fee function + mock_call::<()>(kakarot_mock(), selector!("set_base_fee"), (), 1); + + // Spy on the events + let mut spy = spy_events(); + + // Call the protocol handler set_base_fee, should pass as caller is gas price admin + protocol_handler.set_base_fee(0); + + // Assert that unpause was called on Kakarot + assert_called_with::(kakarot_mock(), selector!("set_base_fee"), 0); + + // Check the BaseFeeChanged event is emitted + let expected = ProtocolHandler::Event::BaseFeeChanged( + ProtocolHandler::BaseFeeChanged { new_base_fee: 0 } + ); + let contract_events = EventsFilterBuilderTrait::from_events(@spy.get_events()) + .with_contract_address(protocol_handler.contract_address) + .build(); + contract_events.assert_emitted(@expected); +} + +#[test] +fn test_protocol_handler_execute_call_wrong_selector_should_fail() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + let unauthoried_selectors = [ + selector!("upgrade"), + selector!("transfer_ownership"), + selector!("pause"), + selector!("unpause"), + ]; + + // Change the caller to operator + start_cheat_caller_address(protocol_handler.contract_address, operator_mock()); + + // Get SafeDispatcher of protocolHandler + let safe_dispatcher = IProtocolHandlerSafeDispatcher { + contract_address: protocol_handler.contract_address + }; + + for selector in unauthoried_selectors + .span() { + // Mock the call to the Kakarot entrypoint + mock_call::<()>(kakarot_mock(), *selector, (), 1); + + // Construct the Call to protocol handler and call execute_call + // Should pass as caller is operator and call is to Kakarot + let call = Call { to: kakarot_mock(), selector: *selector, calldata: [].span() }; + + // Call the protocol handler execute_call + #[feature("safe_dispatcher")] + match safe_dispatcher.execute_call(call) { + Result::Ok(_) => panic!("Entrypoint did not panic"), + Result::Err(panic_data) => { + assert(*panic_data.at(0) == 'UNAUTHORIZED_SELECTOR', *panic_data.at(0)); + } + }; + } +} + + +#[test] +fn test_protocol_handler_execute_call_should_pass() { + let (protocol_handler, _) = setup_contracts_for_testing(); + + let authorized_selectors = [ + selector!("set_native_token"), + selector!("set_coinbase"), + selector!("set_prev_randao"), + selector!("set_block_gas_limit"), + selector!("set_account_contract_class_hash"), + selector!("set_uninitialized_account_class_hash"), + selector!("set_authorized_cairo_precompile_caller"), + selector!("set_cairo1_helpers_class_hash"), + selector!("upgrade_account"), + selector!("set_authorized_pre_eip155_tx"), + selector!("set_l1_messaging_contract_address"), + ]; + + // Change caller to operator + start_cheat_caller_address(protocol_handler.contract_address, operator_mock()); + + for selector in authorized_selectors + .span() { + // Mock the call to Kakarot entrypoint + mock_call::<()>(kakarot_mock(), *selector, (), 1); + + // Construct the Call to protocol handler and call execute_call + // Should pass as caller is operator and call is to Kakarot + let call = Call { to: kakarot_mock(), selector: *selector, calldata: [].span() }; + + // Spy on the events + let mut spy = spy_events(); + + // Call the protocol handler execute_call + protocol_handler.execute_call(call); + + // Assert that selector was called on Kakarot + assert_called_with::<()>(kakarot_mock(), *selector, ()); + + // Check the ExecuteCall event is emitted + let expected = ProtocolHandler::Event::Execution( + ProtocolHandler::Execution { call: call } + ); + let contract_events = EventsFilterBuilderTrait::from_events(@spy.get_events()) + .with_contract_address(protocol_handler.contract_address) + .build(); + contract_events.assert_emitted(@expected); + } +}