Fix uncatchable issue when sending transactions over JSON-RPC and provide some retry-recovery for missing v (#4513).

This commit is contained in:
Richard Moore 2023-12-19 04:52:38 -05:00
parent 6ee1a5f8bb
commit 180221574c
3 changed files with 271 additions and 21 deletions

View File

@ -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");
});
});

View File

@ -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();

View File

@ -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();