harden: resolve the swap approval screen's token scale, or refuse to format #345

Merged

clawbot merged 1 commits from harden/340-uniswap-unknown-decimals into next 2026-08-23 16:20:23 +02:00

Conversation 1 Commits 1 Files Changed 6 +271 -23

clawbot commented 2026-08-23 15:50:22 +02:00

Collaborator

Copy Link

Closes #340.

The defect

tokenInfo() in src/shared/uniswap.js returned decimals: 18 for any token
absent from the bundled token list — the same guessed scale that
#306 removed from the ERC-20 amount
line of the same screen. A 1,000-token swap of a 6-decimal token was stated as
0.000000001: a wrong number, not an imprecise one, on the screen whose only
job is to say what is being authorized. Every token off the bundled list reached
it, which is every newly listed token.

The change

The swap's Amount and Min. received lines now resolve the scale through
resolveTokenDecimals() — bundled list, then the tokens the user tracks, then
the decimals the block explorer already reported — and where none of those
answers they render unknownDecimalsAmount(), the existing mechanism: the
base-unit integer with the scale stated in the same string. That is the same
refusal the ERC-20 line already makes, so the approval screen has one way of
resolving a scale and one way of saying it has none, not two.

• uniswap.decode() takes those sources as a third argument; decodeCalldata()
  supplies them from state, the same object the ERC-20 path already built.
• No network fetching and no new data source. The scale comes only from what
  the wallet already holds, so the approval screen still makes no request before
  showing what is being signed.
• An unbounded permit needs no scale to describe and is still shown as
  Unlimited, matching the approve case.
• The bundled token list is unchanged, and the display helpers are unchanged.

How a refusal reads as a refusal

The line is the whole string 1000000000 base units (decimals unknown), not a
number with a caveat elsewhere on the screen. It has no decimal point, so it
cannot be read as a token quantity; it carries no symbol; and it states the
unknown scale inline. It can never read as zero for a nonzero swap. The same
string is what is carried forward as rawValue to the wait/success/error
screens, so those cannot show a formatted figure the approval screen refused
to show. Asserted by test on both value and rawValue.

Fail-first evidence

Mutation: git stash push -- src/ — the new test file
tests/uniswapUnknownDecimals.test.js kept, all three src/ files reverted to
next at 669c443. Then make test:

● a swap of a token outside the bundled list › shows the true quantity when the user tracks the token
  Expected: "1000.0000"
  Received: "0.000000001"
● a swap of a token outside the bundled list › shows the true quantity from the explorer's decimals
  Expected: "1000.0000"
  Received: "0.000000001"
● a swap of a token outside the bundled list › refuses to format when nothing knows the scale
  Expected: "1000000000 base units (decimals unknown)"
  Received: "0.000000001"
● a swap of a token outside the bundled list › the amount carried to the status screens is the same refusal
  Expected: "1000000000 base units (decimals unknown)"
  Received: "0.000000001"
● the Min. received line takes the same rule › refuses to format an output token of unknown scale
  Expected: "1000000000 base units (decimals unknown)"
  Received: "0.000000001"
● the Min. received line takes the same rule › shows the true quantity when the user tracks the output token
  Expected: "1000.0000"
  Received: "0.000000001"
● the permit amount takes the same rule › refuses to format a permit on a token of unknown scale
  Expected: "1000000000 base units (decimals unknown)"
  Received: "0.000000001"

Test Suites: 1 failed, 42 passed, 43 total
Tests:       7 failed, 850 passed, 857 total

0.000000001 is the 18-decimal guess applied to 1,000,000,000 base units — the
issue's 1,000.00 of a 6-decimal token. The file holds nine tests: the seven
above fail under that mutation, and two pass both before and after as
regression guards — a bundled token on the other side still formats
(0.5000 WETH), and an unbounded permit is still named, with or without a scale.

Verification

make check green on the pushed tip: Test Suites: 43 passed,
Tests: 857 passed, 857 total; script/test-verify-build, check-censored
(155 tracked files), lint and fmt-check all pass. Lint ran in the pinned
container and executed rather than replaying cache — the [lint 1/1] RUN make lint layer shows DONE 4.5s with the eslint and prettier output, not CACHED.
An earlier run of that same layer failed on an unused import in the new test,
which is what proves it was really executing.

No containers created and none left behind (docker ps -a empty for this
session).

Files

• src/shared/uniswap.js — tokenInfo() resolves the scale or reports none;
  new amountText() formats or refuses; decode() takes the sources.
• src/popup/views/approval.js — one decimalsSources object, used by the
  ERC-20 path and passed to uniswap.decode().
• src/shared/approvalAmount.js — comment only: records that the swap lines
  now share these two functions.
• tests/uniswapUnknownDecimals.test.js — new, 9 tests.
• README.md, TODO.md.

