Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

Renew Blockstack IDs #1938

Merged
merged 18 commits into from
Nov 20, 2019
Merged

Renew Blockstack IDs #1938

merged 18 commits into from
Nov 20, 2019

Conversation

yknl
Copy link
Collaborator

@yknl yknl commented Jul 2, 2019

This PR adds name expiry information and basic name renewal feature. Full on-chain names that have less than 60 days until expiration will see a renew button beside it. The renewal transaction will be funded by the built-in BTC wallet.

This is based on a previous PR from @friedger to display expiry information in the all profiles screen.

Renew UI in all profiles screen
image

image

Subdomains do not need to be renewed
image

Testing notes

  1. Find a name that has less than 60 days until expiration.
  2. Go to the all profiles screen.
  3. Click renew.
  4. Enter password and press renew.
  5. Wait for transaction to confirm x 10.
  6. Verify that domain name now has many more days until expiration.

@yknl yknl changed the title Renew Blockstack IDs [WIP] Renew Blockstack IDs Jul 2, 2019
@yknl yknl changed the title [WIP] Renew Blockstack IDs Renew Blockstack IDs Aug 30, 2019
@hstove
Copy link
Collaborator

hstove commented Sep 6, 2019

Find a name that has less than 60 days until expiration.

This has to be my name? I don't have any names that are that old.

@yknl
Copy link
Collaborator Author

yknl commented Sep 7, 2019

Copy link
Collaborator

@hstove hstove left a comment

Choose a reason for hiding this comment

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

I tested this out. I successfully generated a renew transaction. However, I only saw the transaction ID after renewing. I wasn't sure if that was me signing the TX, or actually submitting it. I clicked 'renew' again, assuming it would then submit the TX. Turns out that I renewed it twice :)

I would recommend changing the UX a bit by:

  • Disabling the 'renew' button after successfully submitting a renewal.
  • Adding a message that the TX has been submitted, and you should check your identities page in ~1 hour (?) to ensure that the expiration date has been updated

@hstove
Copy link
Collaborator

hstove commented Sep 11, 2019

I would also recommend some 'confirm' step before submitting the actual TX. I had no idea what the cost was going to be

@markmhendrickson
Copy link

markmhendrickson commented Sep 16, 2019

@yknl were you able to incorporate @hstove's suggested changes? They sound helpful to me. Otherwise, this looks functionally like a good addition to me. And I'm wondering if @timstackblock should QA before we go live.

@yknl
Copy link
Collaborator Author

yknl commented Sep 16, 2019

@hstove is going to take a stab at improving the UX when he gets a chance

@timstackblock
Copy link
Contributor

@markmhx I can take a look is this issue pulled into a zenhub board and ready for QA, I dont see it anywhere?

@markmhendrickson
Copy link

@timstackblock It sounds from @yknl that @hstove needs to make some UX changes then he'll move this from "In Progress" to "Review / QA" on ZenHub.

@timstackblock
Copy link
Contributor

Sure no problem just @ me when you are ready and I will jump right on it.

@moxiegirl moxiegirl added the Impacts DOCS Requires new or updates to our documentation. label Sep 19, 2019
@diwakergupta
Copy link
Collaborator

@hstove @markmhx any update here? More and more people are going to run into name expiration.

@markmhendrickson
Copy link

@diwakergupta I've made a note to discuss this during our weekly kick-off tomorrow.

However, iirc we last decided (over the summer) to keep this effort relatively de-prioritized despite the expectation that names will continue to expire since we don't have a reliable way to reach out to users prompting them to renew (e.g. via email). As such, any UX improvements to make renewals easier will have minimal impact.

@hstove hstove added this to the Browser 0.37 milestone Nov 12, 2019
@moxiegirl
Copy link
Contributor

moxiegirl commented Nov 13, 2019

@yknl Thanks. I knew how to get to the test page, it was having an id to test with that appeared to require a rebuild. Further, users can only renew with a renewable ID. And the blockstack ID is not renewable because there is no expiration, here is the error I am getting.

image

So, I have to buy an id in order to test this process or get an ID to test with.

@moxiegirl
Copy link
Contributor

moxiegirl commented Nov 13, 2019

Information related to renewal

  • first class identity is what we want to call this thing created sub to someone's Blockstack id.
  • This interface only renews .id identities
  • For non .id renewals users must use the command line
  • If you do not renew it someone else can renew the name in the grace priod
  • If you forget to renew, you have a grace period is two months; if you miss the grace period you have to re-regiser

Testing Tips from Ken

  • If you want to verify that the work make sure the name goes through.
  • Renew transaction new block will move out.

cc: @yknl please check

@moxiegirl moxiegirl closed this Nov 13, 2019
@moxiegirl moxiegirl reopened this Nov 13, 2019
@yknl
Copy link
Collaborator Author

yknl commented Nov 13, 2019

first class identity is what we want to call this thing created sub to someone's Blockstack id.

I'm not sure we've settled on this, it seems like a larger product decision. I recommend not emphasizing this for now.

For non .id renewals users must use the command line

For renewing any ID not in the .id namespace, you will need to use the Blockstack CLI tool

If you do not renew it someone else can renew the name in the grace priod

No, only the owner can renew the name during the grace period.

If you want to verify that the work make sure the name goes through.
Renew transaction new block will move out.

For internal testing, you can check that the expire_block of the name has changed to a later block after a renewal transaction has been confirmed.
https://core.blockstack.org/v1/names/blockstacker.id

@hstove hstove self-assigned this Nov 13, 2019
@hstove
Copy link
Collaborator

hstove commented Nov 13, 2019

Changes I've made:

First, there is a pre-step to get the estimated price, before you actual send the TX:

image

image

Then, I added a note that it might take 2 hours to process, and also disabled the "renew" button after it went through.

image

@hstove
Copy link
Collaborator

hstove commented Nov 13, 2019

For reviewers:

First, you need a "first class domain", as we called it, aka a name like myname.id. Visit the 'identities' page, then click 'more'. Then, you should see a message around how long until it expires, with a renew button next to it, as shown earlier in this ticket. Click the renew button to go to this page.

You need BTC to register a name and renew it. Reach out to me if you need some for testing purposes.

For testing, I've changed the logic to allow you to renew your name even if it's 2 years away. Previously, we set that to 60 days. We should consider what we want that threshold to actually be when we release it.

@hstove hstove assigned timstackblock and unassigned hstove Nov 13, 2019
Copy link
Contributor

@moxiegirl moxiegirl left a comment

Choose a reason for hiding this comment

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

@markmhx Here are my review comments.

@@ -224,8 +228,8 @@ class AllProfilesPage extends Component {
</div>
<div className="row m-t-20">
<p className="col form-text text-muted">
Have you recovered and are missing IDs? Just add them
back by using the "Add another ID" for each ID.
Have you recovered and are missing IDs? Just add them back by
Copy link
Contributor

Choose a reason for hiding this comment

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

What does recovered mean here? Recovered from what? Or is it possible to both recover and be missing an ID? Maybe you want to say:

Are you missing IDs after restoring the browser? Use Add another ID for each ID you want to add back.

return 'Renew'
}
if (estimateInProgress) {
return 'Generating estimated cost'
Copy link
Contributor

Choose a reason for hiding this comment

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

Estimating the purchase price. Price is subject to fluctuations in the price of BTC.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is the button copy - I think that's too long to put in a button. When the estimate is generated, it says "Renewing your name will cost roughly XXX BTC"

{this.state.error ? <div className="alert alert-danger">{this.state.error}</div> : null}
{success && <div className="alert alert-success">
<p>{success}</p>
<p style={{ marginBottom: 0 }}>Your renewal might take up to two hours to process</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Processing your renewal can take up to two hours.

If you don't like the rephrase, at least take a period for consistency

<p>
Transfer ownership is an advanced feature. It requires broadcasting a
transaction on Bitcoin network and costs Bitcoin.
Transfer ownership is an advanced feature. It requires
Copy link
Contributor

Choose a reason for hiding this comment

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

Transferring the ownership of a name is an advanced feature. Ownership transfer requires broadcasting a transaction on the Bitcoin network; Transactions cost Bitcoin (BTC).

or

Transferring the ownership of a name is an advanced feature. Ownership transfer requires modifying a name's zone file and broadcasting an update transaction with this change on the Bitcoin network. You must pay for the transaction costs in Bitcoin (BTC).

this.updateAlert('success', 'Broadcasting zone file update transaction...')
this.updateAlert(
'success',
'Broadcasting zone file update transaction...'
Copy link
Contributor

Choose a reason for hiding this comment

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

Broadcasting name transfer update transaction.

We didn't previously mention the zone file. Are users actually likely to understand what a zone file is? If you use the text I gave you about the zone file up above, then STET this.

Updating your zone file is an advanced feature that can break
your Blockstack name and profile. It requires broadcasting a
transaction on Bitcoin network and costs Bitcoin.
Updating your zone file is an advanced feature that can break
Copy link
Contributor

Choose a reason for hiding this comment

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

Why didn't we mention "breaking" above? Maybe just go for consistency and stick with this text:

Transferring the ownership of a name is an advanced feature. Ownership transfer requires modifying a name's zone file and broadcasting an update transaction with this change on the Bitcoin network. You must pay for the transaction costs in Bitcoin (BTC).

or if we should mention this, this is clearer.

Transferring the ownership of a name is an advanced feature. Ownership transfer requires modifying a name's zone file and broadcasting an update transaction with this change on the Bitcoin network. Further, transfers can fail and leave your name in a state where you can no longer use it. You must pay for the transaction costs in Bitcoin (BTC) regardless of whether the transaction succeeds or fails.

I understand this could break my Blockstack
name and will cost me money.
I understand this could break my Blockstack name and will
cost me money.
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I understand I am requesting an ownership transfer of name NAMEHERE. Further, I understand that the transfer can fail leaving my name unavailable. Regardless of whether the owner transfer transaction fails or succeeds, I am required to pay transaction fees in Bitcoin (BTC).

@@ -224,7 +289,7 @@ const registerName = (
'apps may be unusable until you do.'
if (res.status === 409) {
message =
"Sorry, it looks like we weren't able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID."
'Sorry, it looks like we weren\'t able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.'
Copy link
Contributor

Choose a reason for hiding this comment

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

'Sorry, it looks like the blockchain failed to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.'

This ^^^ is more decentralized language "we were able to process" makes it look like a central authority.

@@ -251,7 +316,7 @@ const registerName = (
'apps may be unusable until you do.'
if (e.status === 409) {
message =
"Sorry, it looks like we weren't able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID."
'Sorry, it looks like we weren\'t able to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.'
Copy link
Contributor

Choose a reason for hiding this comment

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

'Sorry, it looks like the blockchain failed to process your name registration. Please contact us at support@blockstack.org for help. Some apps may be unusable until you register an ID.'

@hstove hstove mentioned this pull request Nov 19, 2019
@hstove hstove assigned hstove and unassigned moxiegirl and timstackblock Nov 19, 2019
@hstove
Copy link
Collaborator

hstove commented Nov 20, 2019

Thanks @moxiegirl , great feedback! I've updated the copy to what you've suggested.

@hstove hstove assigned moxiegirl and timstackblock and unassigned hstove Nov 20, 2019
@hstove hstove merged commit 2afbc32 into develop Nov 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Impacts DOCS Requires new or updates to our documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants