Add owner check to get_primary_name (#188)

* Add owner check to get_primary_name

* Fix current and old tests

* More asserts

* More fixes

* Fix syntax

core/sources/config.move

@@ -448,8 +448,8 @@ module aptos_names::config {
         account::create_account_for_test(signer::address_of(aptos));
 
         // initializes coin, which is required for transfers
-        coin::register<AptosCoin>(myself);
         initialize_for_test(myself, aptos);
+        coin::register<AptosCoin>(myself);
 
         assert!(is_enabled(), 0);
         assert!(is_enabled_for_nonadmin(myself), 0);
@@ -484,8 +484,8 @@ module aptos_names::config {
         account::create_account_for_test(signer::address_of(aptos));
 
         // initializes coin, which is required for transfers
-        coin::register<AptosCoin>(myself);
         initialize_for_test(myself, aptos);
+        coin::register<AptosCoin>(myself);
 
         assert!(fund_destination_address() == signer::address_of(myself), 5);
         set_fund_destination_address(myself, signer::address_of(rando));
@@ -499,8 +499,8 @@ module aptos_names::config {
         account::create_account_for_test(signer::address_of(rando));
         account::create_account_for_test(signer::address_of(aptos));
 
-        coin::register<AptosCoin>(myself);
         initialize_for_test(myself, aptos);
+        coin::register<AptosCoin>(myself);
 
         assert!(fund_destination_address() == signer::address_of(myself), 5);
         set_fund_destination_address(rando, signer::address_of(rando));

core_v2/sources/v2_1_config.move

@@ -314,8 +314,8 @@ module aptos_names_v2_1::v2_1_config {
         account::create_account_for_test(signer::address_of(aptos));
 
         // initializes coin, which is required for transfers
-        coin::register<AptosCoin>(myself);
         initialize_for_test(myself, aptos);
+        coin::register<AptosCoin>(myself);
 
         assert!(is_enabled(), 0);
         set_is_enabled(myself, false);
@@ -348,8 +348,8 @@ module aptos_names_v2_1::v2_1_config {
         account::create_account_for_test(signer::address_of(aptos));
 
         // initializes coin, which is required for transfers
-        coin::register<AptosCoin>(myself);
         initialize_for_test(myself, aptos);
+        coin::register<AptosCoin>(myself);
 
         assert!(fund_destination_address() == signer::address_of(myself), 5);
         set_fund_destination_address(myself, signer::address_of(rando));
@@ -363,8 +363,8 @@ module aptos_names_v2_1::v2_1_config {
         account::create_account_for_test(signer::address_of(rando));
         account::create_account_for_test(signer::address_of(aptos));
 
-        coin::register<AptosCoin>(myself);
         initialize_for_test(myself, aptos);
+        coin::register<AptosCoin>(myself);
 
         assert!(fund_destination_address() == signer::address_of(myself), 5);
         set_fund_destination_address(rando, signer::address_of(rando));

core_v2/sources/v2_1_domains.move

@@ -723,6 +723,10 @@ module aptos_names_v2_1::v2_1_domains {
         if (option::is_none(&reverse_record.token_addr)) {
             return option::none()
         };
+        let record_obj = object::address_to_object<NameRecord>(*option::borrow(&reverse_record.token_addr));
+        if (!object::owns(record_obj, account_addr)) {
+            return option::none()
+        };
         let token_addr = *option::borrow(&reverse_record.token_addr);
         let record = borrow_global<NameRecord>(token_addr);
 

router/sources/tests/primary_name_tests.move

@@ -1,13 +1,15 @@
 #[test_only]
 module router::primary_name_tests {
+    use aptos_framework::object;
+    use aptos_framework::timestamp;
+    use aptos_names_v2_1::v2_1_config;
     use router::router;
     use router::router_test_helper;
-    use std::option;
     use std::option::Option;
+    use std::option;
     use std::signer::address_of;
     use std::string::{utf8, String};
     use std::vector;
-    use aptos_framework::object;
 
     const SECONDS_PER_YEAR: u64 = 60 * 60 * 24 * 365;
 
@@ -400,4 +402,105 @@ module router::primary_name_tests {
             assert!(option::is_none(&v2_primary_subdomain_name), 5);
         };
     }
-}
+
+    #[test(
+        router = @router,
+        aptos_names = @aptos_names,
+        aptos_names_v2_1 = @aptos_names_v2_1,
+        user1 = @0x077,
+        user2 = @0x266f,
+        aptos = @0x1,
+        foundation = @0xf01d
+    )]
+    fun test_primary_name_expiration_and_reassignment(
+        router: &signer,
+        aptos_names: &signer,
+        aptos_names_v2_1: &signer,
+        user1: signer,
+        user2: signer,
+        aptos: signer,
+        foundation: signer
+    ) {
+        router::init_module_for_test(router);
+
+        // Bump mode
+        router::set_mode(router, 1);
+
+        let users = router_test_helper::e2e_test_setup(aptos_names, aptos_names_v2_1, user1, &aptos, user2, &foundation);
+        let user1 = vector::borrow(&users, 0);
+        let user2 = vector::borrow(&users, 1);
+        let user1_addr = address_of(user1);
+        let user2_addr = address_of(user2);
+        let domain_name = utf8(b"test");
+        let subdomain_name = utf8(b"test");
+        let subdomain_name_opt = option::some(subdomain_name);
+
+        // Register with v1
+        router::register_domain(user1, domain_name, SECONDS_PER_YEAR, option::none(), option::none());
+
+        router::register_subdomain(
+            user1,
+            domain_name,
+            subdomain_name,
+            SECONDS_PER_YEAR,
+            0,
+            false,
+            option::none(),
+            option::none(),
+        );
+
+        router::set_primary_name(user1, domain_name, subdomain_name_opt);
+
+        // Check that primary name is set properly
+        {
+            let (user1_primary_subdomain_name, user1_primary_domain_name) = get_v2_primary_name(user1_addr);
+            assert!(option::some(domain_name) == user1_primary_domain_name, 3);
+            assert!(option::some(subdomain_name) == user1_primary_subdomain_name, 4);
+        };
+
+        // Expire the primary name
+        v2_1_config::set_reregistration_grace_sec(aptos_names_v2_1, 0);
+        timestamp::update_global_time_for_test_secs(SECONDS_PER_YEAR + 1);
+
+        // Check that the primary name is no longer set
+        {
+            let (user1_primary_subdomain_name, user1_primary_domain_name) = get_v2_primary_name(user1_addr);
+            assert!(option::is_none(&user1_primary_domain_name), 1);
+            assert!(option::is_none(&user1_primary_subdomain_name), 2);
+        };
+
+        // Check that user1 no longer has a primary name
+        let (user1_primary_subdomain_name, user1_primary_domain_name) = get_v1_primary_name(user1_addr);
+        assert!(option::is_none(&user1_primary_domain_name), 1);
+        assert!(option::is_none(&user1_primary_subdomain_name), 2);
+
+        // Register with user2
+        router::register_domain(user2, domain_name, SECONDS_PER_YEAR, option::none(), option::none());
+
+        router::register_subdomain(
+            user2,
+            domain_name,
+            subdomain_name,
+            SECONDS_PER_YEAR * 2,
+            0,
+            false,
+            option::none(),
+            option::none(),
+        );
+
+        router::set_primary_name(user2, domain_name, subdomain_name_opt);
+
+        // The primary name is no longer expired, so check again that user1 no
+        // longer has a primary name
+        {
+            let (user1_primary_subdomain_name, user1_primary_domain_name) = get_v2_primary_name(user1_addr);
+            assert!(option::is_none(&user1_primary_domain_name), 1);
+            assert!(option::is_none(&user1_primary_subdomain_name), 2);
+        };
+
+        // Check that user2 has the primary name
+        let (user2_primary_subdomain_name, user2_primary_domain_name) = get_v2_primary_name(user2_addr);
+        assert!(option::some(domain_name) == user2_primary_domain_name, 3);
+        assert!(option::some(subdomain_name) == user2_primary_subdomain_name, 4);
+    }
+}