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

Fix #6225: Ethereum Name Service (ENS) Navigation #7153

Merged
merged 12 commits into from
Apr 6, 2023

Conversation

StephenHeaps
Copy link
Contributor

@StephenHeaps StephenHeaps commented Mar 27, 2023

Summary of Changes

  • Depends on IPFS support from IPFS navigation support for iOS #7013; this PR cannot merge without IPFS landing first.
  • Refactored the SNS navigation logic out into BraveWallet target with a new DecentralizedDNSHelper class (w/ unit tests).
    • Enables us to re-use some logic, unit test, and lets us add new decentralized DNS services easier later (Unstoppable Domains!).
    • Later we'll move the interstitital pages out into BraveWallet target too. This requires some refactoring of current logic (need to extract TabContentScript / TabContentScriptLoader / InternalSchemeResponse out of Client and into a shared target).
  • Added ENS support. ENS resolve method defaults to 'Ask' similar to existing ENS Offchain resolve method / SNS resolve method. When set to 'Ask' an interstitial page is shown before proceeding with the lookup.
  • Security/privacy review: https://github.com/brave/security/issues/1231

This pull request fixes #6225

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 Web3 / Wallet settings and set
    • Resolve Ethereum Name Service (ENS) Domain Names to Ask
    • Allow ENS Offchain Lookup to Ask
    • Resolve IPFS Resources to ask (optional); ENS will always load to an IPFS url; must be either Ask or Enabled
  2. Navigate to any ENS domain via the omnibox. Ex. offchainexample.eth / http://offchainexample.eth
  3. Verify interstitial to enable Ethereum Name Service is shown.
    • Verify privacy policy and terms of use links open to expected pages for Consensys (Infura)
  4. Tap 'Disable'
  5. Navigate to Web3 / Wallet settings and verify Resolve Ethereum Name Service (ENS) Domain Names is set to Disabled
  6. Change Resolve Ethereum Name Service (ENS) Domain Names back to Ask
  7. Navigate to any ENS offchain domain via the omnibox. Ex. offchainexample.eth / http://offchainexample.eth
  8. Tap Proceed using Infura server
  9. Verify ENS offchain insterstitial is shown.
    • Verify Learn more link opens to the Brave Wallet ENS Offchain privacy considerations page.
  10. Tap Disabled
  11. Navigate to Web3 / Wallet settings and verify Allow ENS Offchain Lookup is set to Disabled
  12. Change Allow ENS Offchain Lookup back to Ask
  13. Navigate to any ENS offchain domain via the omnibox. Ex. offchainexample.eth / http://offchainexample.eth
  14. Verify offchain insterstitial is shown.
  15. Tap Proceed with offchain lookup
  16. Verify either IPFS interstitial is shown, IPFS public gateway url is loaded, or IPFS disabled page shown (note: IPFS out of scope for this PR).

SNS: http://onybose.sol
ENS: http://offchainexample.eth

Screenshots:

6225.mp4
Web3 Settings ENS Interstitial ENS Offchain Interstitial

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).

@github-advanced-security
Copy link

You have successfully added a new SonarCloud configuration ``. As part of the setup process, we have scanned this repository and found no existing alerts. In the future, you will see all code scanning alerts on the repository Security tab.

@nuo-xu nuo-xu force-pushed the wallet/ipfs-navigation branch from 577dd82 to 376308b Compare March 28, 2023 15:19
@StephenHeaps StephenHeaps force-pushed the wallet/ens-navigation branch from f9977d5 to 74a71b2 Compare March 28, 2023 17:50
@StephenHeaps StephenHeaps requested a review from nuo-xu March 28, 2023 18:06
Copy link
Contributor

@soner-yuksel soner-yuksel left a comment

Choose a reason for hiding this comment

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

Only some minor comments, great test coverage and everything.

@StephenHeaps StephenHeaps marked this pull request as ready for review March 30, 2023 13:28
@StephenHeaps StephenHeaps requested a review from a team as a code owner March 30, 2023 13:28
App/iOS/Delegates/AppDelegate.swift Outdated Show resolved Hide resolved
@StephenHeaps StephenHeaps requested a review from Brandon-T March 30, 2023 20:25
@nuo-xu nuo-xu force-pushed the wallet/ipfs-navigation branch from 376308b to 340f7f9 Compare April 3, 2023 18:43
@StephenHeaps StephenHeaps force-pushed the wallet/ens-navigation branch from 9cbb5a6 to af0d9a5 Compare April 4, 2023 21:11
@nuo-xu nuo-xu force-pushed the wallet/ipfs-navigation branch from 340f7f9 to 69a6553 Compare April 4, 2023 22:37
@StephenHeaps StephenHeaps force-pushed the wallet/ens-navigation branch from af0d9a5 to 4b20b7f Compare April 5, 2023 01:03
@StephenHeaps
Copy link
Contributor Author

StephenHeaps commented Apr 5, 2023

@stoletheminerals @nuo-xu @Brandon-T @soner-yuksel I've added support for links tapped in a web view (main frame only), if you could please check out these changes 🙂. I've included a couple ENS/SNS links in this PR description that can be used to test. 8e0828f

@nuo-xu nuo-xu force-pushed the wallet/ipfs-navigation branch from 1385625 to 8e81428 Compare April 6, 2023 15:34
Base automatically changed from wallet/ipfs-navigation to development April 6, 2023 16:00
…NameServiceScriptHandler` to be more generic. Support for SNS, ENS and ENS Offchain (and later Unstoppable Domains can re-use).

Integrate initial support for ENS navigation via omnibar (supported until IPFS url received).
…elper`. Update `SettingsStoreTests` for testing `ensResolveMethod` in setup.
…user-entered url if user opens url bar on interstitial page.
… Update selected tab loading state when user presses stop.
…e optional, returning nil in private mode. Added unit test to verify.
…criptHandler`. Add helper function to initialize `DecentralizedDNSHelper` for BVC.
@StephenHeaps StephenHeaps force-pushed the wallet/ens-navigation branch from 8e0828f to 8155c17 Compare April 6, 2023 18:05
@StephenHeaps StephenHeaps added this to the 1.50 milestone Apr 6, 2023
@StephenHeaps StephenHeaps merged commit 010ed0d into development Apr 6, 2023
@StephenHeaps StephenHeaps deleted the wallet/ens-navigation branch April 6, 2023 19:29
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
…ve/brave-ios#7153)

* Add ENS Resolve Method preference to `Web3DomainSettingsView`

* Refactor `SNSDomain.html`, `SNSDomain.css`, `SNSDomainHandler`, `Web3NameServiceScriptHandler` to be more generic. Support for SNS, ENS and ENS Offchain (and later Unstoppable Domains can re-use).
Integrate initial support for ENS navigation via omnibar (supported until IPFS url received).

* Add new `DecentralizedDNSHelper` to contain decentralized DNS logic to `BraveWallet` target

* Add `DecentralizedDNSHelperTests` for unit testing `DecentralizedDNSHelper`. Update `SettingsStoreTests` for testing `ensResolveMethod` in setup.

* Close keyboard / leave overlay mode when resolving decentralized DNS. Update selected tab loading state when user presses stop.

* Use a single `Web3DomainHandler` by storing service id in url query.

* Verify URL is not a bookmarklet / javascript before returning in `DecentralizedDNSHelper`.

* Support decentralized DNS links when tapped on a website (main-frame only).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow users navigate to .eth domains (ENS)
6 participants