Fix uncatchable issue when sending transactions over JSON-RPC and pro…

…vide some retry-recovery for missing v (#4513).
ethers-io · Dec 19, 2023 · 1802215 · 1802215
1 parent 6ee1a5f
commit 1802215
Show file tree

Hide file tree

Showing 3 changed files with 271 additions and 21 deletions.
diff --git a/src.ts/_tests/test-provider-jsonrpc.ts b/src.ts/_tests/test-provider-jsonrpc.ts
@@ -0,0 +1,207 @@
+import assert from "assert";
+
+import {
+    id, isError, makeError, toUtf8Bytes, toUtf8String,
+    FetchRequest,
+    JsonRpcProvider, Transaction, Wallet
+} from "../index.js";
+
+const StatusMessages: Record<number, string> = {
+  200: "OK",
+  400: "BAD REQUEST",
+  500: "SERVER ERROR",
+};
+
+
+type ProcessRequest = (method: string, params: Array<string>, blockNumber: number) => any;
+
+const wallet = new Wallet(id("test"));
+
+function createProvider(testFunc: ProcessRequest): JsonRpcProvider {
+
+    let blockNumber = 1;
+    const ticker = setInterval(() => { blockNumber++ }, 100);
+    if (ticker.unref) { ticker.unref(); }
+
+    const processReq = (req: { method: string, params: Array<string>, id: any }) => {
+
+        let result = testFunc(req.method, req.params, blockNumber);
+        if (result === undefined) {
+            switch (req.method) {
+                case "eth_blockNumber":
+                    result = blockNumber;
+                    break;
+                case "eth_chainId":
+                    result = "0x1337";
+                    break;
+                case "eth_accounts":
+                    result = [ wallet.address ];
+                    break;
+                default:
+                    console.log("****", req);
+                    return { id, error: "unsupported", jsonrpc: "2.0" };
+            }
+        }
+
+        return { id: req.id, result, jsonrpc: "2.0" };
+    };
+
+    const req = new FetchRequest("http:/\/localhost:8082/");
+    req.getUrlFunc = async (_req, signal) => {
+        const req = JSON.parse(_req.hasBody() ? toUtf8String(_req.body): "");
+
+        let statusCode = 200;
+        const headers = { };
+
+        let resp: any;
+        try {
+            if (Array.isArray(req)) {
+                resp = req.map((r) => processReq(r));
+            } else {
+                resp = processReq(req);
+            }
+
+        } catch(error: any) {
+            statusCode = 500;
+            resp = error.message;
+        }
+
+        const body = toUtf8Bytes(JSON.stringify(resp));
+
+        return {
+            statusCode,
+            statusMessage: StatusMessages[statusCode],
+            headers, body
+        };
+    };
+
+    return new JsonRpcProvider(req, undefined, { cacheTimeout: -1 });
+}
+
+describe("Ensure Catchable Errors", function() {
+    it("Can catch bad broadcast replies", async function() {
+        this.timeout(15000);
+
+        const txInfo = {
+          chainId: 1337,
+          gasLimit: 100000,
+          maxFeePerGas: 2000000000,
+          maxPriorityFeePerGas: 1000000000,
+          to: wallet.address,
+          value: 1,
+        };
+        const txSign = await wallet.signTransaction(txInfo);
+        const txObj = Transaction.from(txSign);
+
+        let count = 0;
+
+        const provider = createProvider((method, params, blockNumber) => {
+
+            switch (method) {
+                case "eth_sendTransaction":
+                    return txObj.hash;
+
+                case "eth_getTransactionByHash": {
+                    count++;
+
+                    // First time; fail!
+                    if (count === 1) {
+                        throw makeError("Faux Error", "SERVER_ERROR", {
+                            request: <any>({ })
+                        });
+                    }
+
+                    // Second time; return null
+                    if (count === 2) { return null; }
+
+                    // Return a valid tx...
+                    const result = Object.assign({ },
+                        txObj.toJSON(),
+                        txObj.signature!.toJSON(),
+                        { hash: txObj.hash, from: wallet.address });
+
+                    // ...eventually mined
+                    if (count > 4) {
+                        result.blockNumber = blockNumber;
+                        result.blockHash = id("test");
+                    }
+
+                    return result;
+                }
+            }
+
+            return undefined;
+        });
+
+        const signer = await provider.getSigner();
+
+        const tx = await signer.sendTransaction(txInfo);
+        assert(tx);
+    });
+
+
+    it("Missing v is recovered", async function() {
+        this.timeout(15000);
+
+        const txInfo = {
+          chainId: 1337,
+          gasLimit: 100000,
+          maxFeePerGas: 2000000000,
+          maxPriorityFeePerGas: 1000000000,
+          to: wallet.address,
+          value: 1,
+        };
+        const txSign = await wallet.signTransaction(txInfo);
+        const txObj = Transaction.from(txSign);
+
+        let count = 0;
+
+        // A provider which is mocked to return a "missing v"
+        // in getTransaction
+
+        const provider = createProvider((method, params, blockNumber) => {
+
+            switch (method) {
+                case "eth_sendTransaction":
+                    return txObj.hash;
+
+                case "eth_getTransactionByHash": {
+                    count++;
+
+                    // The fully valid tx response
+                    const result = Object.assign({ },
+                        txObj.toJSON(),
+                        txObj.signature!.toJSON(),
+                        { hash: txObj.hash, from: wallet.address, sig: null });
+
+                    // First time; fail with a missing v!
+                    if (count < 2) { delete result.v; }
+
+                    // Debug
+                    result._count = count;
+
+                    return result;
+                }
+            }
+
+            return undefined;
+        });
+
+        // Track any "missing v" error
+        let missingV: Error | null = null;
+        provider.on("error", (e) => {
+            if (isError(e, "UNKNOWN_ERROR") && isError(e.error, "INVALID_ARGUMENT")) {
+                if (e.error.argument === "signature" && e.error.shortMessage === "missing v") {
+                    missingV = e.error;
+                }
+            }
+        });
+
+        const signer = await provider.getSigner();
+
+        const tx = await signer.sendTransaction(txInfo);
+        assert.ok(!!tx, "we got a transaction");
+        assert.ok(!!missingV, "missing v error present");
+    });
+});
+
diff --git a/src.ts/providers/abstract-provider.ts b/src.ts/providers/abstract-provider.ts
@@ -862,16 +862,18 @@ export class AbstractProvider implements Provider {
         if (this.#networkPromise == null) {
 
             // Detect the current network (shared with all calls)
-            const detectNetwork = this._detectNetwork().then((network) => {
-                this.emit("network", network, null);
-                return network;
-            }, (error) => {
-                // Reset the networkPromise on failure, so we will try again
-                if (this.#networkPromise === detectNetwork) {
-                    this.#networkPromise = null;
+            const detectNetwork = (async () => {
+                try {
+                    const network = await this._detectNetwork();
+                    this.emit("network", network, null);
+                    return network;
+                } catch (error) {
+                    if (this.#networkPromise === detectNetwork!) {
+                        this.#networkPromise = null;
+                    }
+                    throw error;
                 }
-                throw error;
-            });
+            })();
 
             this.#networkPromise = detectNetwork;
             return (await detectNetwork).clone();

diff --git a/src.ts/providers/provider-jsonrpc.ts b/src.ts/providers/provider-jsonrpc.ts
@@ -21,7 +21,7 @@ import { TypedDataEncoder } from "../hash/index.js";
 import { accessListify } from "../transaction/index.js";
 import {
     defineProperties, getBigInt, hexlify, isHexString, toQuantity, toUtf8Bytes,
-    makeError, assert, assertArgument,
+    isError, makeError, assert, assertArgument,
     FetchRequest, resolveProperties
 } from "../utils/index.js";
 
@@ -41,7 +41,6 @@ import type { Signer } from "./signer.js";
 
 type Timer = ReturnType<typeof setTimeout>;
 
-
 const Primitive = "bigint,boolean,function,number,string,symbol".split(/,/g);
 //const Methods = "getAddress,then".split(/,/g);
 function deepCopy<T = any>(value: T): T {
@@ -363,12 +362,49 @@ export class JsonRpcSigner extends AbstractSigner<JsonRpcApiProvider> {
         // for it; it should show up very quickly
         return await (new Promise((resolve, reject) => {
             const timeouts = [ 1000, 100 ];
+            let invalids = 0;
+
             const checkTx = async () => {
-                // Try getting the transaction
-                const tx = await this.provider.getTransaction(hash);
-                if (tx != null) {
-                    resolve(tx.replaceableTransaction(blockNumber));
-                    return;
+
+                try {
+                    // Try getting the transaction
+                    const tx = await this.provider.getTransaction(hash);
+
+                    if (tx != null) {
+                        resolve(tx.replaceableTransaction(blockNumber));
+                        return;
+                    }
+
+                } catch (error) {
+
+                    // If we were cancelled: stop polling.
+                    // If the data is bad: the node returns bad transactions
+                    // If the network changed: calling again will also fail
+                    // If unsupported: likely destroyed
+                    if (isError(error, "CANCELLED") || isError(error, "BAD_DATA") ||
+                        isError(error, "NETWORK_ERROR" || isError(error, "UNSUPPORTED_OPERATION"))) {
+
+                        if (error.info == null) { error.info = { }; }
+                        error.info.sendTransactionHash = hash;
+
+                        reject(error);
+                        return;
+                    }
+
+                    // Stop-gap for misbehaving backends; see #4513
+                    if (isError(error, "INVALID_ARGUMENT")) {
+                        invalids++;
+                        if (error.info == null) { error.info = { }; }
+                        error.info.sendTransactionHash = hash;
+                        if (invalids > 10) {
+                            reject(error);
+                            return;
+                        }
+                    }
+
+                    // Notify anyone that cares; but we will try again, since
+                    // it is likely an intermittent service error
+                    this.provider.emit("error", makeError("failed to fetch transation after sending (will try again)", "UNKNOWN_ERROR", { error }));
                 }
 
                 // Wait another 4 seconds
@@ -468,7 +504,7 @@ export abstract class JsonRpcApiProvider extends AbstractProvider {
     #scheduleDrain(): void {
         if (this.#drainTimer) { return; }
 
-        // If we aren't using batching, no hard in sending it immeidately
+        // If we aren't using batching, no harm in sending it immediately
         const stallTime = (this._getOption("batchMaxCount") === 1) ? 0: this._getOption("batchStallTime");
 
         this.#drainTimer = setTimeout(() => {
@@ -613,6 +649,7 @@ export abstract class JsonRpcApiProvider extends AbstractProvider {
      *  and should generally call ``super._perform`` as a fallback.
      */
     async _perform(req: PerformActionRequest): Promise<any> {
+
         // Legacy networks do not like the type field being passed along (which
         // is fair), so we delete type if it is 0 and a non-EIP-1559 network
         if (req.method === "call" || req.method === "estimateGas") {
@@ -664,9 +701,14 @@ export abstract class JsonRpcApiProvider extends AbstractProvider {
         // If we are ready, use ``send``, which enabled requests to be batched
         if (this.ready) {
             this.#pendingDetectNetwork = (async () => {
-                const result = Network.from(getBigInt(await this.send("eth_chainId", [ ])));
-                this.#pendingDetectNetwork = null;
-                return result;
+                try {
+                    const result = Network.from(getBigInt(await this.send("eth_chainId", [ ])));
+                    this.#pendingDetectNetwork = null;
+                    return result;
+                } catch (error) {
+                    this.#pendingDetectNetwork = null;
+                    throw error;
+                }
             })();
             return await this.#pendingDetectNetwork;
         }
@@ -1186,7 +1228,6 @@ export class JsonRpcProvider extends JsonRpcApiPollingProvider {
         const request = this._getConnection();
         request.body = JSON.stringify(payload);
         request.setHeader("content-type", "application/json");
-
         const response = await request.send();
         response.assertOk();