fix: the swap decoder reads who the router's transfer commands pay (closes #599)

TRANSFER, PAY_PORTION, PAY_PORTION_FULL_PRECISION (0x07, on the routers with
the 0x7f mask), PERMIT2_TRANSFER_FROM, ACROSS_V4_DEPOSIT_V3 (0x40) and V4
TAKE_PORTION were payouts the decoder did not read, so a swap using one read
Min. paid out with an unknown payee. Each recipient is now read: the sender,
the router, a hop into the next V2 pair, or another address, which is named.
Paying all the router holds of a token pays it out as SWEEP does; a TRANSFER
of CONTRACT_BALANCE of ETH reverts, so it pays nothing out. A
PERMIT2_TRANSFER_FROM into the router is shown with the amount it moves and
the moved token's address, and a bridge deposit names the bridge and each of
its recipient and depositor that is not the sender; either keeps the minimum
from reading received.

Model: opus-5-5

README.md

@@ -2986,39 +2986,42 @@ Currently supported:
   WETH, a `TRANSFER` of `CONTRACT_BALANCE`, a `PAY_PORTION` of 10,000 bips or a
   `PAY_PORTION_FULL_PRECISION` of 1e18 of that token, or an
   `ACROSS_V4_DEPOSIT_V3` of `CONTRACT_BALANCE` of that token, or of ETH when it
-  sets `useNative`. A step paying a token to the V2 pair that the very next step
-  spends, as a V2 exact-in of that token whose `amountIn` is `ALREADY_PAID`
-  (zero) or `CONTRACT_BALANCE`, is a hop, not a payout: Uniswap's mixed routes
-  pay the next V2 pair directly, and the router enforces that swap's own
-  minimum. The decoder checks that the address paid is that pair, deriving it
-  from the V2 factory and pair init code hash the network's routers were
-  deployed with and the first two tokens of that step's path; a payment to any
-  other address is named. (Sepolia's `UniversalRouterV1_2_V2Support` was
-  deployed with placeholders for both values, so no V2 swap step can succeed on
-  that router.) Every other payment counts, in whichever step and of whatever
-  token, a share of the output included: an interface fee paid to another
-  address by `PAY_PORTION`, `TRANSFER` or `TAKE_PORTION` is named, and the
-  minimum reads `Min. paid out`, because the swap's minimum includes the fee.
-  The minimum reads `Min. received` only when some step pays the transaction's
-  sender (the From address on TxApproval, the sender the explorer reports on
-  TransactionDetail) and no `Paid to` line is shown. `Paid to` lines after
-  `Token Out` name each other address paid; one reads
+  sets `useNative`. The router sends ETH as written, so a `TRANSFER` of
+  `CONTRACT_BALANCE` of ETH always reverts and pays out nothing. A step paying a
+  token to the V2 pair that the very next step spends, as a V2 exact-in of that
+  token whose `amountIn` is `ALREADY_PAID` (zero) or `CONTRACT_BALANCE`, is a
+  hop, not a payout: Uniswap's mixed routes pay the next V2 pair directly, and
+  the router enforces that swap's own minimum. The decoder checks that the
+  address paid is that pair, deriving it from the V2 factory and pair init code
+  hash the network's routers were deployed with and the first two tokens of that
+  step's path; a payment to any other address is named. (Sepolia's
+  `UniversalRouterV1_2_V2Support` was deployed with placeholders for both
+  values, so no V2 swap step can succeed on that router.) Every other payment
+  counts, in whichever step and of whatever token, a share of the output
+  included: an interface fee paid to another address by `PAY_PORTION`,
+  `TRANSFER` or `TAKE_PORTION` is named, and the minimum reads `Min. paid out`,
+  because the swap's minimum includes the fee. The minimum reads `Min. received`
+  only when some step pays the transaction's sender (the From address on
+  TxApproval, the sender the explorer reports on TransactionDetail) and no
+  `Paid to` line is shown. `Paid to` lines after `Token Out` name each other
+  address paid; one reads
   `The bridge, which pays the deposit's recipient on another network` for a
   bridge deposit; one reads, for example,
   `The router, 2000.0000 USDT from the sender (later steps may not spend all of it)`
