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

Create skeleton structure for Engine API V2 methods #4876

Merged
merged 5 commits into from
Jan 4, 2023

Conversation

siladu
Copy link
Contributor

@siladu siladu commented Jan 3, 2023

PR description

First step to implementing https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md

Create AbstractEngineNewPayload and AbstractEngineForkchoiceUpdated, extending with V1 and V2 versions.
(AbstractEngineGetPayload and V2 was already introduced in a483f79)

This will be the basis for parallelising getting a productionised version of the Withdrawals draft onto main.

Fixed Issue(s)

Part of #4776

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if
    updates are required.

Changelog

@siladu siladu self-assigned this Jan 3, 2023
@siladu siladu added TeamGroot GH issues worked on by Groot Team mainnet EIP Ethereum Improvement Proposal labels Jan 3, 2023
@siladu siladu changed the title V2 api structure 4776 Create skeleton structure for Engine API V2 methods Jan 3, 2023
@siladu siladu marked this pull request as ready for review January 4, 2023 00:33
@@ -491,7 +503,7 @@ private JsonRpcResponse resp(
new JsonRpcRequestContext(
new JsonRpcRequest(
"2.0",
RpcMethod.ENGINE_FORKCHOICE_UPDATED.getMethodName(),
RpcMethod.ENGINE_FORKCHOICE_UPDATED_V1.getMethodName(),
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be hard-coded to forkchoiceUpdatedV1. Though the way it's used in the test might mean this doesn't matter so much?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot. You're right that it doesn't really matter for these, but I've created a getMethodName template for consistency anyway.

I had done something similar for EngineGetPayload but missed this one.

siladu added 2 commits January 4, 2023 14:24
…esponses

Refactor AbstractEngineGetPayloadTest to use the same pattern

Signed-off-by: Simon Dudley <[email protected]>
@siladu siladu requested a review from jframe January 4, 2023 04:30
Copy link
Contributor

@jframe jframe left a comment

Choose a reason for hiding this comment

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

LGTM

@siladu siladu enabled auto-merge (squash) January 4, 2023 04:42
@siladu siladu merged commit af1ce38 into hyperledger:main Jan 4, 2023
@siladu siladu deleted the v2-api-structure-4776 branch January 4, 2023 04:58
macfarla pushed a commit to macfarla/besu that referenced this pull request Jan 10, 2023
Create AbstractEngineNewPayload and AbstractEngineForkchoiceUpdated, extending with V1 and V2 versions.
(AbstractEngineGetPayload and V2 was already introduced in hyperledger@a483f79)

Signed-off-by: Simon Dudley <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Create AbstractEngineNewPayload and AbstractEngineForkchoiceUpdated, extending with V1 and V2 versions.
(AbstractEngineGetPayload and V2 was already introduced in hyperledger@a483f79)

Signed-off-by: Simon Dudley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EIP Ethereum Improvement Proposal mainnet TeamGroot GH issues worked on by Groot Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants