Skip to content

fix: migrate subdomains to wallet key address #1226

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

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

ahsan-javaid
Copy link
Contributor

Description

Add command in cli to enable users to transfer subdomains to wallet-key addresses that correspond to all data-key addresses

Example: Running the command

stx migrate_subdomains "sound idle panel often situate develop unit text design antenna vendor screen opinion balcony share trigger accuse scatter visa uniform brass update opinion media"

For details refer to issue #1209

Type of Change

  • New feature
  • Bug fix
  • API reference/documentation update
  • Other

Does this introduce a breaking change?

No

Are documentation updates required?

No

Testing information

Provide context on how tests should be performed.
cd packages/cli
npm run build
./bin.js migrate_subdomains "sound idle panel often situate develop unit text design antenna vendor screen opinion balcony share trigger accuse scatter visa uniform brass update opinion media"

Checklist

  • Code is commented where needed
  • Unit test coverage for new or modified code paths
  • npm run test passes
  • Changelog is updated
  • Tag reviewers

@vercel
Copy link

vercel bot commented Apr 4, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/blockstack/stacks-js/HmqgQoY7Q6r5e4v1VWxR9n18kq2s
✅ Preview: https://stacks-js-git-fix-subdomain-migration-to-wall-2b3ed7-blockstack.vercel.app

@markmhendrickson
Copy link

@ahsan-javaid is this ready for testing? cc @pradel

@ahsan-javaid
Copy link
Contributor Author

Update:

  • Working on to write tests (In progress)
  • @asimm241 is developing the api on subdomain-registrar side (In progress)

However mock testing can be done on this PR
Steps:

  1. Clone stacks.js and setup
  2. checkout to branch fix/subdomain-migration-to-wallet-addresses
  3. cd packages/cli
  4. Now run the command below
  5. ./bin.js migrate_subdomains "sound idle panel often situate develop unit text design antenna vendor screen opinion balcony share trigger accuse scatter visa uniform brass update opinion media"

This will trigger the subdomain migration flow but actually subdomain will not be migrated as the actual migration url needs to be added.


I will provide the final update next week once its fully ready to test.

cc: @asimm241

@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2022

Codecov Report

Merging #1226 (1ea90ee) into master (575b58d) will increase coverage by 0.27%.
The diff coverage is 93.87%.

❗ Current head 1ea90ee differs from pull request most recent head 63c4083. Consider uploading reports for the commit 63c4083 to get more accurate results

@@            Coverage Diff             @@
##           master    #1226      +/-   ##
==========================================
+ Coverage   64.78%   65.06%   +0.27%     
==========================================
  Files         126      126              
  Lines        8753     8834      +81     
  Branches     1882     1895      +13     
==========================================
+ Hits         5671     5748      +77     
- Misses       2956     2960       +4     
  Partials      126      126              
Impacted Files Coverage Δ
packages/cli/src/argparse.ts 15.16% <ø> (ø)
packages/cli/src/cli.ts 22.94% <93.67%> (+5.40%) ⬆️
packages/cli/src/utils.ts 33.24% <94.44%> (+3.06%) ⬆️
packages/transactions/src/utils.ts 94.49% <100.00%> (+0.05%) ⬆️

@ahsan-javaid ahsan-javaid force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 2c023c8 to 0962077 Compare April 14, 2022 14:44
@ahsan-javaid ahsan-javaid changed the title [WIP] Subdomain Migration [WIP & Waiting for API] Subdomain Migration Apr 22, 2022
@markmhendrickson
Copy link

markmhendrickson commented Apr 26, 2022

@ahsan-javaid Is this ready to test by chance? And mind providing a link to the API work on the registrar?

@asimm241
Copy link
Contributor

@ahsan-javaid Is this ready to test by chance? And mind providing a link to the API work on the registrar?

@markmhx It will be ready by the end of this week.

@asimm241
Copy link
Contributor

asimm241 commented Apr 26, 2022

@ahsan-javaid Is this ready to test by chance? And mind providing a link to the API work on the registrar?

I'll open up a PR today or tomorrow for the API registrar section.

@ahsan-javaid ahsan-javaid force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 0962077 to 1d17a97 Compare April 28, 2022 12:50
@vercel
Copy link

vercel bot commented Apr 28, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
stacks-js ✅ Ready (Inspect) Visit Preview Jul 19, 2022 at 5:56PM (UTC)
stacksjs ✅ Ready (Inspect) Visit Preview Jul 19, 2022 at 5:56PM (UTC)

@markmhendrickson
Copy link

