diff --git a/src/interface/naming.cairo b/src/interface/naming.cairo index 80a4244..e5caf55 100644 --- a/src/interface/naming.cairo +++ b/src/interface/naming.cairo @@ -102,7 +102,6 @@ trait INaming { ); // admin - fn set_admin(ref self: TContractState, new_admin: ContractAddress); fn set_expiry(ref self: TContractState, root_domain: felt252, expiry: u64); @@ -123,5 +122,4 @@ trait INaming { fn blacklist_renewal_contract(ref self: TContractState, contract: ContractAddress); fn toggle_ar_discount_renew(ref self: TContractState); - } diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 4df93a7..24ec301 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -21,8 +21,9 @@ mod Naming { } }; use identity::interface::identity::{IIdentity, IIdentityDispatcher, IIdentityDispatcherTrait}; - use openzeppelin::token::erc20::interface::{ - IERC20Camel, IERC20CamelDispatcher, IERC20CamelDispatcherTrait + use openzeppelin::{ + access::ownable::OwnableComponent, + token::erc20::interface::{IERC20Camel, IERC20CamelDispatcher, IERC20CamelDispatcherTrait} }; use storage_read::{main::storage_read_component, interface::IStorageRead}; @@ -38,7 +39,9 @@ mod Naming { DomainMigrated: DomainMigrated, SubdomainsReset: SubdomainsReset, SaleMetadata: SaleMetadata, - StorageReadEvent: storage_read_component::Event + StorageReadEvent: storage_read_component::Event, + #[flat] + OwnableEvent: OwnableComponent::Event, } #[derive(Drop, starknet::Event)] @@ -126,7 +129,6 @@ mod Naming { starknetid_contract: ContractAddress, _pricing_contract: ContractAddress, _referral_contract: ContractAddress, - _admin_address: ContractAddress, _domain_data: LegacyMap, _hash_to_domain: LegacyMap<(felt252, usize), felt252>, _address_to_domain: LegacyMap<(ContractAddress, usize), felt252>, @@ -137,6 +139,8 @@ mod Naming { _ar_discount_renew_enabled: bool, #[substorage(v0)] storage_read: storage_read_component::Storage, + #[substorage(v0)] + ownable: OwnableComponent::Storage, } #[constructor] @@ -150,13 +154,17 @@ mod Naming { self.starknetid_contract.write(starknetid); self._pricing_contract.write(pricing); self._referral_contract.write(referral); - self._admin_address.write(admin); + self.ownable.initializer(admin); } component!(path: storage_read_component, storage: storage_read, event: StorageReadEvent); + component!(path: OwnableComponent, storage: ownable, event: OwnableEvent); #[abi(embed_v0)] impl StorageReadComponent = storage_read_component::StorageRead; + #[abi(embed_v0)] + impl OwnableTwoStepImpl = OwnableComponent::OwnableTwoStepImpl; + impl OwnableInternalImpl = OwnableComponent::InternalImpl; #[abi(embed_v0)] impl NamingImpl of INaming { @@ -699,15 +707,10 @@ mod Naming { // ADMIN - fn set_admin(ref self: ContractState, new_admin: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); - self._admin_address.write(new_admin); - } - fn set_expiry( ref self: ContractState, root_domain: felt252, expiry: u64 ) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); let hashed_domain = self.hash_domain(array![root_domain].span()); let domain_data = self._domain_data.read(hashed_domain); let data = DomainData { @@ -726,7 +729,7 @@ mod Naming { } fn claim_balance(ref self: ContractState, erc20: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); let balance = IERC20CamelDispatcher { contract_address: erc20 } .balanceOf(get_contract_address()); let has_claimed = IERC20CamelDispatcher { contract_address: erc20 } @@ -735,45 +738,45 @@ mod Naming { } fn set_discount(ref self: ContractState, discount_id: felt252, discount: Discount) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self.discounts.write(discount_id, discount); } fn set_pricing_contract(ref self: ContractState, pricing_contract: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self._pricing_contract.write(pricing_contract); } fn set_referral_contract(ref self: ContractState, referral_contract: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self._referral_contract.write(referral_contract); } fn upgrade(ref self: ContractState, new_class_hash: ClassHash) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); // todo: use components assert(!new_class_hash.is_zero(), 'Class hash cannot be zero'); starknet::replace_class_syscall(new_class_hash).unwrap(); } fn set_server_pub_key(ref self: ContractState, new_key: felt252) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self._server_pub_key.write(new_key); } fn whitelist_renewal_contract(ref self: ContractState, contract: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self._whitelisted_renewal_contracts.write(contract, true); } fn blacklist_renewal_contract(ref self: ContractState, contract: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self._whitelisted_renewal_contracts.write(contract, false); } fn toggle_ar_discount_renew(ref self: ContractState) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); + self.ownable.assert_only_owner(); self._ar_discount_renew_enabled.write(!self._ar_discount_renew_enabled.read()); } } diff --git a/src/tests/naming.cairo b/src/tests/naming.cairo index a226146..4727a1f 100644 --- a/src/tests/naming.cairo +++ b/src/tests/naming.cairo @@ -5,3 +5,4 @@ mod test_usecases; mod test_features; mod test_altcoin; mod test_ar_discount; +mod test_admin_update; diff --git a/src/tests/naming/test_abuses.cairo b/src/tests/naming/test_abuses.cairo index 6f1547d..7b83f22 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -216,21 +216,7 @@ fn test_renewal_period_too_long() { #[test] #[available_gas(2000000000)] -#[should_panic(expected: ('you are not admin', 'ENTRYPOINT_FAILED'))] -fn test_non_admin_cannot_set_admin() { - // setup - let (_, _, _, naming) = deploy(); - let non_admin_address = contract_address_const::<0x456>(); - set_contract_address(non_admin_address); - - // A non-admin tries to set a new admin - let new_admin = contract_address_const::<0x789>(); - naming.set_admin(new_admin); -} - -#[test] -#[available_gas(2000000000)] -#[should_panic(expected: ('you are not admin', 'ENTRYPOINT_FAILED'))] +#[should_panic(expected: ('Caller is not the owner', 'ENTRYPOINT_FAILED'))] fn test_non_admin_cannot_claim_balance() { // setup let (eth, _, _, naming) = deploy(); diff --git a/src/tests/naming/test_admin_update.cairo b/src/tests/naming/test_admin_update.cairo new file mode 100644 index 0000000..007bb0e --- /dev/null +++ b/src/tests/naming/test_admin_update.cairo @@ -0,0 +1,49 @@ +use starknet::testing; +use starknet::ContractAddress; +use starknet::contract_address::ContractAddressZeroable; +use starknet::contract_address_const; +use starknet::testing::set_contract_address; +use super::super::utils; +use super::common::deploy; +use naming::naming::main::Naming; +use naming::interface::naming::{INamingDispatcher, INamingDispatcherTrait}; +use openzeppelin::{ + access::ownable::interface::{IOwnableTwoStep, IOwnableTwoStepDispatcher, IOwnableTwoStepDispatcherTrait}, + token::erc20::{ + interface::{IERC20Camel, IERC20CamelDispatcher, IERC20CamelDispatcherTrait} +}}; + +#[test] +#[available_gas(2000000000)] +fn test_update_admin() { + // setup + let (_, _, _, naming) = deploy(); + let admin = contract_address_const::<0x123>(); + let new_admin = contract_address_const::<0x456>(); + + let ownable2Step = IOwnableTwoStepDispatcher { contract_address: naming.contract_address }; + assert(ownable2Step.owner() == admin, 'admin not initialized'); + + // Now we go back to the first admin, this time using the ownable2Step + set_contract_address(admin); + ownable2Step.transfer_ownership(new_admin); + set_contract_address(new_admin); + ownable2Step.accept_ownership(); + assert(ownable2Step.owner() == new_admin, 'change of admin failed'); +} + + +#[test] +#[available_gas(2000000000)] +#[should_panic(expected: ('Caller is not the owner', 'ENTRYPOINT_FAILED'))] +fn test_non_admin_cannot_set_admin() { + // setup + let (_, _, _, naming) = deploy(); + let ownable2Step = IOwnableTwoStepDispatcher { contract_address: naming.contract_address }; + let non_admin_address = contract_address_const::<0x456>(); + set_contract_address(non_admin_address); + + // A non-admin tries to set a new admin + let new_admin = contract_address_const::<0x789>(); + ownable2Step.transfer_ownership(new_admin); +} \ No newline at end of file