From 2786a783d69aef1f5b8b9f468a4796beedb4f073 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Wed, 16 Oct 2024 16:33:28 -0400
Subject: [PATCH 01/14] ownable2step first iteration

---
 contracts/src/access/mod.rs              |   1 +
 contracts/src/access/ownable.rs          |   7 +
 contracts/src/access/ownable_two_step.rs | 167 +++++++++++++++++++++++
 3 files changed, 175 insertions(+)
 create mode 100644 contracts/src/access/ownable_two_step.rs

diff --git a/contracts/src/access/mod.rs b/contracts/src/access/mod.rs
index fb0063d21..ec5c62350 100644
--- a/contracts/src/access/mod.rs
+++ b/contracts/src/access/mod.rs
@@ -1,3 +1,4 @@
 //! Contracts implementing access control mechanisms.
 pub mod control;
 pub mod ownable;
+pub mod ownable_two_step;
diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index 157457fd1..da5a9d4e8 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -11,6 +11,7 @@
 use alloy_primitives::Address;
 use alloy_sol_types::sol;
 use stylus_sdk::{
+    call::MethodError,
     evm, msg,
     stylus_proc::{public, sol_storage, SolidityError},
 };
@@ -45,6 +46,12 @@ pub enum Error {
     InvalidOwner(OwnableInvalidOwner),
 }
 
