refactor(assets): consolidate asset identity checks

[CassioMG]

TL;DR

This PR consolidates the extension's native-asset identity checks into a small set of shared predicates, each keyed on the authoritative discriminant for its layer: a token's type, the native contract address derived from the network passphrase, or the full canonical identifier.

The flows themselves are unchanged; what changes is how the identity checks are made, and where that logic lives.

It also adds a lint rule that reports a ===/!== with a native-asset sentinel on either side, so new code reaches for an existing predicate instead of re-deriving one; @shared/helpers/assetIdentity.ts is exempt as the module that defines them, and the convention is written up in docs/skills/freighter-best-practices/references/anti-patterns.md.

Closes https://github.com/stellar/wallet-eng-monorepo/issues/61

Size

Most of the diff is tests — the production surface is small (net +200).

	Files	Lines
Production source	38	+475 / −275 (net +200)
Tests	14	+1131 / −8
Lint rule (plugin + config)	2	+124 / −0
Docs	2	+88 / −1
Total	56	+1818 / −284

Implementation details (for agents)

The shared predicates. @shared/helpers/assetIdentity.ts (new) holds one predicate per identifier layer: isNativeBalance / isNativeAsset (type-based, for a balance object or an SDK Asset), isNativeAssetId (the canonical id, a Horizon asset_type, or a token.type — matches "native" only), getNativeContractId / isNativeContract (contract space, derived via Asset.native().contractId(passphrase)), and isNativeAssetPair(code, issuer) for the raw-strings layer where nothing better is available. isNativeBalance moves here from popup/helpers/balance.ts. Identity for anything other than nativeness — equality, map keys, labels — goes through the existing getCanonicalFromAsset.

Transaction building. useSimulateTxData's getOperation decides its create-account branch from the asset's type via isNativeAsset, and is exported so the operation it builds is pinned on parsed operations.

Balances and signing. getBalanceByKey enters its native-contract branch on the balance's type. The signing screen's fee pre-flight is extracted to hasEnoughXlmForFee in popup/helpers/balance.ts, anchored on the native balance.

Icons, history and display. AssetIcon resolves the native icon from code and issuer together, and its memo comparator compares every render-affecting prop; the callers that build its icon map use the same pair. History icon selection pairs each code with its issuer, the Soroban transfer row decides nativeness from the contract address, and asset-detail operation matching is extracted to operationMatchesAssetKey with separate native and classic arms.

Contract space and add-a-token. getNativeContractDetails derives the native contract address from the passphrase for every network. isAssetSac's native branch, useAssetLookup, useTokenLookup and AddAsset resolve the native contract through isNativeContract; the native search row is built by buildNativeAssetRow, and stellar.expert records are mapped by mapStellarExpertRecord alongside it. getAssetListsForAsset requires both sides of an identity to be present before comparing them.

The lint rule and the convention. config/eslint-plugin-asset-identity/ (new local plugin, wired into eslint.config.js at error level) adds no-asset-code-comparison: it reports a ===/!== with "XLM", "native", the SDK's own native code (Asset.native().code / .getCode()), or the identifier names NATIVE_TOKEN_CODE / HORIZON_NATIVE_ASSET_TYPE on either side. Asset.native().contractId(...) is not reported: a contract-id comparison is the sound check in contract space. Every site the rule flags was migrated in this branch — 62 comparisons across 31 files, each routed to the predicate matching what its operand holds — so it reports zero hits on the tree. The convention is written up in anti-patterns.md §11 with a what-you-hold → which-predicate table, and code-style.md records the rule.

Verification. Full Jest suite green: 1851 passed / 51 skipped across 234 suites. yarn build:extension is clean with the rule live (ESLint runs inside the webpack build for extension/), and the rule was confirmed to fire before the tree was declared clean. Each migrated site has tests covering its predicate's discriminating cases — a genuine native control alongside a classic asset that uses the code XLM with its own issuer — and derived contract ids are pinned against the published PUBLIC and TESTNET addresses. No user-facing strings were added.

