dev: two step ownable (#809)

* dev: two step ownable * chore: lint * test(ownable): two step test suite * dev: apply suggestions from code review Co-authored-by: Eric Nordelo <[email protected]> * chore: removing explicit types * dev: asserting ownable events keys * docs: OwnableTwoStep API * docs: OwnableTwoStep usage and impl section * docs: using OwnableTwoStepComponent everywhere * dev: adding pendingOwner * doc: no more OwnableTwoStepComponent * docs: adding OwnableTwoStepImpl snippet * test: using component_state_for_testing * chore: updating CHANGELOG * chore: update CHANGELOG.md Co-authored-by: Martín Triay <[email protected]> * dev: allow two step ownable transfer to zero * docs: two step interface * docs: access API with separate two step sections * lint: rm extra blank line * chore: PR comments * fix: impl order * doc: small fixes * doc: combining section overview * lint: changelog.md * doc: section ordering --------- Co-authored-by: Eric Nordelo <[email protected]> Co-authored-by: Martín Triay <[email protected]>
OpenZeppelin · Feb 9, 2024 · 6adcb2d · 6adcb2d
1 parent 2815498
commit 6adcb2d
Show file tree

Hide file tree

Showing 9 changed files with 682 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,11 +11,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 
 - EthAccount component and preset (#853)
+- Ownable two-step functionality (#809)
 
 ### Changed
 
 - Bump scarb to v2.4.4 (#853)
 - Bump scarb to v2.5.3 (#898)
+- OwnershipTransferred event args are indexed (#809)
 
 ### Removed
 

diff --git a/docs/modules/ROOT/pages/access.adoc b/docs/modules/ROOT/pages/access.adoc
@@ -20,7 +20,7 @@ OpenZeppelin Contracts for Cairo provides {ownable-cairo} for implementing owner
 
 Integrating this component into a contract first requires assigning an owner.
 The implementing contract's constructor should set the initial owner by passing the owner's address to Ownable's
-xref:/api/access.adoc#AccessControlComponent-initializer[`initializer`] like this:
+xref:/api/access.adoc#OwnableComponent-initializer[`initializer`] like this:
 
 [,javascript]
 ----
@@ -106,6 +106,49 @@ after an initial stage with centralized administration is over.
 WARNING: Removing the owner altogether will mean that administrative tasks that are protected by `assert_only_owner`
 will no longer be callable!
 
+=== Two step transfer
+
+The component also offers a more robust way of transferring ownership via the
+xref:/api/access.adoc#OwnableTwoStepImpl[OwnableTwoStepImpl] implementation. A two step transfer mechanism helps
+to prevent unintended and irreversible owner transfers. Simply replace the `OwnableImpl` and `OwnableCamelOnlyImpl`
+with their respective two step variants:
+
+[,javascript]
+----
+#[abi(embed_v0)]
+impl OwnableTwoStepImpl = OwnableComponent::OwnableTwoStepImpl<ContractState>;
+#[abi(embed_v0)]
+impl OwnableTwoStepCamelOnlyImpl =
+    OwnableComponent::OwnableTwoStepCamelOnlyImpl<ContractState>;
+----
+
+[#interface-twostep]
+==== Interface
+
+This is the full interface of the two step `Ownable` implementation:
+
+[,javascript]
+----
+trait IOwnableTwoStep {
+    /// Returns the address of the current owner.
+    fn owner() -> ContractAddress;
+
+    /// Returns the address of the pending owner.
+    fn pending_owner() -> ContractAddress;
+
+    /// Finishes the two-step ownership transfer process
+    /// by accepting the ownership.
+    fn accept_ownership();
+
+    /// Starts the two-step ownership transfer process
+    /// by setting the pending owner.
+    fn transfer_ownership(new_owner: ContractAddress);
+
+    /// Renounces the ownership of the contract.
+    fn renounce_ownership();
+}
+----
+
 == Role-Based `AccessControl`
 
 While the simplicity of ownership can be useful for simple systems or quick prototyping, different levels of
@@ -326,8 +369,8 @@ roles (such as during construction). But what if we later want to grant the 'min
 By default, *accounts with a role cannot grant it or revoke it from other accounts*: all having a role does is making
 the xref:api/access.adoc#AccessControlComponent-assert_only_role[`assert_only_role`] check pass. To grant and revoke roles dynamically, you will need help from the role's _admin_.
 
-Every role has an associated admin role, which grants permission to call the 
-xref:api/access.adoc#AccessControlComponent-grant_role[`grant_role`] and 
+Every role has an associated admin role, which grants permission to call the
+xref:api/access.adoc#AccessControlComponent-grant_role[`grant_role`] and
 xref:api/access.adoc#AccessControlComponent-revoke_role[`revoke_role`] functions.
 A role can be granted or revoked by using these if the calling account has the corresponding admin role.
 Multiple roles may have the same admin role to make management easier.
@@ -337,7 +380,7 @@ to also grant and revoke it.
 This mechanism can be used to create complex permissioning structures resembling organizational charts, but it also
 provides an easy way to manage simpler applications. `AccessControl` includes a special role with the role identifier
 of `0`, called `DEFAULT_ADMIN_ROLE`, which acts as the *default admin role for all roles*.
-An account with this role will be able to manage any other role, unless 
+An account with this role will be able to manage any other role, unless
 xref:api/access.adoc#AccessControlComponent-_set_role_admin[`_set_role_admin`] is used to select a new admin role.
 
 Let's take a look at the ERC20 token example, this time taking advantage of the default admin role:

diff --git a/docs/modules/ROOT/pages/api/access.adoc b/docs/modules/ROOT/pages/api/access.adoc
@@ -37,6 +37,14 @@ This module includes the internal `assert_only_owner` to restrict a function to
 * xref:OwnableComponent-owner[`++owner(self)++`]
 * xref:OwnableComponent-transfer_ownership[`++transfer_ownership(self, new_owner)++`]
 * xref:OwnableComponent-renounce_ownership[`++renounce_ownership(self)++`]
+
+.OwnableTwoStepImpl
+
+* xref:OwnableComponent-two-step-owner[`++owner(self)++`]
+* xref:OwnableComponent-two-step-pending_owner[`++pending_owner(self)++`]
+* xref:OwnableComponent-two-step-accept_ownership[`++accept_ownership(self)++`]
+* xref:OwnableComponent-two-step-transfer_ownership[`++transfer_ownership(self, new_owner)++`]
+* xref:OwnableComponent-two-step-renounce_ownership[`++renounce_ownership(self)++`]
 --
 
 [.contract-index]
@@ -46,6 +54,13 @@ This module includes the internal `assert_only_owner` to restrict a function to
 
 * xref:OwnableComponent-transferOwnership[`++transferOwnership(self, newOwner)++`]
 * xref:OwnableComponent-renounceOwnership[`++renounceOwnership(self)++`]
+
+.OwnableTwoStepCamelOnlyImpl
+
+* xref:OwnableComponent-two-step-pendingOwner[`++pendingOwner(self)++`]
+* xref:OwnableComponent-two-step-acceptOwnership[`++acceptOwnership(self)++`]
+* xref:OwnableComponent-two-step-transferOwnership[`++transferOwnership(self, new_owner)++`]
+* xref:OwnableComponent-two-step-renounceOwnership[`++renounceOwnership(self)++`]
 --
 
 [.contract-index]
@@ -55,12 +70,15 @@ This module includes the internal `assert_only_owner` to restrict a function to
 
 * xref:OwnableComponent-initializer[`++initializer(self, owner)++`]
 * xref:OwnableComponent-assert_only_owner[`++assert_only_owner(self)++`]
+* xref:OwnableComponent-_accept_ownership[`++_accept_ownership(self)++`]
+* xref:OwnableComponent-_propose_owner[`++_propose_owner(self, new_owner)++`]
 * xref:OwnableComponent-_transfer_ownership[`++_transfer_ownership(self, new_owner)++`]
 --
 
 [.contract-index]
 .Events
 --
+* xref:OwnableComponent-OwnershipTransferStarted[`++OwnershipTransferStarted(previous_owner, new_owner)++`]
 * xref:OwnableComponent-OwnershipTransferred[`++OwnershipTransferred(previous_owner, new_owner)++`]
 --
 
@@ -70,8 +88,9 @@ This module includes the internal `assert_only_owner` to restrict a function to
 [.contract-item]
 [[OwnableComponent-owner]]
 ==== `[.contract-item-name]#++owner++#++(self: @ContractState) → ContractAddress++` [.item-kind]#external#
-
+// tag::owner[]
 Returns the address of the current owner.
+// end::owner[]
 
 [.contract-item]
 [[OwnableComponent-transfer_ownership]]
@@ -85,12 +104,51 @@ Emits an xref:OwnableComponent-OwnershipTransferred[OwnershipTransferred] event.
 [.contract-item]
 [[OwnableComponent-renounce_ownership]]
 ==== `[.contract-item-name]#++renounce_ownership++#++(ref self: ContractState)++` [.item-kind]#external#
-
+// tag::renounce_ownership[]
 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.
 
 NOTE: Renouncing ownership will leave the contract without an owner,
 thereby removing any functionality that is only available to the owner.
+//end::renounce_ownership[]
+
+[#OwnableComponent-Embeddable-Functions-Two-Step]
+==== Embeddable Functions (Two Step Transfer)
+
+[.contract-item]
+[[OwnableComponent-two-step-owner]]
+==== `[.contract-item-name]#++owner++#++(self: @ContractState) → ContractAddress++` [.item-kind]#external#
+include::./access.adoc[tag=owner]
+
+[.contract-item]
+[[OwnableComponent-two-step-pending_owner]]
+==== `[.contract-item-name]#++pending_owner++#++(self: @ContractState) → ContractAddress++` [.item-kind]#external#
+
+Returns the address of the pending owner.
+
+[.contract-item]
+[[OwnableComponent-two-step-accept_ownership]]
+==== `[.contract-item-name]#++accept_ownership++#++(ref self: ContractState)++` [.item-kind]#external#
+
+Transfers ownership of the contract to the pending owner.
+Can only be called by the pending owner.
+Resets pending owner to zero address.
+
+Emits an xref:OwnableComponent-OwnershipTransferred[OwnershipTransferred] event.
+
+[.contract-item]
+[[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.
+Can only be called by the current owner.
+
+Emits an xref:OwnableComponent-OwnershipTransferStarted[OwnershipTransferStarted] event.
+
+[.contract-item]
+[[OwnableComponent-two-step-renounce_ownership]]
+==== `[.contract-item-name]#++renounce_ownership++#++(ref self: ContractState)++` [.item-kind]#external#
+include::./access.adoc[tag=renounce_ownership]
 
 [#OwnableComponent-camelCase-Support]
 ==== camelCase Support
@@ -107,6 +165,33 @@ See xref:OwnableComponent-transfer_ownership[transfer_ownership].
 
 See xref:OwnableComponent-renounce_ownership[renounce_ownership].
 
+[#OwnableComponent-camelCase-Support-Two-Step]
+==== camelCase Support (Two Step Transfer)
+
+[.contract-item]
+[[OwnableComponent-two-step-pendingOwner]]
+==== `[.contract-item-name]#++pendingOwner++#++(self: @ContractState)++` [.item-kind]#external#
+
+See xref:OwnableComponent-two-step-pending_owner[pending_owner].
+
+[.contract-item]
+[[OwnableComponent-two-step-acceptOwnership]]
+==== `[.contract-item-name]#++acceptOwnership++#++(self: @ContractState)++` [.item-kind]#external#
+
+See xref:OwnableComponent-two-step-accept_ownership[accept_ownership].
+
+[.contract-item]
+[[OwnableComponent-two-step-transferOwnership]]
+==== `[.contract-item-name]#++transferOwnership++#++(self: @ContractState)++` [.item-kind]#external#
+
+See xref:OwnableComponent-two-step-transfer_ownership[transfer_ownership].
+
+[.contract-item]
+[[OwnableComponent-two-step-renounceOwnership]]
+==== `[.contract-item-name]#++renounceOwnership++#++(self: @ContractState)++` [.item-kind]#external#
+
+See xref:OwnableComponent-two-step-renounce_ownership[renounce_ownership].
+
 [#OwnableComponent-Internal-Functions]
 ==== Internal Functions
 
@@ -124,6 +209,28 @@ Emits an xref:OwnableComponent-OwnershipTransferred[OwnershipTransferred] event.
 
 Panics if called by any account other than the owner.
 
+[.contract-item]
+[[OwnableComponent-_accept_ownership]]
+==== `[.contract-item-name]#++_accept_ownership++#++(ref self: ContractState)++` [.item-kind]#internal#
+
+Transfers ownership to the pending owner. Resets pending owner to zero address.
+Calls xref:OwnableComponent-_transfer_ownership[_transfer_ownership].
+
+Internal function without access restriction.
+
+[.contract-item]
+[[OwnableComponent-_transfer_ownership]]
+
+[.contract-item]
+[[OwnableComponent-_propose_owner]]
+==== `[.contract-item-name]#++_propose_owner++#++(ref self: ContractState, new_owner: ContractAddress)++` [.item-kind]#internal#
+
+Sets a new pending owner in a two step transfer.
+
+Internal function without access restriction.
+
+Emits an xref:OwnableComponent-OwnershipTransferStarted[OwnershipTransferStarted] event.
+
 [.contract-item]
 [[OwnableComponent-_transfer_ownership]]
 ==== `[.contract-item-name]#++_transfer_ownership++#++(ref self: ContractState, new_owner: ContractAddress)++` [.item-kind]#internal#
@@ -136,6 +243,12 @@ Emits an xref:OwnableComponent-OwnershipTransferred[OwnershipTransferred] event.
 [#OwnableComponent-Events]
 ==== Events
 
+[.contract-item]
+[[OwnableComponent-OwnershipTransferStarted]]
+==== `[.contract-item-name]#++OwnershipTransferStarted++#++(previous_owner: ContractAddress, new_owner: ContractAddress)++` [.item-kind]#event#
+
+Emitted when the pending owner is updated.
+
 [.contract-item]
 [[OwnableComponent-OwnershipTransferred]]
 ==== `[.contract-item-name]#++OwnershipTransferred++#++(previous_owner: ContractAddress, new_owner: ContractAddress)++` [.item-kind]#event#

diff --git a/src/access/ownable/interface.cairo b/src/access/ownable/interface.cairo
@@ -15,3 +15,20 @@ trait IOwnableCamelOnly<TState> {
     fn transferOwnership(ref self: TState, newOwner: ContractAddress);
     fn renounceOwnership(ref self: TState);
 }
+
+#[starknet::interface]
+trait IOwnableTwoStep<TState> {
+    fn owner(self: @TState) -> ContractAddress;
+    fn pending_owner(self: @TState) -> ContractAddress;
+    fn accept_ownership(ref self: TState);
+    fn transfer_ownership(ref self: TState, new_owner: ContractAddress);
+    fn renounce_ownership(ref self: TState);
+}
+
+#[starknet::interface]
+trait IOwnableTwoStepCamelOnly<TState> {
+    fn pendingOwner(self: @TState) -> ContractAddress;
+    fn acceptOwnership(ref self: TState);
+    fn transferOwnership(ref self: TState, newOwner: ContractAddress);
+    fn renounceOwnership(ref self: TState);
+}