Minor improvements to Account package (#1224)

* Apply minor improvements to Account package

* Improve is_tx_version_valid description

* Apply minor improvements to tests

* Update changelog

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)
 

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<felt252>
+    calldata: Span<felt252>
 }
 
 /// 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<felt252>` 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.

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
 

packages/access/src/accesscontrol/accesscontrol.cairo

@@ -193,7 +193,7 @@ pub mod AccessControlComponent {
         impl SRC5: SRC5Component::HasComponent<TContractState>,
         +Drop<TContractState>
     > of InternalTrait<TContractState> {
-        /// Initializes the contract by registering the IAccessControl interface Id.
+        /// Initializes the contract by registering the IAccessControl interface ID.
         fn initializer(ref self: ComponentState<TContractState>) {
             let mut src5_component = get_dep_component_mut!(ref self, SRC5);
             src5_component.register_interface(interface::IACCESSCONTROL_ID);

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<TContractState>, mut calls: Array<Call>
+            self: @ComponentState<TContractState>, calls: Array<Call>
         ) -> Array<Span<felt252>> {
             // 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<TContractState>, mut calls: Array<Call>) -> felt252 {
+        fn __validate__(self: @ComponentState<TContractState>, calls: Array<Call>) -> felt252 {
             self.validate_transaction()
         }
 
@@ -309,8 +296,9 @@ pub mod AccountComponent {
         impl SRC5: SRC5Component::HasComponent<TContractState>,
         +Drop<TContractState>
     > of InternalTrait<TContractState> {
-        /// 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<TContractState>, 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<TContractState>) {
-            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<TContractState>) -> 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);

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<TContractState>, mut calls: Array<Call>
+            self: @ComponentState<TContractState>, calls: Array<Call>
         ) -> Array<Span<felt252>> {
             // 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<TContractState>, mut calls: Array<Call>) -> felt252 {
+        fn __validate__(self: @ComponentState<TContractState>, calls: Array<Call>) -> felt252 {
             self.validate_transaction()
         }
 
@@ -317,8 +302,9 @@ pub mod EthAccountComponent {
         impl SRC5: SRC5Component::HasComponent<TContractState>,
         +Drop<TContractState>
     > of InternalTrait<TContractState> {
-        /// 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<TContractState>, 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<TContractState>) {
-            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<TContractState>) -> 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);

packages/account/src/extensions/src9/src9.cairo

@@ -137,7 +137,7 @@ pub mod SRC9Component {
         impl SRC5: SRC5Component::HasComponent<TContractState>,
         +Drop<TContractState>
     > of InternalTrait<TContractState> {
-        /// 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<TContractState>) {
             let mut src5_component = get_dep_component_mut!(ref self, SRC5);
             src5_component.register_interface(interface::ISRC9_V2_ID);

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);