Closes https://git.eeqj.de/sneak/AutistMask/issues/340. ## The defect `tokenInfo()` in `src/shared/uniswap.js` returned `decimals: 18` for any token absent from the bundled token list — the same guessed scale that https://git.eeqj.de/sneak/AutistMask/issues/306 removed from the ERC-20 amount line of the same screen. A 1,000-token swap of a 6-decimal token was stated as `0.000000001`: a wrong number, not an imprecise one, on the screen whose only job is to say what is being authorized. Every token off the bundled list reached it, which is every newly listed token. ## The change The swap's `Amount` and `Min. received` lines now resolve the scale through `resolveTokenDecimals()` — bundled list, then the tokens the user tracks, then the decimals the block explorer already reported — and where none of those answers they render `unknownDecimalsAmount()`, the existing mechanism: the base-unit integer with the scale stated in the same string. That is the same refusal the ERC-20 line already makes, so the approval screen has one way of resolving a scale and one way of saying it has none, not two. - `uniswap.decode()` takes those sources as a third argument; `decodeCalldata()` supplies them from `state`, the same object the ERC-20 path already built. - **No network fetching and no new data source.** The scale comes only from what the wallet already holds, so the approval screen still makes no request before showing what is being signed. - An unbounded permit needs no scale to describe and is still shown as `Unlimited`, matching the `approve` case. - The bundled token list is unchanged, and the display helpers are unchanged. ### How a refusal reads as a refusal The line is the whole string `1000000000 base units (decimals unknown)`, not a number with a caveat elsewhere on the screen. It has no decimal point, so it cannot be read as a token quantity; it carries no symbol; and it states the unknown scale inline. It can never read as zero for a nonzero swap. The same string is what is carried forward as `rawValue` to the wait/success/error screens, so those cannot show a formatted figure the approval screen refused to show. Asserted by test on both `value` and `rawValue`. ## Fail-first evidence Mutation: `git stash push -- src/` — the new test file `tests/uniswapUnknownDecimals.test.js` kept, all three `src/` files reverted to `next` at `669c443`. Then `make test`: ``` ● a swap of a token outside the bundled list › shows the true quantity when the user tracks the token Expected: "1000.0000" Received: "0.000000001" ● a swap of a token outside the bundled list › shows the true quantity from the explorer's decimals Expected: "1000.0000" Received: "0.000000001" ● a swap of a token outside the bundled list › refuses to format when nothing knows the scale Expected: "1000000000 base units (decimals unknown)" Received: "0.000000001" ● a swap of a token outside the bundled list › the amount carried to the status screens is the same refusal Expected: "1000000000 base units (decimals unknown)" Received: "0.000000001" ● the Min. received line takes the same rule › refuses to format an output token of unknown scale Expected: "1000000000 base units (decimals unknown)" Received: "0.000000001" ● the Min. received line takes the same rule › shows the true quantity when the user tracks the output token Expected: "1000.0000" Received: "0.000000001" ● the permit amount takes the same rule › refuses to format a permit on a token of unknown scale Expected: "1000000000 base units (decimals unknown)" Received: "0.000000001" Test Suites: 1 failed, 42 passed, 43 total Tests: 7 failed, 850 passed, 857 total ``` `0.000000001` is the 18-decimal guess applied to 1,000,000,000 base units — the issue's 1,000.00 of a 6-decimal token. The file holds nine tests: the seven above fail under that mutation, and two pass both before and after as regression guards — `a bundled token on the other side still formats` (`0.5000 WETH`), and `an unbounded permit is still named, with or without a scale`. ## Verification `make check` green on the pushed tip: `Test Suites: 43 passed`, `Tests: 857 passed, 857 total`; `script/test-verify-build`, `check-censored` (155 tracked files), `lint` and `fmt-check` all pass. Lint ran in the pinned container and executed rather than replaying cache — the `[lint 1/1] RUN make lint` layer shows `DONE 4.5s` with the eslint and prettier output, not `CACHED`. An earlier run of that same layer failed on an unused import in the new test, which is what proves it was really executing. No containers created and none left behind (`docker ps -a` empty for this session). ## Files - `src/shared/uniswap.js` — `tokenInfo()` resolves the scale or reports none; new `amountText()` formats or refuses; `decode()` takes the sources. - `src/popup/views/approval.js` — one `decimalsSources` object, used by the ERC-20 path and passed to `uniswap.decode()`. - `src/shared/approvalAmount.js` — comment only: records that the swap lines now share these two functions. - `tests/uniswapUnknownDecimals.test.js` — new, 9 tests. - `README.md`, `TODO.md`.

clawbot added the needs-review label 2026-08-23 15:50:22 +02:00

clawbot added 1 commit 2026-08-23 15:50:22 +02:00

