fix: a V4 step of chained swap actions shows what the user receives (closes #563)

A V4 step's input side is the currency SETTLE or SETTLE_ALL names, else
the one currency some swap action spends and none buys; its output side
is the currency TAKE or TAKE_ALL names, else the one some swap action
buys and none spends. Amount and Min. received come from those actions,
summed when several exact-in actions each state one. A step with an
unread swap action is named only by SETTLE, SETTLE_ALL, TAKE and
TAKE_ALL, and shows no figure. A side a step with a swap action leaves
unknown shows no other step's token or figure. Before, the first swap
action set both sides, SETTLE_ALL and TAKE_ALL went unread, and a
SWAP_EXACT_OUT was read as spending its path's last currency.

Model: opus-5-5

README.md

@@ -1193,15 +1193,16 @@ read:
 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
 that side, or `Unknown (could not be decoded)` instead when a V4 swap action in
-the transaction could not be read (see Transaction Decoding below). A V4 step's
-sides follow three rules:
+the transaction could not be read (see Transaction Decoding below). A V4 step
+with a swap action sets both sides as a swap step does, following three rules:
 
 1. Its input side is the currency its `SETTLE` or `SETTLE_ALL` names, or else
    the one currency that some swap action in the step spends and none buys. Its
    output side is the currency its `TAKE` or `TAKE_ALL` names, or else the one
    currency that some swap action buys and none spends. If there is not exactly
-   one such currency, that side is unknown. A step that buys WETH with USDT and
-   USDC with that WETH, with nothing naming its sides, reads `Swap USDT → USDC`.
+   one such currency, that side is unknown, and no other step's token or figure
+   is shown for it. A step that buys WETH with USDT and USDC with that WETH,
+   with nothing naming its sides, reads `Swap USDT → USDC`.
 2. `Amount` is the `amountIn` of the swap action that spends the input currency.
    If several exact-in actions spend it, each with a stated amount, it is their
    sum; in any other case there is no `Amount` line. The decoder reads no amount
@@ -1210,9 +1211,11 @@ sides follow three rules:
    that buy the output currency.
 3. If any swap action in the step could not be read, only `SETTLE`,
    `SETTLE_ALL`, `TAKE` and `TAKE_ALL` name its sides, and there is no `Amount`
-   or `Min. received` line. Such a step still sets each side as a swap step
-   does, and no other step's token or figure is shown for a side they did not
-   name.
+   or `Min. received` line. A side they did not name is unknown, and no other
+   step's token or figure is shown for it.
+
+A V4 step with no swap action sets only the sides its `SETTLE`, `SETTLE_ALL`,
+`TAKE` or `TAKE_ALL` names.
 
 An `UNWRAP_WETH` step makes `Token Out` ETH only when the output side is WETH,
 on mainnet or Sepolia, or when no step set the output side; a WETH
@@ -2193,14 +2196,17 @@ view would leave a wallet one click from deletion.
   code `4100` and opens no prompt. Two such origins are `null`, which the
   browser reports for a page with an opaque origin, such as one its server
   sandboxes with a Content-Security-Policy, and `file://`, which Chrome reports
-  for a local file. The background script prefers the toolbar popup
-  (`action.openPopup()`) and falls back to a separate popup window
-  (`src/background/index.js`, `requestApproval()`). Only one exists per site at
-  a time: a further connection request from a site whose prompt is still
-  unanswered is refused with EIP-1193 code `-32002` and opens no new prompt. If
-  that prompt was in a toolbar popup that closed before it connected, and the
-  toolbar popup has since been set to open something else, the refused request
-  shows that prompt again.
+  for a local file. Nor is such a page told the active address by
+  `accountsChanged`, even at a connected site's URL: the background names the
+  origin of the tab's URL on the event, and the content script passes the
+  address on only to a page whose own origin is that one. The background script
+  prefers the toolbar popup (`action.openPopup()`) and falls back to a separate
+  popup window (`src/background/index.js`, `requestApproval()`). Only one exists
+  per site at a time: a further connection request from a site whose prompt is
+  still unanswered is refused with EIP-1193 code `-32002` and opens no new
+  prompt. If that prompt was in a toolbar popup that closed before it connected,
+  and the toolbar popup has since been set to open something else, the refused
+  request shows that prompt again.
 - **Elements**:
     - "Connection Request" heading
     - Phishing warning banner (shown when the hostname is on the phishing

src/background/index.js

@@ -1308,6 +1308,12 @@ async function broadcastAccountsChanged() {
         return;
     }
     for (const tab of tabs) {
+        // The tab's URL is all the background knows of the page in it, and a
+        // page at a site's URL may have another origin: a page its server
+        // sandboxes with a Content-Security-Policy has the origin "null". So
+        // the event names the origin it was judged by, and the content script
+        // passes the address on only to a page whose own origin is that one
+        // (src/content/index.js).
         const origin = tab.url ? new URL(tab.url).origin : "";
         const hasPermission =
             activeAddress &&
@@ -1319,6 +1325,7 @@ async function broadcastAccountsChanged() {
             type: "AUTISTMASK_EVENT",
             eventName: "accountsChanged",
             data: hasPermission ? [activeAddress] : [],
+            origin,
         }).catch(() => {});
     }
 }

src/content/index.js

@@ -49,14 +49,23 @@ window.addEventListener("message", (event) => {
 
 // Listen for events pushed from the background (e.g. accountsChanged)
 runtimeApi().onMessage.addListener((msg) => {
-    if (msg.type === "AUTISTMASK_EVENT") {
-        window.postMessage(
-            {
-                type: "AUTISTMASK_EVENT",
-                eventName: msg.eventName,
-                data: msg.data,
-            },
-            "*",
-        );
+    if (msg.type !== "AUTISTMASK_EVENT") return;
+    // The background chooses the addresses in accountsChanged for the site
+    // at the origin the event names, which is the origin of the tab's URL.
+    // This page hears them only when its own origin is that one. A page whose
+    // origin has no host is not a site: one its server sandboxes with a
+    // Content-Security-Policy has the origin "null" whatever its URL, and is
+    // told no address, as its requests are refused.
+    let data = msg.data;
+    if (msg.eventName === "accountsChanged" && msg.origin !== window.origin) {
+        data = [];
     }
+    window.postMessage(
+        {
+            type: "AUTISTMASK_EVENT",
+            eventName: msg.eventName,
+            data,
+        },
+        "*",
+    );
 });

src/shared/uniswap.js

@@ -440,6 +440,12 @@ function totalAmount(amounts) {
 // with an empty path buys, is null and counts in rules 1 and 2 as a currency
 // like any other: a step of only that action has an unknown Token Out and
 // the action's amountOutMin as Min. received.
+//
+// `hasSwapAction` is true when the step has a swap action, read or not.
+// decode() then sets both sides from this step, a side these rules leave
+// unknown included, so no other step's token or figure shows for it. A step
+// with no swap action sets only the sides SETTLE, SETTLE_ALL, TAKE and
+// TAKE_ALL name.
 function decodeV4Swap(input) {
     try {
         const d = coder.decode(["bytes", "bytes[]"], input);
@@ -506,6 +512,7 @@ function decodeV4Swap(input) {
                 amountIn: null,
                 amountOutMin: null,
                 swapUndecoded,
+                hasSwapAction: true,
             };
         }
 
@@ -538,7 +545,14 @@ function decodeV4Swap(input) {
             );
         }
 
-        return { tokenIn, tokenOut, amountIn, amountOutMin, swapUndecoded };
+        return {
+            tokenIn,
+            tokenOut,
+            amountIn,
+            amountOutMin,
+            swapUndecoded,
+            hasSwapAction: swaps.length > 0,
+        };
     } catch {
         return null;
     }
@@ -561,6 +575,7 @@ function decode(data, toAddress, sources) {
         let inputEstablished = false;
         let outputToken = null;
         let minOutput = null;
+        let outputEstablished = false;
         let hasUnwrapWeth = false;
         let hasV2ExactOut = false;
         let swapUndecoded = false;
@@ -596,6 +611,7 @@ function decode(data, toAddress, sources) {
             if (!present(token) && !present(amount)) return;
             outputToken = present(token) ? token : null;
             minOutput = present(amount) ? amount : null;
+            outputEstablished = true;
         };
 
         for (let i = 0; i < commandsBytes.length; i++) {
@@ -668,30 +684,32 @@ function decode(data, toAddress, sources) {
                     const v4 = decodeV4Swap(inputs[i]);
                     if (v4 && v4.swapUndecoded) {
                         // A swap action of this step went unread, so the step
-                        // is marked and still sets both sides as a swap step
-                        // does, with no figure. A side its SETTLE, SETTLE_ALL,
-                        // TAKE or TAKE_ALL did not name is set undetermined,
-                        // not left to an earlier or later step's token and
-                        // figure.
+                        // is marked.
                         swapUndecoded = true;
                         commandNames[i] += " (could not be decoded)";
+                    }
+                    if (v4 && v4.hasSwapAction) {
+                        // A step with a swap action sets both sides as a swap
+                        // step does, the input when no earlier step set it and
+                        // the output always, as the last swap step wins. Each
+                        // side is set as a pair, so a side decodeV4Swap() left
+                        // unknown is set unknown with no figure, rather than
+                        // left to another step's token and figure. A token
+                        // stated without a figure, or a figure without a
+                        // token, likewise never sits beside an earlier step's
+                        // other half.
                         if (!inputEstablished) {
                             setInput(v4.tokenIn, v4.amountIn);
                         }
                         outputToken = v4.tokenOut;
                         minOutput = v4.amountOutMin;
+                        outputEstablished = true;
                     } else if (v4) {
+                        // A step with no swap action sets only the sides its
+                        // SETTLE, SETTLE_ALL, TAKE or TAKE_ALL names:
+                        // setInputOnce() and setOutput() leave alone a side
+                        // whose two halves are both unstated.
                         setInputOnce(v4.tokenIn, v4.amountIn);
-                        // Always update output: last swap step wins. A step
-                        // that carries the Min. received figure but decoded no
-                        // output currency makes the output *undetermined* — it
-                        // is neither ETH nor whatever an earlier step named,
-                        // and that figure is no longer counted in that token.
-                        // Equally, a step that names an output currency but no
-                        // minimum leaves Min. received unstated rather than
-                        // keeping an earlier step's figure beside the new
-                        // token. setOutput() is both rules; a step that states
-                        // neither half leaves the output alone.
                         setOutput(v4.tokenOut, v4.amountOutMin);
                     }
                 }
@@ -705,21 +723,19 @@ function decode(data, toAddress, sources) {
         }
 
         // Resolve token info. A null token on either side means no step named
-        // a currency for it, or a V4 step whose swap action went unread left
-        // it undetermined; tokenInfo() refuses rather than calling it ETH.
+        // a currency for it, or a V4 step with a swap action left it
+        // unknown; tokenInfo() refuses rather than calling it ETH.
         // UNWRAP_WETH turns WETH into ETH, so it makes the output ETH when the
-        // output side is WETH, or when no step set the output side. A step
-        // whose swap action went unread did set it, to undetermined. Any other
+        // output side is WETH, or when no step set the output side. A V4 step
+        // with a swap action did set it, even when it set it unknown. Any other
         // output keeps its own token and figure: a swap that buys USDC and
         // then unwraps the WETH it did not spend receives USDC.
         const outputIsWeth =
             present(outputToken) &&
             WETH_ADDRESSES.includes(outputToken.toLowerCase());
-        const outputUnset =
-            !present(outputToken) && !present(minOutput) && !swapUndecoded;
         const inInfo = tokenInfo(inputToken, sources);
         const outInfo =
-            hasUnwrapWeth && (outputIsWeth || outputUnset)
+            hasUnwrapWeth && (outputIsWeth || !outputEstablished)
                 ? { symbol: "ETH", decimals: 18, address: null }
                 : tokenInfo(outputToken, sources);
 

tests/accountsChangedOpaqueOrigin.test.js

@@ -0,0 +1,167 @@
+// A page whose origin has no host is never told an address by accountsChanged
+// (https://git.eeqj.de/sneak/AutistMask/issues/564).
+//
+// When the user switches address, the background tells each open tab the new
+// active address if the site at the tab's URL is connected. A page at a
+// connected site's URL can still have another origin: one its server
+// sandboxes with a Content-Security-Policy has the origin "null", and every
+// request it makes is refused. Each test here switches the address in the real
+// background, takes the event it sent to a tab at the site's URL, and hands
+// that event to the real content script in a page of the given origin.
+
+const { networkById } = require("../src/shared/networks");
+const { makeStorageStub } = require("./support/storageStub");
+
+const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
+const SITE = "https://dapp.example";
+
+const EXT_URL = "chrome-extension://autistmask/";
+const FROM_POPUP = { url: EXT_URL + "src/popup/index.html" };
+
+// One address, with SITE remembered as allowed for it.
+function storedProfile() {
+    const mainnet = networkById("mainnet");
+    return {
+        hasWallet: true,
+        wallets: [
+            {
+                name: "Wallet 1",
+                type: "hd",
+                addresses: [
+                    { address: ADDRESS, balance: "0", tokenBalances: [] },
+                ],
+            },
+        ],
+        activeAddress: ADDRESS,
+        networkId: "mainnet",
+        rpcUrl: mainnet.defaultRpcUrl,
+        blockscoutUrl: mainnet.defaultBlockscoutUrl,
+        allowedSites: { [ADDRESS]: [SITE] },
+        deniedSites: {},
+        trackedTokens: [],
+    };
+}
+
+async function settle() {
+    for (let i = 0; i < 50; i++) await Promise.resolve();
+}
+
+afterEach(() => {
+    delete global.chrome;
+    delete global.window;
+    jest.resetModules();
+});
+
+// The popup tells the background the active address changed, and the one
+// open tab, at SITE's URL, is sent an event. That event is returned.
+async function eventSentToSiteTab() {
+    jest.resetModules();
+    jest.doMock("../src/shared/balances", () => ({
+        getProvider: () => ({}),
+        refreshBalances: jest.fn(async () => {}),
+    }));
+    jest.doMock("../src/shared/phishingDomains", () => ({
+        isPhishingDomain: () => false,
+    }));
+    jest.doMock("../src/shared/alarms", () => ({
+        BALANCE_REFRESH_ALARM: "balance",
+        BALANCE_REFRESH_PERIOD_MINUTES: 1,
+        ensureRecurringAlarms: jest.fn(async () => {}),
+        registerAlarmHandlers: jest.fn(),
+    }));
+
+    let messageListener = null;
+    const sentToTabs = [];
+    global.chrome = {
+        storage: makeStorageStub({ autistmask: storedProfile() }),
+        runtime: {
+            getURL: (path) => EXT_URL + path,
+            onMessage: {
+                addListener: (fn) => {
+                    messageListener = fn;
+                },
+            },
+            onConnect: { addListener: () => {} },
+            getPlatformInfo: (cb) => cb({ os: "linux" }),
+            lastError: null,
+        },
+        windows: {
+            remove: (id, cb) => {
+                if (cb) cb();
+            },
+            onRemoved: { addListener: () => {} },
+        },
+        tabs: {
+            query: (q, cb) => cb([{ id: 1, url: SITE + "/app" }]),
+            sendMessage: (tabId, msg, cb) => {
+                sentToTabs.push(msg);
+                if (cb) cb();
+            },
+        },
+        action: { setPopup: () => {} },
+    };
+
+    require("../src/background/index");
+    messageListener(
+        { type: "AUTISTMASK_ACTIVE_CHANGED" },
+        FROM_POPUP,
+        () => {},
+    );
+    await settle();
+
+    expect(sentToTabs).toHaveLength(1);
+    return sentToTabs[0];
+}
+
+// The content script in a page whose origin is `pageOrigin` is handed `event`
+// from the background. What it posts to the page is returned.
+function postedToPage(event, pageOrigin) {
+    jest.resetModules();
+    let eventListener = null;
+    const posted = [];
+    global.chrome = {
+        runtime: {
+            onMessage: {
+                addListener: (fn) => {
+                    eventListener = fn;
+                },
+            },
+        },
+    };
+    global.window = {
+        origin: pageOrigin,
+        addEventListener: () => {},
+        postMessage: (data) => posted.push(data),
+    };
+
+    require("../src/content/index");
+    eventListener(event);
+
+    return posted;
+}
+
+describe("accountsChanged after an address switch, at a connected site's URL", () => {
+    test("the site's own page is told the active address", async () => {
+        const event = await eventSentToSiteTab();
+
+        expect(postedToPage(event, SITE)).toEqual([
+            {
+                type: "AUTISTMASK_EVENT",
+                eventName: "accountsChanged",
+                data: [ADDRESS],
+            },
+        ]);
+    });
+
+    test("a page whose origin has no host is told no address", async () => {
+        const event = await eventSentToSiteTab();
+
+        expect(postedToPage(event, "null")).toEqual([
+            {
+                type: "AUTISTMASK_EVENT",
+                eventName: "accountsChanged",
+                data: [],
+            },
+        ]);
+    });
+});

tests/uniswap.test.js

@@ -1411,6 +1411,139 @@ describe("uniswap decoder", () => {
         },
     );
 
+    // A V4 step that spends WETH on USDC and on DAI. Its swap actions buy two
+    // currencies that none of them spends, and CLOSE_CURRENCY, which the
+    // decoder does not read, names no side, so its output side is unknown.
+    const WETH_FOR_USDC_AND_DAI = encodeV4Swap(
+        new Uint8Array([
+            V4_SWAP_EXACT_IN_SINGLE_ID,
+            V4_SWAP_EXACT_IN_SINGLE_ID,
+            V4_CLOSE_CURRENCY_ID,
+            V4_CLOSE_CURRENCY_ID,
+            V4_CLOSE_CURRENCY_ID,
+        ]),
+        [
+            // 0.25 WETH for at least 495 USDC
+            encodeV4ExactInSingle(
+                WETH_ADDR,
+                USDC_ADDR,
+                250000000000000000n,
+                495000000n,
+            ),
+            // 0.25 WETH for at least 495 DAI
+            encodeV4ExactInSingle(
+                WETH_ADDR,
+                DAI_ADDR,
+                250000000000000000n,
+                495000000000000000000n,
+            ),
+            encodeV4CloseCurrency(WETH_ADDR),
+            encodeV4CloseCurrency(USDC_ADDR),
+            encodeV4CloseCurrency(DAI_ADDR),
+        ],
+    );
+
+    // The user ends with USDC and DAI, not the V3 step's WETH. That WETH and
+    // the V3 step's Min. received figure were shown instead.
+    test("a V3 step, then a V4 step whose swap actions buy two currencies, does not show the V3 step's output", () => {
+        const data = buildExecute(
+            solidityPacked(["uint8", "uint8"], [0x00, 0x10]),
+            [
+                encodeV3SwapExactIn(
+                    USER_ADDR,
+                    1000000000n,
+                    500000000000000000n,
+                    [USDT_ADDR, WETH_ADDR],
+                ),
+                WETH_FOR_USDC_AND_DAI,
+            ],
+            9999999999n,
+        );
+        const result = uniswap.decode(data, ROUTER_ADDR);
+        expect(result.name).toBe("Uniswap Swap");
+        expect(detail(result, "Token In").address).toBe(
+            USDT_ADDR.toLowerCase(),
+        );
+        expect(detail(result, "Amount").value).toBe("1000.0000 USDT");
+        expect(detail(result, "Token Out").value).toBe(
+            "Unknown (not named in the calldata)",
+        );
+        expect(detail(result, "Min. received")).toBeUndefined();
+    });
+
+    // The input side of the same gap: the V4 step's swap actions spend USDC
+    // and DAI, two currencies that none of them buys, and nothing names its
+    // input side. The V3 step's WETH and its amount were shown instead.
+    test("a V4 step whose swap actions spend two currencies, then a V3 step, does not show the V3 step's input", () => {
+        const data = buildExecute(
+            solidityPacked(["uint8", "uint8"], [0x10, 0x00]),
+            [
+                encodeV4Swap(
+                    new Uint8Array([
+                        V4_SWAP_EXACT_IN_SINGLE_ID,
+                        V4_SWAP_EXACT_IN_SINGLE_ID,
+                        V4_CLOSE_CURRENCY_ID,
+                        V4_CLOSE_CURRENCY_ID,
+                        V4_CLOSE_CURRENCY_ID,
+                    ]),
+                    [
+                        // 500 USDC for at least 0.25 WETH
+                        encodeV4ExactInSingle(
+                            USDC_ADDR,
+                            WETH_ADDR,
+                            500000000n,
+                            250000000000000000n,
+                        ),
+                        // 500 DAI for at least 0.25 WETH
+                        encodeV4ExactInSingle(
+                            DAI_ADDR,
+                            WETH_ADDR,
+                            500000000000000000000n,
+                            250000000000000000n,
+                        ),
+                        encodeV4CloseCurrency(USDC_ADDR),
+                        encodeV4CloseCurrency(DAI_ADDR),
+                        encodeV4CloseCurrency(WETH_ADDR),
+                    ],
+                ),
+                encodeV3SwapExactIn(
+                    USER_ADDR,
+                    500000000000000000n,
+                    990000000n,
+                    [WETH_ADDR, USDT_ADDR],
+                ),
+            ],
+            9999999999n,
+        );
+        const result = uniswap.decode(data, ROUTER_ADDR);
+        expect(result.name).toBe("Uniswap Swap");
+        expect(detail(result, "Token In").value).toBe(
+            "Unknown (not named in the calldata)",
+        );
+        expect(detail(result, "Amount")).toBeUndefined();
+        expect(detail(result, "Token Out").address).toBe(
+            USDT_ADDR.toLowerCase(),
+        );
+        expect(detail(result, "Min. received").value).toBe("990.0000 USDT");
+    });
+
+    // UNWRAP_WETH makes Token Out ETH when no step set the output side. A V4
+    // step with a swap action did set it, to unknown here.
+    test("an UNWRAP_WETH after a V4 step whose swap actions buy two currencies does not make Token Out ETH", () => {
+        const data = buildExecute(
+            solidityPacked(["uint8", "uint8"], [0x10, 0x0c]),
+            [
+                WETH_FOR_USDC_AND_DAI,
+                encodeWrapEth(USER_ADDR, 0n), // UNWRAP_WETH, same encoding
+            ],
+            9999999999n,
+        );
+        const result = uniswap.decode(data, ROUTER_ADDR);
+        expect(detail(result, "Token Out").value).toBe(
+            "Unknown (not named in the calldata)",
+        );
+    });
+
     // The router works through an exact-out path from its last entry back to
     // its first: the WETH entry buys the USDC, then the USDT entry buys that
     // WETH. The user pays the first entry's USDT, not the last entry's WETH.