@ahsan-javaid @asimm241 Any updates on this work? I've heard from @pradel that his users are now waiting eagerly for this functionality 🙏

@asimm241
Copy link
Contributor

asimm241 commented May 9, 2022

@markmhx This PR is ready, the corresponding PR in the registrar will probably be created today OR tomorrow. Then It can be tested. The unite tests will be added later. :)

@ahsan-javaid ahsan-javaid force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 1d17a97 to 49765da Compare May 9, 2022 10:32
@asimm241
Copy link
Contributor

Corresponding PR in registrar stacks-archive/subdomain-registrar#88

@ahsan-javaid ahsan-javaid changed the title [WIP & Waiting for API] Subdomain Migration Subdomain Migration May 13, 2022
@ahsan-javaid ahsan-javaid marked this pull request as ready for review May 13, 2022 13:49
@ahsan-javaid ahsan-javaid force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 49765da to 7fac749 Compare June 16, 2022 05:25
@ahsan-javaid ahsan-javaid force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 7fac749 to ea18697 Compare June 16, 2022 05:42
@pradel
Copy link
Contributor

pradel commented Jul 14, 2022

@janniks this function will allow legacy users to migrate their usernames to the Hiro wallet.

But what about transferring subdomains for Hiro wallet users, should stacks.js export a function that does this?

@janniks janniks force-pushed the fix/subdomain-migration-to-wallet-addresses branch from b0822fa to 242c8bb Compare July 15, 2022 21:00
@janniks janniks force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 242c8bb to 7bfb84e Compare July 15, 2022 21:36
@unclemantis
Copy link

Would like to see a detailed markdown of exactly the issue being solved. I am a little confused. 👍

@janniks
Copy link
Collaborator

janniks commented Jul 19, 2022

@janniks this function will allow legacy users to migrate their usernames to the Hiro wallet.

But what about transferring subdomains for Hiro wallet users, should stacks.js export a function that does this?

That's a good point. I think they should be separate command though. For example, I would keep migrate_subdomain as the "fix" of legacy ids and add another command transfer_subdomain for the ability to send the subdomain to whoever (by address).
Thoughts?

@janniks
Copy link
Collaborator

janniks commented Jul 19, 2022

Would like to see a detailed markdown of exactly the issue being solved. I am a little confused. 👍

👍 I can add some more detail to the command descriptions. In short, the old Blockstack connect username where owned by a different derivation path of (data private key) of wallets, rather than the wallet key, which the web wallet uses now to own usernames. This command will migrate the address from the old to the current.

@janniks janniks changed the title Subdomain Migration fix: migrate subdomains to wallet key address Jul 19, 2022
@pradel
Copy link
Contributor

pradel commented Jul 19, 2022

That's a good point. I think they should be separate command though. For example, I would keep migrate_subdomain as the "fix" of legacy ids and add another command transfer_subdomain for the ability to send the subdomain to whoever (by address).
Thoughts?

Seems like a good approach 👍

@janniks janniks force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 1ea90ee to 2b0857b Compare July 19, 2022 16:02
@janniks janniks force-pushed the fix/subdomain-migration-to-wallet-addresses branch from 2b0857b to 63c4083 Compare July 19, 2022 17:54
@pradel
Copy link
Contributor

pradel commented Jul 20, 2022

@janniks created a separate issue for this #1323

@pradel
Copy link
Contributor

pradel commented Jul 20, 2022

@janniks wondering what is missing for this one (and the subdomain pr) to go live, our users are eagerly waiting 🤓

@janniks janniks removed the request for review from asimm241 July 20, 2022 15:08
@saralab
Copy link
Contributor

saralab commented Jul 20, 2022

@zone117x : We are awaiting your review, can you take a peek please?

Copy link
Member

@zone117x zone117x left a comment

Choose a reason for hiding this comment

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

The code and tests look good. But I haven't tested this out myself. If the folks testing this change are happy then LGTM.

@janniks
Copy link
Collaborator

janniks commented Jul 28, 2022

✅ Ran final tests, we're just waiting for stacks-archive/subdomain-registrar#88 to be merged (I don't have access), then we can merge this PR.

@janniks janniks merged commit 16a23c9 into master Aug 2, 2022
@janniks janniks deleted the fix/subdomain-migration-to-wallet-addresses branch August 2, 2022 15:04
@janniks
Copy link
Collaborator

janniks commented Aug 3, 2022

📦 Released under 4.3.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable users to transfer subdomains to wallet-key addresses that correspond to all data-key addresses
10 participants