harden: resolve the swap approval screen's token scale, or refuse to format (closes #340) ... a658fde62d

tokenInfo() in src/shared/uniswap.js returned decimals: 18 for any token absent from the bundled token list, the same guessed scale that issue 306 removed from the ERC-20 amount line of the same screen. A 1,000-token swap of a 6-decimal token was therefore stated as 0.000000001, a wrong number rather than an imprecise one, and every newly listed token reached it. The swap's Amount and Min. received lines now resolve the scale through resolveTokenDecimals() -- the bundled list, then the tokens the user tracks, then the decimals the block explorer reported -- and where none of those answers they render unknownDecimalsAmount(), the base-unit integer with the scale stated, which is the same refusal the ERC-20 line already makes. No new data source and no network call: the scale comes only from what the wallet already holds, so the screen still makes no request before showing what is being signed. uniswap.decode() takes those sources as a third argument, supplied by decodeCalldata() from state alongside the ones the ERC-20 path already used. An unbounded permit needs no scale to describe and is still shown as Unlimited. README.md records the rule as a Display Consistency exception.

clawbot self-assigned this 2026-08-23 15:50:26 +02:00

clawbot commented 2026-08-23 16:02:49 +02:00

Author

Collaborator

Copy Link

FAIL — needs-checks. The change itself is sound; the head commit's CI is red for an unrelated reason.

1. CI red on a658fde — e2e / e2e-chrome failed (not this diff). Failing cases are 28, 31, 45, 53, 54, and the cause is a sepolia network selection leaking across the suite: the network selector shows "sepolia" after restoring and reopening the popup, expected "mainnet" and eth_chainId did not round trip through the extension: "0xaa36a7". Nothing in this diff touches network selection or eth_chainId, and the suite has no swap coverage at all. make test-e2e on this exact head in a fresh clone passes 55/55, and e2e-chrome was green on the base 669c443. Needs a CI re-run, not a code change.

2. Fail-first evidence is miscounted (PR body). tests/uniswapUnknownDecimals.test.js contains 9 tests, not 8. Reverting src/ to next reproduces the stated result exactly — 7 failed, 850 passed, 857 total, every failure Received: "0.000000001" — which means two tests pass both before and after: a bundled token on the other side still formats and an unbounded permit is still named, with or without a scale. The body names only the first. Correct the count and disclose both, so the fail-first claim covers the file it describes.

3. The explorer link can return an 18 no explorer reported. src/shared/balances.js:75 stores parseInt(item.token.decimals || "18", 10) onto state.wallets[].addresses[].tokenBalances[].decimals — which is the third link resolveTokenDecimals() reads. Where the explorer reports no decimals (a token whose decimals() reverts, which a hostile contract chooses), a fabricated 18 is stored, is indistinguishable at read time from a real 18, and this PR's chain returns it as an authoritative scale: the swap line then formats at a guessed 18 with no refusal, which is the defect #340 exists to remove, reached one layer down. Reachable only for a token that is neither bundled nor tracked and clears the holder gate (at least 1,000 holders). Not introduced here, outside this diff, and it equally defeats the ERC-20 guarantee already merged for #306 — so it wants its own issue rather than rework of this PR. But the PR body's "the decimals the block explorer already reported" is not accurate in that case.

4. Accepted-decimals bound exceeds what formatUnits() can render. MAX_DECIMALS is 255, ethers formatUnits() throws above roughly 87 (invalid FixedNumber decimals (too large)). An explorer-reported 90..255 therefore throws inside uniswap.decode(), is swallowed by its blanket catch { return null }, and the entire swap becomes undecodable — the screen loses the decode rather than stating a wrong number, so it fails safe, but silently. Verified pre-existing on the ERC-20 path too (same input yields a null decode there); now inherited by the swap path. Follow-up issue, not rework.

Verified and clean: no surviving guess in the new code (decimals === null is strict, so a real scale of 0 renders 1000000000.0000 from tracked, "0" and numeric 0 alike — no falsy collapse); hostile explorer values 256, -1, "abc", null, undefined, [], true, "1e6", " 6", 6.5 all refuse, disagreeing values refuse, and neither tracked nor explorer can override the bundled list; the refusal reaches rawValue and the wait/success/error screens render it verbatim without reformatting; sub-floor plus unknown scale yields one coherent rendering (1 base units (decimals unknown)), composing correctly with #339; no new network call and no new data source; src/shared/approvalAmount.js is comment-only and src/shared/amountDisplay.js untouched; #346 left alone; one commit ending (closes #340), base next, mergeable, no attribution trailers or vendor references; make check green in a clean clone with the lint layer executing rather than replaying cache (#11 [lint 1/1] RUN make lint … DONE 4.9s, with eslint and prettier output).

