fix: estimate a request even when the site sends its gas limit (closes #554)

ethers asks the node for a gas estimate only when the request has no gas
limit, and that estimate is the wallet's only check that a transaction
does not revert. Since a site's `gas` became the gas limit, a reverting
request carrying one opened the approval window and could be signed and
mined to fail. Such a request is now estimated first, with the site's
limit and under the same timeout; a failure goes back to the site with no
window, and the site's gas limit is still the one shown and signed. A
limit too low for the call now fails the same way.

Model: opus-5-5

README.md

@@ -1192,16 +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;
-  an HD or xprv address with no valid `index` is refused before anything is
-  derived (`getSignerForAddress()` in `src/shared/wallet.js`). Each refusal is a
-  full sentence naming the address.
+- An **address** holds ETH and ERC-20 tokens.
 - The user can have multiple wallets, each with multiple addresses (HD) or a
   single address (key).
 
@@ -1527,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 this address's key is refused, see Data
-      Model) → full-sentence error saying why, 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
@@ -1659,13 +1647,6 @@ view would leave a wallet one click from deletion.
       value (`src/shared/transferAmount.js`)
     - "Sign & Send" (wrong password) → "Wrong password." on the password error
       line, no screen change
-    - "Sign & Send" signs as the From address above, not as whichever address is
-      selected: the two are saved separately, so the popup can reopen onto this
-      screen with another address selected. When the selected wallet does not
-      hold the From address → full-sentence error on the password error line,
-      nothing sent (no screen change)
-    - "Sign & Send" (correct password, but the From address's key is refused,
-      see Data Model) → nothing is signed → **ErrorTx** saying why
     - "Back" → **Send**
 
 #### WaitTx (`wait-tx`)
@@ -2094,7 +2075,11 @@ 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 the
+  one shown and signed in place of the node's estimate, but the node still
+  estimates the transaction with that limit, as the check that it neither
+  reverts nor runs out of gas; a request that gives 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
@@ -2129,9 +2114,6 @@ view would leave a wallet one click from deletion.
       shown, exactly as shown, hands the signed transaction to the background to
       broadcast, then → **WaitTx** in the same popup window
     - "Confirm" (wrong password) → error line, no screen change
-    - "Confirm" (correct password, but the From address's key is refused, see
-      Data Model) → full-sentence error on the error line saying why, nothing
-      signed (no screen change)
     - "Reject" → closes popup (returns rejection to background)
     - Popup window closed without answering → the request is rejected with
       EIP-1193 code 4001

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

@@ -547,19 +547,6 @@ function init(_ctx) {
             return;
         }
 
