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

Extend PayloadAttributes with configurable gas_limit #29

Open
ralexstokes opened this issue Jun 8, 2022 · 7 comments
Open

Extend PayloadAttributes with configurable gas_limit #29

ralexstokes opened this issue Jun 8, 2022 · 7 comments

Comments

@ralexstokes
Copy link
Member

If we refer to the current execution Engine API spec for the build inputs: https://github.com/ethereum/execution-apis/blob/main/src/engine/specification.md#payloadattributesv1

we see that there is no way to specify a gas limit.

It is critical for the security of the network that this parameter remains under the control of the validator set and not the builder set (which should be much smaller and possibly not as aligned with ethereum overall).

I think an easy fix is to specify a PayloadAttributesV2 message (h/t @mkalinin ) that simply appends a gas_limit field:

* gasLimit: QUANTITY, 64 Bits - value for the block gas limit of the new payload
@mkalinin
Copy link
Contributor

I think it is reasonable for a validator to be capable of controlling the gas_limit. But only in the case when VC operator clearly understands what effect does this parameter may take and how can it affect the network. Considering this, I would make gas_limit configuration and Engine API parameter optional. With having it optional not experienced stakers could rely on EL and avoid misconfigurations that are potentially dangerous to the network.

Based on what @ralexstokes has proposed above, Engine API changes may look as follows:

### PayloadAttributesV2
- `timestamp`: `QUANTITY`, 64 Bits - value for the `timestamp` field of the new payload
- `prevRandao`: `DATA`, 32 Bytes - value for the `prevRandao` field of the new payload
- `suggestedFeeRecipient`: `DATA`, 20 Bytes - suggested value for the `feeRecipient` field of the new payload
- `gasLimit`: `QUANTITY`, 64 Bits - value for the block gas limit of the new payload

### engine_forkchoiceUpdatedV2

#### Request

* method: "engine_forkchoiceUpdatedV2"
* params: 
  1. `forkchoiceState`: `Object` - instance of [`ForkchoiceStateV1`](#ForkchoiceStateV1)
  2. `payloadAttributes`: `Object|null` - instance of [`PayloadAttributesV2`](#PayloadAttributesV2) or `null`
* timeout: 8s

This change could be deployed as a soft fork in two steps: i) all EL clients release a support of this feature, nodes are upgraded ii) CL clients are released.

I feel like we really need to think about versioning in Engine API and make CL follow Postel's law (echoing @MicahZoltu). This would probably require additional status message but would make soft fork deployment smooth.

@MicahZoltu
Copy link

The execution layer is (presumably) under control of validators, and the execution layer having control doesn't automatically mean that builders control the value. I think we would be better off here just communicating with execution client devs that they shouldn't be letting builders decide gas limit.

@mkalinin
Copy link
Contributor

@ralexstokes How is this change related to gas_limit parameter in the validator registration message of Builders API?

@protolambda
Copy link

I definitely support adding gas_limit, and ideally extra_data too. These things can change per block, so should not rely on static configuration of the engine.

If there will be a V2 payload attributes type, I think it would also be really useful to have the following two payload attributes fields:

  • transactions: an array of bytestrings, to always process in the block (any tx pool transactions would be processed after these).
  • noTxPool: a boolean, to not include any external transactions from the pool (above transactions would still be included, if any).

Note that Geth internally already has a noTxPool-like flag, since it builds an empty block first, and then a block with tx pool asynchronously.

Optimism, and any other L2 that likes to use the standard Engine API, can use these fields to include transactions for deposit/system purposes. We have an open spec here: https://github.com/ethereum-optimism/optimistic-specs/blob/main/specs/exec-engine.md#extended-payloadattributesv1 and implemented it in our geth fork with a very small diff from upstream. Builder infrastructure can be compatible with Optimism with this small change.

And with testing, we can make an engine play through old history of inputs. The engine API newPayload is not sufficient, since it also includes block-outputs as request-inputs. The block outputs may not be always available, or the available data may not be consistent (e.g. a client produced a bad chain of blocks, and another client is tasked with computing the correct state-roots).

Smart-contract tooling would also be able to use these fields to use a real client to build a local chain, step by step, for testing. This way tx inclusion and timestamps can be deterministic in the test, unlike running an engine with clique+miner, which mines empty blocks and randomly includes things from the user RPC.

@ralexstokes
Copy link
Member Author

@mkalinin @MicahZoltu sorry for not providing the context here:

if we add this parameter then we will be able to re-use the Engine API in the context of the external builder network.

currently:

builders in this network acquire the gas limits via the ValidatorRegistrations and they MUST follow the validator's preference here when they are building a block for them to proposer. it is not specified how the builder will go about building this block and it is assumed they will be using custom software to do so.

with this proposal:
if we extend the Engine API so that it includes all of the building parameters then external block builders can re-use off-the-shelf building software (e.g. geth) without having to develop custom software (or at least as much custom software). this lowers the barrier to entry, keeping the builder market in healthy competition.

note:

  • builders can still get validator pref's ahead of time and use custom software to know about the gas limit they should be targeting before they would be able know based on the implied timing of the Engine API (which is "just in time")
  • this parameter does not replace the gas_limit in the ValidatorRegistration, it simply supplements it
  • this parameter does not directly influence any local building a proposer may be doing concurrent with an external block pipeline (note the status quo is likely just running geth which has a single, process-wide gas limit parameter -- if you have multiple validators using one EL then there is also no easy way for them to have differing gas limit preferences but this is not a big deal as we can assume in this setting all of the validators are controlled by one logical entity who wishes to have a uniform gas limit)

@zah
Copy link

zah commented Aug 30, 2022

My impression is that there is a wide-spread confusion in the community regarding how this parameter should be configured.

It would be useful if someone tries to enumerate the various rational (and perhaps adversarial) strategies for setting the value of the parameter. The honest validator document present in this repo can potentially provide some guidelines as well.

@MicahZoltu
Copy link

TL;DR: set it to 30,000,000 and if there is an active DoS attack against the network that can be mitigated by lowering the gas limit (this will likely be communicated publicly by Ethereum Core developers), then update your configuration at that time to lower it until the developers can get patches out.

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

5 participants