Follow-ups / out of scope. Extending the build's lint gate from extension/ to @shared (the docs state the current scope). Issuer rows on the liquidity-pool branch of the trustline approval pane. Worth a smoke test before release: the add-a-token search and icon surfaces, which now resolve the native asset through the shared predicates.

🤖 Generated with Claude Code

[github-actions]

PR Preview build is ready: https://github.com/stellar/freighter/releases/tag/untagged-e1df6561adcfcfc6b081
Backend: V1 prod + V2 beta (no sandbox configured for @CassioMG). SDF collaborators only — install instructions in the release description.

[Copilot]

Pull request overview

Copilot reviewed 56 out of 56 changed files in this pull request and generated no new comments.

[CassioMG]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code

[piyalbasu]

The native check accepts a contract token whose symbol contains a colon

Low severity, pre-existing — not a merge blocker. The behaviour isn't introduced by this PR. Raising it here because the new doc comment asserts the specific invariant that doesn't hold, so this is the natural place to either harden it or correct the comment.

TL;DR: Our asset identifiers are SYMBOL:CONTRACT_ID strings, parsed by splitting on the first colon. A Soroban token's symbol is whatever its contract reports, so it isn't guaranteed to be colon-free the way a classic asset code is. When a symbol contains a colon the split lands in the wrong place, the issuer half comes back empty, and the native check reads the result as real XLM. The new doc comment states this shape can only ever carry a C… issuer and is therefore always rejected — it can also carry an empty string, which is accepted. Classic assets are unaffected: their codes are alphanumeric-only by protocol.

Why it's worth a fix rather than a note: the identifier space isn't colon-safe, but three separate call sites assume it is, each failing differently — one misidentifies the asset as native, one can't extract the contract id at all, one derives an empty asset address. A malformed identifier should fail loudly at the parse boundary instead of flowing onward as a plausible-looking asset.

---

Detailed explanation (for agents)

Root cause: two independently reasonable behaviours compose badly.

The native pair test treats a falsy issuer as "no issuer", which is correct for real native:

freighter/@shared/helpers/assetIdentity.ts

Lines 33 to 41 in 095a2af

	/**
	* Native test for a raw code and issuer. Use it only where neither a token
	* type nor a contract id is available — the native asset carries the native
	* code and no issuer, so both halves are required.
	*/
	export const isNativeAssetPair = (
	code: string | undefined | null,
	issuer: string | undefined | null,
	): boolean => code === NATIVE_ASSET_CODE && !issuer;

But the canonical parser splits on the first colon with a two-element destructure, and isSorobanIssuer("") is true because "".startsWith("G") is false — so a truncated parse yields the plain shape with issuer: "" rather than throwing:

freighter/@shared/helpers/stellar.ts

Lines 181 to 200 in 095a2af

	export const isSorobanIssuer = (issuer: string) => !issuer.startsWith("G");
	export const getAssetFromCanonical = (canonical: string) => {
	if (isNativeAssetId(canonical)) {
	return StellarSdk.Asset.native();
	}
	if (canonical.includes(":")) {
	const [code, issuer] = canonical.split(":");
	if (isSorobanIssuer(issuer)) {
	return {
	code,
	issuer,
	};
	}
	return new StellarSdk.Asset(code, issuer);
	}
	throw new Error(`invalid asset canonical id: ${canonical}`);
	};

Which makes the invariant asserted here untrue — the plain shape does not "only ever carry a C… issuer":

freighter/@shared/helpers/assetIdentity.ts

Lines 49 to 65 in 095a2af

	/**
	* True only for the native asset.
	*
	* `getAssetFromCanonical` returns an SDK `Asset` for classic assets and a plain
	* `{ code, issuer }` for Soroban issuers, so both shapes arrive here. The SDK's
	* own `isNative()` is authoritative when present; the plain shape only ever
	* carries a `C…` issuer, which the pair test correctly rejects.
	*
	* Narrowing on the method rather than `instanceof` keeps this correct across
	* the `stellar-sdk` / `stellar-sdk-next` dual-package split.
	*/
	export const isNativeAsset = (
	asset: Asset | { code: string; issuer?: string },
	): boolean =>
	typeof (asset as Asset).isNative === "function"
	? (asset as Asset).isNative()
	: isNativeAssetPair(asset.code, asset.issuer);

