Transfer existing subdomains to new addresses #88

asimm241 · 2022-05-10T11:40:19Z

Use the following template to create your pull request
This PR implements a new endpoint /transfer that enables users to transfer subdomains to the new addresses.
For further details #79 , hirosystems/stacks.js#1209

Type of Change

New feature

Does this introduce a breaking change?

No, it should not

Testing information

unit tests are yet to be added

Checklist

Code is commented where needed
Unit test coverage for new or modified code paths
npm run test passes
Changelog is updated
Tag 1 of @person1 or @Person2

codecov · 2022-05-10T11:42:10Z

Codecov Report

Merging #88 (a8d0578) into master (41367a8) will decrease coverage by 0.86%.
The diff coverage is 34.93%.

@@            Coverage Diff             @@
##           master      #88      +/-   ##
==========================================
- Coverage   58.40%   57.53%   -0.87%     
==========================================
  Files           8        8              
  Lines         666      756      +90     
==========================================
+ Hits          389      435      +46     
- Misses        277      321      +44

Impacted Files	Coverage Δ
src/http.js	`0.00% <0.00%> (ø)`
src/operations.js	`61.94% <35.89%> (ø)`
src/server.js	`88.18% <80.00%> (-0.65%)`	⬇️
src/db.js	`89.06% <85.71%> (-0.42%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2443e75...a8d0578. Read the comment docs.

markmhendrickson · 2022-05-10T12:59:55Z

cc @pradel

markmhendrickson · 2022-05-18T15:12:44Z

Is there any way we can set up a mainnet-configured instance of the registrar with this functionality so we can "test in prod" so to speak? It doesn't appear that @pradel or Hiro has relevant testnet-based usernames to use for testing here.

pradel · 2022-05-30T10:54:10Z

Hey there, any update?

janniks · 2022-06-07T11:45:18Z

@saralab could you coordinate w/ tintash and devops, if we want/need a mainnet-beta instance to test this branch and next steps?

CharlieC3 · 2022-06-13T14:41:39Z

This PR is deployed to https://registrar-beta.stacks.co/index/ !
cc @asimm241 @markmhx @pradel @janniks

pradel · 2022-06-17T16:44:59Z

Thanks @CharlieC3 I couldn't try it for now as it looks like the cli function is not working properly hirosystems/stacks.js#1226 (comment)

janniks · 2022-06-30T20:51:19Z

Thanks @CharlieC3 for hosting the new instance. 🙏

@asimm241 is the subdomain data only stored in the database or is a hash/tx sent to the blockchain for each username?

I ask because the registrar and registrar-beta are not "in-sync", e.g.

In other words: is this something that will sync at some point / somehow (because, for example, we only poll from the chain every so often), or is this service so off-chain that the two registrar instances will always have separate databases/states?

If the latter holds true, is it possible to clone the prod data from registrar to registrar-beta? @CharlieC3

CharlieC3 · 2022-06-30T21:01:36Z

@janniks the two deployments are using separate databases, and my understanding is that the registrar DB gets written to by a processor behind btc.us (correct me if I'm wrong). Since the processor is not pointing to registrar-beta, it will not change state after initial boot.

If you need this re-synced, can we coordinate on a day/time you're available to use it soon after it's complete to conduct your testing?

janniks · 2022-06-30T22:27:02Z

Got it. 🙏 I tried a few different things now, and replayed some sniffed traffic locally to generate some data on beta — this should be enough for a test-case. Will let you know if we need more 😉

janniks · 2022-07-01T19:52:20Z

During testing for hirosystems/stacks.js#1226 I came across this issue:
The registrar returns Failed to validate your transfer request: signature error: SP1JCBTBSVHSGEFJ6T08567DTRVDSD7CHC6YRHRPD Cannot read property 'length' of undefined for (I think) a valid subdomain operation.

To reproduce we can use the alpha CLI build:

npm install @stacks/[email protected] -g
stx migrate_subdomains "parrot guitar tomorrow old burst grant champion require orbit slush cycle chimney" https://registrar-beta.stacks.co

This command generates the following op:

Subdomain Operation Payload = {
  subdomains_list: [
    {
      subdomainName: 'migration_test_02',
      owner: 'SP1JCBTBSVHSGEFJ6T08567DTRVDSD7CHC6YRHRPD',
      zonefile: '$ORIGIN migration_test_02.id.stx\n' +
        '$TTL 3600\n' +
        '_http._tcp\tIN\tURI\t10\t1\t"https://gaia.blockstack.org/hub/19cBN1PUh9bZay5HV1HAzvU8Ehg9CfosRu/profile.json"\n' +
        '\n',
      sequenceNumber: 1,
      signature: '007608f0ba332b477ab8e1de1250d75a33c30c0940319af3e534d141b6ada88a1204389cd630167488bbe8b5884409b0ac2905fe1ae4114c202a3655a6bf4f8e20'
    }
  ]
}

...and fails (I believe on this line) with:

Failed to validate your transfer request: signature error: SP1JCBTBSVHSGEFJ6T08567DTRVDSD7CHC6YRHRPD Cannot read property 'length' of undefined

@asimm241 Can you check the cause for this?

asimm241 · 2022-07-01T19:59:17Z

@janniks how did you create signature?

janniks · 2022-07-01T22:08:27Z

@janniks how did you create signature?

like this →

janniks · 2022-07-06T22:55:37Z

I finally found the discrepancy.

Both the CLI and the registrar use a method subdomainOpToZFPieces.

The CLI has an additional first line including only the subdomain name. I'm not sure why this was added, but I'm assuming the CLI is incorrect, as it was never merged and is only added in the current feature branch. @kantai can you confirm?

I'll go ahead and stick with the registrar implementation for the next tests.

janniks · 2022-07-06T23:52:59Z

Also pushed a tiny fix in f577b6b. @CharlieC3 what's the process for deploying this branch to registrar-beta?

CharlieC3 · 2022-07-07T02:51:32Z

@janniks The branch is already deployed, to run the latest image the pod just needs to be killed so it can be pulled down again. It should be running now!

janniks · 2022-07-07T02:52:45Z

Perfect thanks 🙏🏻
Will test first thing tmrw

src/server.js

janniks · 2022-07-07T14:57:56Z

src/server.js

+      }
+      let address = ''
+
+      if (this.regtest) {


Maybe we'll leave regtest since it isn't used. But this check seems incorrect. I'll move to comparing with the ~~set network in bskConfig~~ ENV variables

janniks · 2022-07-07T15:35:41Z

Ah yes, that's what I meant. Can I get a kill+redeploy?

Also, are the ENV settings otherwise the same as on prod?

CharlieC3 · 2022-07-07T16:28:37Z

@janniks Done

janniks · 2022-07-07T21:21:39Z

omg, it works! i almost can't believe it 🎉

{
  "status": true,
  "message": "Your subdomains transfer was received, and will \n            be included in the blockchain soon with txId: e2ba5019a5b9e4d7e41671db06c3ed218250462b82c9a6c2dcd5ff439070788a"
}

explorer →

janniks

added some fixes
added a very basic test that triggers validation and storage

@kantai i left the commits as history (we can rebase or sqaush-merge later), so it's more clear what behavior i changed — since i can't completely tell what the state should look like during/after a transfer... should be reviewable as a whole, but if unsure you can also check out the individual changes, in-case i removed something important.

there is still some weird naming/structure from the original change, but i can't prioritize this too much atm, so i'm for keeping the technical debt. (if we want to focus on removing debt, we should start by moving to typescript)

pradel · 2022-07-13T12:40:43Z

@janniks I created a pull request some time ago to upgrade the codebase to typescript incrementally #84

janniks · 2022-07-13T12:41:47Z

Awesome thanks, will take a look! I missed that one 🙏🏻

saralab · 2022-07-20T15:09:50Z

@kantai Awaiting your review, can you take a peek please?

kantai

This LGTM

janniks · 2022-07-26T20:24:45Z

Looks good — I did a final test and am happy 👍
We can merge this (I don't have access) and deploy 🚀

CC @stacks-network/devops — Anybody feel free to merge

pradel · 2022-08-02T07:28:11Z

Hey there, is anyone able to merge this one? :) @CharlieC3

CharlieC3 · 2022-08-02T13:25:45Z

@janniks @pradel Merged! Will deploy today.

# [1.2.0](v1.1.3...v1.2.0) (2022-08-02) ### Features * transfer existing subdomains to new addresses ([#88](#88)) ([0bf15a6](0bf15a6))

blockstack-devops · 2022-08-02T13:26:31Z

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

CharlieC3 · 2022-08-02T14:49:47Z

@pradel Deployed to both mainnet and testnet!

feat: add support for transfer endpoint for subdomain migration

389ef60

asimm241 mentioned this pull request May 10, 2022

fix: migrate subdomains to wallet key address hirosystems/stacks.js#1226

Merged

9 tasks

fix: convert to wrapped public key

f577b6b

janniks reviewed Jul 7, 2022

View reviewed changes

src/server.js Show resolved Hide resolved

janniks reviewed Jul 7, 2022

View reviewed changes

fix: update address version

680e0f6

janniks added 2 commits July 7, 2022 16:33

chore: fix hash data

2222da1

fix: update public key format and error message

498905d

janniks force-pushed the subdomain_migration branch from bc944e9 to 498905d Compare July 7, 2022 20:59

fix: set seqn to 1

4ca91f9

test: add transfer test

a8d0578

janniks approved these changes Jul 12, 2022

View reviewed changes

janniks requested a review from kantai July 12, 2022 19:49

kantai approved these changes Jul 20, 2022

View reviewed changes

CharlieC3 merged commit 0bf15a6 into master Aug 2, 2022

CharlieC3 deleted the subdomain_migration branch August 2, 2022 13:24

blockstack-devops pushed a commit that referenced this pull request Aug 2, 2022

chore(release): 1.2.0 [skip ci]

fdf8f5e

# [1.2.0](v1.1.3...v1.2.0) (2022-08-02) ### Features * transfer existing subdomains to new addresses ([#88](#88)) ([0bf15a6](0bf15a6))

blockstack-devops added the released label Aug 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transfer existing subdomains to new addresses #88

Transfer existing subdomains to new addresses #88

asimm241 commented May 10, 2022 •

edited

Loading

codecov bot commented May 10, 2022 •

edited

Loading

markmhendrickson commented May 10, 2022

markmhendrickson commented May 18, 2022

pradel commented May 30, 2022

janniks commented Jun 7, 2022

CharlieC3 commented Jun 13, 2022

pradel commented Jun 17, 2022

janniks commented Jun 30, 2022 •

edited

Loading

CharlieC3 commented Jun 30, 2022

janniks commented Jun 30, 2022 •

edited

Loading

janniks commented Jul 1, 2022

asimm241 commented Jul 1, 2022

janniks commented Jul 1, 2022

janniks commented Jul 6, 2022

janniks commented Jul 6, 2022

CharlieC3 commented Jul 7, 2022

janniks commented Jul 7, 2022

janniks Jul 7, 2022 •

edited

Loading

janniks commented Jul 7, 2022 •

edited

Loading

CharlieC3 commented Jul 7, 2022

janniks commented Jul 7, 2022 •

edited

Loading

janniks left a comment

pradel commented Jul 13, 2022

janniks commented Jul 13, 2022

saralab commented Jul 20, 2022

kantai left a comment

janniks commented Jul 26, 2022 •

edited

Loading

pradel commented Aug 2, 2022

CharlieC3 commented Aug 2, 2022

blockstack-devops commented Aug 2, 2022

CharlieC3 commented Aug 2, 2022

Transfer existing subdomains to new addresses #88

Transfer existing subdomains to new addresses #88

Conversation

asimm241 commented May 10, 2022 • edited Loading

Type of Change

Does this introduce a breaking change?

Testing information

Checklist

codecov bot commented May 10, 2022 • edited Loading

Codecov Report

markmhendrickson commented May 10, 2022

markmhendrickson commented May 18, 2022

pradel commented May 30, 2022

janniks commented Jun 7, 2022

CharlieC3 commented Jun 13, 2022

pradel commented Jun 17, 2022

janniks commented Jun 30, 2022 • edited Loading

CharlieC3 commented Jun 30, 2022

janniks commented Jun 30, 2022 • edited Loading

janniks commented Jul 1, 2022

asimm241 commented Jul 1, 2022

janniks commented Jul 1, 2022

janniks commented Jul 6, 2022

janniks commented Jul 6, 2022

CharlieC3 commented Jul 7, 2022

janniks commented Jul 7, 2022

janniks Jul 7, 2022 • edited Loading

Choose a reason for hiding this comment

janniks commented Jul 7, 2022 • edited Loading

CharlieC3 commented Jul 7, 2022

janniks commented Jul 7, 2022 • edited Loading

janniks left a comment

Choose a reason for hiding this comment

pradel commented Jul 13, 2022

janniks commented Jul 13, 2022

saralab commented Jul 20, 2022

kantai left a comment

Choose a reason for hiding this comment

janniks commented Jul 26, 2022 • edited Loading

pradel commented Aug 2, 2022

CharlieC3 commented Aug 2, 2022

blockstack-devops commented Aug 2, 2022

CharlieC3 commented Aug 2, 2022

asimm241 commented May 10, 2022 •

edited

Loading

codecov bot commented May 10, 2022 •

edited

Loading

janniks commented Jun 30, 2022 •

edited

Loading

janniks commented Jun 30, 2022 •

edited

Loading

janniks Jul 7, 2022 •

edited

Loading

janniks commented Jul 7, 2022 •

edited

Loading

janniks commented Jul 7, 2022 •

edited

Loading

janniks commented Jul 26, 2022 •

edited

Loading