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

Execution Layer Meeting 187 #1029

Closed
timbeiko opened this issue Apr 27, 2024 · 7 comments
Closed

Execution Layer Meeting 187 #1029

timbeiko opened this issue Apr 27, 2024 · 7 comments

Comments

@timbeiko
Copy link
Collaborator

timbeiko commented Apr 27, 2024

Meeting Info

Agenda

@etan-status
Copy link

etan-status commented Apr 29, 2024

EIP-6110:

  • The proposed deposit flow is to start from the Log events of the Receipt, then to convert it from little endian to big endian, then to RLP into an opaque byte buffer, then prefixed into EIP-7685 "general purpose execution layer request".
  • These are then transformed to JSON for engine API, dropping the prefix once more, and finally end up in a deposit_receipts list in the CL ExecutionPayload.
  • Because EIP-7685 does not preserve ordering, extra index field also needs to be pushed along.

This makes me wonder:

  • Is EIP-7685 needed? I think it would be simpler design to keep CL ExecutionPayload and EL block header close together (as was the case, so far). As in, simply adding the deposit_receipts list to both CL ExecutionPayload as well as its root to the EL block header.
  • If additional operations come up lateron (withdrawals / consolidation, and later down maybe transfers / key changes), additional lists could be used for that, or a StableContainer could be used that supports generic validator_operations (similar to EIP-6493) -- the current proposed operation types are all related to each other would fit well into a StableContainer.

Furthermore:

More context: https://ethereum-magicians.org/t/eip-6110-supply-validator-deposits-on-chain/12072/19

(for devnet0, obviously keep as is. but for subsequent devnets, would be great to consider alternative designs)

@wjmelements
Copy link

I explicate the economic issue with 7623 on ethereum magicians.

Aleneth added a commit to Aleneth/pm that referenced this issue May 6, 2024
@lightclient
Copy link
Member

Is EIP-7685 needed? I think it would be simpler design to keep CL ExecutionPayload and EL block header close together

While implementing 6110 and 7002 I found it cumbersome to continue adding new header and body elements. The feedback I received from other EL teams was similar, but if that's misrepresented I think it should be state.

Because EIP-7685 does not preserve ordering, extra index field also needs to be pushed along.

EIP-7685 delegates that responsibility to the individual request types. So it does preserve ordering. I'm unsure why index exists in EIP-6110, the rationale is vague. You'll notice no index is needed in EIP-7002 because the ordering is preserved.

Does it make sense to add yet another MPT / RLP data structure, or should we use this opportunity to switch to SSZ?

My general feeling is that we should make a focused push to SSZ and migrate all the data structures over instead of having a hodgepodge of MPT / RLP / SSZ strewn across the EL. With that said, we're still in need of a suitable golang SSZ library to even seriously consider this.

@etan-status
Copy link

Thanks for the background info and further explanations on EIP-7685.

My remaining concerns are:

  1. EIP-6110 style deposits and old style deposits are processed in parallel, possibly reordering deposits. RocketPool etc depend on deposits being processed in order, because the very first deposit locks the withdrawal credential, and subsequent top ups with different withdrawal credentials may get stolen if attacker can insert a timely deposit with different withdrawal address before that. They currently have a scrub check that significantly delays the top up; if the deposits can still get reordered that would continue to be necessary, making the design clunky and benefits questionable. The index is there because deposits may get processed out of order.

  2. EIP-7685 has the issue that it only extends the EL block header. The CL ExecutionPayloadHeader / BeaconState track separate lists for the various items. With the deposit/withdrawal/consolidation proposals, this is now at a point where it will no longer possible to compute the EL block hash from just the CL ExecutionPayloadHeader, breaking optimistic sync while the EL is under maintenance. During maintenance, some CL want to simulate a syncing EL internally, and for that it must have the capability to compute the block hash without access to the EL state trie.

  3. EIP-7549 was originally advertised as a trivial change, but introduces a ton of busywork to multiple teams. At the very least, the amount of work required was poorly estimated. This changes not just the CL, but also the VC, Remote signer, and also affects AttesterSlashings and so on, plus has a security research on top to handle the fork transition where old attestations may still be valid but can no longer be included in blocks. We should collect all the implications of the EIP and choose whether teams are still on board with prioritizing it for Electra. The only benefit seems to be some future zk use cases (but I suspect that more changes will be needed to make Ethereum zk friendly).

  4. Generalized indices in BeaconState and ExecutionPayload are broken without a mitigation strategy for the future, breaking anything depending on EIP-4788. Having a reindex in Pectra already is a huge opportunity to push for the SSZ transition, but interest so far seems rather low (despite no real negative opinions voiced). IMO, the effort currently being spent on EIP-7549 could be re-allocated to make a suitable golang SSZ library. Are there any other blockers beside that library? I am trying to get a nimbus-cl/nimbus-el devnet up and running that contains the SSZ changes. @wemeetagain was also looking into getting lodestar-cl compatible with EIP-6493 / EIP-7688. We should also keep in mind that the F-fork will focus on Verkle, and it is extremely unlikely that we want to combine that transition with the SSZ transition. If we don't do SSZ with E, it means to do it in G, until which every fork tends to add a couple more MPTs to the block header, adding more to the problem.

@wjmelements
Copy link

I explicate the economic issue with 7623 on ethereum magicians.

Here are the slides I will use in the meeting.

@charles-cooper
Copy link

ethereum/EIPs#8543 to consider for pectra

@timbeiko
Copy link
Collaborator Author

Closed in favor of #1043

Aleneth added a commit to Aleneth/pm that referenced this issue May 21, 2024
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