Implement Execution/Consensus interface over RPC #3617

KolbyML · 2025-09-10T22:47:24Z

Fixes NIT-2567

I have implemented the JSON-RPC interface for the consensus and execution layer.

I setup a basic cli-arg setup has --node.interconnect which has 2 options

direct the default uses the pre-existing interfaces
jsonrpc runs the Consensus and Execution interfaces over a json rpc

I run direct and it works fine.

jsonrpc fails with "failed to create node err="not connected" which to fix this I assume we need to ensure

the client is connecting to the right jsonrpc server
start the clients after the server is started

^ so basically ensuring the server's are running and properly connect before trying to use the interfaces

In terms of scope Have configuration to use existing interface or new RPC interface I set things up with this in mind. In the future I assume that we will allow users to specify running exclusively the consensus side so that a different execution layer client can be chosen, but to my knowledge that task was out of scope of this issue/pr and will definitely require some re-arranging and refactoring of how things are initialized.

So I believe this Issue is more of a V1 before we add more complexity to how the node is initialized.

If anybody has any questions feel free to ask me

Updated description

#3617 (comment)

…on rpc server and connecting to one on both execution and consensus side

… starting it

ganeshvanahalli · 2025-09-18T21:19:23Z

This PR has been changed from its initial design. Execution and Consensus each have capability to enable connections to them via json-rpc through the flags --execution.rpc-server.* and --node.rpc-server.* respectively, and they are allowed to connect to external such counterparts via json-rpc facilitated by flags --execution.consensus-rpc-client.* and --node.execution-rpc-client.* respectively.

Nitro has two new flags that determine

--execution-node=<bool>
--consensus-execution-use-rpc=<bool>

if an execution node is to be created or instead consensus would prefer connecting over an RPC to execution.ExecutionClient interface implementer and
if we have execution and consensus nodes both starting then if they should communicate over rpc (used for testing) respectively

Testing done

A new CI step has been added to run current system tests with json rpc interconnect enabled.

Resolves NIT-2567

diegoximenes

Nice 🙂, this is quite important for the alternative clients effort!

The way I see the UX on how to run a Nitro node, implemented in this repo, is something like this:

Node operator runs only Execution ./nitro --consensus-node=false --execution-node=true. In that case Execution will connect with Consensus through RPC. We don't even create a arbnode.Node object in this scenario.
Node operator runs only Consensus ./nitro --consensus-node=true --execution-node=false. In that case Consensus will connect with Execution through RPC.
Node operator runs Execution and Consensus ./nitro --execution-node=true --consensus-node=true --consensus-execution-use-rpc=bool. A single process will run both Execution and Consensus.

I don't like the --consensus-execution-use-rpc flag on option 3 BTW, but it is worth having it for testing, at least for a while.
I don't see an advantage of running Option 3 over RPC in production if we have an option to have Consensus and Execution living in the same process.
But without this flag, to test this PR we would need to run two ./nitros, have an external component similar to consensusexecution_consensus.InitAndStartExecutionAndConsensusNodes, coordinating Execution/Consensus to handle the egg/chicken problem related to which component to start first.

It is a little bit different from what this PR is proposing.
WDYT?

arbnode/blockmetadata.go

diegoximenes · 2025-09-22T14:58:25Z

arbnode/node.go

 	ConsensusExecutionSyncer: DefaultConsensusExecutionSyncerConfig,
 	SnapSyncTest:             DefaultSnapSyncConfig,
