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

Clarify Gas Limit in Register Validator endpoint #17

Closed
james-prysm opened this issue May 17, 2022 · 2 comments
Closed

Clarify Gas Limit in Register Validator endpoint #17

james-prysm opened this issue May 17, 2022 · 2 comments

Comments

@james-prysm
Copy link

james-prysm commented May 17, 2022

I wanted to ask to what extent ( or up to the discretion of the validator clients) should we warn users on invalid Gas Limits? or if the builder will handle with something more sensible and is the responsibility is on the builder?

As discussed previously I believe client teams are using the 30M as the default but perhaps that's simply an implementation detail ( instead of the previous block? or whatever is higher?).
note: would need to release to avoid lowering the limit by default if the network settles on something different.

For user overrides what were we thinking ( I just hope an error isn't thrown back to us) for the intended flow on a bad range for gas limit. Sending something very large will result in you not being able to include that many transactions in a block and sending too little the block is not profitable. Will those be adjusted somehow or is it totally up to the validator client.

@ralexstokes
Copy link
Member

ralexstokes commented May 18, 2022

30M is the value "we" currently feel comfortable running as a balance between how big blocks are and how expensive they are for the network to validate. When I say "we", I mean the collective community of client devs, miners and others in the community -- rough consensus across many participating entities.

For this reason, I think it is currently a sensible default if the validator operator does not provide another option. I personally don't see this value radically changing anytime soon so I am not super concerned about arguments around "what if we need to change and that implies a software release cycle".

Setting aside the default, the validator operator is free to supply whatever number they want within the bounds of a 64-bit unsigned integer. You could consider adding warnings for values that are too low (likely user mistake) or too high but it is important for the health of the network that validators get to make this decision and have a path for expressing their preferences in the protocol. You could set a gas floor of 100,000 and a ceiling of 20% over the default as conditions for raising a warning.

From there, I think it is critical the builder follows whatever is in the validator registration. Otherwise, we provide builders with even more influence over the network (and they already have a lot). This means if a builder sees a value of 60M against an ambient gas limit of 30M, they must move 1/1024 from the current limit towards this larger limit. I think any problem of accidentally raising the limit due to misconfiguration is limited due to the ability to only gradually change the limit per block in either direction. We don't have the infra in place to do in-protocol slashing for builder misbehavior yet but I intend to push in that direction and it becomes enshrined with any sort of in-protocol proposer-builder separation.

@james-prysm
Copy link
Author

I'll decide with my team on how to warn users and just follow these guidelines thanks.

bors bot pushed a commit to sigp/lighthouse that referenced this issue Jun 30, 2022
## Issue Addressed

Lays the groundwork for builder API changes by implementing the beacon-API's new `register_validator` endpoint

## Proposed Changes

- Add a routine in the VC that runs on startup (re-try until success), once per epoch or whenever `suggested_fee_recipient` is updated, signing `ValidatorRegistrationData` and sending it to the BN.
  -  TODO: `gas_limit` config options ethereum/builder-specs#17
-  BN only sends VC registration data to builders on demand, but VC registration data *does update* the BN's prepare proposer cache and send an updated fcU to  a local EE. This is necessary for fee recipient consistency between the blinded and full block flow in the event of fallback.  Having the BN only send registration data to builders on demand gives feedback directly to the VC about relay status. Also, since the BN has no ability to sign these messages anyways (so couldn't refresh them if it wanted), and validator registration is independent of the BN head, I think this approach makes sense. 
- Adds upcoming consensus spec changes for this PR ethereum/consensus-specs#2884
  -  I initially applied the bit mask based on a configured application domain.. but I ended up just hard coding it here instead because that's how it's spec'd in the builder repo. 
  -  Should application mask appear in the api?



Co-authored-by: realbigsean <sean@sigmaprime.io>
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

No branches or pull requests

2 participants