Merge pull request #63 from starknet-id/fix/test_use_reset_subdomains…

…_multiple_levels

Fix/test use reset subdomains multiple levels

src/naming/asserts.cairo

@@ -60,6 +60,8 @@ impl AssertionsImpl of AssertionsTrait {
         let mut i: felt252 = 1;
         let stop = (domain.len() + 1).into();
         let mut parent_key = 0;
+        // we start from the top domain and go down until we find you are the owner,
+        // reach the domain beginning or reach a key mismatch (reset parent domain)
         loop {
             assert(i != stop, 'you don\'t own this domain');
             let i_gas_saver = i.try_into().unwrap();

src/naming/internal.cairo

@@ -1,8 +1,6 @@
+use core::array::SpanTrait;
 use naming::{
-    interface::{
-        resolver::{IResolver, IResolverDispatcher, IResolverDispatcherTrait},
-        referral::{IReferral, IReferralDispatcher, IReferralDispatcherTrait},
-    },
+    interface::referral::{IReferral, IReferralDispatcher, IReferralDispatcherTrait},
     naming::main::{
         Naming,
         Naming::{
@@ -51,24 +49,38 @@ impl InternalImpl of InternalTrait {
         };
     }
 
+    // returns the custom resolver to use for a domain (0 if none)
+    // and the parent domain length. If one parent domain has
+    // reset its subdomains, it will break and return its length,
+    // otherwise the parent length would be 0.
     fn domain_to_resolver(
-        self: @Naming::ContractState, domain: Span<felt252>, parent_start_id: u32
+        self: @Naming::ContractState, mut domain: Span<felt252>
     ) -> (ContractAddress, u32) {
-        if parent_start_id == domain.len() {
-            return (ContractAddressZeroable::zero(), 0);
+        let mut custom_resolver = ContractAddressZeroable::zero();
+        let mut parent_length = 0;
+        let mut domain_parent_key = self._domain_data.read(self.hash_domain(domain)).parent_key;
+        loop {
+            if domain.len() == 1 {
+                break;
+            };
+            // will fail on empty domain
+            let parent_domain = domain.slice(1, domain.len() - 1);
+            let hashed_parent_domain = self.hash_domain(parent_domain);
+            let parent_domain_data = self._domain_data.read(hashed_parent_domain);
+            if parent_domain_data.resolver.into() != 0 {
+                custom_resolver = parent_domain_data.resolver;
+                parent_length = parent_domain.len();
+                break;
+            }
+            if domain_parent_key != parent_domain_data.key {
+                // custom_resolver is zero
+                parent_length = parent_domain.len();
+                break;
+            }
+            domain = parent_domain;
+            domain_parent_key = parent_domain_data.parent_key;
         };
-
-        // hashing parent_domain
-        let hashed_domain = self
-            .hash_domain(domain.slice(parent_start_id, domain.len() - parent_start_id));
-
-        let domain_data = self._domain_data.read(hashed_domain);
-
-        if domain_data.resolver.into() != 0 {
-            return (domain_data.resolver, parent_start_id);
-        } else {
-            return self.domain_to_resolver(domain, parent_start_id + 1);
-        }
+        (custom_resolver, parent_length)
     }
 
     fn pay_domain(
@@ -106,7 +118,12 @@ 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(), erc20_addr: erc20);
+                .add_commission(
+                    discounted_price,
+                    sponsor,
+                    sponsored_addr: get_caller_address(),
+                    erc20_addr: erc20
+                );
         }
     }
 
@@ -141,45 +158,4 @@ impl InternalImpl of InternalTrait {
                 );
         }
     }
-
-    // returns domain_hash (or zero) and its value for a specific field
-    fn resolve_util(
-        self: @Naming::ContractState, domain: Span<felt252>, field: felt252, hint: Span<felt252>
-    ) -> (felt252, felt252) {
-        let (resolver, parent_start) = self.domain_to_resolver(domain, 1);
-        if (resolver != ContractAddressZeroable::zero()) {
-            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);
-            // circuit breaker for root domain
-            (
-                hashed_domain,
-                if (domain.len() == 1) {
-                    IIdentityDispatcher { contract_address: self.starknetid_contract.read() }
-                        .get_crosschecked_user_data(domain_data.owner, field)
-                // handle reset subdomains
-                } else {
-                    // todo: optimize by changing the hash definition from H(b, a) to H(a, b)
-                    let parent_key = self
-                        ._domain_data
-                        .read(self.hash_domain(domain.slice(1, domain.len() - 1)))
-                        .key;
-
-                    if parent_key == domain_data.parent_key {
-                        IIdentityDispatcher { contract_address: self.starknetid_contract.read() }
-                            .get_crosschecked_user_data(domain_data.owner, field)
-                    } else {
-                        0
-                    }
-                }
-            )
-        }
-    }
 }

