fix: a V3 exact-out swap step shows its tokens and amounts (closes #601)

A V3_SWAP_EXACT_OUT step was named in the Steps line but never read, so
its tokens read as not named in the calldata and it gave no amounts. It
is now read as a V2_SWAP_EXACT_OUT is: its path from the token it buys
to the token it spends, as the router reads it, its amountInMax as the
most it spends, its amountOut as the minimum, and its recipient as the
address it pays. One the user pays for after the step that sets the
input side joins the Amount total. A V3 path with bytes after its last
whole hop, which the router ignores, leaves either V3 swap step unread.

Also closes #637.

Model: opus-5-5

README.md

@@ -3012,29 +3012,31 @@ Currently supported:
   as the router reads it: a `V3_SWAP_EXACT_IN` path runs from the token it
   spends to the token it buys, and a `V3_SWAP_EXACT_OUT` path from the token it
   buys to the token it spends, so that step's input token is its path's last
-  token and its output token the first. A V4 swap action is read only in the
-  parameter shape deployed on mainnet and Sepolia, not in the newer one on
-  `v4-periphery` `main`, which adds a `minHopPriceX36` field. An action in any
-  other shape is not read, and its step is listed as
-  `V4 Swap (could not be decoded)`. Each step is named and read as the command
-  that the release of the router at that address runs for its command type, as
-  that release's `contracts/libraries/Commands.sol` names it. The decoder reads
-  the type from the command byte with the mask that router uses: `0x1f` for
-  `UniversalRouterV1`, `0x3f` for `UniversalRouterV1_2_V2Support` and
-  `UniversalRouterV2`, and `0x7f` for `UniversalRouterV2_1_1` and
-  `UniversalRouterV2_1_2`. The allow-revert flag (`0x80`) therefore does not
-  change the command a step is read as. Releases run different commands for some
-  types: on `UniversalRouterV1` and `UniversalRouterV1_2_V2Support`, `0x10` to
-  `0x13` are NFT commands, where the later routers run `V4_SWAP`,
-  `V3_POSITION_MANAGER_PERMIT`, `V3_POSITION_MANAGER_CALL` and
-  `V4_INITIALIZE_POOL`. The decoder does not read those routers' NFT commands,
-  and lists each by its name there, for example `Seaport` for `SEAPORT` (`0x10`
-  on `UniversalRouterV1`). A type a release names no command for is listed by
-  its number, for example `Command 0x0e` on `UniversalRouterV1`, which has no
-  `BALANCE_CHECK_ERC20` and reverts on it. The router reverts the whole
-  transaction when a step fails, unless the step's command byte has that flag
-  set and its command is one the router lets fail, as each release's
-  `contracts/base/Dispatcher.sol` shows: on `UniversalRouterV2`,
+  token and its output token the first. The router reads a path as one token
+  address followed by whole hops of a 3-byte fee and a token address, and
+  ignores any bytes after the last hop, so the decoder does not read a path with
+  such bytes. A V4 swap action is read only in the parameter shape deployed on
+  mainnet and Sepolia, not in the newer one on `v4-periphery` `main`, which adds
+  a `minHopPriceX36` field. An action in any other shape is not read, and its
+  step is listed as `V4 Swap (could not be decoded)`. Each step is named and
+  read as the command that the release of the router at that address runs for
+  its command type, as that release's `contracts/libraries/Commands.sol` names
+  it. The decoder reads the type from the command byte with the mask that router
+  uses: `0x1f` for `UniversalRouterV1`, `0x3f` for
+  `UniversalRouterV1_2_V2Support` and `UniversalRouterV2`, and `0x7f` for
+  `UniversalRouterV2_1_1` and `UniversalRouterV2_1_2`. The allow-revert flag
+  (`0x80`) therefore does not change the command a step is read as. Releases run
+  different commands for some types: on `UniversalRouterV1` and
+  `UniversalRouterV1_2_V2Support`, `0x10` to `0x13` are NFT commands, where the
+  later routers run `V4_SWAP`, `V3_POSITION_MANAGER_PERMIT`,
+  `V3_POSITION_MANAGER_CALL` and `V4_INITIALIZE_POOL`. The decoder does not read
+  those routers' NFT commands, and lists each by its name there, for example
+  `Seaport` for `SEAPORT` (`0x10` on `UniversalRouterV1`). A type a release
+  names no command for is listed by its number, for example `Command 0x0e` on
+  `UniversalRouterV1`, which has no `BALANCE_CHECK_ERC20` and reverts on it. The
+  router reverts the whole transaction when a step fails, unless the step's
+  command byte has that flag set and its command is one the router lets fail, as
+  each release's `contracts/base/Dispatcher.sol` shows: on `UniversalRouterV2`,
   `UniversalRouterV2_1_1` and `UniversalRouterV2_1_2`, `PERMIT2_PERMIT`,
   `PERMIT2_PERMIT_BATCH`, `BALANCE_CHECK_ERC20`, `V3_POSITION_MANAGER_PERMIT`,
   `V3_POSITION_MANAGER_CALL`, `V4_INITIALIZE_POOL`, `V4_POSITION_MANAGER_CALL`
@@ -3098,9 +3100,9 @@ Currently supported:
   `CONTRACT_BALANCE` and the router held no WETH from an earlier step when the
   `WRAP_ETH` ran: after two `WRAP_ETH`s of 0.5 ETH into the router, a swap of 1
   WETH from it reads as leaving the WETH there. A V2 or V3 exact-out step paid
-  from the router spends at most its `amountInMax`, an amount the calldata does
-  not state, so the decoder counts it as spending nothing the router holds. A
-  `SETTLE` of `OPEN_DELTA` (zero), and a `SETTLE_ALL`, pay only what the step
+  from the router spends an amount the calldata does not state, at most its
+  `amountInMax`, so the decoder counts it as spending nothing the router holds.
+  A `SETTLE` of `OPEN_DELTA` (zero), and a `SETTLE_ALL`, pay only what the step
   still owes of their token when they run. The decoder takes that to be the
   amounts the step's swap actions state they spend of that token only when every
   action in the step could be read, every swap action names the token it spends
@@ -3201,16 +3203,16 @@ Currently supported:
   input or one of whose `SETTLE`s or `SETTLE_ALL`s it cannot read, which the
   router still runs; a V2 or V3 exact-in or exact-out step or a
   `PERMIT2_TRANSFER_FROM` whose input it cannot read, such as a V3 step whose
-  path is shorter than one token address; every `PERMIT2_TRANSFER_FROM_BATCH`;
-  every `EXECUTE_SUB_PLAN`, wherever it stands; and every step it fails on. Each
-  such step gives one `Moved from the sender` line among those, reading
-  `Unknown (could not be decoded)`. That line, or any line for a token the
-  decoder does not know, leaves no `Amount` line, whose figure might be too low,
-  and such a step before a `PERMIT2_PERMIT` keeps the permit's amount from
-  standing. Then each `PERMIT2_TRANSFER_FROM` but one into the router, in step
-  order, gives three lines: `Moved from the sender`, the amount it moves, for
-  example `1000000.0000 USDT`; `Token moved`, giving the token's address as the
-  `Token In` line does; and `Moved to`, which reads
+  path is shorter than one token address or has bytes after its last hop; every
+  `PERMIT2_TRANSFER_FROM_BATCH`; every `EXECUTE_SUB_PLAN`, wherever it stands;
+  and every step it fails on. Each such step gives one `Moved from the sender`
+  line among those, reading `Unknown (could not be decoded)`. That line, or any
+  line for a token the decoder does not know, leaves no `Amount` line, whose
+  figure might be too low, and such a step before a `PERMIT2_PERMIT` keeps the
+  permit's amount from standing. Then each `PERMIT2_TRANSFER_FROM` but one into
+  the router, in step order, gives three lines: `Moved from the sender`, the
+  amount it moves, for example `1000000.0000 USDT`; `Token moved`, giving the
+  token's address as the `Token In` line does; and `Moved to`, which reads
   `The V2 pair the next swap spends` for a hop and `The sender` for `address(1)`
   on every router but `UniversalRouterV1`, and otherwise gives the address. A
   move into the router needs none of them, because its `Paid to` line and
@@ -3218,14 +3220,14 @@ Currently supported:
   where it goes, and so its amount is shown once. These lines change neither the
   `Paid to` lines nor the minimum's label. A step goes unread when its recipient
   does not decode as an address, when it is a V3 swap step whose path is shorter
-  than one token address, or when it is a command the decoder does not read:
-  every command other than `PERMIT2_PERMIT`, `PERMIT2_TRANSFER_FROM`,
-  `V2_SWAP_EXACT_IN`, `V2_SWAP_EXACT_OUT`, `V3_SWAP_EXACT_IN`,
-  `V3_SWAP_EXACT_OUT`, `V4_SWAP`, `SWEEP`, `TRANSFER`, `PAY_PORTION`,
-  `PAY_PORTION_FULL_PRECISION`, `ACROSS_V4_DEPOSIT_V3`, `WRAP_ETH`,
-  `UNWRAP_WETH` and `BALANCE_CHECK_ERC20` (`PERMIT2_TRANSFER_FROM_BATCH`,
-  `EXECUTE_SUB_PLAN` and the NFT commands among them), or a type its release
-  names no command for.
+  than one token address or has bytes after its last hop, or when it is a
+  command the decoder does not read: every command other than `PERMIT2_PERMIT`,
+  `PERMIT2_TRANSFER_FROM`, `V2_SWAP_EXACT_IN`, `V2_SWAP_EXACT_OUT`,
+  `V3_SWAP_EXACT_IN`, `V3_SWAP_EXACT_OUT`, `V4_SWAP`, `SWEEP`, `TRANSFER`,
+  `PAY_PORTION`, `PAY_PORTION_FULL_PRECISION`, `ACROSS_V4_DEPOSIT_V3`,
+  `WRAP_ETH`, `UNWRAP_WETH` and `BALANCE_CHECK_ERC20`
+  (`PERMIT2_TRANSFER_FROM_BATCH`, `EXECUTE_SUB_PLAN` and the NFT commands among
+  them), or a type its release names no command for.
 
 ### Non-Goals Forever
 

src/shared/uniswap.js

@@ -593,10 +593,13 @@ function decodeV2SwapExactOut(input) {
 }
 
 // The first and last token of a V3 swap path (packed: token(20) + fee(3) +
-// token(20) ...), or null when it is shorter than one token.
+// token(20) ...), or null when it is not one token followed by whole hops of
+// a fee and a token (40 hex digits, then 46 per hop). The router reads only
+// whole hops and ignores any bytes after the last one, so the last 20 bytes
+// of such a path are not the token it reads there.
 function decodeV3Path(pathHex) {
     const hex = pathHex.startsWith("0x") ? pathHex.slice(2) : pathHex;
-    if (hex.length < 40) return null;
+    if (hex.length < 40 || (hex.length - 40) % 46 !== 0) return null;
     const first = "0x" + hex.slice(0, 40);
     const last = "0x" + hex.slice(-40);
     return { first, last };

tests/uniswap.test.js

@@ -2227,6 +2227,56 @@ describe("uniswap decoder", () => {
             expect(detail(result, "Paid to").value).toBe(UNDECODED);
         });
 
+        // https://git.eeqj.de/sneak/AutistMask/issues/637 — the router reads
+        // a V3 path as one token and whole hops of a 3-byte fee and a token,
+        // and ignores any bytes after the last hop. Each path below is
+        // `tokens` and then the last 19 bytes of a bundled token's address
+        // whose first byte is the last token's last byte, so the path's last
+        // 20 bytes spell a token the router does not read. Such a path is not
+        // read, as one shorter than one token is not.
+        const pathEndingIn = (tokens, address) =>
+            encodeV3Path(tokens) + address.slice(4).toLowerCase();
+        const SPK_ADDR = "0xc20059e0317DE91738d13af027DfC4a50781b066";
+        const IUSD_ADDR = "0x48f9e38f3070AD8945DFEae3FA70987722E3D89c";
+        const v3Swap = (amount, otherAmount, path) =>
+            coder.encode(
+                ["address", "uint256", "uint256", "bytes", "bool"],
+                [USER_ADDR, amount, otherAmount, path, true],
+            );
+
+        // A V3 exact-in of 1 WETH for USDC pays out USDC, not IUSD.
+        test("a V3_SWAP_EXACT_IN whose path has bytes after its last hop names no Token Out", () => {
+            const path = pathEndingIn([WETH_ADDR, USDC_ADDR], IUSD_ADDR);
+            const result = decodeSteps([0x00], [v3Swap(ONE_WETH, 1n, path)]);
+            expect(detail(result, "Token Out").address).toBeUndefined();
+            expect(linesBetweenTokens(result)).toEqual([
+                ["Moved from the sender", UNDECODED],
+            ]);
+        });
+
+        // A V3 exact-out of 1,500 USDC for at most 0.5 WETH spends WETH, not
+        // SPK. After it, a PERMIT2_PERMIT of 1 WETH and a V3 exact-in of 0.5
+        // WETH: the permit's amount does not stand, because the step before
+        // it also takes WETH.
+        test("a V3_SWAP_EXACT_OUT whose path has bytes after its last hop, then a PERMIT2_PERMIT and a V3 exact-in, shows no Amount figure", () => {
+            const path = pathEndingIn([USDC_ADDR, WETH_ADDR], SPK_ADDR);
+            const result = decodeSteps(
+                [0x01, 0x0a, 0x00],
+                [
+                    v3Swap(1500000000n, ONE_WETH / 2n, path),
+                    encodePermit2(WETH_ADDR, ONE_WETH, ROUTER_ADDR),
+                    encodeV3SwapExactIn(USER_ADDR, ONE_WETH / 2n, 1n, [
+                        WETH_ADDR,
+                        USDC_ADDR,
+                    ]),
+                ],
+            );
+            expect(linesBetweenTokens(result)).toEqual([
+                ["Moved from the sender", UNDECODED],
+            ]);
+            expect(detail(result, "Paid to").value).toBe(UNDECODED);
+        });
+
         // A later step's amount that cannot be added to the Amount line,
         // because the amount of the step that set the input side is not a
         // quantity, gets lines of its own. When that step stated no amount,