diff --git a/README.md b/README.md index be4e0a6..aef395d 100644 --- a/README.md +++ b/README.md @@ -926,7 +926,10 @@ rule: the ERC-20 `transfer`/`approve` line (`src/popup/views/approval.js`) and the swap's `Amount` and `Min. received` lines (`src/shared/uniswap.js`). The token permission warning on the signature screen takes the same rule for its amounts. An unbounded allowance or permit needs no scale to describe and is -still shown as `Unlimited`. +still shown as `Unlimited`. A source's answer counts only if it is a whole +number from 0 to 80: `decimals()` returns a `uint8`, but `formatUnits()` cannot +format more than 80 decimal places, so a token that reports 81 to 255 is shown +as one whose scale nothing knows. The rule holds only if nothing invents a scale UPSTREAM of it. Those three sources are read as authoritative, so a value written into one of them cannot be diff --git a/TODO.md b/TODO.md index 368785b..80729d2 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,15 @@ but the review is broader than any of them. # Completed Steps +- 2026-10-04: A token that reports more than 80 decimal places has no known + scale ([#350](https://git.eeqj.de/sneak/AutistMask/issues/350)). The shared + scale check `toDecimals()` accepted any `uint8`, but `formatUnits()` throws + above 80, so such a token left a swap or an ERC-20 call on the approval screen + undecoded, with nothing saying why. The check now stops at 80, and both + approval paths show the base-unit amount with the scale stated as unknown. The + balance list and the history list use the same check, so the same token no + longer stops an address's token balances from refreshing or its history from + loading. - 2026-10-04: Debug mode no longer writes RPC API keys to the console ([#410](https://git.eeqj.de/sneak/AutistMask/issues/410)). `debugFetch` logged every request's full URL and body, so an RPC endpoint with a key in its path diff --git a/src/shared/approvalAmount.js b/src/shared/approvalAmount.js index 53ab436..82991fe 100644 --- a/src/shared/approvalAmount.js +++ b/src/shared/approvalAmount.js @@ -23,11 +23,11 @@ // disputed is refused rather than guessed at. // Solidity's decimals() is a uint8, and every source here is ultimately -// reporting that call's result. toDecimals() is that check, shared with the -// send path rather than copied: the bundled list stores numbers, the -// explorer's copy arrives as a string, and a token the user added by hand -// carries whatever lookupTokenInfo() got back, so the accepted types are -// enumerated rather than coerced. +// reporting that call's result. toDecimals() is that check, stopping at the 80 +// places formatUnits() accepts, and shared with the send path rather than +// copied: the bundled list stores numbers, the explorer's copy arrives as a +// string, and a token the user added by hand carries whatever lookupTokenInfo() +// got back, so the accepted types are enumerated rather than coerced. const { toDecimals } = require("./transferAmount"); const { TOKEN_BY_ADDRESS } = require("./tokenList"); const { isSpoofedSymbol } = require("./symbolSpoof"); diff --git a/src/shared/transferAmount.js b/src/shared/transferAmount.js index 1ddcf70..49cb318 100644 --- a/src/shared/transferAmount.js +++ b/src/shared/transferAmount.js @@ -27,9 +27,11 @@ const { parseUnits } = require("ethers"); -// Solidity's decimals() returns a uint8, so anything outside that range is not -// an answer this wallet can use. -const MAX_DECIMALS = 255; +// Solidity's decimals() returns a uint8, but ethers' formatUnits() and +// parseUnits() refuse more than 80 decimal places ("invalid FixedNumber +// decimals (too large)"). A scale of 81 to 255 can be neither displayed nor +// encoded, so it is not an answer this wallet can use, the same as no answer. +const MAX_DECIMALS = 80; const UNKNOWN_DISPLAYED_DECIMALS_MESSAGE = "The transfer was not sent, because the number of decimal places this" + @@ -55,7 +57,7 @@ function mismatchMessage(displayed, onChain) { // A decimals value from any source as a number, or null if it is not one. // decimals() comes back from ethers as a bigint and the explorer's copy arrives // as a string, so both of those are accepted alongside a plain number; anything -// fractional, negative, out of uint8 range, or of any other type at all is not. +// fractional, negative, above MAX_DECIMALS, or of any other type at all is not. // // The types are enumerated rather than coerced because Number() is far too // willing: Number([]) is 0 and Number(true) is 1, so a coercing check would diff --git a/tests/approvalAmount.test.js b/tests/approvalAmount.test.js index 86be059..8324c23 100644 --- a/tests/approvalAmount.test.js +++ b/tests/approvalAmount.test.js @@ -184,6 +184,22 @@ describe("decodeCalldata amount", () => { expect(line).not.toMatch(/0\.0000/); }); + // A token added by hand carries whatever its decimals() returned, and a + // uint8 reaches 255, but formatUnits() throws above 80. The throw left the + // call undecoded rather than refused + // (https://git.eeqj.de/sneak/AutistMask/issues/350). + test("a token reporting more than 80 decimals shows base units", () => { + state.trackedTokens = [ + { address: NOVEL_TOKEN, symbol: "NOVEL", decimals: 81 }, + ]; + expect( + amountLine(transferData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN), + ).toBe("5000000000 base units (decimals unknown)"); + expect(amountLine(approveData(FIVE_THOUSAND_AT_SIX), NOVEL_TOKEN)).toBe( + "5000000000 base units (decimals unknown)", + ); + }); + test("an unbounded allowance is still named, with or without a scale", () => { expect(amountLine(approveData(MAX_UINT256), NOVEL_TOKEN)).toBe( "Unlimited", diff --git a/tests/transferAmount.test.js b/tests/transferAmount.test.js index dffdfc4..16bce0e 100644 --- a/tests/transferAmount.test.js +++ b/tests/transferAmount.test.js @@ -4,7 +4,7 @@ // contract at signing time, with nothing comparing the two, so a token whose // on-chain scale differed signed an amount that was never displayed. -const { parseUnits } = require("ethers"); +const { formatUnits, parseUnits } = require("ethers"); const { displayedDecimals, transferAmountUnits, @@ -25,7 +25,17 @@ describe("displayedDecimals", () => { expect(displayedDecimals(MAX_DECIMALS)).toBe(MAX_DECIMALS); }); - test("refuses anything that is not a uint8", () => { + // decimals() is a uint8, but formatUnits() and parseUnits() stop at 80 + // places, so a larger scale cannot be shown or encoded + // (https://git.eeqj.de/sneak/AutistMask/issues/350). + test("accepts exactly the scales the formatter accepts", () => { + expect(() => formatUnits(1n, MAX_DECIMALS)).not.toThrow(); + expect(() => parseUnits("1", MAX_DECIMALS)).not.toThrow(); + expect(() => formatUnits(1n, MAX_DECIMALS + 1)).toThrow(); + expect(() => parseUnits("1", MAX_DECIMALS + 1)).toThrow(); + }); + + test("refuses anything that is not a uint8 the formatter accepts", () => { for (const bad of [ null, undefined, diff --git a/tests/uniswapUnknownDecimals.test.js b/tests/uniswapUnknownDecimals.test.js index c8ecc57..86fa427 100644 --- a/tests/uniswapUnknownDecimals.test.js +++ b/tests/uniswapUnknownDecimals.test.js @@ -126,6 +126,19 @@ describe("a swap of a token outside the bundled list", () => { test("a bundled token on the other side still formats", () => { expect(swapDetail(data(), "Min. received").value).toBe("0.5000 WETH"); }); + + // A token added by hand carries whatever its decimals() returned, and a + // uint8 reaches 255, but formatUnits() throws above 80. The throw left the + // whole swap undecoded rather than refused + // (https://git.eeqj.de/sneak/AutistMask/issues/350). + test("refuses to format when the token reports more than 80 decimals", () => { + state.trackedTokens = [ + { address: NOVEL, symbol: "NOVEL", decimals: 81 }, + ]; + expect(swapDetail(data(), "Amount").value).toBe( + "1000000000 base units (decimals unknown)", + ); + }); }); describe("the Min. received line takes the same rule", () => {