Node network config#7989
Conversation
🦋 Changeset detectedLatest commit: 6a5f25d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new node network configuration that is automatically added to all Hardhat projects. The node network is similar to the default network but with a key difference: its type must always be edr-simulated and cannot be overridden to http or any other type.
Changes:
- Added automatic extension of user config to include a
nodenetwork withedr-simulatedtype - Added validation to enforce that the
nodenetwork type must beedr-simulated - Updated the
nodetask action to use the newnodenetwork instead of respecting the global--networkoption
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| v-next/hardhat/src/internal/builtin-plugins/network-manager/hook-handlers/config.ts | Extends user config to add the node network with default EDR network values |
| v-next/hardhat/src/internal/builtin-plugins/network-manager/type-validation.ts | Adds validation to ensure node network type is always edr-simulated |
| v-next/hardhat/src/internal/builtin-plugins/node/task-action.ts | Simplifies node task to use hardcoded node network and assertion |
| v-next/hardhat/test/internal/builtin-plugins/network-manager/hook-handlers/config.ts | Adds comprehensive tests for node network extension and validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alcuadrado
force-pushed
the
node-network-config
branch
from
5fa4743 to
e720f20
Compare
| override?: EdrNetworkConfigOverride; | ||
| } = { | ||
| network, | ||
| network: "node", |
There was a problem hiding this comment.
Does this mean we can’t run npx hardhat node --network myNodeNetwork anymore?
There was a problem hiding this comment.
That's right. That's ignored. Maybe we should print warning or throw an error
There was a problem hiding this comment.
Or maybe use node just by default. Is that possible?
There was a problem hiding this comment.
It wouldn't be possible, because --network has a default value of "default"
There was a problem hiding this comment.
I guess if users want to be able to override we can add a task option --network-config or something like that in the future.
There was a problem hiding this comment.
can't you do?
const network =
hre.globalOptions.network !== undefined
? hre.globalOptions.network
: hre.config.network.node;
| assert.ok( | ||
| extendedConfig.networks?.node !== undefined, | ||
| "node network should be defined", | ||
| ); |
There was a problem hiding this comment.
Minor: this assertion is redundant, since the next one would fail if the node network is undefined. I'm fine if you want to keep it, as it makes the intent more explicit.
| /* eslint-disable-next-line @typescript-eslint/consistent-type-assertions | ||
| -- testing invalid network type for js users */ | ||
| const validationErrors = await validateNetworkUserConfig(config as any); |
schaable
left a comment
schaable
left a comment
There was a problem hiding this comment.
Looks good. Left a question and some minor comments on the tests.
This network config is validated to be of type `edr-simulated` during the config validation phase.
alcuadrado
force-pushed
the
node-network-config
branch
from
67b7545 to
78b76a4
Compare
alcuadrado
force-pushed
the
node-network-config
branch
from
78b76a4 to
5c2d49d
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
This has been released as part of |
This PR introduces a new
nodenetwork config, which is present by default, and whose type must always beedr-simulated. This last requirement is enforced during the validation of the config.Depends on: #7805
Docs PR: NomicFoundation/hardhat-website#222