diff --git a/TODO.md b/TODO.md index c6c4b31..2409815 100644 --- a/TODO.md +++ b/TODO.md @@ -45,6 +45,18 @@ but the review is broader than any of them. # Completed Steps +- 2026-08-23: A swap whose output token the calldata never named is said to be + unknown instead of being called ETH + ([#353](https://git.eeqj.de/sneak/AutistMask/issues/353)). `tokenInfo(null)` + answers `{symbol: "ETH", decimals: 18}`, and a V4 step could take the + `Min. received` figure while naming no output currency, so the approval screen + stated the wrong asset at the wrong scale. Null is not how V4 spells native + ETH: v4-core's `type Currency is address` wraps `address(0)` for it, which + reaches the decoder as the truthy string + `0x0000000000000000000000000000000000000000` and is named ETH there already. + `Token Out` now reads `Unknown (not named in the calldata)` and + `Min. received` falls to the base-unit refusal from + [#340](https://git.eeqj.de/sneak/AutistMask/issues/340). - 2026-08-23: The background no longer reads or writes the shared `state` singleton ([#324](https://git.eeqj.de/sneak/AutistMask/issues/324)), which also closes the cold-worker wrong-chain send diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 59be572..9f19a72 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -429,8 +429,16 @@ function decode(data, toAddress, sources) { if (!inputToken && v4.tokenIn) inputToken = v4.tokenIn; if (!inputAmount && v4.amountIn) inputAmount = v4.amountIn; - // Always update output: last swap step wins - if (v4.tokenOut) outputToken = v4.tokenOut; + // Always update output: last swap step wins. A step + // that carries the Min. received figure but decoded no + // output currency makes the output *undetermined* — it + // is neither ETH nor whatever an earlier step named, + // and that figure is no longer counted in that token. + if (v4.tokenOut) { + outputToken = v4.tokenOut; + } else if (v4.amountOutMin) { + outputToken = null; + } if (v4.amountOutMin) minOutput = v4.amountOutMin; } } @@ -443,11 +451,26 @@ function decode(data, toAddress, sources) { } } - // Resolve token info + // Resolve token info. + // + // A null `outputToken` means undetermined, not native ETH, so it is + // not handed to tokenInfo() — which maps null to ETH at 18 decimals + // for the input side's benefit. Uniswap V4 spells native ETH as + // `Currency.wrap(address(0))` (v4-core `type Currency is address`), + // and a Currency is ABI-encoded as a plain address word, so every + // decode site here gets back the truthy string + // "0x0000000000000000000000000000000000000000" for it — never null. + // tokenInfo() already names that ETH, and an UNWRAP_WETH output is + // caught above, so nothing that genuinely outputs ETH arrives null. + // Only a step whose output currency did not decode does, and naming + // that ETH states the wrong asset and formats Min. received at the + // wrong scale. const inInfo = tokenInfo(inputToken, sources); const outInfo = hasUnwrapWeth ? { symbol: "ETH", decimals: 18, address: null } - : tokenInfo(outputToken, sources); + : outputToken + ? tokenInfo(outputToken, sources) + : { symbol: null, decimals: null, address: null }; const inSymbol = inInfo.symbol; const outSymbol = outInfo.symbol; @@ -513,6 +536,16 @@ function decode(data, toAddress, sources) { }); } else if (outSymbol) { details.push({ label: "Token Out", value: outSymbol }); + } else { + // Nothing established the output token, so the line says that + // rather than going missing or naming a token by default. It reads + // as a refusal, the same stance unknownDecimalsAmount() takes on a + // scale, so a Min. received figure below it is never attached to a + // token the calldata did not state. + details.push({ + label: "Token Out", + value: "Unknown (not named in the calldata)", + }); } if (minOutput !== null && minOutput !== undefined) { diff --git a/tests/uniswapUndeterminedTokenOut.test.js b/tests/uniswapUndeterminedTokenOut.test.js new file mode 100644 index 0000000..6af16c6 --- /dev/null +++ b/tests/uniswapUndeterminedTokenOut.test.js @@ -0,0 +1,211 @@ +// What the dApp approval screen says the output token of a V4 swap is when the +// calldata did not name one. +// +// Issue #353: `src/shared/uniswap.js` took a V4 step's `amountOutMin` while +// leaving `outputToken` null, and `tokenInfo(null)` answers +// `{symbol: "ETH", decimals: 18}`. An undetermined output was therefore stated +// to the user as ETH, with `Min. received` formatted at 18 decimals — the same +// class as https://git.eeqj.de/sneak/AutistMask/issues/340 and +// https://git.eeqj.de/sneak/AutistMask/issues/306, but naming the wrong asset +// rather than the wrong scale. +// +// The determination this file pins: null is NOT how V4 spells native ETH. +// v4-core declares `type Currency is address` and wraps `address(0)` for +// native ETH, and a Currency is ABI-encoded as a plain address word, so a +// native-ETH currency reaches the decoder as the string +// "0x0000000000000000000000000000000000000000" — truthy, and already named +// ETH by tokenInfo(). Both cases are asserted below: the zero address stays +// ETH, and null refuses. + +const { AbiCoder, Interface, solidityPacked } = require("ethers"); +const uniswap = require("../src/shared/uniswap"); +const { unknownDecimalsAmount } = require("../src/shared/approvalAmount"); + +const ROUTER = "0x66a9893cc07d91d95644aedd05d03f95e1dba8af"; +const USER = "0x66133E8ea0f5D1d612D2502a968757D1048c214a"; +const ZERO = "0x0000000000000000000000000000000000000000"; +// Both in the bundled list: USDT at 6 decimals, WETH at 18. +const USDT = "0xdAC17F958D2ee523a2206206994597C13D831ec7"; +const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2"; + +const REFUSAL = "Unknown (not named in the calldata)"; + +// The figure whose asset is in question. At 18 decimals it renders as +// 0.000000000001; in base units it renders as itself. +const MIN_OUT = 1234567n; +const HALF_ETH = 500000000000000000n; + +const V4_SWAP_EXACT_IN_SINGLE = 0x06; +const V4_SWAP_EXACT_IN = 0x07; +const V4_SETTLE = 0x0b; +const V4_TAKE = 0x0e; + +const coder = AbiCoder.defaultAbiCoder(); +const routerIface = new Interface([ + "function execute(bytes commands, bytes[] inputs, uint256 deadline)", +]); + +function execute(commands, inputs) { + return routerIface.encodeFunctionData("execute", [ + commands, + inputs, + 9999999999n, + ]); +} + +function v4Input(actions, params) { + return coder.encode( + ["bytes", "bytes[]"], + [new Uint8Array(actions), params], + ); +} + +// IV4Router.ExactInputParams as the decoder reads it: +// (Currency currencyIn, PathKey[] path, uint128 amountIn, uint128 minOut). +// An empty path names no output currency at all. +function exactInParams(currencyIn, path, amountIn, amountOutMin) { + return coder.encode( + [ + "tuple(address,tuple(address,uint24,int24,address,bytes)[],uint128,uint128)", + ], + [[currencyIn, path, amountIn, amountOutMin]], + ); +} + +// IV4Router.ExactInputSingleParams: (PoolKey, bool zeroForOne, uint128 +// amountIn, uint128 minOut, bytes hookData). +function exactInSingleParams(currency0, currency1, zeroForOne, min) { + return coder.encode( + [ + "tuple(tuple(address,address,uint24,int24,address),bool,uint128,uint128,bytes)", + ], + [ + [ + [currency0, currency1, 500, 10, ZERO], + zeroForOne, + 1000000n, + min, + "0x", + ], + ], + ); +} + +// V3_SWAP_EXACT_IN (command 0x00) input: path is token(20) fee(3) token(20). +function v3ExactIn(tokenIn, tokenOut, amountIn, amountOutMin) { + const path = + "0x" + + tokenIn.slice(2).toLowerCase() + + "000bb8" + + tokenOut.slice(2).toLowerCase(); + return coder.encode( + ["address", "uint256", "uint256", "bytes", "bool"], + [USER, amountIn, amountOutMin, path, true], + ); +} + +function detail(data, label) { + const decoded = uniswap.decode(data, ROUTER, {}); + expect(decoded).not.toBeNull(); + return decoded.details.find((d) => d.label === label); +} + +describe("a V4 step that states a Min. received but no output currency", () => { + // SWAP_EXACT_IN with an empty path: amountOutMin decodes, no currency out + // does, and there is no TAKE to supply one. + const data = () => + execute("0x10", [ + v4Input( + [V4_SWAP_EXACT_IN], + [exactInParams(USDT, [], 5000000n, MIN_OUT)], + ), + ]); + + test("says the output token is unknown instead of naming ETH", () => { + expect(detail(data(), "Token Out").value).toBe(REFUSAL); + }); + + test("keeps a Token Out line, so the figure is never unattributed", () => { + const out = detail(data(), "Token Out"); + expect(out).toBeDefined(); + // A refusal is not a token: nothing to link to an explorer. + expect(out.address).toBeUndefined(); + expect(out.isToken).toBeUndefined(); + }); + + test("refuses to scale Min. received rather than assuming 18", () => { + expect(detail(data(), "Min. received").value).toBe( + unknownDecimalsAmount(MIN_OUT), + ); + }); + + test("does not name ETH in the swap title either", () => { + const decoded = uniswap.decode(data(), ROUTER, {}); + expect(decoded.name).toBe("Uniswap Swap"); + }); +}); + +describe("a V4 step whose Min. received supersedes an earlier step's", () => { + // V3 USDT -> WETH, then a V4 step that carries the final Min. received but + // names no currency. The figure belongs to the V4 step, so it must not be + // read against the token the V3 step named. + const data = () => + execute(solidityPacked(["uint8", "uint8"], [0x00, 0x10]), [ + v3ExactIn(USDT, WETH, 5000000n, HALF_ETH), + v4Input( + [V4_SWAP_EXACT_IN], + [exactInParams(WETH, [], HALF_ETH, MIN_OUT)], + ), + ]); + + test("does not keep naming the earlier step's output token", () => { + expect(detail(data(), "Token Out").value).toBe(REFUSAL); + }); + + test("refuses to scale the superseding figure", () => { + expect(detail(data(), "Min. received").value).toBe( + unknownDecimalsAmount(MIN_OUT), + ); + }); +}); + +describe("native ETH out, which V4 spells as the zero address", () => { + // The pin on the other half of the determination. PoolKey currencies are + // ordered, so native ETH is currency0; zeroForOne false swaps USDT in for + // ETH out. TAKE names the same zero-address currency. + const data = () => + execute("0x10", [ + v4Input( + [V4_SETTLE, V4_SWAP_EXACT_IN_SINGLE, V4_TAKE], + [ + coder.encode( + ["address", "uint256", "bool"], + [USDT, 5000000n, true], + ), + exactInSingleParams(ZERO, USDT, false, HALF_ETH), + coder.encode( + ["address", "address", "uint256"], + [ZERO, USER, 0n], + ), + ], + ), + ]); + + test("a Currency of address(0) decodes to a truthy address string", () => { + // The fact the whole determination rests on: an absent output currency + // and a native-ETH one are distinguishable here, because the ABI + // decoder never yields null for an address word. + const [currency] = coder.decode( + ["address", "address", "uint256"], + coder.encode(["address", "address", "uint256"], [ZERO, USER, 0n]), + ); + expect(currency).toBe(ZERO); + expect(Boolean(currency)).toBe(true); + }); + + test("is still named ETH and still scaled at 18 decimals", () => { + expect(detail(data(), "Token Out").value).toBe("ETH"); + expect(detail(data(), "Min. received").value).toBe("0.5000 ETH"); + expect(uniswap.decode(data(), ROUTER, {}).name).toBe("Swap USDT → ETH"); + }); +});