diff --git a/src/interface/naming.cairo b/src/interface/naming.cairo index 80a4244..3441026 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); @@ -124,4 +123,5 @@ trait INaming { fn toggle_ar_discount_renew(ref self: TContractState); + fn update_admin(ref self: TContractState, new_admin: ContractAddress); } diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 4df93a7..4b6e953 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)] @@ -137,6 +140,8 @@ mod Naming { _ar_discount_renew_enabled: bool, #[substorage(v0)] storage_read: storage_read_component::Storage, + #[substorage(v0)] + ownable: OwnableComponent::Storage, } #[constructor] @@ -151,12 +156,17 @@ mod Naming { 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 +709,16 @@ mod Naming { // ADMIN - fn set_admin(ref self: ContractState, new_admin: ContractAddress) { + fn update_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); + self.ownable.initializer(new_admin); + self._admin_address.write(Zeroable::zero()); } 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 +737,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 +746,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..e12c42f 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -225,12 +225,12 @@ fn test_non_admin_cannot_set_admin() { // A non-admin tries to set a new admin let new_admin = contract_address_const::<0x789>(); - naming.set_admin(new_admin); + naming.update_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..cbd9efe --- /dev/null +++ b/src/tests/naming/test_admin_update.cairo @@ -0,0 +1,37 @@ +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 }; + + // we call the update_admin function with the new admin + set_contract_address(admin); + naming.update_admin(new_admin); + assert(ownable2Step.owner() == new_admin, 'change of admin failed'); + + // Now we go back to the first admin, this time using the ownable2Step + set_contract_address(new_admin); + ownable2Step.transfer_ownership(admin); + set_contract_address(admin); + ownable2Step.accept_ownership(); + assert(ownable2Step.owner() == admin, 'change of admin failed'); +} \ No newline at end of file