Migrate Pausable to component (#773)

* update Scarb * fix formatting * migrate pausable to component * fix formatting * add documentation * fix comments * Apply suggestions from code review Co-authored-by: Eric Nordelo <[email protected]> * Apply suggestions from code review Co-authored-by: Eric Nordelo <[email protected]> * fix spdx and comment block * change _comp to _component * remove unnecessary import * fix comments --------- Co-authored-by: Eric Nordelo <[email protected]>
OpenZeppelin · Oct 12, 2023 · d55661a · d55661a
1 parent 0b77977
commit d55661a
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 32 deletions.
diff --git a/src/security/pausable.cairo b/src/security/pausable.cairo
@@ -6,7 +6,12 @@ trait IPausable<TState> {
     fn is_paused(self: @TState) -> bool;
 }
 
-#[starknet::contract]
+/// # Pausable Component
+///
+/// The Pausable component allows the using contract to implement an
+/// emergency stop mechanism. Only functions that call `assert_paused`
+/// or `assert_not_paused` will be affected by this mechanism.
+#[starknet::component]
 mod Pausable {
     use starknet::ContractAddress;
     use starknet::get_caller_address;
@@ -23,11 +28,13 @@ mod Pausable {
         Unpaused: Unpaused,
     }
 
+    /// Emitted when the pause is triggered by `account`.
     #[derive(Drop, starknet::Event)]
     struct Paused {
         account: ContractAddress
     }
 
+    /// Emitted when the pause is lifted by `account`.
     #[derive(Drop, starknet::Event)]
     struct Unpaused {
         account: ContractAddress
@@ -38,30 +45,41 @@ mod Pausable {
         const NOT_PAUSED: felt252 = 'Pausable: not paused';
     }
 
-    #[external(v0)]
-    impl PausableImpl of super::IPausable<ContractState> {
-        fn is_paused(self: @ContractState) -> bool {
+    #[embeddable_as(PausableImpl)]
+    impl Pausable<
+        TContractState, +HasComponent<TContractState>
+    > of super::IPausable<ComponentState<TContractState>> {
+        /// Returns true if the contract is paused, and false otherwise.
+        fn is_paused(self: @ComponentState<TContractState>) -> bool {
             self.Pausable_paused.read()
         }
     }
 
     #[generate_trait]
-    impl InternalImpl of InternalTrait {
-        fn assert_not_paused(self: @ContractState) {
+    impl InternalImpl<
+        TContractState, +HasComponent<TContractState>
+    > of InternalTrait<TContractState> {
+        /// Makes a function only callable when the contract is not paused.
+        fn assert_not_paused(self: @ComponentState<TContractState>) {
             assert(!self.Pausable_paused.read(), Errors::PAUSED);
         }
 
-        fn assert_paused(self: @ContractState) {
+        /// Makes a function only callable when the contract is paused.
+        fn assert_paused(self: @ComponentState<TContractState>) {
             assert(self.Pausable_paused.read(), Errors::NOT_PAUSED);
         }
 
-        fn _pause(ref self: ContractState) {
+        /// Triggers a stopped state.
+        /// The contract must not already be paused.
+        fn _pause(ref self: ComponentState<TContractState>) {
             self.assert_not_paused();
             self.Pausable_paused.write(true);
             self.emit(Paused { account: get_caller_address() });
         }
 
-        fn _unpause(ref self: ContractState) {
+        /// Lifts the pause on the contract.
+        /// The contract must already be paused.
+        fn _unpause(ref self: ComponentState<TContractState>) {
             self.assert_paused();
             self.Pausable_paused.write(false);
             self.emit(Unpaused { account: get_caller_address() });

diff --git a/src/tests/mocks.cairo b/src/tests/mocks.cairo
@@ -11,6 +11,7 @@ mod erc721_receiver;
 mod initializable_mock;
 mod non_implementing_mock;
 mod ownable_mocks;
+mod pausable_mock;
 mod reentrancy_attacker_mock;
 mod reentrancy_mock;
 mod snake20_mock;

diff --git a/src/tests/mocks/pausable_mock.cairo b/src/tests/mocks/pausable_mock.cairo
@@ -0,0 +1,22 @@
+#[starknet::contract]
+mod PausableMock {
+    use openzeppelin::security::pausable::Pausable as pausable_component;
+
+    component!(path: pausable_component, storage: pausable, event: PausableEvent);
+
+    #[abi(embed_v0)]
+    impl PausableImpl = pausable_component::PausableImpl<ContractState>;
+    impl InternalImpl = pausable_component::InternalImpl<ContractState>;
+
+    #[storage]
+    struct Storage {
+        #[substorage(v0)]
+        pausable: pausable_component::Storage
+    }
+
+    #[event]
+    #[derive(Drop, starknet::Event)]
+    enum Event {
+        PausableEvent: pausable_component::Event
+    }
+}
diff --git a/src/tests/security/test_pausable.cairo b/src/tests/security/test_pausable.cairo
@@ -2,7 +2,7 @@ use openzeppelin::security::pausable::Pausable::InternalImpl;
 use openzeppelin::security::pausable::Pausable::PausableImpl;
 use openzeppelin::security::pausable::Pausable::Paused;
 use openzeppelin::security::pausable::Pausable::Unpaused;
-use openzeppelin::security::pausable::Pausable;
+use openzeppelin::tests::mocks::pausable_mock::PausableMock;
 use openzeppelin::tests::utils::constants::{CALLER, ZERO};
 use openzeppelin::tests::utils;
 use starknet::ContractAddress;
@@ -13,8 +13,8 @@ use starknet::testing;
 // Setup
 //
 
-fn STATE() -> Pausable::ContractState {
-    Pausable::contract_state_for_testing()
+fn STATE() -> PausableMock::ContractState {
+    PausableMock::contract_state_for_testing()
 }
 
 //
@@ -25,13 +25,13 @@ fn STATE() -> Pausable::ContractState {
 #[available_gas(2000000)]
 fn test_is_paused() {
     let mut state = STATE();
-    assert(!PausableImpl::is_paused(@state), 'Should not be paused');
+    assert(!state.pausable.is_paused(), 'Should not be paused');
 
-    InternalImpl::_pause(ref state);
-    assert(PausableImpl::is_paused(@state), 'Should be paused');
+    state.pausable._pause();
+    assert(state.pausable.is_paused(), 'Should be paused');
 
-    InternalImpl::_unpause(ref state);
-    assert(!PausableImpl::is_paused(@state), 'Should not be paused');
+    state.pausable._unpause();
+    assert(!state.pausable.is_paused(), 'Should not be paused');
 }
 
 //
@@ -42,16 +42,16 @@ fn test_is_paused() {
 #[available_gas(2000000)]
 fn test_assert_paused_when_paused() {
     let mut state = STATE();
-    InternalImpl::_pause(ref state);
-    InternalImpl::assert_paused(@state);
+    state.pausable._pause();
+    state.pausable.assert_paused();
 }
 
 #[test]
 #[available_gas(2000000)]
 #[should_panic(expected: ('Pausable: not paused',))]
 fn test_assert_paused_when_not_paused() {
     let state = STATE();
-    InternalImpl::assert_paused(@state);
+    state.pausable.assert_paused();
 }
 
 //
@@ -63,15 +63,15 @@ fn test_assert_paused_when_not_paused() {
 #[should_panic(expected: ('Pausable: paused',))]
 fn test_assert_not_paused_when_paused() {
     let mut state = STATE();
-    InternalImpl::_pause(ref state);
-    InternalImpl::assert_not_paused(@state);
+    state.pausable._pause();
+    state.pausable.assert_not_paused();
 }
 
 #[test]
 #[available_gas(2000000)]
 fn test_assert_not_paused_when_not_paused() {
     let state = STATE();
-    InternalImpl::assert_not_paused(@state);
+    state.pausable.assert_not_paused();
 }
 
 //
@@ -84,19 +84,19 @@ fn test_pause_when_unpaused() {
     let mut state = STATE();
     testing::set_caller_address(CALLER());
 
-    InternalImpl::_pause(ref state);
+    state.pausable._pause();
 
     assert_event_paused(CALLER());
-    assert(PausableImpl::is_paused(@state), 'Should be paused');
+    assert(state.pausable.is_paused(), 'Should be paused');
 }
 
 #[test]
 #[available_gas(2000000)]
 #[should_panic(expected: ('Pausable: paused',))]
 fn test_pause_when_paused() {
     let mut state = STATE();
-    InternalImpl::_pause(ref state);
-    InternalImpl::_pause(ref state);
+    state.pausable._pause();
+    state.pausable._pause();
 }
 
 //
@@ -109,22 +109,22 @@ fn test_unpause_when_paused() {
     let mut state = STATE();
     testing::set_caller_address(CALLER());
 
-    InternalImpl::_pause(ref state);
+    state.pausable._pause();
     utils::drop_event(ZERO());
 
-    InternalImpl::_unpause(ref state);
+    state.pausable._unpause();
 
     assert_event_unpaused(CALLER());
-    assert(!PausableImpl::is_paused(@state), 'Should not be paused');
+    assert(!state.pausable.is_paused(), 'Should not be paused');
 }
 
 #[test]
 #[available_gas(2000000)]
 #[should_panic(expected: ('Pausable: not paused',))]
 fn test_unpause_when_unpaused() {
     let mut state = STATE();
-    assert(!PausableImpl::is_paused(@state), 'Should be paused');
-    InternalImpl::_unpause(ref state);
+    assert(!state.pausable.is_paused(), 'Should be paused');
+    state.pausable._unpause();
 }
 
 //