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

EIP-7547: Inclusion Lists #3617

Open
wants to merge 38 commits into
base: dev
Choose a base branch
from

Conversation

michaelneuder
Copy link

@michaelneuder michaelneuder commented Mar 7, 2024

IL POC Spec - see https://notes.ethereum.org/@mikeneuder/il-poc-spec.

Pulling from many sources including: https://github.com/hwwhww/consensus-specs/pull/2/files and bca66b0 into one upstream PR.

Core elements

  1. Unconditional – guaranteed inclusion with a 3 million gas limit on the IL.
  2. Anywhere in block – the transactions in the slot N IL must appear anywhere in either the slot N or slot N+1 payloads.
  3. IL not committed to in beacon block – the slot N beacon block has no reference to the slot N IL summary.
  4. Unbundled gossip – the IL is an independent P2P topic.

Details

For the key files: beacon-chain.md, fork-choice.md, p2p-networking.md, & validator.md, we present extended details to highlight the critical changes.

beacon-chain.md

Overview: Add the inclusion list data to the execution layer block, the state transition, and the block processing.

Itemized:

  • Define a constant domain for IL signature verification.
  • Create a constant for size of IL: MAX_TRANSACTIONS_PER_INCLUSION_LIST
  • Create new containers.
    • InclusionListSummaryEntry
    • InclusionListSummary
    • SignedInclusionListSummary
    • InclusionList
  • Extended containers.
    • ExecutionPayload
    • ExecutionPayloadHeader
    • BeaconState
  • State transition
    • Define NewInclusionListRequest for engine API
    • Define new engine API to be called upon receipt of a new IL: verify_and_notify_new_inclusion_list.
  • Block processing
    • Update process_execution_payload to verify signature over IL.
    • Define verify_inclusion_list_summary_signature as helper.

fork-choice.md

Overview: Add inclusion list verification handling.

Itemized:

  • Add on_inclusion_list handler to be called upon receipt of a new inclusion list. It updates the store if the IL is valid.
  • Add helper function to check for availability of IL: is_inclusion_list_available.

p2p-networking.md

Overview: Create the inclusion list topic for the p2p network.

Itemized:

  • Add SignedInclusionList object which is an envelope for the full IL.
  • Add the inclusion_list topic and associated validation rules.

validator.md

Overview: Define the new validator behavior and engine APIs for fetching and validating inclusion lists from the EL.

Itemized:

  • Define engine_getInclusionListV1 engine API interface, which requests a new IL constructed by the EL and returns a SignedInclusionList.
  • Beacon chain responsibilities
    • Add condition to validate inclusion lists before rebroadcasting them.
    • Add condition to block validation to ensure there exists an available inclusion list.
    • Add condition for honest block production that ensures a signed IL is published.

@hwwhww hwwhww added the general:enhancement New feature or request label Mar 12, 2024
michaelneuder and others added 11 commits March 12, 2024 13:30
- _[REJECT]_ The slot `message.signedSummary.message.slot` is the current slot.
- _[REJECT]_ The `signature` is valid for the current proposer `message.signedSummary.message.proposer_index`.
- _[REJECT]_ The `message.signedSummary.message.parent_hash` is not the current head.
- _[REJECT]_ The inclusion list transactions `message.transactions` length is within upperbound `MAX_TRANSACTIONS_PER_INCLUSION_LIST`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels more like an EL responsibility

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ill remove the parent hash one. i think the other stuff feels like CL side verification, no ?

specs/_features/eip7547/p2p-networking.md Outdated Show resolved Hide resolved

```python
class SignedInclusionList(Container):
message: InclusionList
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering about the usefulness of having a signed container for InclusionList.
The DoS potential here if we propagate an InclusionList instead of a SignedInclusionList is that an attacker can send multiple valid spam objects by changing the transactions field in the InclusionList.
However, given that as a node, I need to have just seen a valid InclusionList object for a slot, I can safely ignore new InclusionList objects that have the same SignedInclusionListSummary and not process or forward them over the network

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no strong opinion on this, will ask potuz

specs/_features/eip7547/validator.md Outdated Show resolved Hide resolved
children = [
root for (root, block) in blocks.items()
if block.parent_root == block_root
and is_inclusion_list_available(root)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work since you may have grand children that are valid

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this work by doing the following:

  • as you add something to forkchoice, include an "IL available bool", store this bool along with the block
  • if the bool is true, iterate back to the justified root flipping all to true
  • is_inclusion_list_available here just checks this bool is true

Effectively tracking the latest seen block with an available IL which I think is what we want

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really work because we may have to reorg back to a head for witch we haven't seen an IL. Without reconstructing it we always rely on external sources

Copy link
Contributor

@potuz potuz Mar 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you make this work by doing the following:

  • as you add something to forkchoice, include an "IL available bool", store this bool along with the block
  • if the bool is true, iterate back to the justified root flipping all to true
  • is_inclusion_list_available here just checks this bool is true

Effectively tracking the latest seen block with an available IL which I think is what we want

Not sure if this was after or before the Discord discussion, but just in case and to leave it documented, this doesn't work either since you may be turning for true a node where no one has seen the IL except the child's builder. In this case this node should never be taken for head or the EL should be able to reconstruct the IL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
general:enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants