[LWDM] fix(settings): list hidden native coins in accounts settings - #20350
Open
mcayuelas-ledger wants to merge 1 commit into
Open
[LWDM] fix(settings): list hidden native coins in accounts settings#20350mcayuelas-ledger wants to merge 1 commit into
mcayuelas-ledger wants to merge 1 commit into
Conversation
live-github-bot
Bot
added
desktop
live-github-bot
Bot
changed the title
mcayuelas-ledger
marked this pull request as ready for review
August 3, 2026 15:00
LucasWerey
previously approved these changes
Aug 3, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the “Hidden tokens” settings list so it correctly surfaces hidden native coins (stored as plain currency.id) alongside hidden tokens, and makes token resolution more robust by preventing a single failing CAL lookup from discarding the whole list. The change is implemented in live-common and propagated to both Desktop and Mobile settings screens, with updated tests and copy.
Changes:
- Update
loadBlacklistedTokenSectionsto resolve native coins from the crypto registry first, then fall back to CAL token lookup with per-id failure isolation. - Widen Desktop/Mobile settings UI types from
TokenCurrencytoCryptoOrTokenCurrencyand render the expanded list accordingly. - Update Desktop settings description copy + add/adjust unit tests; add a changeset.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libs/ledger-live-common/src/account/helpers.ts | Resolve hidden ids as native coins first, then tokens via CAL with isolated error handling; group into parent sections. |
| libs/ledger-live-common/src/account/helpers.test.ts | Update assertions for assets and add coverage for native coin resolution + isolated CAL failures. |
| apps/ledger-live-mobile/src/screens/Settings/Accounts/index.tsx | Widen list item type to CryptoOrTokenCurrency and map assets to SectionList rows. |
| apps/ledger-live-desktop/static/i18n/en/app.json | Update the settings description to mention hiding from asset detail page options menu. |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Accounts/BlacklistedTokens.tsx | Widen list rendering from tokens-only to CryptoOrTokenCurrency items. |
| apps/ledger-live-desktop/src/renderer/screens/settings/sections/Accounts/BlacklistedTokens.test.tsx | Update copy expectation and add coverage for rendering a hidden native coin without CAL calls. |
| .changeset/wild-donkeys-hunt.md | Changeset describing the fix and impacted packages. |
Comment on lines
5513
to
5516
| "title": "Hidden tokens", | ||
| "desc": "You can hide tokens by going to the parent account then right-clicking on the token and selecting 'Hide token'.", | ||
| "desc": "You can hide an asset from its detail page using the options menu, or hide a token by going to the parent account then right-clicking on the token and selecting 'Hide token'.", | ||
| "count": "1 token", | ||
| "count_other": "{{count}} tokens" |
Contributor
Web Tools Build Status
|
Contributor
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 7 projects with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 desktop-mainPath:
📁 desktop-preloaderPath:
📁 desktop-rendererPath:
📁 desktop-webviewDappPreloaderPath:
📁 desktop-webviewPreloaderPath:
📁 desktop-workersPath:
📁 mobilePath:
Generated by Rsdoctor GitHub Action |
mcayuelas-ledger
force-pushed
the
fix/settings-hidden-assets-not-listed
branch
from
August 3, 2026 15:12
337811c to
fe6a893
Compare
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (2)
apps/ledger-live-desktop/static/i18n/en/app.json:5516
- This section still labels the feature as “Hidden tokens” and uses “token(s)” in the count, but the PR now includes hidden native coins in the list (so the copy becomes inaccurate, e.g. Bitcoin would be counted as a token). Consider updating the title/count wording to be asset-based (and aligning other locales + affected tests accordingly).
"title": "Hidden tokens",
"desc": "You can hide an asset from its detail page using the options menu, or hide a token by going to the parent account then right-clicking on the token and selecting 'Hide token'.",
"count": "1 token",
"count_other": "{{count}} tokens"
.changeset/wild-donkeys-hunt.md:5
- This PR changes the exported
loadBlacklistedTokenSectionsAPI (return shapetokens→assets, and the array can now includeCryptoCurrencyentries). That’s a breaking change for@ledgerhq/live-common, so the changeset should bemajor(or the code should preserve backward compatibility).
"@ledgerhq/live-common": minor
"ledger-live-desktop": minor
"live-mobile": minor
LucasWerey
previously approved these changes
Aug 3, 2026
mcayuelas-ledger
force-pushed
the
fix/settings-hidden-assets-not-listed
branch
from
August 3, 2026 15:24
fe6a893 to
51bc3da
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
libs/ledger-live-common/src/account/helpers.ts:29
- The doc comment says token lookups are isolated so one failure keeps the rest of the list, but the implementation only isolates Promise rejections via
.catch(() => undefined). A synchronous throw fromfindTokenByIdwill still reject the wholePromise.alland discard the list. Either update the comment to specify it only covers rejected lookups, or wrap the lookup in a try/catch to truly isolate all failures.
* Load blacklisted (hidden) assets and organize them into sections by parent currency.
* Hidden ids can be native coins (grouped under themselves) as well as tokens (grouped
* under their parent). Token lookups are isolated so one failure keeps the rest of the list.
libs/ledger-live-common/src/account/helpers.ts:55
- Grouping sections by
s.parentCurrency === parentCurrencyrelies on referential equality ofCryptoCurrencyobjects. Since@domain/entity-currency-cryptoaccessors are explicitly intended to become selectors over a dynamic slice (where referential stability may not be guaranteed), grouping byidis safer and makes the intent clearer.
const index = sections.findIndex(s => s.parentCurrency === parentCurrency);
| @@ -0,0 +1,7 @@ | |||
| --- | |||
| "@ledgerhq/live-common": minor | |||
|
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.
✅ Checklist
npx changesetwas attached.📝 Description
Assets hidden from the asset detail page (options menu) are stored in
settings.blacklistedTokenIdsusing the plaincurrency.id. Since hiding is enabled for any asset type, that id can be a native coin (e.g.bitcoin) as well as a token (e.g.ethereum/erc20/usdt).The Settings > Accounts > "Hidden tokens" list resolved every stored id exclusively through the CAL token lookup (
findTokenById), so native coin ids resolved toundefinedand were silently dropped: the row showed a count and chevron (driven byblacklistedTokenIds.length) but expanded to an empty dropdown. Worse, the lookups ran inside a singlePromise.all, so one failing/rejected CAL lookup discarded the entire list, hiding genuinely hidden tokens too.loadBlacklistedTokenSectionsnow resolves each id as a native coin from the crypto registry first (no network call), falling back to the CAL token lookup only for unresolved ids, with each lookup isolated via.catch(() => undefined). Native coins are grouped under their own currency header; tokens under their parent. The desktop and mobile hidden-lists were widened fromTokenCurrencytoCryptoOrTokenCurrencyaccordingly, and the settings description was updated to mention hiding an asset from its detail page.Screen.Recording.2026-08-03.at.16.52.08.mov
❓ Context
🧐 Checklist for the PR Reviewers