Split node task in subtasks #874

fvictorio · 2020-10-13T21:04:46Z

No description provided.

packages/hardhat-core/src/builtin-tasks/node.ts

alcuadrado · 2020-10-14T13:16:59Z

I think the only missing thing is creating a new provider if the network is not hardhat.

Can you take a look at this @wighawag? Thanks!

Also, it would be great to give the users a way to stop the node, but I'm not sure how we can do it. Any ideas?

fvictorio · 2020-10-14T14:27:30Z

Also, it would be great to give the users a way to stop the node, but I'm not sure how we can do it. Any ideas?

I thought the same thing. One (not very elegant) solution is for the server to check all requests before delegating them to the handler and, if the request is hardhat_close, the server calls this.close.

packages/hardhat-core/src/builtin-tasks/node.ts

alcuadrado · 2020-10-14T15:13:24Z

I thought the same thing. One (not very elegant) solution is for the server to check all requests before delegating them to the handler and, if the request is hardhat_close, the server calls this.close.

Yes, that's the best I can come up with TBH. Maybe just make it super explicit, like hardhat_closeJsonRpcServer? And I think I wouldn't forward it to the provider.

fvictorio · 2020-10-14T17:44:12Z

@alcuadrado this should be ready for review

packages/hardhat-core/src/builtin-tasks/node.ts

alcuadrado · 2020-10-14T20:11:18Z

packages/hardhat-core/src/builtin-tasks/node.ts

@@ -118,7 +257,7 @@ task(TASK_NODE, "Starts a JSON-RPC server on top of Hardhat Network")
        console.log();


I think these console.log calls should be placed in TASK_NODE_SERVER_READY

alcuadrado · 2020-10-14T20:11:32Z

packages/hardhat-core/src/builtin-tasks/node.ts

@@ -137,6 +276,8 @@ task(TASK_NODE, "Starts a JSON-RPC server on top of Hardhat Network")
        const networkConfig = config.networks[HARDHAT_NETWORK_NAME];
        logHardhatNetworkAccounts(networkConfig);


Also, this should be in TASK_NODE_SERVER_READY

alcuadrado · 2020-10-14T20:13:39Z

packages/hardhat-core/src/types/builtin-tasks/node.ts

