diff --git a/README.md b/README.md index cce151a..aef88ba 100644 --- a/README.md +++ b/README.md @@ -3226,36 +3226,41 @@ Currently supported: which it does only in the plain shape of Uniswap's own routes: it comes before the step that sets the input side, it is the only `PERMIT2_PERMIT` in the transaction and there is no `PERMIT2_PERMIT_BATCH`, its spender is the router, - the router may not let it fail, no step before it takes its token or a token - the decoder does not know, and the decoder reads every step after it in full. - Permit2 then lets the router take no more of the token than that amount, so - the permit covers every take of it. A later permit or batch may raise what - Permit2 lets the router take, and so may a step the decoder does not read in - full: a command the decoder does not read (listed at the end of this section), - a step whose input it cannot read, in whole or in part, and a step it fails - on. Every other `PERMIT2_PERMIT`, before the step that sets the input side or - after it, leaves `Token In` and the `Amount` line as they read with no permit: - a permit of 5,000 DAI to another address and then a V3 exact-in of 600 USDT - read `Swap USDT → USDC` with `Amount 600.0000 USDT`, and so do that exact-in - and then a permit of 5,000 DAI to the router. When a permit's amount stands - and the step that sets the input side spends a token other than the permit's, - or one the decoder does not know, that step's amount gets lines of its own, - unless it states no amount and the user pays it none of its input token. Each - take the total does not count gives three lines after the `Amount` line (after - `Token In` when there is none): one 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: `Moved from the sender`, its - amount, worded as the `Amount` line words one, or + the router may not let it fail, no step before it takes its token, a + `PERMIT2_TRANSFER_FROM` of it included, or a token the decoder does not know, + and the decoder reads every step after it in full. Permit2 then lets the + router take no more of the token than that amount, so the permit covers every + take of it. A step before the permit takes under whatever allowance the router + already had, so it may take more than the permit's amount, which would then + read lower than what leaves the user's wallet: a `PERMIT2_TRANSFER_FROM` of + 2,000 USDT into the router, then a permit of 600 USDT to the router and a V3 + exact-in of 2,000 USDT paid from the router, read `Amount 2000.0000 USDT`. A + later permit or batch may raise what Permit2 lets the router take, and so may + a step the decoder does not read in full: a command the decoder does not read + (listed at the end of this section), a step whose input it cannot read, in + whole or in part, and a step it fails on. Every other `PERMIT2_PERMIT`, before + the step that sets the input side or after it, leaves `Token In` and the + `Amount` line as they read with no permit: a permit of 5,000 DAI to another + address and then a V3 exact-in of 600 USDT read `Swap USDT → USDC` with + `Amount 600.0000 USDT`, and so do that exact-in and then a permit of 5,000 DAI + to the router. When a permit's amount stands and the step that sets the input + side spends a token other than the permit's, or one the decoder does not know, + that step's amount gets lines of its own, unless it states no amount and the + user pays it none of its input token. Each take the total does not count gives + three lines after the `Amount` line (after `Token In` when there is none): one + 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: + `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 `Moved to`, which reads `The swap step that spends it`. A token the decoder diff --git a/src/shared/uniswap.js b/src/shared/uniswap.js index 315e4df..d7ed833 100644 --- a/src/shared/uniswap.js +++ b/src/shared/uniswap.js @@ -1524,14 +1524,16 @@ function decode(data, toAddress, chainId, sources, fromAddress) { // side never stands, so only one before it is kept here. if (p && !inputEstablished) { // Whether a step before it took its token, or may - // have: one in `userPaid`, when its token is the - // permit's or not known. No step has set the input - // side yet. + // have: one in `userPaid`, `movedIntoRouter` or + // `movedFromSender`, when its token is the permit's + // or not known. No step has set the input side yet. const mayBeItsToken = (token) => !present(token) || sameAddress(token, p.token); - const tookBefore = userPaid.some((t) => - mayBeItsToken(t.token), - ); + const tookBefore = [ + ...userPaid, + ...movedIntoRouter, + ...movedFromSender, + ].some((t) => mayBeItsToken(t.token)); permit = { ...p, step: i, @@ -2009,16 +2011,18 @@ function decode(data, toAddress, chainId, sources, fromAddress) { // set the input side, it is the only PERMIT2_PERMIT in the // transaction and there is no PERMIT2_PERMIT_BATCH, its spender is // the router, the router may not let it fail, no step before it took - // its token or one not known, and this decoder read every step after - // it in full (see stepsUnread). Permit2 then lets the router take no - // more of the token than that amount, so the permit covers every take - // of it. A later permit or batch, or a later step this decoder did - // not read, may raise what Permit2 lets the router take. Every other - // PERMIT2_PERMIT leaves Token In and the Amount line as they read with - // no permit, and gets lines of its own below. A step that may take - // from the sender what this decoder did not read leaves no figure, - // permit or not (see spentUnread()), and so does a permit or batch - // whose input it cannot read (see permitUnread()). An + // its token or one not known (a PERMIT2_TRANSFER_FROM of its token + // included), and this decoder read every step after it in full (see + // stepsUnread). Permit2 then lets the router take no more of the token + // than that amount, so the permit covers every take of it. A step + // before it takes under whatever allowance the router already had, so + // it may take more. A later permit or batch, or a later step this + // decoder did not read, may raise what Permit2 lets the router take. + // Every other PERMIT2_PERMIT leaves Token In and the Amount line as + // they read with no permit, and gets lines of its own below. A step + // that may take from the sender what this decoder did not read leaves + // no figure, permit or not (see spentUnread()), and so does a permit + // or batch whose input it cannot read (see permitUnread()). An // EXECUTE_SUB_PLAN step leaves both sides unknown (below). const stepCommands = Array.from( commandsBytes, diff --git a/tests/uniswap.test.js b/tests/uniswap.test.js index 9f0a322..9637eca 100644 --- a/tests/uniswap.test.js +++ b/tests/uniswap.test.js @@ -2196,6 +2196,62 @@ describe("uniswap decoder", () => { }, ); + // https://git.eeqj.de/sneak/AutistMask/issues/655 — a + // PERMIT2_TRANSFER_FROM before the permit moves the permit's token + // under whatever allowance the router already had, so it may move + // more than the permit's amount. The permit's amount then does not + // stand, and the Amount line gives the total the steps take, as with + // no permit. On next at f53b503 the Amount line read the permit's + // 600.0000 USDT, and the permit got no lines of its own. + const USDT_2000 = 2000000000n; + // A PERMIT2_TRANSFER_FROM of `amount` of USDT to `recipient`. + const permit2TransferFrom = (recipient, amount) => + coder.encode( + ["address", "address", "uint160"], + [USDT_ADDR, recipient, amount], + ); + test.each([ + { + kind: "a PERMIT2_TRANSFER_FROM of 2,000 USDT into the router, then a PERMIT2_PERMIT of 600 USDT and a V3 exact-in of 2,000 USDT paid from the router,", + inputs: [ + permit2TransferFrom(ROUTER_ADDR, USDT_2000), + encodePermit2(USDT_ADDR, USDT_600, ROUTER_ADDR), + coder.encode( + ["address", "uint256", "uint256", "bytes", "bool"], + [ + USER_ADDR, + USDT_2000, + 1n, + encodeV3Path([USDT_ADDR, USDC_ADDR]), + false, + ], + ), + ], + lines: [["Amount", "2000.0000 USDT"], ...USDT_600_LINES], + }, + { + kind: "a PERMIT2_TRANSFER_FROM of 2,000 USDT to another address, then a PERMIT2_PERMIT of 600 USDT and a V3 exact-in of 600 USDT,", + inputs: [ + permit2TransferFrom(OTHER_ADDR, USDT_2000), + encodePermit2(USDT_ADDR, USDT_600, ROUTER_ADDR), + V3_600, + ], + lines: [ + ["Amount", "600.0000 USDT"], + ["Moved from the sender", "2000.0000 USDT"], + ["Token moved", "USDT (" + USDT_ADDR + ")"], + ["Moved to", OTHER_ADDR], + ...USDT_600_LINES, + ], + }, + ])( + "$kind shows the total the steps take and the permit's own lines", + ({ inputs, lines }) => { + const result = decodeSteps([0x02, 0x0a, 0x00], 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) =>