+impl MethodError for Error {
+    fn encode(self) -> alloc::vec::Vec<u8> {
+        self.into()
+    }
+}
+
 sol_storage! {
     /// State of an `Ownable` contract.
     pub struct Ownable {
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
new file mode 100644
index 000000000..a3c069d51
--- /dev/null
+++ b/contracts/src/access/ownable_two_step.rs
@@ -0,0 +1,167 @@
+//! Contract module which provides an access control mechanism, where
+//! there is an account (an owner) that can be granted exclusive access to
+//! specific functions.
+//!
+//! This extension of the `Ownable` contract includes a two-step mechanism to
+//! transfer ownership, where the new owner must call
+//! [`Ownable2Step::accept_ownership`] in order to replace the old one. This can
+//! help prevent common mistakes, such as transfers of ownership to
+//! incorrect accounts, or to contracts that are unable to interact with the
+//! permission system.
+//!
+//! The initial owner is set to the address provided by the deployer. This can
+//! later be changed with [`Ownable2Step::transfer_ownership`] and
+//! [`Ownable2Step::accept_ownership`].
+//!
+//! This module is used through inheritance. It will make available all
+//! functions from parent (`Ownable`).
+
+use alloy_primitives::Address;
+use alloy_sol_types::sol;
+use stylus_sdk::{
+    evm, msg,
+    stylus_proc::{public, sol_storage, SolidityError},
+};
+
+use crate::access::ownable::{
+    Error as OwnableError, Ownable, OwnableUnauthorizedAccount,
+};
+
+// use super::ownable::{self, OwnableUnauthorizedAccount};
+
+sol! {
+    /// Emitted when ownership transfer starts.
+    event OwnershipTransferStarted(address indexed previous_owner, address indexed new_owner);
+
+    /// Emitted when ownership gets transferred between accounts.
+    // TODO: Can we remove this and use the one in Ownable directly?
+    event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
+}
+
+// TODO: Since we are not introducing any new error type
+// would we be better removing this and just relying on OwnableError?
+/// An error that occurred in the implementation of an [`Ownable2Step`]
+/// contract.
+#[derive(SolidityError, Debug)]
+pub enum Error {
+    /// Error type from [`Ownable`] contract.
+    Ownable(OwnableError),
+}
+
+sol_storage! {
+    /// State of an `Ownable2Step` contract.
+    pub struct Ownable2Step {
+        /// [`Ownable`] contract.
+        Ownable _ownable;
+        /// Pending owner of the contract.
+        address _pending_owner;
+    }
+}
+
+#[public]
+impl Ownable2Step {
+    /// Returns the address of the current owner.
+    pub fn owner(&self) -> Address {
+        self._ownable.owner()
+    }
+
+    /// Returns the address of the pending owner.
+    pub fn pending_owner(&self) -> Address {
+        self._pending_owner.get()
+    }
+
+    /// Checks if the [`msg::sender`] is set as the owner.
+    ///
+    /// # Errors
+    ///
+    /// If called by any account other than the owner, then the error
+    /// [`OwnableError::UnauthorizedAccount`] is returned.
+    pub fn only_owner(&self) -> Result<(), OwnableError> {
+        self._ownable.only_owner()?;
+        Ok(())
+    }
+
+    /// Initiates the transfer of ownership to a new account (`new_owner`).
+    /// Can only be called by the current owner.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `new_owner` - The next owner of this contract.
+    ///
+    /// # Errors
+    ///
+    /// If called by any account other than the owner, then the error
+    /// [`OwnableError::UnauthorizedAccount`] is returned.
+    pub fn transfer_ownership(
+        &mut self,
+        new_owner: Address,
+    ) -> Result<(), Error> {
+        self._ownable.only_owner()?;
+        self._pending_owner.set(new_owner);
+
+        let current_owner = self.owner();
+        evm::log(OwnershipTransferStarted {
+            previous_owner: current_owner,
+            new_owner,
+        });
+        Ok(())
+    }
+
+    /// Accepts the ownership of the contract. Can only be called by the
+    /// pending owner.
+    ///
+    /// # Errors
+    ///
+    /// If called by any account other than the pending owner, then the error
+    /// [`OwnableError::UnauthorizedAccount`] is returned.
+    pub fn accept_ownership(&mut self) -> Result<(), Error> {
+        let sender = msg::sender();
+        let pending_owner = self.pending_owner();
+        if sender != pending_owner {
+            return Err(OwnableError::UnauthorizedAccount(
+                OwnableUnauthorizedAccount { account: sender },
+            )
+            .into());
+        }
+        self._transfer_ownership(sender);
+        Ok(())
+    }
+
+    /// Leaves the contract without owner. It will not be possible to call
+    /// [`Self::only_owner`] functions. Can only be called by the current owner.
+    ///
+    /// NOTE: Renouncing ownership will leave the contract without an owner,
+    /// thereby disabling any functionality that is only available to the owner.
+    ///
+    /// # Errors
+    ///
+    /// If not called by the owner, then the error
+    /// [`OwnableError::UnauthorizedAccount`]
+    pub fn renounce_ownership(&mut self) -> Result<(), OwnableError> {
+        self._ownable.renounce_ownership()?;
+        Ok(())
+    }
+}
+
+impl Ownable2Step {
+    /// Transfers ownership of the contract to a new account (`new_owner`) and
+    /// sets [`Self::pending_owner`] to zero to avoid situations where the
+    /// transfer has been completed or the current owner renounces, but
+    /// [`Self::pending_owner`] can still accept ownership.
+    /// Internal function without access restriction.
+    ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    /// * `new_owner` - Account that's gonna be the next owner.
+    fn _transfer_ownership(&mut self, new_owner: Address) {
+        self._pending_owner.set(Address::ZERO);
+        self._ownable._transfer_ownership(new_owner);
+    }
+}
+
+#[cfg(all(test, feature = "std"))]
+mod tests {
+    // Test cases for Ownable2Step
+}

From 02b8ba2f11264b0d7c1863296dcee8772291bc86 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Wed, 16 Oct 2024 16:45:00 -0400
Subject: [PATCH 02/14] move only_owner to internal impl

---
 contracts/src/access/ownable.rs          | 34 ++++++++++++------------
 contracts/src/access/ownable_two_step.rs | 11 --------
 2 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index da5a9d4e8..4f0fc2fd5 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -67,23 +67,6 @@ impl Ownable {
         self._owner.get()
     }
 
-    /// Checks if the [`msg::sender`] is set as the owner.
-    ///
-    /// # Errors
-    ///
-    /// If called by any account other than the owner, then the error
-    /// [`Error::UnauthorizedAccount`] is returned.
-    pub fn only_owner(&self) -> Result<(), Error> {
-        let account = msg::sender();
-        if self.owner() != account {
-            return Err(Error::UnauthorizedAccount(
-                OwnableUnauthorizedAccount { account },
-            ));
-        }
-
-        Ok(())
-    }
-
     /// Transfers ownership of the contract to a new account (`new_owner`). Can
     /// only be called by the current owner.
     ///
@@ -131,6 +114,23 @@ impl Ownable {
 }
 
 impl Ownable {
+    /// Checks if the [`msg::sender`] is set as the owner.
+    ///
+    /// # Errors
+    ///
+    /// If called by any account other than the owner, then the error
+    /// [`Error::UnauthorizedAccount`] is returned.
+    pub fn only_owner(&self) -> Result<(), Error> {
+        let account = msg::sender();
+        if self.owner() != account {
+            return Err(Error::UnauthorizedAccount(
+                OwnableUnauthorizedAccount { account },
+            ));
+        }
+
+        Ok(())
+    }
+    
     /// Transfers ownership of the contract to a new account (`new_owner`).
     /// Internal function without access restriction.
     ///
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index a3c069d51..f58dd5589 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -70,17 +70,6 @@ impl Ownable2Step {
         self._pending_owner.get()
     }
 
-    /// Checks if the [`msg::sender`] is set as the owner.
-    ///
-    /// # Errors
-    ///
-    /// If called by any account other than the owner, then the error
-    /// [`OwnableError::UnauthorizedAccount`] is returned.
-    pub fn only_owner(&self) -> Result<(), OwnableError> {
-        self._ownable.only_owner()?;
-        Ok(())
-    }
-
     /// Initiates the transfer of ownership to a new account (`new_owner`).
     /// Can only be called by the current owner.
     ///

From 70bb32e6e2e41b517e32845ffad6672cf0586054 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Wed, 16 Oct 2024 17:11:33 -0400
Subject: [PATCH 03/14] add unit tests

---
 contracts/src/access/ownable_two_step.rs | 128 ++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 3 deletions(-)

diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index f58dd5589..520061bfb 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -128,7 +128,8 @@ impl Ownable2Step {
     /// If not called by the owner, then the error
     /// [`OwnableError::UnauthorizedAccount`]
     pub fn renounce_ownership(&mut self) -> Result<(), OwnableError> {
-        self._ownable.renounce_ownership()?;
+        self._ownable.only_owner()?;
+        self._transfer_ownership(Address::ZERO);
         Ok(())
     }
 }
@@ -150,7 +151,128 @@ impl Ownable2Step {
     }
 }
 
-#[cfg(all(test, feature = "std"))]
+#[cfg(all(test))]
 mod tests {
-    // Test cases for Ownable2Step
+    use alloy_primitives::{address, Address};
+    use stylus_sdk::msg;
+
+    use super::{Error, Ownable2Step, OwnableError};
+
+    const ALICE: Address = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");
+    const BOB: Address = address!("B0B0cB49ec2e96DF5F5fFB081acaE66A2cBBc2e2");
+
+    #[motsu::test]
+    fn reads_owner(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+        let owner = contract.owner();
+        assert_eq!(owner, msg::sender());
+    }
+
+    #[motsu::test]
+    fn reads_pending_owner(contract: Ownable2Step) {
+        contract._pending_owner.set(ALICE);
+        let pending_owner = contract.pending_owner();
+        assert_eq!(pending_owner, ALICE);
+    }
+
+    #[motsu::test]
+    fn initiates_ownership_transfer(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+
+        contract.transfer_ownership(ALICE).expect("should initiate ownership transfer");
+        let pending_owner = contract._pending_owner.get();
+        assert_eq!(pending_owner, ALICE);
+        assert_eq!(contract.owner(), msg::sender());
+    }
+
+    #[motsu::test]
+    fn prevents_non_owners_from_initiating_transfer(contract: Ownable2Step) {
+        contract._ownable._owner.set(ALICE);
+
+        let err = contract.transfer_ownership(BOB).unwrap_err();
+        assert!(matches!(err, Error::Ownable(OwnableError::UnauthorizedAccount(_))));
+    }
+
+    #[motsu::test]
+    fn accepts_ownership(contract: Ownable2Step) {
+        contract._ownable._owner.set(ALICE);
+        contract._pending_owner.set(msg::sender());
+
+        contract.accept_ownership().expect("should accept ownership");
+        assert_eq!(contract.owner(), msg::sender());
+        assert_eq!(contract.pending_owner(), Address::ZERO);
+    }
+
+    #[motsu::test]
+    fn prevents_non_pending_owner_from_accepting(contract: Ownable2Step) {
+        contract._ownable._owner.set(ALICE);
+        contract._pending_owner.set(BOB);
+
+        let err = contract.accept_ownership().unwrap_err();
+        assert!(matches!(err, Error::Ownable(OwnableError::UnauthorizedAccount(_))));
+    }
+
+    #[motsu::test]
+    fn completes_two_step_ownership_transfer(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+
+        contract.transfer_ownership(ALICE).expect("should initiate ownership transfer");
+        assert_eq!(contract.pending_owner(), ALICE);
+
+        // Simulate ALICE accepting ownership, since we cannot set `msg::sender`
+        // in tests yet.
+        contract._pending_owner.set(msg::sender());
+        contract.accept_ownership().expect("should accept ownership");
+
+        assert_eq!(contract.owner(), msg::sender());
+        assert_eq!(contract.pending_owner(), Address::ZERO);
+    }
+
+    #[motsu::test]
+    fn renounces_ownership(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+
+        contract.renounce_ownership().expect("should renounce ownership");
+        assert_eq!(contract.owner(), Address::ZERO);
+    }
+
+    #[motsu::test]
+    fn prevents_non_owners_from_renouncing(contract: Ownable2Step) {
+        contract._ownable._owner.set(ALICE);
+
+        let err = contract.renounce_ownership().unwrap_err();
+        assert!(matches!(err, OwnableError::UnauthorizedAccount(_)));
+    }
+
+    #[motsu::test]
+    fn cancels_transfer_on_renounce(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+        contract._pending_owner.set(ALICE);
+
+        contract.renounce_ownership().expect("should renounce ownership");
+        assert_eq!(contract.owner(), Address::ZERO);
+        assert_eq!(contract.pending_owner(), Address::ZERO);
+    }
+
+    #[motsu::test]
+    fn allows_owner_to_cancel_transfer(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+        contract._pending_owner.set(ALICE);
+
+        contract.transfer_ownership(Address::ZERO).expect("should cancel transfer");
+        assert_eq!(contract.pending_owner(), Address::ZERO);
+        assert_eq!(contract.owner(), msg::sender());
+    }
+
+    #[motsu::test]
+    fn allows_owner_to_overwrite_transfer(contract: Ownable2Step) {
+        contract._ownable._owner.set(msg::sender());
+
+        contract.transfer_ownership(ALICE).expect("should initiate ownership transfer");
+        assert_eq!(contract.pending_owner(), ALICE);
+
+        contract.transfer_ownership(BOB).expect("should overwrite transfer");
+        assert_eq!(contract.pending_owner(), BOB);
+        assert_eq!(contract.owner(), msg::sender());
+    }
 }

From e00fbc34dfca93c9808d1b7fb12cd25a3e3736d4 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Wed, 16 Oct 2024 17:18:15 -0400
Subject: [PATCH 04/14] add std and fmt

---
 contracts/src/access/ownable.rs          |  2 +-
 contracts/src/access/ownable_two_step.rs | 28 ++++++++++++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index 4f0fc2fd5..14dfa2ae0 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -130,7 +130,7 @@ impl Ownable {
 
         Ok(())
     }
-    
+
     /// Transfers ownership of the contract to a new account (`new_owner`).
     /// Internal function without access restriction.
     ///
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 520061bfb..92806fab9 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -151,7 +151,7 @@ impl Ownable2Step {
     }
 }
 
-#[cfg(all(test))]
+#[cfg(all(test, feature = "std"))]
 mod tests {
     use alloy_primitives::{address, Address};
     use stylus_sdk::msg;
@@ -179,7 +179,9 @@ mod tests {
     fn initiates_ownership_transfer(contract: Ownable2Step) {
         contract._ownable._owner.set(msg::sender());
 
-        contract.transfer_ownership(ALICE).expect("should initiate ownership transfer");
+        contract
+            .transfer_ownership(ALICE)
+            .expect("should initiate ownership transfer");
         let pending_owner = contract._pending_owner.get();
         assert_eq!(pending_owner, ALICE);
         assert_eq!(contract.owner(), msg::sender());
@@ -190,7 +192,10 @@ mod tests {
         contract._ownable._owner.set(ALICE);
 
         let err = contract.transfer_ownership(BOB).unwrap_err();
-        assert!(matches!(err, Error::Ownable(OwnableError::UnauthorizedAccount(_))));
+        assert!(matches!(
+            err,
+            Error::Ownable(OwnableError::UnauthorizedAccount(_))
+        ));
     }
 
     #[motsu::test]
@@ -209,14 +214,19 @@ mod tests {
         contract._pending_owner.set(BOB);
 
         let err = contract.accept_ownership().unwrap_err();
-        assert!(matches!(err, Error::Ownable(OwnableError::UnauthorizedAccount(_))));
+        assert!(matches!(
+            err,
+            Error::Ownable(OwnableError::UnauthorizedAccount(_))
+        ));
     }
 
     #[motsu::test]
     fn completes_two_step_ownership_transfer(contract: Ownable2Step) {
         contract._ownable._owner.set(msg::sender());
 
-        contract.transfer_ownership(ALICE).expect("should initiate ownership transfer");
+        contract
+            .transfer_ownership(ALICE)
+            .expect("should initiate ownership transfer");
         assert_eq!(contract.pending_owner(), ALICE);
 
         // Simulate ALICE accepting ownership, since we cannot set `msg::sender`
@@ -259,7 +269,9 @@ mod tests {
         contract._ownable._owner.set(msg::sender());
         contract._pending_owner.set(ALICE);
 
-        contract.transfer_ownership(Address::ZERO).expect("should cancel transfer");
+        contract
+            .transfer_ownership(Address::ZERO)
+            .expect("should cancel transfer");
         assert_eq!(contract.pending_owner(), Address::ZERO);
         assert_eq!(contract.owner(), msg::sender());
     }
@@ -268,7 +280,9 @@ mod tests {
     fn allows_owner_to_overwrite_transfer(contract: Ownable2Step) {
         contract._ownable._owner.set(msg::sender());
 
-        contract.transfer_ownership(ALICE).expect("should initiate ownership transfer");
+        contract
+            .transfer_ownership(ALICE)
+            .expect("should initiate ownership transfer");
         assert_eq!(contract.pending_owner(), ALICE);
 
         contract.transfer_ownership(BOB).expect("should overwrite transfer");

From 2fdc8555e2b5fbf367df3147b72b5a26116c5665 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Thu, 7 Nov 2024 15:06:35 +0700
Subject: [PATCH 05/14] add integration tests

---
 Cargo.lock                                    |  13 +
 Cargo.toml                                    |   4 +-
 contracts/src/access/ownable_two_step.rs      |  28 ++-
 examples/ownable-two-step/Cargo.toml          |  24 ++
 examples/ownable-two-step/src/constructor.sol |  28 +++
 examples/ownable-two-step/src/lib.rs          |  35 +++
 examples/ownable-two-step/tests/abi/mod.rs    |  21 ++
 .../tests/ownable_two_step.rs                 | 235 ++++++++++++++++++
 lib/e2e/README.md                             |   3 +-
 9 files changed, 381 insertions(+), 10 deletions(-)
 create mode 100644 examples/ownable-two-step/Cargo.toml
 create mode 100644 examples/ownable-two-step/src/constructor.sol
 create mode 100644 examples/ownable-two-step/src/lib.rs
 create mode 100644 examples/ownable-two-step/tests/abi/mod.rs
 create mode 100644 examples/ownable-two-step/tests/ownable_two_step.rs

diff --git a/Cargo.lock b/Cargo.lock
index d36efed44..e9cc22203 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2604,6 +2604,19 @@ dependencies = [
  "tokio",
 ]
 
+[[package]]
+name = "ownable-two-step"
+version = "0.0.0"
+dependencies = [
+ "alloy",
+ "alloy-primitives",
+ "e2e",
+ "eyre",
+ "openzeppelin-stylus",
+ "stylus-sdk",
+ "tokio",
+]
+
 [[package]]
 name = "owo-colors"
 version = "4.0.0"
diff --git a/Cargo.toml b/Cargo.toml
index 1043b69e7..bef968dad 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -18,7 +18,8 @@ members = [
   "examples/basic/token",
   "examples/basic/script",
   "examples/ecdsa",
-  "benches",
+  "benches", 
+  "examples/ownable-two-step",
 ]
 default-members = [
   "contracts",
@@ -34,6 +35,7 @@ default-members = [
   "examples/erc721-metadata",
   "examples/merkle-proofs",
   "examples/ownable",
+  "examples/ownable-two-step",
   "examples/access-control",
   "examples/basic/token",
   "examples/ecdsa",
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 92806fab9..2d59a881e 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -27,8 +27,6 @@ use crate::access::ownable::{
     Error as OwnableError, Ownable, OwnableUnauthorizedAccount,
 };
 
-// use super::ownable::{self, OwnableUnauthorizedAccount};
-
 sol! {
     /// Emitted when ownership transfer starts.
     event OwnershipTransferStarted(address indexed previous_owner, address indexed new_owner);
@@ -37,7 +35,6 @@ sol! {
     // TODO: Can we remove this and use the one in Ownable directly?
     event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
 }
-
 // TODO: Since we are not introducing any new error type
 // would we be better removing this and just relying on OwnableError?
 /// An error that occurred in the implementation of an [`Ownable2Step`]
@@ -60,6 +57,9 @@ sol_storage! {
 
 #[public]
 impl Ownable2Step {
+    //TODO: For functions re-exported from Ownable, should we refer
+    // to the docs in Ownable?
+    // Check what we've don in other examples and in Solidity.
     /// Returns the address of the current owner.
     pub fn owner(&self) -> Address {
         self._ownable.owner()
@@ -86,7 +86,7 @@ impl Ownable2Step {
         &mut self,
         new_owner: Address,
     ) -> Result<(), Error> {
-        self._ownable.only_owner()?;
+        self.only_owner()?;
         self._pending_owner.set(new_owner);
 
         let current_owner = self.owner();
@@ -127,8 +127,8 @@ impl Ownable2Step {
     ///
     /// If not called by the owner, then the error
     /// [`OwnableError::UnauthorizedAccount`]
-    pub fn renounce_ownership(&mut self) -> Result<(), OwnableError> {
-        self._ownable.only_owner()?;
+    pub fn renounce_ownership(&mut self) -> Result<(), Error> {
+        self.only_owner()?;
         self._transfer_ownership(Address::ZERO);
         Ok(())
     }
@@ -149,6 +149,17 @@ impl Ownable2Step {
         self._pending_owner.set(Address::ZERO);
         self._ownable._transfer_ownership(new_owner);
     }
+
+    /// Checks if the [`msg::sender`] is set as the owner.
+    ///
+    /// # Errors
+    ///
+    /// If called by any account other than the owner, then the error
+    /// [`Error::UnauthorizedAccount`] is returned.
+    pub fn only_owner(&self) -> Result<(), Error> {
+        self._ownable.only_owner()?;
+        Ok(())
+    }
 }
 
 #[cfg(all(test, feature = "std"))]
@@ -251,7 +262,10 @@ mod tests {
         contract._ownable._owner.set(ALICE);
 
         let err = contract.renounce_ownership().unwrap_err();
-        assert!(matches!(err, OwnableError::UnauthorizedAccount(_)));
+        assert!(matches!(
+            err,
+            Error::Ownable(OwnableError::UnauthorizedAccount(_))
+        ));
     }
 
     #[motsu::test]
diff --git a/examples/ownable-two-step/Cargo.toml b/examples/ownable-two-step/Cargo.toml
new file mode 100644
index 000000000..50f223ce8
--- /dev/null
+++ b/examples/ownable-two-step/Cargo.toml
@@ -0,0 +1,24 @@
+[package]
+name = "ownable-two-step"
+edition.workspace = true
+license.workspace = true
+repository.workspace = true
+publish = false
+version = "0.0.0"
+
+[dependencies]
+openzeppelin-stylus.workspace = true
+alloy-primitives.workspace = true
+stylus-sdk.workspace = true
+
+[dev-dependencies]
+alloy.workspace = true
+e2e.workspace = true
+tokio.workspace = true
+eyre.workspace = true
+
+[lib]
+crate-type = ["lib", "cdylib"]
+
+[features]
+e2e = []
diff --git a/examples/ownable-two-step/src/constructor.sol b/examples/ownable-two-step/src/constructor.sol
new file mode 100644
index 000000000..15ad2af6f
--- /dev/null
+++ b/examples/ownable-two-step/src/constructor.sol
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.21;
+
+contract Ownable2StepExample {
+    mapping(address => uint256) _balances;
+    mapping(address => mapping(address => uint256)) _allowances;
+    uint256 _totalSupply;
+
+    address private _owner;
+    address private _pendingOwner;
+
+    error OwnableInvalidOwner(address owner);
+
+    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
+
+    constructor(address initialOwner) {
+        if (initialOwner == address(0)) {
+            revert OwnableInvalidOwner(address(0));
+        }
+        _transferOwnership(initialOwner);
+    }
+
+    function _transferOwnership(address newOwner) internal virtual {
+        address oldOwner = _owner;
+        _owner = newOwner;
+        emit OwnershipTransferred(oldOwner, newOwner);
+    }
+}
diff --git a/examples/ownable-two-step/src/lib.rs b/examples/ownable-two-step/src/lib.rs
new file mode 100644
index 000000000..7b0cb0f0f
--- /dev/null
+++ b/examples/ownable-two-step/src/lib.rs
@@ -0,0 +1,35 @@
+#![cfg_attr(not(test), no_main)]
+extern crate alloc;
+
+use alloc::vec::Vec;
+
+use alloy_primitives::{Address, U256};
+use openzeppelin_stylus::{
+    access::ownable_two_step::Ownable2Step,
+    token::erc20::{Erc20, IErc20},
+};
+use stylus_sdk::prelude::{entrypoint, public, sol_storage};
+
+sol_storage! {
+    #[entrypoint]
+    struct Ownable2StepExample {
+        #[borrow]
+        Erc20 erc20;
+        #[borrow]
+        Ownable2Step ownable;
+    }
+}
+
+#[public]
+#[inherit(Erc20, Ownable2Step)]
+impl Ownable2StepExample {
+    pub fn transfer(
+        &mut self,
+        to: Address,
+        value: U256,
+    ) -> Result<(), Vec<u8>> {
+        self.ownable.only_owner()?;
+        self.erc20.transfer(to, value)?;
+        Ok(())
+    }
+}
diff --git a/examples/ownable-two-step/tests/abi/mod.rs b/examples/ownable-two-step/tests/abi/mod.rs
new file mode 100644
index 000000000..ad958ef02
--- /dev/null
+++ b/examples/ownable-two-step/tests/abi/mod.rs
@@ -0,0 +1,21 @@
+#![allow(dead_code)]
+use alloy::sol;
+
+sol!(
+    #[sol(rpc)]
+   contract Ownable2Step {
+        error OwnableUnauthorizedAccount(address account);
+        error OwnableInvalidOwner(address owner);
+
+        #[derive(Debug, PartialEq)]
+        event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
+        #[derive(Debug, PartialEq)]
+        event OwnershipTransferStarted(address indexed previousOwner, address indexed newOwner);
+
+        function owner() public view virtual returns (address owner);
+        function pendingOwner() public view returns (address pendingOwner);
+        function renounceOwnership() public virtual onlyOwner;
+        function transferOwnership(address newOwner) public virtual;
+        function acceptOwnership() public virtual;
+    }
+);
diff --git a/examples/ownable-two-step/tests/ownable_two_step.rs b/examples/ownable-two-step/tests/ownable_two_step.rs
new file mode 100644
index 000000000..8795b0585
--- /dev/null
+++ b/examples/ownable-two-step/tests/ownable_two_step.rs
@@ -0,0 +1,235 @@
+#![cfg(feature = "e2e")]
+
+use abi::{
+    Ownable2Step,
+    Ownable2Step::{OwnershipTransferStarted, OwnershipTransferred},
+};
+use alloy::{primitives::Address, sol};
+use e2e::{receipt, send, Account, EventExt, ReceiptExt, Revert};
+use eyre::Result;
+
+use crate::Ownable2StepExample::constructorCall;
+
+mod abi;
+
+sol!("src/constructor.sol");
+
+fn ctr(owner: Address) -> constructorCall {
+    constructorCall { initialOwner: owner }
+}
+
+// ============================================================================
+// Integration Tests: Ownable2Step
+// ============================================================================
+
+#[e2e::test]
+async fn constructs(alice: Account) -> Result<()> {
+    let alice_addr = alice.address();
+    let receipt =
+        alice.as_deployer().with_constructor(ctr(alice_addr)).deploy().await?;
+    let contract = Ownable2Step::new(receipt.address()?, &alice.wallet);
+
+    assert!(receipt.emits(OwnershipTransferred {
+        previousOwner: Address::ZERO,
+        newOwner: alice_addr,
+    }));
+
+    let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
+    assert_eq!(owner, alice_addr);
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, Address::ZERO);
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn construct_reverts_when_zero_owner(alice: Account) -> Result<()> {
+    let err = alice
+        .as_deployer()
+        .with_constructor(ctr(Address::ZERO))
+        .deploy()
+        .await
+        .expect_err("should not deploy due to `OwnableInvalidOwner`");
+
+    assert!(err.reverted_with(Ownable2Step::OwnableInvalidOwner {
+        owner: Address::ZERO
+    }));
+    Ok(())
+}
+
+#[e2e::test]
+async fn transfer_ownership_initiates_transfer(
+    alice: Account,
+    bob: Account,
+) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+
+    let receipt = receipt!(contract.transferOwnership(bob_addr))?;
+    assert!(receipt.emits(OwnershipTransferStarted {
+        previousOwner: alice_addr,
+        newOwner: bob_addr,
+    }));
+
+    // Current owner is still Alice
+    let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
+    assert_eq!(owner, alice_addr);
+
+    // Pending owner is Bob
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, bob_addr);
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn transfer_ownership_reverts_when_not_owner(
+    alice: Account,
+    bob: Account,
+) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(bob_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+
+    let err = send!(contract.transferOwnership(bob_addr))
+        .expect_err("should not transfer when not owned");
+    err.reverted_with(Ownable2Step::OwnableUnauthorizedAccount {
+        account: alice_addr,
+    });
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn accept_ownership(alice: Account, bob: Account) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+    receipt!(contract.transferOwnership(bob_addr))?;
+
+    // Connect as Bob and accept ownership
+    let contract = Ownable2Step::new(contract_addr, &bob.wallet);
+    let receipt = receipt!(contract.acceptOwnership())?;
+    assert!(receipt.emits(OwnershipTransferred {
+        previousOwner: alice_addr,
+        newOwner: bob_addr,
+    }));
+
+    let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
+    assert_eq!(owner, bob_addr);
+
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, Address::ZERO);
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn accept_ownership_reverts_when_not_pending_owner(
+    alice: Account,
+    bob: Account,
+    charlie: Account,
+) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+    let charlie_addr = charlie.address();
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+    receipt!(contract.transferOwnership(bob_addr))?;
+
+    // Connect as Charlie and attempt to accept ownership
+    let contract = Ownable2Step::new(contract_addr, &charlie.wallet);
+    let err = send!(contract.acceptOwnership())
+        .expect_err("should not accept when not pending owner");
+    err.reverted_with(Ownable2Step::OwnableUnauthorizedAccount {
+        account: charlie_addr,
+    });
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn renounce_ownership(alice: Account) -> Result<()> {
+    let alice_addr = alice.address();
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+
+    let receipt = receipt!(contract.renounceOwnership())?;
+    assert!(receipt.emits(OwnershipTransferred {
+        previousOwner: alice_addr,
+        newOwner: Address::ZERO,
+    }));
+
+    let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
+    assert_eq!(owner, Address::ZERO);
+
+    // Pending owner is set to zero
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, Address::ZERO);
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn renounce_ownership_reverts_when_not_owner(
+    alice: Account,
+    bob: Account,
+) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &bob.wallet);
+
+    let err = send!(contract.renounceOwnership())
+        .expect_err("should prevent non-owner from renouncing");
+    err.reverted_with(Ownable2Step::OwnableUnauthorizedAccount {
+        account: bob_addr,
+    });
+
+    let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
+    assert_eq!(owner, alice_addr);
+
+    Ok(())
+}
diff --git a/lib/e2e/README.md b/lib/e2e/README.md
index dbd5c850f..79a222399 100644
--- a/lib/e2e/README.md
+++ b/lib/e2e/README.md
@@ -31,7 +31,7 @@ async fn accounts_are_funded(alice: Account) -> eyre::Result<()> {
 }
 ```
 
-A `Account` is a thin wrapper over a [`PrivateKeySigner`] and an `alloy` provider with a
+An `Account` is a thin wrapper over a [`PrivateKeySigner`] and an `alloy` provider with a
 [`WalletFiller`]. Both of them are connected to the RPC endpoint defined by the
 `RPC_URL` environment variable. This means that a `Account` is the main proxy
 between the RPC and the test code.
@@ -47,7 +47,6 @@ async fn foo(alice: Account, bob: Account) -> eyre::Result<()> {
 }
 ```
 
-[`LocalWallet`]: https://github.com/alloy-rs/alloy/blob/8aa54828c025a99bbe7e2d4fc9768605d172cc6d/crates/signer-local/src/lib.rs#L37
 
 [`WalletFiller`]: https://github.com/alloy-rs/alloy/blob/8aa54828c025a99bbe7e2d4fc9768605d172cc6d/crates/provider/src/fillers/wallet.rs#L30
 

From c08e2e16333704742a6ba9d70962a88737224887 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Thu, 7 Nov 2024 15:28:46 +0700
Subject: [PATCH 06/14] fix docs, clean contract and use nextest in CI

---
 .github/workflows/test.yml                    | 12 +++++-----
 contracts/src/access/ownable_two_step.rs      | 22 +++----------------
 examples/ownable-two-step/src/lib.rs          |  2 +-
 .../tests/ownable_two_step.rs                 |  4 +++-
 4 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 5a1de1079..e51aaa4cf 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -11,7 +11,7 @@ permissions:
   contents: read
 on:
   push:
-    branches: [ main ]
+    branches: [main]
     paths-ignore:
       - "**.md"
       - "**.adoc"
@@ -32,7 +32,7 @@ jobs:
       matrix:
         # Run on stable and beta to ensure that tests won't break on the next
         # version of the rust toolchain.
-        toolchain: [ stable, beta ]
+        toolchain: [stable, beta]
     steps:
       - uses: actions/checkout@v4
         with:
@@ -52,11 +52,11 @@ jobs:
 
         # https://twitter.com/jonhoo/status/1571290371124260865
       - name: Run unit tests
-        run: cargo test --locked --features std --all-targets
+        run: cargo nextest run --locked --features std --all-targets
 
         # https://github.com/rust-lang/cargo/issues/6669
       - name: Run doc tests
-        run: cargo test --locked --features std --doc
+        run: cargo nextest run --locked --features std --doc
   os-check:
     # Run cargo test on MacOS and Windows.
     runs-on: ${{ matrix.os }}
@@ -64,7 +64,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ macos-latest ]
+        os: [macos-latest]
         # Windows fails because of `stylus-proc`.
         # os: [macos-latest, windows-latest]
     steps:
@@ -83,7 +83,7 @@ jobs:
         run: cargo generate-lockfile
 
       - name: Run unit tests
-        run: cargo test --locked --features std --all-targets
+        run: cargo nextest run --locked --features std --all-targets
   coverage:
     # Use llvm-cov to build and collect coverage and outputs in a format that
     # is compatible with codecov.io.
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 2d59a881e..0eaddc3fa 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -32,11 +32,9 @@ sol! {
     event OwnershipTransferStarted(address indexed previous_owner, address indexed new_owner);
 
     /// Emitted when ownership gets transferred between accounts.
-    // TODO: Can we remove this and use the one in Ownable directly?
     event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
 }
-// TODO: Since we are not introducing any new error type
-// would we be better removing this and just relying on OwnableError?
+
 /// An error that occurred in the implementation of an [`Ownable2Step`]
 /// contract.
 #[derive(SolidityError, Debug)]
@@ -57,9 +55,6 @@ sol_storage! {
 
 #[public]
 impl Ownable2Step {
-    //TODO: For functions re-exported from Ownable, should we refer
-    // to the docs in Ownable?
-    // Check what we've don in other examples and in Solidity.
     /// Returns the address of the current owner.
     pub fn owner(&self) -> Address {
         self._ownable.owner()
@@ -86,7 +81,7 @@ impl Ownable2Step {
         &mut self,
         new_owner: Address,
     ) -> Result<(), Error> {
-        self.only_owner()?;
+        self._ownable.only_owner()?;
         self._pending_owner.set(new_owner);
 
         let current_owner = self.owner();
@@ -128,7 +123,7 @@ impl Ownable2Step {
     /// If not called by the owner, then the error
     /// [`OwnableError::UnauthorizedAccount`]
     pub fn renounce_ownership(&mut self) -> Result<(), Error> {
-        self.only_owner()?;
+        self._ownable.only_owner()?;
         self._transfer_ownership(Address::ZERO);
         Ok(())
     }
@@ -149,17 +144,6 @@ impl Ownable2Step {
         self._pending_owner.set(Address::ZERO);
         self._ownable._transfer_ownership(new_owner);
     }
-
-    /// Checks if the [`msg::sender`] is set as the owner.
-    ///
-    /// # Errors
-    ///
-    /// If called by any account other than the owner, then the error
-    /// [`Error::UnauthorizedAccount`] is returned.
-    pub fn only_owner(&self) -> Result<(), Error> {
-        self._ownable.only_owner()?;
-        Ok(())
-    }
 }
 
 #[cfg(all(test, feature = "std"))]
diff --git a/examples/ownable-two-step/src/lib.rs b/examples/ownable-two-step/src/lib.rs
index 7b0cb0f0f..11d9583ea 100644
--- a/examples/ownable-two-step/src/lib.rs
+++ b/examples/ownable-two-step/src/lib.rs
@@ -28,7 +28,7 @@ impl Ownable2StepExample {
         to: Address,
         value: U256,
     ) -> Result<(), Vec<u8>> {
-        self.ownable.only_owner()?;
+        self.ownable._ownable.only_owner()?;
         self.erc20.transfer(to, value)?;
         Ok(())
     }
diff --git a/examples/ownable-two-step/tests/ownable_two_step.rs b/examples/ownable-two-step/tests/ownable_two_step.rs
index 8795b0585..28be08ea3 100644
--- a/examples/ownable-two-step/tests/ownable_two_step.rs
+++ b/examples/ownable-two-step/tests/ownable_two_step.rs
@@ -44,7 +44,9 @@ async fn constructs(alice: Account) -> Result<()> {
 }
 
 #[e2e::test]
-async fn construct_reverts_when_zero_owner(alice: Account) -> Result<()> {
+async fn construct_reverts_when_owner_is_zero_address(
+    alice: Account,
+) -> Result<()> {
     let err = alice
         .as_deployer()
         .with_constructor(ctr(Address::ZERO))

From 6462e934053b58313c5b9ea489305e09b2a14f92 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Thu, 7 Nov 2024 15:42:28 +0700
Subject: [PATCH 07/14] install nextest

---
 .github/workflows/test.yml | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 5524e0e7f..63db2e44d 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -44,6 +44,11 @@ jobs:
           toolchain: ${{ matrix.toolchain }}
           rustflags: ""
 
+      - name: "Install nextest"
+        uses: taiki-e/install-action@v2
+        with:
+          tool: cargo-nextest
+
       - name: Cargo generate-lockfile
         # Enable this ci template to run regardless of whether the lockfile is
         # checked in or not.
@@ -56,7 +61,7 @@ jobs:
 
         # https://github.com/rust-lang/cargo/issues/6669
       - name: Run doc tests
-        run: cargo nextest run --locked --features std --doc
+        run: cargo test --locked --features std --doc
   os-check:
     # Run cargo test on MacOS and Windows.
     runs-on: ${{ matrix.os }}
@@ -78,6 +83,11 @@ jobs:
           toolchain: stable
           rustflags: ""
 
+      - name: "Install nextest"
+        uses: taiki-e/install-action@v2
+        with:
+          tool: cargo-nextest
+
       - name: Cargo generate-lockfile
         if: hashFiles('Cargo.lock') == ''
         run: cargo generate-lockfile

From b84d40733b545ecad759bbe62a65efc3fa747311 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Thu, 7 Nov 2024 15:52:23 +0700
Subject: [PATCH 08/14] fix docs

---
 contracts/src/access/ownable_two_step.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 0eaddc3fa..43c3b386c 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -113,7 +113,8 @@ impl Ownable2Step {
     }
 
     /// Leaves the contract without owner. It will not be possible to call
-    /// [`Self::only_owner`] functions. Can only be called by the current owner.
+    /// [`Ownable::only_owner`] functions. Can only be called by the current
+    /// owner.
     ///
     /// NOTE: Renouncing ownership will leave the contract without an owner,
     /// thereby disabling any functionality that is only available to the owner.

From ab0f4afb65c926a6fcc6dd76adaa5fd6aca3223b Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Fri, 8 Nov 2024 06:05:22 +0700
Subject: [PATCH 09/14] improve docs and use workspace package

---
 Cargo.lock                               |  6 +--
 contracts/src/access/ownable.rs          | 23 +++++++++++
 contracts/src/access/ownable_two_step.rs | 50 +++++++++++++++++++++---
 examples/basic/script/Cargo.toml         |  2 +-
 examples/ownable-two-step/Cargo.toml     |  2 +-
 examples/ownable/Cargo.toml              |  2 +-
 6 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 75d2f1904..ec00212a8 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -801,7 +801,7 @@ dependencies = [
 
 [[package]]
 name = "basic-example-script"
-version = "0.0.0"
+version = "0.1.1"
 dependencies = [
  "alloy",
  "alloy-primitives",
@@ -2644,7 +2644,7 @@ dependencies = [
 
 [[package]]
 name = "ownable-example"
-version = "0.0.0"
+version = "0.1.1"
 dependencies = [
  "alloy",
  "alloy-primitives",
@@ -2657,7 +2657,7 @@ dependencies = [
 
 [[package]]
 name = "ownable-two-step"
-version = "0.0.0"
+version = "0.1.1"
 dependencies = [
  "alloy",
  "alloy-primitives",
diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index 14dfa2ae0..f265d3bc3 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -18,6 +18,9 @@ use stylus_sdk::{
 
 sol! {
     /// Emitted when ownership gets transferred between accounts.
+    ///
+    /// * `previous_owner` - Address of the previous owner.
+    /// * `new_owner` - Address of the new owner.
     #[allow(missing_docs)]
     event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
 }
@@ -63,6 +66,10 @@ sol_storage! {
 #[public]
 impl Ownable {
     /// Returns the address of the current owner.
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
     pub fn owner(&self) -> Address {
         self._owner.get()
     }
@@ -79,6 +86,10 @@ impl Ownable {
     ///
     /// If `new_owner` is the zero address, then the error
     /// [`OwnableInvalidOwner`] is returned.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
     pub fn transfer_ownership(
         &mut self,
         new_owner: Address,
@@ -102,10 +113,18 @@ impl Ownable {
     /// NOTE: Renouncing ownership will leave the contract without an owner,
     /// thereby disabling any functionality that is only available to the owner.
     ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    ///
     /// # Errors
     ///
     /// If not called by the owner, then the error
     /// [`Error::UnauthorizedAccount`] is returned.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
     pub fn renounce_ownership(&mut self) -> Result<(), Error> {
         self.only_owner()?;
         self._transfer_ownership(Address::ZERO);
@@ -138,6 +157,10 @@ impl Ownable {
     ///
     /// * `&mut self` - Write access to the contract's state.
     /// * `new_owner` - Account that's gonna be the next owner.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
     pub fn _transfer_ownership(&mut self, new_owner: Address) {
         let previous_owner = self._owner.get();
         self._owner.set(new_owner);
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 43c3b386c..61b95a3e2 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -13,8 +13,8 @@
 //! later be changed with [`Ownable2Step::transfer_ownership`] and
 //! [`Ownable2Step::accept_ownership`].
 //!
-//! This module is used through inheritance. It will make available all
-//! functions from parent (`Ownable`).
+//! This module uses [`Ownable`] as a member, and makes all its public functions
+//! available.
 
 use alloy_primitives::Address;
 use alloy_sol_types::sol;
@@ -29,9 +29,16 @@ use crate::access::ownable::{
 
 sol! {
     /// Emitted when ownership transfer starts.
+    ///
+    /// * `previous_owner` - Address of the previous owner.
+    /// * `new_owner` - Address of the new owner, to which the ownership will be
+    ///   transferred.
     event OwnershipTransferStarted(address indexed previous_owner, address indexed new_owner);
 
     /// Emitted when ownership gets transferred between accounts.
+    ///
+    /// * `previous_owner` - Address of the previous owner.
+    /// * `new_owner` - Address of the new owner.
     event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
 }
 
@@ -56,17 +63,29 @@ sol_storage! {
 #[public]
 impl Ownable2Step {
     /// Returns the address of the current owner.
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
     pub fn owner(&self) -> Address {
         self._ownable.owner()
     }
 
     /// Returns the address of the pending owner.
+    ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
     pub fn pending_owner(&self) -> Address {
         self._pending_owner.get()
     }
 
-    /// Initiates the transfer of ownership to a new account (`new_owner`).
-    /// Can only be called by the current owner.
+    /// Starts the ownership transfer of the contract to a new account. Replaces
+    /// the pending transfer if there is one. Can only be called by the
+    /// current owner.
+    ///
+    /// Setting `newOwner` to the zero address is allowed; this can be used to
+    /// cancel an initiated ownership transfer.
     ///
     /// # Arguments
     ///
@@ -77,6 +96,10 @@ impl Ownable2Step {
     ///
     /// If called by any account other than the owner, then the error
     /// [`OwnableError::UnauthorizedAccount`] is returned.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferStarted`] event.
     pub fn transfer_ownership(
         &mut self,
         new_owner: Address,
@@ -95,10 +118,18 @@ impl Ownable2Step {
     /// Accepts the ownership of the contract. Can only be called by the
     /// pending owner.
     ///
+    /// # Arguments
+    ///
+    /// * `&mut self` - Write access to the contract's state.
+    ///
     /// # Errors
     ///
     /// If called by any account other than the pending owner, then the error
     /// [`OwnableError::UnauthorizedAccount`] is returned.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
     pub fn accept_ownership(&mut self) -> Result<(), Error> {
         let sender = msg::sender();
         let pending_owner = self.pending_owner();
@@ -119,10 +150,15 @@ impl Ownable2Step {
     /// NOTE: Renouncing ownership will leave the contract without an owner,
     /// thereby disabling any functionality that is only available to the owner.
     ///
+    /// # Arguments
     /// # Errors
     ///
     /// If not called by the owner, then the error
-    /// [`OwnableError::UnauthorizedAccount`]
+    /// [`OwnableError::UnauthorizedAccount`] is returned.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
     pub fn renounce_ownership(&mut self) -> Result<(), Error> {
         self._ownable.only_owner()?;
         self._transfer_ownership(Address::ZERO);
@@ -141,6 +177,10 @@ impl Ownable2Step {
     ///
     /// * `&mut self` - Write access to the contract's state.
     /// * `new_owner` - Account that's gonna be the next owner.
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
     fn _transfer_ownership(&mut self, new_owner: Address) {
         self._pending_owner.set(Address::ZERO);
         self._ownable._transfer_ownership(new_owner);
diff --git a/examples/basic/script/Cargo.toml b/examples/basic/script/Cargo.toml
index 55a9d05d3..7b81d8e91 100644
--- a/examples/basic/script/Cargo.toml
+++ b/examples/basic/script/Cargo.toml
@@ -4,7 +4,7 @@ edition.workspace = true
 license.workspace = true
 repository.workspace = true
 publish = false
-version = "0.0.0"
+version.workspace = true
 
 [dependencies]
 basic-example = { path = "../token" }
diff --git a/examples/ownable-two-step/Cargo.toml b/examples/ownable-two-step/Cargo.toml
index 50f223ce8..172c38d8b 100644
--- a/examples/ownable-two-step/Cargo.toml
+++ b/examples/ownable-two-step/Cargo.toml
@@ -4,7 +4,7 @@ edition.workspace = true
 license.workspace = true
 repository.workspace = true
 publish = false
-version = "0.0.0"
+version.workspace = true
 
 [dependencies]
 openzeppelin-stylus.workspace = true
diff --git a/examples/ownable/Cargo.toml b/examples/ownable/Cargo.toml
index 3234d5fd0..03409625e 100644
--- a/examples/ownable/Cargo.toml
+++ b/examples/ownable/Cargo.toml
@@ -4,7 +4,7 @@ edition.workspace = true
 license.workspace = true
 repository.workspace = true
 publish = false
-version = "0.0.0"
+version.workspace = true
 
 [dependencies]
 openzeppelin-stylus.workspace = true

From 6358af5948d406ccc0da88eeb1c0382747942045 Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Fri, 8 Nov 2024 06:26:15 +0700
Subject: [PATCH 10/14] add IOwnable

---
 contracts/src/access/ownable.rs          | 60 +++++++++++++++---------
 contracts/src/access/ownable_two_step.rs |  2 +-
 2 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index f265d3bc3..e30ae5210 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -10,6 +10,7 @@
 //! to the owner.
 use alloy_primitives::Address;
 use alloy_sol_types::sol;
+use openzeppelin_stylus_proc::interface_id;
 use stylus_sdk::{
     call::MethodError,
     evm, msg,
@@ -63,16 +64,18 @@ sol_storage! {
     }
 }
 
-#[public]
-impl Ownable {
+/// Interface for IOwnable.
+#[interface_id]
+pub trait IOwnable {
+    /// The error type associated to this Ownable trait implementation.
+    type Error: Into<alloc::vec::Vec<u8>>;
+
     /// Returns the address of the current owner.
     ///
     /// # Arguments
     ///
     /// * `&self` - Read access to the contract's state.
-    pub fn owner(&self) -> Address {
-        self._owner.get()
-    }
+    fn owner(&self) -> Address;
 
     /// Transfers ownership of the contract to a new account (`new_owner`). Can
     /// only be called by the current owner.
@@ -90,25 +93,11 @@ impl Ownable {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferred`] event.
-    pub fn transfer_ownership(
-        &mut self,
-        new_owner: Address,
-    ) -> Result<(), Error> {
-        self.only_owner()?;
-
-        if new_owner == Address::ZERO {
-            return Err(Error::InvalidOwner(OwnableInvalidOwner {
-                owner: Address::ZERO,
-            }));
-        }
-
-        self._transfer_ownership(new_owner);
-
-        Ok(())
-    }
+    fn transfer_ownership(&mut self, new_owner: Address) -> Result<(), Error>;
 
     /// Leaves the contract without owner. It will not be possible to call
-    /// [`Self::only_owner`] functions. Can only be called by the current owner.
+    /// functions that require `only_owner`. Can only be called by the current
+    /// owner.
     ///
     /// NOTE: Renouncing ownership will leave the contract without an owner,
     /// thereby disabling any functionality that is only available to the owner.
@@ -125,7 +114,32 @@ impl Ownable {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferred`] event.
-    pub fn renounce_ownership(&mut self) -> Result<(), Error> {
+    fn renounce_ownership(&mut self) -> Result<(), Error>;
+}
+
+#[public]
+impl IOwnable for Ownable {
+    type Error = Error;
+
+    fn owner(&self) -> Address {
+        self._owner.get()
+    }
+
+    fn transfer_ownership(&mut self, new_owner: Address) -> Result<(), Error> {
+        self.only_owner()?;
+
+        if new_owner == Address::ZERO {
+            return Err(Error::InvalidOwner(OwnableInvalidOwner {
+                owner: Address::ZERO,
+            }));
+        }
+
+        self._transfer_ownership(new_owner);
+
+        Ok(())
+    }
+
+    fn renounce_ownership(&mut self) -> Result<(), Error> {
         self.only_owner()?;
         self._transfer_ownership(Address::ZERO);
         Ok(())
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 61b95a3e2..3efedf52f 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -24,7 +24,7 @@ use stylus_sdk::{
 };
 
 use crate::access::ownable::{
-    Error as OwnableError, Ownable, OwnableUnauthorizedAccount,
+    Error as OwnableError, IOwnable, Ownable, OwnableUnauthorizedAccount,
 };
 
 sol! {

From cea83569c495b04feba8a8032cdf50b7cbb73e5d Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Fri, 8 Nov 2024 06:31:25 +0700
Subject: [PATCH 11/14] fix tests

---
 contracts/src/access/ownable.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index e30ae5210..650d831ea 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -187,7 +187,7 @@ mod tests {
     use alloy_primitives::{address, Address};
     use stylus_sdk::msg;
 
-    use super::{Error, Ownable};
+    use super::{Error, IOwnable, Ownable};
 
     const ALICE: Address = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");
 

From cdac275502db3a810816ec886b41bccd93f70efc Mon Sep 17 00:00:00 2001
From: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
Date: Fri, 8 Nov 2024 06:39:40 +0700
Subject: [PATCH 12/14] changelog

---
 .github/pull_request_template.md | 1 +
 CHANGELOG.md                     | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md
index 385188f3a..3cd298a57 100644
--- a/.github/pull_request_template.md
+++ b/.github/pull_request_template.md
@@ -19,3 +19,4 @@ Some of the items may not apply.
 
 - [ ] Tests
 - [ ] Documentation
+- [ ] Changelog
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94cc5d580..64c3524a3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ERC-1155 Multi Token Standard. #275
 - `SafeErc20` Utility. #289
 - Finite Fields arithmetics. #376
+- `Ownable2Step` contract. #352
+- `IOwnable` trait. #352
+
+### Changed(breaking)
+
+- Removed `only_owner` from the public interface of `Ownable`. #352
 
 ### Changed
 

From 4742b71e1be97d952b9c09281702cb1f5c592eef Mon Sep 17 00:00:00 2001
From: Daniel Bigos <daniel.bigos@openzeppelin.com>
Date: Fri, 8 Nov 2024 16:19:56 +0100
Subject: [PATCH 13/14] ref: apply CR changes for Ownable2Step (#399)

Co-authored-by: Gustavo Gonzalez <gustavo.gonzalez@openzeppelin.com>
---
 contracts/src/access/ownable.rs               |  30 +++--
 contracts/src/access/ownable_two_step.rs      | 124 +++++++++++-------
 .../tests/ownable_two_step.rs                 |  94 +++++++++++++
 3 files changed, 191 insertions(+), 57 deletions(-)

diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs
index 650d831ea..50692973f 100644
--- a/contracts/src/access/ownable.rs
+++ b/contracts/src/access/ownable.rs
@@ -64,10 +64,10 @@ sol_storage! {
     }
 }
 
-/// Interface for IOwnable.
+/// Interface for an [`Ownable`] contract.
 #[interface_id]
 pub trait IOwnable {
-    /// The error type associated to this Ownable trait implementation.
+    /// The error type associated to the trait implementation.
     type Error: Into<alloc::vec::Vec<u8>>;
 
     /// Returns the address of the current owner.
@@ -77,8 +77,8 @@ pub trait IOwnable {
     /// * `&self` - Read access to the contract's state.
     fn owner(&self) -> Address;
 
-    /// Transfers ownership of the contract to a new account (`new_owner`). Can
-    /// only be called by the current owner.
+    /// Transfers ownership of the contract to a new account (`new_owner`).
+    /// Can only be called by the current owner.
     ///
     /// # Arguments
     ///
@@ -93,7 +93,10 @@ pub trait IOwnable {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferred`] event.
-    fn transfer_ownership(&mut self, new_owner: Address) -> Result<(), Error>;
+    fn transfer_ownership(
+        &mut self,
+        new_owner: Address,
+    ) -> Result<(), Self::Error>;
 
     /// Leaves the contract without owner. It will not be possible to call
     /// functions that require `only_owner`. Can only be called by the current
@@ -114,7 +117,7 @@ pub trait IOwnable {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferred`] event.
-    fn renounce_ownership(&mut self) -> Result<(), Error>;
+    fn renounce_ownership(&mut self) -> Result<(), Self::Error>;
 }
 
 #[public]
@@ -125,10 +128,13 @@ impl IOwnable for Ownable {
         self._owner.get()
     }
 
-    fn transfer_ownership(&mut self, new_owner: Address) -> Result<(), Error> {
+    fn transfer_ownership(
+        &mut self,
+        new_owner: Address,
+    ) -> Result<(), Self::Error> {
         self.only_owner()?;
 
-        if new_owner == Address::ZERO {
+        if new_owner.is_zero() {
             return Err(Error::InvalidOwner(OwnableInvalidOwner {
                 owner: Address::ZERO,
             }));
@@ -139,7 +145,7 @@ impl IOwnable for Ownable {
         Ok(())
     }
 
-    fn renounce_ownership(&mut self) -> Result<(), Error> {
+    fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
         self.only_owner()?;
         self._transfer_ownership(Address::ZERO);
         Ok(())
@@ -149,6 +155,10 @@ impl IOwnable for Ownable {
 impl Ownable {
     /// Checks if the [`msg::sender`] is set as the owner.
     ///
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
+    ///
     /// # Errors
     ///
     /// If called by any account other than the owner, then the error
@@ -170,7 +180,7 @@ impl Ownable {
     /// # Arguments
     ///
     /// * `&mut self` - Write access to the contract's state.
-    /// * `new_owner` - Account that's gonna be the next owner.
+    /// * `new_owner` - Account that is going to be the next owner.
     ///
     /// # Events
     ///
diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 3efedf52f..9e189ce81 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -31,15 +31,21 @@ sol! {
     /// Emitted when ownership transfer starts.
     ///
     /// * `previous_owner` - Address of the previous owner.
-    /// * `new_owner` - Address of the new owner, to which the ownership will be
-    ///   transferred.
-    event OwnershipTransferStarted(address indexed previous_owner, address indexed new_owner);
+    /// * `new_owner` - Address of the new owner, to which the ownership
+    ///   will be transferred.
+    event OwnershipTransferStarted(
+        address indexed previous_owner,
+        address indexed new_owner
+    );
 
     /// Emitted when ownership gets transferred between accounts.
     ///
     /// * `previous_owner` - Address of the previous owner.
     /// * `new_owner` - Address of the new owner.
-    event OwnershipTransferred(address indexed previous_owner, address indexed new_owner);
+    event OwnershipTransferred(
+        address indexed previous_owner,
+        address indexed new_owner
+    );
 }
 
 /// An error that occurred in the implementation of an [`Ownable2Step`]
@@ -60,32 +66,33 @@ sol_storage! {
     }
 }
 
-#[public]
-impl Ownable2Step {
+/// Interface for an [`Ownable2Step`] contract.
+pub trait IOwnable2Step {
+    /// The error type associated to the trait implementation.
+    type Error: Into<alloc::vec::Vec<u8>>;
+
     /// Returns the address of the current owner.
     ///
+    /// Re-export of [`Ownable::owner`].
+    ///
     /// # Arguments
     ///
     /// * `&self` - Read access to the contract's state.
-    pub fn owner(&self) -> Address {
-        self._ownable.owner()
-    }
+    fn owner(&self) -> Address;
 
     /// Returns the address of the pending owner.
     ///
     /// # Arguments
     ///
     /// * `&self` - Read access to the contract's state.
-    pub fn pending_owner(&self) -> Address {
-        self._pending_owner.get()
-    }
+    fn pending_owner(&self) -> Address;
 
-    /// Starts the ownership transfer of the contract to a new account. Replaces
-    /// the pending transfer if there is one. Can only be called by the
+    /// Starts the ownership transfer of the contract to a new account.
+    /// Replaces the pending transfer if there is one. Can only be called by the
     /// current owner.
     ///
-    /// Setting `newOwner` to the zero address is allowed; this can be used to
-    /// cancel an initiated ownership transfer.
+    /// Setting `new_owner` to `Address::ZERO` is allowed; this can be used
+    /// to cancel an initiated ownership transfer.
     ///
     /// # Arguments
     ///
@@ -100,23 +107,13 @@ impl Ownable2Step {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferStarted`] event.
-    pub fn transfer_ownership(
+    fn transfer_ownership(
         &mut self,
         new_owner: Address,
-    ) -> Result<(), Error> {
-        self._ownable.only_owner()?;
-        self._pending_owner.set(new_owner);
+    ) -> Result<(), Self::Error>;
 
-        let current_owner = self.owner();
-        evm::log(OwnershipTransferStarted {
-            previous_owner: current_owner,
-            new_owner,
-        });
-        Ok(())
-    }
-
-    /// Accepts the ownership of the contract. Can only be called by the
-    /// pending owner.
+    /// Accepts the ownership of the contract.
+    /// Can only be called by the pending owner.
     ///
     /// # Arguments
     ///
@@ -130,18 +127,7 @@ impl Ownable2Step {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferred`] event.
-    pub fn accept_ownership(&mut self) -> Result<(), Error> {
-        let sender = msg::sender();
-        let pending_owner = self.pending_owner();
-        if sender != pending_owner {
-            return Err(OwnableError::UnauthorizedAccount(
-                OwnableUnauthorizedAccount { account: sender },
-            )
-            .into());
-        }
-        self._transfer_ownership(sender);
-        Ok(())
-    }
+    fn accept_ownership(&mut self) -> Result<(), Self::Error>;
 
     /// Leaves the contract without owner. It will not be possible to call
     /// [`Ownable::only_owner`] functions. Can only be called by the current
@@ -159,7 +145,50 @@ impl Ownable2Step {
     /// # Events
     ///
     /// Emits a [`OwnershipTransferred`] event.
-    pub fn renounce_ownership(&mut self) -> Result<(), Error> {
+    fn renounce_ownership(&mut self) -> Result<(), Self::Error>;
+}
+
+#[public]
+impl IOwnable2Step for Ownable2Step {
+    type Error = Error;
+
+    fn owner(&self) -> Address {
+        self._ownable.owner()
+    }
+
+    fn pending_owner(&self) -> Address {
+        self._pending_owner.get()
+    }
+
+    fn transfer_ownership(
+        &mut self,
+        new_owner: Address,
+    ) -> Result<(), Self::Error> {
+        self._ownable.only_owner()?;
+        self._pending_owner.set(new_owner);
+
+        let current_owner = self.owner();
+        evm::log(OwnershipTransferStarted {
+            previous_owner: current_owner,
+            new_owner,
+        });
+        Ok(())
+    }
+
+    fn accept_ownership(&mut self) -> Result<(), Self::Error> {
+        let sender = msg::sender();
+        let pending_owner = self.pending_owner();
+        if sender != pending_owner {
+            return Err(OwnableError::UnauthorizedAccount(
+                OwnableUnauthorizedAccount { account: sender },
+            )
+            .into());
+        }
+        self._transfer_ownership(sender);
+        Ok(())
+    }
+
+    fn renounce_ownership(&mut self) -> Result<(), Error> {
         self._ownable.only_owner()?;
         self._transfer_ownership(Address::ZERO);
         Ok(())
@@ -168,9 +197,10 @@ impl Ownable2Step {
 
 impl Ownable2Step {
     /// Transfers ownership of the contract to a new account (`new_owner`) and
-    /// sets [`Self::pending_owner`] to zero to avoid situations where the
-    /// transfer has been completed or the current owner renounces, but
-    /// [`Self::pending_owner`] can still accept ownership.
+    /// sets [`Self::pending_owner`] to `Address::ZERO` to avoid situations
+    /// where the transfer has been completed or the current owner renounces,
+    /// but [`Self::pending_owner`] can still accept ownership.
+    ///
     /// Internal function without access restriction.
     ///
     /// # Arguments
@@ -192,7 +222,7 @@ mod tests {
     use alloy_primitives::{address, Address};
     use stylus_sdk::msg;
 
-    use super::{Error, Ownable2Step, OwnableError};
+    use super::{Error, IOwnable2Step, Ownable2Step, OwnableError};
 
     const ALICE: Address = address!("A11CEacF9aa32246d767FCCD72e02d6bCbcC375d");
     const BOB: Address = address!("B0B0cB49ec2e96DF5F5fFB081acaE66A2cBBc2e2");
diff --git a/examples/ownable-two-step/tests/ownable_two_step.rs b/examples/ownable-two-step/tests/ownable_two_step.rs
index 28be08ea3..74c253989 100644
--- a/examples/ownable-two-step/tests/ownable_two_step.rs
+++ b/examples/ownable-two-step/tests/ownable_two_step.rs
@@ -36,6 +36,7 @@ async fn constructs(alice: Account) -> Result<()> {
 
     let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
     assert_eq!(owner, alice_addr);
+
     let Ownable2Step::pendingOwnerReturn { pendingOwner } =
         contract.pendingOwner().call().await?;
     assert_eq!(pendingOwner, Address::ZERO);
@@ -57,6 +58,7 @@ async fn construct_reverts_when_owner_is_zero_address(
     assert!(err.reverted_with(Ownable2Step::OwnableInvalidOwner {
         owner: Address::ZERO
     }));
+
     Ok(())
 }
 
@@ -151,6 +153,98 @@ async fn accept_ownership(alice: Account, bob: Account) -> Result<()> {
     Ok(())
 }
 
+#[e2e::test]
+async fn transfer_ownership_cancel_transfer(
+    alice: Account,
+    bob: Account,
+) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+    receipt!(contract.transferOwnership(bob_addr))?;
+
+    let receipt = receipt!(contract.transferOwnership(Address::ZERO))?;
+    assert!(receipt.emits(OwnershipTransferStarted {
+        previousOwner: alice_addr,
+        newOwner: Address::ZERO,
+    }));
+
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, Address::ZERO);
+
+    Ok(())
+}
+
+#[e2e::test]
+async fn overwrite_previous_transfer_ownership(
+    alice: Account,
+    bob: Account,
+    charlie: Account,
+) -> Result<()> {
+    let alice_addr = alice.address();
+    let bob_addr = bob.address();
+    let charlie_addr = charlie.address();
+
+    let contract_addr = alice
+        .as_deployer()
+        .with_constructor(ctr(alice_addr))
+        .deploy()
+        .await?
+        .address()?;
+
+    let contract = Ownable2Step::new(contract_addr, &alice.wallet);
+
+    let receipt = receipt!(contract.transferOwnership(bob_addr))?;
+    assert!(receipt.emits(OwnershipTransferred {
+        previousOwner: alice_addr,
+        newOwner: bob_addr,
+    }));
+
+    let receipt = receipt!(contract.transferOwnership(charlie_addr))?;
+    assert!(receipt.emits(OwnershipTransferred {
+        previousOwner: alice_addr,
+        newOwner: charlie_addr,
+    }));
+
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, charlie_addr);
+
+    // Connect as Bob and try to accept ownership
+    let contract = Ownable2Step::new(contract_addr, &bob.wallet);
+    let err = send!(contract.acceptOwnership())
+        .expect_err("should not accept when not pending owner");
+
+    err.reverted_with(Ownable2Step::OwnableUnauthorizedAccount {
+        account: bob_addr,
+    });
+
+    // Connect as Charlie and accept ownership
+    let contract = Ownable2Step::new(contract_addr, &charlie.wallet);
+    let receipt = receipt!(contract.acceptOwnership())?;
+    assert!(receipt.emits(OwnershipTransferred {
+        previousOwner: alice_addr,
+        newOwner: charlie_addr,
+    }));
+
+    let Ownable2Step::pendingOwnerReturn { pendingOwner } =
+        contract.pendingOwner().call().await?;
+    assert_eq!(pendingOwner, Address::ZERO);
+
+    let Ownable2Step::ownerReturn { owner } = contract.owner().call().await?;
+    assert_eq!(owner, charlie_addr);
+
+    Ok(())
+}
+
 #[e2e::test]
 async fn accept_ownership_reverts_when_not_pending_owner(
     alice: Account,

From 3405f7611e45fc1c23026f47a9882382552741ca Mon Sep 17 00:00:00 2001
From: Daniel Bigos <daniel.bigos@openzeppelin.com>
Date: Fri, 15 Nov 2024 00:54:46 +0100
Subject: [PATCH 14/14] ref: improvements to Ownable2Step (#404)

---
 contracts/src/access/ownable_two_step.rs           | 14 +++-----------
 .../ownable-two-step/tests/ownable_two_step.rs     |  4 ++--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/contracts/src/access/ownable_two_step.rs b/contracts/src/access/ownable_two_step.rs
index 9e189ce81..f5d9a2be1 100644
--- a/contracts/src/access/ownable_two_step.rs
+++ b/contracts/src/access/ownable_two_step.rs
@@ -38,14 +38,6 @@ sol! {
         address indexed new_owner
     );
 
-    /// Emitted when ownership gets transferred between accounts.
-    ///
-    /// * `previous_owner` - Address of the previous owner.
-    /// * `new_owner` - Address of the new owner.
-    event OwnershipTransferred(
-        address indexed previous_owner,
-        address indexed new_owner
-    );
 }
 
 /// An error that occurred in the implementation of an [`Ownable2Step`]
@@ -126,7 +118,7 @@ pub trait IOwnable2Step {
     ///
     /// # Events
     ///
-    /// Emits a [`OwnershipTransferred`] event.
+    /// Emits a [`crate::access::ownable::OwnershipTransferred`] event.
     fn accept_ownership(&mut self) -> Result<(), Self::Error>;
 
     /// Leaves the contract without owner. It will not be possible to call
@@ -144,7 +136,7 @@ pub trait IOwnable2Step {
     ///
     /// # Events
     ///
-    /// Emits a [`OwnershipTransferred`] event.
+    /// Emits a [`crate::access::ownable::OwnershipTransferred`] event.
     fn renounce_ownership(&mut self) -> Result<(), Self::Error>;
 }
 
@@ -210,7 +202,7 @@ impl Ownable2Step {
     ///
     /// # Events
     ///
-    /// Emits a [`OwnershipTransferred`] event.
+    /// Emits a [`crate::access::ownable::OwnershipTransferred`] event.
     fn _transfer_ownership(&mut self, new_owner: Address) {
         self._pending_owner.set(Address::ZERO);
         self._ownable._transfer_ownership(new_owner);
diff --git a/examples/ownable-two-step/tests/ownable_two_step.rs b/examples/ownable-two-step/tests/ownable_two_step.rs
index 74c253989..0e2216231 100644
--- a/examples/ownable-two-step/tests/ownable_two_step.rs
+++ b/examples/ownable-two-step/tests/ownable_two_step.rs
@@ -203,13 +203,13 @@ async fn overwrite_previous_transfer_ownership(
     let contract = Ownable2Step::new(contract_addr, &alice.wallet);
 
     let receipt = receipt!(contract.transferOwnership(bob_addr))?;
-    assert!(receipt.emits(OwnershipTransferred {
+    assert!(receipt.emits(OwnershipTransferStarted {
         previousOwner: alice_addr,
         newOwner: bob_addr,
     }));
 
     let receipt = receipt!(contract.transferOwnership(charlie_addr))?;
-    assert!(receipt.emits(OwnershipTransferred {
+    assert!(receipt.emits(OwnershipTransferStarted {
         previousOwner: alice_addr,
         newOwner: charlie_addr,
     }));