-
Notifications
You must be signed in to change notification settings - Fork 88
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
Fix registration date error #139
base: master
Are you sure you want to change the base?
Fix registration date error #139
Conversation
src/registrar.js
Outdated
@@ -233,6 +242,9 @@ export default class Registrar { | |||
ret.gracePeriodEndDate = gracePeriodEndDate | |||
} | |||
} | |||
if (legacyEntry.registrationDate && permEntry.registrationDate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An auction for this name was started in 2018, but the auction never completed. The name was first registered recently, but the manager shows a registration date in 2018.
Doesn't this pick permEntry.registrationDate
, regardless of auction completed or not?
For example, apt-get.eth was one of the names registered during the auction but showing the last registered name using your PR branch (on the left).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Yes, you are right. After having a closer look, I wonder if the ens manager app should even display the registration date when the auction never completed in the legacy registrar to remain consistent with current behavior of permanent registrar (i.e. registration date is not reported for names registered with permanent registrar). That could be a new feature though. Last commit resets the registration date to 0 when the auction never finalized so that the registration date does not show up. Finally, I am not sure if this validity check should be implemented in the ensdomains/ui library or in the ens manager app (getRegistrarEntry()
in resolvers.js).
This reverts commit 3879e31.
Fixes ensdomains/ens-app#1331
This PR:
Before
After