Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: investigate slow request handler imports #6481

Draft
wants to merge 2 commits into
base: v-next
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,12 @@ export async function createHandlersArray<
const resolvedAccounts = await Promise.all(
accounts.map((acc) => acc.getHexString()),
);

requestHandlers.push(
new LocalAccountsHandler(networkConnection.provider, resolvedAccounts),
const localAccountsHandler = new LocalAccountsHandler(
networkConnection.provider,
);
await localAccountsHandler.initializePrivateKeys(resolvedAccounts);

requestHandlers.push(localAccountsHandler);
} else if (isHttpNetworkHdAccountsConfig(accounts)) {
const { HDWalletHandler } = await import(
"./handlers/accounts/hd-wallet-handler.js"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ export class HDWalletHandler extends LocalAccountsHandler {
passphrase,
);

return new HDWalletHandler(provider, privateKeys);
const hdWalletHandler = new HDWalletHandler(provider);
await hdWalletHandler.initializePrivateKeys(privateKeys);

return hdWalletHandler;
}
private constructor(provider: EthereumProvider, privateKeys: string[]) {
super(provider, privateKeys);
private constructor(provider: EthereumProvider) {
super(provider);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import {
hexStringToBigInt,
hexStringToBytes,
} from "@nomicfoundation/hardhat-utils/hex";
import { addr, Transaction } from "micro-eth-signer";
import * as typed from "micro-eth-signer/typed-data";
import { signTyped } from "micro-eth-signer/typed-data";

import { getRequestParams } from "../../../json-rpc.js";
import { rpcAddress } from "../../../rpc/types/address.js";
Expand All @@ -33,13 +30,8 @@ import { ChainId } from "../chain-id/chain-id.js";
export class LocalAccountsHandler extends ChainId implements RequestHandler {
readonly #addressToPrivateKey: Map<string, Uint8Array> = new Map();

constructor(
provider: EthereumProvider,
localAccountsHexPrivateKeys: string[],
) {
constructor(provider: EthereumProvider) {
super(provider);

this.#initializePrivateKeys(localAccountsHexPrivateKeys);
}

public async handle(
Expand Down Expand Up @@ -69,6 +61,11 @@ export class LocalAccountsHandler extends ChainId implements RequestHandler {

const params = getRequestParams(jsonRpcRequest);

const {
personal: { sign },
signTyped,
} = await import("micro-eth-signer/typed-data");

if (jsonRpcRequest.method === "eth_sign") {
if (params.length > 0) {
const [address, data] = validateParams(params, rpcAddress, rpcData);
Expand All @@ -81,9 +78,10 @@ export class LocalAccountsHandler extends ChainId implements RequestHandler {
}

const privateKey = this.#getPrivateKeyForAddress(address);

return this.#createJsonRpcResponse(
jsonRpcRequest.id,
typed.personal.sign(data, privateKey),
sign(data, privateKey),
);
}
}
Expand All @@ -103,7 +101,7 @@ export class LocalAccountsHandler extends ChainId implements RequestHandler {
const privateKey = this.#getPrivateKeyForAddress(address);
return this.#createJsonRpcResponse(
jsonRpcRequest.id,
typed.personal.sign(data, privateKey),
sign(data, privateKey),
);
}
}
Expand Down Expand Up @@ -212,13 +210,19 @@ export class LocalAccountsHandler extends ChainId implements RequestHandler {
}
}

#initializePrivateKeys(localAccountsHexPrivateKeys: string[]) {
public async initializePrivateKeys(
localAccountsHexPrivateKeys: string[],
): Promise<void> {
const privateKeys: Uint8Array[] = localAccountsHexPrivateKeys.map((h) =>
hexStringToBytes(h),
);

const {
addr: { fromPrivateKey },
} = await import("micro-eth-signer");

for (const pk of privateKeys) {
const address = addr.fromPrivateKey(pk).toLowerCase();
const address = fromPrivateKey(pk).toLowerCase();
this.#addressToPrivateKey.set(address, pk);
}
}
Expand Down Expand Up @@ -266,9 +270,14 @@ export class LocalAccountsHandler extends ChainId implements RequestHandler {
gasLimit: transactionRequest.gas,
};

const {
Transaction,
addr: { addChecksum },
} = await import("micro-eth-signer");

const accessList = txData.accessList?.map(({ address, storageKeys }) => {
return {
address: addr.addChecksum(bytesToHexString(address)),
address: addChecksum(bytesToHexString(address)),
storageKeys:
storageKeys !== null
? storageKeys.map((k) => bytesToHexString(k))
Expand All @@ -291,7 +300,7 @@ export class LocalAccountsHandler extends ChainId implements RequestHandler {
// null or undefined addresses. Therefore, these values must be converted to "0x", the expected format.
checksummedAddress = "0x";
} else {
checksummedAddress = addr.addChecksum(
checksummedAddress = addChecksum(
bytesToHexString(txData.to).toLowerCase(),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,14 @@ const runSolidityTests: NewTaskActionFunction<TestActionArguments> = async (
// directory or a file with a `.t.sol` extension in the `sources.solidity` directory
rootFilePaths = (
await Promise.all([
getAllFilesMatching(hre.config.paths.tests.solidity, (f) =>
f.endsWith(".sol"),
getAllFilesMatching(
hre.config.paths.tests.solidity,
(
f, // /tests/solidity
) => f.endsWith(".sol"),
),
...hre.config.paths.sources.solidity.map(async (dir) => {
// /contracts
return getAllFilesMatching(dir, (f) => f.endsWith(".t.sol"));
}),
])
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import assert from "node:assert/strict";
import { execSync } from "node:child_process";
import path from "node:path";
import { describe, it } from "node:test";

import { getAllFilesMatching } from "@nomicfoundation/hardhat-utils/fs";
import debug from "debug";

const log = debug(
"hardhat:test:network-manager:request-handlers:request-array",
);

/**
* Example debug output:
* 2025-03-18T10:54:37.575Z hardhat:test:network-manager:request-handlers:request-array importing hd-wallet-handler.ts took 37ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing local-accounts.ts took 36ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing fixed-sender-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing chain-id-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing chain-id.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing automatic-gas-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing automatic-gas-price-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing fixed-gas-handler.ts took 11ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing automatic-sender-handler.ts took 10ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing sender.ts took 10ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing fixed-gas-price-handler.ts took 10ms
* 2025-03-18T10:54:37.576Z hardhat:test:network-manager:request-handlers:request-array importing multiplied-gas-estimation.ts took 10ms
*/

describe(
"HandlersArray",
{
skip: process.env.HARDHAT_DISABLE_SLOW_TESTS === "true",
},
async () => {
it(`should load all the handlers in a reasonable amount of time`, async () => {
const handlersDir = path.resolve(
"src/internal/builtin-plugins/network-manager/request-handlers/handlers",
);
const handlers = await getAllFilesMatching(handlersDir);
const handlerImports = [];

const nodeOptions: any = {
cwd: process.cwd(),
stdio: "pipe",
env: {
...process.env,
FORCE_COLOR: "0",
NO_COLOR: "1",
},
};

// NOTE: First, we run a dummy command to warm up node. The number of runs
// is arbitrary, but it seems to be enough to get reasonable stability.
for (let i = 0; i < 20; i++) {
execSync("node --import tsx/esm -e ''", nodeOptions);
}

for (const handler of handlers) {
const output = execSync(
`node --import tsx/esm -e "const start = Date.now(); await import('${handler}'); console.log(Date.now() - start);"`,
{
cwd: process.cwd(),
stdio: "pipe",
env: {
...process.env,
FORCE_COLOR: "0",
NO_COLOR: "1",
},
},
);
const duration = parseInt(output.toString().trim(), 10);
handlerImports.push({
handler: path.basename(handler),
duration,
});
}

handlerImports.sort((a, b) => b.duration - a.duration);

for (const { handler, duration } of handlerImports) {
log(`importing ${handler} took ${duration}ms`);
}

// NOTE: The maximum import duration is arbitrary, but it seems to reasonably detect the outliers.
const maxImportDuration = 20;
const longestHandlerImport = handlerImports[0];

assert.ok(

Check failure on line 88 in v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/handlers-array.ts

View workflow job for this annotation

GitHub Actions / [hardhat] ci on ubuntu-latest (Node 22)

should load all the handlers in a reasonable amount of time

AssertionError: The maximum import duration of 92ms (local-accounts.ts) exceeds the 20ms limit - Expected + Received - true + false at TestContext.<anonymous> (test/internal/builtin-plugins/network-manager/request-handlers/handlers-array.ts:88:14)

Check failure on line 88 in v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/handlers-array.ts

View workflow job for this annotation

GitHub Actions / [hardhat] ci on macos-13 (Node 22)

should load all the handlers in a reasonable amount of time

AssertionError: The maximum import duration of 106ms (hd-wallet-handler.ts) exceeds the 20ms limit - Expected + Received - true + false at TestContext.<anonymous> (test/internal/builtin-plugins/network-manager/request-handlers/handlers-array.ts:88:14)

Check failure on line 88 in v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/handlers-array.ts

View workflow job for this annotation

GitHub Actions / [hardhat] ci on macos-latest (Node 22)

should load all the handlers in a reasonable amount of time

AssertionError: The maximum import duration of 46ms (hd-wallet-handler.ts) exceeds the 20ms limit - Expected + Received - true + false at TestContext.<anonymous> (test/internal/builtin-plugins/network-manager/request-handlers/handlers-array.ts:88:14)
longestHandlerImport.duration < maxImportDuration,
`The maximum import duration of ${longestHandlerImport.duration}ms (${longestHandlerImport.handler}) exceeds the ${maxImportDuration}ms limit`,
);
});
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe("LocalAccountsHandler", () => {
"0xec02c2b7019e75378a05018adc30a0252ba705670acb383a1d332e57b0b792d2",
];

beforeEach(() => {
beforeEach(async () => {
mockedProvider = new EthereumMockedProvider();

mockedProvider.setReturnValue(
Expand All @@ -86,7 +86,8 @@ describe("LocalAccountsHandler", () => {
);
mockedProvider.setReturnValue("eth_accounts", []);

localAccountsHandler = new LocalAccountsHandler(mockedProvider, accounts);
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys(accounts);
});

describe("resolveRequest", () => {
Expand Down Expand Up @@ -141,7 +142,8 @@ describe("LocalAccountsHandler", () => {
it("should be compatible with parity's implementation", async () => {
// This test was created by using Parity Ethereum
// v2.2.5-beta-7fbcdfeed-20181213 and calling eth_sign
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
"0x6e59a6617c48d76d3b21d722eaba867e16ecf54ab3da7a93724f51812bc6d1aa",
]);

Expand All @@ -163,7 +165,8 @@ describe("LocalAccountsHandler", () => {

it("should be compatible with ganache-cli's implementation", async () => {
// This test was created by using Ganache CLI v6.1.6 (ganache-core: 2.1.5)
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
"0xf159c85082f4dd4ee472583a37a1b5683c727ec99708f3d94ff05faa7a7a70ce",
]);

Expand Down Expand Up @@ -194,7 +197,8 @@ describe("LocalAccountsHandler", () => {

it("should be compatible with geth's implementation", async () => {
// This test was created by using Geth 1.8.20-stable
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
"0xf2d19e944851ea0faa9440e24a22ddab850210cae46b306a3fde4c98b22a0dcb",
]);

Expand Down Expand Up @@ -251,7 +255,8 @@ describe("LocalAccountsHandler", () => {
// This test was taken from the `eth_signTypedData` example from the
// EIP-712 specification.
// <https://eips.ethereum.org/EIPS/eip-712#eth_signtypeddata>
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
// keccak256("cow")
"0xc85ef7d79691fe79573b1a7064c19c1a9819ebdbd1faaab1a8ec92344438aaf4",
]);
Expand Down Expand Up @@ -309,7 +314,8 @@ describe("LocalAccountsHandler", () => {
});

it("should be compatible with stringified JSON input", async () => {
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
// keccak256("cow")
"0xc85ef7d79691fe79573b1a7064c19c1a9819ebdbd1faaab1a8ec92344438aaf4",
]);
Expand Down Expand Up @@ -406,7 +412,8 @@ describe("LocalAccountsHandler", () => {
describe("personal_sign", () => {
it("should be compatible with geth's implementation", async () => {
// This test was created by using Geth 1.10.12-unstable and calling personal_sign
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
"0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80",
]);

Expand All @@ -428,7 +435,8 @@ describe("LocalAccountsHandler", () => {

it("should be compatible with metamask's implementation", async () => {
// This test was created by using Metamask 10.3.0
localAccountsHandler = new LocalAccountsHandler(mockedProvider, [
localAccountsHandler = new LocalAccountsHandler(mockedProvider);
await localAccountsHandler.initializePrivateKeys([
"0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80",
]);

Expand Down
Loading