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

Add engine_forkchoiceUpdatedV4 #608

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mcdee
Copy link

@mcdee mcdee commented Dec 10, 2024

Validator clients have the ability to read a configuration file that provides per-validator values for fee recipient and gas limit, however although execution nodes are passed the fee recipient they are not passed the gas limit. This results in the execution nodes using what users will see as incorrect values.

This change adds a gasLimit field to the payloadAttributes section of the fork choice updated call, which provides this additional information and allows execution nodes to create payloads with the appropriate gas limit.

@g11tech
Copy link
Contributor

g11tech commented Dec 10, 2024

also "/eth/v1/validator/prepare_beacon_proposer" need to have gas limit included in api request as well

@mcdee
Copy link
Author

mcdee commented Dec 10, 2024

also "/eth/v1/validator/prepare_beacon_proposer" need to have gas limit included in api request as well

Yes we'll need to bump the version as well.

That said, the beacon node does obtain this information in /eth/v1/validator/register_validator. Although it isn't a perfect solution by any means the beacon node could obtain the gas limit from here until we have a v2 of prepare_beacon_proposer in place.

@nflaig
Copy link
Member

nflaig commented Dec 10, 2024

That said, the beacon node does obtain this information in /eth/v1/validator/register_validator. Although it isn't a perfect solution by any means the beacon node could obtain the gas limit from here until we have a v2 of prepare_beacon_proposer in place.

maybe a good opportunity to consolidate those apis as suggested in ethereum/beacon-APIs#435

@mcdee
Copy link
Author

mcdee commented Dec 10, 2024

That said, the beacon node does obtain this information in /eth/v1/validator/register_validator. Although it isn't a perfect solution by any means the beacon node could obtain the gas limit from here until we have a v2 of prepare_beacon_proposer in place.

maybe a good opportunity to consolidate those apis as suggested in ethereum/beacon-APIs#435

If the consensus nodes want to go along with the suggestion at ethereum/beacon-APIs#435 (comment) that would certainly simplify things. The edge case of the beacon node not knowing the validator registrations directly after a restart is, though, real so we'd need to consider how to handle this.

@nflaig
Copy link
Member

nflaig commented Dec 10, 2024

If the consensus nodes want to go along with the suggestion at ethereum/beacon-APIs#435 (comment) that would certainly simplify things. The edge case of the beacon node not knowing the validator registrations directly after a restart is, though, real so we'd need to consider how to handle this.

Let's continue that discussion on the beacon-api issue

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants