Skip to content

Closes #729 - #782

Open
mansur-codes wants to merge 4 commits into
Miracle656:mainfrom
mansur-codes:main
Open

mansur-codes wants to merge 4 commits into
Miracle656:mainfrom
mansur-codes:main

Conversation

@mansur-codes

Copy link
Copy Markdown

Summary

Stellar mainnet has 20+ issuers for USDY and EURC each, most of them fakes — one counterfeit EURC even publishes a blackrock.com.se domain. The wallet had no way to distinguish the real issuer from impostors; it resolved whatever it found first.

This PR adds a static verified asset registry shared across web and mobile, pins exact mainnet issuers for USDC, XLM, EURC, AQUA, and USDY, and enforces that any unregistered issuer renders as "Unverified: CODE (issuer GABC…)" everywhere: balance rows, pickers, quotes. A new CI script (verify-asset-registry.mjs) fetches each issuer's stellar.toml at build time and fails the pipeline on any mismatch.

Related issue

Closes #729

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Docs
  • Tests
  • CI / tooling

Component

  • Wallet frontend
  • SDK
  • Contracts
  • Agent

Checklist

  • I have read CONTRIBUTING.md
  • cargo test passes (contracts)
  • npm run typecheck passes (wallet / sdk / agent)
  • npm run build passes (wallet / agent)
  • I added or updated tests where relevant
  • I updated docs / README where relevant

Screenshots / test output

npm run verify:assets   ✓ 5/5 registry entries verified against Stellar mainnet
npm run verify:lockfile ✓ lockfile clean
npm run typecheck       ✓ passed (wallet, mobile)
npm test                ✓ passed (wallet, mobile)

@drips-wave

drips-wave Bot commented Sep 24, 2026

Copy link
Copy Markdown

@mansur-codes Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@vercel

vercel Bot commented Sep 24, 2026

Copy link
Copy Markdown

@mansur-codes is attempting to deploy a commit to the miracle656's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The registry data here is genuinely good — and after what the rest of this week's PRs did with addresses, that's worth saying first. All five issuers are valid strkeys and all five resolve on live mainnet Horizon with matching home_domain. No fabrication. The tests are real too, including the lookalike-issuer case asserting 'Unverified: USDY (issuer GFAK…)'. Three things block it.

1. The main acceptance criterion is unwired

#729's AC #2 is: "An asset that is not in it renders as 'Unverified: CODE (issuer GABC…)' everywhere it appears: balance rows, pickers, quotes."

