Merge branch 'main' of github.com:OpenZeppelin/cairo-contracts into f…

…eat/governance2-#294

CHANGELOG.md

@@ -20,9 +20,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - GovernorTimelockExecutionComponent
   - GovernorVotesQuorumFractionComponent
   - GovernorVotesComponent
+- `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)
 
+- The initializer in `OwnableComponent` now checks that `owner` is not the zero address (#1221)
 - Add `verifying_contract` member to the `Delegation` struct used in Votes `delegate_by_sig` (#1214)
 use crate::votes::VotesComponent::VotingUnitsTrait;
 - VotingUnitsTrait moved from `openzeppelin_governance::votes::votes` to `openzeppelin_governance::votes::VotesComponent` (#1214)

Scarb.toml

@@ -24,8 +24,8 @@ keywords.workspace = true
 [workspace.package]
 version = "0.19.0"
 edition = "2024_07"
-cairo-version = "2.8.4"
-scarb-version = "2.8.4"
+cairo-version = "2.8.5"
+scarb-version = "2.8.5"
 authors = ["OpenZeppelin Community <[email protected]>"]
 description = "OpenZeppelin Contracts written in Cairo for Starknet, a decentralized ZK Rollup"
 documentation = "https://docs.openzeppelin.com/contracts-cairo"
@@ -40,8 +40,8 @@ keywords = [
 ]
 
 [workspace.dependencies]
-assert_macros = "2.8.4"
-starknet = "2.8.4"
+assert_macros = "2.8.5"
+starknet = "2.8.5"
 snforge_std = "0.33.0"
 
 [dependencies]

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/access.adoc

@@ -156,7 +156,8 @@ Emits an xref:OwnableComponent-OwnershipTransferred[OwnershipTransferred] event.
 [[OwnableComponent-two-step-transfer_ownership]]
 ==== `[.contract-item-name]#++transfer_ownership++#++(ref self: ContractState, new_owner: ContractAddress)++` [.item-kind]#external#
 
-Starts the two step ownership transfer process, by setting the pending owner.
+Starts the two step ownership transfer process, by setting the pending owner. Setting `new_owner` to the zero address is allowed, this can be used to cancel an initiated ownership transfer.
+
 Can only be called by the current owner.
 
 Emits an xref:OwnableComponent-OwnershipTransferStarted[OwnershipTransferStarted] event.
@@ -211,6 +212,10 @@ See xref:OwnableComponent-two-step-renounce_ownership[renounce_ownership].
 
 Initializes the contract and sets `owner` as the initial owner.
 
+Requirements:
+
+- `owner` cannot be the zero address.
+
 Emits an xref:OwnableComponent-OwnershipTransferred[OwnershipTransferred] event.
 
 [.contract-item]

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
 

docs/modules/ROOT/pages/api/utilities.adoc

@@ -136,7 +136,7 @@ Returns the contract address when passing the given arguments to {deploy_syscall
 
 [.contract-item]
 [[deployments-compute_hash_on_elements]]
-==== `[.contract-item-name]#++compute_hash_on_elements++#++(mut data: Span<felt252>) → felt252++` [.item-kind]#function#
+==== `[.contract-item-name]#++compute_hash_on_elements++#++(data: Span<felt252>) → felt252++` [.item-kind]#function#
 
 Creates a Pedersen hash chain with the elements of `data` and returns the finalized hash.
 

packages/access/src/accesscontrol/accesscontrol.cairo

@@ -3,8 +3,19 @@
 
 /// # AccessControl Component
 ///
-/// The AccessControl component allows contracts to implement role-based access control mechanisms.
-/// Roles are referred to by their `felt252` identifier.
+/// The AccessControl component enables role-based access control mechanisms. This is a lightweight
+/// implementation that doesn't support on-chain enumeration of role members, though role membership
+/// can be tracked off-chain through contract events.
+///
+/// Roles can be granted and revoked dynamically via `grant_role` and `revoke_role`. Each role
+/// has an associated admin role that controls who can grant and revoke it. By default, all roles
+/// use `DEFAULT_ADMIN_ROLE` as their admin role.
+/// Accounts can also renounce roles they have been granted by using `renounce_role`.
+///
+/// More complex role hierarchies can be created using `set_role_admin`.
+///
+/// WARNING: The `DEFAULT_ADMIN_ROLE` is its own admin, meaning it can grant and revoke itself.
+/// Extra precautions should be taken to secure accounts with this role.
 #[starknet::component]
 pub mod AccessControlComponent {
     use crate::accesscontrol::interface;
@@ -31,16 +42,16 @@ pub mod AccessControlComponent {
 
     /// Emitted when `account` is granted `role`.
     ///
-    /// `sender` is the account that originated the contract call, an admin role
-    /// bearer (except if `_grant_role` is called during initialization from the constructor).
+    /// `sender` is the account that originated the contract call, an account with the admin role
+    /// or the deployer address if `grant_role` is called from the constructor.
     #[derive(Drop, PartialEq, starknet::Event)]
     pub struct RoleGranted {
         pub role: felt252,
         pub account: ContractAddress,
         pub sender: ContractAddress
     }
 
-    /// Emitted when `account` is revoked `role`.
+    /// Emitted when `role` is revoked for `account`.
     ///
     /// `sender` is the account that originated the contract call:
     ///   - If using `revoke_role`, it is the admin role bearer.
@@ -55,7 +66,7 @@ pub mod AccessControlComponent {
     /// Emitted when `new_admin_role` is set as `role`'s admin role, replacing `previous_admin_role`
     ///
     /// `DEFAULT_ADMIN_ROLE` is the starting admin for all roles, despite
-    /// {RoleAdminChanged} not being emitted signaling this.
+    /// `RoleAdminChanged` not being emitted signaling this.
     #[derive(Drop, PartialEq, starknet::Event)]
     pub struct RoleAdminChanged {
         pub role: felt252,
@@ -89,7 +100,7 @@ pub mod AccessControlComponent {
 
         /// Grants `role` to `account`.
         ///
-        /// If `account` had not been already granted `role`, emits a `RoleGranted` event.
+        /// If `account` has not been already granted `role`, emits a `RoleGranted` event.
         ///
         /// Requirements:
         ///
@@ -104,7 +115,7 @@ pub mod AccessControlComponent {
 
         /// Revokes `role` from `account`.
         ///
-        /// If `account` had been granted `role`, emits a `RoleRevoked` event.
+        /// If `account` has been granted `role`, emits a `RoleRevoked` event.
         ///
         /// Requirements:
         ///
@@ -132,7 +143,7 @@ pub mod AccessControlComponent {
         fn renounce_role(
             ref self: ComponentState<TContractState>, role: felt252, account: ContractAddress
         ) {
-            let caller: ContractAddress = get_caller_address();
+            let caller = get_caller_address();
             assert(caller == account, Errors::INVALID_CALLER);
             self._revoke_role(role, account);
         }
@@ -182,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/access/src/ownable/ownable.cairo

@@ -35,6 +35,8 @@ pub mod OwnableComponent {
         OwnershipTransferStarted: OwnershipTransferStarted
     }
 
+    /// Emitted when `new_owner` is set as owner of the contract.
+    /// `new_owner` can be set to zero only if the ownership is renounced.
     #[derive(Drop, PartialEq, starknet::Event)]
     pub struct OwnershipTransferred {
         #[key]
@@ -43,6 +45,9 @@ pub mod OwnableComponent {
         pub new_owner: ContractAddress,
     }
 
+    /// Emitted when `transfer_ownership` is called on a contract that implements `IOwnableTwoStep`.
+    /// `previous_owner` is the address of the current owner.
+    /// `new_owner` is the address of the pending owner.
     #[derive(Drop, PartialEq, starknet::Event)]
     pub struct OwnershipTransferStarted {
         #[key]
@@ -54,7 +59,6 @@ pub mod OwnableComponent {
     pub mod Errors {
         pub const NOT_OWNER: felt252 = 'Caller is not the owner';
         pub const NOT_PENDING_OWNER: felt252 = 'Caller is not the pending owner';
-        pub const ZERO_ADDRESS_CALLER: felt252 = 'Caller is the zero address';
         pub const ZERO_ADDRESS_OWNER: felt252 = 'New owner is the zero address';
     }
 
@@ -83,8 +87,8 @@ pub mod OwnableComponent {
             self._transfer_ownership(new_owner);
         }
 
-        /// Leaves the contract without owner. It will not be possible to call `assert_only_owner`
-        /// functions anymore. Can only be called by the current owner.
+        /// Leaves the contract without an owner. It will not be possible to call
+        /// `assert_only_owner` functions anymore. Can only be called by the current owner.
         ///
         /// Requirements:
         ///
@@ -129,6 +133,8 @@ pub mod OwnableComponent {
 
         /// Starts the two-step ownership transfer process by setting the pending owner.
         ///
+        /// Setting `new_owner` to the zero address is allowed; this can be used to cancel an
+        /// initiated ownership transfer.
         /// Requirements:
         ///
         /// - The caller is the contract owner.
@@ -150,7 +156,8 @@ pub mod OwnableComponent {
         ///
         /// Emits an `OwnershipTransferred` event.
         fn renounce_ownership(ref self: ComponentState<TContractState>) {
-            Ownable::renounce_ownership(ref self);
+            self.assert_only_owner();
+            self._transfer_ownership(Zero::zero());
         }
     }
 
@@ -270,8 +277,13 @@ pub mod OwnableComponent {
     > of InternalTrait<TContractState> {
         /// Sets the contract's initial owner.
         ///
+        /// Requirements:
+        ///
+        /// - `owner` is not the zero address.
+        ///
         /// This function should be called at construction time.
         fn initializer(ref self: ComponentState<TContractState>, owner: ContractAddress) {
+            assert(!owner.is_zero(), Errors::ZERO_ADDRESS_OWNER);
             self._transfer_ownership(owner);
         }
 
@@ -280,7 +292,6 @@ pub mod OwnableComponent {
         fn assert_only_owner(self: @ComponentState<TContractState>) {
             let owner = self.Ownable_owner.read();
             let caller = get_caller_address();
-            assert(!caller.is_zero(), Errors::ZERO_ADDRESS_CALLER);
             assert(caller == owner, Errors::NOT_OWNER);
         }
 
@@ -297,10 +308,7 @@ pub mod OwnableComponent {
 
             let previous_owner: ContractAddress = self.Ownable_owner.read();
             self.Ownable_owner.write(new_owner);
-            self
-                .emit(
-                    OwnershipTransferred { previous_owner: previous_owner, new_owner: new_owner }
-                );
+            self.emit(OwnershipTransferred { previous_owner, new_owner });
         }
 
         /// Sets a new pending owner.
@@ -311,12 +319,7 @@ pub mod OwnableComponent {
         fn _propose_owner(ref self: ComponentState<TContractState>, new_owner: ContractAddress) {
             let previous_owner = self.Ownable_owner.read();
             self.Ownable_pending_owner.write(new_owner);
-            self
-                .emit(
-                    OwnershipTransferStarted {
-                        previous_owner: previous_owner, new_owner: new_owner
-                    }
-                );
+            self.emit(OwnershipTransferStarted { previous_owner, new_owner });
         }
     }
 }