From 4b07d6882a04fa2c257bc045e32b2ae5c25f9641 Mon Sep 17 00:00:00 2001 From: alexfertel Date: Tue, 23 Jul 2024 11:51:46 +0200 Subject: [PATCH] feat(docs): document Ownable (#207) Resolves #206 & #12 --- README.md | 7 ++- contracts/src/access/control.rs | 8 +-- contracts/src/access/ownable.rs | 4 +- contracts/src/lib.rs | 46 +++++++++++++++- .../src/token/erc20/extensions/burnable.rs | 12 ++--- .../src/token/erc20/extensions/metadata.rs | 5 +- .../src/token/erc721/extensions/burnable.rs | 20 +++---- .../src/token/erc721/extensions/enumerable.rs | 36 +++++++------ .../src/token/erc721/extensions/metadata.rs | 4 +- contracts/src/token/erc721/mod.rs | 10 ++-- docs/modules/ROOT/nav.adoc | 1 + docs/modules/ROOT/pages/access-control.adoc | 53 +++++++++++++++++++ lib/crypto/src/lib.rs | 21 +++++++- lib/crypto/src/merkle.rs | 6 +-- 14 files changed, 178 insertions(+), 55 deletions(-) create mode 100644 docs/modules/ROOT/pages/access-control.adoc diff --git a/README.md b/README.md index 831f4547..a036747a 100644 --- a/README.md +++ b/README.md @@ -32,13 +32,12 @@ a `Cargo.toml`, like so: openzeppelin-stylus = { git = "https://github.com/OpenZeppelin/rust-contracts-stylus" } ``` -We recommend pinning to a specific version -- expect rapid iteration. Also note -that the library's name has yet to be decided. +We recommend pinning to a specific version -- expect rapid iteration. Once defined as a dependency, use one of our pre-defined implementations by importing them: -```rust,ignore +```rust use openzeppelin_stylus::token::erc20::Erc20; sol_storage! { @@ -85,4 +84,4 @@ Refer to our [Security Policy](SECURITY.md) for more details. ## License -OpenZeppelin Contracts for Stylus is released under the [MIT License](LICENSE). +OpenZeppelin Contracts for Stylus is released under the [MIT License](./LICENSE). diff --git a/contracts/src/access/control.rs b/contracts/src/access/control.rs index 8d9dd12b..90e8bd0e 100644 --- a/contracts/src/access/control.rs +++ b/contracts/src/access/control.rs @@ -21,8 +21,8 @@ //! //! ```rust,ignore //! pub fn foo() { -//! assert!(self.has_role(MY_ROLE.into(), msg::sender()); -//! ... +//! assert!(self.has_role(MY_ROLE.into(), msg::sender())); +//! // ... //! } //! ``` //! @@ -42,7 +42,7 @@ //! `AccessControlDefaultAdminRules` to enforce additional security measures for //! this role. //! -//! [enumerable ext]: TBD +//! [enumerable ext]: ./TODO use alloy_primitives::{Address, B256}; use alloy_sol_types::sol; use stylus_proc::SolidityError; @@ -153,7 +153,7 @@ impl AccessControl { /// Returns the admin role that controls `role`. See [`Self::grant_role`] /// and [`Self::revoke_role`]. /// - /// To change a role's admin, use [`Self::set_role_admin`]. + /// To change a role's admin, use [`Self::_set_role_admin`]. /// /// # Arguments /// diff --git a/contracts/src/access/ownable.rs b/contracts/src/access/ownable.rs index 09386bd5..7c51d191 100644 --- a/contracts/src/access/ownable.rs +++ b/contracts/src/access/ownable.rs @@ -89,7 +89,7 @@ impl Ownable { /// # Errors /// /// If `new_owner` is the zero address, then the error - /// [`Error::OwnableInvalidOwner`] is returned. + /// [`OwnableInvalidOwner`] is returned. pub fn transfer_ownership( &mut self, new_owner: Address, @@ -108,7 +108,7 @@ impl Ownable { } /// Leaves the contract without owner. It will not be possible to call - /// [`only_owner`] functions. Can only be called by the current owner. + /// [`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. diff --git a/contracts/src/lib.rs b/contracts/src/lib.rs index 72d2c01c..dd4c8c73 100644 --- a/contracts/src/lib.rs +++ b/contracts/src/lib.rs @@ -1,6 +1,50 @@ -#![doc = include_str!("../../README.md")] +/*! +# OpenZeppelin Contracts for Stylus + +A library for secure smart contract development written in Rust for +[Arbitrum Stylus](https://docs.arbitrum.io/stylus/stylus-gentle-introduction). +This library offers common smart contract primitives and affordances that take +advantage of the nature of Stylus. + +> This project is still in a very early and experimental phase. It has never +> been audited nor thoroughly reviewed for security vulnerabilities. Do not use +> in production. + +## Usage + +To start using it, add `openzeppelin-stylus` to your `Cargo.toml`, or simply run +`cargo add openzeppelin-stylus`. + +```toml +[dependencies] +openzeppelin-stylus = "x.x.x" +``` + +We recommend pinning to a specific version -- expect rapid iteration. + +Once defined as a dependency, use one of our pre-defined implementations by +importing them: + +```ignore +use openzeppelin_stylus::token::erc20::Erc20; + +sol_storage! { + #[entrypoint] + struct MyContract { + #[borrow] + Erc20 erc20; + } +} + +#[external] +#[inherit(Erc20)] +impl MyContract { } +``` +*/ + #![allow(clippy::pub_underscore_fields, clippy::module_name_repetitions)] #![cfg_attr(not(feature = "std"), no_std, no_main)] +#![deny(rustdoc::broken_intra_doc_links)] extern crate alloc; #[global_allocator] diff --git a/contracts/src/token/erc20/extensions/burnable.rs b/contracts/src/token/erc20/extensions/burnable.rs index 3d36c3cb..42dea4ca 100644 --- a/contracts/src/token/erc20/extensions/burnable.rs +++ b/contracts/src/token/erc20/extensions/burnable.rs @@ -9,8 +9,8 @@ use crate::token::erc20::{Erc20, Error}; /// their own tokens and those that they have an allowance for, /// in a way that can be recognized off-chain (via event analysis). pub trait IErc20Burnable { - /// Destroys a `value` amount of tokens from the caller. - /// lowering the total supply. + /// Destroys a `value` amount of tokens from the caller. lowering the total + /// supply. /// /// Relies on the `update` mechanism. /// @@ -25,11 +25,11 @@ pub trait IErc20Burnable { /// /// # Events /// - /// Emits a [`Transfer`] event. + /// Emits a [`super::super::Transfer`] event. fn burn(&mut self, value: U256) -> Result<(), Error>; - /// Destroys a `value` amount of tokens from `account`, - /// lowering the total supply. + /// Destroys a `value` amount of tokens from `account`, lowering the total + /// supply. /// /// Relies on the `update` mechanism. /// @@ -49,7 +49,7 @@ pub trait IErc20Burnable { /// /// # Events /// - /// Emits a [`Transfer`] event. + /// Emits a [`super::super::Transfer`] event. fn burn_from(&mut self, account: Address, value: U256) -> Result<(), Error>; } diff --git a/contracts/src/token/erc20/extensions/metadata.rs b/contracts/src/token/erc20/extensions/metadata.rs index ab49b7af..f3db7420 100644 --- a/contracts/src/token/erc20/extensions/metadata.rs +++ b/contracts/src/token/erc20/extensions/metadata.rs @@ -10,7 +10,7 @@ pub const DEFAULT_DECIMALS: u8 = 18; use crate::utils::Metadata; sol_storage! { - /// Metadata of the [`Erc20`] token. + /// Metadata of the [`super::super::Erc20`] token. /// /// It has hardcoded `decimals` to [`DEFAULT_DECIMALS`]. pub struct Erc20Metadata { @@ -51,7 +51,8 @@ pub trait IErc20Metadata { /// /// NOTE: This information is only used for *display* purposes: in /// no way it affects any of the arithmetic of the contract, including - /// [`Erc20::balance_of`] and [`Erc20::transfer`]. + /// [`super::super::IErc20::balance_of`] and + /// [`super::super::IErc20::transfer`]. fn decimals(&self) -> u8; } diff --git a/contracts/src/token/erc721/extensions/burnable.rs b/contracts/src/token/erc721/extensions/burnable.rs index 3f9e6cf8..713009ee 100644 --- a/contracts/src/token/erc721/extensions/burnable.rs +++ b/contracts/src/token/erc721/extensions/burnable.rs @@ -8,8 +8,9 @@ use crate::token::erc721::{Erc721, Error}; /// An [`Erc721`] token that can be burned (destroyed). pub trait IErc721Burnable { /// Burns `token_id`. - /// The approval is cleared when the token is burned. - /// Relies on the `_burn` mechanism. + /// + /// The approval is cleared when the token is burned. Relies on the `_burn` + /// mechanism. /// /// # Arguments /// @@ -17,8 +18,8 @@ pub trait IErc721Burnable { /// /// # Errors /// - /// If token does not exist, then the error - /// [`Error::NonexistentToken`] is returned. + /// If token does not exist, then the error [`Error::NonexistentToken`] is + /// returned. /// If the caller does not have the right to approve, then the error /// [`Error::InsufficientApproval`] is returned. /// @@ -29,17 +30,18 @@ pub trait IErc721Burnable { /// /// # Events /// - /// Emits a [`Transfer`] event. + /// Emits a [`super::super::Transfer`] event. fn burn(&mut self, token_id: U256) -> Result<(), Error>; } impl IErc721Burnable for Erc721 { fn burn(&mut self, token_id: U256) -> Result<(), Error> { - // Setting an "auth" arguments enables the [`Erc721::_is_authorized`] - // check which verifies that the token exists (from != `Address::ZERO`). + // Setting an "auth" arguments enables the + // [`super::super::Erc721::_is_authorized`] check which verifies that + // the token exists (from != `Address::ZERO`). // - // Therefore, it is not needed to verify - // that the return value is not 0 here. + // Therefore, it is not needed to verify that the return value is not 0 + // here. self._update(Address::ZERO, token_id, msg::sender())?; Ok(()) } diff --git a/contracts/src/token/erc721/extensions/enumerable.rs b/contracts/src/token/erc721/extensions/enumerable.rs index f68d0126..f2aa522d 100644 --- a/contracts/src/token/erc721/extensions/enumerable.rs +++ b/contracts/src/token/erc721/extensions/enumerable.rs @@ -1,12 +1,15 @@ //! Optional `Enumerable` extension of the ERC-721 standard. //! -//! This implements an optional extension of [`Erc721`] defined in the EIP -//! that adds enumerability of all the token ids in the contract -//! as well as all token ids owned by each account. +//! This implements an optional extension of [`super::super::Erc721`] defined in +//! the EIP that adds enumerability of all the token ids in the contract as well +//! as all token ids owned by each account. //! -//! CAUTION: [`Erc721`] extensions that implement custom -//! [`Erc721::balance_of`] logic, such as [`Erc721Consecutive`], interfere with -//! enumerability and should not be used together with [`Erc721Enumerable`]. +//! CAUTION: [`super::super::Erc721`] extensions that implement custom +//! [`super::super::Erc721::balance_of`] logic, such as `Erc721Consecutive`, +//! interfere with enumerability and should not be used together with +//! [`Erc721Enumerable`]. +// TODO: Add link for `Erc721Consecutive` to module docs. + use alloy_primitives::{uint, Address, U256}; use alloy_sol_types::sol; use stylus_proc::{external, sol_storage, SolidityError}; @@ -63,11 +66,11 @@ sol_storage! { pub trait IErc721Enumerable { // TODO: fn supports_interface (#33) - /// Returns a token ID owned by `owner` - /// at a given `index` of its token list. + /// Returns a token ID owned by `owner` at a given `index` of its token + /// list. /// - /// Use along with [`Erc721::balance_of`] - /// to enumerate all of `owner`'s tokens. + /// Use along with [`super::super::Erc721::balance_of`] to enumerate all of + /// `owner`'s tokens. /// /// # Arguments /// @@ -75,8 +78,8 @@ pub trait IErc721Enumerable { /// /// # Errors /// - /// * If an `owner`'s token query is out of bounds for `index`, - /// then the error [`Error::OutOfBoundsIndex`] is returned. + /// * If an `owner`'s token query is out of bounds for `index`, then the + /// error [`Error::OutOfBoundsIndex`] is returned. fn token_of_owner_by_index( &self, owner: Address, @@ -93,7 +96,8 @@ pub trait IErc721Enumerable { /// Returns a token ID at a given `index` of all the tokens /// stored by the contract. /// - /// Use along with [`Erc721::total_supply`] to enumerate all tokens. + /// Use along with [`Self::total_supply`] to + /// enumerate all tokens. /// /// # Arguments /// @@ -149,7 +153,7 @@ impl Erc721Enumerable { /// # Errors /// /// If owner address is `Address::ZERO`, then the error - /// [`Error::InvalidOwner`] is returned. + /// [`crate::token::erc721::Error::InvalidOwner`] is returned. pub fn _add_token_to_owner_enumeration( &mut self, to: Address, @@ -200,7 +204,7 @@ impl Erc721Enumerable { /// # Errors /// /// If owner address is `Address::ZERO`, then the error - /// [`Error::InvalidOwner`] is returned. + /// [`crate::token::erc721::Error::InvalidOwner`] is returned. pub fn _remove_token_from_owner_enumeration( &mut self, from: Address, @@ -283,7 +287,7 @@ impl Erc721Enumerable { self._all_tokens.pop(); } - /// See [`Erc721::_increase_balance`]. + /// See [`crate::token::erc721::Erc721::_increase_balance`]. /// Check if tokens can be minted in batch. /// /// Mechanism to be consistent with [Solidity version](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v5.0.0/contracts/token/ERC721/extensions/ERC721Enumerable.sol#L163-L171) diff --git a/contracts/src/token/erc721/extensions/metadata.rs b/contracts/src/token/erc721/extensions/metadata.rs index b5dd2450..9bb5b34a 100644 --- a/contracts/src/token/erc721/extensions/metadata.rs +++ b/contracts/src/token/erc721/extensions/metadata.rs @@ -7,11 +7,11 @@ use stylus_proc::{external, sol_storage}; use crate::utils::Metadata; sol_storage! { - /// Metadata of the [`Erc721`] token. + /// Metadata of an [`crate::token::erc721::Erc721`] token. pub struct Erc721Metadata { /// Common Metadata. Metadata _metadata; - /// Base URI for tokens + /// Base URI for tokens. string _base_uri; } } diff --git a/contracts/src/token/erc721/mod.rs b/contracts/src/token/erc721/mod.rs index a4b2f9bc..a3b97b0d 100644 --- a/contracts/src/token/erc721/mod.rs +++ b/contracts/src/token/erc721/mod.rs @@ -271,7 +271,7 @@ pub trait IErc721 { /// * `to` - Account of the recipient. /// * `token_id` - Token id as a number. /// * `data` - Additional data with no specified format, sent in the call to - /// [`Self::_check_on_erc721_received`]. + /// [`Erc721::_check_on_erc721_received`]. /// /// # Errors /// @@ -293,7 +293,7 @@ pub trait IErc721 { /// * `to` cannot be the zero address. /// * The `token_id` token must exist and be owned by `from`. /// * If the caller is not `from`, it must be approved to move this token by - /// either [`Self::_approve`] or [`Self::set_approval_for_all`]. + /// either [`Erc721::_approve`] or [`Erc721::set_approval_for_all`]. /// * If `to` refers to a smart contract, it must implement /// [`IERC721Receiver::on_erc_721_received`], which is called upon a /// `safe_transfer`. @@ -369,7 +369,7 @@ pub trait IErc721 { /// /// If the token does not exist, then the error /// [`Error::NonexistentToken`] is returned. - /// If `auth` (param of [`Self::_approve`]) does not have a right to + /// If `auth` (param of [`Erc721::_approve`]) does not have a right to /// approve this token, then the error /// [`Error::InvalidApprover`] is returned. /// @@ -845,7 +845,7 @@ impl Erc721 { /// If `to` is `Address::ZERO`, then the error /// [`Error::InvalidReceiver`] is returned. /// If `token_id` does not exist, then the error - /// [`Error::ERC721NonexistentToken`] is returned. + /// [`ERC721NonexistentToken`] is returned. /// If the previous owner is not `from`, then the error /// [`Error::IncorrectOwner`] is returned. /// @@ -909,7 +909,7 @@ impl Erc721 { /// If `to` is `Address::ZERO`, then the error /// [`Error::InvalidReceiver`] is returned. /// If `token_id` does not exist, then the error - /// [`Error::ERC721NonexistentToken`] is returned. + /// [`ERC721NonexistentToken`] is returned. /// If the previous owner is not `from`, then the error /// [`Error::IncorrectOwner`] is returned. /// diff --git a/docs/modules/ROOT/nav.adoc b/docs/modules/ROOT/nav.adoc index 54ce7c08..66223e6a 100644 --- a/docs/modules/ROOT/nav.adoc +++ b/docs/modules/ROOT/nav.adoc @@ -1,3 +1,4 @@ * xref:index.adoc[Overview] +* xref:access-control.adoc[Access Control] * xref:crypto.adoc[Cryptography] diff --git a/docs/modules/ROOT/pages/access-control.adoc b/docs/modules/ROOT/pages/access-control.adoc new file mode 100644 index 00000000..6c08d9ba --- /dev/null +++ b/docs/modules/ROOT/pages/access-control.adoc @@ -0,0 +1,53 @@ += Access Control + +Access control—that is, "who is allowed to do this thing"—is incredibly important in the world of smart contracts. The access control of your contract may govern who can mint tokens, vote on proposals, freeze transfers, and many other things. It is therefore *critical* to understand how you implement it, lest someone else https://blog.openzeppelin.com/on-the-parity-wallet-multisig-hack-405a8c12e8f7[steals your whole system]. + +[[ownership-and-ownable]] +== Ownership and `Ownable` + +The most common and basic form of access control is the concept of _ownership_: there's an account that is the `owner` of a contract and can do administrative tasks on it. This approach is perfectly reasonable for contracts that have a single administrative user. + +OpenZeppelin Contracts for Stylus provides https://docs.rs/openzeppelin-stylus/latest/openzeppelin_stylus/access/ownable/struct.Ownable.html[`Ownable`] for implementing ownership in your contracts. + +```rust +use openzeppelin_stylus::access::ownable::Ownable; + +sol_storage! { + #[entrypoint] + struct OwnableExample { + #[borrow] + Ownable ownable; + } +} + +#[external] +#[inherit(Ownable)] +impl MyContract { + fn normal_thing(&self) { + // anyone can call this normal_thing() + } + + pub fn special_thing( + &mut self, + ) -> Result<(), Vec> { + self.ownable.only_owner()?; + + // only the owner can call special_thing()! + + Ok(()) + } +} +``` + +At deployment, the https://docs.rs/openzeppelin-stylus/latest/openzeppelin_stylus/access/ownable/struct.Ownable.html#method.owner[`owner`] of an `Ownable` contract is set to the provided `initial_owner` parameter. + +Ownable also lets you: + +* https://docs.rs/openzeppelin-stylus/latest/openzeppelin_stylus/access/ownable/struct.Ownable.html#method.transfer_ownership[`transfer_ownership`] from the owner account to a new one, and +* https://docs.rs/openzeppelin-stylus/latest/openzeppelin_stylus/access/ownable/struct.Ownable.html#method.renounce_ownership[`renounce_ownership`] for the owner to relinquish this administrative privilege, a common pattern after an initial stage with centralized administration is over. + +WARNING: Removing the owner altogether will mean that administrative tasks that are protected by `only_owner` will no longer be callable! + +Note that *a contract can also be the owner of another one*! This opens the door to using, for example, a https://gnosis-safe.io[Gnosis Safe], an https://aragon.org[Aragon DAO], or a totally custom contract that _you_ create. + +In this way, you can use _composability_ to add additional layers of access control complexity to your contracts. Instead of having a single regular Ethereum account (Externally Owned Account, or EOA) as the owner, you could use a 2-of-3 multisig run by your project leads, for example. Prominent projects in the space, such as https://makerdao.com[MakerDAO], use systems similar to this one. diff --git a/lib/crypto/src/lib.rs b/lib/crypto/src/lib.rs index 23931887..8c7fc012 100644 --- a/lib/crypto/src/lib.rs +++ b/lib/crypto/src/lib.rs @@ -1,4 +1,23 @@ -#![doc = include_str!("../README.md")] +/*! +Common cryptographic procedures for a blockchain environment. + +> Note that `crypto` is still `0.*.*`, so breaking changes +> [may occur at any time](https://semver.org/#spec-item-4). If you must depend +> on `crypto`, we recommend pinning to a specific version, i.e., `=0.y.z`. + +## Verifying Merkle Proofs + +[`merkle.rs`](./src/merkle.rs) provides: + +- A `verify` function which can prove that some value is part of a + [Merkle tree]. +- A `verify_multi_proof` function which can prove multiple values are part of a + [Merkle tree]. + +[Merkle tree]: https://en.wikipedia.org/wiki/Merkle_tree + +*/ + #![cfg_attr(not(feature = "std"), no_std, no_main)] extern crate alloc; diff --git a/lib/crypto/src/merkle.rs b/lib/crypto/src/merkle.rs index 90e71a71..48770605 100644 --- a/lib/crypto/src/merkle.rs +++ b/lib/crypto/src/merkle.rs @@ -1,8 +1,8 @@ //! This module deals with verification of Merkle Tree proofs. //! -//! The tree and the proofs can be generated using `OpenZeppelin`'s [merkle -//! tree library][https://github.com/OpenZeppelin/merkle-tree]. You will find a -//! quickstart guide in its README. +//! The tree and the proofs can be generated using `OpenZeppelin`'s +//! [merkle tree library](https://github.com/OpenZeppelin/merkle-tree). You will +//! find a quickstart guide in its README. //! //! WARNING: You should avoid using leaf values that are 64 bytes long //! prior to hashing, or use a hash function other than keccak256 for