search

paradigmxyz / reth

Modular, contributor-friendly and blazing-fast implementation of the Ethereum protocol, in Rust
https://reth.rs/
Apache License 2.0
3.9k stars 1.14k forks source link

Allow opting out of beacon engine timestamp check #11651

Open emostov opened 1 week ago

emostov commented 1 week ago

Describe the feature

To support sub second block times, it would be helpful to disable the timestamp check here

https://github.com/paradigmxyz/reth/blob/1ba631ba9581973e7c6cadeea92cfe1802aceb4a/crates/consensus/beacon/src/engine/mod.rs#L1174-L1176

The Ethereum engine api spec enforces that the timestamp on new execution block must be greater then the previous block. Execution block timestamps are denominated in seconds, so if blocks are being requested within a second of each other they are likely to have the exact same timestamp and Reth will not create/validate the block.

To quickly fix this I propose allowing a way to opt out of the timestamp check - either with a runtime configuration or compilation feature gate. @mattsse mentioned here that they have some pointers on how to tackle this.

Additional context

No response

### Tasks
nadtech-hub commented 1 week ago

Do you mind me working on this one?

mattsse commented 1 week ago

the way this should be done is by extending

https://github.com/paradigmxyz/reth/blob/0dbc37463964e6985910a58008113e4c90eb506d/crates/engine/primitives/src/lib.rs#L45-L45

with checks for payload against header and then plugged into the engine impl

emostov commented 6 days ago

@mattsse thanks for the response! I think I might be missing something. From my read, implementing EngineValidator will allow me to add custom validation at the RPC level but will not allow me bypass the timestamp validation done by the engine.

Specifically in the case of forkchoice updated, the ensure_well_formed_attributes is called here

https://github.com/paradigmxyz/reth/blob/0dbc37463964e6985910a58008113e4c90eb506d/crates/rpc/rpc-engine-api/src/engine_api.rs#L601

which then uses the consensus engine handle to normally call the new engine-tree logic here:

https://github.com/paradigmxyz/reth/blob/0dbc37463964e6985910a58008113e4c90eb506d/crates/rpc/rpc-engine-api/src/engine_api.rs#L617

In the engine-tree logic it appears the payload is still unconditionally subject to the timestamp check here

https://github.com/paradigmxyz/reth/blob/0dbc37463964e6985910a58008113e4c90eb506d/crates/engine/tree/src/tree/mod.rs#L2491

Which is executed by the BeaconEngineMessage::ForkchoiceUpdated request handler

https://github.com/paradigmxyz/reth/blob/0dbc37463964e6985910a58008113e4c90eb506d/crates/engine/tree/src/tree/mod.rs#L970

Is my understanding correct here?

EDIT: rereading your comment again, is your suggestion to create a PR that adds a method to the EngineValidator trait for checking the payload against the header and then calling that new method in process_payload_attributes instead of doing the timestamp check? If so, I can work on that

mattsse commented 6 days ago

ereading your comment again, is your suggestion to create a PR that adds a method to the EngineValidator trait for

I thought about this a bit, and I think we could start introducing a different trait for this entirely like a new validator trait or similar

emostov commented 5 days ago

Makes sense. I will try and work on something later this week. Feel free to assign it to me