ChainSafe / lodestar

🌟 TypeScript Implementation of Ethereum Consensus
https://lodestar.chainsafe.io
Apache License 2.0
1.14k stars 278 forks source link

Do early notifyNewPayload call to execution engine #6381

Closed twoeths closed 1 month ago

twoeths commented 7 months ago

Problem description

As an execution client, Nethermind expects consensus client to call notifyNewPayload as soon as possible.

Solution description

Prysm already does that almost right after they receive gossip block https://github.com/prysmaticlabs/prysm/blob/373c853d17300a0a96e3ceca0ebb77683b6ebc1f/beacon-chain/blockchain/receive_block.go#L105

while lodestar only call notifyNewPayload api after it passes gossip validation, we should be able to do the same to Prysm right after we receive a gossip block. This is expected to improve block time to head which improve attestation performance too.

Additional context

No response

ensi321 commented 7 months ago

As an execution client, Nethermind expects consensus client to call notifyNewPayload as soon as possible.

Do you mean all EL clients expect new payload as soon as CL client receives a gossip block, or is it only Nethermind?

I might need to look up if other non-prysm clients have this mechanism in place

philknows commented 7 months ago

We only discussed this with Nethermind as they ran tests for which consensus client connected to them was delivering newPayload the fastest. My assumption as that all EL clients should expect this ASAP so they can be fast for attestation but we have purposefully delayed this until after gossip validation whereas Prysm does not. The results showed that Prysm was first in delivering the calls ~75% of the time, Teku second about ~20% of the time and Lighthouse 3rd at ~5% of the time.

twoeths commented 1 month ago

this is not as high priority as it seems to be, now the "till become head" depends a lot on how early blob comes since deneb

philknows commented 1 month ago

telegram-cloud-photo-size-4-5803356877348782105-y

Some analysis also from today by Nethermind on our attestation metrics without having tried this. If there's no real benefit to experimenting with doing this task now that it's dependent on blob arrivals, we could close this. If the effort is low to see what benefits we might get, it might still be worth trying though. However, will note the lower priority requirement here.

twoeths commented 1 month ago

with EIP-7732, we'll execution payload is processed separately from block so we don't need this anymore, closing the issue cc @philknows

see https://eips.ethereum.org/EIPS/eip-7732