From fcdd6d1b02306f18f1089656f15b87d1701ffde3 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 14 Sep 2023 07:08:21 +0200 Subject: [PATCH] feat: add Errors modules (#691) --- src/access/accesscontrol/accesscontrol.cairo | 9 +++- src/access/ownable/ownable.cairo | 12 +++-- src/account/account.cairo | 15 ++++-- src/introspection/src5.cairo | 6 ++- src/security/initializable.cairo | 6 ++- src/security/pausable.cairo | 11 ++++- src/security/reentrancyguard.cairo | 6 ++- src/token/erc20/erc20.cairo | 21 ++++++--- src/token/erc721/erc721.cairo | 49 ++++++++++++-------- 9 files changed, 96 insertions(+), 39 deletions(-) diff --git a/src/access/accesscontrol/accesscontrol.cairo b/src/access/accesscontrol/accesscontrol.cairo index 718137ecf..90dfe421c 100644 --- 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 { 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 index b249d7528..ee621afa4 100644 --- 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 index 3bda0e34a..ce2eb8c2f 100644 --- 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 index 75ad1d3b2..51a4e6720 100644 --- a/src/introspection/src5.cairo +++ b/src/introspection/src5.cairo @@ -10,6 +10,10 @@ mod SRC5 { supported_interfaces: LegacyMap } + mod Errors { + const INVALID_ID: felt252 = 'SRC5: invalid id'; + } + #[external(v0)] impl SRC5Impl of interface::ISRC5 { 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 index e0e7e658f..934813276 100644 --- a/src/security/initializable.cairo +++ b/src/security/initializable.cairo @@ -8,6 +8,10 @@ mod Initializable { initialized: bool } + mod Errors { + const INITIALIZED: felt252 = 'Initializable: is initialized'; + } + #[generate_trait] impl InternalImpl of InternalTrait { fn is_initialized(self: @ContractState) -> bool { @@ -15,7 +19,7 @@ mod Initializable { } 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 index 73480add7..7441891a8 100644 --- 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 { 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 index f08713776..d6a218c62 100644 --- 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 index 9448b8ef3..df5d1fd25 100644 --- 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 }); diff --git a/src/token/erc721/erc721.cairo b/src/token/erc721/erc721.cairo index 0abffb4d1..559b57c9c 100644 --- a/src/token/erc721/erc721.cairo +++ b/src/token/erc721/erc721.cairo @@ -60,6 +60,19 @@ mod ERC721 { approved: bool } + mod Errors { + const INVALID_TOKEN_ID: felt252 = 'ERC721: invalid token ID'; + const INVALID_ACCOUNT: felt252 = 'ERC721: invalid account'; + const UNAUTHORIZED: felt252 = 'ERC721: unauthorized caller'; + const APPROVAL_TO_OWNER: felt252 = 'ERC721: approval to owner'; + const SELF_APPROVAL: felt252 = 'ERC721: self approval'; + const INVALID_RECEIVER: felt252 = 'ERC721: invalid receiver'; + const ALREADY_MINTED: felt252 = 'ERC721: token already minted'; + const WRONG_SENDER: felt252 = 'ERC721: wrong sender'; + const SAFE_MINT_FAILED: felt252 = 'ERC721: safe mint failed'; + const SAFE_TRANSFER_FAILED: felt252 = 'ERC721: safe transfer failed'; + } + #[constructor] fn constructor( ref self: ContractState, @@ -103,7 +116,7 @@ mod ERC721 { } fn token_uri(self: @ContractState, token_id: u256) -> felt252 { - assert(self._exists(token_id), 'ERC721: invalid token ID'); + assert(self._exists(token_id), Errors::INVALID_TOKEN_ID); self._token_uri.read(token_id) } } @@ -111,7 +124,7 @@ mod ERC721 { #[external(v0)] impl ERC721MetadataCamelOnlyImpl of interface::IERC721MetadataCamelOnly { fn tokenURI(self: @ContractState, tokenId: u256) -> felt252 { - assert(self._exists(tokenId), 'ERC721: invalid token ID'); + assert(self._exists(tokenId), Errors::INVALID_TOKEN_ID); self._token_uri.read(tokenId) } } @@ -119,7 +132,7 @@ mod ERC721 { #[external(v0)] impl ERC721Impl of interface::IERC721 { fn balance_of(self: @ContractState, account: ContractAddress) -> u256 { - assert(!account.is_zero(), 'ERC721: invalid account'); + assert(!account.is_zero(), Errors::INVALID_ACCOUNT); self._balances.read(account) } @@ -128,7 +141,7 @@ mod ERC721 { } fn get_approved(self: @ContractState, token_id: u256) -> ContractAddress { - assert(self._exists(token_id), 'ERC721: invalid token ID'); + assert(self._exists(token_id), Errors::INVALID_TOKEN_ID); self._token_approvals.read(token_id) } @@ -144,7 +157,7 @@ mod ERC721 { let caller = get_caller_address(); assert( owner == caller || ERC721Impl::is_approved_for_all(@self, owner, caller), - 'ERC721: unauthorized caller' + Errors::UNAUTHORIZED ); self._approve(to, token_id); } @@ -159,8 +172,7 @@ mod ERC721 { ref self: ContractState, from: ContractAddress, to: ContractAddress, token_id: u256 ) { assert( - self._is_approved_or_owner(get_caller_address(), token_id), - 'ERC721: unauthorized caller' + self._is_approved_or_owner(get_caller_address(), token_id), Errors::UNAUTHORIZED ); self._transfer(from, to, token_id); } @@ -173,8 +185,7 @@ mod ERC721 { data: Span ) { assert( - self._is_approved_or_owner(get_caller_address(), token_id), - 'ERC721: unauthorized caller' + self._is_approved_or_owner(get_caller_address(), token_id), Errors::UNAUTHORIZED ); self._safe_transfer(from, to, token_id, data); } @@ -242,7 +253,7 @@ mod ERC721 { let owner = self._owners.read(token_id); match owner.is_zero() { bool::False(()) => owner, - bool::True(()) => panic_with_felt252('ERC721: invalid token ID') + bool::True(()) => panic_with_felt252(Errors::INVALID_TOKEN_ID) } } @@ -262,7 +273,7 @@ mod ERC721 { fn _approve(ref self: ContractState, to: ContractAddress, token_id: u256) { let owner = self._owner_of(token_id); - assert(owner != to, 'ERC721: approval to owner'); + assert(owner != to, Errors::APPROVAL_TO_OWNER); self._token_approvals.write(token_id, to); self.emit(Approval { owner, approved: to, token_id }); @@ -274,14 +285,14 @@ mod ERC721 { operator: ContractAddress, approved: bool ) { - assert(owner != operator, 'ERC721: self approval'); + assert(owner != operator, Errors::SELF_APPROVAL); self._operator_approvals.write((owner, operator), approved); self.emit(ApprovalForAll { owner, operator, approved }); } fn _mint(ref self: ContractState, to: ContractAddress, token_id: u256) { - assert(!to.is_zero(), 'ERC721: invalid receiver'); - assert(!self._exists(token_id), 'ERC721: token already minted'); + assert(!to.is_zero(), Errors::INVALID_RECEIVER); + assert(!self._exists(token_id), Errors::ALREADY_MINTED); self._balances.write(to, self._balances.read(to) + 1); self._owners.write(token_id, to); @@ -292,9 +303,9 @@ mod ERC721 { fn _transfer( ref self: ContractState, from: ContractAddress, to: ContractAddress, token_id: u256 ) { - assert(!to.is_zero(), 'ERC721: invalid receiver'); + assert(!to.is_zero(), Errors::INVALID_RECEIVER); let owner = self._owner_of(token_id); - assert(from == owner, 'ERC721: wrong sender'); + assert(from == owner, Errors::WRONG_SENDER); // Implicit clear approvals, no need to emit an event self._token_approvals.write(token_id, Zeroable::zero()); @@ -324,7 +335,7 @@ mod ERC721 { self._mint(to, token_id); assert( _check_on_erc721_received(Zeroable::zero(), to, token_id, data), - 'ERC721: safe mint failed' + Errors::SAFE_MINT_FAILED ); } @@ -337,12 +348,12 @@ mod ERC721 { ) { self._transfer(from, to, token_id); assert( - _check_on_erc721_received(from, to, token_id, data), 'ERC721: safe transfer failed' + _check_on_erc721_received(from, to, token_id, data), Errors::SAFE_TRANSFER_FAILED ); } fn _set_token_uri(ref self: ContractState, token_id: u256, token_uri: felt252) { - assert(self._exists(token_id), 'ERC721: invalid token ID'); + assert(self._exists(token_id), Errors::INVALID_TOKEN_ID); self._token_uri.write(token_id, token_uri) } }