The round trip, for a symbol of XLM: and contract CAS3…OWMA:

getCanonicalFromAsset("XLM:", "CAS3…OWMA")  →  "XLM::CAS3…OWMA"
getAssetFromCanonical("XLM::CAS3…OWMA")     →  split(":") = ["XLM", "", "CAS3…OWMA"]
                                            →  { code: "XLM", issuer: "" }
isNativeAsset({ code: "XLM", issuer: "" })  →  isNativeAssetPair("XLM", "")  →  true

Why a colon can occur. Contract-token canonicals are built by string-concatenating the symbol:

freighter/@shared/api/internal.ts

Lines 1005 to 1015 in 095a2af

	);
	const total = new BigNumber(balance);
	tokenBalances[`${symbol}:${tokenId}`] = {
	token: { issuer: { key: tokenId }, code: symbol },
	contractId: tokenId,
	total,
	symbol,
	...rest,
	};

freighter/extension/src/popup/helpers/account.ts

Lines 194 to 199 in 095a2af

	}
	if ("contractId" in bal && "symbol" in bal) {
	assetOperationMap[
	getCanonicalFromAsset(bal.symbol, bal.contractId || "")
	] = [];
	}

…and the symbol is the raw simulation result of symbol() on a third-party contract, with no normalization on the way in:

freighter/@shared/helpers/soroban/token.ts

Lines 85 to 99 in 095a2af

	export const getSymbol = async (
	contractId: string,
	server: SorobanRpc.Server,
	builder: TransactionBuilder,
	) => {
	const contract = new Contract(contractId);
	const tx = builder
	.addOperation(contract.call("symbol"))
	.setTimeout(TimeoutInfinite)
	.build();
	const result = await simulateTx<string>(tx, server);
	return result;
	};

Search-result rows have the same exposure, sourcing code from stellar.expert's record.code or an issuer's tomlInfo.code. I grepped for asset-code normalization and there is none in the repo. Classic assets can't reach this — [a-zA-Z0-9]{1,12} is enforced by the SDK's Asset constructor.

Where it surfaces. The single production caller of isNativeAsset chooses between a token payment and an XLM-funded createAccount:

freighter/extension/src/popup/components/send/SendAmount/hooks/useSimulateTxData.tsx

Lines 211 to 223 in 095a2af

	// create account if unfunded and sending the native asset
	if (!isFunded && isNativeAsset(sourceAsset)) {
	let createAccountDestination = destination;
	if (isMuxedAccount(destination)) {
	// encode muxed account to address
	createAccountDestination = extractBaseAddress(destination);
	}
	return Operation.createAccount({
	destination: createAccountDestination,
	startingBalance: amount,
	});
	}

Behaviour differs by entry point:

