diff --git a/CHANGELOG.md b/CHANGELOG.md index b182243de..7c8efd650 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,10 +12,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - SRC9 (Outside Execution) integration to account presets (#1201) +- `is_tx_version_valid` utility function to `openzeppelin_account::utils` (#1224) ### Changed - Remove `mut` from `data` param in `compute_hash_on_elements` (#1206) +- Remove `mut` from `calls` param in `__execute__` function of Account and EthAccount components (#1224) +- Remove `mut` from `calls` param in `__validate__` function of Account and EthAccount components (#1224) ### Changed (Breaking) diff --git a/docs/modules/ROOT/pages/accounts.adoc b/docs/modules/ROOT/pages/accounts.adoc index 077591d95..af4a3dca8 100644 --- a/docs/modules/ROOT/pages/accounts.adoc +++ b/docs/modules/ROOT/pages/accounts.adoc @@ -32,7 +32,7 @@ supporting this execution flow and interoperability with DApps in the ecosystem. struct Call { to: ContractAddress, selector: felt252, - calldata: Array + calldata: Span } /// Standard Account Interface @@ -50,7 +50,7 @@ pub trait ISRC6 { ---- WARNING: The `calldata` member of the `Call` struct in the accounts has been updated to `Span` for optimization -purposes, but the interface Id remains the same for backwards compatibility. This inconsistency will be fixed in future releases. +purposes, but the interface ID remains the same for backwards compatibility. This inconsistency will be fixed in future releases. {snip-6}[SNIP-6] adds the `is_valid_signature` method. This method is not used by the protocol, but it's useful for DApps to verify the validity of signatures, supporting features like Sign In with Starknet. @@ -70,7 +70,7 @@ pub trait ISRC5 { } ---- -{snip-6}[SNIP-6] compliant accounts must return `true` when queried for the ISRC6 interface Id. +{snip-6}[SNIP-6] compliant accounts must return `true` when queried for the ISRC6 interface ID. Even though these interfaces are not enforced by the protocol, it's recommended to implement them for enabling interoperability with the ecosystem. diff --git a/docs/modules/ROOT/pages/api/account.adoc b/docs/modules/ROOT/pages/api/account.adoc index dde7678fa..78944c81b 100644 --- a/docs/modules/ROOT/pages/api/account.adoc +++ b/docs/modules/ROOT/pages/api/account.adoc @@ -252,7 +252,7 @@ See xref:AccountComponent-set_public_key[set_public_key]. [[AccountComponent-initializer]] ==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, public_key: felt252)++` [.item-kind]#internal# -Initializes the account with the given public key, and registers the ISRC6 interface ID. +Initializes the account with the given public key, and registers the `ISRC6` interface ID. Emits an {OwnerAdded} event. @@ -503,7 +503,7 @@ See xref:EthAccountComponent-set_public_key[set_public_key]. [[EthAccountComponent-initializer]] ==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState, public_key: EthPublicKey)++` [.item-kind]#internal# -Initializes the account with the given public key, and registers the ISRC6 interface ID. +Initializes the account with the given public key, and registers the `ISRC6` interface ID. Emits an {OwnerAdded} event. @@ -692,7 +692,7 @@ Returns the status of a given nonce. `true` if the nonce is available to use. [[SRC9Component-initializer]] ==== `[.contract-item-name]#++initializer++#++(ref self: ComponentState)++` [.item-kind]#internal# -Initializes the account by registering the `ISRC9_V2` interface Id. +Initializes the account by registering the `ISRC9_V2` interface ID. == Presets diff --git a/packages/access/src/accesscontrol/accesscontrol.cairo b/packages/access/src/accesscontrol/accesscontrol.cairo index c9d0eb906..09761ad3a 100644 --- a/packages/access/src/accesscontrol/accesscontrol.cairo +++ b/packages/access/src/accesscontrol/accesscontrol.cairo @@ -193,7 +193,7 @@ pub mod AccessControlComponent { impl SRC5: SRC5Component::HasComponent, +Drop > of InternalTrait { - /// Initializes the contract by registering the IAccessControl interface Id. + /// Initializes the contract by registering the IAccessControl interface ID. fn initializer(ref self: ComponentState) { let mut src5_component = get_dep_component_mut!(ref self, SRC5); src5_component.register_interface(interface::IACCESSCONTROL_ID); diff --git a/packages/account/src/account.cairo b/packages/account/src/account.cairo index a1a67a384..29f96b455 100644 --- a/packages/account/src/account.cairo +++ b/packages/account/src/account.cairo @@ -10,14 +10,12 @@ pub mod AccountComponent { use core::num::traits::Zero; use core::poseidon::PoseidonTrait; use crate::interface; - use crate::utils::{MIN_TRANSACTION_VERSION, QUERY_OFFSET}; - use crate::utils::{execute_calls, is_valid_stark_signature}; + use crate::utils::{is_tx_version_valid, execute_calls, is_valid_stark_signature}; use openzeppelin_introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; use openzeppelin_introspection::src5::SRC5Component::SRC5Impl; use openzeppelin_introspection::src5::SRC5Component; use starknet::account::Call; use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; - use starknet::{get_caller_address, get_contract_address, get_tx_info}; #[storage] pub struct Storage { @@ -66,34 +64,23 @@ pub mod AccountComponent { /// Requirements: /// /// - The transaction version must be greater than or equal to `MIN_TRANSACTION_VERSION`. - /// - If the transaction is a simulation (version than `QUERY_OFFSET`), it must be + /// - If the transaction is a simulation (version >= `QUERY_OFFSET`), it must be /// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION`. fn __execute__( - self: @ComponentState, mut calls: Array + self: @ComponentState, calls: Array ) -> Array> { // Avoid calls from other contracts // https://github.com/OpenZeppelin/cairo-contracts/issues/344 - let sender = get_caller_address(); + let sender = starknet::get_caller_address(); assert(sender.is_zero(), Errors::INVALID_CALLER); - - // Check tx version - let tx_info = get_tx_info().unbox(); - let tx_version: u256 = tx_info.version.into(); - // Check if tx is a query - if (tx_version >= QUERY_OFFSET) { - assert( - QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION - ); - } else { - assert(MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION); - } + assert(is_tx_version_valid(), Errors::INVALID_TX_VERSION); execute_calls(calls.span()) } /// Verifies the validity of the signature for the current transaction. /// This function is used by the protocol to verify `invoke` transactions. - fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { + fn __validate__(self: @ComponentState, calls: Array) -> felt252 { self.validate_transaction() } @@ -309,8 +296,9 @@ pub mod AccountComponent { impl SRC5: SRC5Component::HasComponent, +Drop > of InternalTrait { - /// Initializes the account by setting the initial public key - /// and registering the ISRC6 interface Id. + /// Initializes the account with the given public key, and registers the ISRC6 interface ID. + /// + /// Emits an `OwnerAdded` event. fn initializer(ref self: ComponentState, public_key: felt252) { let mut src5_component = get_dep_component_mut!(ref self, SRC5); src5_component.register_interface(interface::ISRC6_ID); @@ -319,8 +307,8 @@ pub mod AccountComponent { /// Validates that the caller is the account itself. Otherwise it reverts. fn assert_only_self(self: @ComponentState) { - let caller = get_caller_address(); - let self = get_contract_address(); + let caller = starknet::get_caller_address(); + let self = starknet::get_contract_address(); assert(self == caller, Errors::UNAUTHORIZED); } @@ -341,7 +329,7 @@ pub mod AccountComponent { let message_hash = PoseidonTrait::new() .update_with('StarkNet Message') .update_with('accept_ownership') - .update_with(get_contract_address()) + .update_with(starknet::get_contract_address()) .update_with(current_owner) .finalize(); @@ -352,7 +340,7 @@ pub mod AccountComponent { /// Validates the signature for the current transaction. /// Returns the short string `VALID` if valid, otherwise it reverts. fn validate_transaction(self: @ComponentState) -> felt252 { - let tx_info = get_tx_info().unbox(); + let tx_info = starknet::get_tx_info().unbox(); let tx_hash = tx_info.transaction_hash; let signature = tx_info.signature; assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); diff --git a/packages/account/src/eth_account.cairo b/packages/account/src/eth_account.cairo index 3b19610c7..e1c81f19d 100644 --- a/packages/account/src/eth_account.cairo +++ b/packages/account/src/eth_account.cairo @@ -13,16 +13,12 @@ pub mod EthAccountComponent { use crate::interface::EthPublicKey; use crate::interface; use crate::utils::secp256_point::Secp256PointStorePacking; - use crate::utils::{MIN_TRANSACTION_VERSION, QUERY_OFFSET}; - use crate::utils::{execute_calls, is_valid_eth_signature}; + use crate::utils::{is_tx_version_valid, execute_calls, is_valid_eth_signature}; use openzeppelin_introspection::src5::SRC5Component::InternalTrait as SRC5InternalTrait; use openzeppelin_introspection::src5::SRC5Component::SRC5Impl; use openzeppelin_introspection::src5::SRC5Component; use starknet::SyscallResultTrait; use starknet::account::Call; - use starknet::get_caller_address; - use starknet::get_contract_address; - use starknet::get_tx_info; use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess}; #[storage] @@ -72,34 +68,23 @@ pub mod EthAccountComponent { /// Requirements: /// /// - The transaction version must be greater than or equal to `MIN_TRANSACTION_VERSION`. - /// - If the transaction is a simulation (version than `QUERY_OFFSET`), it must be + /// - If the transaction is a simulation (version >= `QUERY_OFFSET`), it must be /// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION`. fn __execute__( - self: @ComponentState, mut calls: Array + self: @ComponentState, calls: Array ) -> Array> { // Avoid calls from other contracts // https://github.com/OpenZeppelin/cairo-contracts/issues/344 - let sender = get_caller_address(); + let sender = starknet::get_caller_address(); assert(sender.is_zero(), Errors::INVALID_CALLER); - - // Check tx version - let tx_info = get_tx_info().unbox(); - let tx_version: u256 = tx_info.version.into(); - // Check if tx is a query - if (tx_version >= QUERY_OFFSET) { - assert( - QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION - ); - } else { - assert(MIN_TRANSACTION_VERSION <= tx_version, Errors::INVALID_TX_VERSION); - } + assert(is_tx_version_valid(), Errors::INVALID_TX_VERSION); execute_calls(calls.span()) } /// Verifies the validity of the signature for the current transaction. /// This function is used by the protocol to verify `invoke` transactions. - fn __validate__(self: @ComponentState, mut calls: Array) -> felt252 { + fn __validate__(self: @ComponentState, calls: Array) -> felt252 { self.validate_transaction() } @@ -317,8 +302,9 @@ pub mod EthAccountComponent { impl SRC5: SRC5Component::HasComponent, +Drop > of InternalTrait { - /// Initializes the account by setting the initial public key - /// and registering the ISRC6 interface Id. + /// Initializes the account with the given public key, and registers the ISRC6 interface ID. + /// + /// Emits an `OwnerAdded` event. fn initializer(ref self: ComponentState, public_key: EthPublicKey) { let mut src5_component = get_dep_component_mut!(ref self, SRC5); src5_component.register_interface(interface::ISRC6_ID); @@ -327,8 +313,8 @@ pub mod EthAccountComponent { /// Validates that the caller is the account itself. Otherwise it reverts. fn assert_only_self(self: @ComponentState) { - let caller = get_caller_address(); - let self = get_contract_address(); + let caller = starknet::get_caller_address(); + let self = starknet::get_contract_address(); assert(self == caller, Errors::UNAUTHORIZED); } @@ -349,7 +335,7 @@ pub mod EthAccountComponent { let message_hash = PoseidonTrait::new() .update_with('StarkNet Message') .update_with('accept_ownership') - .update_with(get_contract_address()) + .update_with(starknet::get_contract_address()) .update_with(current_owner.get_coordinates().unwrap_syscall()) .finalize(); @@ -360,7 +346,7 @@ pub mod EthAccountComponent { /// Validates the signature for the current transaction. /// Returns the short string `VALID` if valid, otherwise it reverts. fn validate_transaction(self: @ComponentState) -> felt252 { - let tx_info = get_tx_info().unbox(); + let tx_info = starknet::get_tx_info().unbox(); let tx_hash = tx_info.transaction_hash; let signature = tx_info.signature; assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE); diff --git a/packages/account/src/extensions/src9/src9.cairo b/packages/account/src/extensions/src9/src9.cairo index 182bf4735..a5d3609af 100644 --- a/packages/account/src/extensions/src9/src9.cairo +++ b/packages/account/src/extensions/src9/src9.cairo @@ -137,7 +137,7 @@ pub mod SRC9Component { impl SRC5: SRC5Component::HasComponent, +Drop > of InternalTrait { - /// Initializes the account by registering the ISRC9_V2 interface Id. + /// Initializes the account by registering the ISRC9_V2 interface ID. fn initializer(ref self: ComponentState) { let mut src5_component = get_dep_component_mut!(ref self, SRC5); src5_component.register_interface(interface::ISRC9_V2_ID); diff --git a/packages/account/src/tests/extensions/test_src9.cairo b/packages/account/src/tests/extensions/test_src9.cairo index 9547ce586..20fe1c9fc 100644 --- a/packages/account/src/tests/extensions/test_src9.cairo +++ b/packages/account/src/tests/extensions/test_src9.cairo @@ -10,10 +10,8 @@ use openzeppelin_testing::constants::{RECIPIENT, OWNER, OTHER, FELT_VALUE}; use openzeppelin_utils::cryptography::snip12::OffchainMessageHash; use snforge_std::signature::KeyPairTrait; use snforge_std::signature::stark_curve::{StarkCurveKeyPairImpl, StarkCurveSignerImpl}; -use snforge_std::{ - start_cheat_caller_address, cheat_caller_address, start_cheat_block_timestamp_global -}; -use snforge_std::{test_address, load, CheatSpan}; +use snforge_std::{start_cheat_block_timestamp_global, test_address, load, CheatSpan}; +use snforge_std::{start_cheat_caller_address, cheat_caller_address}; use starknet::account::Call; use starknet::storage::StorageMapWriteAccess; use starknet::{ContractAddress, contract_address_const}; @@ -124,7 +122,7 @@ fn test_execute_from_outside_v2_uses_nonce() { } #[test] -#[should_panic(expected: ('SRC9: invalid caller',))] +#[should_panic(expected: 'SRC9: invalid caller')] fn test_execute_from_outside_v2_caller_mismatch() { let mut state = setup(); let mut outside_execution = setup_outside_execution(RECIPIENT(), false); @@ -136,7 +134,7 @@ fn test_execute_from_outside_v2_caller_mismatch() { } #[test] -#[should_panic(expected: ('SRC9: now >= execute_before',))] +#[should_panic(expected: 'SRC9: now >= execute_before')] fn test_execute_from_outside_v2_call_after_execute_before() { let mut state = setup(); let outside_execution = setup_outside_execution(RECIPIENT(), false); @@ -147,7 +145,7 @@ fn test_execute_from_outside_v2_call_after_execute_before() { } #[test] -#[should_panic(expected: ('SRC9: now >= execute_before',))] +#[should_panic(expected: 'SRC9: now >= execute_before')] fn test_execute_from_outside_v2_call_equal_to_execute_before() { let mut state = setup(); let outside_execution = setup_outside_execution(RECIPIENT(), false); @@ -158,7 +156,7 @@ fn test_execute_from_outside_v2_call_equal_to_execute_before() { } #[test] -#[should_panic(expected: ('SRC9: now <= execute_after',))] +#[should_panic(expected: 'SRC9: now <= execute_after')] fn test_execute_from_outside_v2_call_before_execute_after() { let mut state = setup(); let outside_execution = setup_outside_execution(RECIPIENT(), false); @@ -169,7 +167,7 @@ fn test_execute_from_outside_v2_call_before_execute_after() { } #[test] -#[should_panic(expected: ('SRC9: now <= execute_after',))] +#[should_panic(expected: 'SRC9: now <= execute_after')] fn test_execute_from_outside_v2_call_equal_to_execute_after() { let mut state = setup(); let outside_execution = setup_outside_execution(RECIPIENT(), false); @@ -180,7 +178,7 @@ fn test_execute_from_outside_v2_call_equal_to_execute_after() { } #[test] -#[should_panic(expected: ('SRC9: duplicated nonce',))] +#[should_panic(expected: 'SRC9: duplicated nonce')] fn test_execute_from_outside_v2_invalid_nonce() { let mut state = setup(); let outside_execution = setup_outside_execution(RECIPIENT(), false); @@ -191,7 +189,7 @@ fn test_execute_from_outside_v2_invalid_nonce() { } #[test] -#[should_panic(expected: ('SRC9: invalid signature',))] +#[should_panic(expected: 'SRC9: invalid signature')] fn test_execute_from_outside_v2_invalid_signature() { let key_pair = KeyPairTrait::generate(); let account = setup_account(key_pair.public_key); diff --git a/packages/account/src/tests/test_account.cairo b/packages/account/src/tests/test_account.cairo index 4924f3ab4..dae79c71f 100644 --- a/packages/account/src/tests/test_account.cairo +++ b/packages/account/src/tests/test_account.cairo @@ -11,15 +11,11 @@ use openzeppelin_test_common::mocks::account::DualCaseAccountMock; use openzeppelin_test_common::mocks::simple::{ISimpleMockDispatcher, ISimpleMockDispatcherTrait}; use openzeppelin_testing as utils; use openzeppelin_testing::constants::stark::{KEY_PAIR, KEY_PAIR_2}; -use openzeppelin_testing::constants::{ - SALT, ZERO, OTHER, CALLER, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION -}; +use openzeppelin_testing::constants::{SALT, QUERY_OFFSET, QUERY_VERSION, MIN_TRANSACTION_VERSION}; +use openzeppelin_testing::constants::{ZERO, OTHER, CALLER}; use openzeppelin_testing::signing::StarkKeyPair; -use snforge_std::{ - start_cheat_signature_global, start_cheat_transaction_version_global, - start_cheat_transaction_hash_global -}; -use snforge_std::{spy_events, test_address, start_cheat_caller_address}; +use snforge_std::{spy_events, start_cheat_signature_global, start_cheat_transaction_hash_global}; +use snforge_std::{test_address, start_cheat_caller_address, start_cheat_transaction_version_global}; use starknet::account::Call; // @@ -48,13 +44,13 @@ fn setup_dispatcher( let contract_class = utils::declare_class("DualCaseAccountMock"); let calldata = array![key_pair.public_key]; - let address = utils::deploy(contract_class, calldata); - let dispatcher = AccountABIDispatcher { contract_address: address }; + let contract_address = utils::deploy(contract_class, calldata); + let dispatcher = AccountABIDispatcher { contract_address }; start_cheat_signature_global(array![data.r, data.s].span()); start_cheat_transaction_hash_global(data.tx_hash); start_cheat_transaction_version_global(MIN_TRANSACTION_VERSION); - start_cheat_caller_address(address, ZERO()); + start_cheat_caller_address(contract_address, ZERO()); (dispatcher, contract_class.class_hash.into()) } @@ -114,7 +110,7 @@ fn test_validate_deploy() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_deploy_invalid_signature_data() { let key_pair = KEY_PAIR(); let mut data = SIGNED_TX_DATA(key_pair); @@ -125,7 +121,7 @@ fn test_validate_deploy_invalid_signature_data() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_deploy_invalid_signature_length() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -136,7 +132,7 @@ fn test_validate_deploy_invalid_signature_length() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_deploy_empty_signature() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -159,7 +155,7 @@ fn test_validate_declare() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_declare_invalid_signature_data() { let key_pair = KEY_PAIR(); let mut data = SIGNED_TX_DATA(key_pair); @@ -170,7 +166,7 @@ fn test_validate_declare_invalid_signature_data() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_declare_invalid_signature_length() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -181,7 +177,7 @@ fn test_validate_declare_invalid_signature_length() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_declare_empty_signature() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -197,16 +193,14 @@ fn test_execute_with_version(version: Option) { // Deploy target contract let calldata = array![]; - let address = utils::declare_and_deploy("SimpleMock", calldata); - let simple_mock = ISimpleMockDispatcher { contract_address: address }; + let contract_address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address }; // Craft call and add to calls array let amount = 200; let calldata = array![amount]; let call = Call { - to: simple_mock.contract_address, - selector: selector!("increase_balance"), - calldata: calldata.span() + to: contract_address, selector: selector!("increase_balance"), calldata: calldata.span() }; let calls = array![call]; @@ -243,7 +237,7 @@ fn test_execute_query_version() { } #[test] -#[should_panic(expected: ('Account: invalid tx version',))] +#[should_panic(expected: 'Account: invalid tx version')] fn test_execute_invalid_query_version() { test_execute_with_version(Option::Some(QUERY_OFFSET)); } @@ -254,7 +248,7 @@ fn test_execute_future_query_version() { } #[test] -#[should_panic(expected: ('Account: invalid tx version',))] +#[should_panic(expected: 'Account: invalid tx version')] fn test_execute_invalid_version() { test_execute_with_version(Option::Some(MIN_TRANSACTION_VERSION - 1)); } @@ -270,7 +264,7 @@ fn test_validate() { } #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_validate_invalid() { let key_pair = KEY_PAIR(); let mut data = SIGNED_TX_DATA(key_pair); @@ -288,25 +282,21 @@ fn test_multicall() { // Deploy target contract let calldata = array![]; - let address = utils::declare_and_deploy("SimpleMock", calldata); - let simple_mock = ISimpleMockDispatcher { contract_address: address }; + let contract_address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address }; // Craft 1st call let amount1 = 300; let calldata1 = array![amount1]; let call1 = Call { - to: simple_mock.contract_address, - selector: selector!("increase_balance"), - calldata: calldata1.span() + to: contract_address, selector: selector!("increase_balance"), calldata: calldata1.span() }; // Craft 2nd call let amount2 = 500; let calldata2 = array![amount2]; let call2 = Call { - to: simple_mock.contract_address, - selector: selector!("increase_balance"), - calldata: calldata2.span() + to: contract_address, selector: selector!("increase_balance"), calldata: calldata2.span() }; // Bundle calls and execute @@ -339,7 +329,7 @@ fn test_multicall_zero_calls() { } #[test] -#[should_panic(expected: ('Account: invalid caller',))] +#[should_panic(expected: 'Account: invalid caller')] fn test_account_called_from_contract() { let state = setup(KEY_PAIR()); let account_address = test_address(); @@ -379,7 +369,7 @@ fn test_public_key_setter_and_getter() { } #[test] -#[should_panic(expected: ('Account: unauthorized',))] +#[should_panic(expected: 'Account: unauthorized')] fn test_public_key_setter_different_account() { let mut state = COMPONENT_STATE(); let account_address = test_address(); @@ -418,7 +408,7 @@ fn test_public_key_setter_and_getter_camel() { } #[test] -#[should_panic(expected: ('Account: unauthorized',))] +#[should_panic(expected: 'Account: unauthorized')] fn test_public_key_setter_different_account_camel() { let mut state = COMPONENT_STATE(); let account_address = test_address(); @@ -462,7 +452,7 @@ fn test_assert_only_self_true() { } #[test] -#[should_panic(expected: ('Account: unauthorized',))] +#[should_panic(expected: 'Account: unauthorized')] fn test_assert_only_self_false() { let mut state = COMPONENT_STATE(); let account_address = test_address(); @@ -487,7 +477,7 @@ fn test_assert_valid_new_owner() { #[test] -#[should_panic(expected: ('Account: invalid signature',))] +#[should_panic(expected: 'Account: invalid signature')] fn test_assert_valid_new_owner_invalid_signature() { let key_pair = KEY_PAIR(); let state = setup(key_pair); diff --git a/packages/account/src/tests/test_eth_account.cairo b/packages/account/src/tests/test_eth_account.cairo index 7c844a075..49ae89e2a 100644 --- a/packages/account/src/tests/test_eth_account.cairo +++ b/packages/account/src/tests/test_eth_account.cairo @@ -14,16 +14,12 @@ use openzeppelin_test_common::mocks::account::DualCaseEthAccountMock; use openzeppelin_test_common::mocks::simple::{ISimpleMockDispatcher, ISimpleMockDispatcherTrait}; use openzeppelin_testing as utils; use openzeppelin_testing::constants::secp256k1::{KEY_PAIR, KEY_PAIR_2}; -use openzeppelin_testing::constants::{ - SALT, ZERO, OTHER, CALLER, QUERY_VERSION, MIN_TRANSACTION_VERSION -}; +use openzeppelin_testing::constants::{SALT, QUERY_VERSION, MIN_TRANSACTION_VERSION}; +use openzeppelin_testing::constants::{ZERO, OTHER, CALLER}; use openzeppelin_testing::signing::Secp256k1KeyPair; use openzeppelin_utils::serde::SerializedAppend; -use snforge_std::{ - start_cheat_signature_global, start_cheat_transaction_version_global, - start_cheat_transaction_hash_global, start_cheat_caller_address -}; -use snforge_std::{spy_events, test_address}; +use snforge_std::{spy_events, start_cheat_signature_global, start_cheat_transaction_hash_global}; +use snforge_std::{test_address, start_cheat_caller_address, start_cheat_transaction_version_global}; use starknet::account::Call; // @@ -135,7 +131,7 @@ fn test_validate_deploy() { } #[test] -#[should_panic(expected: ('EthAccount: invalid signature',))] +#[should_panic(expected: 'EthAccount: invalid signature')] fn test_validate_deploy_invalid_signature_data() { let key_pair = KEY_PAIR(); let mut data = SIGNED_TX_DATA(key_pair); @@ -146,7 +142,7 @@ fn test_validate_deploy_invalid_signature_data() { } #[test] -#[should_panic(expected: ('Signature: Invalid format.',))] +#[should_panic(expected: 'Signature: Invalid format.')] fn test_validate_deploy_invalid_signature_length() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -158,7 +154,7 @@ fn test_validate_deploy_invalid_signature_length() { } #[test] -#[should_panic(expected: ('Signature: Invalid format.',))] +#[should_panic(expected: 'Signature: Invalid format.')] fn test_validate_deploy_empty_signature() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -181,7 +177,7 @@ fn test_validate_declare() { } #[test] -#[should_panic(expected: ('EthAccount: invalid signature',))] +#[should_panic(expected: 'EthAccount: invalid signature')] fn test_validate_declare_invalid_signature_data() { let key_pair = KEY_PAIR(); let mut data = SIGNED_TX_DATA(key_pair); @@ -192,7 +188,7 @@ fn test_validate_declare_invalid_signature_data() { } #[test] -#[should_panic(expected: ('Signature: Invalid format.',))] +#[should_panic(expected: 'Signature: Invalid format.')] fn test_validate_declare_invalid_signature_length() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -204,7 +200,7 @@ fn test_validate_declare_invalid_signature_length() { } #[test] -#[should_panic(expected: ('Signature: Invalid format.',))] +#[should_panic(expected: 'Signature: Invalid format.')] fn test_validate_declare_empty_signature() { let key_pair = KEY_PAIR(); let (account, class_hash) = setup_dispatcher(key_pair, SIGNED_TX_DATA(key_pair)); @@ -222,16 +218,14 @@ fn test_execute_with_version(version: Option) { // Deploy target contract let calldata = array![]; - let address = utils::declare_and_deploy("SimpleMock", calldata); - let simple_mock = ISimpleMockDispatcher { contract_address: address }; + let contract_address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address }; // Craft call and add to calls array let amount = 200; let calldata = array![amount]; let call = Call { - to: simple_mock.contract_address, - selector: selector!("increase_balance"), - calldata: calldata.span() + to: contract_address, selector: selector!("increase_balance"), calldata: calldata.span() }; let calls = array![call]; @@ -297,25 +291,21 @@ fn test_multicall() { // Deploy target contract let calldata = array![]; - let address = utils::declare_and_deploy("SimpleMock", calldata); - let simple_mock = ISimpleMockDispatcher { contract_address: address }; + let contract_address = utils::declare_and_deploy("SimpleMock", calldata); + let simple_mock = ISimpleMockDispatcher { contract_address }; // Craft 1st call let amount1 = 300; let calldata1 = array![amount1]; let call1 = Call { - to: simple_mock.contract_address, - selector: selector!("increase_balance"), - calldata: calldata1.span() + to: contract_address, selector: selector!("increase_balance"), calldata: calldata1.span() }; // Craft 2nd call let amount2 = 500; let calldata2 = array![amount2]; let call2 = Call { - to: simple_mock.contract_address, - selector: selector!("increase_balance"), - calldata: calldata2.span() + to: contract_address, selector: selector!("increase_balance"), calldata: calldata2.span() }; // Bundle calls and execute @@ -407,7 +397,7 @@ fn test_public_key_setter_and_getter() { } #[test] -#[should_panic(expected: ('EthAccount: unauthorized',))] +#[should_panic(expected: 'EthAccount: unauthorized')] fn test_public_key_setter_different_account() { let mut state = COMPONENT_STATE(); let key_pair = KEY_PAIR(); @@ -451,7 +441,7 @@ fn test_public_key_setter_and_getter_camel() { } #[test] -#[should_panic(expected: ('EthAccount: unauthorized',))] +#[should_panic(expected: 'EthAccount: unauthorized')] fn test_public_key_setter_different_account_camel() { let mut state = COMPONENT_STATE(); let key_pair = KEY_PAIR(); @@ -499,7 +489,7 @@ fn test_assert_only_self_true() { } #[test] -#[should_panic(expected: ('EthAccount: unauthorized',))] +#[should_panic(expected: 'EthAccount: unauthorized')] fn test_assert_only_self_false() { let state = COMPONENT_STATE(); @@ -522,7 +512,7 @@ fn test_assert_valid_new_owner() { } #[test] -#[should_panic(expected: ('EthAccount: invalid signature',))] +#[should_panic(expected: 'EthAccount: invalid signature')] fn test_assert_valid_new_owner_invalid_signature() { let key_pair = KEY_PAIR(); let state = setup(key_pair); diff --git a/packages/account/src/tests/test_signature.cairo b/packages/account/src/tests/test_signature.cairo index f7841b2b3..0d6e9d648 100644 --- a/packages/account/src/tests/test_signature.cairo +++ b/packages/account/src/tests/test_signature.cairo @@ -83,7 +83,7 @@ fn test_is_valid_eth_signature_bad_sig() { } #[test] -#[should_panic(expected: ('Signature: Invalid format.',))] +#[should_panic(expected: 'Signature: Invalid format.')] fn test_is_valid_eth_signature_invalid_format_sig() { let key_pair = secp256k1::KEY_PAIR(); let data = eth_signature_data(key_pair); diff --git a/packages/account/src/utils.cairo b/packages/account/src/utils.cairo index 4537a1c1f..e31f8779e 100644 --- a/packages/account/src/utils.cairo +++ b/packages/account/src/utils.cairo @@ -14,20 +14,27 @@ pub const QUERY_OFFSET: u256 = 0x100000000000000000000000000000000; // QUERY_OFFSET + TRANSACTION_VERSION pub const QUERY_VERSION: u256 = 0x100000000000000000000000000000001; -pub fn execute_calls(mut calls: Span) -> Array> { +pub fn execute_calls(calls: Span) -> Array> { let mut res = array![]; - loop { - match calls.pop_front() { - Option::Some(call) => { - let _res = execute_single_call(call); - res.append(_res); - }, - Option::None => { break (); }, - }; + for call in calls { + res.append(execute_single_call(call)) }; res } +/// If the transaction is a simulation (version >= `QUERY_OFFSET`), it must be +/// greater than or equal to `QUERY_OFFSET` + `MIN_TRANSACTION_VERSION` to be considered valid. +/// Otherwise, it must be greater than or equal to `MIN_TRANSACTION_VERSION`. +pub fn is_tx_version_valid() -> bool { + let tx_info = starknet::get_tx_info().unbox(); + let tx_version = tx_info.version.into(); + if tx_version >= QUERY_OFFSET { + QUERY_OFFSET + MIN_TRANSACTION_VERSION <= tx_version + } else { + MIN_TRANSACTION_VERSION <= tx_version + } +} + fn execute_single_call(call: @Call) -> Span { let Call { to, selector, calldata } = *call; starknet::syscalls::call_contract_syscall(to, selector, calldata).unwrap_syscall()