fix(contacts): offer only registerable networks in add address flow - #21470
Draft
claudiiafg wants to merge 1 commit into
Draft
fix(contacts): offer only registerable networks in add address flow#21470claudiiafg wants to merge 1 commit into
claudiiafg wants to merge 1 commit into
Conversation
Contributor
Web Tools Build Status
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, aligns eligibility with device support, and includes targeted regression tests to prevent reintroducing the dead-end.
Pull request overview
This PR fixes a dead-end in the Contacts “add address” flow by ensuring the network picker only allows networks that can successfully resolve to a Contacts device context (i.e., the flow can reach a device signature).
Changes:
- Gate
resolveEligibleAddressCurrencyIdson device-supported coin apps via a shared predicate (isContactDeviceCurrencySupported). - Add regression tests asserting “every offered network must resolve a device context” and extend existing device-context tests to cover EVM networks with their own coin apps.
- Update docs and selection-viewmodel tests to reflect the refined eligibility set (notably adding Tron and excluding unsupported networks like Ethereum Classic).
File summaries
| File | Description |
|---|---|
| features/platform/contacts/src/utils/resolveEligibleAddressCurrencyIds.web.test.ts | Adds invariant test that all offered networks can resolve a device context; adjusts expected eligible networks. |
| features/platform/contacts/src/utils/resolveEligibleAddressCurrencyIds.ts | Filters eligible networks by both family and device support (isContactDeviceCurrencySupported). |
| features/platform/contacts/src/device/resolveContactDeviceContext.ts | Extracts currency support logic into findContactDeviceCurrency and exports isContactDeviceCurrencySupported. |
| features/platform/contacts/src/device/resolveContactDeviceContext.test.ts | Adds coverage for rejecting EVM networks with their own coin app; adds tests for isContactDeviceCurrencySupported. |
| features/platform/contacts/README.md | Documents that eligible networks are additionally constrained by device-registerable support. |
| features/flow/flow-contacts-add-address/src/state/useAddAddressCurrencySelectionViewModel.web.test.tsx | Updates expectations for the eligible network list passed to the currency selection port. |
| .changeset/LIVE-36688-contacts-signable-networks.md | Adds a changeset describing the user-facing fix across Desktop/Mobile and the contacts package. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 2 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-rendererPath:
📦 Download Diff Report: desktop-renderer Bundle Diff 📁 mobilePath:
📦 Download Diff Report: mobile Bundle Diff Generated by Rsdoctor GitHub Action |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Adding an address to a contact could dead-end: entering a name and tapping Continue did nothing — no device prompt, no error, no way forward.
Previous behaviour
The network picker and the device hand-off disagreed about which networks are supported:
resolveEligibleAddressCurrencyIdsoffered every network in an eligible family (evmby default).resolveContactDeviceContextonly accepts networks whose Ledger coin app the Contacts kit knows (EthereumandTron).23 EVM networks fall in that gap — mostly dead legacy chains, but also Ethereum Classic, Sonic and Sei. Picking one threw
UnsupportedContactDeviceCurrencyErrorwhile building the operation, i.e. before the intent existed, sosetActiveIntentnever fired, the Device Intent Executor never mounted, and the throw was swallowed by theconsole.warncatch inuseContactDetailScreenViewModel.Fix
resolveEligibleAddressCurrencyIdsnow also gates on device support, so a network can only be offered if the flow can actually reach a signature on it. The supported-app check inresolveContactDeviceContextis extracted into an exportedisContactDeviceCurrencySupportedpredicate rather than duplicated.selectableNetworkIdsdisables rather than hides, so the affected networks stay visible and greyed out and tapping one shows the existing Contacts explanation ("… isn't supported yet") instead of a button that silently does nothing. Desktop shares the resolver, so it gets the same fix through its built-in tooltip.Regression check
resolveEligibleAddressCurrencyIds.web.test.tsasserts the invariant directly — every network the picker offers must resolve a device context — so a newly added EVM network with its own coin app fails the suite instead of reaching users.Note: a second cause of the same symptom, the add-address drawer holding the queued-bottom-sheet slot so the executor could never open, was fixed separately in 1c9242d.
🔗 Context