Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Fix #5787: Send to ENS wallet address #7030

Merged
merged 7 commits into from
Mar 9, 2023
Merged

Fix #5787: Send to ENS wallet address #7030

merged 7 commits into from
Mar 9, 2023

Conversation

StephenHeaps
Copy link
Contributor

@StephenHeaps StephenHeaps commented Feb 28, 2023 •
edited
Loading

Summary of Changes

This pull request fixes #5787

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

  1. Open wallet/web3 settings and verify default setting of Allow ENS Offchain Lookup is set to Ask
  2. Navigate to send token view
  3. Select an Ethereum network
  4. Enter ethereum.eth as a send to address and verify it resolves to the address as expected (with loading state).
  5. Enter offchainexample.eth as a send address and verify you are prompted to enable ENS offchain lookup.
  6. Tap Use ENS Domain and verify it resolves to the address as expected (with loading state).
  7. Navigate to wallet/web3 settings and verify Allow ENS Offchain Lookup is shown as Enabled
  8. Navigate back to send token view
  9. Enter offchainexample.eth as a send address and verify you are no longer prompted to enable ENS offchain lookup.
  10. Navigate to wallet/web3 settings and verify Allow ENS Offchain Lookup is shown as Disabled
  11. Navigate back to send token view
  12. Enter offchainexample.eth as a send address and verify it does not resolve the address

Screenshots:

send.to.eth.address.mp4

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@StephenHeaps StephenHeaps added this to the 1.49 milestone Feb 28, 2023
@StephenHeaps StephenHeaps self-assigned this Feb 28, 2023
@StephenHeaps StephenHeaps marked this pull request as ready for review February 28, 2023 17:07
@StephenHeaps StephenHeaps requested a review from a team as a code owner February 28, 2023 17:07
@StephenHeaps StephenHeaps force-pushed the wallet/send-to-ens-address branch 2 times, most recently from 7ee1f31 to c272398 Compare March 1, 2023 18:19
@stoletheminerals
Copy link
Contributor

It's better to show a relevant error message when offchain look-up is disabled. wdyt @StephenHeaps ?
image

@StephenHeaps
Copy link
Contributor Author

It's better to show a relevant error message when offchain look-up is disabled. wdyt @StephenHeaps ?

@stoletheminerals I think this is a good idea. I've opened a core ticket to support this brave/brave-browser#28916 (we need to know if the entered domain requires offchain lookup to show the message so we don't disable all ENS lookups when ENS offchain lookup is disabled), and an iOS ticket to update iOS if this PR has merged prior to the core api update brave/brave-browser#36696

@StephenHeaps
Copy link
Contributor Author

One minor UI tweak with iOS 14 support removed is updating ENS Offchain hyperlink style in Web3 preferences:

Previous Updated
221922314-29a670e8-1403-424c-b8c4-aad85cf18611 Simulator Screen Shot - iPhone 12 Pro - 2023-03-07 at 12 12 27

@stoletheminerals
Copy link
Contributor

Maybe Learn more instead of Learn More since the whole sentence is the link now? 🙂 Would look cleaner imo

@StephenHeaps
Copy link
Contributor Author

Maybe Learn more instead of Learn More since the whole sentence is the link now? 🙂 Would look cleaner imo

I agree 🙂 Updated in 4ca083e

@StephenHeaps StephenHeaps merged commit efac58e into development Mar 9, 2023
@StephenHeaps StephenHeaps deleted the wallet/send-to-ens-address branch March 9, 2023 14:27
@srirambv srirambv mentioned this pull request Mar 10, 2023
Closed
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
@StephenHeaps
Fix brave/brave-ios#5787: Send to ENS wallet address (brave/brave-ios…
3005b48 
…#7030)

* Support sending to an ENS domain in a native send transaction, including offchain ENS.

* Add ENS Offchain preference and hyperlink to `Web3DomainSettingsView`

* Unit tests for verifying ENS resolution, ENS Offchain consent required, ENS failure, creating Ethereum transaction using resolved address.

* Use iOS 15+ open url action now that iOS 14 support is dropped. Update UI/UX for ENS preference to use markdown
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ShivanKaul ShivanKaul ShivanKaul left review comments

@nuo-xu nuo-xu nuo-xu approved these changes

@stoletheminerals stoletheminerals stoletheminerals approved these changes

Assignees

@StephenHeaps StephenHeaps

Labels
None yet
Projects
None yet
Milestone
1.49
Development

Successfully merging this pull request may close these issues.

Wallet: Support sending transactions to a domain (ENS)
4 participants
@StephenHeaps @stoletheminerals @nuo-xu @ShivanKaul