$ git grep -n "formatAssetLabel\|isRegisteredIssuer" -- ':!*__tests__*' ':!scripts/*'
frontend/mobile/lib/assets.ts:121:export function isRegisteredIssuer(
frontend/mobile/lib/assets.ts:125:export function formatAssetLabel(
frontend/wallet/lib/assets.ts:103:export function isRegisteredIssuer(
frontend/wallet/lib/assets.ts:107:export function formatAssetLabel(

Definitions only — zero call sites. No balance row, picker or quote screen is touched, and the existing consumers (app/assets/page.tsx, app/assets.tsx) still call getRegisteredAsset('USDY') with one argument. So nothing in the app renders "Unverified:" today. The functions and their tests are the easy half; the wiring is the half that closes the issue.

2. expo-asset ^57.0.18 is the wrong major, and conflicts with main

package.json:  "expo": "~54.0.0",
               "expo-asset": "^57.0.18",     ← SDK-57 release
main:          "expo-asset": "~12.0.13",     ← SDK-54 release

npm dist-tags: sdk-54 → 12.0.13, sdk-57 → 57.0.18, latest → 57.0.18. This took latest and pinned an SDK-57 package onto an SDK-54 app; the lockfile then pulled a whole SDK-57 subtree (expo-asset/node_modules/{@expo/env, @expo/image-utils, expo-constants, semver}), which is most of the +89 lines.

That's not hypothetical here — a mismatched expo-constants major is exactly what silently broke the About screen and the update check earlier this week, because the API it read had been removed.

I added expo-asset ~12.0.13 to main in bad9c03 (expo-font needs it but doesn't declare it), so please drop the expo-asset entry entirely and regenerate with plain npm install in frontend/mobile — never --legacy-peer-deps, which corrupts that lockfile. Currently:

CONFLICT (content): frontend/mobile/package-lock.json
CONFLICT (content): frontend/mobile/package.json

3. The CI script fails open, and only checks half the registry

The script does hit the real network and works — but:

  • It passes entries it never verified. When the toml can't be fetched it prints ✓ Horizon issuer account verified for domain "circle.com" and returns. Circle's toml wasn't fetched in my run, so USDC and EURC both printed ✓ with no toml check at all — which is the AC ("fails when a registry entry's home domain does not match its issuer's stellar.toml") going unenforced for 2 of 5 entries. Same fail-open when currencies.length === 0. A verifier that can't reach its source should fail, not pass.
  • It only parses frontend/wallet/lib/assets.ts. The mobile registry is a hand-duplicated copy and is never verified, so drift between them is undetectable. #729 asked for "One registry module, shared by web and mobile" — the duplication is the thing the script most needs to catch, and can't.
  • It runs on every push, so CI now depends on horizon.stellar.org, stellar.org, aqua.network and ondo.finance being reachable. Please move it to a scheduled job or mark it continue-on-error.

Smaller

frontend/wallet/lib/__tests__/feeBump.test.ts gains a TextEncoder/TextDecoder polyfill unrelated to #729 — and it's now redundant, since main fixed that file in 818f539 by stubbing the @veil/sdk barrel. Drop the hunk on rebase.

Please coordinate with #760

Open PR #760 ("flag unverified asset impersonators", closes #730) adds a competing lib/assetRegistry.ts in both apps and edits frontend/mobile/lib/assets.ts — the same file you rewrite. It also does the wiring you've left out (AssetRow.tsx, AssetsList.tsx, dashboard/page.tsx). Merging both would leave two parallel registries, which for a security feature means drift is the vulnerability.

Your registry is the better-sourced one and #760's is the wiring. Worth agreeing which module is canonical before either lands — happy to help broker that if you two want to split it.

@Miracle656 Miracle656 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good progress — two of the three blockers are properly closed, and the main one was the one that actually mattered.

Closed:

  • 1 — the wiring. formatAssetLabel and isRegisteredIssuer have call sites now, which is what #729's AC #2 asked for:

    frontend/mobile/components/AssetRow.tsx:30   {formatAssetLabel(asset.code, asset.issuer, getNetworkName())}
    frontend/mobile/components/AssetRow.tsx:39   <Text style={styles.unverified}>Unverified asset</Text>
    frontend/wallet/app/assets/page.tsx:90       if (!isRegisteredIssuer(line.code, line.issuer)) continue
    frontend/wallet/app/assets/page.tsx:468      {formatAssetLabel(line.code, line.issuer)}
    

    That is the half that closes the issue, and it is done.

  • 2 — expo-asset. Now ~12.0.13 against expo ~54.0.0. Correct major, and the SDK-57 subtree is gone from the lockfile.

  • 3b — the script parses the mobile registry too, so drift between the two copies is now detectable.

Two things left, and one of them is new.

The CI step now undoes a change that landed yesterday

.github/workflows/ci.yml:29

      - run: npm run verify:assets

main no longer has that line. #892 merged yesterday and moved the live check off the push path entirely: verify:assets:offline (StrKey, SAC derivation, wallet/mobile parity — no network) stayed fast, and the network-dependent half became a scheduled job in .github/workflows/asset-registry-verify.yml. Your branch is current with main, so this line is re-adding what that PR removed, and every contributor's push would start depending on horizon.stellar.org, circle.com, aqua.network and ondo.finance being reachable.

It would also be red immediately rather than eventually: see #978 — circle.com/.well-known/stellar.toml returns 404, so USDC and EURC cannot be corroborated by toml at all today. That is a real gap in our registry metadata, but it is not something a contributor's unrelated PR should go red for.

Use npm run verify:assets:offline there if you want a push-path check — it covers the parity your point 3b work is about, without the network.

While you are rebasing: scripts/verify-asset-registry.mjs was also restructured by #892, so that file is worth re-reading before changing it further. The fail-open paths you were asked to close may already be closed there.

Build byproducts are still committed

frontend/wallet/public/sw.js      1+/1-   regenerated service worker
frontend/wallet/next-env.d.ts     1+/1-
frontend/wallet/middleware.js     2+/0-

git checkout -- on those three. sw.js matters beyond tidiness: committing a locally-built service worker pins chunk hashes that will not exist in the real deploy. (This catches me too — I dirtied the same files running next build last week.)

And #760 is still open

#760 adds a competing lib/assetRegistry.ts in both apps and edits frontend/mobile/lib/assets.ts, which you rewrite. Now that your wiring is in, the two PRs overlap on the display layer as well as the data. Yours is the better-sourced registry; whichever lands second will need to drop its own copy rather than merge alongside it. Two parallel registries for a feature whose purpose is detecting impostors would make drift the vulnerability.

Fix the CI line and drop the three byproducts and I am happy to merge this.

@Miracle656

Copy link
Copy Markdown
Owner

Correction to my review — I lumped frontend/wallet/middleware.js in with the build byproducts, and it is not one. Please keep that change.

It is the CSP connect-src allowlist, and you are adding:

+  "https://stellar.org",
+  "https://aqua.network",

main already lists https://ondo.finance and https://circle.com there for the same reason. Without those two entries the browser blocks the stellar.toml fetches for the Stellar and Aquarius issuers, so the verification this PR is about would fail in the browser and nowhere else. That is a necessary part of the feature, and I should have read it rather than pattern-matching on the filename.

The two that genuinely are byproducts:

frontend/wallet/next-env.d.ts     -import "./.next/dev/types/routes.d.ts"
                                  +import "./.next/types/routes.d.ts"
frontend/wallet/public/sw.js      regenerated minified service worker

git checkout -- frontend/wallet/next-env.d.ts frontend/wallet/public/sw.js on those two only.

So the remaining list is shorter than my review implied: the ci.yml line, and those two files. Sorry for the noise.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A verified asset registry, pinned by issuer

2 participants