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

@@ -2071,7 +2071,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

TODO.md

@@ -2,9 +2,10 @@
 
 - `git pull` `next` and cut a branch from it — one branch per issue, named
   `issue-<N>-<slug>`. Never branch from `main`.
-- Do the work as one commit whose title ends with ` (closes #N)`.
-- Add no entry to Completed Steps. What landed is recorded on the tracker and in
-  `git log`, where each commit title names the issue it closes.
+- Do the work as one commit whose title ends with ` (closes #N)`, with the
+  `TODO.md` update in that same commit.
+- Move Next Step to the top of Completed Steps; move the top item of Future
+  Steps into Next Step.
 - Run `make fmt`, then `make check`. A feature branch may be red; `next` and
   `main` may not.
 - Rebase onto current `next` immediately before pushing — other branches land on

src/shared/approvalTx.js

@@ -31,6 +31,7 @@ const {
     VoidSigner,
     accessListify,
     getAddress,
+    getBigInt,
     getBytes,
     hexlify,
     toQuantity,
@@ -47,10 +48,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 +86,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 +123,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/walletDefects.js

@@ -36,12 +36,11 @@ 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, sending and private key export are disabled for this " +
-                "wallet. Anything these addresses hold can be reached only " +
-                "with the extended private key you imported, if you kept a " +
-                "copy of it, in software that repeats the same non-standard " +
-                "derivation. Check the addresses in a block explorer before " +
-                "deciding what to do.",
+            "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.",
             "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/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/walletDefects.test.js

@@ -153,18 +153,6 @@ 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 not send the user to other software without saying they need
-    // their own copy of that key.
-    test("it says only the user's own copy of the imported key reaches the addresses", () => {
-        const body = defect.paragraphs.join(" ");
-
-        expect(body).toMatch(/private key export are disabled/);
-        expect(body).toMatch(
-            /only with the extended private key you imported, if you kept a copy of it/,
-        );
-    });
-
     test("it says the wallet is not deleted and what the user can do", () => {
         const body = defect.paragraphs.join(" ");