fix: each browser-suite run builds and removes an image of its own (closes #547)

script/test-e2e and script/test-e2e-firefox tagged their image with one
fixed name. When two clones ran a suite at once, the second build moved
that tag, the image store deleted the first run's now-untagged image, and
that suite exited 125 without running a test. Each run now tags its image
with the suite's name and twelve random hex digits, runs it by that tag,
and removes it in the scripts' existing exit and interrupt cleanup, so no
run leaves an image behind. The image-ID file that guarded against a
moving tag is no longer needed. README.md's End-to-End Tests section says
so.

Model: opus-5-5

README.md

@@ -347,6 +347,12 @@ are outside `make check`. Neither opens the popup from the toolbar button: both
 load its page in an ordinary tab, so what the toolbar popup itself adds, its
 size and its closing when it loses focus, is covered by neither.
 
+Each run builds its image under a tag of its own, `autistmask-e2e-chrome:` or
+`autistmask-e2e-firefox:` followed by twelve random hex digits, and removes that
+image when it exits, an interrupt included. Two clones can therefore run the
+same suite at once without one build taking the other's image away, and no run
+leaves an image behind.
+
 ### Chrome (`make test-e2e`)
 
 `make test-e2e` builds `dist/chrome/` and drives the **real popup in a real

script/test-e2e

@@ -22,13 +22,18 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome"
-
-IIDFILE=""
+# Each run builds, runs and then removes an image under a tag of its own.
+# Under one tag shared by every run, a second clone's build moves the tag
+# off this run's image, and the image store can then delete that untagged
+# image before or while this run uses it. The tag is random rather than
+# $$ because CI jobs share one docker daemon, but each job's container
+# numbers its processes from scratch.
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-chrome:$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
 
 cleanup() {
-    if [ -n "$IIDFILE" ]; then
-        rm -f "$IIDFILE"
+    # No image to remove when the build failed or was interrupted.
+    if docker image inspect "$IMAGE" >/dev/null 2>&1; then
+        docker image rm "$IMAGE" >/dev/null
     fi
 }
 
@@ -40,22 +45,15 @@ main() {
         exit 1
     fi
 
-    IIDFILE="$(mktemp)"
     trap cleanup EXIT
     trap 'cleanup; exit 130' INT TERM
 
     echo "Building the Chrome e2e image (extension included)..."
     # --no-cache: the image build runs make build and its checks, and a
     # cached layer is a check that did not run.
-    docker build --no-cache --iidfile "$IIDFILE" -t "$IMAGE" \
-        -f tests/e2e/Dockerfile .
+    docker build --no-cache -t "$IMAGE" -f tests/e2e/Dockerfile .
 
     echo "Running e2e suite in the pinned Playwright container..."
-    # The image is run by ID, not by tag: where two clones of this repo run
-    # the suite at once, the other build can move the tag between this
-    # build and this run, and the suite would then silently test the other
-    # checkout.
-    #
     # --ipc=host: Chromium's shared-memory needs more than the default
     #   64MB /dev/shm or renderers crash.
     # HOME=/tmp: the image's root home is not a reliable place for the
@@ -79,7 +77,7 @@ main() {
         -e HOME=/tmp \
         -e PW_EXPERIMENTAL_SERVICE_WORKER_NETWORK_EVENTS=1 \
         -e "E2E_TRACE_NETWORK=${E2E_TRACE_NETWORK:-0}" \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/run.js
 
     # Where chrome.storage.local lives, and what moves it: two unpacked loads
@@ -95,7 +93,7 @@ main() {
     docker run --rm \
         --ipc=host \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/storagePartition.js
 }
 

script/test-e2e-firefox

@@ -21,13 +21,13 @@ set -eu
 SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
 ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
 
-IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox"
-
-IIDFILE=""
+# A tag of this run's own, as in script/test-e2e, which says why.
+IMAGE="$("$SCRIPT_DIR/projectname")-e2e-firefox:$(od -An -N6 -tx1 /dev/urandom | tr -d ' \n')"
 
 cleanup() {
-    if [ -n "$IIDFILE" ]; then
-        rm -f "$IIDFILE"
+    # No image to remove when the build failed or was interrupted.
+    if docker image inspect "$IMAGE" >/dev/null 2>&1; then
+        docker image rm "$IMAGE" >/dev/null
     fi
 }
 
@@ -39,22 +39,15 @@ main() {
         exit 1
     fi
 
-    IIDFILE="$(mktemp)"
     trap cleanup EXIT
     trap 'cleanup; exit 130' INT TERM
 
     echo "Building the pinned Firefox e2e image (extension included)..."
     # --no-cache: the image build runs make build and its checks, and a
     # cached layer is a check that did not run.
-    docker build --no-cache --iidfile "$IIDFILE" -t "$IMAGE" \
-        -f tests/e2e/firefox/Dockerfile .
+    docker build --no-cache -t "$IMAGE" -f tests/e2e/firefox/Dockerfile .
 
     echo "Running the Firefox e2e suite..."
-    # The image is run by ID, not by tag: where two clones of this repo run
-    # the suite at once, the other build can move the tag between this
-    # build and this run, and the suite would then silently test the other
-    # checkout.
-    #
     # --shm-size=1g: Firefox needs more than the default 64MB /dev/shm.
     # --network none: the suite stubs nothing, so this is what keeps the
     #   run offline and deterministic. The extension swallows its own
@@ -72,7 +65,7 @@ main() {
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/firefox/run.js dist/firefox
 
     # The install/uninstall/re-install property, against the packaged XPI
@@ -85,7 +78,7 @@ main() {
         --shm-size=1g \
         --network none \
         -e HOME=/tmp \
-        "$(cat "$IIDFILE")" \
+        "$IMAGE" \
         node tests/e2e/firefox/reinstall.js
 }
 

src/popup/views/transactionDetail.js

@@ -183,7 +183,7 @@ function render() {
         if (el) el.classList.add("hidden");
     }
 
-    loadFullTxDetails(tx.hash, tx.chainId);
+    loadFullTxDetails(tx.hash, tx.to, tx.chainId);
 
     const isoStr = isoDate(tx.timestamp);
     $("tx-detail-time").innerHTML =
@@ -291,7 +291,7 @@ function populateOnChainDetails(txData, chainId) {
     }
 }
 
-async function loadFullTxDetails(txHash, chainId) {
+async function loadFullTxDetails(txHash, toAddress, chainId) {
     const section = $("tx-detail-calldata-section");
     const actionEl = $("tx-detail-calldata-action");
     const detailsEl = $("tx-detail-calldata-details");
@@ -313,10 +313,7 @@ async function loadFullTxDetails(txHash, chainId) {
         const inputData = txData.raw_input || txData.input || null;
         if (!inputData || inputData === "0x") return;
 
-        // Decoded against the address the transaction was sent to, as the
-        // explorer reports it, not the history entry's `to`: for a token
-        // transfer that is the transfer's recipient, not the token contract.
-        const decoded = decodeCalldata(inputData, txData.to?.hash || "");
+        const decoded = decodeCalldata(inputData, toAddress || "");
         if (decoded) {
             // Render decoded calldata matching approval view style
             actionEl.textContent = decoded.name;

src/shared/walletDefects.js

@@ -36,11 +36,13 @@ const DEFECTS = {
                 "derivation path beneath that key instead of from a master " +
                 "key, so the addresses listed here are not the ones that key " +
                 "produces under the standard path.",
-            "Signing and sending are disabled for this wallet. The addresses " +
-                "do descend from the extended private key you imported, so " +
-                "anything they hold is still reachable by software that " +
-                "repeats the same non-standard derivation. Check them in a " +
-                "block explorer before deciding what to do.",
+            "Signing, sending and private key export are disabled for this " +
+                "wallet. Anything these addresses hold can be reached outside " +
+                "this wallet with the extended private key you imported, if " +
+                "you kept a copy of it, or with the master extended private " +
+                "key or recovery phrase it came from, in software that " +
+                "repeats the same non-standard derivation. Check the " +
+                "addresses in a block explorer before deciding what to do.",
             "To see the addresses this key produces under the standard path, " +
                 "import the master extended private key, or the recovery " +
                 "phrase it came from, as a new wallet. Nothing here has been " +

tests/tokenTransferDetail.test.js

@@ -1,176 +0,0 @@
-// The decoded details on the transaction detail screen of a token transfer
-// (https://git.eeqj.de/sneak/AutistMask/issues/515).
-//
-// A history row for a token transfer holds the transfer's recipient as its
-// `to`, not the contract the transaction was sent to. The screen used to decode
-// the transaction's data against that `to`, so its Token line named the
-// recipient, and the amount read "decimals unknown" for a token the wallet
-// tracks. It decodes against the address the explorer reports the transaction
-// was sent to.
-//
-// The row is the one the real history parser makes from the explorer's answer,
-// opened through transactionDetail.show(), which is what a click on the row
-// calls. Driven against a minimal DOM stub in the shape
-// tests/contractCreation.test.js uses.
-
-const { Interface } = require("ethers");
-
-jest.mock("../src/shared/log", () => ({
-    log: {
-        debugf: () => {},
-        infof: () => {},
-        warnf: () => {},
-        errorf: () => {},
-    },
-    debugFetch: async (url) => ({
-        ok: true,
-        json: async () => mockExplorerAnswer(url),
-    }),
-    setRuntimeDebug: () => {},
-    isDebug: () => false,
-}));
-
-globalThis.chrome = {
-    storage: { local: { get: async () => ({}), set: async () => {} } },
-};
-
-const { state } = require("../src/shared/state");
-const { fetchRecentTransactions } = require("../src/shared/transactions");
-const transactionDetail = require("../src/popup/views/transactionDetail");
-
-const EXPLORER = "https://explorer.example/api/v2";
-const HOLDER = "0x0000000000000000000000000000000000000a11";
-const RECIPIENT = "0x711e691962E5c054b78071C789A0C8CE657377aD";
-// A token the wallet tracks and the bundled token list does not name.
-const TOKEN = "0x74de5d4FCbf63E00296fd95d33236B9794016631";
-const TX_HASH =
-    "0x85215772ed26ea8b39c2b3b18779030487efbe0b5fd7e882592b2f62b837be84";
-
-// HOLDER sends 1234.5 of the 6-decimal token to RECIPIENT.
-const TRANSFER_DATA = new Interface([
-    "function transfer(address to, uint256 amount)",
-]).encodeFunctionData("transfer", [RECIPIENT, 1234500000n]);
-
-// The explorer's answers: the transfer in HOLDER's token transfers, and the
-// transaction itself, sent to the token contract.
-function mockExplorerAnswer(url) {
-    if (
-        url ===
-        EXPLORER + "/addresses/" + HOLDER + "/token-transfers?type=ERC-20"
-    ) {
-        return {
-            items: [
-                {
-                    transaction_hash: TX_HASH,
-                    block_number: 100,
-                    timestamp: "2026-10-01T00:00:00Z",
-                    from: { hash: HOLDER },
-                    to: { hash: RECIPIENT },
-                    total: { value: "1234500000", decimals: "6" },
-                    token: {
-                        address_hash: TOKEN,
-                        symbol: "TDOL",
-                        holders_count: "5000",
-                    },
-                },
-            ],
-        };
-    }
-    if (url === EXPLORER + "/transactions/" + TX_HASH) {
-        return { to: { hash: TOKEN }, raw_input: TRANSFER_DATA };
-    }
-    return { items: [] };
-}
-
-function makeElement(id) {
-    const classes = new Set();
-    const el = {
-        id,
-        textContent: "",
-        value: "",
-        innerHTML: "",
-        style: {},
-        dataset: {},
-        classList: {
-            add: (...names) => names.forEach((n) => classes.add(n)),
-            remove: (...names) => names.forEach((n) => classes.delete(n)),
-            contains: (n) => classes.has(n),
-            toggle: () => false,
-        },
-        addEventListener: () => {},
-        querySelectorAll: () => [],
-        appendChild: () => {},
-    };
-    // Views reach for .parentElement to hide whole sections.
-    Object.defineProperty(el, "parentElement", {
-        get: () => node(id + "-parent"),
-    });
-    return el;
-}
-
-function makeDocument() {
-    const els = new Map();
-    return {
-        getElementById(id) {
-            // The debug banner is created on demand by helpers.js; absent
-            // is the state a non-debug, non-testnet popup is in.
-            if (id === "debug-banner") return null;
-            if (!els.has(id)) els.set(id, makeElement(id));
-            return els.get(id);
-        },
-        createElement: () => makeElement("created"),
-        body: { prepend: () => {} },
-    };
-}
-
-function node(id) {
-    return globalThis.document.getElementById(id);
-}
-
-// One line of the decoded details: its label and what is drawn under it.
-function decodedLine(label) {
-    return node("tx-detail-calldata-details")
-        .innerHTML.split('<div class="mb-2">')
-        .find((line) =>
-            line.startsWith(`<div class="text-muted">${label}</div>`),
-        );
-}
-
-beforeEach(() => {
-    globalThis.document = makeDocument();
-    globalThis.window = { location: { search: "" } };
-    state.blockscoutUrl = EXPLORER;
-    state.wallets = [
-        {
-            name: "Main",
-            type: "key",
-            addresses: [{ address: HOLDER, balance: "0.0000" }],
-        },
-    ];
-    state.trackedTokens = [{ address: TOKEN, symbol: "TDOL", decimals: 6 }];
-    state.viewData = {};
-    state.viewStack = [];
-    state.currentView = null;
-});
-
-test("a token transfer's decoded details name the token contract and the amount at its scale", async () => {
-    const [row] = await fetchRecentTransactions(HOLDER, EXPLORER, 1);
-    expect(row.to).toBe(RECIPIENT);
-    expect(row.contractAddress).toBe(TOKEN.toLowerCase());
-
-    transactionDetail.show(row);
-    // The decoded details are drawn once the explorer has answered.
-    for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
-
-    expect(node("tx-detail-calldata-action").textContent).toBe(
-        "Token Transfer",
-    );
-    const token = decodedLine("Token");
-    expect(token).toContain('<div class="font-bold">TDOL</div>');
-    expect(token).toContain(TOKEN);
-    expect(token).not.toContain(RECIPIENT);
-    expect(decodedLine("Recipient")).toContain(RECIPIENT);
-    expect(decodedLine("Amount")).toContain(
-        '<div class="font-bold">1234.5000 TDOL</div>',
-    );
-});

tests/walletDefects.test.js

@@ -153,6 +153,18 @@ describe("the explanatory copy", () => {
         expect(all).not.toMatch(/\blost\b|\bstolen\b|\bgone\b/i);
     });
 
+    // The wallet holds the imported key but will not export it, so the copy
+    // must name every key the user may hold that reaches the addresses
+    // outside it.
+    test("it names the keys that reach the addresses outside this wallet", () => {
+        const body = defect.paragraphs.join(" ");
+
+        expect(body).toMatch(/private key export are disabled/);
+        expect(body).toMatch(
+            /with the extended private key you imported, if you kept a copy of it, or with the master extended private key or recovery phrase it came from, in software that repeats the same non-standard derivation/,
+        );
+    });
+
     test("it says the wallet is not deleted and what the user can do", () => {
         const body = defect.paragraphs.join(" ");