Update contracts to Move 2 to have a better example of a good contract#194
Open
gregnazario wants to merge 5 commits into
Open
Update contracts to Move 2 to have a better example of a good contract#194gregnazario wants to merge 5 commits into
gregnazario wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the Aptos Names / router contracts and tests to Move 2 syntax, updating storage access patterns and modernizing common stdlib usage.
Changes:
- Replaces
borrow_global(_mut)/vector::borrow/option::is_*patterns with Move 2-style indexing, method calls, andforloops. - Updates multiple packages’
Move.tomlfiles withdev-addressesto support Move 2 examples and local testing. - Refactors core, router, and v2.1 modules to use new storage and string/vector APIs.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| transfer/Move.toml | Updates dev addresses used by the transfer package for the Move 2 example setup |
| router/sources/tests/target_address_tests.move | Updates tests to Move 2 vector/option idioms (&users[i], .borrow(), .is_none()) |
| router/sources/tests/subdomain_transfer_tests.move | Updates test indexing to Move 2 vector indexing |
| router/sources/tests/router_tests.move | Modernizes option checks and loop increment syntax in tests |
| router/sources/tests/router_test_helper.move | Refactors account setup loop to Move 2 for + vector indexing |
| router/sources/tests/router_management_tests.move | Updates vector/option usage patterns to Move 2 in management tests |
| router/sources/tests/renewal_domain_tests.move | Updates test indexing to Move 2 vector indexing |
| router/sources/tests/registration_tests.move | Updates option borrow/is_none calls to Move 2 method style |
| router/sources/tests/primary_name_tests.move | Updates helper and assertions to Move 2 option/vector idioms |
| router/sources/tests/migration_tests.move | Updates option borrow/is_none calls to Move 2 method style |
| router/sources/tests/domain_admin_tests.move | Updates test indexing to Move 2 vector indexing |
| router/sources/router.move | Migrates router module storage access and option operations to Move 2 patterns |
| router/Move.toml | Adds dev-addresses (incl. router) to support Move 2 example builds/tests |
| register/Move.toml | Adds dev-addresses to support local Move 2 testing |
| distribute/Move.toml | Adds dev-addresses and restores aptos_names dependency after framework deps |
| core_v2/sources/v2_1_token_helper.move | Updates option/string usage and visibility syntax toward Move 2 style |
| core_v2/sources/v2_1_string_validator.move | Refactors vector operations and loops to Move 2 method/indexing syntax |
| core_v2/sources/v2_1_domains.move | Migrates global resource access + option usage to Move 2 patterns |
| core_v2/sources/v2_1_config.move | Migrates config resource accessors/mutators to Move 2 storage syntax |
| core_v2/sources/tests/v2_1_test_helper.move | Refactors helper logic to Move 2 option/vector/string method calls |
| core_v2/sources/tests/v2_1_subdomain_e2e_tests.move | Updates tests to Move 2 vector indexing and option methods |
| core_v2/sources/tests/v2_1_domain_e2e_tests.move | Updates tests to Move 2 vector indexing and option methods |
| core_v2/Move.toml | Adds dev-addresses for local Move 2 workflows |
| core/sources/verify.move | Updates visibility syntax (public(friend) → friend) |
| core/sources/utf8_utils.move | Refactors vector ops and loops to Move 2 method/indexing syntax |
| core/sources/token_helper.move | Migrates token helper to Move 2 storage and string/option method style |
| core/sources/test_helper.move | Refactors helper logic to Move 2 option/vector/string method calls |
| core/sources/subdomain_e2e_tests.move | Updates e2e tests to Move 2 vector indexing and vector literal syntax |
| core/sources/price_model.move | Refactors arithmetic/loop style to Move 2 idioms |
| core/sources/is_enabled_tests.move | Updates tests to Move 2 vector indexing |
| core/sources/domains.move | Migrates domain registry & reverse lookup logic to Move 2 storage/table APIs |
| core/sources/domain_e2e_tests.move | Updates e2e tests and one loop to Move 2 for range syntax |
| core/sources/config.move | Migrates config storage access and vector ops to Move 2 patterns |
| core/Move.toml | Adds dev-addresses for local Move 2 workflows |
| bulk_migrate/Move.toml | Adds dev-addresses to support local Move 2 workflows |
| bulk_force_renewal/Move.toml | Adds missing named addresses + dev-addresses for Move 2 builds |
| bulk_clear/Move.toml | Adds missing named addresses + dev-addresses for Move 2 builds |
| bulk/sources/bulk_tests.move | Updates tests to Move 2 vector indexing |
| bulk/sources/bulk.move | Refactors bulk operations loops/indexing to Move 2 for + vec[i] |
| bulk/Move.toml | Adds named addresses and dev-addresses for Move 2 builds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+42
to
46
| let combined = subdomain_name.borrow_mut(); | ||
| combined.append_utf8(DOMAIN_DELIMITER); | ||
| combined.append(domain_name); | ||
| *combined | ||
| } |
There was a problem hiding this comment.
This returns *combined after mutably borrowing the String inside an Option<String>. Moving a String out of a mutable borrow is invalid (and will fail to compile). Instead, extract/move the String out of the Option<String> first (e.g., via an extract/unwrap pattern), mutate the owned String, and return that owned value.
Comment on lines
+88
to
99
| for (idx in 0..domain_names.length()) { | ||
| router::register_subdomain( | ||
| domain_admin, | ||
| *vector::borrow(&domain_names, idx), | ||
| *vector::borrow(&subdomain_names, idx), | ||
| *vector::borrow(&expiration_time_secs, idx), | ||
| *vector::borrow(&expiration_policies, idx), | ||
| *vector::borrow(&transferrable, idx), | ||
| option::some(*vector::borrow(&target_addrs, idx)), | ||
| option::some(*vector::borrow(&to_addrs, idx)), | ||
| domain_names[idx], | ||
| subdomain_names[idx], | ||
| expiration_time_secs[idx], | ||
| expiration_policies[idx], | ||
| transferrable[idx], | ||
| option::some(target_addrs[idx]), | ||
| option::some(to_addrs[idx]), | ||
| ); | ||
| idx = idx + 1 | ||
| } |
There was a problem hiding this comment.
This loop indexes subdomain_names, expiration_time_secs, expiration_policies, transferrable, target_addrs, and to_addrs using domain_names.length() without validating they all have the same length. If any vector is shorter, this will abort due to out-of-bounds indexing. Add explicit length equality asserts for each parallel vector (similar to the earlier functions in this module).
Dev-addresses are removed so CI works with the old named-addresses system. Hardcode repository address in bulk_migrate and bulk_force_renewal to match register and bulk_clear.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.