From d55661a14d09087b0b81610bba775368dc48bd04 Mon Sep 17 00:00:00 2001 From: Andrew Fleming Date: Thu, 12 Oct 2023 18:31:48 -0400 Subject: [PATCH] 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 * Apply suggestions from code review Co-authored-by: Eric Nordelo * fix spdx and comment block * change _comp to _component * remove unnecessary import * fix comments --------- Co-authored-by: Eric Nordelo --- src/security/pausable.cairo | 36 +++++++++++++++----- src/tests/mocks.cairo | 1 + src/tests/mocks/pausable_mock.cairo | 22 ++++++++++++ src/tests/security/test_pausable.cairo | 46 +++++++++++++------------- 4 files changed, 73 insertions(+), 32 deletions(-) create mode 100644 src/tests/mocks/pausable_mock.cairo diff --git a/src/security/pausable.cairo b/src/security/pausable.cairo index b29bbf19d..b7904cbc8 100644 --- a/src/security/pausable.cairo +++ b/src/security/pausable.cairo @@ -6,7 +6,12 @@ trait IPausable { 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 { - fn is_paused(self: @ContractState) -> bool { + #[embeddable_as(PausableImpl)] + impl Pausable< + TContractState, +HasComponent + > of super::IPausable> { + /// Returns true if the contract is paused, and false otherwise. + fn is_paused(self: @ComponentState) -> bool { self.Pausable_paused.read() } } #[generate_trait] - impl InternalImpl of InternalTrait { - fn assert_not_paused(self: @ContractState) { + impl InternalImpl< + TContractState, +HasComponent + > of InternalTrait { + /// Makes a function only callable when the contract is not paused. + fn assert_not_paused(self: @ComponentState) { 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) { 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) { 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) { 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 index 7a43b2a72..e598542dd 100644 --- 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 new file mode 100644 index 000000000..5ffc5900a --- /dev/null +++ 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; + impl InternalImpl = pausable_component::InternalImpl; + + #[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 index f1aff256e..2044a841f 100644 --- 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,8 +42,8 @@ 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] @@ -51,7 +51,7 @@ fn test_assert_paused_when_paused() { #[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,10 +84,10 @@ 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] @@ -95,8 +95,8 @@ fn test_pause_when_unpaused() { #[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,13 +109,13 @@ 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] @@ -123,8 +123,8 @@ fn test_unpause_when_paused() { #[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(); } //