diff --git a/README.md b/README.md index 63e2994..e20ba2b 100644 --- a/README.md +++ b/README.md @@ -1277,22 +1277,18 @@ see Transaction Decoding), do not always read as a number. They can read: step pays nothing itself and swaps whatever an earlier step sent to the pair, so the calldata states no quantity. A V3 exact-in `amountIn` of zero has no such meaning and is shown as a zero. -- `None (no minimum guaranteed)`: the swap's `Min. received` line, when the - minimum it shows is zero, whether a V2, V3 or V4 swap's minimum or a - `BALANCE_CHECK_ERC20` step's `minBalance`. Before - [#359](https://git.eeqj.de/sneak/AutistMask/issues/359), a zero `minBalance` - read `0.0000` when the token's scale was known. The router passes a balance - check whenever the balance is at least `minBalance`, so a zero `minBalance` - guarantees nothing: it sets `Token Out` and `Min. received` only when the - output side holds no minimum, not even a zero one, at the point the check is - reached, and otherwise leaves the current token and figure in place. A nonzero - `minBalance` sets both lines, as a swap step does. Only a check of the - transaction's sender's own balance sets either line; a check of any other - address sets neither (see Transaction Decoding). A `BALANCE_CHECK_ERC20` step - that the router lets fail without reverting the transaction (see Transaction - Decoding) guarantees nothing either, whatever its `minBalance`, and is read as - one whose `minBalance` is zero. A minimum stated by a V4 swap step the router - lets fail is likewise read as zero. +- `None (no minimum guaranteed)`: the swap's `Min. received` line, when it shows + a V2, V3 or V4 swap's minimum of zero. The router passes a + `BALANCE_CHECK_ERC20` step whenever the balance is at least `minBalance`, so a + zero `minBalance` guarantees nothing. A check with a zero `minBalance` sets + neither `Token Out` nor `Min. received`. A nonzero `minBalance` sets both + lines, as a swap step does. Only a check of the transaction's sender's own + balance sets either line; a check of any other address sets neither (see + Transaction Decoding). A `BALANCE_CHECK_ERC20` step that the router lets fail + without reverting the transaction (see Transaction Decoding) guarantees + nothing either, whatever its `minBalance`, and is read as one whose + `minBalance` is zero. A minimum stated by a V4 swap step the router lets fail + is likewise read as zero. The swap's `Token In` and `Token Out` lines name a currency, not an amount; each reads `Unknown (not named in the calldata)` when the decoder found no token for @@ -3397,7 +3393,8 @@ Currently supported: `Balance Check (of another address)`. A check whose input does not decode, such as one whose owner word has a nonzero top byte (the router reads the owner from the word's last 20 bytes), likewise sets neither, and it is listed - as `Balance Check (could not be decoded)`. When a swap step, `TAKE` or + as `Balance Check (could not be decoded)`. A check with a zero `minBalance` + sets neither `Token Out` nor the minimum. When a swap step, `TAKE` or `TAKE_PORTION` pays the router any token, the swap's output or another, a `WRAP_ETH` pays it WETH, or an `UNWRAP_WETH` into the router turns WETH a step paid it into ETH, that token stays in the router unless a later step pays out diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 5e98da6..6e140b7 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -1629,13 +1629,13 @@ function decode(data, toAddress, chainId, sources, fromAddress) { } else { // The router passes this check whenever the owner // holds at least minBalance, so a zero one guarantees - // nothing and does not replace a minimum an earlier - // step stated. A check the router lets fail guarantees - // nothing either, so it is read as a zero one. Any + // nothing. A check the router lets fail guarantees + // nothing either, so it is read as a zero one. A zero + // check sets neither Token Out nor the minimum. Any // other minBalance sets the output side as a swap - // does. It pays nobody. + // does. The check pays nobody. const minBalance = allowedToFail ? 0n : b.minBalance; - if (!(minBalance === 0n && present(minOutput))) { + if (minBalance !== 0n) { setOutput(i, b.token, minBalance, []); } keepsRouterWeth = true; diff --git a/tests/uniswap.test.js b/tests/uniswap.test.js index 736c724..511f3ca 100644 --- a/tests/uniswap.test.js +++ b/tests/uniswap.test.js @@ -4380,10 +4380,9 @@ describe("uniswap decoder", () => { expect(detail(result, "Min. received").value).toBe("2.0000 USDC"); }); - // With no minimum stated before it, a zero balance check is what sets the - // output side, and it guarantees nothing. No step pays anyone, so the - // minimum is not said to be received. - test("a zero balance check with no earlier minimum states no minimum", () => { + // A zero balance check sets neither Token Out nor the minimum. The route + // pays only the WRAP_ETH's WETH, to the router, where it stays. + test("a zero balance check after a WRAP_ETH into the router names no output token and no minimum", () => { const data = buildExecute( solidityPacked(["uint8", "uint8"], [0x0b, 0x0e]), [ @@ -4396,9 +4395,16 @@ describe("uniswap decoder", () => { const result = uniswap.decode(data, ROUTER_ADDR, "0x1", {}, USER_ADDR); expect(result).not.toBeNull(); - expect(detail(result, "Token Out").value).toContain("USDT"); - expect(detail(result, "Min. paid out").value).toBe( - "None (no minimum guaranteed)", + expect(detail(result, "Token Out").value).toBe( + "Unknown (not named in the calldata)", + ); + expect(detail(result, "Min. paid out")).toBeUndefined(); + expect(detail(result, "Min. received")).toBeUndefined(); + expect(detail(result, "Paid to").value).toBe( + "The router, where it stays (no later step pays it out)", + ); + expect(detail(result, "Token moved into the router").address).toBe( + WETH_ADDR, ); }); diff --git a/tests/uniswapRecipient.test.js b/tests/uniswapRecipient.test.js index 4e9b9e6..548ee97 100644 --- a/tests/uniswapRecipient.test.js +++ b/tests/uniswapRecipient.test.js @@ -688,6 +688,62 @@ describe("an UNWRAP_WETH makes the output ETH only when the router holds it", () ]); }); + // The same route, then a balance check of the sender's USDC with a zero + // minimum. The check guarantees nothing, so Token Out stays ETH + // (https://git.eeqj.de/sneak/AutistMask/issues/649). + test("the same route, then a zero balance check of USDC, still reads as ETH with no minimum", () => { + const result = decode( + execute( + v4Swap( + SWAP_USDT_FOR_WETH, + SETTLE_USDT, + take(WETH, MSG_SENDER, (ONE_WETH * 6n) / 10n), + take(WETH, ADDRESS_THIS), + ), + unwrapWeth(MSG_SENDER), + balanceCheck(USDC, 0n), + ), + ); + expect(result.name).toBe("Swap USDT → ETH"); + expect(detail(result, "Token Out").value).toBe("ETH"); + expect(labels(result)).toEqual([ + "Protocol", + "Token In", + "Amount", + "Token Out", + "Steps", + "Deadline", + ]); + }); + + // A PERMIT2_TRANSFER_FROM moves the sender's WETH into the router. An + // UNWRAP_WETH pays that WETH to the user as ETH. A zero balance check of + // USDC sets neither Token Out nor the minimum, before the UNWRAP_WETH or + // after it (https://git.eeqj.de/sneak/AutistMask/issues/649). + test.each([ + { + kind: "before the UNWRAP_WETH", + steps: [balanceCheck(USDC, 0n), unwrapWeth(MSG_SENDER)], + }, + { + kind: "after the UNWRAP_WETH", + steps: [unwrapWeth(MSG_SENDER), balanceCheck(USDC, 0n)], + }, + ])( + "WETH moved into the router and unwrapped, with a zero balance check of USDC $kind, reads as ETH with no minimum", + ({ steps }) => { + const result = decode( + execute( + permit2TransferFrom(WETH, ADDRESS_THIS, ONE_WETH), + ...steps, + ), + ); + expect(detail(result, "Token Out").value).toBe("ETH"); + expect(detail(result, "Min. received")).toBeUndefined(); + expect(detail(result, "Min. paid out")).toBeUndefined(); + }, + ); + // A TAKE the decoder cannot read may pay part of the WETH to the user. test("one V4 step paying WETH to the router beside a TAKE that cannot be read, then an UNWRAP_WETH, reads as ETH with no minimum", () => { const result = decode( @@ -2820,17 +2876,16 @@ describe("the allow-revert flag", () => { }, ); - test("a BALANCE_CHECK_ERC20 with the flag, and no minimum before it, states none", () => { + test("a BALANCE_CHECK_ERC20 with the flag, and no minimum before it, names no output token and no minimum", () => { const check = balanceCheck(WETH, ONE_WETH); expect(detail(decode(execute(check)), "Min. paid out").value).toBe( "1.0000 WETH", ); const withFlag = decode(execute(flagged(check))); - expect(detail(withFlag, "Token Out").address).toBe(WETH); - expect(detail(withFlag, "Min. paid out").value).toBe( - "None (no minimum guaranteed)", - ); + expect(detail(withFlag, "Token Out").value).toBe(UNNAMED); + expect(detail(withFlag, "Min. paid out")).toBeUndefined(); + expect(detail(withFlag, "Min. received")).toBeUndefined(); }); // Only the routers from UniversalRouterV2 on let a PERMIT2_PERMIT fail.