src/naming/main.cairo

@@ -17,7 +17,8 @@ mod Naming {
         interface::{
             naming::{INaming, INamingDispatcher, INamingDispatcherTrait},
             pricing::{IPricing, IPricingDispatcher, IPricingDispatcherTrait},
-            auto_renewal::{IAutoRenewal, IAutoRenewalDispatcher, IAutoRenewalDispatcherTrait}
+            auto_renewal::{IAutoRenewal, IAutoRenewalDispatcher, IAutoRenewalDispatcherTrait},
+            resolver::{IResolver, IResolverDispatcher, IResolverDispatcherTrait}
         }
     };
     use identity::interface::identity::{IIdentity, IIdentityDispatcher, IIdentityDispatcherTrait};
@@ -177,8 +178,23 @@ mod Naming {
         fn resolve(
             self: @ContractState, domain: Span<felt252>, field: felt252, hint: Span<felt252>
         ) -> felt252 {
-            let (_, value) = self.resolve_util(domain, field, hint);
-            value
+            let (resolver, parent_length) = self.domain_to_resolver(domain);
+            // if there is a resolver starting from the top
+            if (resolver != ContractAddressZeroable::zero()) {
+                IResolverDispatcher { contract_address: resolver }
+                    .resolve(domain.slice(0, domain.len() - parent_length), field, hint)
+            } else {
+                let hashed_domain = self.hash_domain(domain);
+                let domain_data = self._domain_data.read(hashed_domain);
+                // if there was a reset subdomains starting from the top
+                if parent_length != 0 {
+                    0
+                // otherwise, we just read the identity
+                } else {
+                    IIdentityDispatcher { contract_address: self.starknetid_contract.read() }
+                        .get_crosschecked_user_data(domain_data.owner, field)
+                }
+            }
         }
 
         // This functions allows to resolve a domain to a native address. Its output is designed
@@ -187,27 +203,44 @@ mod Naming {
         fn domain_to_address(
             self: @ContractState, domain: Span<felt252>, hint: Span<felt252>
         ) -> ContractAddress {
-            // resolve must be performed first because it calls untrusted resolving contracts
-            let (hashed_domain, value) = self.resolve_util(domain, 'starknet', hint);
-            if value != 0 {
-                let addr: Option<ContractAddress> = value.try_into();
-                return addr.unwrap();
-            };
-            let data = self._domain_data.read(hashed_domain);
-            if data.address.into() != 0 {
-                if domain.len() != 1 {
-                    let parent_key = self
-                        ._domain_data
-                        .read(self.hash_domain(domain.slice(1, domain.len() - 1)))
-                        .key;
-                    if parent_key == data.parent_key {
-                        return data.address;
-                    };
-                };
-                return data.address;
-            };
-            IIdentityDispatcher { contract_address: self.starknetid_contract.read() }
-                .owner_from_id(self.domain_to_id(domain))
+            let (resolver, parent_length) = self.domain_to_resolver(domain);
+            // if there is a resolver starting from the top
+            if (resolver != ContractAddressZeroable::zero()) {
+                let addr: Option<ContractAddress> = IResolverDispatcher {
+                    contract_address: resolver
+                }
+                    .resolve(domain.slice(0, domain.len() - parent_length), 'starknet', hint)
+                    .try_into();
+                addr.unwrap()
+            } else {
+                // if there was a reset subdomains starting from the top
+                if parent_length != 0 {
+                    ContractAddressZeroable::zero()
+                // otherwise we read the identity
+                } else {
+                    let hashed_domain = self.hash_domain(domain);
+                    let domain_data = self._domain_data.read(hashed_domain);
+                    let identity_address = IIdentityDispatcher {
+                        contract_address: self.starknetid_contract.read()
+                    }
+                        .get_crosschecked_user_data(domain_data.owner, 'starknet');
+                    if identity_address != 0 {
+                        let addr: Option<ContractAddress> = identity_address.try_into();
+                        addr.unwrap()
+                    } else {
+                        if domain_data.address.into() != 0 {
+                            // no need to check for keys as it was checked in domain_to_resolver
+                            return domain_data.address;
+                        } else {
+                            // if no legacy address is found, it returns the identity owner
+                            IIdentityDispatcher {
+                                contract_address: self.starknetid_contract.read()
+                            }
+                                .owner_from_id(self.domain_to_id(domain))
+                        }
+                    }
+                }
+            }
         }
 
         // This returns the stored DomainData associated to this domain
@@ -221,22 +254,29 @@ mod Naming {
         }
 
         // This returns the identity (StarknetID) owning the domain
-        fn domain_to_id(self: @ContractState, domain: Span<felt252>) -> u128 {
+        fn domain_to_id(self: @ContractState, mut domain: Span<felt252>) -> u128 {
             let data = self._domain_data.read(self.hash_domain(domain));
             // todo: revert when try catch are available 
             if domain.len() == 0 {
                 return 0;
             };
-            if domain.len() != 1 {
-                let parent_key = self
-                    ._domain_data
-                    .read(self.hash_domain(domain.slice(1, domain.len() - 1)))
-                    .key;
-                if parent_key != data.parent_key {
-                    return 0;
-                };
+
+            let mut parent_key = data.parent_key;
+            let mut output = data.owner;
+            loop {
+                if domain.len() == 1 {
+                    break;
+                }
+                let parent_domain = domain.slice(1, domain.len() - 1);
+                let parent_domain_data = self._domain_data.read(self.hash_domain(parent_domain));
+                if parent_domain_data.key != parent_key {
+                    output = 0;
+                    break;
+                }
+                domain = parent_domain;
+                parent_key = parent_domain_data.parent_key;
             };
-            data.owner
+            output
         }
 
         // This function allows to find which domain to use to display an account

src/tests/naming/test_abuses.cairo

@@ -311,6 +311,71 @@ fn test_transfer_from_returns_false() {
         .buy(1, aller, 365, ContractAddressZeroable::zero(), ContractAddressZeroable::zero(), 0, 0);
 }
 
+#[test]
+#[available_gas(2000000000)]
+fn test_use_reset_subdomains_multiple_levels() {
+    // setup
+    let (eth, pricing, identity, naming) = deploy();
+    let alpha = contract_address_const::<0x123>();
+    let bravo = contract_address_const::<0x456>();
+    let charlie = contract_address_const::<0x789>();
+    // In this example we will use utf-8 encoded strings like 'toto' which is not 
+    // what is actually defined in the starknetid standard, it's just easier for testings
+
+    // we mint the ids
+    set_contract_address(alpha);
+    identity.mint(1);
+    set_contract_address(bravo);
+    identity.mint(2);
+    set_contract_address(charlie);
+    identity.mint(3);
+
+    // we check how much a domain costs
+    let (_, price) = pricing.compute_buy_price(5, 365);
+
+    // we allow the naming to take our money
+    set_contract_address(alpha);
+    eth.approve(naming.contract_address, price);
+
+    // we buy with no resolver, no sponsor, no discount and empty metadata
+    naming
+        .buy(
+            1, 'ccccc', 365, ContractAddressZeroable::zero(), ContractAddressZeroable::zero(), 0, 0
+        );
+
+    let root_domain = array!['ccccc'].span();
+    let subdomain = array!['bbbbb', 'ccccc'].span();
+
+    // we transfer bb.cc.stark to id2
+    naming.transfer_domain(subdomain, 2);
+
+    // and make sure the owner has been updated
+    assert(naming.domain_to_id(subdomain) == 2, 'owner not updated correctly');
+
+    set_contract_address(bravo);
+    // we transfer aa.bb.cc.stark to id3
+    let subsubdomain = array!['aaaaa', 'bbbbb', 'ccccc'].span();
+    naming.transfer_domain(subsubdomain, 3);
+    // and make sure the owner has been updated
+    assert(naming.domain_to_id(subsubdomain) == 3, 'owner2 not updated correctly');
+
+    // now charlie should be able to create a subbsubsubdomain (example.aa.bb.cc.stark):
+    set_contract_address(charlie);
+    let subsubsubdomain = array!['example', 'aaaaa', 'bbbbb', 'ccccc'].span();
+    naming.transfer_domain(subsubsubdomain, 4);
+
+    // alpha resets subdomains of ccccc.stark
+    set_contract_address(alpha);
+    naming.reset_subdomains(root_domain);
+
+    // ensure root domain still resolves
+    assert(naming.domain_to_id(root_domain) == 1, 'owner3 not updated correctly');
+    // ensure the subdomain was reset
+    assert(naming.domain_to_id(subdomain) == 0, 'owner4 not updated correctly');
+    // ensure the subsubdomain was reset
+    assert(naming.domain_to_id(subsubdomain) == 0, 'owner5 not updated correctly');
+}
+
 #[test]
 #[available_gas(2000000000)]
 #[should_panic(expected: ('domain can\'t be empty', 'ENTRYPOINT_FAILED'))]

src/tests/naming/test_altcoin.cairo

@@ -21,7 +21,6 @@ use naming::pricing::Pricing;
 use naming::naming::utils::UtilsImpl;
 use super::common::{deploy, deploy_stark};
 use super::super::utils;
-use core::debug::PrintTrait;
 use wadray::Wad;
 
 #[test]

src/tests/naming/test_custom_resolver.cairo

@@ -55,9 +55,7 @@ mod CustomResolver {
 fn test_custom_resolver() {
     // setup
     let (eth, pricing, identity, naming) = deploy();
-    let custom_resolver = IERC20CamelDispatcher {
-        contract_address: utils::deploy(CustomResolver::TEST_CLASS_HASH, ArrayTrait::new())
-    };
+    let custom_resolver = utils::deploy(CustomResolver::TEST_CLASS_HASH, ArrayTrait::new());
 
     let caller = contract_address_const::<0x123>();
     set_contract_address(caller);
@@ -73,13 +71,13 @@ fn test_custom_resolver() {
     // 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
+    // we buy with a custom resolver, no sponsor, no discount and empty metadata
     naming
         .buy(
             id,
             th0rgal,
             365,
-            custom_resolver.contract_address,
+            custom_resolver,
             ContractAddressZeroable::zero(),
             0,
             0
@@ -92,7 +90,6 @@ fn test_custom_resolver() {
     assert(naming.domain_to_address(domain, array![].span()) == caller, 'wrong domain target');
 
     let domain = array![1, 2, 3, th0rgal].span();
-
     let new_target = contract_address_const::<0x6>();
     // let's try the resolving
     assert(naming.resolve(domain, 'starknet', array![].span()) == 1 + 2 + 3, 'wrong target');