[LWDM] feat(wallet-framework): zod branded CryptoCurrencyId/TokenCurrencyId - #20260
Conversation
Web Tools Build Status
|
There was a problem hiding this comment.
Pull request overview
This PR introduces Zod-branded (opaque) identifier types for currency IDs inside @ledgerhq/ledger-wallet-framework, and updates framework + coin-module call sites to explicitly “brand” IDs at boundaries via Schema.parse() to prevent accidentally passing arbitrary strings where currency identifiers are expected.
Changes:
- Added
CryptoCurrencyIdSchema/TokenCurrencyIdSchema(non-empty branded strings) and updatedCryptoCurrency.id,TokenCurrency.id, andTokenCurrency.parentCurrencyIdto use the branded types. - Updated framework helpers/mocks/bridge utilities and multiple coin-modules to brand IDs at legacy → framework boundaries using
schema.parse(). - Added
zodas a dependency of@ledgerhq/ledger-wallet-framework(lockfile + package.json).
Reviewed changes
Copilot reviewed 65 out of 66 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds zod resolution for the framework importer. |
| libs/ledger-wallet-framework/src/types.ts | Introduces branded currency-id schemas/types and updates currency interfaces to use them. |
| libs/ledger-wallet-framework/src/mocks/fixtures/nfts.ts | Brands superAccount.currency.id before using it in helpers. |
| libs/ledger-wallet-framework/src/mocks/account.ts | Brands superAccount.currency.id before using it in helpers. |
| libs/ledger-wallet-framework/src/derivation.ts | Adjusts legacy derivations map typing to avoid branded-key issues. |
| libs/ledger-wallet-framework/src/bridge/jsHelpers.ts | Brands currency IDs at sync/scan boundaries. |
| libs/ledger-wallet-framework/src/account/index.test.ts | Updates token fixture IDs to use TokenCurrencyIdSchema.parse(...). |
| libs/ledger-wallet-framework/src/account/helpers.ts | Brands currencies/tokens returned from @ledgerhq/types-live account shapes. |
| libs/ledger-wallet-framework/package.json | Adds zod dependency to the framework package. |
| libs/coin-modules/coin-vechain/src/bridge/transaction.test.ts | Brands mocked currency IDs returned by getAccountCurrency(). |
| libs/coin-modules/coin-vechain/src/bridge/synchronisation.test.ts | Brands token + parent currency IDs in test token fixture. |
| libs/coin-modules/coin-tron/src/bridge/getTransactionStatus.ts | Brands currency ID before calling getFeesUnit(...) for formatting. |
| libs/coin-modules/coin-ton/src/tests/unit/txn.unit.test.ts | Brands token and parent currency IDs in mocked token currency. |
| libs/coin-modules/coin-sui/src/test/config.test.ts | Brands mock currency ID in config tests. |
| libs/coin-modules/coin-sui/src/signer/getAddress.test.ts | Brands currency ID in address resolver tests. |
| libs/coin-modules/coin-sui/src/bridge/synchronisation.test.ts | Removes unnecessary CryptoAssetsStore cast while setting framework store. |
| libs/coin-modules/coin-sui/src/bridge/synchronisation.migration.integ.test.ts | Removes unnecessary CryptoAssetsStore cast while setting framework store. |
| libs/coin-modules/coin-sui/src/bridge/index.test.ts | Brands mock currency ID in bridge tests. |
| libs/coin-modules/coin-stacks/src/bridge/utils/misc.integ.test.ts | Removes unnecessary CryptoAssetsStore cast while setting framework store. |
| libs/coin-modules/coin-solana/src/prepareTransaction.integ.test.ts | Brands token + parent currency IDs in test token fixture. |
| libs/coin-modules/coin-icon/src/buildTransaction.ts | Brands currency ID before calling currency-dependent helpers. |
| libs/coin-modules/coin-icon/src/broadcast.ts | Brands currency ID before broadcasting transaction. |
| libs/coin-modules/coin-icon/src/api/node.ts | Brands currency ID before deriving RPC URLs; introduces schema import. |
| libs/coin-modules/coin-hedera/src/test/fixtures/currency.fixture.ts | Brands crypto + token IDs in currency fixtures. |
| libs/coin-modules/coin-hedera/src/preload-data.test.ts | Brands unsupported currency IDs in tests. |
| libs/coin-modules/coin-hedera/src/logic/utils.ts | Brands account currency ID before reading preload data. |
| libs/coin-modules/coin-hedera/src/logic/listOperations.v2.test.ts | Brands token IDs in list-operations tests. |
| libs/coin-modules/coin-hedera/src/bridge/utils.test.ts | Brands token IDs in token-currency fixtures in tests. |
| libs/coin-modules/coin-hedera/src/bridge/receive.ts | Brands account currency ID for getAddress boundary. |
| libs/coin-modules/coin-hedera/src/bridge/getTransactionStatus.ts | Brands account currency and token IDs at boundaries for rates/preload/association checks. |
| libs/coin-modules/coin-filecoin/src/common-logic/utils.unit.test.ts | Brands currency ID in account shape info used by unit tests. |
| libs/coin-modules/coin-evm/src/network/node/rpc.common.ts | Brands account.currency.id before node API usage. |
| libs/coin-modules/coin-evm/src/network/node/ledger.test.ts | Brands currency IDs in node tests (optimism/scroll). |
| libs/coin-modules/coin-evm/src/network/gasTracker/ledger.test.ts | Brands fakeCurrency.id in tests. |
| libs/coin-modules/coin-evm/src/network/explorer/ledger.test.ts | Brands fakeCurrency.id in tests. |
| libs/coin-modules/coin-evm/src/logic/estimateFees.test.ts | Brands currency IDs in estimate-fees tests. |
| libs/coin-modules/coin-cosmos/src/mock.ts | Brands account.currency.id before address generation. |
| libs/coin-modules/coin-concordium/src/test/testHelpers.ts | Brands mock currency ID in test helper. |
| libs/coin-modules/coin-celo/src/bridge/synchronisation.integ.test.ts | Removes unnecessary CryptoAssetsStore cast while setting framework store. |
| libs/coin-modules/coin-celo/src/bridge/syncHelpers.test.ts | Brands currency IDs in sync-hash tests. |
| libs/coin-modules/coin-cardano/src/prepareTransaction.ts | Brands currency ID before fetching network info. |
| libs/coin-modules/coin-cardano/src/getTransactionStatus/getTransactionStatus.ts | Brands currency ID for sanctioned-address checks. |
| libs/coin-modules/coin-cardano/src/buildTransaction.ts | Brands currency ID before isTestnet(...) boundary. |
| libs/coin-modules/coin-cardano/src/buildSubAccounts.unit.test.ts | Brands token and parent currency IDs in token fixture. |
| libs/coin-modules/coin-cardano/src/broadcast.ts | Brands currency ID before submit/broadcast boundary. |
| libs/coin-modules/coin-canton/src/test/fixtures.ts | Brands mock currency ID in test fixtures. |
| libs/coin-modules/coin-canton/src/bridge/signOperation.ts | Brands currency ID before crafting/signing. |
| libs/coin-modules/coin-canton/src/bridge/prepareTransaction.ts | Brands currency ID before fee estimation/CAL token cache access. |
| libs/coin-modules/coin-canton/src/bridge/getTransactionStatus.ts | Brands currency ID before topology validation boundary. |
| libs/coin-modules/coin-canton/src/bridge/buildSubAccounts.test.ts | Brands token + parent currency IDs in test token currency factory. |
| libs/coin-modules/coin-canton/src/bridge/broadcast.ts | Brands currency ID before broadcasting. |
| libs/coin-modules/coin-bitcoin/src/signRawOperation.ts | Brands currency ID before device signing boundary. |
| libs/coin-modules/coin-bitcoin/src/signOperation.ts | Brands currency ID before device signing boundary. |
| libs/coin-modules/coin-bitcoin/src/logic.ts | Relaxes keyed maps/signatures to string to avoid branded-key typing issues. |
| libs/coin-modules/coin-bitcoin/src/hw-signMessage.ts | Brands account currency ID before signing message boundary. |
| libs/coin-modules/coin-bitcoin/src/getTransactionStatus.ts | Brands currency ID before validation/sanctions checks. |
| libs/coin-modules/coin-bitcoin/src/chain-adapters/zcash/index.ts | Brands currency ID before signerContext boundary. |
| libs/coin-modules/coin-bitcoin/src/bridge/js.ts | Brands currency ID before full-viewing-key retrieval boundary. |
| libs/coin-modules/coin-aptos/src/tests/bridge/synchronisation.test.ts | Brands token + parent currency IDs in mocked token data. |
| libs/coin-modules/coin-aptos/src/tests/bridge/logic.test.ts | Brands token + parent currency IDs in mocked token data. |
| libs/coin-modules/coin-algorand/src/mock.ts | Brands account.currency.id before address generation. |
| libs/coin-modules/coin-algorand/src/deviceTransactionConfig.ts | Brands token + parent currency IDs before display formatting boundary. |
| libs/coin-modules/coin-aleo/src/logic/utils.ts | Brands token + parent currency IDs before encoding token account IDs. |
| libs/coin-modules/coin-aleo/src/logic/listOperations.test.ts | Brands token IDs in list-operations tests. |
| libs/coin-modules/coin-aleo/src/bridge/tokens.test.ts | Brands token IDs in tokens util tests. |
| libs/coin-modules/coin-aleo/src/tests/fixtures/currency.fixture.ts | Brands crypto + token IDs in fixtures. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
libs/coin-modules/coin-icon/src/api/node.ts:10
- These imports pull in type-only symbols as value imports. Please switch to
import type(ortypemodifiers) to avoid unnecessary runtime imports and stay consistent with other coin modules.
Rsdoctor Bundle Diff AnalysisFound 7 projects in monorepo, 0 projects with changes. 📊 Quick Summary
Generated by Rsdoctor GitHub Action |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 70 out of 71 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
libs/coin-modules/coin-evm/src/network/node/rpc.common.ts:5
CryptoCurrencyis only used as a TypeScript type in this file, so it should be imported withimport type(seelibs/coin-modules/coin-evm/src/network/node/rpc.test.ts:3for the local convention). This avoids retaining a value import in the emitted JS and keeps imports consistent across the package.
libs/coin-modules/coin-icon/src/api/node.ts:1- This file imports
CryptoCurrencyas a value, but it is only used as a TypeScript type in this module. In this codebase, the convention is to useimport typefor type-only imports (e.g.libs/coin-modules/coin-icon/src/logic.ts:2). Switching to type-only imports avoids unnecessary runtime imports and aligns with the established pattern.
5a98f9c to
2abebf5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 39 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (4)
libs/coin-modules/coin-evm/src/network/node/ledger.test.ts:4
CryptoCurrencyis only used as a TypeScript type here. Importing it as a value can unnecessarily introduce a runtime dependency on@ledgerhq/ledger-wallet-framework/types(which now importszod). Prefer a type-only import forCryptoCurrencyand keep the schema as the only runtime import.
import { CryptoCurrency, CryptoCurrencyIdSchema } from "@ledgerhq/ledger-wallet-framework/types";
libs/coin-modules/coin-evm/src/network/gasTracker/ledger.test.ts:4
CryptoCurrencyis only used as a TypeScript type in this test. Prefer a type-only import to avoid pulling@ledgerhq/ledger-wallet-framework/types(and itszoddependency) into the runtime import list unnecessarily.
import { CryptoCurrency, CryptoCurrencyIdSchema } from "@ledgerhq/ledger-wallet-framework/types";
libs/coin-modules/coin-evm/src/network/explorer/ledger.test.ts:4
CryptoCurrencyappears to be used only for typing in this file. Prefer a type-only import and keep the runtime import limited toCryptoCurrencyIdSchemato reduce unnecessary runtime dependencies now thattypes.tsimportszod.
import { CryptoCurrency, CryptoCurrencyIdSchema } from "@ledgerhq/ledger-wallet-framework/types";
libs/coin-modules/coin-bitcoin/src/logic.ts:195
- Changing
currencyIdfromCryptoCurrencyId | ...to plainstringweakens the type-safety this PR is trying to introduce (it re-allows passing arbitrary strings without an explicit boundary crossing). Consider switching this back to a branded id type (e.g.CryptoCurrencyIdorCryptoCurrency["id"], plus any explicit legacy literals) and updating the unit tests/callers that pass raw string literals to useCryptoCurrencyIdSchema.parse(...)at the boundary.
export const mapTxToOperations = (
tx: TX,
currencyId: string,
accountId: string,
accountAddresses: Set<string>,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 48 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
libs/ledger-wallet-framework/src/types.ts:9
- This change introduces a public API surface change in
@ledgerhq/ledger-wallet-framework(new exported schemas + CryptoCurrencyId/TokenCurrencyId becoming branded types). The repo convention is to add a Changeset for user-facing/library API changes so releases capture the breaking/minor bump explicitly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 51 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
libs/ledger-wallet-framework/src/account/index.test.ts:18
- This test still type-asserts the JSON fixture to
TokenCurrency, which bypasses the new brandedid/parentCurrencyIdtypes and defeats the goal of forcing explicit boundary branding. Prefer constructing aTokenCurrencyby parsing the id fields (and you can drop the lint-disable +ascast).
libs/ledger-wallet-framework/src/types.ts:9 - This introduces new exported runtime schemas/types (
CryptoCurrencyIdSchema,TokenCurrencyIdSchema) and changes the public types ofCryptoCurrency.id/TokenCurrency.id/TokenCurrency.parentCurrencyId. That’s a publishable API change for@ledgerhq/ledger-wallet-framework, so a Changeset entry should be added to document the change and bump the package version accordingly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 50 out of 51 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
libs/ledger-wallet-framework/src/account/index.test.ts:18
tokenDatais asserted asTokenCurrency, but after switchingTokenCurrency.id/parentCurrencyIdto branded ids this bypasses the new boundary guarantees and leaves those fields unvalidated/unbranded. It’s safer to keep the fixture but explicitly brand the id fields when buildingTOKEN.
libs/coin-modules/coin-bitcoin/src/logic.ts:195mapTxToOperationsnow acceptscurrencyId: string, which weakens the branded-id safety this PR is introducing (callers can pass any arbitrary string without an explicit boundary parse). Consider typing this asCryptoCurrency["id"]/CryptoCurrencyIdinstead and updating the small number of call sites/tests that currently pass raw literals (e.g. parse withCryptoCurrencyIdSchema.parse("zcash")).
export const mapTxToOperations = (
tx: TX,
currencyId: string,
accountId: string,
accountAddresses: Set<string>,
- Add CryptoCurrencyIdSchema / TokenCurrencyIdSchema as Zod branded string schemas - Update CryptoCurrency.id, TokenCurrency.id, TokenCurrency.parentCurrencyId to branded types - Add zod as a dependency of ledger-wallet-framework - Fix all coin-module consumers to brand id fields at boundaries via schema.parse() (coin-aleo, coin-algorand, coin-aptos, coin-bitcoin, coin-canton, coin-cardano, coin-celo, coin-concordium, coin-cosmos, coin-evm, coin-filecoin, coin-hedera, coin-icon, coin-solana, coin-stacks, coin-sui, coin-ton, coin-tron, coin-vechain) - No as-casts used — every boundary calls parse() for both type safety and runtime validation
- helpers.test.ts: add valid id/parentCurrencyId to mock CryptoCurrency/TokenCurrency objects - coin-tester-cardano: parse TokenCurrencyId in fixtures.ts and cardanoTokenYaci.ts - coin-tester-solana: parse TokenCurrencyId for VIRTUAL and TSLAX tokens - coin-tester-stellar: parse TokenCurrencyId/CryptoCurrencyId for USDC token
Account.currency/token flow through from @ledgerhq/types-live, where id/parentCurrencyId are already typed any (ahead of the now-merged types-cryptoassets removal). any assigns freely into the new branded CryptoCurrencyId/TokenCurrencyId slots, so the CryptoCurrencyIdSchema.parse wrapper objects added across coin-modules were dead weight - revert them back to passing the account currency straight through.
CI surfaced consumers of the ledger-wallet-framework's CryptoCurrencyId/ TokenCurrencyId branded types outside coin-modules that were never migrated: live-countervalues, live-countervalues-react, ledger-live-common and a mobile test build fresh CryptoCurrency/TokenCurrency fixtures with raw string ids, and wallet-cli's coinframework adapter lost its type predicate once TokenCurrency.id became a real branded type instead of any. Parse the literals through the brand schemas and give the balance mapper an explicit return type so the filter predicate widens correctly.
SonarCloud flagged 20.7% duplication on new code (limit 3%) for this PR: the three hardcoded token literals shared an identical field shape, differing only in id/contractAddress/tokenType/name/ticker. Extract a makeHederaToken factory to remove the repetition.
Follow-up to #20251 (LIVE-35206), which banned CryptoCurrency/TokenCurrency/ etc. from @ledgerhq/ledger-wallet-framework outside coin-modules. That guard predates CryptoCurrencyIdSchema/TokenCurrencyIdSchema (added in this PR), so add the same four names (+ the CryptoCurrencyId/TokenCurrencyId types) to the existing no-restricted-imports paths in desktop, mobile (both the main rule and its test/debug override) and live-common. Fixes the one real violation the guard now catches: the mobile useTopBarViewModel test and two live-common swap hook tests parsed ids through the framework's schema instead of @domain/entity-currency-*.
Only ledger-wallet-framework gets a changeset — everything else in this branch is coin-module/app test and lint adjustments with no logic change.
635fa12
7adf25d to
635fa12
Compare
|
|


Summary
Introduces Zod branded string types for currency identifiers in
@ledgerhq/ledger-wallet-framework, making it impossible to pass an arbitrarystringwhere a currency id is required without an explicit boundary crossing.CryptoCurrencyIdSchema = z.string().min(1).brand<"CryptoCurrencyId">()— non-empty branded stringTokenCurrencyIdSchema = z.string().min(1).brand<"TokenCurrencyId">()— non-empty branded stringCryptoCurrency.id,TokenCurrency.id,TokenCurrency.parentCurrencyIdupdated to branded types in frameworkschema.parse()(noascasts)Boundary pattern used throughout:
This PR depends on #20012 (
any-widening prerequisite) being merged first, or alternatively thetypes-cryptoassetsidfields staying asstring(which requires the spread+parse pattern at every consumer boundary — which this PR already does).Test plan
pnpm nx run-many -t typecheck -p ledger-live-desktop -p live-mobiletsc --noEmit --customConditions nodepasses in every modified coin-module