diff --git a/README.md b/README.md index 5f5bd87..9a9ef4a 100644 --- a/README.md +++ b/README.md @@ -1251,8 +1251,14 @@ A V4 step with no swap action sets only the sides its `SETTLE`, `SETTLE_ALL`, `TAKE` or `TAKE_ALL` names. An `UNWRAP_WETH` step makes `Token Out` ETH only when the output side is WETH, -on mainnet or Sepolia, or when no step set the output side; a WETH -`Min. received` figure then reads in ETH. Otherwise `Token Out` and +on mainnet or Sepolia, that the router holds when the step runs (Transaction +Decoding below gives what the router holds), or when no step set the output +side; a WETH `Min. received` figure then reads in ETH. A later step that sets +the output side sets it anew. The router unwraps only the WETH it holds, so a +swap that pays its WETH to the user and then unwraps shows WETH. The decoder +follows what the router holds by token, not by step: a swap that pays part of +its WETH to the user and part to the router, and then unwraps, shows ETH with +the minimum of the step that set the output side. Otherwise `Token Out` and `Min. received` keep the output side's own token and figure, whether the swap was paid in ETH or in a token: a V2 exact-out swap that buys USDC and then unwraps the WETH it did not spend shows USDC. The token permission warning on diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 3f9177d..e29755f 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -1219,6 +1219,10 @@ function decode(data, toAddress, chainId, sources, fromAddress) { let minOutput = null; let outputEstablished = false; let hasUnwrapWeth = false; + // Set when an UNWRAP_WETH pays out, as ETH, the output side's WETH, + // which the router holds; cleared whenever the output side is set + // again. + let outputUnwrapped = false; let hasV2OrV3ExactOut = false; let swapUndecoded = false; let hasSubPlan = false; @@ -1260,6 +1264,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) { outputToken = present(token) ? token : null; minOutput = present(amount) ? amount : null; outputEstablished = true; + outputUnwrapped = false; }; // What the steps take from the sender beyond the amount of the step @@ -1744,6 +1749,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) { outputToken = v4.tokenOut; minOutput = v4.amountOutMin; outputEstablished = true; + outputUnwrapped = false; } else if (v4) { // A step with no swap action sets only the sides its // SETTLE, SETTLE_ALL, TAKE or TAKE_ALL names: @@ -1772,6 +1778,17 @@ function decode(data, toAddress, chainId, sources, fromAddress) { } } else if (command === "UNWRAP_WETH") { hasUnwrapWeth = true; + // The router unwraps all the WETH it holds, the output + // side's included when it is WETH the router holds, even + // when the recipient cannot be read. WETH a step paid to + // anyone else is not unwrapped. + if ( + present(outputToken) && + WETH_ADDRESSES.includes(outputToken.toLowerCase()) && + heldByRouter.has(outputToken.toLowerCase()) + ) { + outputUnwrapped = true; + } const u = decodeUnwrapWeth(inputs[i]); if (u) { addPayee(u.recipient); @@ -1912,23 +1929,22 @@ function decode(data, toAddress, chainId, sources, fromAddress) { outputToken = null; minOutput = null; outputEstablished = true; + outputUnwrapped = false; } // Resolve token info. A null token on either side means no step named // a currency for it, a V4 step with a swap action left it unknown, or // an EXECUTE_SUB_PLAN step did; tokenInfo() refuses rather than calling // it ETH. UNWRAP_WETH turns WETH into ETH, so it makes the output ETH - // when the output side is WETH, or when no step set the output side. A - // V4 step with a swap action, or an EXECUTE_SUB_PLAN step, did set it, - // even when it set it unknown. Any other output keeps its own token - // and figure: a swap that buys USDC and then unwraps the WETH it did - // not spend receives USDC. - const outputIsWeth = - present(outputToken) && - WETH_ADDRESSES.includes(outputToken.toLowerCase()); + // when it unwraps the output side's WETH (`outputUnwrapped`), or when + // no step set the output side. A V4 step with a swap action, or + // an EXECUTE_SUB_PLAN step, did set it, even when it set it unknown. + // Any other output keeps its own token and figure: a swap that pays + // its WETH to the user and then unwraps receives WETH, and one that + // buys USDC and then unwraps the WETH it did not spend receives USDC. const inInfo = tokenInfo(inputToken, sources); const outInfo = - hasUnwrapWeth && (outputIsWeth || !outputEstablished) + outputUnwrapped || (hasUnwrapWeth && !outputEstablished) ? { symbol: "ETH", decimals: 18, address: null } : tokenInfo(outputToken, sources); diff --git a/tests/uniswap.test.js b/tests/uniswap.test.js index 277201f..9144d6a 100644 --- a/tests/uniswap.test.js +++ b/tests/uniswap.test.js @@ -875,7 +875,7 @@ describe("uniswap decoder", () => { const data = buildExecute( solidityPacked(["uint8", "uint8"], [0x08, 0x0c]), [ - encodeV2SwapExactIn(USER_ADDR, 1000000n, 500000000000000n, [ + encodeV2SwapExactIn(ROUTER_ADDR, 1000000n, 500000000000000n, [ USDC_ADDR, SEPOLIA_WETH_ADDR, ]), @@ -958,7 +958,7 @@ describe("uniswap decoder", () => { const data = buildExecute( solidityPacked(["uint8", "uint8"], [0x08, 0x0c]), [ - encodeV2SwapExactIn(USER_ADDR, 1000000n, 500000000000000n, [ + encodeV2SwapExactIn(ROUTER_ADDR, 1000000n, 500000000000000n, [ USDT_ADDR, WETH_ADDR, ]), diff --git a/tests/uniswapRecipient.test.js b/tests/uniswapRecipient.test.js index fc4c786..fdedc12 100644 --- a/tests/uniswapRecipient.test.js +++ b/tests/uniswapRecipient.test.js @@ -7,9 +7,11 @@ // (https://git.eeqj.de/sneak/AutistMask/issues/592), the address whose // balance a BALANCE_CHECK_ERC20 checks // (https://git.eeqj.de/sneak/AutistMask/issues/600), any token a swap -// leaves in the router (https://git.eeqj.de/sneak/AutistMask/issues/606), and +// leaves in the router (https://git.eeqj.de/sneak/AutistMask/issues/606), // what each PERMIT2_TRANSFER_FROM moves -// (https://git.eeqj.de/sneak/AutistMask/issues/611). +// (https://git.eeqj.de/sneak/AutistMask/issues/611), and when an +// UNWRAP_WETH makes the output ETH +// (https://git.eeqj.de/sneak/AutistMask/issues/607). // // Every swap command, SWEEP, WRAP_ETH, UNWRAP_WETH, TRANSFER, PAY_PORTION, // PAY_PORTION_FULL_PRECISION, PERMIT2_TRANSFER_FROM, ACROSS_V4_DEPOSIT_V3 and @@ -531,7 +533,7 @@ describe("an output left in the router is not received", () => { { kind: "an UNWRAP_WETH to the user, then the swap", data: execute(unwrapWeth(MSG_SENDER), toRouter), - min: "1.0000 ETH", + min: "1.0000 WETH", }, { kind: "the swap, then a WRAP_ETH of zero to the user", @@ -568,6 +570,64 @@ describe("an output left in the router is not received", () => { }); }); +// https://git.eeqj.de/sneak/AutistMask/issues/607: an UNWRAP_WETH pays out, +// as ETH, the WETH the router holds when it runs. Token Out read ETH whenever +// the output side was WETH, so a swap that paid its WETH to the user before +// the UNWRAP_WETH read as ETH, though the user receives WETH. +describe("an UNWRAP_WETH makes the output ETH only when the router holds it", () => { + test("a swap paying its WETH to the user, then an UNWRAP_WETH, reads as WETH", () => { + const result = decode( + execute(swapToWeth(MSG_SENDER), unwrapWeth(MSG_SENDER)), + ); + expect(result.name).toBe("Swap USDT → WETH"); + expect(labels(result)).toEqual(PAID_TO_USER); + expect(detail(result, "Min. received").value).toBe("1.0000 WETH"); + }); + + test("a swap paying its WETH to the router, then an UNWRAP_WETH to the user, reads as ETH", () => { + const result = decode( + execute(swapToWeth(ADDRESS_THIS), unwrapWeth(MSG_SENDER)), + ); + expect(result.name).toBe("Swap USDT → ETH"); + expect(labels(result)).toEqual(PAID_TO_USER); + expect(detail(result, "Min. received").value).toBe("1.0000 ETH"); + }); + + // The user receives the first step's WETH as WETH and the second's as + // ETH. The second step sets the output side, and the router holds its + // WETH, so the output reads as ETH with the second step's minimum. + test("a swap paying part of its WETH to the user and the rest to the router, then an UNWRAP_WETH, reads as ETH", () => { + const result = decode( + execute( + swapToWeth(MSG_SENDER), + v2Swap(0x08, ADDRESS_THIS, USDC_1990, ONE_WETH / 2n, [ + USDC, + WETH, + ]), + unwrapWeth(MSG_SENDER), + ), + ); + expect(result.name).toBe("Swap USDT → ETH"); + expect(detail(result, "Min. received").value).toBe("0.5000 ETH"); + }); + + // A step after the UNWRAP_WETH that sets the output side sets it anew. + test("a swap paying WETH to the user after an UNWRAP_WETH of an earlier swap's WETH reads as WETH", () => { + const result = decode( + execute( + swapToWeth(ADDRESS_THIS), + unwrapWeth(MSG_SENDER), + v2Swap(0x08, MSG_SENDER, USDC_1990, ONE_WETH / 2n, [ + USDC, + WETH, + ]), + ), + ); + expect(result.name).toBe("Swap USDT → WETH"); + expect(detail(result, "Min. received").value).toBe("0.5000 WETH"); + }); +}); + // https://git.eeqj.de/sneak/AutistMask/issues/606: any token a step pays the // router stays there unless a later step pays out or spends all of it, not // only the swap's output. A V4 SETTLE can take more of the user's tokens than