+	RPCServer:                DefaultRPCServerConfig,
+	ExecutionRPCClient: rpcclient.ClientConfig{


Any specific reason for not using rpcclient.DefaultClientConfig?

rpcclient.DefaultClientConfig has URL = "self-auth" we dont want that as default so

diegoximenes · 2025-09-22T15:10:58Z

arbnode/node.go

+	if configFetcher.Get().ExecutionRPCClient.URL != "" {
+		execConfigFetcher := func() *rpcclient.ClientConfig { return &configFetcher.Get().ExecutionRPCClient }
+		executionClient = executionrpcclient.NewExecutionRpcClient(execConfigFetcher, nil)
+	}


nitpick: This code block is replicated in CreateNodeFullExecutionClient.
How about abstracting that into a function and calling it here and in CreateNodeFullExecutionClient?

implementation has changed now and I feel like this is a small enough code diff to not warrant defining a function over...

cmd/nitro/nitro.go

diegoximenes · 2025-09-22T17:13:09Z

consensus/interface.go

+
+var ErrSequencerInsertLockTaken = errors.New("insert lock taken")
+
+type MessageResult struct {


Why is this defined in consensus instead of execution?

not sure, i've removed it

consensus/rpcclient/client.go

consensus/rpcserver/server.go

diegoximenes · 2025-09-26T12:34:03Z

To keep in mind:

The main goal of this PR is to enable our current Nitro golang Consensus node to run with Erigon and Nethermind execution clients. The first milestone for Erigon and Nethermind is to only implement simple execution clients, i.e., execution clients that implement the ExecutionClient interface, so we are not worried about other behaviors that are described by the ExecutionRecorder/ExecutionSequencer/ExecutionBatchPoster interfaces.
Also, we should not be worried, for now at least, about the scenario in which our current Nitro golang Execution node connects with a Consensus node that is not the one that is implemented in this repo.

By not worry I mean that we don't need to cover them in this PR, but we should definitely consider that those scenarios, could, and ideally will happen 🙂, so changes here shouldn't make our lives too difficult when covering them in the future.

There is this test, in which is possible to create a Nitro node with an execution client that only implements the ExecutionClient interface, and not ExecutionRecorder, etc.
It uses CreateNodeExecutionClient, instead of CreateNodeFullExecutionClient, under the hood.

That said, for this PR, the minimal requirements could be:

If --consensus-node==false then fail during config validation step. Or don't even add this --consensus-node config for now, it can be easier 🙂
If --execution-node==false

We can assume that there will be an external execution client interacting with this Consensus node.
Use CreateNodeExecutionClient instead of CreateNodeFullExecutionClient
Consensus will need to have an RPC server so an external execution client can "connect" to it.
We should avoid creating an execution.ExecutionNode object, or at least not set the consensus client to the execution.ExecutionNode object. It can become a weird situation if this ExecutionNode starts calling Consensus, which will already be called by the external execution client. Two execution clients will be connected with this Consensus node, it can become a mess 😬.

If --execution-node==true and --consensus-execution-use-rpc==true then:

We will need to validate Execution config, to fail if it is trying to use Sequencer/Recorder/BatchPoster behavior.
use CreateNodeExecutionClient instead of CreateNodeFullExecutionClient
Consensus will need to have an RPC server
Execution will need to have an RPC server

Else we do what is already being done on master.

I am OK with covering all execution behaviors with the RPC approach right now TBH, it seems the way to go long term wise, but it is definitely not a requirement.
In this way CreateNodeFullExecutionClient can still be used for those scenarios, and there will be less config validation steps.
Code diff of this PR will be a little bit bigger though.
If you want to follow this approach I would first get I buy in from Tsahi, he mentioned some times that he doesn't want to implement the RPC interfaces for other behaviors right now.

We could have the same behavior without the --execution-node and --consensus-execution-use-rpc flags BTW, only relying on the rpc flags such as --execution-rpc-server and --execution-rpc-client.
But it becomes less explicit, and more confusing IMO, e.g.:

--execution-rpc-server and --execution-rpc-client are set is analogous to: --execution-node==true and --consensus-execution-use-rpc==true
--execution-rpc-server is not set and --execution-rpc-client is set is analogous to: --execution-node==false

WDYT?

ganeshvanahalli · 2025-09-26T19:50:30Z

To keep in mind:

...

I've addressed your comments and added two new flags to nitro -

--execution-node=<bool>
--consensus-execution-use-rpc=<bool>

that determine if an execution node is to be created or instead consensus would prefer connecting over an RPC to execution.ExecutionClient interface implementer and
if we have execution and consensus nodes both starting then if they should communicate over rpc (used for testing)

KolbyML and others added 9 commits September 10, 2025 16:12

Implement Execution/Consensus interface over RPC

4f132e9

Merge branch 'master' into Execution/Consensus-interface-over-RPC

969a1b6

Merge branch 'master' into Execution/Consensus-interface-over-RPC

8219a5e

update implementation, separate out responsibilities of starting a js…

2a18ad2

…on rpc server and connecting to one on both execution and consensus side

fix CI flag

eaad44a

Merge branch 'master' into Execution/Consensus-interface-over-RPC

901dbea

bug fix

18097ee

Merge branch 'master' into Execution/Consensus-interface-over-RPC

f54a0f0

fix failing tests and refactor code to not use executionClient before…

eeb55d3

… starting it

ganeshvanahalli marked this pull request as ready for review September 18, 2025 22:11

ganeshvanahalli requested review from diegoximenes, magicxyyz and tsahee September 18, 2025 22:11

ganeshvanahalli assigned diegoximenes Sep 18, 2025

diegoximenes requested changes Sep 23, 2025

View reviewed changes

diegoximenes assigned ganeshvanahalli and unassigned diegoximenes Sep 23, 2025

merge master and resolve conflicts

f35e01b

ganeshvanahalli added 2 commits September 26, 2025 14:43

address PR comments

acb0165

address PR comment

baca94a

ganeshvanahalli assigned diegoximenes and unassigned ganeshvanahalli Sep 26, 2025

ganeshvanahalli requested a review from diegoximenes September 26, 2025 21:06

fix challenge tests

61b8b17

tsahee assigned tsahee and unassigned diegoximenes Oct 8, 2025

merge upstream master

55e3d05

ganeshvanahalli added 4 commits October 8, 2025 12:09

minor fix

7c9c449

bugfix

e9b35e3

Merge branch 'master' into Execution/Consensus-interface-over-RPC

6894bbe

Merge branch 'master' into Execution/Consensus-interface-over-RPC

b71389d


		var ErrSequencerInsertLockTaken = errors.New("insert lock taken")

		type MessageResult struct {

Implement Execution/Consensus interface over RPC #3617

Are you sure you want to change the base?

Implement Execution/Consensus interface over RPC #3617

Conversation

KolbyML commented Sep 10, 2025 • edited by ganeshvanahalli Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Updated description

Uh oh!

ganeshvanahalli commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing done

Uh oh!

diegoximenes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

diegoximenes Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

ganeshvanahalli Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

diegoximenes Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

ganeshvanahalli Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

diegoximenes Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

ganeshvanahalli Sep 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

diegoximenes commented Sep 26, 2025

Uh oh!

ganeshvanahalli commented Sep 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KolbyML commented Sep 10, 2025 •

edited by ganeshvanahalli

Loading

ganeshvanahalli commented Sep 18, 2025 •

edited

Loading