Skip to content
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

Add support for Unstoppable domains #2953

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mikeymop
Copy link

@mikeymop mikeymop commented Apr 9, 2022

Details

This PR adds support for Unstoppable Domain resolution so that users can input a domain such as brad.crypto and have the address resolve to a Cardano address. Currently these changes are in the proof of concept state as I would like feedback from the team on these changes in hopes to make this PR as beneficial as possible.

This currently works by instantiating a new mobx @action observable on resolveDomain() in source/renderer/app/stores/WalletStore. It resolves the domain to either a Cardano address or an error message only when a . is detected in the reciever field.

On the send form I tried my best to match the existing code an return `[false, error.message] in the validator for the reciever field.

Questions:

  1. I have noticed a few places that may be valid for storing the error message translations.
    Currently I am referencing messages /source/common/utils/unsResolution.ts bundled with the logic for resolving domains.
    Would it be preferred to store these translations with the others designated for the wallet send-form in source/renderer/app/components/wallet/send-form/messages.ts?

  2. I have become aware that CORS will challenge you when running an electron app in dev mode because all calls would then have to be to localhost. The resolution library makes a call to a contract to resolve the domains located on infura.io.
    I tried remedying this temporarily by adding the uris to the webRequest before send headers in createMainWindow on source/main/windows/main.ts.

Todos

  • Add support for reading domains from input only if a domain is entered into the send screen.
  • Add translations for the various domain resolution errors.
  • Determine the best location for the translations (see details)
  • Present the address the domain resolves to onto the ui below the send form.
  • Resolve CORS errors when running the application in dev mode so that the resolution can be more thoroughly tested.

Screenshots

TBD

Testing Checklist

  • Slack QA thread
  • Run the application in dev mode as normal
  • In the address field on the send screen, enter a domain such as brad.crypto instead of a Cardano address.
  • Once the . is entered observe that resolution errors are presenting on screen up until the full brad.crypto is entered.
  • Observe that the resolved address is inserted below the send form.

Review Checklist

Basics

  • PR assigned to the PR author(s)
  • input-output-hk/daedalus-dev and input-output-hk/daedalus-qa assigned as PR reviewers
  • If there are UI changes, Alexander Rukin assigned as an additional reviewer
  • All visual regression testing has been reviewed (assign run Chromatic label to PR to trigger the run)
  • PR has appropriate labels (release-vNext, feature/bug/chore, WIP)
  • PR link is added to a Jira ticket, ticket moved to In Review
  • PR is updated to the most recent version of the target branch (and there are no conflicts)
  • PR has a good description that summarizes all changes
  • PR contains screenshots (in case of UI changes)
  • CHANGELOG entry has been added to the top of the appropriate section (Features, Fixes, Chores) and is linked to the correct PR on GitHub
  • There are no missing translations (running yarn manage:translations produces no changes)
  • Text changes are proofread and approved (Jane Wild / Amy Reeve)
  • Japanese text changes are proofread and approved (Junko Oda)
  • Storybook works and no stories are broken (yarn storybook)
  • In case of dependency changes yarn.lock file is updated

Code Quality

  • Important parts of the code are properly commented and documented
  • Code is properly typed with typescript types
  • React components are split-up enough to avoid unnecessary re-renderings
  • Any code that only works in main process is neatly separated from components

Testing

  • New feature/change is covered by acceptance tests
  • New feature/change is manually tested and approved by QA team
  • All existing acceptance tests are still up-to-date
  • New feature/change is covered by Daedalus Testing scenario
  • All existing Daedalus Testing scenarios are still up-to-date

After Review

  • Update Slack QA thread by marking it with a green checkmark

@lucas-barros
Copy link
Contributor

@mikeymop thank you for the contribution. I'll discuss this internally with the team to see how we should move forward with this functionality.

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.

2 participants