Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Adds reverse lookup support (primary name) #4

Merged
merged 16 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/Move.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ aptos_names_funds = "0x78ee3915e67ef5d19fa91d1e05e60ae08751efd12ce58e23fc1109de8

[dependencies.AptosFramework]
git = 'https://github.com/aptos-labs/aptos-core.git'
rev = 'main'
rev = 'mainnet'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this from main to mainnet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran into build error when using main.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were the errors you ran into?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check out the github action presubmit from right before my commit which updates this file: https://github.com/aptos-labs/aptos-names-contracts/actions/runs/3705462363/jobs/6279398045

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's strange 🤔 Can you try framework-mainnet instead of mainnet? This is the separate branch to track mainnet Move binaries.

subdir = 'aptos-move/framework/aptos-framework'

[dependencies.AptosToken]
git = 'https://github.com/aptos-labs/aptos-core.git'
rev = 'main'
rev = 'mainnet'
subdir = 'aptos-move/framework/aptos-token'
133 changes: 120 additions & 13 deletions core/sources/domains.move
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ module aptos_names::domains {
registry: Table<NameRecordKeyV1, NameRecordV1>,
}

struct ReverseLookupRegistryV1 has key, store {
0xChucky marked this conversation as resolved.
Show resolved Hide resolved
Copy link

@geekflyer geekflyer Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a name like PrimaryNameByAddressRegistryV1 would a better name to indicate that this map can only be used to reverse lookup primary names, not secondary ones.

registry: Table<address, NameRecordKeyV1>
}

struct SetReverseLookupEventsV1 has key, store {
set_reverse_lookup_events: event::EventHandle<SetReverseLookupEventV1>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for keeping this separate from ReverseLookupRegistryV1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separating events from their data is the pattern we went with, so I'm just sticking to that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is helpful or just spammy and inefficient storage wise (1 extra resource that contains 1 single event). Why not just add it to ReverseLookupRegistryV1? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway this is likely not a blocker.

}

