fix: failed requests reach the page with an error code and the node's data (closes #533)

A page's request that the node refuses now reaches it with the node's own JSON-RPC code and data: a proxied read such as a reverting eth_call, a transaction whose gas estimate reverts, and a failed broadcast. Every other refusal carries a code too: 4100 when there is no address, -32602 when the site asks for two different gas limits or a transaction type this wallet does not sign, and -32603 for a failure inside the wallet. nodeError() in approvalVerify.js reads the node's error out of what ethers throws, as failureTexts() already did inline. Model: opus-5-5
2026-10-18 18:41:10 +00:00
parent e48308c5b6
commit e2b50ad4a9
6 changed files with 390 additions and 30 deletions
@@ -1161,9 +1161,12 @@ describe("a nonce collision is reported as a transaction that did not go out", (
        const pending = bg.requestTx();
        await settle();

+        // As ethers throws it: its own classification, and the node's
+        // JSON-RPC error under info.error.
        bg.broadcastTransaction.mockRejectedValue(
            Object.assign(new Error("nonce too low"), {
                code: "NONCE_EXPIRED",
+                info: { error: { code: -32000, message: "nonce too low" } },
            }),
        );
        const answer = bg.send(
@@ -1182,8 +1185,10 @@ describe("a nonce collision is reported as a transaction that did not go out", (
            retryable: false,
            stage: "nonce",
        });
+        // The wallet's sentence, with the node's own code.
        expect(pending.result()).toEqual({
            error: {
+                code: -32000,
                message: expect.stringMatching(
                    /transaction was not sent, because its nonce/,
                ),
@@ -1233,6 +1238,7 @@ describe("a nonce collision is reported as a transaction that did not go out", (
        });
        expect(second.result()).toEqual({
            error: {
+                code: -32603,
                message: expect.stringMatching(/nonce had already been used/),
            },
        });
@@ -1323,6 +1329,7 @@ describe("what the approval is verified against", () => {
        );
        expect(pending.result()).toEqual({
            error: {
+                code: -32603,
                message: expect.stringMatching(/approved maximum fee per gas/),
            },
        });
@@ -1382,7 +1389,10 @@ describe("what the approval is verified against", () => {
            expect.objectContaining({ retryable: false, stage: "verify" }),
        );
        expect(pending.result()).toEqual({
-            error: { message: expect.stringMatching(/active address changed/) },
+            error: {
+                code: -32603,
+                message: expect.stringMatching(/active address changed/),
+            },
        });
    });

@@ -1526,7 +1536,10 @@ describe("what the approval is verified against", () => {
            }),
        );
        expect(pending.result()).toEqual({
-            error: { message: expect.stringMatching(/active address changed/) },
+            error: {
+                code: -32603,
+                message: expect.stringMatching(/active address changed/),
+            },
        });
    });

@@ -1576,7 +1589,9 @@ describe("what the approval is verified against", () => {
            error: refusal,
            retryable: false,
        });
-        expect(pending.result()).toEqual({ error: { message: refusal } });
+        expect(pending.result()).toEqual({
+            error: { code: -32603, message: refusal },
+        });
    });

    // ethers signs a `bool` field sent as the string "false" as true, and one
@@ -1611,7 +1626,9 @@ describe("what the approval is verified against", () => {
                error: refusal,
                retryable: false,
            });
-            expect(pending.result()).toEqual({ error: { message: refusal } });
+            expect(pending.result()).toEqual({
+                error: { code: -32603, message: refusal },
+            });
        },
    );

