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

ExecutionClient returns PromiseInterface #2856

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

diegoximenes
Copy link
Contributor

@diegoximenes diegoximenes commented Dec 24, 2024

Resolves NIT-2566

As a preparation for running Consensus and Execution in different processes, in which they communicate through the network, this PR updates ExecutionClient interface functions to return promises.
Later, clients implementing EecutionClient will handle the network communication with Consensus side.

Instead of going over all functions from the different types of execution clients (ExecutionSequencer, ExecutionRecorder, etc), this PR only updates ExecutionClient.
Most alternative execution clients, besides the one that we already have based on geth, will only implement ExecutionClient instead of all execution clients.
So it seems reasonable to scope down and only introduce changes related to ExecutionClient for now.

This PR also introduces TestExecutionClientOnly, that guarantees that using a execution node that only implements functions related to ExecutionClient is enough to handle some scenarios.

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Dec 24, 2024
@diegoximenes diegoximenes marked this pull request as ready for review December 26, 2024 19:17
Copy link
Contributor

@tsahee tsahee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small changes requested. Others can be either made now or on a separate PR

execution/interface.go Outdated Show resolved Hide resolved
execution/gethexec/node.go Outdated Show resolved Hide resolved
execution/interface.go Show resolved Hide resolved
arbnode/node.go Outdated Show resolved Hide resolved
@diegoximenes diegoximenes marked this pull request as draft January 3, 2025 14:04
@diegoximenes diegoximenes requested a review from tsahee January 3, 2025 16:01
@diegoximenes diegoximenes marked this pull request as ready for review January 3, 2025 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants