fix: the dApp approval screen renders a 5,000-token transfer as "Amount 0.0000" #306
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Found by the pre-1.0 deployability audit (#303). Blocker: the user approves a drain that the screen shows as zero.
src/popup/views/approval.js:60-62:decodeCalldataconsults only the 512-entry bundled list. It never readsstate.trackedTokensand never reads the decimals Blockscout already reported and stored inaddr.tokenBalances. Every token outside the bundled list is formatted as 18 decimals.Reproduction
transfer(0xC0FFEE..., 5000000000)on a 6-decimal token = 5,000 tokens:The same understatement applies to
approve, where an allowance draining a 6-decimal token also renders0.0000.Consequence
The user reads
0.0000, sees a token they hold listed as "Unknown token", and confirms. They lose the whole balance, or grant an unbounded allowance. Precondition: any ERC-20 outside the bundled top-list — which is most of them, including anything the user added by contract address.Definition of done
state.trackedTokens, then the address's own Blockscout-reported decimals.approveas well astransfer.make checkgreen.Plan:
New
src/shared/approvalAmount.js(pure, no DOM):resolveTokenDecimals(address, { trackedTokens, wallets })— bundledTOKEN_BY_ADDRESS, thenstate.trackedTokens, then the explorer-reporteddecimalson any address'stokenBalancesentry for that contract. Returnsnullwhen no source has a usable uint8, and also when two explorer entries for the same contract disagree, since a disputed scale is not a known one.unknownDecimalsAmount(raw)— the refusal rendering:"5000000000 base units (decimals unknown)". NoformatUnitscall is reached on that path, so no guessed-scale number can be produced at all.decodeCalldatainsrc/popup/views/approval.jscalls both, forapproveandtransferalike; theUnlimitedallowance rendering is unaffected because it never needed a scale. The value also feedspendingTxDetails.amount, so the status screens carry the same string rather than a formatted one.src/shared/transferAmount.jsis the send path's counterpart but goes the other way (decimal string plus scale to base units, refusing on a scale mismatch at signing time); the only part that fits here is its uint8 bound, which is imported rather than restated. Rationale goes in the PR body.Symbol resolution (the
Unknown tokenline) is untouched — outside this issue's stated scope.Built as described, on
issue-306-approval-decimals; PR #321 (basenext) carries the detail.Verification:
make checkgreen — 36 suites, 788 tests,test-verify-build18 cases,check-censored144 files; lint executed inside the pinned container rather than off cache.The new tests were proved non-vacuous by restoring only the old lookup (
token ? token.decimals : 18): 6 of the 15 fail, and the unknown-decimalstransfercase then receives exactly"0.0000"— this issue's screen output — while the other 782 tests in the suite stay green, so nothing existing depended on the 18 default.