diff --git a/README.md b/README.md index 8ae51e1..edde902 100644 --- a/README.md +++ b/README.md @@ -3263,15 +3263,25 @@ Currently supported: of a token other than `Token In`; one of `Token In` after an amount that is not a quantity (a V4 open delta, `ALREADY_PAID`, which keep their wording on the `Amount` line, or no stated amount, as of a V4 exact-out step); and one - whose amount the decoder does not know. When the step that sets the input side - states no amount, the user pays it some of its input token, and there is no - `Amount` line, that step gets them too whenever another take's lines give a - figure, or a `PERMIT2_TRANSFER_FROM` of its token gives one, on the lines - below or, into the router, on its `Paid to` line, so that figure cannot read - as all that leaves. When it states no amount and the user pays it none of its - input token, as when the router pays it, that step takes none of that token - from the user's wallet and never gets them, with a permit or without. They - come in step order, the step that sets the input side first when it has them: + whose amount the decoder does not know. When there is no `Amount` line, each + take the total counts gets them too, so that every amount the decoder reads is + shown: the amount the step that sets the input side states, when it is a + quantity and the user pays that step (a V2 or V3 step whose `payerIsUser` is + true, a V4 step the user pays some of its input token, or a `WRAP_ETH`, whose + ETH comes from the transaction's value), and each later take of `Token In` + added to it. A step that sets the input side and is paid from the router takes + nothing from the user's wallet and gets none. A `PERMIT2_PERMIT` the decoder + cannot read, then V3 exact-ins of 600 USDT and 400 DAI paid by the user, give + lines for both amounts. When the step that sets the input side states no + amount, the user pays it some of its input token, and there is no `Amount` + line, that step gets them too whenever another take's lines give a figure, or + a `PERMIT2_TRANSFER_FROM` of its token gives one, on the lines below or, into + the router, on its `Paid to` line, so that figure cannot read as all that + leaves. When it states no amount and the user pays it none of its input token, + as when the router pays it, that step takes none of that token from the user's + wallet and never gets them, with a permit or without. They come in step order, + except that the step that sets the input side comes first when it has them, + followed, when there is no `Amount` line, by the other takes the total counts: `Moved from the sender`, its amount, worded as the `Amount` line words one, or `Unknown (could not be decoded)` when the decoder does not know it; `Token moved`, giving the token's address as the `Token In` line does; and diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index d7ed833..31d12af 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -1234,9 +1234,11 @@ function decode(data, toAddress, chainId, sources, fromAddress) { let inputToken = null; let inputAmount = null; let inputEstablished = false; - // Whether the user pays the V4 step that set the input side any of - // its input currency (see decodeV4Swap()). Only a V4 step can set the - // input side with no amount, the one case in which this is read. + // Whether the user pays the step that set the input side: a V2 or V3 + // swap step with `payerIsUser` true, a V4 step the user pays some of + // its input currency (see decodeV4Swap()), or a WRAP_ETH, whose ETH + // comes from the transaction's value. A step paid from the router + // takes nothing from the sender. let inputPaidByUser = false; // The last PERMIT2_PERMIT this decoder read before a step set the // input side, or null (see permitStands after the loop). @@ -1622,6 +1624,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) { if (s) { if (!inputEstablished) { setInputOnce(s.tokenIn, s.amountIn); + inputPaidByUser = s.payerIsUser; } else if (s.payerIsUser) { addUserPaid(s.tokenIn, s.amountIn); } @@ -1645,6 +1648,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) { if (s) { if (!inputEstablished) { setInputOnce(s.tokenIn, s.amountIn); + inputPaidByUser = s.payerIsUser; } else if (s.payerIsUser) { addUserPaid(s.tokenIn, s.amountIn); } @@ -1676,6 +1680,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) { if (s) { if (!inputEstablished) { setInputOnce(s.tokenIn, s.amountInMax); + inputPaidByUser = s.payerIsUser; } else if (s.payerIsUser) { addUserPaid(s.tokenIn, s.amountInMax); } @@ -1771,10 +1776,13 @@ function decode(data, toAddress, chainId, sources, fromAddress) { } else if (command === "WRAP_ETH") { const w = decodeWrapEth(inputs[i]); if (w) { - setInputOnce( - "0x0000000000000000000000000000000000000000", - w.amount, - ); + if (!inputEstablished) { + setInputOnce( + "0x0000000000000000000000000000000000000000", + w.amount, + ); + inputPaidByUser = true; + } addPayee(w.recipient); // CONTRACT_BALANCE wraps all the router's ETH. if (w.amount === CONTRACT_BALANCE) { @@ -2041,6 +2049,11 @@ function decode(data, toAddress, chainId, sources, fromAddress) { // (see spentUnread()), or a token it does not know. const spentFromSender = []; let total = null; + // The takes from the sender that `total` adds up, each a quantity, as + // { token, amount }. When there is no Amount line, each gets lines of + // its own (see below). The amount of a step that set the input side + // and is paid from the router counts in `total` but is not among them. + const inTotal = []; // Whether the step that set the input side counts as a take from the // sender, which may get lines of its own. The amount it stated counts // whoever pays it. Stating none, it counts only when the user pays it @@ -2053,6 +2066,9 @@ function decode(data, toAddress, chainId, sources, fromAddress) { let firstTakeUnstated = null; if (!permitStands || sameAddress(inputToken, permit.token)) { total = inputAmount; + if (typeof inputAmount === "bigint" && inputPaidByUser) { + inTotal.push({ token: inputToken, amount: inputAmount }); + } if (hasFirstTake && !present(inputAmount)) { firstTakeUnstated = { token: inputToken, amount: null }; } @@ -2071,6 +2087,7 @@ function decode(data, toAddress, chainId, sources, fromAddress) { typeof t.amount === "bigint" ) { total += t.amount; + inTotal.push(t); continue; } spentFromSender.push(t); @@ -2202,7 +2219,9 @@ function decode(data, toAddress, chainId, sources, fromAddress) { // A null token in `spentFromSender`, a step this decoder did not read // or a token it does not know, and one in `permits`, a permit or // batch it could not read, leave no Amount figure, which may then be - // too low. + // too low. With no Amount line, each take the total adds up gets + // lines of its own, first, so that every amount this decoder read is + // shown. if ( present(inputAmount) && spentFromSender.every((s) => present(s.token)) && @@ -2214,6 +2233,8 @@ function decode(data, toAddress, chainId, sources, fromAddress) { value: amount.display, rawValue: amount.raw, }); + } else { + spentFromSender.unshift(...inTotal); } // What steps take from the sender beyond the Amount line, in the shape diff --git a/tests/uniswap.test.js b/tests/uniswap.test.js index a2b2e27..2abe38e 100644 --- a/tests/uniswap.test.js +++ b/tests/uniswap.test.js @@ -1447,6 +1447,20 @@ describe("uniswap decoder", () => { USDT_ADDR, USDC_ADDR, ]); + // The lines of its 600 USDT when there is no Amount line. + // https://git.eeqj.de/sneak/AutistMask/issues/659 — on next at + // 5f2b606 no line gave the 600 USDT then. + const USDT_600_GROUP = [ + ["Moved from the sender", "600.0000 USDT"], + ["Token moved", "USDT (" + USDT_ADDR + ")"], + ["Moved to", "The swap step that spends it"], + ]; + // The same for a step's 400 USDT. + const USDT_400_GROUP = [ + ["Moved from the sender", "400.0000 USDT"], + ["Token moved", "USDT (" + USDT_ADDR + ")"], + ["Moved to", "The swap step that spends it"], + ]; // V4 SETTLE of `amount` of `currency`, and TAKE of `amount` of // `currency` to `recipient`. const settle = (currency, amount, payerIsUser) => @@ -1673,6 +1687,7 @@ describe("uniswap decoder", () => { encodeV4SettleAll(USDT_ADDR, USDT_400), ], lines: [ + ...USDT_600_GROUP, ["Moved from the sender", UNDECODED], ["Token moved", "USDT (" + USDT_ADDR + ")"], ["Moved to", "The swap step that spends it"], @@ -1693,6 +1708,7 @@ describe("uniswap decoder", () => { encodeV4TakeAll(USDC_ADDR, 0n), ], lines: [ + ...USDT_600_GROUP, ["Moved from the sender", UNDECODED], ["Token moved", "USDT (" + USDT_ADDR + ")"], ["Moved to", "The swap step that spends it"], @@ -1824,6 +1840,8 @@ describe("uniswap decoder", () => { // A step that may take from the sender what the decoder did not read // leaves no Amount figure, which may be too low, and one line says so. + // The amount of the step that set the input side then gets lines of + // its own. const UNREAD_SWAP = V4_SWAP_ACTION_SHAPES[0].newer; test.each([ { @@ -1871,6 +1889,7 @@ describe("uniswap decoder", () => { [V3_600, encodeV4Swap(new Uint8Array(actions), params)], ); expect(linesBetweenTokens(result)).toEqual([ + ...USDT_600_GROUP, ["Moved from the sender", UNDECODED], ]); }); @@ -1893,6 +1912,9 @@ describe("uniswap decoder", () => { ], ), ], + // Nothing read says the user pays this step, so its 400 USDT + // gets no lines of its own. + first: [], }, { kind: "a V3 step of 600 USDT, then a V4 step whose input cannot be read", @@ -1963,9 +1985,27 @@ describe("uniswap decoder", () => { ), ], }, - ])("$kind", ({ commands, inputs }) => { + // Each take the total would add up gets lines of its own. + { + kind: "a V3 step of 600 USDT and a V2 step of 400 USDT the user pays for, then a PERMIT2_TRANSFER_FROM_BATCH", + commands: [0x00, 0x08, 0x0d], + inputs: [ + V3_600, + encodeV2SwapExactIn(USER_ADDR, USDT_400, 390000000n, [ + USDT_ADDR, + USDC_ADDR, + ]), + coder.encode( + ["tuple(address,address,uint160,address)[]"], + [[[USER_ADDR, OTHER_ADDR, USDT_400, USDT_ADDR]]], + ), + ], + first: [...USDT_600_GROUP, ...USDT_400_GROUP], + }, + ])("$kind", ({ commands, inputs, first = USDT_600_GROUP }) => { const result = decodeSteps(commands, inputs); expect(linesBetweenTokens(result)).toEqual([ + ...first, ["Moved from the sender", UNDECODED], ]); }); @@ -1984,6 +2024,7 @@ describe("uniswap decoder", () => { commands: [0x00, 0x08, 0x21], inputs: [V3_600, V2_DAI_400, SUB_PLAN], lines: [ + ...USDT_600_GROUP, ["Moved from the sender", "400.0000 DAI"], ["Token moved", "DAI (" + DAI_ADDR + ")"], ["Moved to", "The swap step that spends it"], @@ -1995,6 +2036,7 @@ describe("uniswap decoder", () => { commands: [0x21, 0x00, 0x08], inputs: [SUB_PLAN, V3_600, V2_DAI_400], lines: [ + ...USDT_600_GROUP, ["Moved from the sender", UNDECODED], ["Moved from the sender", "400.0000 DAI"], ["Token moved", "DAI (" + DAI_ADDR + ")"], @@ -2013,6 +2055,7 @@ describe("uniswap decoder", () => { SUB_PLAN, ], lines: [ + ...USDT_600_GROUP, ["Moved from the sender", UNDECODED], ["Moved from the sender", "400.0000 DAI"], ["Token moved", "DAI (" + DAI_ADDR + ")"], @@ -2290,6 +2333,103 @@ describe("uniswap decoder", () => { expect(linesBetweenTokens(result)).toEqual(lines); }); + // A step that sets the input side and is paid from the router takes + // nothing from the sender, so with no Amount line its amount gets no + // lines of its own: in each route below the sender's 600 USDT moves + // into the router first, and such lines would say it left twice. + const INTO_ROUTER_600 = permit2TransferFrom( + USDT_ADDR, + ROUTER_ADDR, + USDT_600, + ); + const V3_600_FROM_ROUTER = coder.encode( + ["address", "uint256", "uint256", "bytes", "bool"], + [ + USER_ADDR, + USDT_600, + 590000000n, + encodeV3Path([USDT_ADDR, USDC_ADDR]), + false, + ], + ); + const TRANSFER_FROM_BATCH = coder.encode( + ["tuple(address,address,uint160,address)[]"], + [[[USER_ADDR, OTHER_ADDR, USDT_400, USDT_ADDR]]], + ); + test.each([ + { + kind: "a PERMIT2_TRANSFER_FROM of 600 USDT into the router, a V3 exact-in of 600 USDT paid from the router, then a PERMIT2_TRANSFER_FROM_BATCH", + commands: [0x02, 0x00, 0x0d], + inputs: [ + INTO_ROUTER_600, + V3_600_FROM_ROUTER, + TRANSFER_FROM_BATCH, + ], + lines: [["Moved from the sender", UNDECODED]], + }, + { + kind: "a PERMIT2_PERMIT that cannot be read, a PERMIT2_TRANSFER_FROM of 600 USDT into the router, then a V3 exact-in of 600 USDT paid from the router", + commands: [0x0a, 0x02, 0x00], + inputs: [ + dirtyWord( + encodePermit2(USDT_ADDR, 5000000000n, ROUTER_ADDR), + 0, + ), + INTO_ROUTER_600, + V3_600_FROM_ROUTER, + ], + lines: [["Permit2 allowance", UNDECODED]], + }, + { + kind: "a PERMIT2_TRANSFER_FROM of 600 USDT into the router, a V4 step swapping 600 USDT whose SETTLE the router pays, then a PERMIT2_TRANSFER_FROM_BATCH", + commands: [0x02, 0x10, 0x0d], + inputs: [ + INTO_ROUTER_600, + encodeV4Swap( + new Uint8Array([ + V4_SWAP_EXACT_IN_SINGLE_ID, + V4_SETTLE_ID, + V4_TAKE_ALL_ID, + ]), + [ + encodeV4ExactInSingle( + USDT_ADDR, + USDC_ADDR, + USDT_600, + 590000000n, + ), + settle(USDT_ADDR, USDT_600, false), + encodeV4TakeAll(USDC_ADDR, 0n), + ], + ), + TRANSFER_FROM_BATCH, + ], + lines: [["Moved from the sender", UNDECODED]], + }, + { + kind: "a PERMIT2_TRANSFER_FROM of 600 USDT into the router, a V2 exact-in of CONTRACT_BALANCE paid from the router, then an EXECUTE_SUB_PLAN", + commands: [0x02, 0x08, 0x21], + inputs: [ + INTO_ROUTER_600, + coder.encode( + ["address", "uint256", "uint256", "address[]", "bool"], + [ + USER_ADDR, + 2n ** 255n, // CONTRACT_BALANCE + 590000000n, + [USDT_ADDR, USDC_ADDR], + false, + ], + ), + SUB_PLAN, + ], + lines: [["Moved from the sender", UNDECODED]], + }, + ])("$kind", ({ commands, inputs, lines }) => { + const result = decodeSteps(commands, inputs); + expect(linesBetweenTokens(result)).toEqual(lines); + }); + // A V3_SWAP_EXACT_OUT the user pays for, of at most `amountInMax` // USDT for USDC. const v3ExactOut = (amountInMax) => @@ -2635,10 +2775,35 @@ describe("uniswap decoder", () => { [input, V3_600], ); expect(linesBetweenTokens(result)).toEqual([ + ...USDT_600_GROUP, ["Permit2 allowance", UNDECODED], ]); }, ); + + // https://git.eeqj.de/sneak/AutistMask/issues/659 — on next at + // 5f2b606 only the 400 DAI was shown. + test("a PERMIT2_PERMIT that cannot be read, then a V3 exact-in of 600 USDT and a V2 exact-in of 400 DAI, both paid by the user, shows both amounts", () => { + const result = decodeSteps( + [0x0a, 0x00, 0x08], + [ + dirtyWord( + encodePermit2(USDT_ADDR, USDT_5000, ROUTER_ADDR), + 0, + ), + V3_600, + encodeV2SwapExactIn(USER_ADDR, DAI_400, 390000000n, [ + DAI_ADDR, + USDC_ADDR, + ]), + ], + ); + expect(linesBetweenTokens(result)).toEqual([ + ...USDT_600_GROUP, + ...DAI_GROUP, + ["Permit2 allowance", UNDECODED], + ]); + }); }); // A V3_SWAP_EXACT_OUT whose path is shorter than one token cannot be @@ -2650,7 +2815,12 @@ describe("uniswap decoder", () => { [USER_ADDR, 390000000n, USDT_400, "0x", true], ); const result = decodeSteps([0x00, 0x01], [V3_600, unreadable]); + // With a V3 exact-out step, the 600 USDT reads as a maximum, as + // the Amount line would. expect(linesBetweenTokens(result)).toEqual([ + ["Moved from the sender", "Up to 600.0000 USDT"], + ["Token moved", "USDT (" + USDT_ADDR + ")"], + ["Moved to", "The swap step that spends it"], ["Moved from the sender", UNDECODED], ]); expect(detail(result, "Paid to").value).toBe(UNDECODED); @@ -2701,6 +2871,9 @@ describe("uniswap decoder", () => { ], ); expect(linesBetweenTokens(result)).toEqual([ + ["Moved from the sender", "Up to 0.5000 WETH"], + ["Token moved", "WETH (" + WETH_ADDR + ")"], + ["Moved to", "The swap step that spends it"], ["Moved from the sender", UNDECODED], ...permitLines("1.0000 WETH", WETH_ADDR, "WETH", ROUTER_ADDR), ]); @@ -2856,14 +3029,15 @@ describe("uniswap decoder", () => { ); // The V4 exact-out route above the other way round, where the later - // take's unknown amount leaves no figure at all, and other ways the - // step that sets the input side states no amount. + // take's unknown amount leaves no Amount figure and the 400 USDT gets + // lines of its own, and other ways the step that sets the input side + // states no amount. test.each([ { kind: "a V2 exact-out of at most 400 USDT, then a V4 exact-out step of at most 600 USDT", commands: [0x09, 0x10], inputs: [V2_EXACT_OUT_400, V4_EXACT_OUT_600], - lines: UNKNOWN_USDT, + lines: [...UP_TO_400_USDT, ...UNKNOWN_USDT], }, { kind: "a PERMIT2_PERMIT of 1 USDT to another spender, a V4 exact-out step of at most 600 USDT, then a V2 exact-out of at most 400 USDT", diff --git a/tests/uniswapRecipient.test.js b/tests/uniswapRecipient.test.js index a152bc9..5839033 100644 --- a/tests/uniswapRecipient.test.js +++ b/tests/uniswapRecipient.test.js @@ -1439,8 +1439,17 @@ describe("any token left in the router is not received", () => { "Token In", // A later step whose SETTLE or SETTLE_ALL the decoder cannot read // may take anything from the sender: one line says so, in place - // of the Amount line (https://git.eeqj.de/sneak/AutistMask/issues/625). - unread ? "Moved from the sender" : "Amount", + // of the Amount line (https://git.eeqj.de/sneak/AutistMask/issues/625), + // after the lines the WRAP_ETH's 1 ETH then gets + // (https://git.eeqj.de/sneak/AutistMask/issues/659). + ...(unread + ? [ + "Moved from the sender", + "Token moved", + "Moved to", + "Moved from the sender", + ] + : ["Amount"]), // A later swap step the user pays for spends `spent` of a token // other than Token In. ...(spent @@ -1457,9 +1466,11 @@ describe("any token left in the router is not received", () => { expect(detail(result, "Moved from the sender").value).toBe(spent); } if (unread) { - expect(detail(result, "Moved from the sender").value).toBe( - UNDECODED, - ); + expect( + result.details + .filter((d) => d.label === "Moved from the sender") + .map((d) => d.value), + ).toEqual(["1.0000 ETH", UNDECODED]); } expect(paidTo(result)).toEqual([STAYS_IN_ROUTER]); expect(detail(result, "Token moved into the router")).toEqual(left);