Skip to content

Commit

Permalink
Add prefixes to storage members (#743)
Browse files Browse the repository at this point in the history
* feat: add prefixes

* Update src/access/accesscontrol/accesscontrol.cairo

Co-authored-by: Andrew Fleming <[email protected]>

---------

Co-authored-by: Andrew Fleming <[email protected]>
  • Loading branch information
ericnordelo and andrew-fleming authored Sep 26, 2023
1 parent a1b559c commit 74eb4e8
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 103 deletions.
14 changes: 7 additions & 7 deletions src/access/accesscontrol/accesscontrol.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ mod AccessControl {

#[storage]
struct Storage {
role_admin: LegacyMap<felt252, felt252>,
role_members: LegacyMap<(felt252, ContractAddress), bool>,
AccessControl_role_admin: LegacyMap<felt252, felt252>,
AccessControl_role_member: LegacyMap<(felt252, ContractAddress), bool>,
}

#[event]
Expand Down Expand Up @@ -82,11 +82,11 @@ mod AccessControl {
#[external(v0)]
impl AccessControlImpl of interface::IAccessControl<ContractState> {
fn has_role(self: @ContractState, role: felt252, account: ContractAddress) -> bool {
self.role_members.read((role, account))
self.AccessControl_role_member.read((role, account))
}

fn get_role_admin(self: @ContractState, role: felt252) -> felt252 {
self.role_admin.read(role)
self.AccessControl_role_admin.read(role)
}

fn grant_role(ref self: ContractState, role: felt252, account: ContractAddress) {
Expand Down Expand Up @@ -151,22 +151,22 @@ mod AccessControl {
fn _grant_role(ref self: ContractState, role: felt252, account: ContractAddress) {
if !AccessControlImpl::has_role(@self, role, account) {
let caller: ContractAddress = get_caller_address();
self.role_members.write((role, account), true);
self.AccessControl_role_member.write((role, account), true);
self.emit(RoleGranted { role, account, sender: caller });
}
}

fn _revoke_role(ref self: ContractState, role: felt252, account: ContractAddress) {
if AccessControlImpl::has_role(@self, role, account) {
let caller: ContractAddress = get_caller_address();
self.role_members.write((role, account), false);
self.AccessControl_role_member.write((role, account), false);
self.emit(RoleRevoked { role, account, sender: caller });
}
}

fn _set_role_admin(ref self: ContractState, role: felt252, admin_role: felt252) {
let previous_admin_role: felt252 = AccessControlImpl::get_role_admin(@self, role);
self.role_admin.write(role, admin_role);
self.AccessControl_role_admin.write(role, admin_role);
self.emit(RoleAdminChanged { role, previous_admin_role, new_admin_role: admin_role });
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/access/ownable/ownable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod Ownable {

#[storage]
struct Storage {
_owner: ContractAddress
Ownable_owner: ContractAddress
}

#[event]
Expand Down Expand Up @@ -38,15 +38,15 @@ mod Ownable {
}

fn assert_only_owner(self: @ContractState) {
let owner: ContractAddress = self._owner.read();
let owner: ContractAddress = self.Ownable_owner.read();
let caller: ContractAddress = get_caller_address();
assert(!caller.is_zero(), Errors::ZERO_ADDRESS_CALLER);
assert(caller == owner, Errors::NOT_OWNER);
}

fn _transfer_ownership(ref self: ContractState, new_owner: ContractAddress) {
let previous_owner: ContractAddress = self._owner.read();
self._owner.write(new_owner);
let previous_owner: ContractAddress = self.Ownable_owner.read();
self.Ownable_owner.write(new_owner);
self
.emit(
OwnershipTransferred { previous_owner: previous_owner, new_owner: new_owner }
Expand All @@ -57,7 +57,7 @@ mod Ownable {
#[external(v0)]
impl OwnableImpl of interface::IOwnable<ContractState> {
fn owner(self: @ContractState) -> ContractAddress {
self._owner.read()
self.Ownable_owner.read()
}

fn transfer_ownership(ref self: ContractState, new_owner: ContractAddress) {
Expand Down
12 changes: 6 additions & 6 deletions src/account/account.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ mod Account {

#[storage]
struct Storage {
public_key: felt252
Account_public_key: felt252
}

#[event]
Expand Down Expand Up @@ -150,20 +150,20 @@ mod Account {
#[external(v0)]
impl PublicKeyImpl of super::PublicKeyTrait<ContractState> {
fn get_public_key(self: @ContractState) -> felt252 {
self.public_key.read()
self.Account_public_key.read()
}

fn set_public_key(ref self: ContractState, new_public_key: felt252) {
assert_only_self();
self.emit(OwnerRemoved { removed_owner_guid: self.public_key.read() });
self.emit(OwnerRemoved { removed_owner_guid: self.Account_public_key.read() });
self._set_public_key(new_public_key);
}
}

#[external(v0)]
impl PublicKeyCamelImpl of super::PublicKeyCamelTrait<ContractState> {
fn getPublicKey(self: @ContractState) -> felt252 {
self.public_key.read()
self.Account_public_key.read()
}

fn setPublicKey(ref self: ContractState, newPublicKey: felt252) {
Expand Down Expand Up @@ -202,7 +202,7 @@ mod Account {
}

fn _set_public_key(ref self: ContractState, new_public_key: felt252) {
self.public_key.write(new_public_key);
self.Account_public_key.write(new_public_key);
self.emit(OwnerAdded { new_owner_guid: new_public_key });
}

Expand All @@ -213,7 +213,7 @@ mod Account {

if valid_length {
check_ecdsa_signature(
hash, self.public_key.read(), *signature.at(0_u32), *signature.at(1_u32)
hash, self.Account_public_key.read(), *signature.at(0_u32), *signature.at(1_u32)
)
} else {
false
Expand Down
8 changes: 4 additions & 4 deletions src/introspection/src5.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod SRC5 {

#[storage]
struct Storage {
supported_interfaces: LegacyMap<felt252, bool>
SRC5_supported_interfaces: LegacyMap<felt252, bool>
}

mod Errors {
Expand All @@ -20,7 +20,7 @@ mod SRC5 {
if interface_id == interface::ISRC5_ID {
return true;
}
self.supported_interfaces.read(interface_id)
self.SRC5_supported_interfaces.read(interface_id)
}
}

Expand All @@ -34,12 +34,12 @@ mod SRC5 {
#[generate_trait]
impl InternalImpl of InternalTrait {
fn register_interface(ref self: ContractState, interface_id: felt252) {
self.supported_interfaces.write(interface_id, true);
self.SRC5_supported_interfaces.write(interface_id, true);
}

fn deregister_interface(ref self: ContractState, interface_id: felt252) {
assert(interface_id != interface::ISRC5_ID, Errors::INVALID_ID);
self.supported_interfaces.write(interface_id, false);
self.SRC5_supported_interfaces.write(interface_id, false);
}
}
}
6 changes: 3 additions & 3 deletions src/security/initializable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
mod Initializable {
#[storage]
struct Storage {
initialized: bool
Initializable_initialized: bool
}

mod Errors {
Expand All @@ -15,12 +15,12 @@ mod Initializable {
#[generate_trait]
impl InternalImpl of InternalTrait {
fn is_initialized(self: @ContractState) -> bool {
self.initialized.read()
self.Initializable_initialized.read()
}

fn initialize(ref self: ContractState) {
assert(!self.is_initialized(), Errors::INITIALIZED);
self.initialized.write(true);
self.Initializable_initialized.write(true);
}
}
}
12 changes: 6 additions & 6 deletions src/security/pausable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod Pausable {

#[storage]
struct Storage {
paused: bool
Pausable_paused: bool
}

#[event]
Expand Down Expand Up @@ -41,29 +41,29 @@ mod Pausable {
#[external(v0)]
impl PausableImpl of super::IPausable<ContractState> {
fn is_paused(self: @ContractState) -> bool {
self.paused.read()
self.Pausable_paused.read()
}
}

#[generate_trait]
impl InternalImpl of InternalTrait {
fn assert_not_paused(self: @ContractState) {
assert(!self.paused.read(), Errors::PAUSED);
assert(!self.Pausable_paused.read(), Errors::PAUSED);
}

fn assert_paused(self: @ContractState) {
assert(self.paused.read(), Errors::NOT_PAUSED);
assert(self.Pausable_paused.read(), Errors::NOT_PAUSED);
}

fn _pause(ref self: ContractState) {
self.assert_not_paused();
self.paused.write(true);
self.Pausable_paused.write(true);
self.emit(Paused { account: get_caller_address() });
}

fn _unpause(ref self: ContractState) {
self.assert_paused();
self.paused.write(false);
self.Pausable_paused.write(false);
self.emit(Unpaused { account: get_caller_address() });
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/security/reentrancyguard.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod ReentrancyGuard {

#[storage]
struct Storage {
entered: bool
ReentrancyGuard_entered: bool
}

mod Errors {
Expand All @@ -17,12 +17,12 @@ mod ReentrancyGuard {
#[generate_trait]
impl InternalImpl of InternalTrait {
fn start(ref self: ContractState) {
assert(!self.entered.read(), Errors::REENTRANT_CALL);
self.entered.write(true);
assert(!self.ReentrancyGuard_entered.read(), Errors::REENTRANT_CALL);
self.ReentrancyGuard_entered.write(true);
}

fn end(ref self: ContractState) {
self.entered.write(false);
self.ReentrancyGuard_entered.write(false);
}
}
}
8 changes: 4 additions & 4 deletions src/tests/access/test_ownable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use openzeppelin::access::ownable::Ownable::InternalImpl;
use openzeppelin::access::ownable::Ownable::OwnableCamelOnlyImpl;
use openzeppelin::access::ownable::Ownable::OwnableImpl;
use openzeppelin::access::ownable::Ownable::OwnershipTransferred;
use openzeppelin::access::ownable::Ownable::_owner::InternalContractMemberStateTrait;
use openzeppelin::access::ownable::Ownable::Ownable_owner::InternalContractMemberStateTrait;
use openzeppelin::access::ownable::Ownable;
use openzeppelin::tests::utils::constants::{ZERO, OTHER, OWNER};
use openzeppelin::tests::utils;
Expand Down Expand Up @@ -35,12 +35,12 @@ fn setup() -> Ownable::ContractState {
#[available_gas(2000000)]
fn test_initializer() {
let mut state = STATE();
assert(state._owner.read().is_zero(), 'Should be zero');
assert(state.Ownable_owner.read().is_zero(), 'Should be zero');
InternalImpl::initializer(ref state, OWNER());

assert_event_ownership_transferred(ZERO(), OWNER());

assert(state._owner.read() == OWNER(), 'Owner should be set');
assert(state.Ownable_owner.read() == OWNER(), 'Owner should be set');
}

//
Expand Down Expand Up @@ -84,7 +84,7 @@ fn test__transfer_ownership() {

assert_event_ownership_transferred(OWNER(), OTHER());

assert(state._owner.read() == OTHER(), 'Owner should be OTHER');
assert(state.Ownable_owner.read() == OTHER(), 'Owner should be OTHER');
}

//
Expand Down
10 changes: 5 additions & 5 deletions src/tests/security/test_reentrancyguard.cairo
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use openzeppelin::security::reentrancyguard::ReentrancyGuard::InternalImpl;
use openzeppelin::security::reentrancyguard::ReentrancyGuard::entered::InternalContractMemberStateTrait;
use openzeppelin::security::reentrancyguard::ReentrancyGuard::ReentrancyGuard_entered::InternalContractMemberStateTrait;
use openzeppelin::security::reentrancyguard::ReentrancyGuard;
use openzeppelin::tests::mocks::reentrancy_attacker_mock::Attacker;
use openzeppelin::tests::mocks::reentrancy_mock::IReentrancyMockDispatcher;
Expand All @@ -26,9 +26,9 @@ fn deploy_mock() -> IReentrancyMockDispatcher {
fn test_reentrancy_guard_start() {
let mut state = STATE();

assert(!state.entered.read(), 'Should not be entered');
assert(!state.ReentrancyGuard_entered.read(), 'Should not be entered');
InternalImpl::start(ref state);
assert(state.entered.read(), 'Should be entered');
assert(state.ReentrancyGuard_entered.read(), 'Should be entered');
}

#[test]
Expand All @@ -47,9 +47,9 @@ fn test_reentrancy_guard_end() {
let mut state = STATE();

InternalImpl::start(ref state);
assert(state.entered.read(), 'Should be entered');
assert(state.ReentrancyGuard_entered.read(), 'Should be entered');
InternalImpl::end(ref state);
assert(!state.entered.read(), 'Should no longer be entered');
assert(!state.ReentrancyGuard_entered.read(), 'Should no longer be entered');
}

//
Expand Down
14 changes: 7 additions & 7 deletions src/tests/token/test_erc721.cairo
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use ERC721::_owners::InternalContractMemberStateTrait as OwnersTrait;
use ERC721::_token_approvals::InternalContractMemberStateTrait as TokenApprovalsTrait;
use ERC721::ERC721_owners::InternalContractMemberStateTrait as OwnersTrait;
use ERC721::ERC721_token_approvals::InternalContractMemberStateTrait as TokenApprovalsTrait;

use array::ArrayTrait;
use integer::u256;
Expand Down Expand Up @@ -191,17 +191,17 @@ fn test__exists() {
let token_id = TOKEN_ID;

assert(!InternalImpl::_exists(@state, token_id), 'Token should not exist');
assert(state._owners.read(token_id) == zero, 'Invalid owner');
assert(state.ERC721_owners.read(token_id) == zero, 'Invalid owner');

InternalImpl::_mint(ref state, RECIPIENT(), token_id);

assert(InternalImpl::_exists(@state, token_id), 'Token should exist');
assert(state._owners.read(token_id) == RECIPIENT(), 'Invalid owner');
assert(state.ERC721_owners.read(token_id) == RECIPIENT(), 'Invalid owner');

InternalImpl::_burn(ref state, token_id);

assert(!InternalImpl::_exists(@state, token_id), 'Token should not exist');
assert(state._owners.read(token_id) == zero, 'Invalid owner');
assert(state.ERC721_owners.read(token_id) == zero, 'Invalid owner');
}

//
Expand Down Expand Up @@ -1329,9 +1329,9 @@ fn test__burn() {
InternalImpl::_burn(ref state, TOKEN_ID);
assert_event_transfer(OWNER(), ZERO(), TOKEN_ID);

assert(state._owners.read(TOKEN_ID) == ZERO(), 'Ownership after');
assert(state.ERC721_owners.read(TOKEN_ID) == ZERO(), 'Ownership after');
assert(ERC721Impl::balance_of(@state, OWNER()) == 0, 'Balance of owner after');
assert(state._token_approvals.read(TOKEN_ID) == ZERO(), 'Approval after');
assert(state.ERC721_token_approvals.read(TOKEN_ID) == ZERO(), 'Approval after');
}

#[test]
Expand Down
Loading

0 comments on commit 74eb4e8

Please sign in to comment.