fix: use the gas limit a site sends as gas (closes #207)

Sites send the gas limit under its JSON-RPC name, `gas`. ethers only
reads `gasLimit`, so the field was dropped and the node's estimate was
shown and signed instead. The page's request now takes `gas` as the gas
limit, and the existing ceiling, display and comparison with the signed
transaction cover it as they already did `gasLimit`. A request that
gives both with different values is refused.

Model: opus-5-5

README.md

@@ -239,8 +239,12 @@ provide:
   `make check` stays non-mutating. Linting runs only in the container, because
   an ESLint result that depends on whichever ESLint the host happens to have is
   not a result, so docker is required to lint.
-- `script/fmt` — format all files (writes), on the host
-- `script/fmt-check` — check formatting (read-only), on the host
+- `script/fmt` — format all files (writes), on the host, with the `prettier`
+  that `script/bootstrap` installs in `node_modules/`; when that one is missing
+  it fails and says to run `script/bootstrap`, instead of using a `prettier`
+  from the `PATH`
+- `script/fmt-check` — check formatting (read-only), on the host, with the same
+  `prettier` and the same refusal
 - `script/check` — run `script/test`, `script/lint` and `script/fmt-check`
 - `script/check-censored` — assert the competitor name RULES.md bars appears
   nowhere in the working tree or under `dist/` outside its documented
@@ -1188,14 +1192,7 @@ The core hierarchy is **Wallets → Addresses**:
       of its stored `xpub` by `src/shared/walletDefects.js`, explained in the
       wallet list, and blocked from signing, sending and private-key export. It
       is never deleted or rewritten.
-- An **address** holds ETH and ERC-20 tokens. In an HD or xprv wallet each
-  address record also stores `index`, the derivation index it was made at: 0 for
-  the first address, the index the import scan found it at, or `nextIndex` when
-  "+" added it. An address's `index` and its position in the wallet's list
-  differ once the scan has passed over an unused index or an address has been
-  removed. Sending, site transactions, site signatures and private-key export
-  derive the key at `index`, and refuse it unless it is that address's own key
-  (`getSignerForAddress()` in `src/shared/wallet.js`).
+- An **address** holds ETH and ERC-20 tokens.
 - The user can have multiple wallets, each with multiple addresses (HD) or a
   single address (key).
 
@@ -1521,9 +1518,6 @@ view would leave a wallet one click from deletion.
       change)
     - "Reveal" (wrong password) → full-sentence error on the error line, nothing
       revealed (no screen change)
-    - "Reveal" (correct password, but the key derived is not this address's own,
-      see Data Model) → full-sentence error saying so, nothing revealed (no
-      screen change)
     - "Back" → previous screen (AddressDetail)
     - Settings gear → **Settings**, whose "Back" goes to AddressDetail: leaving
       drops the address the screen was showing, so it also takes the screen off
@@ -2081,7 +2075,9 @@ view would leave a wallet one click from deletion.
   artifact can be compared with it field for field. A nonce the site supplies is
   ignored: the nonce is always the account's next nonce from the node, so a site
   cannot replace one of the user's pending transactions or leave this one stuck
-  behind a gap. A request that cannot be populated — unreachable node, reverting
+  behind a gap. A gas limit the site supplies, as `gas` or `gasLimit`, is used
+  in place of the node's estimate; a request giving both with different values
+  is refused. A request that cannot be populated — unreachable node, reverting
   gas estimate — opens no window and is failed back to the site. Only one
   transaction approval exists at a time: populating fixes the nonce, so a second
   `eth_sendTransaction` arriving while one is unanswered is refused with
@@ -2669,7 +2665,8 @@ plaintext list of anyone's domain names.
 When a dApp on a blocklisted domain requests a wallet connection, transaction
 approval, or signature, the approval popup displays a prominent red warning
 banner alerting the user. The domain checker matches exact hostnames and all
-parent domains (subdomain matching).
+parent domains (subdomain matching), and checks a hostname written with a
+trailing dot (`name.`) as `name`.
 
 #### Transaction Decoding
 

script/fmt