-  for each move into the router; one reads
-  `The router, where it stays (no later step pays it out)` when the output stays
-  in the router; and one more reads `Unknown (could not be decoded)` when a step
-  that may pay an address went unread. When no step pays anyone, a single line
-  reads `Unknown (not named in the calldata)`. Wherever there is a `Paid to`
-  line, the minimum reads `Min. paid out`. A step goes unread when its recipient
-  does not decode as an 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`, `V4_SWAP`,
-  `SWEEP`, `TRANSFER`, `PAY_PORTION`, `PAY_PORTION_FULL_PRECISION`,
-  `ACROSS_V4_DEPOSIT_V3`, `WRAP_ETH`, `UNWRAP_WETH` and `BALANCE_CHECK_ERC20`
-  (`V3_SWAP_EXACT_OUT`, `PERMIT2_TRANSFER_FROM_BATCH` and `EXECUTE_SUB_PLAN`
-  among them).
+  for each move into the router, followed by a `Token moved into the router`
+  line that gives the moved token's address as the `Token In` line does; one
+  reads `The router, where it stays (no later step pays it out)` when the output
+  stays in the router; and one more reads `Unknown (could not be decoded)` when
+  a step that may pay an address went unread. When no step pays anyone, a single
+  line reads `Unknown (not named in the calldata)`. Wherever there is a
+  `Paid to` line, the minimum reads `Min. paid out`. A step goes unread when its
+  recipient does not decode as an 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`, `V4_SWAP`, `SWEEP`, `TRANSFER`, `PAY_PORTION`,
+  `PAY_PORTION_FULL_PRECISION`, `ACROSS_V4_DEPOSIT_V3`, `WRAP_ETH`,
+  `UNWRAP_WETH` and `BALANCE_CHECK_ERC20` (`V3_SWAP_EXACT_OUT`,
+  `PERMIT2_TRANSFER_FROM_BATCH` and `EXECUTE_SUB_PLAN` among them).
 
 ### Non-Goals Forever
 

src/shared/uniswap.js

