Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix uncatchable issue when sending transactions over JSON-RPC and pro…
…vide some retry-recovery for missing v (#4513).
  • Loading branch information
ricmoo committed Dec 19, 2023
1 parent 6ee1a5f commit 1802215
Show file tree
Hide file tree
Showing 3 changed files with 271 additions and 21 deletions.
207 changes: 207 additions & 0 deletions 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");
});
});

20 changes: 11 additions & 9 deletions src.ts/providers/abstract-provider.ts
Expand Up @@ -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();
Expand Down
65 changes: 53 additions & 12 deletions src.ts/providers/provider-jsonrpc.ts
Expand Up @@ -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";

Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit 1802215

Please sign in to comment.