+import { EIP1193Provider } from "../provider";
+
+export interface JsonRpcServer {
+  getProvider(name: string): EIP1193Provider;


This is only for testing purposes, so I don't think it should be placed here.

Fixed 👍 close isn't used in the task either, but I guess it makes sense to have it in the interface anyway, wdyt?

Yes, I think that can be really useful for people overriding the tasks.

alcuadrado · 2020-10-14T20:14:26Z

packages/hardhat-core/src/builtin-tasks/node.ts

@@ -137,6 +276,8 @@ task(TASK_NODE, "Starts a JSON-RPC server on top of Hardhat Network")
        const networkConfig = config.networks[HARDHAT_NETWORK_NAME];
        logHardhatNetworkAccounts(networkConfig);

+        await run(TASK_NODE_SERVER_READY, { provider, server });


This task should receive the hostname and actual port.

alcuadrado · 2020-10-14T20:14:49Z

packages/hardhat-core/src/builtin-tasks/node.ts


-        const server = new JsonRpcServer(serverConfig);
+        await run(TASK_NODE_SERVER_CREATED, { provider, server });


This plugin should receive the hostname and port

wighawag · 2020-10-14T21:59:32Z

I had a quick look, but this looks great!
this would be all I need to implement my node override.

is the log enabled when the server start or on when the provider is created ?
I am asking because if it is only when the server start, I might never need to disable it as I could execute the deployment in TASK_NODE_GET_PROVIDER and then the server can start like usual.

alcuadrado · 2020-10-14T22:42:28Z

Thanks for your comments, @wighawag.

Right now, you should override the get provider task, disable logging, do your things, and re enable logging.

BTW, i think we should add the new method to this array, @fvictorio https://github.com/nomiclabs/buidler/blob/2.0/packages/hardhat-core/src/internal/hardhat-network/provider/provider.ts#L45 -- Otherwise the hardhat_setLoggingEnabled call will be logged.

Co-authored-by: Patricio Palladino <[email protected]>

fvictorio · 2020-10-15T17:30:27Z

packages/hardhat-core/src/internal/sentry/transport.ts

-      const { verbose = false, configPath } = event.extra ?? {};
+      const extra: { verbose?: boolean; configPath?: string } =
+        event.extra ?? {};
+      const { verbose = false, configPath } = extra;


I'm not sure why this stopped compiling, maybe the sentry dependency was updated for some reason?

Anyway, it seems like event.extra values are unknown now, so this is necessary.

I've seen this before. I think sentry is a bit sloppy with semver

fvictorio added 6 commits October 13, 2020 14:22

Add subtasks for creating the provider and the server

457de15

Avoij creating a new provider

6056633

Use network.provider when it corresponds to hardhat network

5deaccd

Add RPC for enabling logging

a7372eb

Add subtask hooks in the node task

e46b8a3

Merge branch '2.0' into split-node-task

2c72f91

fvictorio commented Oct 13, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

packages/hardhat-core/src/builtin-tasks/node.ts Show resolved Hide resolved

Add fork params to node task

c4f6d59

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Show resolved Hide resolved

fvictorio added 5 commits October 14, 2020 11:10

Create provider when network is not hardhat

525f67f

Add params to TASK_NODE_GET_PROVIDER

a93fcf8

Enable logging after reset

2410d2e

Simplify condition for throwing JSONRPC_UNSUPPORTED_NETWORK

e614962

Add interface for JsonRpcServer

375a31a

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

fvictorio added 3 commits October 14, 2020 12:21

Simplify logic when creating the provider

9b20f94

Make fork params optional in subtask

e747103

Fix wrong network name

3a9ece5

fvictorio force-pushed the split-node-task branch from 7bbcfa0 to 3a9ece5 Compare October 14, 2020 17:27

Use 0.0.0.0 as default hostname when inside docker

dc2d57c

alcuadrado reviewed Oct 14, 2020

View reviewed changes

packages/hardhat-core/src/builtin-tasks/node.ts Outdated Show resolved Hide resolved

alcuadrado reviewed Oct 14, 2020

View reviewed changes

fvictorio added 2 commits October 14, 2020 18:32

Merge branch '2.0' into split-node-task

d6417d1

Fix linter and error-list test

cdb92ea

fvictorio and others added 7 commits October 14, 2020 21:03

Add hardhat_setLoggingEnabled to private rpc methods

d38b9bb

Update packages/hardhat-core/src/builtin-tasks/node.ts

69b541e

Co-authored-by: Patricio Palladino <[email protected]>

Move logs to the TASK_NODE_SERVER_READY subtask

5669d83

Remove getProvider from JsonRpcServer interface

2306026

Add hostname and port params to node subtask

18026e3

Remove unnecessary import

10381fa

Fix typecheck problem

05e32f4

fvictorio commented Oct 15, 2020

View reviewed changes

fvictorio added 2 commits October 15, 2020 15:23

Merge branch '2.0' into split-node-task

ab99200

Fix error code numbers

ff66873

alcuadrado merged commit 29b1c5e into 2.0 Oct 15, 2020

alcuadrado deleted the split-node-task branch October 15, 2020 20:11

github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split node task in subtasks #874

Split node task in subtasks #874

fvictorio commented Oct 13, 2020

alcuadrado commented Oct 14, 2020

fvictorio commented Oct 14, 2020

alcuadrado commented Oct 14, 2020

fvictorio commented Oct 14, 2020

alcuadrado Oct 14, 2020

alcuadrado Oct 14, 2020

alcuadrado Oct 14, 2020

fvictorio Oct 15, 2020

alcuadrado Oct 15, 2020

alcuadrado Oct 14, 2020

alcuadrado Oct 14, 2020

wighawag commented Oct 14, 2020

alcuadrado commented Oct 14, 2020

fvictorio Oct 15, 2020

alcuadrado Oct 15, 2020

		@@ -118,7 +257,7 @@ task(TASK_NODE, "Starts a JSON-RPC server on top of Hardhat Network")
		console.log();

		@@ -137,6 +276,8 @@ task(TASK_NODE, "Starts a JSON-RPC server on top of Hardhat Network")
		const networkConfig = config.networks[HARDHAT_NETWORK_NAME];
		logHardhatNetworkAccounts(networkConfig);


		const server = new JsonRpcServer(serverConfig);
		await run(TASK_NODE_SERVER_CREATED, { provider, server });

Split node task in subtasks #874

Split node task in subtasks #874

Conversation

fvictorio commented Oct 13, 2020

alcuadrado commented Oct 14, 2020

fvictorio commented Oct 14, 2020

alcuadrado commented Oct 14, 2020

fvictorio commented Oct 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wighawag commented Oct 14, 2020

alcuadrado commented Oct 14, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment