-
Notifications
You must be signed in to change notification settings - Fork 245
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
Verify ENS in the background #1824
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (6)
|
7eefde7
to
9c02047
Compare
9c02047
to
fb2f5ab
Compare
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.
Overall looks good, just few small comments. My main one would be:
Why are we adding ENS stuff into protocol
package?
Wouldn't it make sense to make a separate ens
package?
Or at least protocol/ens
?
Currently ENS are verified explicitly by status-react, this is not ideal as if that fails it will have to be explicilty retried in status-react. This commits changes that behavior so that ENS are verified in a loop and updated if new messages are received.
fb2f5ab
to
e153c60
Compare
Thanks for the review, addressed the feedback:
The protocol naming is not very descriptive of the responsibilities, basically the I would move it to |
Currently ENS are verified explicitly by status-react, this is not ideal
as if that fails it will have to be explicilty retried in status-react.
This commits changes that behavior so that ENS are verified in a loop
and updated if new messages are received.
status: ready