@@ -25,6 +25,13 @@ run_yarn() {
 
 main() {
     cd "$ROOT"
+    # Without the repo's own prettier, yarn runs whatever prettier is on
+    # the PATH, which may be a version that formats files differently.
+    if [ ! -x node_modules/.bin/prettier ]; then
+        echo "fmt: this repo's prettier is not installed in node_modules/," \
+            "so run script/bootstrap first." >&2
+        exit 1
+    fi
     echo "Formatting..."
     run_yarn run fmt
 }

script/fmt-check

@@ -26,6 +26,13 @@ run_yarn() {
 
 main() {
     cd "$ROOT"
+    # Without the repo's own prettier, yarn runs whatever prettier is on
+    # the PATH, which may be a version that formats files differently.
+    if [ ! -x node_modules/.bin/prettier ]; then
+        echo "fmt-check: this repo's prettier is not installed in" \
+            "node_modules/, so run script/bootstrap first." >&2
+        exit 1
+    fi
     echo "Checking formatting..."
     run_yarn run fmt-check
 }

src/background/index.js

@@ -633,8 +633,9 @@ function isExtensionSender(sender) {
 // had refused.
 //
 // TX and sign approvals do not decide here. They stay pending across a
-// disconnect — the user can reopen the toolbar popup — and are rejected by the
-// windows.onRemoved listener below.
+// disconnect. When their window closes, the windows.onRemoved listener below
+// rejects them, unless an approved attempt is still running, in which case
+// that attempt answers the page.
 runtime.onConnect.addListener((port) => {
     if (port.name.startsWith("approval:")) {
         const id = port.name.split(":")[1];
@@ -662,7 +663,9 @@ runtime.onConnect.addListener((port) => {
             const approval = pendingApprovals[id];
             if (approval) {
                 if (approval.type === "tx" || approval.type === "sign") {
-                    // Keep pending — user can reopen the toolbar popup
+                    // When its window closes, windows.onRemoved rejects it,
+                    // unless an approved attempt is still running, in which
+                    // case that attempt answers the page.
                     return;
                 }
                 settleApproval(id, { approved: false, remember: false });

src/popup/views/addWallet.js

@@ -184,12 +184,7 @@ async function importMnemonic(ctx) {
         encryptedSecret: encrypted,
         nextIndex: 1,
         addresses: [
-            {
-                address: firstAddress,
-                index: 0,
-                balance: "0.0000",
-                tokenBalances: [],
-            },
+            { address: firstAddress, balance: "0.0000", tokenBalances: [] },
         ],
     };
     state.wallets.push(wallet);
@@ -210,7 +205,6 @@ async function importMnemonic(ctx) {
     if (scan.addresses.length > 1) {
         wallet.addresses = scan.addresses.map((a) => ({
             address: a.address,
-            index: a.index,
             balance: "0.0000",
             tokenBalances: [],
         }));
@@ -305,12 +299,7 @@ async function importXprvKey(ctx) {
         encryptedSecret: encrypted,
         nextIndex: 1,
         addresses: [
-            {
-                address: firstAddress,
-                index: 0,
-                balance: "0.0000",
-                tokenBalances: [],
-            },
+            { address: firstAddress, balance: "0.0000", tokenBalances: [] },
         ],
     };
     state.wallets.push(wallet);
@@ -331,7 +320,6 @@ async function importXprvKey(ctx) {
     if (scan.addresses.length > 1) {
         wallet.addresses = scan.addresses.map((a) => ({
             address: a.address,
-            index: a.index,
             balance: "0.0000",
             tokenBalances: [],
         }));

src/popup/views/approval.js

@@ -867,15 +867,15 @@ function gateOnWalletDefect(errorId, buttonId, address) {
     return true;
 }
 
-// Locate the wallet and the stored address record owning an address. Returns
-// null when no wallet holds it. Approvals look up the address they were raised
-// for, not whichever address is active now: the approval named one account,
-// and signing with another is what verification refuses.
+// Locate the wallet and the address index owning an address. Returns null when
+// no wallet holds it. Approvals look up the address they were raised for, not
+// whichever address is active now: the approval named one account, and signing
+// with another is what verification refuses.
 function findWalletFor(address) {
     for (const wallet of state.wallets) {
         for (let i = 0; i < wallet.addresses.length; i++) {
             if (wallet.addresses[i].address === address) {
-                return { wallet, addr: wallet.addresses[i] };
+                return { wallet, addrIndex: i };
             }
         }
     }
@@ -1002,7 +1002,7 @@ function init(_ctx) {
         try {
             const signer = getSignerForAddress(
                 active.wallet,
-                active.addr,
+                active.addrIndex,
                 decryptedSecret,
             );
             // Sign the approved transaction exactly as it was displayed. The
@@ -1130,7 +1130,7 @@ function init(_ctx) {
         try {
             const signer = getSignerForAddress(
                 active.wallet,
-                active.addr,
+                active.addrIndex,
                 decryptedSecret,
             );
             const sp = pendingSignParams;

src/popup/views/confirmTx.js

@@ -583,7 +583,7 @@ function init(_ctx) {
         try {
             const signer = getSignerForAddress(
                 wallet,
-                wallet.addresses[state.selectedAddress],
+                state.selectedAddress,
                 decryptedSecret,
             );
             const provider = getProvider(state.rpcUrl, state.networkId);

src/popup/views/exportPrivkey.js

@@ -135,18 +135,7 @@ async function reveal() {
         // decrypt ran, the wipe has already happened, so the key is not
         // even derived, let alone written.
         if (!isCurrentReveal(generation)) return;
-        let signer;
-        try {
-            signer = getSignerForAddress(
-                wallet,
-                wallet.addresses[addressIndex],
-                secret,
-            );
-        } catch (e) {
-            // The password was right; it is the key that was refused.
-            showError("export-privkey-password-error", e.message);
-            return;
-        }
+        const signer = getSignerForAddress(wallet, addressIndex, secret);
         $("export-privkey-password").value = "";
         $("export-privkey-password-section").classList.add("hidden");
         $("export-privkey-value").textContent = signer.privateKey;

src/popup/views/home.js

@@ -354,7 +354,6 @@ function render(ctx) {
             );
             wallet.addresses.push({
                 address: newAddr,
-                index: wallet.nextIndex,
                 balance: "0.0000",
                 tokenBalances: [],
             });

src/shared/approvalTx.js

@@ -1,13 +1,14 @@
 // Preparation of the transaction an approval screen displays.
 //
-// A dApp's eth_sendTransaction normally fixes only `to`, `value` and `data`.
-// The nonce, the gas limit and the fees have to be filled in from the network
-// before anything can be signed, and whoever fills them in decides what the
-// user is shown. That work used to happen in the popup, after the user had
-// already approved: the numbers on the approval screen came from the popup and
-// were compared against nothing, so a compromised popup could display one fee
-// and sign another, and the ceilings in approvalVerify.js were all that stood
-// between the user and a fee that hands the validator the balance.
+// A dApp's eth_sendTransaction normally fixes `to`, `value` and `data`, and
+// sometimes the gas limit. The nonce, the fees, and the gas limit when the
+// site gives none have to be filled in from the network before anything can
+// be signed, and whoever fills them in decides what the user is shown. That
+// work used to happen in the popup, after the user had already approved: the
+// numbers on the approval screen came from the popup and were compared against
+// nothing, so a compromised popup could display one fee and sign another, and
+// the ceilings in approvalVerify.js were all that stood between the user and a
+// fee that hands the validator the balance.
 //
 // So it happens here instead, in the background, before the approval window is
 // opened. The background populates the transaction, shows that object, and
@@ -31,6 +32,7 @@ const {
     VoidSigner,
     accessListify,
     getAddress,
+    getBigInt,
     getBytes,
     hexlify,
     toQuantity,
@@ -47,10 +49,11 @@ const {
 // minutes long, which is not a wait anyone will sit through.
 const POPULATE_TIMEOUT_MS = 20000;
 
-// The request fields taken from the page. Anything else is dropped rather than
-// passed to ethers: the object is page-controlled, and a future ethers that
-// learns to carry a new transaction field must not start picking one up out of
-// it without this module knowing.
+// The request fields taken from the page, plus `gas`, which requestFrom()
+// takes as the gas limit. Anything else is dropped rather than passed to
+// ethers: the object is page-controlled, and a future ethers that learns to
+// carry a new transaction field must not start picking one up out of it
+// without this module knowing.
 //
 // The nonce is not taken from the page; it is always the account's next nonce
 // from the network. A page that chose it could replace one of the user's
@@ -84,6 +87,16 @@ function present(v) {
     return v !== null && v !== undefined && v !== "";
 }
 
+// Whether two quantities from the page are the same number. One that is not a
+// number matches nothing.
+function sameQuantity(a, b) {
+    try {
+        return getBigInt(a) === getBigInt(b);
+    } catch {
+        return false;
+    }
+}
+
 // These strings reach the user through the requesting page, so they are full
 // sentences even when the tail of one came from ethers or from the node.
 function sentence(text) {
@@ -111,6 +124,20 @@ function requestFrom(txParams, from) {
     for (const key of REQUEST_FIELDS) {
         if (present(txParams[key])) request[key] = txParams[key];
     }
+    // JSON-RPC calls the gas limit `gas`, and that is the name sites send.
+    // ethers only reads `gasLimit`: left as `gas` it would be dropped and an
+    // estimate signed in its place.
+    if (present(txParams.gas)) {
+        if (
+            present(request.gasLimit) &&
+            !sameQuantity(request.gasLimit, txParams.gas)
+        ) {
+            throw fail(
+                "The site asked for two different gas limits for this transaction.",
+            );
+        }
+        request.gasLimit = txParams.gas;
+    }
     if (
         present(request.type) &&
         !ALLOWED_TX_TYPES.includes(Number(request.type))

src/shared/phishingDomains.js

@@ -110,11 +110,14 @@ function hashListed(hash) {
  * Generate hostname variants for subdomain matching.
  * "sub.evil.com" yields ["sub.evil.com", "evil.com"].
  *
+ * A trailing dot is dropped: "evil.com." is the fully qualified spelling of
+ * "evil.com", and the browser loads the same site from either.
+ *
  * @param {string} hostname
  * @returns {string[]}
  */
 function hostnameVariants(hostname) {
-    const h = hostname.toLowerCase();
+    const h = hostname.toLowerCase().replace(/\.$/, "");
     const variants = [h];
     const parts = h.split(".");
     // Parent domains: a.b.c.d -> b.c.d, c.d

src/shared/wallet.js

@@ -110,21 +110,16 @@ function addressFromPrivateKey(key) {
     return w.address;
 }
 
-// The signer for `addr`, one of the wallet's stored address records. An HD or
-// xprv address is derived at its own `index`, never at its position in the
-// wallet's list: the two differ once an import scan passes over an unused
-// index, or once an address that is not the last is removed. Whatever the
-// wallet type, the key is refused unless it is the key of `addr.address`.
-function getSignerForAddress(walletData, addr, decryptedSecret) {
-    let signer;
+function getSignerForAddress(walletData, addrIndex, decryptedSecret) {
     if (walletData.type === "hd") {
         const node = HDNodeWallet.fromPhrase(
             decryptedSecret,
             "",
             BIP44_ETH_PATH,
         );
-        signer = node.deriveChild(addr.index);
-    } else if (walletData.type === "xprv") {
+        return node.deriveChild(addrIndex);
+    }
+    if (walletData.type === "xprv") {
         // Checked here rather than through masterXprvOrThrow so the message
         // fits the situation: nobody is importing anything at signing time,
         // and this wallet is already in storage. src/shared/walletDefects.js
@@ -142,18 +137,9 @@ function getSignerForAddress(walletData, addr, decryptedSecret) {
                     "is not a master key, so it cannot sign.",
             );
         }
-        signer = node.derivePath(BIP44_ETH_PATH).deriveChild(addr.index);
-    } else {
-        signer = new Wallet(decryptedSecret);
+        return node.derivePath(BIP44_ETH_PATH).deriveChild(addrIndex);
     }
-    if (signer.address.toLowerCase() !== addr.address.toLowerCase()) {
-        throw new Error(
-            "The key this wallet holds for " +
-                addr.address +
-                " belongs to a different address, so it was not used.",
-        );
-    }
-    return signer;
+    return new Wallet(decryptedSecret);
 }
 
 function isValidMnemonic(mnemonic) {

tests/addressKey.test.js

@@ -1,386 +0,0 @@
-// Sending, a site's signature request and private key export each use the key
-// of the address they name: the key at the index that address was derived at,
-// not the one at its position in the wallet's list. The two differ after an
-// import whose scan passed over an unused index, and after an address that is
-// not the last is removed (https://git.eeqj.de/sneak/AutistMask/issues/510).
-//
-// Driven through the real import, "+" button, address removal, confirmation
-// screen, signature approval screen and export screen, with the node, the vault
-// and the DOM stubbed.
-
-"use strict";
-
-// The addresses the stub node reports a balance for, and the signed
-// transactions it was handed.
-const mockNode = { funded: new Set(), broadcast: [] };
-
-jest.mock("ethers", () => {
-    const actual = jest.requireActual("ethers");
-    class StubProvider {
-        async getBalance(address) {
-            return mockNode.funded.has(address) ? 1n : 0n;
-        }
-        async getTransactionCount() {
-            return 0;
-        }
-        async estimateGas() {
-            return 21000n;
-        }
-        async getFeeData() {
-            return {
-                maxFeePerGas: 20000000000n,
-                maxPriorityFeePerGas: 1000000000n,
-                gasPrice: 10000000000n,
-            };
-        }
-        async getNetwork() {
-            return { chainId: 1n };
-        }
-        async getCode() {
-            return "0x";
-        }
-        async lookupAddress() {
-            return null;
-        }
-        async broadcastTransaction(signed) {
-            mockNode.broadcast.push(signed);
-            return { hash: "0x" + "ab".repeat(32) };
-        }
-    }
-    return { ...actual, JsonRpcProvider: StubProvider };
-});
-
-// No password is hashed: what the wallet stores is the phrase itself.
-jest.mock("../src/shared/vault", () => ({
-    encryptWithPassword: async (secret) => secret,
-    decryptWithPassword: async (secret) => secret,
-}));
-
-// The wait screen polls for a receipt; these tests stop at the broadcast.
-jest.mock("../src/popup/views/txStatus", () => ({
-    showWait: jest.fn(),
-    showError: jest.fn(),
-}));
-
-jest.mock("ethereum-blockies-base64", () => () => "data:image/png;base64,x");
-
-global.fetch = jest.fn(() => {
-    throw new Error("tests must not perform network requests");
-});
-
-const {
-    HDNodeWallet,
-    Transaction,
-    getBytes,
-    verifyMessage,
-} = require("ethers");
-
-// Publicly known development phrase; never fund it.
-const PHRASE = "test test test test test test test test test test test junk";
-const PASSWORD = "correct horse battery staple";
-const RECIPIENT = "0xC0FfEE0000000000000000000000000000c0fFEe";
-// "Hello", as the hex a page sends.
-const MESSAGE = "0x48656c6c6f";
-
-// The address and key at derivation index `index` of PHRASE, worked out with
-// ethers directly rather than through the wallet's own code.
-function derived(index) {
-    return HDNodeWallet.fromPhrase(PHRASE, "", "m/44'/60'/0'/0/" + index);
-}
-
-// ------------------------------------------------------------ DOM stub
-
-function makeElement(id) {
-    const classes = new Set();
-    const el = {
-        id,
-        textContent: "",
-        title: "",
-        value: "",
-        innerHTML: "",
-        disabled: false,
-        style: {},
-        dataset: {},
-        listeners: {},
-        classList: {
-            add: (...names) => names.forEach((n) => classes.add(n)),
-            remove: (...names) => names.forEach((n) => classes.delete(n)),
-            contains: (n) => classes.has(n),
-            toggle: (n, force) => {
-                const on = force === undefined ? !classes.has(n) : force;
-                if (on) classes.add(n);
-                else classes.delete(n);
-                return on;
-            },
-        },
-        addEventListener: (name, fn) => {
-            el.listeners[name] = el.listeners[name] || [];
-            el.listeners[name].push(fn);
-        },
-        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"),
-        addEventListener: () => {},
-        body: { prepend: () => {} },
-    };
-}
-
-function node(id) {
-    return globalThis.document.getElementById(id);
-}
-
-function click(element) {
-    const event = { stopPropagation: () => {} };
-    return Promise.all((element.listeners.click || []).map((fn) => fn(event)));
-}
-
-// show() starts the fee estimate without awaiting it; this lets it settle.
-async function settle() {
-    for (let i = 0; i < 10; i++) await new Promise((r) => setTimeout(r, 0));
-}
-
-// ------------------------------------------------------------ harness
-
-globalThis.chrome = {
-    storage: { local: { get: async () => ({}), set: async () => {} } },
-};
-
-const { state } = require("../src/shared/state");
-const { removeAddressFromState } = require("../src/shared/walletDelete");
-const addWallet = require("../src/popup/views/addWallet");
-const home = require("../src/popup/views/home");
-const confirmTx = require("../src/popup/views/confirmTx");
-const approval = require("../src/popup/views/approval");
-const exportPrivkey = require("../src/popup/views/exportPrivkey");
-const txStatus = require("../src/popup/views/txStatus");
-
-// What the background answers when the approval window asks which approval it
-// was opened for, and every message the popup sent it.
-let approvalDetails;
-let sent;
-
-// The "+" button of the wallet list's last render. Each render attaches the
-// handler to the buttons it drew, as the browser's fresh list would have.
-let plusButton;
-
-beforeEach(() => {
-    approvalDetails = null;
-    sent = [];
-    mockNode.broadcast = [];
-    globalThis.document = makeDocument();
-    globalThis.window = { close: () => {} };
-    globalThis.chrome = {
-        storage: { local: { get: async () => ({}), set: async () => {} } },
-        runtime: {
-            connect: () => ({ postMessage: () => {} }),
-            sendMessage: (msg, reply) => {
-                sent.push(msg);
-                if (!reply) return;
-                reply(
-                    msg.type === "AUTISTMASK_GET_APPROVAL"
-                        ? approvalDetails
-                        : null,
-                );
-            },
-        },
-    };
-    node("wallet-list").querySelectorAll = (selector) => {
-        if (selector !== ".btn-add-address") return [];
-        plusButton = makeElement("btn-add-address");
-        plusButton.dataset.wallet = "0";
-        return [plusButton];
-    };
-    // The explorer and price lookups these screens start fail offline, and
-    // say so; that is not under test.
-    for (const method of ["log", "warn", "error"]) {
-        jest.spyOn(console, method).mockImplementation(() => {});
-    }
-    txStatus.showError.mockClear();
-
-    state.wallets = [];
-    state.rpcUrl = "https://rpc.example.invalid";
-    state.networkId = "mainnet";
-    state.viewStack = [];
-    state.currentView = "main";
-
-    addWallet.init({
-        renderWalletList: () => {},
-        doRefreshAndRender: () => {},
-        pageClosed: new AbortController().signal,
-    });
-    confirmTx.init({});
-    approval.init({});
-    exportPrivkey.init();
-});
-
-afterEach(() => {
-    jest.restoreAllMocks();
-});
-
-// Import PHRASE on the add-wallet screen, with the stub node reporting a
-// balance at each of `fundedIndices`, and wait for the address scan.
-async function importPhrase(fundedIndices) {
-    mockNode.funded = new Set(fundedIndices.map((i) => derived(i).address));
-    await click(node("tab-mnemonic"));
-    node("wallet-mnemonic").value = PHRASE;
-    node("add-wallet-password").value = PASSWORD;
-    node("add-wallet-password-confirm").value = PASSWORD;
-    await click(node("btn-add-wallet-confirm"));
-}
-
-// Press the wallet's "+" button on Home.
-async function addAddress() {
-    home.render({
-        doRefreshAndRender: () => {},
-        pageClosed: new AbortController().signal,
-    });
-    await click(plusButton);
-}
-
-// The key the export screen reveals for the address at `position`.
-async function exportedKey(position) {
-    exportPrivkey.show(0, position);
-    node("export-privkey-password").value = PASSWORD;
-    await click(node("btn-export-privkey-confirm"));
-    return node("export-privkey-value").textContent;
-}
-
-// Send from the address at `position`; resolves to the signed transaction the
-// node was handed, or undefined when none was.
-async function sendFrom(position) {
-    state.selectedWallet = 0;
-    state.selectedAddress = position;
-    confirmTx.show({
-        from: state.wallets[0].addresses[position].address,
-        to: RECIPIENT,
-        amount: "0.5",
-        token: "ETH",
-        balance: "1.0",
-    });
-    await settle();
-    node("confirm-tx-password").value = PASSWORD;
-    await click(node("btn-confirm-send"));
-    return mockNode.broadcast.pop();
-}
-
-// A site asks the address at `position` to sign MESSAGE; resolves to the
-// response the approval window sent the background.
-async function signFor(position) {
-    const address = state.wallets[0].addresses[position].address;
-    approvalDetails = {
-        type: "sign",
-        origin: "https://dapp.example",
-        isPhishingDomain: false,
-        approvedFrom: address,
-        signParams: {
-            method: "personal_sign",
-            message: MESSAGE,
-            from: address,
-        },
-    };
-    await approval.show(1);
-    node("approve-sign-password").value = PASSWORD;
-    await click(node("btn-approve-sign"));
-    return sent.find((msg) => msg.type === "AUTISTMASK_SIGN_RESPONSE");
-}
-
-describe.each([
-    [
-        "an import whose scan passed over index 3",
-        async () => {
-            await importPhrase([0, 1, 2, 4]);
-        },
-        // Address 4 on Home, derived at index 4.
-        { position: 3, index: 4 },
-    ],
-    [
-        "removing the middle one of three addresses",
-        async () => {
-            await importPhrase([0]);
-            await addAddress();
-            await addAddress();
-            removeAddressFromState(state, 0, 1);
-        },
-        // Address 2 on Home after the removal, derived at index 2.
-        { position: 1, index: 2 },
-    ],
-])("after %s", (_name, setUp, { position, index }) => {
-    beforeEach(setUp);
-
-    test("the wallet lists the address derived at that index there", () => {
-        expect(state.wallets[0].addresses[position].address).toBe(
-            derived(index).address,
-        );
-    });
-
-    test("export reveals that address's key", async () => {
-        expect(await exportedKey(position)).toBe(derived(index).privateKey);
-    });
-
-    test("a send is signed by that address", async () => {
-        const signed = await sendFrom(position);
-        expect(Transaction.from(signed).from).toBe(derived(index).address);
-    });
-
-    test("a site's signature is made by that address", async () => {
-        const response = await signFor(position);
-        expect(verifyMessage(getBytes(MESSAGE), response.signature)).toBe(
-            derived(index).address,
-        );
-    });
-});
-
-describe("an address whose stored index derives a different address", () => {
-    function refusal() {
-        return (
-            "The key this wallet holds for " +
-            derived(0).address +
-            " belongs to a different address, so it was not used."
-        );
-    }
-
-    beforeEach(async () => {
-        await importPhrase([0]);
-        state.wallets[0].addresses[0].index = 1;
-    });
-
-    test("export reveals nothing and says why", async () => {
-        expect(await exportedKey(0)).toBe("");
-        expect(node("export-privkey-password-error").textContent).toBe(
-            refusal(),
-        );
-    });
-
-    test("nothing is sent", async () => {
-        expect(await sendFrom(0)).toBeUndefined();
-        expect(txStatus.showError).toHaveBeenCalledWith(
-            expect.anything(),
-            null,
-            refusal(),
-        );
-    });
-
-    test("nothing is signed", async () => {
-        const response = await signFor(0);
-        expect(response.signature).toBeUndefined();
-        expect(response.error).toBe(refusal());
-    });
-});

tests/approvalNonce.test.js

@@ -136,9 +136,7 @@ async function openTxApproval(approvedTx) {
             xpub: "xpub-wallet-1",
             encryptedSecret: "encrypted-secret-1",
             nextIndex: 1,
-            addresses: [
-                { address: FROM, index: 0, balance: "0", tokenBalances: [] },
-            ],
+            addresses: [{ address: FROM, balance: "0", tokenBalances: [] }],
         },
     ];
     approval.init({});

tests/approvalTx.test.js

@@ -9,7 +9,7 @@
 // requesting page or the RPC node can say turns it into an approval that
 // should never have been raised.
 
-const { Network, Wallet } = require("ethers");
+const { Network, Transaction, Wallet, toQuantity } = require("ethers");
 const {
     prepareApprovalTx,
     serializeApprovedTx,
@@ -138,6 +138,43 @@ describe("prepareApprovalTx", () => {
         expect(approved.maxFeePerGas).toBe("0x12a05f200");
     });
 
+    // `gas` is the JSON-RPC name for the gas limit and the one sites send.
+    // The node would estimate 21000; the site's 200000 is what is shown and
+    // what is signed.
+    test("uses the gas limit a site sends as gas", async () => {
+        const approved = await prepareApprovalTx(
+            providerWith(),
+            signer.address,
+            { ...TX_PARAMS, gas: "0x30d40" },
+        );
+        expect(approved.gasLimit).toBe("0x30d40");
+        const signed = Transaction.from(
+            await signer.signTransaction({ ...approved }),
+        );
+        expect(signed.gasLimit).toBe(200000n);
+    });
+
+    test("accepts gas and gasLimit that name the same number", async () => {
+        const approved = await prepareApprovalTx(
+            providerWith(),
+            signer.address,
+            { ...TX_PARAMS, gas: "0x30d40", gasLimit: 200000 },
+        );
+        expect(approved.gasLimit).toBe("0x30d40");
+    });
+
+    test("refuses gas and gasLimit that disagree", async () => {
+        await expect(
+            prepareApprovalTx(providerWith(), signer.address, {
+                ...TX_PARAMS,
+                gas: "0x30d40",
+                gasLimit: "0x5208",
+            }),
+        ).rejects.toThrow(
+            "The site asked for two different gas limits for this transaction.",
+        );
+    });
+
     test("carries an access list the request asked for", async () => {
         const approved = await prepareApprovalTx(
             providerWith(),
@@ -212,6 +249,15 @@ describe("prepareApprovalTx", () => {
         ).rejects.toThrow(/gas limit no network this wallet supports/);
     });
 
+    test("refuses a gas limit the site sent above the ceiling", async () => {
+        await expect(
+            prepareApprovalTx(providerWith(), signer.address, {
+                ...TX_PARAMS,
+                gas: toQuantity(MAX_GAS_LIMIT + 1n),
+            }),
+        ).rejects.toThrow(/gas limit no network this wallet supports/);
+    });
+
     // The combined bound at population: a gas limit and a fee that are each
     // under their own ceiling but multiply to thousands of ETH is refused
     // before the approval window opens, so the user is never shown a

tests/approvalVerify.test.js

@@ -58,7 +58,6 @@ const TX_PARAMS = {
     to: RECIPIENT,
     value: "0x2386f26fc10000",
     data: "0xdeadbeef",
-    gas: "0x5208",
 };
 
 // The values populateTransaction() fills in when the dApp fixed none of them.
@@ -1455,11 +1454,7 @@ describe("background population to popup signing to verification", () => {
 
     // What the popup does with it: signs it as given, populating nothing.
     async function popupSigns(approvedTx) {
-        const localSigner = getSignerForAddress(
-            walletData,
-            { address: signer.address },
-            SIGNER_KEY,
-        );
+        const localSigner = getSignerForAddress(walletData, 0, SIGNER_KEY);
         return localSigner.signTransaction({ ...approvedTx });
     }
 
@@ -1554,11 +1549,7 @@ describe("background population to popup signing to verification", () => {
     // address the user has switched to.
     test("the approved from stops the popup signing with another key", async () => {
         const approvedTx = await backgroundPrepares(TX_PARAMS);
-        const otherSigner = getSignerForAddress(
-            walletData,
-            { address: other.address },
-            OTHER_KEY,
-        );
+        const otherSigner = getSignerForAddress(walletData, 0, OTHER_KEY);
         await expect(
             otherSigner.signTransaction({ ...approvedTx }),
         ).rejects.toThrow(/from address mismatch/);

tests/phishingDomains.test.js

@@ -89,6 +89,12 @@ describe("vendored blocklist", () => {
         expect(isPhishingDomain(LISTED[0].toUpperCase())).toBe(true);
     });
 
+    test("a hostname ending in a dot is the same host", () => {
+        expect(isPhishingDomain(LISTED[1] + ".")).toBe(true);
+        expect(isPhishingDomain("wallet." + LISTED[1] + ".")).toBe(true);
+        expect(isPhishingDomain(CLEAN[0] + ".")).toBe(false);
+    });
+
     test("returns false for an empty or missing hostname", () => {
         expect(isPhishingDomain("")).toBe(false);
         expect(isPhishingDomain(null)).toBe(false);
@@ -133,6 +139,13 @@ describe("hostnameVariants", () => {
     test("lowercases hostnames", () => {
         expect(hostnameVariants("Evil.COM")).toEqual(["evil.com"]);
     });
+
+    test("drops the trailing dot of a fully qualified hostname", () => {
+        expect(hostnameVariants("sub.evil.com.")).toEqual([
+            "sub.evil.com",
+            "evil.com",
+        ]);
+    });
 });
 
 describe("domain hashing", () => {

tests/wallet.test.js

@@ -416,7 +416,7 @@ describe("extended key depth", () => {
         expect(() =>
             wallet.getSignerForAddress(
                 { type: "xprv" },
-                { address: VECTOR_ADDRESSES[0], index: 0 },
+                0,
                 accountXprv(VECTOR_PHRASE),
             ),
         ).toThrow(/master/i);
@@ -504,7 +504,7 @@ describe("getSignerForAddress", () => {
     test.each([0, 1, 2])("hd wallet, address index %i", (index) => {
         const signer = wallet.getSignerForAddress(
             { type: "hd" },
-            { address: VECTOR_ADDRESSES[index], index },
+            index,
             VECTOR_PHRASE,
         );
 
@@ -515,7 +515,7 @@ describe("getSignerForAddress", () => {
     test.each([0, 1, 2])("xprv wallet, address index %i", (index) => {
         const signer = wallet.getSignerForAddress(
             { type: "xprv" },
-            { address: VECTOR_ADDRESSES[index], index },
+            index,
             masterXprv(VECTOR_PHRASE),
         );
 
@@ -527,7 +527,7 @@ describe("getSignerForAddress", () => {
         for (const index of [0, 1, 2]) {
             const signer = wallet.getSignerForAddress(
                 { type: "privkey" },
-                { address: VECTOR_ADDRESSES[1], index },
+                index,
                 VECTOR_PRIVATE_KEYS[1],
             );
 
@@ -535,33 +535,10 @@ describe("getSignerForAddress", () => {
         }
     });
 
-    // https://git.eeqj.de/sneak/AutistMask/issues/510: an address whose stored
-    // index does not derive it, or a private key that is not its key.
-    test.each([
-        ["hd", { address: VECTOR_ADDRESSES[2], index: 1 }, VECTOR_PHRASE],
-        [
-            "xprv",
-            { address: VECTOR_ADDRESSES[2], index: 1 },
-            masterXprv(VECTOR_PHRASE),
-        ],
-        ["privkey", { address: VECTOR_ADDRESSES[2] }, VECTOR_PRIVATE_KEYS[1]],
-    ])(
-        "%s wallet refuses a key that is not the address's own",
-        (type, addr, secret) => {
-            expect(() =>
-                wallet.getSignerForAddress({ type }, addr, secret),
-            ).toThrow(
-                "The key this wallet holds for " +
-                    VECTOR_ADDRESSES[2] +
-                    " belongs to a different address, so it was not used.",
-            );
-        },
-    );
-
     test("the returned signer signs recoverably as the expected address", async () => {
         const signer = wallet.getSignerForAddress(
             { type: "hd" },
-            { address: VECTOR_ADDRESSES[1], index: 1 },
+            1,
             VECTOR_PHRASE,
         );
         const message = "AutistMask derivation test";

tests/walletDefects.test.js

@@ -55,7 +55,7 @@ function healthyXprvWallet(name = "Master") {
         xpub,
         nextIndex: 1,
         encryptedSecret: "irrelevant-to-these-tests",
-        addresses: [{ address: firstAddress, index: 0, balance: "0.0000" }],
+        addresses: [{ address: firstAddress, balance: "0.0000" }],
     };
 }
 
@@ -558,7 +558,7 @@ describe("no path throws an unhandled error for a defective wallet", () => {
         try {
             wallet.getSignerForAddress(
                 { type: "xprv" },
-                brokenXprvWallet().addresses[0],
+                0,
                 accountXprv(VECTOR_PHRASE),
             );
         } catch (e) {
@@ -573,7 +573,7 @@ describe("no path throws an unhandled error for a defective wallet", () => {
     test("a healthy xprv wallet signs as it always did", () => {
         const signer = wallet.getSignerForAddress(
             { type: "xprv" },
-            healthyXprvWallet().addresses[0],
+            0,
             masterXprv(VECTOR_PHRASE),
         );