Rework providers #5994

ChristopherDedominici · 2024-11-27T11:02:02Z

DESCRIPTION
This PR reworks the logic for the providers.

Main Points:

the providers are now stored in an array, making it easier to track which provider is being executed by stepping through a simple for loop. This replaces the previous solution, which relied on recursive behavior and was harder to follow.
e2e tests have been simplified. The old tests have been replaced with a single e2e file, which is easier to maintain.
minor renaming adjustments have been made for improved clarity.

QUESTIONS

the v2 code was using this function to convert from number to rpc quantity. The v3 code is following this logic. I think the logic is the same, but I'd like to have a double check on this.

vercel · 2024-11-27T11:02:07Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hardhat	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 6, 2024 11:31am

changeset-bot · 2024-11-27T11:02:07Z

⚠️ No Changeset found

Latest commit: 64ed264

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

ChristopherDedominici · 2024-11-28T15:59:41Z

...nternal/builtin-plugins/network-manager/request-handlers/handlers/accounts/local-accounts.ts

+  public async handle(
+    jsonRpcRequest: JsonRpcRequest,
+  ): Promise<JsonRpcRequest | JsonRpcResponse> {
+    const response = await this.#resolveRequest(jsonRpcRequest);


This handler can modify a JSON-RPC request and return a JSON-RPC response. In the v2 implementation, the execution order first handled the response logic and then processed the request modification. I have retained the same approach here. See the v2 code here

ChristopherDedominici · 2024-11-28T16:01:37Z

v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/e2e.ts

@@ -0,0 +1,271 @@
+import type { HttpNetworkHDAccountsConfig } from "../../../../../src/types/config.js";


Are these e2e tests comprehensive enough, or is there an additional scenario you believe would be worth including?

ChristopherDedominici · 2024-11-28T16:03:50Z

v-next/hardhat/src/internal/builtin-plugins/network-manager/request-handlers/hanlders-array.ts

+    //
+    // EDR Network handles this in a more performant way, so we don't use
+    // the AutomaticGasPrice for it.
+    if (networkConfig.type === "http") {


Is this the if condition that can be removed because also supported in EDR networks?
See related issues here:

Add resolution for the 'accounts' property in edr configurations #5827

Revisit the 'create' method in HardhatEthersSigner #5796

Let's remove this one, but not the one in line 92. Let's let EDR handle the signing of transactions from its own accounts.

v-next/hardhat/src/internal/builtin-plugins/network-manager/hook-handlers/network.ts

alcuadrado · 2024-12-05T20:17:34Z

v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/e2e.ts

+      params: [
+        {
+          to: "0x0000000000000000000000000000000000000012",
+          maxFeePerGas: "0x99",


this value here confuses me a bit. Why is it necessary?

It's not strictly necessary. It's more of an additional check to be sure that there is no conflict with the logic if maxFeePerGas is present. But it can be removed, I did it here

v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/e2e.ts

alcuadrado

I left a few comments, but this looks good. It's now definitely simpler to reason about it.

…ok-handlers/network.ts Co-authored-by: Patricio Palladino <[email protected]>

…rk-providers

… into rework-providers

ChristopherDedominici added 2 commits November 27, 2024 11:41

remove reworked-files

3ccca1a

new handlers logic + chain-id handler

05ce4ca

github-actions bot added the status:triaging label Nov 27, 2024

github-actions bot assigned kanej Nov 27, 2024

vercel bot deployed to Preview November 27, 2024 11:03 View deployment

ChristopherDedominici added 3 commits November 27, 2024 15:21

rename chain-id files

e27875f

add automatic and fixed gas handlers

ffb4230

add automatic and fixed gas price handlers

31720c3

ChristopherDedominici linked an issue Nov 27, 2024 that may be closed by this pull request

Add the replacement of provider wrappers using hooks (M2) #5719

Closed

vercel bot deployed to Preview November 27, 2024 15:18 View deployment

add automatic and fixed sender

ef072a8

vercel bot deployed to Preview November 27, 2024 16:21 View deployment

ChristopherDedominici added 2 commits November 28, 2024 15:54

add LocalAccountsHandler and HDAccountsHandler

f8089cd

add logic to create handler-array

7d5f6eb

vercel bot deployed to Preview November 28, 2024 14:56 View deployment

revert change in local-account-handler

ccdff2a

vercel bot deployed to Preview November 28, 2024 15:19 View deployment

modify chain-id handlers

64634a3

vercel bot deployed to Preview November 28, 2024 15:24 View deployment

update docs for the function createHandlersArray

ce212d4

vercel bot deployed to Preview November 28, 2024 15:29 View deployment

improve if condition in createHandlersArray

df725ab

vercel bot deployed to Preview November 28, 2024 15:36 View deployment

remove "assert.rejects"

647044d

vercel bot deployed to Preview November 28, 2024 15:50 View deployment

ChristopherDedominici commented Nov 28, 2024

View reviewed changes

ChristopherDedominici added status:ready This issue is ready to be worked on and removed status:triaging labels Nov 28, 2024

ChristopherDedominici assigned ChristopherDedominici and unassigned kanej Nov 28, 2024

ChristopherDedominici marked this pull request as ready for review November 28, 2024 16:06

ChristopherDedominici requested a review from alcuadrado November 28, 2024 16:06

ChristopherDedominici mentioned this pull request Dec 5, 2024

micro-eth-signer issue when deploying local accounts #6017

Merged

alcuadrado reviewed Dec 5, 2024

View reviewed changes

v-next/hardhat/src/internal/builtin-plugins/network-manager/hook-handlers/network.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Dec 5, 2024

View reviewed changes

v-next/hardhat/test/internal/builtin-plugins/network-manager/request-handlers/e2e.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Dec 5, 2024

View reviewed changes

removed "if http condition" for the AutomaticGasPriceHandler

2fa55f6

vercel bot deployed to Preview December 6, 2024 11:01 View deployment

ChristopherDedominici and others added 2 commits December 6, 2024 12:06

remove unused "maxFeePerGas" in e2e test

71d8935

Update v-next/hardhat/src/internal/builtin-plugins/network-manager/ho…

406593b

…ok-handlers/network.ts Co-authored-by: Patricio Palladino <[email protected]>

vercel bot deployed to Preview December 6, 2024 11:09 View deployment

ChristopherDedominici added 2 commits December 6, 2024 12:12

Merge branch 'v-next' of github.com:NomicFoundation/hardhat into rewo…

345ad12

…rk-providers

Merge branch 'rework-providers' of github.com:NomicFoundation/hardhat…

f74aae2

… into rework-providers

vercel bot deployed to Preview December 6, 2024 11:13 View deployment

fix WeakMap errors

64ed264

vercel bot deployed to Preview December 6, 2024 11:31 View deployment

ChristopherDedominici requested a review from alcuadrado December 6, 2024 11:33

alcuadrado approved these changes Dec 9, 2024

View reviewed changes

ChristopherDedominici added this pull request to the merge queue Dec 9, 2024

Merged via the queue into v-next with commit 7273a6a Dec 9, 2024
45 checks passed

ChristopherDedominici deleted the rework-providers branch December 9, 2024 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework providers #5994

Rework providers #5994

ChristopherDedominici commented Nov 27, 2024 •

edited

Loading

vercel bot commented Nov 27, 2024 •

edited

Loading

changeset-bot bot commented Nov 27, 2024 •

edited

Loading

ChristopherDedominici Nov 28, 2024

ChristopherDedominici Nov 28, 2024

ChristopherDedominici Nov 28, 2024 •

edited

Loading

alcuadrado Dec 5, 2024

ChristopherDedominici Dec 6, 2024

alcuadrado Dec 5, 2024

ChristopherDedominici Dec 6, 2024 •

edited

Loading

alcuadrado left a comment

		@@ -0,0 +1,271 @@
		import type { HttpNetworkHDAccountsConfig } from "../../../../../src/types/config.js";

Rework providers #5994

Rework providers #5994

Conversation

ChristopherDedominici commented Nov 27, 2024 • edited Loading

vercel bot commented Nov 27, 2024 • edited Loading

changeset-bot bot commented Nov 27, 2024 • edited Loading

⚠️ No Changeset found

ChristopherDedominici Nov 28, 2024

Choose a reason for hiding this comment

ChristopherDedominici Nov 28, 2024

Choose a reason for hiding this comment

ChristopherDedominici Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

alcuadrado Dec 5, 2024

Choose a reason for hiding this comment

ChristopherDedominici Dec 6, 2024

Choose a reason for hiding this comment

alcuadrado Dec 5, 2024

Choose a reason for hiding this comment

ChristopherDedominici Dec 6, 2024 • edited Loading

Choose a reason for hiding this comment

alcuadrado left a comment

Choose a reason for hiding this comment

ChristopherDedominici commented Nov 27, 2024 •

edited

Loading

vercel bot commented Nov 27, 2024 •

edited

Loading

changeset-bot bot commented Nov 27, 2024 •

edited

Loading

ChristopherDedominici Nov 28, 2024 •

edited

Loading

ChristopherDedominici Dec 6, 2024 •

edited

Loading