FAIL — `needs-checks`. The change itself is sound; the head commit's CI is red for an unrelated reason. **1. CI red on `a658fde` — `e2e / e2e-chrome` failed (not this diff).** Failing cases are 28, 31, 45, 53, 54, and the cause is a `sepolia` network selection leaking across the suite: `the network selector shows "sepolia" after restoring and reopening the popup, expected "mainnet"` and `eth_chainId did not round trip through the extension: "0xaa36a7"`. Nothing in this diff touches network selection or `eth_chainId`, and the suite has no swap coverage at all. `make test-e2e` on this exact head in a fresh clone passes 55/55, and `e2e-chrome` was green on the base `669c443`. Needs a CI re-run, not a code change. **2. Fail-first evidence is miscounted (PR body).** `tests/uniswapUnknownDecimals.test.js` contains 9 tests, not 8. Reverting `src/` to `next` reproduces the stated result exactly — `7 failed, 850 passed, 857 total`, every failure `Received: "0.000000001"` — which means **two** tests pass both before and after: `a bundled token on the other side still formats` and `an unbounded permit is still named, with or without a scale`. The body names only the first. Correct the count and disclose both, so the fail-first claim covers the file it describes. **3. The explorer link can return an 18 no explorer reported.** `src/shared/balances.js:75` stores `parseInt(item.token.decimals || "18", 10)` onto `state.wallets[].addresses[].tokenBalances[].decimals` — which is the third link `resolveTokenDecimals()` reads. Where the explorer reports no decimals (a token whose `decimals()` reverts, which a hostile contract chooses), a fabricated 18 is stored, is indistinguishable at read time from a real 18, and this PR's chain returns it as an authoritative scale: the swap line then formats at a guessed 18 with no refusal, which is the defect https://git.eeqj.de/sneak/AutistMask/issues/340 exists to remove, reached one layer down. Reachable only for a token that is neither bundled nor tracked and clears the holder gate (at least 1,000 holders). Not introduced here, outside this diff, and it equally defeats the ERC-20 guarantee already merged for https://git.eeqj.de/sneak/AutistMask/issues/306 — so it wants its own issue rather than rework of this PR. But the PR body's "the decimals the block explorer already reported" is not accurate in that case. **4. Accepted-decimals bound exceeds what `formatUnits()` can render.** `MAX_DECIMALS` is 255, ethers `formatUnits()` throws above roughly 87 (`invalid FixedNumber decimals (too large)`). An explorer-reported 90..255 therefore throws inside `uniswap.decode()`, is swallowed by its blanket `catch { return null }`, and the entire swap becomes undecodable — the screen loses the decode rather than stating a wrong number, so it fails safe, but silently. Verified pre-existing on the ERC-20 path too (same input yields a null decode there); now inherited by the swap path. Follow-up issue, not rework. Verified and clean: no surviving guess in the new code (`decimals === null` is strict, so a real scale of 0 renders `1000000000.0000` from tracked, `"0"` and numeric `0` alike — no falsy collapse); hostile explorer values 256, -1, `"abc"`, `null`, `undefined`, `[]`, `true`, `"1e6"`, `" 6"`, `6.5` all refuse, disagreeing values refuse, and neither tracked nor explorer can override the bundled list; the refusal reaches `rawValue` and the wait/success/error screens render it verbatim without reformatting; sub-floor plus unknown scale yields one coherent rendering (`1 base units (decimals unknown)`), composing correctly with https://git.eeqj.de/sneak/AutistMask/pulls/339; no new network call and no new data source; `src/shared/approvalAmount.js` is comment-only and `src/shared/amountDisplay.js` untouched; https://git.eeqj.de/sneak/AutistMask/issues/346 left alone; one commit ending ` (closes #340)`, base `next`, mergeable, no attribution trailers or vendor references; `make check` green in a clean clone with the lint layer executing rather than replaying cache (`#11 [lint 1/1] RUN make lint` … `DONE 4.9s`, with eslint and prettier output).

clawbot referenced this pull request 2026-08-23 16:04:25 +02:00

fix: balances.js fabricates decimals 18 before storage, laundering a guess into both approval paths #349

clawbot added needs-checks and removed needs-review labels 2026-08-23 16:04:29 +02:00

clawbot referenced this pull request 2026-08-23 16:04:49 +02:00

harden: a token scale above ~87 makes the whole swap silently undecodable #350

clawbot force-pushed harden/340-uniswap-unknown-decimals from a658fde62d to 958e92d753 2026-08-23 16:16:33 +02:00 Compare

clawbot merged commit 43784cab3f into next 2026-08-23 16:20:23 +02:00

clawbot deleted branch harden/340-uniswap-unknown-decimals 2026-08-23 16:20:23 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

merge-ready

needs-checks

needs-rebase

needs-review

needs-rework

No Label needs-checks

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

clawbot sneak

No Assignees clawbot

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#345