@@ -448,10 +448,12 @@ function decodeSweep(input) {
 
 // Decode TRANSFER (command 0x05) input bytes, and those of PAY_PORTION (0x06)
 // and PAY_PORTION_FULL_PRECISION (0x07), which share its layout. Each pays
-// `token` out of the router to the recipient. TRANSFER pays `amount`, or all
-// the router holds when `amount` is CONTRACT_BALANCE. PAY_PORTION pays
-// `amount` ten-thousandths of what the router holds, and
-// PAY_PORTION_FULL_PRECISION `amount` 1e18ths of it.
+// `token` out of the router to the recipient. TRANSFER pays `amount`, or, of a
+// token other than ETH, all the router holds when `amount` is
+// CONTRACT_BALANCE; it sends ETH as written, so a TRANSFER of
+// CONTRACT_BALANCE of ETH (`token` the zero address) always reverts.
+// PAY_PORTION pays `amount` ten-thousandths of what the router holds, and
+// PAY_PORTION_FULL_PRECISION `amount` 1e18ths of it, of ETH too.
 // - UniversalRouterV1: https://github.com/Uniswap/universal-router/blob/cda697da221e24ca143bc4f55023694a2a16796f/contracts/modules/Payments.sol
 // - UniversalRouterV2_1_2: https://github.com/Uniswap/universal-router/blob/802fe4c18f47300e0f183e2a42e9146ec2ea9fc3/contracts/modules/Payments.sol
 // ABI: (address token, address recipient, uint256 amount)
@@ -1105,7 +1107,18 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
                                 : cmdId === 0x06
                                   ? 10000n
                                   : 10n ** 18n;
-                        if (t.amount === all && !isRouter(t.recipient)) {
+                        // A TRANSFER of ETH has no such amount: the router
+                        // sends ETH as written, so a TRANSFER of
+                        // CONTRACT_BALANCE of ETH always reverts.
+                        const ethTransfer =
+                            cmdId === 0x05 &&
+                            t.token ===
+                                "0x0000000000000000000000000000000000000000";
+                        if (
+                            t.amount === all &&
+                            !ethTransfer &&
+                            !isRouter(t.recipient)
+                        ) {
                             heldByRouter.delete(t.token.toLowerCase());
                         }
                     } else {
@@ -1361,7 +1374,8 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
         // router, when the output stays in the router, and when a payout
         // could not be read. The output is received by the sender only when
         // some step pays the sender and there is no such line. When no step
-        // names anyone paid, one line says so.
+        // names anyone paid, one line says so. `paidTo` holds these lines and
+        // the token line after each move into the router.
         const outputHeld =
             present(outputToken) && heldByRouter.has(outputToken.toLowerCase());
         const paidTo = paidElsewhere.map((address) => ({
@@ -1372,20 +1386,26 @@ function decode(data, toAddress, chainId, sources, fromAddress) {
         if (bridgeDeposit) {
             paidTo.push({ label: "Paid to", value: PAID_TO_BRIDGE });
         }
+        // Each move into the router is followed by a line giving the moved
+        // token's address, in the shape of the Token In line, so the screens
+        // show it as they show that one.
         for (const m of movedIntoRouter) {
             const info = tokenInfo(m.token, sources);
-            // amountText() names the token only when it knows both its
-            // symbol and its scale.
-            const ofToken =
-                info.symbol && info.decimals !== null ? "" : " of " + m.token;
             paidTo.push({
                 label: "Paid to",
                 value:
                     "The router, " +
                     amountText(m.amount, info).display +
-                    ofToken +
                     " from the sender (later steps may not spend all of it)",
             });
+            paidTo.push({
+                label: "Token moved into the router",
+                value: info.symbol
+                    ? info.symbol + " (" + m.token + ")"
+                    : m.token,
+                address: m.token,
+                isToken: true,
+            });
         }
         if (outputHeld) {
             paidTo.push({ label: "Paid to", value: STAYS_IN_ROUTER });

tests/uniswapRecipient.test.js

@@ -54,6 +54,8 @@ const ZERO = "0x0000000000000000000000000000000000000000";
 const USDT = "0xdAC17F958D2ee523a2206206994597C13D831ec7";
 const USDC = "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48";
 const WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
+// A token the wallet does not know.
+const UNKNOWN_TOKEN = "0x5555555555555555555555555555555555555555";
 const ONE_WETH = 1000000000000000000n;
 const USDT_2000 = 2000000000n;
 const USDC_1990 = 1990000000n;
@@ -623,8 +625,24 @@ describe("each transfer command's recipient", () => {
         "PERMIT2_TRANSFER_FROM into the router as $kind names what it moves, and nothing is received",
         ({ r }) => {
             const result = decode(movedThenSwapped(r));
-            expect(labels(result)).toEqual(PAID_ELSEWHERE);
+            expect(labels(result)).toEqual([
+                "Protocol",
+                "Token In",
+                "Amount",
+                "Token Out",
+                "Paid to",
+                "Token moved into the router",
+                "Min. paid out",
+                "Steps",
+                "Deadline",
+            ]);
             expect(paidTo(result)).toEqual([movedIntoRouter("2000.0000 USDT")]);
+            expect(detail(result, "Token moved into the router")).toEqual({
+                label: "Token moved into the router",
+                value: "USDT (" + USDT + ")",
+                address: USDT,
+                isToken: true,
+            });
             expect(detail(result, "Min. paid out").value).toBe("1.0000 WETH");
         },
     );
@@ -655,17 +673,22 @@ describe("each transfer command's recipient", () => {
         },
     );
 
-    test("a move into the router of a token the wallet does not know names the token", () => {
-        const unknown = "0x5555555555555555555555555555555555555555";
+    test("a move into the router of a token the wallet does not know gives the token's address", () => {
         const result = decode(
             execute(
-                permit2TransferFrom(unknown, ADDRESS_THIS, 5n),
+                permit2TransferFrom(UNKNOWN_TOKEN, ADDRESS_THIS, 5n),
                 swapToWeth(MSG_SENDER),
             ),
         );
         expect(paidTo(result)).toEqual([
-            movedIntoRouter("5 base units (decimals unknown) of " + unknown),
+            movedIntoRouter("5 base units (decimals unknown)"),
         ]);
+        expect(detail(result, "Token moved into the router")).toEqual({
+            label: "Token moved into the router",
+            value: UNKNOWN_TOKEN,
+            address: UNKNOWN_TOKEN,
+            isToken: true,
+        });
     });
 
     test("UniversalRouterV1 pays a PERMIT2_TRANSFER_FROM to address(1) or address(2) as written, and maps a TRANSFER's", () => {
@@ -743,6 +766,32 @@ describe("a payout of part of the router's output", () => {
         expect(detail(result, "Min. paid out").value).toBe("1.0000 WETH");
     });
 
+    // The router sends ETH as written, so a TRANSFER of CONTRACT_BALANCE of
+    // ETH always reverts; PAY_PORTION reads the router's ETH balance.
+    test("a TRANSFER of CONTRACT_BALANCE of ETH pays out nothing; a PAY_PORTION of 10,000 bips of ETH pays out all of it", () => {
+        const ethToRouter = v4Swap(
+            SWAP_USDT_FOR_ETH,
+            SETTLE_USDT,
+            take(ZERO, ADDRESS_THIS),
+        );
+
+        const transferred = decode(
+            execute(
+                ethToRouter,
+                transfer(0x05, ZERO, MSG_SENDER, CONTRACT_BALANCE),
+            ),
+        );
+        expect(detail(transferred, "Token Out").value).toBe("ETH");
+        expect(paidTo(transferred)).toEqual([STAYS_IN_ROUTER]);
+        expect(detail(transferred, "Min. received")).toBeUndefined();
+
+        const portion = decode(
+            execute(ethToRouter, transfer(0x06, ZERO, MSG_SENDER, 10000n)),
+        );
+        expect(labels(portion)).toEqual(PAID_TO_USER);
+        expect(detail(portion, "Min. received").value).toBe("1.0000 ETH");
+    });
+
     // An interface fee: a share of the output to another address, the rest
     // to the user. The swap's minimum includes the fee, so it is not what the
     // user receives.
@@ -1459,3 +1508,25 @@ describe("the screens judge the output against the transaction's sender", () =>
         expect(toOther).not.toContain("Min. received");
     });
 });
+
+// The address of each token moved into the router, a known one and one the
+// wallet does not know, is shown as every other address is: with an explorer
+// link, and copied when clicked. Neither token is the swap's.
+describe("the screens show each token moved into the router as an address", () => {
+    const twoMovesThenSwap = execute(
+        permit2TransferFrom(USDC, ADDRESS_THIS, USDC_1990),
+        permit2TransferFrom(UNKNOWN_TOKEN, ADDRESS_THIS, 5n),
+        swapToWeth(MSG_SENDER),
+    );
+
+    test.each([
+        { screen: "approval", show: showApproval },
+        { screen: "transaction detail", show: showDetail },
+    ])("the $screen screen", async ({ show }) => {
+        const html = await show(twoMovesThenSwap);
+        for (const token of [USDC, UNKNOWN_TOKEN]) {
+            expect(html).toContain('data-copy="' + token + '"');
+            expect(html).toContain("/address/" + token + '"');
+        }
+    });
+});