Skip to content

Stop emitting tokens with default decimals=18 when metadata is missing#10

Open
benwoody wants to merge 1 commit into
EthereumPhone:feature-token_extensionsfrom
benwoody:fix/issue-9-decimals-default
Open

Stop emitting tokens with default decimals=18 when metadata is missing#10
benwoody wants to merge 1 commit into
EthereumPhone:feature-token_extensionsfrom
benwoody:fix/issue-9-decimals-default

Conversation

@benwoody

Copy link
Copy Markdown

Refs #9 — addresses the primary symptom (off-by-10¹² displayed balance for non-18-decimal tokens) on the swap-screen path.

Problem

AlchemyTokenBalanceRepository#getCombinedTokens was constructing a TokenAsset with decimals = 18 whenever a token had a balance row but no matching TokenMetadataEntity in the local DB. For
6-decimal tokens like USDC, a real balance of ~10 USDC displayed as ~10⁻¹¹ USDC.

The home view already filters tokens without metadata via DefaultGroupedTokenRepository, so in practice this code path only fed the swap UI — which is exactly where the wildly wrong number was
visible.

Fix

Drop the assetsWithoutMetadata branch entirely. Tokens without metadata are no longer emitted from getCombinedTokens() until their TokenMetadataEntity row is populated. Matches the home-view
behavior and stops emitting wrong numbers.

Out of scope, intentional follow-ups

Same decimals = 18 antipattern exists in two more places not touched by this PR:

  1. SwapViewModel.kt:920–928 and :947–957 — the else fallback and the catch block in convertToSwapToken both fabricate a TokenAsset with decimals = 18 and a zero address. Fixing properly
    requires either restructuring the function to return SwapToken? and updating callers, or surfacing failure as a UI-level error. Worth a separate PR.

  2. DefaultGroupedTokenRepository.kt:79–83 — silently skips tokens without metadata. Not strictly a bug, but amplifies the symptom. Belongs in the principled follow-up that wires
    OnChainTokenMetadataFetcher into the no-metadata path so non-18-decimal tokens display correctly rather than being hidden.

Happy to tackle either as follow-up PRs if you want.

No test added

No existing test coverage for AlchemyTokenBalanceRepository#getCombinedTokens and no MockWebServer-style pattern in the repo. Happy to add one (and any necessary test infrastructure) in a follow-up
if desired.

To maintainers:

Can the branches get cleaned up if you accept PRs? I would assume main would be the main branch to build from.

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.

1 participant