feat: Ownable2Step (#352)

Resolves #290 

- Introduce a new Ownable2Step contract
- Make `onlyOwner()` internal in Ownable and implement `MethodError` to
be able to use the error in our new contract(we've already seen we will
need this for others)
- Propose we use nextest instead of vainilla `cargo test` in the CI. It
is faster(not as relevant right now) and the output is so much nicer

#### PR Checklist
- [x] Unit Tests
- [x] e2e Tests
- [x] Documentation

---------

Co-authored-by: Daniel Bigos <[email protected]>

Author: ggonzalez94

.github/pull_request_template.md

@@ -19,3 +19,4 @@ Some of the items may not apply.
 
 - [ ] Tests
 - [ ] Documentation
+- [ ] Changelog

.github/workflows/test.yml

@@ -11,7 +11,7 @@ permissions:
   contents: read
 on:
   push:
-    branches: [ main, release/* ]
+    branches: [main, release/*]
     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:
@@ -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.
@@ -52,7 +57,7 @@ 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
@@ -64,7 +69,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:
@@ -78,12 +83,17 @@ 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
 
       - 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.

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
 

Cargo.lock

@@ -19,6 +19,7 @@ members = [
   "examples/basic/token",
   "examples/basic/script",
   "examples/ecdsa",
+  "examples/ownable-two-step",
   "examples/safe-erc20",
   "benches",
 ]
@@ -38,6 +39,7 @@ default-members = [
   "examples/safe-erc20",
   "examples/merkle-proofs",
   "examples/ownable",
+  "examples/ownable-two-step",
   "examples/access-control",
   "examples/basic/token",
   "examples/ecdsa",

Cargo.toml

@@ -1,3 +1,4 @@
 //! Contracts implementing access control mechanisms.
 pub mod control;
 pub mod ownable;
+pub mod ownable_two_step;

contracts/src/access/mod.rs

@@ -10,13 +10,18 @@
 //! 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,
     stylus_proc::{public, sol_storage, SolidityError},
 };
 
 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);
 }
@@ -45,6 +50,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 {
@@ -53,32 +64,21 @@ sol_storage! {
     }
 }
 
-#[public]
-impl Ownable {
-    /// Returns the address of the current owner.
-    pub fn owner(&self) -> Address {
-        self._owner.get()
-    }
+/// Interface for an [`Ownable`] contract.
+#[interface_id]
+pub trait IOwnable {
+    /// The error type associated to the trait implementation.
+    type Error: Into<alloc::vec::Vec<u8>>;
 
-    /// Checks if the [`msg::sender`] is set as the owner.
+    /// Returns the address of the current owner.
     ///
-    /// # Errors
+    /// # Arguments
     ///
-    /// 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 },
-            ));
-        }
+    /// * `&self` - Read access to the contract's state.
+    fn owner(&self) -> Address;
 
-        Ok(())
-    }
-
-    /// 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
     ///
@@ -89,13 +89,52 @@ impl Ownable {
     ///
     /// If `new_owner` is the zero address, then the error
     /// [`OwnableInvalidOwner`] is returned.
-    pub fn transfer_ownership(
+    ///
+    /// # Events
+    ///
+    /// Emits a [`OwnershipTransferred`] event.
+    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
+    /// owner.
+    ///
+    /// 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.
+    fn renounce_ownership(&mut self) -> Result<(), Self::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> {
+    ) -> Result<(), Self::Error> {
         self.only_owner()?;
 
-        if new_owner == Address::ZERO {
+        if new_owner.is_zero() {
             return Err(Error::InvalidOwner(OwnableInvalidOwner {
                 owner: Address::ZERO,
             }));
@@ -106,31 +145,46 @@ impl Ownable {
         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.
+    fn renounce_ownership(&mut self) -> Result<(), Self::Error> {
+        self.only_owner()?;
+        self._transfer_ownership(Address::ZERO);
+        Ok(())
+    }
+}
+
+impl Ownable {
+    /// Checks if the [`msg::sender`] is set as the owner.
     ///
-    /// NOTE: Renouncing ownership will leave the contract without an owner,
-    /// thereby disabling any functionality that is only available to the owner.
+    /// # Arguments
+    ///
+    /// * `&self` - Read access to the contract's state.
     ///
     /// # Errors
     ///
-    /// If not called by the owner, then the error
+    /// If called by any account other than the owner, then the error
     /// [`Error::UnauthorizedAccount`] is returned.
-    pub fn renounce_ownership(&mut self) -> Result<(), Error> {
-        self.only_owner()?;
-        self._transfer_ownership(Address::ZERO);
+    pub fn only_owner(&self) -> Result<(), Error> {
+        let account = msg::sender();
+        if self.owner() != account {
+            return Err(Error::UnauthorizedAccount(
+                OwnableUnauthorizedAccount { account },
+            ));
+        }
+
         Ok(())
     }
-}
 
-impl Ownable {
     /// Transfers ownership of the contract to a new account (`new_owner`).
     /// 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.
+    /// * `new_owner` - Account that is going to 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);
@@ -143,7 +197,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");