From e9c80c8f782cf6565d0435c06b48a6ba8852801b Mon Sep 17 00:00:00 2001 From: Iris Date: Fri, 17 May 2024 15:19:45 +0200 Subject: [PATCH 01/12] fix: unused imports --- src/interface/resolver.cairo | 2 -- src/naming/asserts.cairo | 7 ------- src/naming/internal.cairo | 4 +--- src/naming/main.cairo | 5 ----- src/naming/utils.cairo | 2 +- 5 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/interface/resolver.cairo b/src/interface/resolver.cairo index ef719c6..f55f73e 100644 --- a/src/interface/resolver.cairo +++ b/src/interface/resolver.cairo @@ -1,5 +1,3 @@ -use starknet::ContractAddress; - #[starknet::interface] trait IResolver { fn resolve( diff --git a/src/naming/asserts.cairo b/src/naming/asserts.cairo index 0612a7a..5196863 100644 --- a/src/naming/asserts.cairo +++ b/src/naming/asserts.cairo @@ -1,12 +1,6 @@ use core::traits::TryInto; use core::array::SpanTrait; use naming::{ - interface::{ - naming::{INaming, INamingDispatcher, INamingDispatcherTrait}, - resolver::{IResolver, IResolverDispatcher, IResolverDispatcherTrait}, - pricing::{IPricing, IPricingDispatcher, IPricingDispatcherTrait}, - referral::{IReferral, IReferralDispatcher, IReferralDispatcherTrait}, - }, naming::main::{ Naming, Naming::{ @@ -25,7 +19,6 @@ use starknet::{ use openzeppelin::token::erc20::interface::{ IERC20Camel, IERC20CamelDispatcher, IERC20CamelDispatcherTrait }; -use integer::{u256_safe_divmod, u256_as_non_zero}; use naming::naming::utils::UtilsTrait; #[generate_trait] diff --git a/src/naming/internal.cairo b/src/naming/internal.cairo index 88ac815..4546257 100644 --- a/src/naming/internal.cairo +++ b/src/naming/internal.cairo @@ -1,8 +1,6 @@ use naming::{ interface::{ - naming::{INaming, INamingDispatcher, INamingDispatcherTrait}, resolver::{IResolver, IResolverDispatcher, IResolverDispatcherTrait}, - pricing::{IPricing, IPricingDispatcher, IPricingDispatcherTrait}, referral::{IReferral, IReferralDispatcher, IReferralDispatcherTrait}, }, naming::main::{ @@ -18,7 +16,7 @@ use naming::{ use identity::interface::identity::{IIdentity, IIdentityDispatcher, IIdentityDispatcherTrait}; use starknet::{ contract_address::ContractAddressZeroable, ContractAddress, get_caller_address, - get_contract_address, get_block_timestamp + get_contract_address }; use openzeppelin::token::erc20::interface::{ IERC20Camel, IERC20CamelDispatcher, IERC20CamelDispatcherTrait diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 7d66c14..cdc6f6c 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -8,7 +8,6 @@ mod Naming { use array::{ArrayTrait, SpanTrait}; use zeroable::Zeroable; use starknet::class_hash::ClassHash; - use integer::{u256_safe_divmod, u256_as_non_zero}; use core::pedersen; use hash::LegacyHash; use ecdsa::check_ecdsa_signature; @@ -17,14 +16,10 @@ mod Naming { naming::{asserts::AssertionsTrait, internal::InternalTrait, utils::UtilsTrait}, interface::{ naming::{INaming, INamingDispatcher, INamingDispatcherTrait}, - resolver::{IResolver, IResolverDispatcher, IResolverDispatcherTrait}, pricing::{IPricing, IPricingDispatcher, IPricingDispatcherTrait}, - referral::{IReferral, IReferralDispatcher, IReferralDispatcherTrait}, auto_renewal::{IAutoRenewal, IAutoRenewalDispatcher, IAutoRenewalDispatcherTrait} } }; - use clone::Clone; - use array::ArrayTCloneImpl; use identity::interface::identity::{IIdentity, IIdentityDispatcher, IIdentityDispatcherTrait}; use openzeppelin::token::erc20::interface::{ IERC20Camel, IERC20CamelDispatcher, IERC20CamelDispatcherTrait diff --git a/src/naming/utils.cairo b/src/naming/utils.cairo index c8eb529..9de0088 100644 --- a/src/naming/utils.cairo +++ b/src/naming/utils.cairo @@ -1,7 +1,7 @@ use core::array::SpanTrait; use naming::{naming::main::{Naming, Naming::_hash_to_domainContractMemberStateTrait}}; use integer::{u256_safe_divmod, u256_as_non_zero}; -use wadray::{Wad, WAD_SCALE}; +use wadray::Wad; #[generate_trait] impl UtilsImpl of UtilsTrait { From 109f9cc79dd28f2a24f57f754e8f039c09257ddf Mon Sep 17 00:00:00 2001 From: Iris Date: Fri, 17 May 2024 15:34:16 +0200 Subject: [PATCH 02/12] fix: remove unused metadata argument in set_expiry --- src/interface/naming.cairo | 2 +- src/naming/main.cairo | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interface/naming.cairo b/src/interface/naming.cairo index 170b443..80a4244 100644 --- a/src/interface/naming.cairo +++ b/src/interface/naming.cairo @@ -104,7 +104,7 @@ trait INaming { // admin fn set_admin(ref self: TContractState, new_admin: ContractAddress); - fn set_expiry(ref self: TContractState, root_domain: felt252, expiry: u64, metadata: felt252); + fn set_expiry(ref self: TContractState, root_domain: felt252, expiry: u64); fn claim_balance(ref self: TContractState, erc20: ContractAddress); diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 7d66c14..23ea5b7 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -708,7 +708,7 @@ mod Naming { } fn set_expiry( - ref self: ContractState, root_domain: felt252, expiry: u64, metadata: felt252 + ref self: ContractState, root_domain: felt252, expiry: u64 ) { assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); let hashed_domain = self.hash_domain(array![root_domain].span()); From a764a4d3c906255eb8fbd5a3bf48c1bb3250ad98 Mon Sep 17 00:00:00 2001 From: Iris Date: Tue, 21 May 2024 09:40:28 +0200 Subject: [PATCH 03/12] fix: user could buy empty domain --- src/naming/main.cairo | 2 ++ src/tests/naming/test_abuses.cairo | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 7d66c14..0177233 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -284,6 +284,7 @@ mod Naming { let (hashed_domain, now, expiry) = self.assert_purchase_is_possible(id, domain, days); // we need a u256 to be able to perform safe divisions let domain_len = self.get_chars_len(domain.into()); + assert(domain_len != 0, 'domain can\' be empty'); // find domain cost let (erc20, price) = IPricingDispatcher { contract_address: self._pricing_contract.read() @@ -311,6 +312,7 @@ mod Naming { let (hashed_domain, now, expiry) = self.assert_purchase_is_possible(id, domain, days); // we need a u256 to be able to perform safe divisions let domain_len = self.get_chars_len(domain.into()); + assert(domain_len != 0, 'domain can\' be empty'); // check quote timestamp is still valid assert(get_block_timestamp() <= max_validity, 'quotation expired'); diff --git a/src/tests/naming/test_abuses.cairo b/src/tests/naming/test_abuses.cairo index ce22568..7934396 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -298,3 +298,29 @@ fn test_use_reset_subdomains() { naming.transfer_domain(subsubdomain2, 4); } +#[test] +#[available_gas(2000000000)] +#[should_panic(expected: ('domain can\' be empty', 'ENTRYPOINT_FAILED'))] +fn test_buy_empty_domain() { + // setup + let (eth, pricing, identity, naming) = deploy(); + let alpha = contract_address_const::<0x123>(); + + // we mint the id + set_contract_address(alpha); + identity.mint(1); + + set_contract_address(alpha); + let empty_domain: felt252 = 0; + + // we check how much a domain costs + let (_, price) = pricing.compute_buy_price(0, 365); + + // we allow the naming to take our money + eth.approve(naming.contract_address, price); + + // we buy with no resolver, no sponsor, no discount and empty metadata + naming + .buy(1, empty_domain, 365, ContractAddressZeroable::zero(), ContractAddressZeroable::zero(), 0, 0); +} + From e6d99972fb7dd6f5792158946bfaa6677806f558 Mon Sep 17 00:00:00 2001 From: Iris Date: Tue, 21 May 2024 10:26:03 +0200 Subject: [PATCH 04/12] fix: check retrun values of ERC20 transfer & transferFrom functions --- src/naming/internal.cairo | 3 +- src/naming/main.cairo | 3 +- src/tests/naming/common.cairo | 71 ++++++++++++++++++++++++++++++ src/tests/naming/test_abuses.cairo | 28 +++++++++++- 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/naming/internal.cairo b/src/naming/internal.cairo index 88ac815..cc2943c 100644 --- a/src/naming/internal.cairo +++ b/src/naming/internal.cairo @@ -102,8 +102,9 @@ impl InternalImpl of InternalTrait { }; // pay the price - IERC20CamelDispatcher { contract_address: erc20 } + let has_payed = IERC20CamelDispatcher { contract_address: erc20 } .transferFrom(get_caller_address(), get_contract_address(), discounted_price); + assert(has_payed, 'payment failed'); // add sponsor commission if eligible if sponsor.into() != 0 { IReferralDispatcher { contract_address: self._referral_contract.read() } diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 7d66c14..2a55470 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -732,8 +732,9 @@ mod Naming { assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); let balance = IERC20CamelDispatcher { contract_address: erc20 } .balanceOf(get_contract_address()); - IERC20CamelDispatcher { contract_address: erc20 } + let has_claimed = IERC20CamelDispatcher { contract_address: erc20 } .transfer(get_caller_address(), balance); + assert(has_claimed, 'Claim failed'); } fn set_discount(ref self: ContractState, discount_id: felt252, discount: Discount) { diff --git a/src/tests/naming/common.cairo b/src/tests/naming/common.cairo index 555a716..c18c927 100644 --- a/src/tests/naming/common.cairo +++ b/src/tests/naming/common.cairo @@ -87,3 +87,74 @@ fn deploy_stark() -> IERC20CamelDispatcher { let stark = utils::deploy(ERC20::TEST_CLASS_HASH, array![]); IERC20CamelDispatcher { contract_address: stark } } + +fn deploy_with_erc20_fail() -> (IERC20CamelDispatcher, IPricingDispatcher, IIdentityDispatcher, INamingDispatcher) { + //erc20 + let eth = utils::deploy(ERC20Fail::TEST_CLASS_HASH, array![]); + + // pricing + let pricing = utils::deploy(Pricing::TEST_CLASS_HASH, array![eth.into()]); + + // identity + let identity = utils::deploy(Identity::TEST_CLASS_HASH, array![0x123, 0, 0, 0]); + + // naming + let admin = 0x123; + let address = utils::deploy( + Naming::TEST_CLASS_HASH, array![identity.into(), pricing.into(), 0, admin] + ); + + ( + IERC20CamelDispatcher { contract_address: eth }, + IPricingDispatcher { contract_address: pricing }, + IIdentityDispatcher { contract_address: identity }, + INamingDispatcher { contract_address: address } + ) +} + +#[starknet::contract] +mod ERC20Fail { + use starknet::ContractAddress; + use openzeppelin::token::erc20::erc20::ERC20Component::InternalTrait as ERC20InternalTrait; + use openzeppelin::{ + token::erc20::{ERC20Component, dual20::DualCaseERC20Impl, ERC20HooksEmptyImpl} + }; + + component!(path: ERC20Component, storage: erc20, event: ERC20Event); + + #[abi(embed_v0)] + impl ERC20Impl = ERC20Component::ERC20Impl; + impl ERC20CamelOnlyImpl = ERC20Component::ERC20CamelOnlyImpl; + impl ERC20InternalImpl = ERC20Component::InternalImpl; + + #[constructor] + fn constructor(ref self: ContractState) { + self.erc20.initializer("ether", "ETH"); + let target = starknet::contract_address_const::<0x123>(); + self.erc20._mint(target, 0x100000000000000000000000000000000); + } + + #[storage] + struct Storage { + #[substorage(v0)] + erc20: ERC20Component::Storage, + } + + #[event] + #[derive(Drop, starknet::Event)] + enum Event { + #[flat] + ERC20Event: ERC20Component::Event, + } + + #[abi(per_item)] + #[generate_trait] + impl ExternalImpl of ExternalTrait { + #[external(v0)] + fn transferFrom( + ref self: ContractState, sender: ContractAddress, recipient: ContractAddress, amount: u256 + ) -> bool { + false + } + } +} \ No newline at end of file diff --git a/src/tests/naming/test_abuses.cairo b/src/tests/naming/test_abuses.cairo index ce22568..3c8bb72 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -19,7 +19,7 @@ use naming::interface::naming::{INamingDispatcher, INamingDispatcherTrait}; use naming::interface::pricing::{IPricingDispatcher, IPricingDispatcherTrait}; use naming::naming::main::Naming; use naming::pricing::Pricing; -use super::common::deploy; +use super::common::{deploy, deploy_with_erc20_fail}; #[test] #[available_gas(2000000000)] @@ -298,3 +298,29 @@ fn test_use_reset_subdomains() { naming.transfer_domain(subsubdomain2, 4); } +#[test] +#[available_gas(2000000000)] +#[should_panic(expected: ('payment failed', 'ENTRYPOINT_FAILED'))] +fn test_transfer_from_returns_false() { + // setup + let (eth, pricing, identity, naming) = deploy_with_erc20_fail(); + let alpha = contract_address_const::<0x123>(); + + // we mint the id + set_contract_address(alpha); + identity.mint(1); + + set_contract_address(alpha); + let aller: felt252 = 35683102; + + // we check how much a domain costs + let (_, price) = pricing.compute_buy_price(5, 365); + + // we allow the naming to take our money + eth.approve(naming.contract_address, price); + + // we buy with no resolver, no sponsor, no discount and empty metadata + // in pay_domain transferFrom will return false + naming + .buy(1, aller, 365, ContractAddressZeroable::zero(), ContractAddressZeroable::zero(), 0, 0); +} \ No newline at end of file From 5b3ce9e97479c5c0366f2d7f3a9282e5e02e951a Mon Sep 17 00:00:00 2001 From: Iris Date: Mon, 27 May 2024 15:09:25 +0200 Subject: [PATCH 05/12] fix: return hashed_domain if resolver returns 0 --- src/naming/internal.cairo | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/naming/internal.cairo b/src/naming/internal.cairo index 88ac815..4b03b77 100644 --- a/src/naming/internal.cairo +++ b/src/naming/internal.cairo @@ -149,11 +149,14 @@ impl InternalImpl of InternalTrait { ) -> (felt252, felt252) { let (resolver, parent_start) = self.domain_to_resolver(domain, 1); if (resolver != ContractAddressZeroable::zero()) { - ( - 0, - IResolverDispatcher { contract_address: resolver } - .resolve(domain.slice(0, parent_start), field, hint) - ) + let resolver_res = IResolverDispatcher { contract_address: resolver } + .resolve(domain.slice(0, parent_start), field, hint); + if resolver_res == 0 { + let hashed_domain = self.hash_domain(domain); + return (0, hashed_domain); + + } + return (0, resolver_res); } else { let hashed_domain = self.hash_domain(domain); let domain_data = self._domain_data.read(hashed_domain); From aef5a6338804b35567b2d9900e80318177e6dc4d Mon Sep 17 00:00:00 2001 From: Iris Date: Mon, 27 May 2024 15:10:22 +0200 Subject: [PATCH 06/12] fix: remove unecessary space --- src/naming/internal.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/naming/internal.cairo b/src/naming/internal.cairo index 4b03b77..de8fe70 100644 --- a/src/naming/internal.cairo +++ b/src/naming/internal.cairo @@ -154,7 +154,6 @@ impl InternalImpl of InternalTrait { if resolver_res == 0 { let hashed_domain = self.hash_domain(domain); return (0, hashed_domain); - } return (0, resolver_res); } else { From 86c0820c04c0ca6fa652ce3aa62e8abf851e9a1e Mon Sep 17 00:00:00 2001 From: Thomas Marchand Date: Tue, 28 May 2024 14:41:49 +0100 Subject: [PATCH 07/12] fix: domain can't be empty typo --- src/naming/main.cairo | 4 ++-- src/tests/naming/test_abuses.cairo | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 060d399..4df93a7 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -279,7 +279,7 @@ mod Naming { let (hashed_domain, now, expiry) = self.assert_purchase_is_possible(id, domain, days); // we need a u256 to be able to perform safe divisions let domain_len = self.get_chars_len(domain.into()); - assert(domain_len != 0, 'domain can\' be empty'); + assert(domain_len != 0, 'domain can\'t be empty'); // find domain cost let (erc20, price) = IPricingDispatcher { contract_address: self._pricing_contract.read() @@ -307,7 +307,7 @@ mod Naming { let (hashed_domain, now, expiry) = self.assert_purchase_is_possible(id, domain, days); // we need a u256 to be able to perform safe divisions let domain_len = self.get_chars_len(domain.into()); - assert(domain_len != 0, 'domain can\' be empty'); + assert(domain_len != 0, 'domain can\'t be empty'); // check quote timestamp is still valid assert(get_block_timestamp() <= max_validity, 'quotation expired'); diff --git a/src/tests/naming/test_abuses.cairo b/src/tests/naming/test_abuses.cairo index b924344..6f1547d 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -350,4 +350,3 @@ fn test_buy_empty_domain() { naming .buy(1, empty_domain, 365, ContractAddressZeroable::zero(), ContractAddressZeroable::zero(), 0, 0); } - From e21dda5d567bba1e20917a7dce04371123a9670d Mon Sep 17 00:00:00 2001 From: Thomas Marchand Date: Thu, 30 May 2024 17:04:21 +0100 Subject: [PATCH 08/12] feat: add config for voyager verification --- Scarb.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Scarb.toml b/Scarb.toml index 3ef57ec..bed1104 100644 --- a/Scarb.toml +++ b/Scarb.toml @@ -23,6 +23,9 @@ casm-add-pythonic-hints = true build-external-contracts = ["identity::identity::main::Identity"] +[tool.voyager] +naming = { path = "src/naming.cairo" } + [lib] sierra = true casm = false From 309e39df6923bec415b3b88e013b280cc632103f Mon Sep 17 00:00:00 2001 From: Iris Date: Wed, 5 Jun 2024 12:06:33 +0200 Subject: [PATCH 09/12] feat: update admin --- src/interface/naming.cairo | 2 +- src/naming/main.cairo | 19 ++++++++---- src/tests/naming.cairo | 1 + src/tests/naming/test_abuses.cairo | 2 +- src/tests/naming/test_admin_update.cairo | 38 ++++++++++++++++++++++++ 5 files changed, 55 insertions(+), 7 deletions(-) create mode 100644 src/tests/naming/test_admin_update.cairo 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..994ea20 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] @@ -154,9 +159,13 @@ mod Naming { } 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,9 +708,9 @@ 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); } fn set_expiry( 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..9385a1e 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -225,7 +225,7 @@ 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] diff --git a/src/tests/naming/test_admin_update.cairo b/src/tests/naming/test_admin_update.cairo new file mode 100644 index 0000000..ac215b4 --- /dev/null +++ b/src/tests/naming/test_admin_update.cairo @@ -0,0 +1,38 @@ +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() == contract_address_const::<0>(), 'admin should be 0'); + + // 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 From fcccbb2240ae3bd104bfb31ff15814b1bea76d35 Mon Sep 17 00:00:00 2001 From: Iris Date: Wed, 5 Jun 2024 12:14:26 +0200 Subject: [PATCH 10/12] feat: remove update_admin and finalize upgrade --- src/interface/naming.cairo | 2 -- src/naming/main.cairo | 26 +++++++++------------ src/tests/naming/test_abuses.cairo | 16 +------------ src/tests/naming/test_admin_update.cairo | 29 ++++++++++++++++-------- 4 files changed, 32 insertions(+), 41 deletions(-) diff --git a/src/interface/naming.cairo b/src/interface/naming.cairo index 3441026..e5caf55 100644 --- a/src/interface/naming.cairo +++ b/src/interface/naming.cairo @@ -122,6 +122,4 @@ trait INaming { fn blacklist_renewal_contract(ref self: TContractState, contract: ContractAddress); 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 994ea20..9e8fd4a 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -156,6 +156,7 @@ 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); @@ -708,15 +709,10 @@ mod Naming { // ADMIN - fn update_admin(ref self: ContractState, new_admin: ContractAddress) { - assert(get_caller_address() == self._admin_address.read(), 'you are not admin'); - self.ownable.initializer(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 { @@ -735,7 +731,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 } @@ -744,45 +740,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/test_abuses.cairo b/src/tests/naming/test_abuses.cairo index 9385a1e..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.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 index ac215b4..007bb0e 100644 --- a/src/tests/naming/test_admin_update.cairo +++ b/src/tests/naming/test_admin_update.cairo @@ -22,17 +22,28 @@ fn test_update_admin() { let new_admin = contract_address_const::<0x456>(); let ownable2Step = IOwnableTwoStepDispatcher { contract_address: naming.contract_address }; - assert(ownable2Step.owner() == contract_address_const::<0>(), 'admin should be 0'); - - // 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'); + assert(ownable2Step.owner() == admin, 'admin not initialized'); // 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.transfer_ownership(new_admin); + set_contract_address(new_admin); ownable2Step.accept_ownership(); - assert(ownable2Step.owner() == admin, 'change of admin failed'); + 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 From a72c07865beb1fe4c7a9c7794bb1d91abb5ee70f Mon Sep 17 00:00:00 2001 From: Iris Date: Wed, 5 Jun 2024 15:00:19 +0200 Subject: [PATCH 11/12] fix: write admin_address to 0 and update is_admin checks --- src/naming/main.cairo | 22 ++++++++++++---------- src/tests/naming/test_abuses.cairo | 2 +- src/tests/naming/test_admin_update.cairo | 1 - 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/naming/main.cairo b/src/naming/main.cairo index 994ea20..4b6e953 100644 --- a/src/naming/main.cairo +++ b/src/naming/main.cairo @@ -156,6 +156,7 @@ 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); @@ -711,12 +712,13 @@ mod Naming { fn update_admin(ref self: ContractState, new_admin: ContractAddress) { assert(get_caller_address() == self._admin_address.read(), 'you are not 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 { @@ -735,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 } @@ -744,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/test_abuses.cairo b/src/tests/naming/test_abuses.cairo index 9385a1e..e12c42f 100644 --- a/src/tests/naming/test_abuses.cairo +++ b/src/tests/naming/test_abuses.cairo @@ -230,7 +230,7 @@ fn test_non_admin_cannot_set_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 index ac215b4..cbd9efe 100644 --- a/src/tests/naming/test_admin_update.cairo +++ b/src/tests/naming/test_admin_update.cairo @@ -22,7 +22,6 @@ fn test_update_admin() { let new_admin = contract_address_const::<0x456>(); let ownable2Step = IOwnableTwoStepDispatcher { contract_address: naming.contract_address }; - assert(ownable2Step.owner() == contract_address_const::<0>(), 'admin should be 0'); // we call the update_admin function with the new admin set_contract_address(admin); From c0f19acdcd40bfb787c3832d9488ee3bfc4eb8dc Mon Sep 17 00:00:00 2001 From: Iris Date: Mon, 10 Jun 2024 09:44:11 +0200 Subject: [PATCH 12/12] ref: update add erc20 addr in add_commission --- src/interface/referral.cairo | 3 ++- src/naming/internal.cairo | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/interface/referral.cairo b/src/interface/referral.cairo index 9fe8748..cc96c4c 100644 --- a/src/interface/referral.cairo +++ b/src/interface/referral.cairo @@ -6,6 +6,7 @@ trait IReferral { self: @TContractState, amount: u256, sponsor_addr: ContractAddress, - sponsored_addr: ContractAddress + sponsored_addr: ContractAddress, + erc20_addr: ContractAddress, ); } diff --git a/src/naming/internal.cairo b/src/naming/internal.cairo index ee374a0..573cde2 100644 --- a/src/naming/internal.cairo +++ b/src/naming/internal.cairo @@ -106,7 +106,7 @@ impl InternalImpl of InternalTrait { // add sponsor commission if eligible if sponsor.into() != 0 { IReferralDispatcher { contract_address: self._referral_contract.read() } - .add_commission(discounted_price, sponsor, sponsored_addr: get_caller_address()); + .add_commission(discounted_price, sponsor, sponsored_addr: get_caller_address(), erc20_addr: erc20); } }