From a3b5eea59687c65a7ff7117e6aa94d01cfff9eec Mon Sep 17 00:00:00 2001 From: Alisander Qoshqosh Date: Fri, 4 Oct 2024 14:46:24 +0400 Subject: [PATCH 1/4] fix: L-04 Public exposure of pause and unpause functions --- contracts/src/utils/pausable.rs | 62 ++++++++++++++++++--------------- examples/erc20/src/lib.rs | 10 +++++- examples/erc721/src/lib.rs | 10 +++++- 3 files changed, 52 insertions(+), 30 deletions(-) diff --git a/contracts/src/utils/pausable.rs b/contracts/src/utils/pausable.rs index a49aec77..9d3a7851 100644 --- a/contracts/src/utils/pausable.rs +++ b/contracts/src/utils/pausable.rs @@ -9,6 +9,10 @@ //! //! Note that they will not be pausable by simply including this module, //! only once the modifiers are put in place. +//! +//! Note that [`Pausable::pause`] and [`Pausable::unpause`] methods are not +//! exposed by default. +//! You should expose them manually in your contract's abi. use alloy_sol_types::sol; use stylus_proc::{public, sol_storage, SolidityError}; @@ -68,73 +72,75 @@ impl Pausable { self._paused.get() } - /// Triggers `Paused` state. + /// Modifier to make a function callable only when the contract is NOT + /// paused. /// /// # Arguments /// - /// * `&mut self` - Write access to the contract's state. + /// * `&self` - Read access to the contract's state. /// /// # Errors /// - /// If the contract is in `Paused` state, then the error + /// If the contract is in the `Paused` state, then the error /// [`Error::EnforcedPause`] is returned. - pub fn pause(&mut self) -> Result<(), Error> { - self.when_not_paused()?; - self._paused.set(true); - evm::log(Paused { account: msg::sender() }); + pub fn when_not_paused(&self) -> Result<(), Error> { + if self._paused.get() { + return Err(Error::EnforcedPause(EnforcedPause {})); + } Ok(()) } - /// Triggers `Unpaused` state. + /// Modifier to make a function callable + /// only when the contract is paused. /// /// # Arguments /// - /// * `&mut self` - Write access to the contract's state. + /// * `&self` - Read access to the contract's state. /// /// # Errors /// /// If the contract is in `Unpaused` state, then the error /// [`Error::ExpectedPause`] is returned. - pub fn unpause(&mut self) -> Result<(), Error> { - self.when_paused()?; - self._paused.set(false); - evm::log(Unpaused { account: msg::sender() }); + pub fn when_paused(&self) -> Result<(), Error> { + if !self._paused.get() { + return Err(Error::ExpectedPause(ExpectedPause {})); + } Ok(()) } +} - /// Modifier to make a function callable only when the contract is NOT - /// paused. +impl Pausable { + /// Triggers `Paused` state. /// /// # Arguments /// - /// * `&self` - Read access to the contract's state. + /// * `&mut self` - Write access to the contract's state. /// /// # Errors /// - /// If the contract is in the `Paused` state, then the error + /// If the contract is in `Paused` state, then the error /// [`Error::EnforcedPause`] is returned. - pub fn when_not_paused(&self) -> Result<(), Error> { - if self._paused.get() { - return Err(Error::EnforcedPause(EnforcedPause {})); - } + pub fn pause(&mut self) -> Result<(), Error> { + self.when_not_paused()?; + self._paused.set(true); + evm::log(Paused { account: msg::sender() }); Ok(()) } - /// Modifier to make a function callable - /// only when the contract is paused. + /// Triggers `Unpaused` state. /// /// # Arguments /// - /// * `&self` - Read access to the contract's state. + /// * `&mut self` - Write access to the contract's state. /// /// # Errors /// /// If the contract is in `Unpaused` state, then the error /// [`Error::ExpectedPause`] is returned. - pub fn when_paused(&self) -> Result<(), Error> { - if !self._paused.get() { - return Err(Error::ExpectedPause(ExpectedPause {})); - } + pub fn unpause(&mut self) -> Result<(), Error> { + self.when_paused()?; + self._paused.set(false); + evm::log(Unpaused { account: msg::sender() }); Ok(()) } } diff --git a/examples/erc20/src/lib.rs b/examples/erc20/src/lib.rs index 407e87d0..fba5ca54 100644 --- a/examples/erc20/src/lib.rs +++ b/examples/erc20/src/lib.rs @@ -9,7 +9,7 @@ use openzeppelin_stylus::{ extensions::{capped, Capped, Erc20Metadata, IErc20Burnable}, Erc20, IErc20, }, - utils::Pausable, + utils::{pausable::Error, Pausable}, }; use stylus_sdk::prelude::{entrypoint, public, sol_storage}; @@ -105,4 +105,12 @@ impl Erc20Example { self.pausable.when_not_paused()?; self.erc20.transfer_from(from, to, value).map_err(|e| e.into()) } + + pub fn pause(&mut self) -> Result<(), Error> { + self.pausable.pause() + } + + pub fn unpause(&mut self) -> Result<(), Error> { + self.pausable.unpause() + } } diff --git a/examples/erc721/src/lib.rs b/examples/erc721/src/lib.rs index 8d2a4d62..5046c99a 100644 --- a/examples/erc721/src/lib.rs +++ b/examples/erc721/src/lib.rs @@ -9,7 +9,7 @@ use openzeppelin_stylus::{ extensions::{Erc721Enumerable as Enumerable, IErc721Burnable}, Erc721, IErc721, }, - utils::Pausable, + utils::{pausable::Error, Pausable}, }; use stylus_sdk::{ abi::Bytes, @@ -151,4 +151,12 @@ impl Erc721Example { Ok(()) } + + pub fn pause(&mut self) -> Result<(), Error> { + self.pausable.pause() + } + + pub fn unpause(&mut self) -> Result<(), Error> { + self.pausable.unpause() + } } From cef3e38a1b3715c4d0d23e10def8092e58e3229f Mon Sep 17 00:00:00 2001 From: Alisander Qoshqosh Date: Fri, 4 Oct 2024 23:33:48 +0400 Subject: [PATCH 2/4] remove public modifiers --- contracts/src/utils/pausable.rs | 61 ++++++++++++++++---------------- examples/erc20/tests/abi/mod.rs | 4 --- examples/erc20/tests/erc20.rs | 24 ------------- examples/erc721/tests/abi/mod.rs | 4 --- examples/erc721/tests/erc721.rs | 24 ------------- 5 files changed, 30 insertions(+), 87 deletions(-) diff --git a/contracts/src/utils/pausable.rs b/contracts/src/utils/pausable.rs index 9d3a7851..34a514fb 100644 --- a/contracts/src/utils/pausable.rs +++ b/contracts/src/utils/pausable.rs @@ -8,7 +8,8 @@ //! which can be added to the functions of your contract. //! //! Note that they will not be pausable by simply including this module, -//! only once the modifiers are put in place. +//! only once functions [`Pausable::when_not_paused`] and +//! [`Pausable::when_paused`] are put in place. //! //! Note that [`Pausable::pause`] and [`Pausable::unpause`] methods are not //! exposed by default. @@ -71,76 +72,74 @@ impl Pausable { fn paused(&self) -> bool { self._paused.get() } +} - /// Modifier to make a function callable only when the contract is NOT - /// paused. +impl Pausable { + /// Triggers `Paused` state. /// /// # Arguments /// - /// * `&self` - Read access to the contract's state. + /// * `&mut self` - Write access to the contract's state. /// /// # Errors /// - /// If the contract is in the `Paused` state, then the error + /// If the contract is in `Paused` state, then the error /// [`Error::EnforcedPause`] is returned. - pub fn when_not_paused(&self) -> Result<(), Error> { - if self._paused.get() { - return Err(Error::EnforcedPause(EnforcedPause {})); - } + pub fn pause(&mut self) -> Result<(), Error> { + self.when_not_paused()?; + self._paused.set(true); + evm::log(Paused { account: msg::sender() }); Ok(()) } - /// Modifier to make a function callable - /// only when the contract is paused. + /// Triggers `Unpaused` state. /// /// # Arguments /// - /// * `&self` - Read access to the contract's state. + /// * `&mut self` - Write access to the contract's state. /// /// # Errors /// /// If the contract is in `Unpaused` state, then the error /// [`Error::ExpectedPause`] is returned. - pub fn when_paused(&self) -> Result<(), Error> { - if !self._paused.get() { - return Err(Error::ExpectedPause(ExpectedPause {})); - } + pub fn unpause(&mut self) -> Result<(), Error> { + self.when_paused()?; + self._paused.set(false); + evm::log(Unpaused { account: msg::sender() }); Ok(()) } -} -impl Pausable { - /// Triggers `Paused` state. + /// Returns error when the contract is NOT paused. /// /// # Arguments /// - /// * `&mut self` - Write access to the contract's state. + /// * `&self` - Read access to the contract's state. /// /// # Errors /// - /// If the contract is in `Paused` state, then the error + /// If the contract is in the `Paused` state, then the error /// [`Error::EnforcedPause`] is returned. - pub fn pause(&mut self) -> Result<(), Error> { - self.when_not_paused()?; - self._paused.set(true); - evm::log(Paused { account: msg::sender() }); + pub fn when_not_paused(&self) -> Result<(), Error> { + if self._paused.get() { + return Err(Error::EnforcedPause(EnforcedPause {})); + } Ok(()) } - /// Triggers `Unpaused` state. + /// Returns error when the contract is paused. /// /// # Arguments /// - /// * `&mut self` - Write access to the contract's state. + /// * `&self` - Read access to the contract's state. /// /// # Errors /// /// If the contract is in `Unpaused` state, then the error /// [`Error::ExpectedPause`] is returned. - pub fn unpause(&mut self) -> Result<(), Error> { - self.when_paused()?; - self._paused.set(false); - evm::log(Unpaused { account: msg::sender() }); + pub fn when_paused(&self) -> Result<(), Error> { + if !self._paused.get() { + return Err(Error::ExpectedPause(ExpectedPause {})); + } Ok(()) } } diff --git a/examples/erc20/tests/abi/mod.rs b/examples/erc20/tests/abi/mod.rs index 0e652d26..9b7405ff 100644 --- a/examples/erc20/tests/abi/mod.rs +++ b/examples/erc20/tests/abi/mod.rs @@ -24,10 +24,6 @@ sol!( function paused() external view returns (bool paused); function pause() external; function unpause() external; - #[derive(Debug)] - function whenPaused() external view; - #[derive(Debug)] - function whenNotPaused() external view; error EnforcedPause(); error ExpectedPause(); diff --git a/examples/erc20/tests/erc20.rs b/examples/erc20/tests/erc20.rs index cc82c8de..a94b499e 100644 --- a/examples/erc20/tests/erc20.rs +++ b/examples/erc20/tests/erc20.rs @@ -1061,18 +1061,6 @@ async fn pauses(alice: Account) -> eyre::Result<()> { assert_eq!(true, paused); - let result = contract.whenPaused().call().await; - - assert!(result.is_ok()); - - let err = contract - .whenNotPaused() - .call() - .await - .expect_err("should return `EnforcedPause`"); - - assert!(err.reverted_with(Erc20::EnforcedPause {})); - Ok(()) } @@ -1096,18 +1084,6 @@ async fn unpauses(alice: Account) -> eyre::Result<()> { assert_eq!(false, paused); - let result = contract.whenNotPaused().call().await; - - assert!(result.is_ok()); - - let err = contract - .whenPaused() - .call() - .await - .expect_err("should return `ExpectedPause`"); - - assert!(err.reverted_with(Erc20::ExpectedPause {})); - Ok(()) } diff --git a/examples/erc721/tests/abi/mod.rs b/examples/erc721/tests/abi/mod.rs index b02f4448..929689bd 100644 --- a/examples/erc721/tests/abi/mod.rs +++ b/examples/erc721/tests/abi/mod.rs @@ -24,10 +24,6 @@ sol!( function pause() external; function unpause() external; #[derive(Debug)] - function whenPaused() external view; - #[derive(Debug)] - function whenNotPaused() external view; - #[derive(Debug)] function tokenOfOwnerByIndex(address owner, uint256 index) external view returns (uint256 tokenId); #[derive(Debug)] function tokenByIndex(uint256 index) external view returns (uint256 tokenId); diff --git a/examples/erc721/tests/erc721.rs b/examples/erc721/tests/erc721.rs index b8bf68fd..fbd522d6 100644 --- a/examples/erc721/tests/erc721.rs +++ b/examples/erc721/tests/erc721.rs @@ -1371,18 +1371,6 @@ async fn pauses(alice: Account) -> eyre::Result<()> { assert!(paused); - let result = contract.whenPaused().call().await; - - assert!(result.is_ok()); - - let err = contract - .whenNotPaused() - .call() - .await - .expect_err("should return `EnforcedPause`"); - - assert!(err.reverted_with(Erc721::EnforcedPause {})); - Ok(()) } @@ -1401,18 +1389,6 @@ async fn unpauses(alice: Account) -> eyre::Result<()> { assert_eq!(false, paused); - let result = contract.whenNotPaused().call().await; - - assert!(result.is_ok()); - - let err = contract - .whenPaused() - .call() - .await - .expect_err("should return `ExpectedPause`"); - - assert!(err.reverted_with(Erc721::ExpectedPause {})); - Ok(()) } From 4d5402f39a7f4a205ad98886c6fab1b523a68647 Mon Sep 17 00:00:00 2001 From: Alisander Qoshqosh Date: Fri, 4 Oct 2024 23:41:53 +0400 Subject: [PATCH 3/4] use consistent Vec error type at examples --- examples/erc20/src/lib.rs | 10 +++++----- examples/erc721/src/lib.rs | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/erc20/src/lib.rs b/examples/erc20/src/lib.rs index fba5ca54..4e64a6ec 100644 --- a/examples/erc20/src/lib.rs +++ b/examples/erc20/src/lib.rs @@ -9,7 +9,7 @@ use openzeppelin_stylus::{ extensions::{capped, Capped, Erc20Metadata, IErc20Burnable}, Erc20, IErc20, }, - utils::{pausable::Error, Pausable}, + utils::Pausable, }; use stylus_sdk::prelude::{entrypoint, public, sol_storage}; @@ -106,11 +106,11 @@ impl Erc20Example { self.erc20.transfer_from(from, to, value).map_err(|e| e.into()) } - pub fn pause(&mut self) -> Result<(), Error> { - self.pausable.pause() + pub fn pause(&mut self) -> Result<(), Vec> { + self.pausable.pause().map_err(|e| e.into()) } - pub fn unpause(&mut self) -> Result<(), Error> { - self.pausable.unpause() + pub fn unpause(&mut self) -> Result<(), Vec> { + self.pausable.unpause().map_err(|e| e.into()) } } diff --git a/examples/erc721/src/lib.rs b/examples/erc721/src/lib.rs index 5046c99a..695f4fb8 100644 --- a/examples/erc721/src/lib.rs +++ b/examples/erc721/src/lib.rs @@ -9,7 +9,7 @@ use openzeppelin_stylus::{ extensions::{Erc721Enumerable as Enumerable, IErc721Burnable}, Erc721, IErc721, }, - utils::{pausable::Error, Pausable}, + utils::Pausable, }; use stylus_sdk::{ abi::Bytes, @@ -152,11 +152,11 @@ impl Erc721Example { Ok(()) } - pub fn pause(&mut self) -> Result<(), Error> { - self.pausable.pause() + pub fn pause(&mut self) -> Result<(), Vec> { + self.pausable.pause().map_err(|e| e.into()) } - pub fn unpause(&mut self) -> Result<(), Error> { - self.pausable.unpause() + pub fn unpause(&mut self) -> Result<(), Vec> { + self.pausable.unpause().map_err(|e| e.into()) } } From ad7b9b20a548ac33496a1604e2b57bbb3b2c1a7c Mon Sep 17 00:00:00 2001 From: Alisander Qoshqosh Date: Fri, 4 Oct 2024 23:44:03 +0400 Subject: [PATCH 4/4] ++ --- contracts/src/utils/pausable.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/src/utils/pausable.rs b/contracts/src/utils/pausable.rs index 34a514fb..afdf8bd1 100644 --- a/contracts/src/utils/pausable.rs +++ b/contracts/src/utils/pausable.rs @@ -8,8 +8,7 @@ //! which can be added to the functions of your contract. //! //! Note that they will not be pausable by simply including this module, -//! only once functions [`Pausable::when_not_paused`] and -//! [`Pausable::when_paused`] are put in place. +//! only once function [`Pausable::when_not_paused`] is put in place. //! //! Note that [`Pausable::pause`] and [`Pausable::unpause`] methods are not //! exposed by default.