Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Errors tracking #691

Merged
merged 2 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/access/accesscontrol/accesscontrol.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 9 additions & 3 deletions src/access/ownable/ownable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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);
}
Expand Down
15 changes: 11 additions & 4 deletions src/account/account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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]
Expand Down
6 changes: 5 additions & 1 deletion src/introspection/src5.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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);
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/security/initializable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/security/pausable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion src/security/reentrancyguard.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
21 changes: 15 additions & 6 deletions src/token/erc20/erc20.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 });
Expand All @@ -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 });
}
Expand All @@ -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 });
Expand Down
Loading