@@ -1649,6 +1666,7 @@ describe("what the approval is verified against", () => {

        expect(pending.result()).toEqual({
            error: {
+                code: -32603,
                message: expect.stringMatching(
                    /active address changed while this transaction was being prepared/,
                ),
@@ -1671,8 +1689,11 @@ describe("what the approval is verified against", () => {
        const pending = bg.requestTx();
        await settle();

+        // A failure with no node error in it is answered as the wallet's own;
+        // tests/rpcErrorCodes.test.js has the node's own revert.
        expect(pending.result()).toEqual({
            error: {
+                code: -32603,
                message: expect.stringMatching(
                    /could not be prepared.*execution reverted/,
                ),
@@ -0,0 +1,282 @@
+// What a page is told when a request fails
+// (https://git.eeqj.de/sneak/AutistMask/issues/533).
+//
+// EIP-1193 gives every rejected request a numeric `code`, and lets it carry
+// `data`. A node's error used to reach the page as its message alone: a
+// reverting eth_call lost code 3 and the revert data a site needs to decode
+// the revert, and a site could not tell that failure from any other.
+//
+// Each request here goes from a page through the real background to a fake
+// node, and what is checked is the answer the background gives the page. The
+// content script relays that answer as it is, and src/content/inpage.js turns
+// its code and data into the error the page catches (tests/inpageErrors.test.js).
+//
+// The background reaches a node two ways, and the fake node answers both:
+// fetch(), which the read-only methods are proxied through, and ethers' own
+// HTTP transport, which a transaction is prepared through.
+
+const { AbiCoder } = require("ethers");
+const { networkById } = require("../src/shared/networks");
+const { makeStorageStub } = require("./support/storageStub");
+
+const ORIGIN = "https://dapp.example";
+const ADDRESS = "0x66133E8ea0f5D1d612D2502a968757D1048c214a";
+const TOKEN = "0x1111111111111111111111111111111111111111";
+const SEPOLIA = networkById("sepolia");
+
+// Error("bal"), which a token whose transfer() does require(..., "bal")
+// reverts with when the balance is too low.
+const REVERT_DATA =
+    "0x08c379a0" +
+    AbiCoder.defaultAbiCoder().encode(["string"], ["bal"]).slice(2);
+
+// What a node answers for a call that reverts, and for an address that is not
+// one.
+const REVERTED = {
+    code: 3,
+    message: "execution reverted: bal",
+    data: REVERT_DATA,
+};
+const INVALID_ADDRESS = { code: -32602, message: "invalid string length" };
+
+const WITH_ADDRESS = {
+    hasWallet: true,
+    wallets: [
+        {
+            name: "Wallet 1",
+            type: "hd",
+            xpub: "xpub-1",
+            addresses: [
+                { address: ADDRESS, balance: "0.0", tokenBalances: [] },
+            ],
+        },
+    ],
+    activeAddress: ADDRESS,
+    networkId: "sepolia",
+    rpcUrl: SEPOLIA.defaultRpcUrl,
+    blockscoutUrl: SEPOLIA.defaultBlockscoutUrl,
+    allowedSites: { [ADDRESS]: [ORIGIN] },
+    deniedSites: {},
+    trackedTokens: [],
+};
+
+const NO_ADDRESS = {
+    ...WITH_ADDRESS,
+    hasWallet: false,
+    wallets: [],
+    activeAddress: null,
+    allowedSites: {},
+};
+
+const realFetch = globalThis.fetch;
+
+afterEach(() => {
+    delete global.chrome;
+    globalThis.fetch = realFetch;
+});
+
+// The background, over a stored profile, with a node that answers each
+// JSON-RPC method from `node`: a method's entry is the result, or
+// `{ error }` for the error the node answers with. A method not in `node`
+// is answered "method not found", so a test that reaches one fails on it.
+// `node` null is a node that cannot be reached at all.
+function loadBackground(profile, node) {
+    jest.resetModules();
+
+    jest.doMock("../src/shared/phishingDomains", () => ({
+        isPhishingDomain: () => false,
+    }));
+    jest.doMock("../src/shared/alarms", () => ({
+        BALANCE_REFRESH_ALARM: "balance",
+        BALANCE_REFRESH_PERIOD_MINUTES: 1,
+        ensureRecurringAlarms: jest.fn(async () => {}),
+        registerAlarmHandlers: jest.fn(),
+    }));
+
+    function reply({ id, method }) {
+        const entry = node[method];
+        if (entry === undefined) {
+            return {
+                jsonrpc: "2.0",
+                id,
+                error: { code: -32601, message: "method not found: " + method },
+            };
+        }
+        if (entry && entry.error)
+            return { jsonrpc: "2.0", id, error: entry.error };
+        return { jsonrpc: "2.0", id, result: entry };
+    }
+
+    globalThis.fetch = jest.fn(async (url, opts) => {
+        if (!node) throw new TypeError("fetch failed");
+        return { status: 200, json: async () => reply(JSON.parse(opts.body)) };
+    });
+
+    // The ethers the background is about to load: jest.resetModules() above
+    // means it is a fresh copy, not the one this file required at the top.
+    require("ethers").FetchRequest.registerGetUrl(async (req) => {
+        if (!node) throw new Error("connect ECONNREFUSED");
+        const body = JSON.parse(Buffer.from(req.body).toString("utf8"));
+        const answer = Array.isArray(body) ? body.map(reply) : reply(body);
+        return {
+            statusCode: 200,
+            statusMessage: "OK",
+            headers: { "content-type": "application/json" },
+            body: Buffer.from(JSON.stringify(answer)),
+        };
+    });
+
+    let messageListener = null;
+    global.chrome = {
+        storage: makeStorageStub({ autistmask: profile }),
+        runtime: {
+            getURL: (path) => "chrome-extension://autistmask/" + path,
+            onMessage: {
+                addListener: (fn) => {
+                    messageListener = fn;
+                },
+            },
+            onConnect: { addListener: () => {} },
+            lastError: null,
+        },
+        windows: { onRemoved: { addListener: () => {} } },
+        action: { setPopup: () => {} },
+    };
+
+    require("../src/background/index");
+
+    // The answer the page gets to one request.
+    return function request(method, params) {
+        return new Promise((resolve) => {
+            messageListener(
+                { type: "AUTISTMASK_RPC", method, params },
+                { origin: ORIGIN },
+                resolve,
+            );
+        });
+    };
+}
+
+const TRANSFER = {
+    from: ADDRESS,
+    to: TOKEN,
+    // transfer(ADDRESS, 1)
+    data:
+        "0xa9059cbb" +
+        AbiCoder.defaultAbiCoder()
+            .encode(["address", "uint256"], [ADDRESS, 1])
+            .slice(2),
+};
+
+describe("a node's error reaches the page with the node's code and data", () => {
+    test("a reverting eth_call", async () => {
+        const request = loadBackground(WITH_ADDRESS, {
+            eth_call: { error: REVERTED },
+        });
+
+        const answer = await request("eth_call", [TRANSFER, "latest"]);
+
+        expect(answer).toEqual({ error: REVERTED });
+    });
+
+    test("an eth_getBalance for an address that is not one", async () => {
+        const request = loadBackground(WITH_ADDRESS, {
+            eth_getBalance: { error: INVALID_ADDRESS },
+        });
+
+        const answer = await request("eth_getBalance", ["0x1234", "latest"]);
+
+        expect(answer).toEqual({ error: INVALID_ADDRESS });
+    });
+
+    // The wallet's own sentence, with the node's code and revert data, so the
+    // site can decode why its transaction would revert.
+    test("an eth_sendTransaction whose gas estimate reverts", async () => {
+        const request = loadBackground(WITH_ADDRESS, {
+            eth_getTransactionCount: "0x7",
+            eth_estimateGas: { error: REVERTED },
+        });
+
+        const answer = await request("eth_sendTransaction", [TRANSFER]);
+
+        expect(answer).toEqual({
+            error: {
+                code: REVERTED.code,
+                message: expect.stringMatching(
+                    /^The transaction could not be prepared: execution reverted/,
+                ),
+                data: REVERT_DATA,
+            },
+        });
+    });
+});
+
+// One request for each way a request is refused without the node's code: by
+// the wallet itself, or because the node could not be reached.
+describe("no rejected request reaches the page without a numeric code", () => {
+    test.each([
+        [
+            "eth_requestAccounts with no address",
+            NO_ADDRESS,
+            {},
+            "eth_requestAccounts",
+            [],
+            4100,
+        ],
+        [
+            "personal_sign with no address",
+            NO_ADDRESS,
+            {},
+            "personal_sign",
+            ["0x00", ADDRESS],
+            4100,
+        ],
+        [
+            "eth_signTypedData_v4 with no address",
+            NO_ADDRESS,
+            {},
+            "eth_signTypedData_v4",
+            [ADDRESS, "{}"],
+            4100,
+        ],
+        [
+            "eth_sendTransaction with no address",
+            NO_ADDRESS,
+            {},
+            "eth_sendTransaction",
+            [TRANSFER],
+            4100,
+        ],
+        [
+            "eth_sendTransaction with two different gas limits",
+            WITH_ADDRESS,
+            {},
+            "eth_sendTransaction",
+            [{ ...TRANSFER, gas: "0x5208", gasLimit: "0x5209" }],
+            -32602,
+        ],
+        [
+            "eth_call to a node that cannot be reached",
+            WITH_ADDRESS,
+            null,
+            "eth_call",
+            [TRANSFER, "latest"],
+            -32603,
+        ],
+        [
+            "eth_sendTransaction to a node that cannot be reached",
+            WITH_ADDRESS,
+            null,
+            "eth_sendTransaction",
+            [TRANSFER],
+            -32603,
+        ],
+    ])("%s", async (_name, profile, node, method, params, code) => {
+        const request = loadBackground(profile, node);
+
+        const answer = await request(method, params);
+
+        expect(answer.error.code).toBe(code);
+        expect(answer.error.message).toEqual(expect.any(String));
+    });
+});