/// Holder for `SetNameAddressEventV1` events
struct SetNameAddressEventsV1 has key, store {
set_name_events: event::EventHandle<SetNameAddressEventV1>,
Expand All @@ -81,6 +89,12 @@ module aptos_names::domains {
register_name_events: event::EventHandle<RegisterNameEventV1>,
}

struct SetReverseLookupEventV1 has drop, store {
subdomain_name: Option<String>,
domain_name: String,
target_address: Option<address>,
}

/// A name (potentially subdomain) has had it's address changed
/// This could be to a new address, or it could have been cleared
struct SetNameAddressEventV1 has drop, store {
Expand Down Expand Up @@ -135,7 +149,19 @@ module aptos_names::domains {
token_helper::initialize(account);
}

fun register_domain_generic(sign: &signer, domain_name: String, num_years: u8) acquires NameRegistryV1, RegisterNameEventsV1, SetNameAddressEventsV1 {
public fun init_reverse_lookup_registry_v1(account: &signer) {
assert!(signer::address_of(account) == @aptos_names, ENOT_AUTHORIZED);
0xChucky marked this conversation as resolved.
Show resolved Hide resolved

move_to(account, ReverseLookupRegistryV1 {
registry: table::new()
});

move_to(account, SetReverseLookupEventsV1 {
set_reverse_lookup_events: account::new_event_handle<SetReverseLookupEventV1>(account),
});
}

fun register_domain_generic(sign: &signer, domain_name: String, num_years: u8) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
assert!(config::is_enabled(), error::unavailable(ENOT_ENABLED));
assert!(num_years > 0 && num_years <= config::max_number_of_years_registered(), error::out_of_range(EINVALID_NUMBER_YEARS));

Expand All @@ -155,18 +181,16 @@ module aptos_names::domains {
coin::transfer<AptosCoin>(sign, config::fund_destination_address(), price);

register_name_internal(sign, subdomain_name, domain_name, registration_duration_secs, price);
// Automatically set the name to point to the sender's address
set_name_address_internal(subdomain_name, domain_name, signer::address_of(sign));
}

/// A wrapper around `register_name` as an entry function.
/// Option<String> is not currently serializable, so we have these convenience methods
public entry fun register_domain(sign: &signer, domain_name: String, num_years: u8) acquires NameRegistryV1, RegisterNameEventsV1, SetNameAddressEventsV1 {
public entry fun register_domain(sign: &signer, domain_name: String, num_years: u8) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
assert!(config::unrestricted_mint_enabled(), error::permission_denied(EVALID_SIGNATURE_REQUIRED));
register_domain_generic(sign, domain_name, num_years);
}

public entry fun register_domain_with_signature(sign: &signer, domain_name: String, num_years: u8, signature: vector<u8>) acquires NameRegistryV1, RegisterNameEventsV1, SetNameAddressEventsV1 {
public entry fun register_domain_with_signature(sign: &signer, domain_name: String, num_years: u8, signature: vector<u8>) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
let account_address = signer::address_of(sign);
verify::assert_register_domain_signature_verifies(signature, account_address, domain_name);
register_domain_generic(sign, domain_name, num_years);
Expand All @@ -175,7 +199,7 @@ module aptos_names::domains {
/// A wrapper around `register_name` as an entry function.
/// Option<String> is not currently serializable, so we have these convenience method
/// `expiration_time_sec` is the timestamp, in seconds, when the name expires
public entry fun register_subdomain(sign: &signer, subdomain_name: String, domain_name: String, expiration_time_sec: u64) acquires NameRegistryV1, RegisterNameEventsV1 {
public entry fun register_subdomain(sign: &signer, subdomain_name: String, domain_name: String, expiration_time_sec: u64) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
assert!(config::is_enabled(), error::unavailable(ENOT_ENABLED));

assert!(name_is_registerable(option::some(subdomain_name), domain_name), error::invalid_state(ENAME_NOT_AVAILABLE));
Expand Down Expand Up @@ -203,7 +227,7 @@ module aptos_names::domains {
/// For domains, the registration duration is only allowed to be in increments of 1 year, for now
/// Since the owner of the domain is the only one that can create the subdomain, we allow them to decide how long they want the underlying registration to be
/// The maximum subdomain registration duration is limited to the duration of its parent domain registration
fun register_name_internal(sign: &signer, subdomain_name: Option<String>, domain_name: String, registration_duration_secs: u64, price: u64) acquires NameRegistryV1, RegisterNameEventsV1 {
fun register_name_internal(sign: &signer, subdomain_name: Option<String>, domain_name: String, registration_duration_secs: u64, price: u64) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
let aptos_names = borrow_global_mut<NameRegistryV1>(@aptos_names);

let name_expiration_time_secs = timestamp::now_seconds() + registration_duration_secs;
Expand Down Expand Up @@ -232,6 +256,16 @@ module aptos_names::domains {

table::upsert(&mut aptos_names.registry, name_record_key, name_record);

let account_addr = signer::address_of(sign);
let reverse_lookup_result = get_reverse_lookup(account_addr);
if (option::is_none(&reverse_lookup_result)) {
// If the user has no reverse lookup set, set the user's reverse lookup.
set_reverse_lookup(sign, &NameRecordKeyV1 { subdomain_name, domain_name });
} else if (option::is_none(&subdomain_name)) {
// Automatically set the name to point to the sender's address
set_name_address_internal(subdomain_name, domain_name, signer::address_of(sign));
movekevin marked this conversation as resolved.
Show resolved Hide resolved
};

event::emit_event<RegisterNameEventV1>(
&mut borrow_global_mut<RegisterNameEventsV1>(@aptos_names).register_name_events,
RegisterNameEventV1 {
Expand Down Expand Up @@ -265,15 +299,15 @@ module aptos_names::domains {
/// The `registration_duration_secs` parameter is the number of seconds to register the domain for, but is not limited to the maximum set in the config for domains registered normally.
/// This allows, for example, to create a domain for the system address for 100 years so we don't need to worry about expiry
/// Or for moderation purposes, it allows us to seize a racist/harassing domain for 100 years, and park it somewhere safe
public entry fun force_create_or_seize_domain_name(sign: &signer, domain_name: String, registration_duration_secs: u64) acquires NameRegistryV1, RegisterNameEventsV1 {
public entry fun force_create_or_seize_domain_name(sign: &signer, domain_name: String, registration_duration_secs: u64) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
force_create_or_seize_name(sign, option::none(), domain_name, registration_duration_secs);
}

public entry fun force_create_or_seize_subdomain_name(sign: &signer, subdomain_name: String, domain_name: String, registration_duration_secs: u64) acquires NameRegistryV1, RegisterNameEventsV1 {
public entry fun force_create_or_seize_subdomain_name(sign: &signer, subdomain_name: String, domain_name: String, registration_duration_secs: u64) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
force_create_or_seize_name(sign, option::some(subdomain_name), domain_name, registration_duration_secs);
}

public fun force_create_or_seize_name(sign: &signer, subdomain_name: Option<String>, domain_name: String, registration_duration_secs: u64) acquires NameRegistryV1, RegisterNameEventsV1 {
public fun force_create_or_seize_name(sign: &signer, subdomain_name: Option<String>, domain_name: String, registration_duration_secs: u64) acquires NameRegistryV1, RegisterNameEventsV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
config::assert_signer_is_admin(sign);
register_name_internal(sign, subdomain_name, domain_name, registration_duration_secs, 0);
}
Expand Down Expand Up @@ -361,16 +395,16 @@ module aptos_names::domains {
}
}

public entry fun set_domain_address(sign: &signer, domain_name: String, new_address: address) acquires NameRegistryV1, SetNameAddressEventsV1 {
public entry fun set_domain_address(sign: &signer, domain_name: String, new_address: address) acquires NameRegistryV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
set_name_address(sign, option::none(), domain_name, new_address);
}


public entry fun set_subdomain_address(sign: &signer, subdomain_name: String, domain_name: String, new_address: address) acquires NameRegistryV1, SetNameAddressEventsV1 {
public entry fun set_subdomain_address(sign: &signer, subdomain_name: String, domain_name: String, new_address: address) acquires NameRegistryV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
set_name_address(sign, option::some(subdomain_name), domain_name, new_address);
}

public fun set_name_address(sign: &signer, subdomain_name: Option<String>, domain_name: String, new_address: address) acquires NameRegistryV1, SetNameAddressEventsV1 {
public fun set_name_address(sign: &signer, subdomain_name: Option<String>, domain_name: String, new_address: address) acquires NameRegistryV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
let signer_addr = signer::address_of(sign);
let (is_owner, token_id) = is_owner_of_name(signer_addr, subdomain_name, domain_name);
assert!(is_owner, error::permission_denied(ENOT_OWNER_OF_NAME));
Expand All @@ -379,6 +413,27 @@ module aptos_names::domains {
let (_property_version, expiration_time_sec, _target_address) = get_name_record_v1_props(&name_record);
let (property_keys, property_values, property_types) = get_name_property_map(subdomain_name, expiration_time_sec);
token_helper::set_token_props(signer_addr, property_keys, property_values, property_types, token_id);

// If the signer's reverse lookup is the domain, and the new address is not the signer, clear the signer's reverse lookup.
// Example:
// The current state is bob.apt points to @a and the reverse lookup of @a points to bob.apt.
// The owner wants to set bob.apt to point to @b.
// The new state should be bob.apt points to @b, and the reverse lookup of @a should be none.
let maybe_reverse_lookup = get_reverse_lookup(signer_addr);
if (option::is_none(&maybe_reverse_lookup)) {
return
};
let current_reverse_lookup = option::borrow(&maybe_reverse_lookup);
let key = NameRecordKeyV1 { subdomain_name, domain_name };
if (*current_reverse_lookup == key && signer_addr != new_address) {
let registry = &mut borrow_global_mut<ReverseLookupRegistryV1>(@aptos_names).registry;
table::remove<address, NameRecordKeyV1>(registry, signer_addr);
emit_set_reverse_lookup_event_v1(
subdomain_name,
domain_name,
option::none()
);
};
}

fun set_name_address_internal(subdomain_name: Option<String>, domain_name: String, new_address: address): NameRecordV1 acquires NameRegistryV1, SetNameAddressEventsV1 {
Expand Down Expand Up @@ -438,6 +493,39 @@ module aptos_names::domains {
};
}

public fun set_reverse_lookup(account: &signer, key: &NameRecordKeyV1) acquires NameRegistryV1, ReverseLookupRegistryV1, SetNameAddressEventsV1, SetReverseLookupEventsV1 {
0xChucky marked this conversation as resolved.
Show resolved Hide resolved
set_reverse_lookup_internal(account, key);

let account_addr = signer::address_of(account);
let (maybe_subdomain_name, domain_name) = get_name_record_key_v1_props(key);
set_name_address(account, maybe_subdomain_name, domain_name, account_addr);
}

public fun get_reverse_lookup(account_addr: address): Option<NameRecordKeyV1> acquires ReverseLookupRegistryV1 {
let registry = &borrow_global_mut<ReverseLookupRegistryV1>(@aptos_names).registry;
if (table::contains(registry, account_addr)) {
option::some(*table::borrow(registry, account_addr))
} else {
option::none()
}
}

fun set_reverse_lookup_internal(account: &signer, key: &NameRecordKeyV1) acquires ReverseLookupRegistryV1, SetReverseLookupEventsV1 {
let account_addr = signer::address_of(account);
let (maybe_subdomain_name, domain_name) = get_name_record_key_v1_props(key);
let (is_owner, _) = is_owner_of_name(account_addr, maybe_subdomain_name, domain_name);
assert!(is_owner, ENOT_AUTHORIZED);
0xChucky marked this conversation as resolved.
Show resolved Hide resolved

let registry = &mut borrow_global_mut<ReverseLookupRegistryV1>(@aptos_names).registry;
table::upsert(registry, account_addr, *key);

emit_set_reverse_lookup_event_v1(
maybe_subdomain_name,
domain_name,
option::some(account_addr)
);
}

fun emit_set_name_address_event_v1(subdomain_name: Option<String>, domain_name: String, property_version: u64, expiration_time_secs: u64, new_address: Option<address>) acquires SetNameAddressEventsV1 {
let event = SetNameAddressEventV1 {
subdomain_name,
Expand All @@ -453,6 +541,19 @@ module aptos_names::domains {
);
}

fun emit_set_reverse_lookup_event_v1(subdomain_name: Option<String>, domain_name: String, target_address: Option<address>) acquires SetReverseLookupEventsV1 {
let event = SetReverseLookupEventV1 {
subdomain_name,
domain_name,
target_address,
};

event::emit_event<SetReverseLookupEventV1>(
&mut borrow_global_mut<SetReverseLookupEventsV1>(@aptos_names).set_reverse_lookup_events,
event,
);
}

public fun get_name_property_map(subdomain_name: Option<String>, expiration_time_sec: u64): (vector<String>, vector<vector<u8>>, vector<String>) {
let type;
if (option::is_some(&subdomain_name)) {
Expand Down Expand Up @@ -499,6 +600,7 @@ module aptos_names::domains {
#[test_only]
public fun init_module_for_test(account: &signer) {
init_module(account);
init_reverse_lookup_registry_v1(account);
}

#[test_only]
Expand All @@ -511,6 +613,11 @@ module aptos_names::domains {
event::counter(&borrow_global<RegisterNameEventsV1>(@aptos_names).register_name_events)
}

#[test_only]
public fun get_set_reverse_lookup_event_v1_count(): u64 acquires SetReverseLookupEventsV1 {
event::counter(&borrow_global<SetReverseLookupEventsV1>(@aptos_names).set_reverse_lookup_events)
}

#[test(aptos = @0x1)]
fun test_time_is_expired(aptos: &signer) {
timestamp::set_time_has_started_for_testing(aptos);
Expand Down
Loading