-        // Sign as the From address this screen shows. The selection is saved
-        // apart from this screen, so the popup can reopen onto it with another
-        // address selected.
-        const addr = wallet.addresses.find((a) => a.address === pendingTx.from);
-        if (!addr) {
-            showError(
-                "confirm-tx-password-error",
-                "The selected wallet does not hold the From address above, " +
-                    "so nothing was sent. Please go back and send again.",
-            );
-            return;
-        }
-
         const password = $("confirm-tx-password").value;
         if (!password) {
             showError(
@@ -594,7 +581,11 @@ function init(_ctx) {
 
         let tx;
         try {
-            const signer = getSignerForAddress(wallet, addr, decryptedSecret);
+            const signer = getSignerForAddress(
+                wallet,
+                state.selectedAddress,
+                decryptedSecret,
+            );
             const provider = getProvider(state.rpcUrl, state.networkId);
             const connectedSigner = signer.connect(provider);
 

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))
@@ -174,6 +201,7 @@ async function prepareApprovalTx(provider, from, txParams) {
         throw fail("There is no active address to send this transaction from.");
     }
     const request = requestFrom(txParams || {}, from);
+    const signer = new VoidSigner(getAddress(from), provider);
 
     let populated;
     try {
@@ -181,10 +209,20 @@ async function prepareApprovalTx(provider, from, txParams) {
         // nonce, gas, fee and chain id are populated exactly as they were when
         // the popup did this. VoidSigner cannot sign, which is the point: the
         // background prepares, the popup signs.
+        //
+        // ethers asks the node for a gas estimate only when the request has
+        // no gas limit, and that estimate is the only check that the
+        // transaction does not revert. So a request carrying the site's gas
+        // limit is estimated first, with that limit: one that reverts, or
+        // needs more gas than the site gave it, fails here as well. The
+        // site's gas limit is still the one populated.
         populated = await withTimeout(
-            new VoidSigner(getAddress(from), provider).populateTransaction(
-                request,
-            ),
+            (async () => {
+                if (present(request.gasLimit)) {
+                    await signer.estimateGas(request);
+                }
+                return signer.populateTransaction(request);
+            })(),
             POPULATE_TIMEOUT_MS,
             "The transaction could not be prepared: the network did not answer in time.",
         );

src/shared/wallet.js

@@ -110,33 +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. An HD or xprv
-// record without a valid `index` is refused before anything is derived.
-// Whatever the wallet type, the key is refused unless it is the key of
-// `addr.address`.
-function getSignerForAddress(walletData, addr, decryptedSecret) {
-    if (
-        (walletData.type === "hd" || walletData.type === "xprv") &&
-        !(Number.isInteger(addr.index) && addr.index >= 0)
-    ) {
-        throw new Error(
-            "No derivation index is stored for " +
-                addr.address +
-                ", so its key cannot be derived.",
-        );
-    }
-    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
@@ -154,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,439 +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).
-// A send is signed by the From address its confirmation screen shows, whichever
-// address is selected.
-//
-// 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,
-    Mnemonic,
-    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";
-// PHRASE's master extended private key, as the xprv import takes it.
-const XPRV = HDNodeWallet.fromSeed(
-    Mnemonic.fromPhrase(PHRASE).computeSeed(),
-).extendedKey;
-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"));
-}
-
-// The same, importing XPRV on the add-wallet screen's xprv tab.
-async function importXprv(fundedIndices) {
-    mockNode.funded = new Set(fundedIndices.map((i) => derived(i).address));
-    await click(node("tab-xprv"));
-    node("import-xprv-key").value = XPRV;
-    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;
-}
-
-// Confirm a send whose From line shows `from`, with the address at position
-// `selected` selected; resolves to the signed transaction the node was handed,
-// or undefined when none was.
-async function confirmSend(from, selected) {
-    state.selectedWallet = 0;
-    state.selectedAddress = selected;
-    confirmTx.show({
-        from,
-        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();
-}
-
-// Send from the address at `position`, with that address selected, as the Send
-// button leaves it.
-function sendFrom(position) {
-    return confirmSend(state.wallets[0].addresses[position].address, position);
-}
-
-// 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 },
-    ],
-    [
-        "an xprv import whose scan passed over index 3",
-        async () => {
-            await importXprv([0, 1, 2, 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,
-        );
-    });
-});
-
-// The selection and the confirmation screen are saved apart, so the popup can
-// reopen onto the screen with another address selected than its From line
-// shows.
-describe("a confirmation screen whose From is not the selected address", () => {
-    beforeEach(async () => {
-        await importPhrase([0, 1]);
-    });
-
-    test("the send is signed by the From address shown", async () => {
-        const signed = await confirmSend(derived(1).address, 0);
-        expect(Transaction.from(signed).from).toBe(derived(1).address);
-    });
-
-    test("nothing is sent when the selected wallet does not hold it", async () => {
-        expect(await confirmSend(derived(5).address, 0)).toBeUndefined();
-        expect(node("confirm-tx-password-error").textContent).toBe(
-            "The selected wallet does not hold the From address above, " +
-                "so nothing was sent. Please go back and send again.",
-        );
-    });
-});
-
-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,49 @@ describe("prepareApprovalTx", () => {
         expect(approved.maxFeePerGas).toBe("0x12a05f200");
     });
 
+    // `gas` is the JSON-RPC name for the gas limit and the one sites send.
+    // The node is still asked for its estimate, with the site's gas limit,
+    // because the estimate is the check that the transaction does not revert.
+    // It estimates 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 estimateGas = jest.fn(async () => 21000n);
+        const approved = await prepareApprovalTx(
+            providerWith({ estimateGas }),
+            signer.address,
+            { ...TX_PARAMS, gas: "0x30d40" },
+        );
+        expect(estimateGas).toHaveBeenCalledWith(
+            expect.objectContaining({ gasLimit: 200000n }),
+        );
+        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 +255,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
@@ -251,6 +303,22 @@ describe("prepareApprovalTx", () => {
         expect(thrown.message).toMatch(/^[A-Z].*\.$/);
     });
 
+    test("fails back a reverting request that sends its own gas limit", async () => {
+        const reverting = providerWith({
+            estimateGas: async () => {
+                throw new Error("execution reverted: ERC20: transfer amount");
+            },
+        });
+        await expect(
+            prepareApprovalTx(reverting, signer.address, {
+                ...TX_PARAMS,
+                gas: "0x30d40",
+            }),
+        ).rejects.toThrow(
+            /^The transaction could not be prepared: execution reverted/,
+        );
+    });
+
     // Without a bound, an unreachable node leaves the page's promise pending
     // with nothing on screen to explain it.
     test("gives up on a node that never answers", async () => {

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/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,54 +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.",
-            );
-        },
-    );
-
-    // An HD or xprv address record without a usable `index` is refused in a
-    // sentence, not with what ethers says about the value it was handed.
-    test.each([
-        ["hd", VECTOR_PHRASE],
-        ["xprv", masterXprv(VECTOR_PHRASE)],
-    ])("%s wallet refuses an address with no valid index", (type, secret) => {
-        for (const index of [undefined, null, -1, 1.5, "1"]) {
-            expect(() =>
-                wallet.getSignerForAddress(
-                    { type },
-                    { address: VECTOR_ADDRESSES[1], index },
-                    secret,
-                ),
-            ).toThrow(
-                "No derivation index is stored for " +
-                    VECTOR_ADDRESSES[1] +
-                    ", so its key cannot be derived.",
-            );
-        }
-    });
-
     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" }],
     };
 }
 
@@ -74,11 +74,7 @@ function brokenXprvWallet(name = "Imported xprv") {
         nextIndex: 1,
         encryptedSecret: "irrelevant-to-these-tests",
         addresses: [
-            {
-                address: node.deriveChild(0).address,
-                index: 0,
-                balance: "0.0000",
-            },
+            { address: node.deriveChild(0).address, balance: "0.0000" },
         ],
     };
 }
@@ -574,7 +570,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) {
@@ -589,7 +585,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),
         );