feat: add Errors modules (#691)

OpenZeppelin · Sep 14, 2023 · fcdd6d1 · fcdd6d1
1 parent 90be39f
commit fcdd6d1
Show file tree

Hide file tree

Showing 9 changed files with 96 additions and 39 deletions.
diff --git a/src/access/accesscontrol/accesscontrol.cairo b/src/access/accesscontrol/accesscontrol.cairo
@@ -58,6 +58,11 @@ mod AccessControl {
         new_admin_role: felt252
     }
 
+    mod Errors {
+        const INVALID_CALLER: felt252 = 'Can only renounce role for self';
+        const MISSING_ROLE: felt252 = 'Caller is missing role';
+    }
+
     #[external(v0)]
     impl SRC5Impl of ISRC5<ContractState> {
         fn supports_interface(self: @ContractState, interface_id: felt252) -> bool {
@@ -98,7 +103,7 @@ mod AccessControl {
 
         fn renounce_role(ref self: ContractState, role: felt252, account: ContractAddress) {
             let caller: ContractAddress = get_caller_address();
-            assert(caller == account, 'Can only renounce role for self');
+            assert(caller == account, Errors::INVALID_CALLER);
             self._revoke_role(role, account);
         }
     }
@@ -140,7 +145,7 @@ mod AccessControl {
         fn assert_only_role(self: @ContractState, role: felt252) {
             let caller: ContractAddress = get_caller_address();
             let authorized: bool = AccessControlImpl::has_role(self, role, caller);
-            assert(authorized, 'Caller is missing role');
+            assert(authorized, Errors::MISSING_ROLE);
         }
 
         fn _grant_role(ref self: ContractState, role: felt252, account: ContractAddress) {

diff --git a/src/access/ownable/ownable.cairo b/src/access/ownable/ownable.cairo
@@ -25,6 +25,12 @@ mod Ownable {
         new_owner: ContractAddress,
     }
 
+    mod Errors {
+        const NOT_OWNER: felt252 = 'Caller is not the owner';
+        const ZERO_ADDRESS_CALLER: felt252 = 'Caller is the zero address';
+        const ZERO_ADDRESS_OWNER: felt252 = 'New owner is the zero address';
+    }
+
     #[generate_trait]
     impl InternalImpl of InternalTrait {
         fn initializer(ref self: ContractState, owner: ContractAddress) {
@@ -34,8 +40,8 @@ mod Ownable {
         fn assert_only_owner(self: @ContractState) {
             let owner: ContractAddress = self._owner.read();
             let caller: ContractAddress = get_caller_address();
-            assert(!caller.is_zero(), 'Caller is the zero address');
-            assert(caller == owner, 'Caller is not the owner');
+            assert(!caller.is_zero(), Errors::ZERO_ADDRESS_CALLER);
+            assert(caller == owner, Errors::NOT_OWNER);
         }
 
         fn _transfer_ownership(ref self: ContractState, new_owner: ContractAddress) {
@@ -55,7 +61,7 @@ mod Ownable {
         }
 
         fn transfer_ownership(ref self: ContractState, new_owner: ContractAddress) {
-            assert(!new_owner.is_zero(), 'New owner is the zero address');
+            assert(!new_owner.is_zero(), Errors::ZERO_ADDRESS_OWNER);
             self.assert_only_owner();
             self._transfer_ownership(new_owner);
         }

diff --git a/src/account/account.cairo b/src/account/account.cairo
@@ -66,6 +66,13 @@ mod Account {
         removed_owner_guid: felt252
     }
 
+    mod Errors {
+        const INVALID_CALLER: felt252 = 'Account: invalid caller';
+        const INVALID_SIGNATURE: felt252 = 'Account: invalid signature';
+        const INVALID_TX_VERSION: felt252 = 'Account: invalid tx version';
+        const UNAUTHORIZED: felt252 = 'Account: unauthorized';
+    }
+
     #[constructor]
     fn constructor(ref self: ContractState, _public_key: felt252) {
         self.initializer(_public_key);
@@ -81,13 +88,13 @@ mod Account {
             // Avoid calls from other contracts
             // https://github.com/OpenZeppelin/cairo-contracts/issues/344
             let sender = get_caller_address();
-            assert(sender.is_zero(), 'Account: invalid caller');
+            assert(sender.is_zero(), Errors::INVALID_CALLER);
 
             // Check tx version
             let tx_info = get_tx_info().unbox();
             let version = tx_info.version;
             if version != TRANSACTION_VERSION {
-                assert(version == QUERY_VERSION, 'Account: invalid tx version');
+                assert(version == QUERY_VERSION, Errors::INVALID_TX_VERSION);
             }
 
             _execute_calls(calls)
@@ -190,7 +197,7 @@ mod Account {
             let tx_info = get_tx_info().unbox();
             let tx_hash = tx_info.transaction_hash;
             let signature = tx_info.signature;
-            assert(self._is_valid_signature(tx_hash, signature), 'Account: invalid signature');
+            assert(self._is_valid_signature(tx_hash, signature), Errors::INVALID_SIGNATURE);
             starknet::VALIDATED
         }
 
@@ -218,7 +225,7 @@ mod Account {
     fn assert_only_self() {
         let caller = get_caller_address();
         let self = get_contract_address();
-        assert(self == caller, 'Account: unauthorized');
+        assert(self == caller, Errors::UNAUTHORIZED);
     }
 
     #[internal]

diff --git a/src/introspection/src5.cairo b/src/introspection/src5.cairo
@@ -10,6 +10,10 @@ mod SRC5 {
         supported_interfaces: LegacyMap<felt252, bool>
     }
 
+    mod Errors {
+        const INVALID_ID: felt252 = 'SRC5: invalid id';
+    }
+
     #[external(v0)]
     impl SRC5Impl of interface::ISRC5<ContractState> {
         fn supports_interface(self: @ContractState, interface_id: felt252) -> bool {
@@ -34,7 +38,7 @@ mod SRC5 {
         }
 
         fn deregister_interface(ref self: ContractState, interface_id: felt252) {
-            assert(interface_id != interface::ISRC5_ID, 'SRC5: invalid id');
+            assert(interface_id != interface::ISRC5_ID, Errors::INVALID_ID);
             self.supported_interfaces.write(interface_id, false);
         }
     }

diff --git a/src/security/initializable.cairo b/src/security/initializable.cairo
@@ -8,14 +8,18 @@ mod Initializable {
         initialized: bool
     }
 
+    mod Errors {
+        const INITIALIZED: felt252 = 'Initializable: is initialized';
+    }
+
     #[generate_trait]
     impl InternalImpl of InternalTrait {
         fn is_initialized(self: @ContractState) -> bool {
             self.initialized.read()
         }
 
         fn initialize(ref self: ContractState) {
-            assert(!self.is_initialized(), 'Initializable: is initialized');
+            assert(!self.is_initialized(), Errors::INITIALIZED);
             self.initialized.write(true);
         }
     }

diff --git a/src/security/pausable.cairo b/src/security/pausable.cairo
@@ -22,15 +22,22 @@ mod Pausable {
         Paused: Paused,
         Unpaused: Unpaused,
     }
+
     #[derive(Drop, starknet::Event)]
     struct Paused {
         account: ContractAddress
     }
+
     #[derive(Drop, starknet::Event)]
     struct Unpaused {
         account: ContractAddress
     }
 
+    mod Errors {
+        const PAUSED: felt252 = 'Pausable: paused';
+        const NOT_PAUSED: felt252 = 'Pausable: not paused';
+    }
+
     #[external(v0)]
     impl PausableImpl of super::IPausable<ContractState> {
         fn is_paused(self: @ContractState) -> bool {
@@ -41,11 +48,11 @@ mod Pausable {
     #[generate_trait]
     impl InternalImpl of InternalTrait {
         fn assert_not_paused(self: @ContractState) {
-            assert(!self.paused.read(), 'Pausable: paused');
+            assert(!self.paused.read(), Errors::PAUSED);
         }
 
         fn assert_paused(self: @ContractState) {
-            assert(self.paused.read(), 'Pausable: not paused');
+            assert(self.paused.read(), Errors::NOT_PAUSED);
         }
 
         fn _pause(ref self: ContractState) {

diff --git a/src/security/reentrancyguard.cairo b/src/security/reentrancyguard.cairo
@@ -10,10 +10,14 @@ mod ReentrancyGuard {
         entered: bool
     }
 
+    mod Errors {
+        const REENTRANT_CALL: felt252 = 'ReentrancyGuard: reentrant call';
+    }
+
     #[generate_trait]
     impl InternalImpl of InternalTrait {
         fn start(ref self: ContractState) {
-            assert(!self.entered.read(), 'ReentrancyGuard: reentrant call');
+            assert(!self.entered.read(), Errors::REENTRANT_CALL);
             self.entered.write(true);
         }
 

diff --git a/src/token/erc20/erc20.cairo b/src/token/erc20/erc20.cairo
@@ -40,6 +40,15 @@ mod ERC20 {
         value: u256
     }
 
+    mod Errors {
+        const APPROVE_FROM_ZERO: felt252 = 'ERC20: approve from 0';
+        const APPROVE_TO_ZERO: felt252 = 'ERC20: approve to 0';
+        const TRANSFER_FROM_ZERO: felt252 = 'ERC20: transfer from 0';
+        const TRANSFER_TO_ZERO: felt252 = 'ERC20: transfer to 0';
+        const BURN_FROM_ZERO: felt252 = 'ERC20: burn from 0';
+        const MINT_TO_ZERO: felt252 = 'ERC20: mint to 0';
+    }
+
     #[constructor]
     fn constructor(
         ref self: ContractState,
@@ -188,14 +197,14 @@ mod ERC20 {
         }
 
         fn _mint(ref self: ContractState, recipient: ContractAddress, amount: u256) {
-            assert(!recipient.is_zero(), 'ERC20: mint to 0');
+            assert(!recipient.is_zero(), Errors::MINT_TO_ZERO);
             self._total_supply.write(self._total_supply.read() + amount);
             self._balances.write(recipient, self._balances.read(recipient) + amount);
             self.emit(Transfer { from: Zeroable::zero(), to: recipient, value: amount });
         }
 
         fn _burn(ref self: ContractState, account: ContractAddress, amount: u256) {
-            assert(!account.is_zero(), 'ERC20: burn from 0');
+            assert(!account.is_zero(), Errors::BURN_FROM_ZERO);
             self._total_supply.write(self._total_supply.read() - amount);
             self._balances.write(account, self._balances.read(account) - amount);
             self.emit(Transfer { from: account, to: Zeroable::zero(), value: amount });
@@ -204,8 +213,8 @@ mod ERC20 {
         fn _approve(
             ref self: ContractState, owner: ContractAddress, spender: ContractAddress, amount: u256
         ) {
-            assert(!owner.is_zero(), 'ERC20: approve from 0');
-            assert(!spender.is_zero(), 'ERC20: approve to 0');
+            assert(!owner.is_zero(), Errors::APPROVE_FROM_ZERO);
+            assert(!spender.is_zero(), Errors::APPROVE_TO_ZERO);
             self._allowances.write((owner, spender), amount);
             self.emit(Approval { owner, spender, value: amount });
         }
@@ -216,8 +225,8 @@ mod ERC20 {
             recipient: ContractAddress,
             amount: u256
         ) {
-            assert(!sender.is_zero(), 'ERC20: transfer from 0');
-            assert(!recipient.is_zero(), 'ERC20: transfer to 0');
+            assert(!sender.is_zero(), Errors::TRANSFER_FROM_ZERO);
+            assert(!recipient.is_zero(), Errors::TRANSFER_TO_ZERO);
             self._balances.write(sender, self._balances.read(sender) - amount);
             self._balances.write(recipient, self._balances.read(recipient) + amount);
             self.emit(Transfer { from: sender, to: recipient, value: amount });