• Asset picker — isToken is derived from the un-round-tripped contract id, so it's true and the flow diverts to the Soroban path before getOperation runs. Not affected.
• Send deep link — isToken is derived from the parsed asset, whose issuer is now "", so it's false and the classic path is taken:

  freighter/extension/src/popup/views/Send/hooks/useSendQueryParams.ts

  Lines 120 to 127 in 095a2af

  	if (assetParam) {
  	try {
  	const asset = getAssetFromCanonical(assetParam);
  	dispatch(saveAsset(assetParam));
  	dispatch(saveIsCollectible(false));
  	dispatch(saveIsToken(isContractId(asset.issuer)));
  	} catch {
  	// Invalid asset param: keep current asset/flags when already selected.

  . Worth noting this path already accepts asset=native directly, so the parse quirk doesn't widen what a crafted URL can preselect.
• Most likely observed symptom — the contract-id extractor requires exactly two segments, so a colon-bearing symbol yields undefined, the token falls through to classic handling, and it renders with code XLM and an empty asset address:

  freighter/extension/src/popup/helpers/soroban.ts

  Lines 232 to 244 in 095a2af

  	return tokenId;
  	}
  	// Check if it's SYMBOL:CONTRACTID format (Soroban token)
  	// Split by : and check if the second part is a contract ID
  	const parts = tokenId.split(":");
  	if (parts.length === 2 && isContractId(parts[1])) {
  	// This is a Soroban token in SYMBOL:CONTRACTID format
  	return parts[1];
  	}
  	// Classic token format: CODE:ISSUER (no contract ID)
  	return undefined;

Adjacent, same shape: getCanonicalFromAsset(bal.symbol, bal.contractId || "") maps a symbol-XLM token with a missing contract id straight to the key "native".

Repro as a unit test — drop into @shared/helpers/__tests__/assetIdentity.test.ts alongside the new isNativeAsset cases; it fails on this branch and needs nothing deployed:

// add to the existing imports:
// import { getAssetFromCanonical, getCanonicalFromAsset } from "@shared/helpers/stellar";

it("rejects a contract token whose symbol contains a colon", () => {
  const contract = "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA";
  const canonical = getCanonicalFromAsset("XLM:", contract);
  expect(isNativeAsset(getAssetFromCanonical(canonical))).toBe(false);
});

Suggested fixes (in increasing order of depth):

1. Cheap guard, wrong layer: tighten the pair test to require issuer == null instead of any falsy issuer. Don't — "" legitimately means "no issuer" for native elsewhere (buildNativeAssetRow sets issuer: "", and getCanonicalFromAsset maps a falsy issuer to "native"), so this would break the native search row and the native canonical.
2. Fix the parse boundary: in getAssetFromCanonical, split on the last colon and require the issuer half to be a valid G…/C… StrKey, throwing invalid asset canonical id otherwise. A malformed identifier then fails loudly instead of becoming a silently mangled asset, and every consumer is fixed at once — including the contract-id extractor and the asset-address derivation.
3. Close it at the source: reject codes containing : in getCanonicalFromAsset and normalize the symbol when a Soroban balance is constructed, so a colon can't enter the identifier space at all. Only this option also resolves the display-layer confusion.

Correcting the doc comment on isNativeAsset is worth doing regardless of which fix lands, since it currently documents an invariant the code doesn't enforce.

[piyalbasu]

Test comment describes hardcoding this PR already removed

Trivial — not a merge blocker. Comment-only, no runtime effect. Raising it because it's provably wrong in its own commit rather than merely debatable, and it's a one-line edit.

TL;DR: The comment above the pinned native SAC addresses says they mirror values a helper hardcodes "before Task 8 removes them" — but this PR is what removed that hardcoding, so the comment describes a state that no longer exists as of this commit. It also points at an internal task-plan step that appears nowhere else in the repository, which won't mean anything to the next reader.

---

Detailed explanation (for agents)

The comment:

freighter/@shared/helpers/__tests__/assetIdentity.test.ts

Lines 18 to 26 in 095a2af

	"GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN";
	// The published native SAC addresses. Pinned rather than re-derived, so these
	// tests also assert that deriving from the passphrase reproduces the values
	// getNativeContractDetails hardcodes before Task 8 removes them.
	const NATIVE_SAC_PUBLIC =
	"CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA";
	const NATIVE_SAC_TESTNET =
	"CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC";

The contract id is already derived from the passphrase in this same PR, so nothing is hardcoding it "before Task 8":

freighter/extension/src/popup/helpers/searchAsset.ts

Lines 45 to 52 in 095a2af

	org: "",
	};
	// The native SAC address derives deterministically from the network
	// passphrase, which keeps every network correct by construction.
	const contract = getNativeContractId(networkDetails.networkPassphrase);
	switch (networkDetails.network as keyof typeof NETWORKS) {

git grep "Task 8" over the tree at this head returns exactly one hit — the comment itself.

Note the issuer field on the PUBLIC branch of that helper is still a literal, but that's a different field from the contract id the comment is about.

Suggested replacement — keeps the reason the values are pinned, drops the stale forward reference:

// The published native SAC addresses, pinned rather than re-derived, so these
// tests assert that deriving from the passphrase reproduces the known-good
// values rather than just agreeing with themselves.

[piyalbasu]

The classic-asset token type accepts any string, including "native"

Pre-existing, not a merge blocker — and the affected file isn't in this PR. Filing it here because this PR is the first code to route that field through a typed helper, which is what surfaces it, and because a reviewer looking at the new double cast deserves to know the cast isn't the problem.

TL;DR: The balance type that should mean "any asset type except native" is built with Omit over a string enum. Omit removes keys, and a string union's keys are String's methods — so nothing is removed and the result is an object type that accepts any string at all, "native" included. The type therefore fails at its only job, and that's why the new native-balance predicate needs a double cast to compare it: as far as the compiler is concerned the field isn't a string. Exclude is the operator that does what was intended here, and switching to it lets the cast go away entirely.

---

Detailed explanation (for agents)

The type, unchanged since before this PR:

freighter/@shared/api/types/account-balance.ts

Lines 18 to 28 in 095a2af

	export interface ClassicAsset {
	token: {
	type: Omit<SdkAssetType, "native">;
	code: string;
	issuer: { key: string };
	};
	available: BigNumber;
	total: BigNumber;
	buyingLiabilities: string;
	sellingLiabilities: string;
	blockaidData: Blockaid.TokenScanResponse;

The cast it forces, new in this PR:

freighter/@shared/helpers/assetIdentity.ts

Lines 43 to 47 in 095a2af

	/** True only for a balance whose token declares the native type. */
	export const isNativeBalance = (balance: AssetType): balance is NativeAsset =>
	"token" in balance &&
	"type" in balance.token &&
	isNativeAssetId(balance.token.type as unknown as string);

Why. SdkAssetType is a string enum, and Omit<T, K> expands to Pick<T, Exclude<keyof T, K>>. keyof a string-enum union yields String's prototype members (toString, charAt, length, …), none of which is "native" — so nothing is omitted, and the result is an object type describing String's shape. Any string structurally satisfies it. Verified against the repo's own TypeScript:

type Omitted  = Omit<SdkAssetType, "native">;
type Excluded = Exclude<SdkAssetType, "native">;

const a: Omitted  = "native";                    // no error  ← the value it exists to exclude
const b: Omitted  = "totally-not-an-asset-type"; // no error  ← any string at all
const c: Excluded = "native";                    // TS2322
const d: Excluded = "totally-not-an-asset-type"; // TS2322

Scope of the fix. git grep "token\.type" finds exactly one production consumer — the predicate linked above. So:

-    type: Omit<SdkAssetType, "native">;
+    type: Exclude<SdkAssetType, "native">;

-  isNativeAssetId(balance.token.type as unknown as string);
+  isNativeAssetId(balance.token.type);

After that, the narrowed field is SdkAssetType.native | Exclude<SdkAssetType, "native">, which is assignable to string, so the predicate typechecks unaided — and comparing a ClassicAsset's type against "native" becomes a compile error (TS2367, no overlap) instead of something a cast waves through, which is the outcome this PR's whole convention is after.

Why it predates this PR. @shared/api/types/account-balance.ts is not among this PR's changed files, and the Omit is present at the merge base (2d8876c). At that base the same read was a bare balance.token.type === "native" in extension/src/popup/helpers/balance.ts, which compiled only because comparing that object type to a string was permitted. Correctly out of scope for this PR's findings — worth its own two-line change.

[CassioMG]

Re: the native check accepts a contract token whose symbol contains a colon

Reproduced — your unit test fails on the previous head exactly as written. Fixed in feea4235 with your fix #2, at the parse boundary.

TL;DR: splitCanonical now splits on the last colon, and getAssetFromCanonical requires the issuer half to be a contract address, a public key, or the liquidity-pool sentinel — anything else throws invalid asset canonical id instead of flowing on as a mangled asset. The other six canonical parses route through the same helper, so the identifier space is colon-safe consistently rather than in one function. The isNativeAsset doc comment is corrected too.

One wrinkle worth recording, since it would bite anyone implementing this from the description: the pool sentinel is a legitimate non-StrKey issuer. LP shares are keyed <poolId>:lp and reach the parser through getBalanceByAsset, so a bare "must be a StrKey" check would have thrown for every LP holder. It's allowlisted explicitly and pinned by a test.

Details

• splitCanonical —

  freighter/@shared/helpers/stellar.ts

  Lines 191 to 211 in 66e102b

  	export const splitCanonical = (
  	canonical: string,
  	): { code: string; issuer: string } => {
  	const separator = canonical.lastIndexOf(":");
  	if (separator === -1) {
  	return { code: canonical, issuer: "" };
  	}
  	return {
  	code: canonical.slice(0, separator),
  	issuer: canonical.slice(separator + 1),
  	};
  	};
  	/**
  	* Resolves a canonical identifier to an asset.
  	*
  	* A contract token or a liquidity-pool share comes back as the plain
  	* `{ code, issuer }` shape; a classic asset comes back as an SDK `Asset`, whose
  	* constructor validates the code and issuer. Anything else — including an
  	* issuer half that is empty or not a StrKey — is rejected here, at the parse
  	* boundary, rather than flowing on as a plausible-looking asset.

• Validating parser, including the pool-sentinel arm —

  freighter/@shared/helpers/stellar.ts

  Lines 213 to 239 in 66e102b

  	export const getAssetFromCanonical = (canonical: string) => {
  	if (isNativeAssetId(canonical)) {
  	return StellarSdk.Asset.native();
  	}
  	const { code, issuer } = splitCanonical(canonical);
  	if (!issuer) {
  	throw new Error(`invalid asset canonical id: ${canonical}`);
  	}
  	if (StellarSdk.StrKey.isValidContract(issuer) || issuer === LP_ISSUER_KEY) {
  	return { code, issuer };
  	}
  	if (StellarSdk.StrKey.isValidEd25519PublicKey(issuer)) {
  	return new StellarSdk.Asset(code, issuer);
  	}
  	throw new Error(`invalid asset canonical id: ${canonical}`);
  	};
  	export const getCanonicalFromAsset = (
  	assetCode: string,
  	assetIssuer?: string,
  	) => {
  	if (isNativeAssetPair(assetCode, assetIssuer)) {
  	return "native";

• Routed through it: getContractIdFromTokenId and isSacContract (popup/helpers/soroban.ts), getAssetSacAddress (%40shared/helpers/soroban/token.ts), getAssetAddress (useSimulateTxData), sendWarnings, the hidden-asset keys in popup/helpers/account.ts, and the icon batching in %40shared/api/internal.ts.
• Your repro, as a test —

  freighter/@shared/helpers/__tests__/assetIdentity.test.ts

  Lines 117 to 121 in 66e102b

  	it("rejects a contract token whose symbol contains a colon", () => {
  	const contract = "CAS3J7GYLGXMF6TDJBBYYSE3HQ6BBSMLNUQ34T6TZMYMW2EVH34XOWMA";
  	const canonical = getCanonicalFromAsset("XLM:", contract);
  	expect(isNativeAsset(getAssetFromCanonical(canonical))).toBe(false);
  	});

• Eight parser cases including the LP sentinel and the rejection paths —

  freighter/@shared/helpers/__tests__/canonical.test.ts

  Lines 64 to 70 in 66e102b

  	it("rejects an issuer that is neither a G address, a C address nor the pool sentinel", () => {
  	expect(() => getAssetFromCanonical("XLM:")).toThrow(
  	/invalid asset canonical id/,
  	);
  	expect(() => getAssetFromCanonical("USDC:not-an-issuer")).toThrow();
  	});
  	});

Full suite 1860 passed / 0 failed; yarn build:extension exit 0.

[CassioMG]

Re: the classic-asset token type accepts any string

Confirmed against the repo's tsc — Omit accepts both "native" and "totally-not-an-asset-type" without error, Exclude rejects both (TS2322). Fixed in b81e3324; the double cast is gone.

TL;DR: Switching to Exclude immediately made the build fail — which is the useful part. The v2 balance mapper feeds a string-typed token.type straight into the classic shape, and that assignment only ever compiled because the type was loose. So the fix is two lines in account-balance.ts plus typing the v2 API boundary the same way; no cast anywhere, and the mapper is now checked rather than merely permitted.

Details

• Omit → Exclude —

  freighter/@shared/api/types/account-balance.ts

  Line 20 in 66e102b

  type: Exclude<SdkAssetType, "native">;

• Cast removed from the predicate —

  freighter/@shared/helpers/assetIdentity.ts

  Lines 44 to 47 in 66e102b

  	export const isNativeBalance = (balance: AssetType): balance is NativeAsset =>
  	"token" in balance &&
  	"type" in balance.token &&
  	isNativeAssetId(balance.token.type);

• The build failure it surfaced: mapClassic / mapSac in %40shared/api/helpers/mapAccountBalancesV2.ts (TS2322, Type 'string' is not assignable to '"credit_alphanum4" | "credit_alphanum12" | "liquidity_pool_shares"'). Fixed at the boundary rather than with a cast — V2ClassicBalance and V2SacBalance now declare type: Exclude<SdkAssetType, "native">, matching how that file already literal-types the native branch (token: { type: "native"; code: "XLM" }) and discriminates with token_type: "NATIVE":

  freighter/@shared/api/types/backend-api.ts

  Lines 83 to 87 in 66e102b

  	token: {
  	type: Exclude<SdkAssetType, "native">;
  	code: string;
  	issuer: V2TokenIssuer;
  	};

As you noted, comparing a ClassicAsset's type against "native" is now TS2367 rather than something a cast waves through.

[CassioMG]

Re: test comment describes hardcoding this PR already removed

Replaced with your wording in 66e102b1. The Task 8 reference was a leak from the internal plan this branch was written against — it should never have reached the tree.

TL;DR: A sweep of the whole diff for the same shape found four more spots narrating the previous behaviour rather than stating what the test checks — three test names ("a network the old table omitted", "a network the table doesn't cover", "the table omitted") and one comment about "the empty string the table used to fall back to". All rephrased in the same commit; the assertions are untouched.

Details

• Pinned-address comment, your replacement —

  freighter/@shared/helpers/__tests__/assetIdentity.test.ts

  Lines 20 to 23 in 66e102b

  	// predicate has to say so.
  	const XLM_CODED_ISSUER =
  	"GA5ZSEJYB37JRC5AVCIA5MOP4RHTM335X2KGX3IHOJAPP5RE34K4KZVN";

• Also rephrased: assetIdentity.test.ts ("derives a distinct address on FUTURENET"), searchAsset.test.js (name + the fall-back comment), searchAsset.test.ts, soroban.test.ts — each now names the network and behaviour under test instead of what preceded it.
• git grep "Task 8" over the tree now returns nothing.

[Copilot]

Pull request overview

Copilot reviewed 61 out of 61 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

extension/src/popup/helpers/assetList.ts:97

• Matching only issuer does not establish asset identity: one Stellar account can issue multiple asset codes. As written, getAssetListsForAsset reports a list for an unlisted asset whenever that list contains a different asset from the same issuer, and AddAsset then treats the token as verified. Compare (code, issuer) for classic assets (while retaining contract-id matching), and update the new shared-issuer test to cover different codes.

[Copilot]

Pull request overview

Copilot reviewed 61 out of 61 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 61 out of 61 changed files in this pull request and generated no new comments.