Repository navigation
Restrict currency treatment to verified asset IDs - #790
Conversation
Tickers are self-reported metadata: anyone can mint an asset with ticker "EUR" and the wallet would render the official flag logo, format its balance as fiat, and apply the EUR exchange rate to the portfolio total. Add isVerifiedAsset(assetId) to WalletContext, backed by the curated asset registry and the verified-assets list (both keyed by asset ID), and require it before a ticker earns any currency treatment: - usePortfolioFiat: no fiat price or sats-equivalent for unverified assets, so they no longer inflate the portfolio total - AssetCard, TransactionsList, Transaction, Send form: official token logos and fiat-style amount formatting only for verified asset IDs; unverified assets fall back to the generic avatar and plain amounts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThe wallet now exposes asset verification status and uses it to gate ticker-based formatting, token logos, send-form icons, transaction labels, and fiat portfolio calculations. Tests cover verified and unverified asset behavior. ChangesVerified asset handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AssetsContext
participant WalletProvider
participant WalletViews
participant PortfolioHook
AssetsContext->>WalletProvider: provide asset registration
WalletProvider->>WalletProvider: combine registration and icon approval
WalletProvider-->>WalletViews: return isVerifiedAsset(assetId)
WalletProvider-->>PortfolioHook: return isVerifiedAsset(assetId)
WalletViews->>WalletViews: derive trusted ticker, labels, and logos
PortfolioHook->>PortfolioHook: compute fiat only for verified assets
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Transaction.tsx (1)
151-175: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude
tickerin thelabelfallback sequence to prevent redundant names.In
TransactionsList.tsx, the label falls back totickerbeforename(accountTicker ?? ticker ?? meta?.name). However, inTransaction.tsx,tickeris omitted from thelabelfallback:const label = accountTicker ?? name ?? `${a.assetId.slice(0, 8)}...`For unverified assets,
accountTickerisundefined. If the asset has both anameand aticker,labelwill evaluate toname. This causes the asset amount to display as{amount} {name}, and immediately below it, the secondary text will also render{name}(becausename && ticker && !accountTickerevaluates to true).Adding
tickerto the fallback sequence ensures consistency with the list view and prevents displaying the name twice.🐛 Proposed fix
- const label = accountTicker ?? name ?? `${a.assetId.slice(0, 8)}...` + const label = accountTicker ?? ticker ?? name ?? `${a.assetId.slice(0, 8)}...` const tokenLogoTicker = tokenLogoTickerForTicker(trustedTicker) return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/Wallet/Transaction.tsx` around lines 151 - 175, Update the label fallback in the asset rendering block of Transaction.tsx to use ticker before name, matching the fallback order used by TransactionsList.tsx. Preserve the existing accountTicker priority and assetId truncation fallback so unverified assets with both a name and ticker display the ticker as the primary label without duplicating the name.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TransactionsList.tsx`:
- Around line 113-124: Convert a.amount to BigInt at the
prettyCurrencyAssetAmount call in the transaction row rendering, matching the
existing pattern in Transaction.tsx while leaving the label and trustedTicker
handling unchanged.
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 943-946: Update the asset selector and selected-asset summary
around AssetIcon to gate ticker display with isVerifiedAsset(asset.assetId). For
unverified assets, render the asset name and amount without any ticker suffix;
retain the existing ticker-based labels only for verified assets.
- Around line 69-71: Update AssetIcon so every asset visual is gated by
verified, not only the ticker used by TokenLogo. Restrict the asset.icon branch
and fallback/initial rendering to verified assets, ensuring unverified assets
render no misleading icon or ticker-derived initial while preserving verified
behavior.
---
Outside diff comments:
In `@src/screens/Wallet/Transaction.tsx`:
- Around line 151-175: Update the label fallback in the asset rendering block of
Transaction.tsx to use ticker before name, matching the fallback order used by
TransactionsList.tsx. Preserve the existing accountTicker priority and assetId
truncation fallback so unverified assets with both a name and ticker display the
ticker as the primary label without duplicating the name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f401d68e-2fb0-4ab0-843a-8aab16199010
📒 Files selected for processing (9)
src/components/AssetCard.tsxsrc/components/TransactionsList.tsxsrc/hooks/usePortfolioFiat.tssrc/providers/wallet.tsxsrc/screens/Wallet/Send/Form.tsxsrc/screens/Wallet/Transaction.tsxsrc/test/components/AssetCard.test.tsxsrc/test/hooks/usePortfolioFiat.test.tsxsrc/test/screens/mocks.ts
…ting
Address review feedback on the verified-asset gating:
- Show an "Unverified" badge next to the asset name in AssetCard rows and
the send-form asset selector, so a self-reported ticker ("USD", "EUR")
is visibly flagged instead of silently trusted
- AssetIcon now reads isVerifiedAsset from WalletContext itself instead of
prop-drilling a verified flag through three call sites (same pattern as
AssetCard)
- Transaction detail: fall back to ticker before name for the asset label,
matching TransactionsList and avoiding the name rendering twice
- Add a regression test pinning that an unverified asset with ticker "BTC"
is not rendered as native bitcoin; drop a redundant test wrapper
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
Deploying wallet-mutinynet with
|
| Latest commit: |
0846ceb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9aa952ae.arkade-wallet.pages.dev |
| Branch Preview URL: | https://claude-asset-id-verification.arkade-wallet.pages.dev |
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
0846ceb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://aea9abbc.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://claude-asset-id-verification.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
0846ceb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cae6580b.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://claude-asset-id-verification.wallet-bitcoin.pages.dev |
The asset selector now only offers verified assets. Unverified assets can still be sent deliberately via the Assets app detail screen, which preselects the asset in the send form — the trigger keeps its "Unverified" badge for that case, and the dropdown badge is gone since every listed option is verified. Holding only unverified assets no longer surfaces the selector. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/screens/Wallet/Transaction.tsx (1)
167-169: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGate unverified asset text with an explicit "Unverified" badge.
While official logos and fiat formatting are correctly restricted, the unverified
tickerornameis still displayed here as the primarylabelwithout an indicator. This allows a self-reported ticker (likeUSD) to appear trusted in the receipt view.Please import
Badgeat the top of the file (import { Badge } from '../../components/ui/badge') and append the badge next to the label when the asset is not trusted.Based on learnings, if an asset is unverified, it may be shown as plain text only when accompanied by an explicit “Unverified” badge.
🛡️ Proposed fix to add the badge
- <span className='transaction-detail-asset__amount'> - {prettyCurrencyAssetAmount(BigInt(a.amount), decimals, trustedTicker)} {label} - </span> + <span className='transaction-detail-asset__amount'> + {prettyCurrencyAssetAmount(BigInt(a.amount), decimals, trustedTicker)} {label} + {!trusted ? ( + <Badge variant='outline' className='ml-1.5'> + Unverified + </Badge> + ) : null} + </span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/screens/Wallet/Transaction.tsx` around lines 167 - 169, Update the transaction asset amount rendering in Transaction to import and use Badge, appending an explicit “Unverified” badge beside label whenever the asset is not trusted. Preserve the existing label and amount rendering for trusted assets, and ensure unverified labels are never shown without the badge.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/screens/Wallet/Transaction.tsx`:
- Around line 167-169: Update the transaction asset amount rendering in
Transaction to import and use Badge, appending an explicit “Unverified” badge
beside label whenever the asset is not trusted. Preserve the existing label and
amount rendering for trusted assets, and ensure unverified labels are never
shown without the badge.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae68d8e2-665a-49f1-9787-951ed616860a
📒 Files selected for processing (4)
src/components/AssetCard.tsxsrc/screens/Wallet/Send/Form.tsxsrc/screens/Wallet/Transaction.tsxsrc/test/components/AssetCard.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/components/AssetCard.test.tsx
- src/components/AssetCard.tsx
The receipt view showed an unverified asset's self-reported ticker or name as the label with no indicator. Append the same Unverified badge used in asset cards and the send selector, so unverified labels are never shown without it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
Apply review cleanups, behavior-identical except one addition: - Extract the Unverified badge into a shared UnverifiedBadge component (was copy-pasted verbatim in AssetCard, the send selector, and the transaction receipt) - Add trustedAssetTickers() next to the other ticker helpers in TokenLogo.tsx as the single sanctioned derivation of accountTicker / trustedTicker, replacing the duplicated inline trio in TransactionsList and Transaction - Drop the dead `assetId &&` conjunct in AssetCard's badge condition and the identity BigInt() wrap in the receipt (Asset.amount is bigint) - Show the Unverified badge on activity-list asset rows too — the one surface still printing self-reported tickers without it Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
For asset transfers tx.amount is only the dust carrying the asset; rendering it fiat-converted as Amount/Total reads as a price for the asset. Omit both rows on asset receipts (Table already skips empty values), keeping fees. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
The send picker now only lists verified assets, and nothing is verifiable on regtest, so the e2e asset-send test timed out waiting for the selector. Route both sends through the Arkade Mint asset detail screen, which preselects the asset in the send form — the sanctioned path for unverified assets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
Takes #790's isVerifiedAsset as the single trust gate: portfolio designations, receipt asset rows, the send-form picker/logos, and activity rows all require a verified asset ID before a ticker earns currency treatment, with the Unverified badge on anything self-reported. Asset transfers that can't be valued as an account hide Amount/Total (master semantics) instead of showing the dust carrier as a price. Also picks up the Virtual Coins asset-id display improvements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UewCGCJveUjTUgmBxMk3Ps
It fixes #791
Summary
This change implements a security measure to prevent unverified assets from claiming currency identity (official logos, fiat pricing, fiat-style formatting) based on self-reported tickers. Currency treatment is now restricted to asset IDs that are either in the curated verified assets list or approved by the icon approval manager.
Key Changes
isVerifiedAssetfunction to WalletContext: Checks if an asset ID is in the curated list (viaAssetsContext.isRegistered) or approved by the icon approval managerImplementation Details
isVerifiedAssetfunction is centralized in WalletProvider and checks both the icon approval manager and the assets registrytrustedTickervariable is introduced throughout the codebase to distinguish between self-reported tickers and tickers that have earned currency treatment through verificationtrustedTickerinstead of raw ticker valueshttps://claude.ai/code/session_01EUQWiNVnjj2dtcLLFR9pWf
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests