Resolve TODOs and Improve EIP-7805 (FOCIL) Spec #4351

jihoonsong · 2025-06-01T10:35:25Z

This PR resolves TODOs and improves the honest validator section of EIP-7805 spec. It contains minimal test changes and rest of tests will be added by the subsequent PR.

specs/_features/eip7805/beacon-chain.md

specs/_features/eip7805/fork-choice.md

specs/_features/eip7805/beacon-chain.md

specs/_features/eip7805/validator.md

GrapeBaBa · 2025-06-27T09:37:27Z

There is a constant that is inconsistent in the SEPC and EIP documents.
In the EIP it used MAX_TRANSACTIONS_PER_INCLUSION_LIST, how in the spec it used MAX_TRANSACTIONS_PER_PAYLOAD.

specs/_features/eip7805/beacon-chain.md

jihoonsong · 2025-07-08T08:30:46Z

As of now, the latest discussion on this PR can be found here: https://hackmd.io/@jihoonsong/S1c2dhMrle

Please review it and provide some feedback. Thank you.

jihoonsong · 2025-07-11T13:01:17Z

This PR has been followed by lots of discussions and changes. To preserve history, those changes are contained in the second commit. For easier review, here is the change log:

Add InclusionListStore to specify IL equivocation handling logic and IL transactions retrieval while abstracting it from the perspective of the Store.
- This comes with get_inclusion_list_store, on_inclusion_list, store_inclusion_list and get_inclusion_list_transactions.
- Compared to the first version of EIP-7805 spec, on_inclusion_list is modified to not repeat the p2p networking validation rule, consistent with other fork choice handlers.
Add validate_inclusion_lists to the fork choice module for IL validation in on_block. validate_inclusion_lists validates the IL constraints using the ExecutionEngine interface.
Add notify_inclusion_lists to ExecutionEngine, which is used for IL validation.
- The name notify_inclusion_lists aligns with existing similar functions, which are notify_new_payload and notify_forkchoice_updated.
Revert changes to state_transition and use validate_inclusion_lists, an abstract ExecutionEngine call, for IL validation.
- The HackMD article provides detailed rationale.
Add get_inclusion_list to ExecutionEngine.
- An inclusion list committee member uses this ExecutionEngine interface to perform their inclusion list committee duty.
Modify notify_forkchoice_updated to add inclusion_list_transactions to PayloadAttributes.
- The proposer uses this ExecutionEngine interface to produce an execution payload that satisfies the IL constraints.
Specify that the block for slot N satisfies the ILs for slot N-1.
- The block for slot N is built during slot N-1 (without timing games) and satisfies the ILs for the same slot.
Reformat Honest Validator to match the existing spec writing convention.
- It now uses the ExecutionEngine interface instead of the actual Engine APIs.
- Some sections were reformatted to match those of previous forks.
Modify get_inclusion_list_committee's return type to support hash_tree_root calculation.

jtraglia

@jihoonsong this looks great, nice work! Just a couple nits + notes for later.

jtraglia · 2025-07-15T22:18:25Z

specs/_features/eip7805/fork-choice.md

+    key = (inclusion_list.slot, inclusion_list.inclusion_list_committee_root)
+
+    inclusion_lists = store.inclusion_lists.setdefault(key, set())
+    equivocators = store.equivocators.setdefault(key, set())


The setdefault stuff here is ugly 😅 I'll look into fixing that in a follow up PR.

I believe we should just remove setdefault stuff. This is because the store must already be initialized outside of this function call, thus setdefault becomes redundant

It's not about store. It's just about Python's Dictionary.

I also don't like this part and that's why I asked @jtraglia if we can use DefaultDict. Let me think about how to improve this.

jtraglia · 2025-07-15T22:29:27Z

specs/_features/eip7805/fork-choice.md

+    inclusion_list_transactions = {
+        transaction
+        for inclusion_list in inclusion_lists
+        if inclusion_list.validator_index not in equivocators
+        for transaction in inclusion_list.transactions
+    }
+
+    return list(inclusion_list_transactions)


This is implicitly deduping the list of transactions, right? I think we should make this a little more explicit, because I almost missed this myself. Maybe something like this:

Suggested change

inclusion_list_transactions = {

transaction

for inclusion_list in inclusion_lists

if inclusion_list.validator_index not in equivocators

for transaction in inclusion_list.transactions

}

return list(inclusion_list_transactions)

inclusion_list_transactions = [

transaction

for inclusion_list in inclusion_lists

if inclusion_list.validator_index not in equivocators

for transaction in inclusion_list.transactions

]

# Deduplicate, order does not need to be preserved

return list(set(inclusion_list_transactions))

Well, the deduplication comes from the fact that inclusion_lists field of InclusionListStore has the value of set. Would adding a comment regarding this inside get_inclusion_list_transactions enhance readability?

jtraglia · 2025-07-15T22:45:55Z

specs/_features/eip7805/validator.md


-## New inclusion list committee duty
+*Note*: The only change to `prepare_execution_payload` is to call
+`get_inclusion_list_store()` and `get_inclusion_list_transactions()` to set the


We don't usually include the () to indicate a function in the specs. Personally, I do the same though 😢

Suggested change

`get_inclusion_list_store()` and `get_inclusion_list_transactions()` to set the

`get_inclusion_list_store` and `get_inclusion_list_transactions` to set the

jtraglia · 2025-07-15T22:52:01Z

specs/_features/eip7805/validator.md

+create and broadcast the `signed_inclusion_list` to the global `inclusion_list`
+subnet by `INCLUSION_LIST_SUBMISSION_DEADLINE` seconds into the slot after
+processing the block for the current slot and confirming it as the head. If no
+block is received by `INCLUSION_LIST_SUBMISSION_DEADLINE - 1` seconds into the


I think there might need to be a config variable for this too. Let's deal with this later though.

I think I added it at L54. Do you mean preset?

mkalinin

Left a few comments, a general question: what if we introduce inclusion_list.md and move InclusionListStore and on_inclusion_list to that new document? This will make FC spec cleaner. cc @jtraglia

mkalinin · 2025-07-16T05:26:50Z

specs/_features/eip7805/fork-choice.md

+#### New `notify_inclusion_lists`
+
+```python
+def notify_inclusion_lists(


A slight preference on the naming: is_inclusion_list_satisfied or something like that. Similarly to is_block_hash_valid and is_valid_versioned_hashes

mkalinin · 2025-07-16T05:28:26Z

specs/_features/eip7805/fork-choice.md

+    key = (inclusion_list.slot, inclusion_list.inclusion_list_committee_root)
+
+    inclusion_lists = store.inclusion_lists.setdefault(key, set())
+    equivocators = store.equivocators.setdefault(key, set())


I believe we should just remove setdefault stuff. This is because the store must already be initialized outside of this function call, thus setdefault becomes redundant

mkalinin · 2025-07-16T05:31:42Z

specs/_features/eip7805/fork-choice.md

+            continue
+
+        if stored_inclusion_list != inclusion_list:
+            equivocators.add(inclusion_list.validator_index)


We should stop processing equivocations at some point in time. Potentially, at the beginning of the next slot. I believe this check can be introduced in a separate PR

Yes I still remember this discussion on Discord. As it would change both the EIP and the CL spec, I think we need more eyes on this. I guess we can revisit this after we're in right momentum, i.e., SFIing EIP-7805. For now, I think we could focus on improvements and tests. But I'll keep this in mind.

mkalinin · 2025-07-16T05:32:26Z

specs/_features/eip7805/fork-choice.md

+### New `store_inclusion_list`
+
+```python
+def store_inclusion_list(


This function processes the list, but not necessarily stores it. Slight preference in favour of process_inclusion_list name

mkalinin · 2025-07-16T06:24:40Z

specs/_features/eip7805/validator.md


 ```python
 def get_sync_committee_message(
    state: BeaconState,
    block_root: Root,
    validator_index: ValidatorIndex,
    privkey: int,
-    store: Store,
+    store: Store,  # [New in EIP7805]


A better way to spec this out would be stating which block_root should passed to the get_sync_committee_message call without modification of the function

Sounds great!

mkalinin · 2025-07-16T06:27:47Z

specs/_features/eip7805/validator.md

-Some validators are selected to submit signed inclusion list. Validators should
-call `get_inclusion_committee_assignment` at the beginning of an epoch to be
-prepared to submit their inclusion list during the next epoch.
+*Note*: The proposer should call `prepare_execution_payload` at


It doesn’t sound right, a proposer may call prepare_execution_payload at 11.5s into a slot or even later in the case of re-org. I think the spec should say that proposer must not start the build process before the PROPOSER_INCLUSION_LIST_CUT_OFF and that a proposer must use a snapshot of ILs made at PROPOSER_INCLUSION_LIST_CUT_OFF in the prepare_execution_payload call

I think it's up to proposers. They still have the freedom to start payload building process before PROPOSER_INCLUSION_LIST_CUT_OFF and we don't want to take it away. But you're right, this is misleading. A proposer can choose either start payload building earlier and update it with ILs after the deadline, or start payload building after the deadline. Let me change it.

Is this safe for a proposer to start building earlier? It can lead to the situation where proposer’s ILs are more relaxed because they started to build earlier and haven’t received some ILs yet. I think proposer "should not” start the build process before the cut off — this would probably be the right wording

Isn't it safe as long as a proposer update the building process after the deadline? What I meant is a proposer can start payload building earlier AND update it with ILs after the deadline.

Oh, I see what you mean but in this case proposer will have to ensure that no new IL arrived after it has started to build earlier. Otherwise, that early built payload can’t be safely used. I agree, this should be up to proposer’s implementation. So, the ultimate requirement is: built payload should satisfy IL constraints observed by a proposer at the cut off

github-project-automation bot added this to Lodestar Consensus Devnet Planning Jun 1, 2025

jihoonsong marked this pull request as draft June 1, 2025 10:35

jihoonsong force-pushed the focil-tests branch 2 times, most recently from 93df232 to 3824af8 Compare June 1, 2025 15:15

mehdi-aouadi reviewed Jun 2, 2025

View reviewed changes

specs/_features/eip7805/beacon-chain.md Outdated Show resolved Hide resolved

specs/_features/eip7805/fork-choice.md Outdated Show resolved Hide resolved

specs/_features/eip7805/fork-choice.md Show resolved Hide resolved

jihoonsong mentioned this pull request Jun 5, 2025

FOCIL Breakout #12 ethereum/pm#1564

Closed

jihoonsong force-pushed the focil-tests branch 4 times, most recently from f6951fa to 281edd1 Compare June 19, 2025 23:01

jihoonsong changed the title ~~Add EIP-7805 (FOCIL) Tests~~ Resolve TODOs and Improve EIP-7805 (FOCIL) Spec Jun 19, 2025

jihoonsong marked this pull request as ready for review June 19, 2025 23:31

jihoonsong force-pushed the focil-tests branch from 281edd1 to d3f32d9 Compare June 19, 2025 23:36

ethereum deleted a comment from ethereum-code-reviewer bot Jun 20, 2025

jihoonsong force-pushed the focil-tests branch from d3f32d9 to 247c175 Compare June 21, 2025 23:39

agnxsh mentioned this pull request Jun 22, 2025

init: Focil (EIP-7805) status-im/nimbus-eth2#7253

Draft

jtraglia reviewed Jun 23, 2025

View reviewed changes

specs/_features/eip7805/beacon-chain.md Outdated Show resolved Hide resolved

jtraglia reviewed Jun 23, 2025

View reviewed changes

specs/_features/eip7805/validator.md Show resolved Hide resolved

jihoonsong force-pushed the focil-tests branch from 247c175 to 319c467 Compare June 23, 2025 14:13

jtraglia mentioned this pull request Jun 23, 2025

Release v1.6.0-alpha.3 #4399

Merged

6 tasks

jihoonsong force-pushed the focil-tests branch from 319c467 to f9929ef Compare June 23, 2025 14:16

mehdi-aouadi mentioned this pull request Jun 25, 2025

FOCIL improvements Consensys/teku#9603

Open

jihoonsong force-pushed the focil-tests branch 2 times, most recently from 3626348 to 2ec3c4a Compare June 26, 2025 17:57

jihoonsong force-pushed the focil-tests branch from 2ec3c4a to 9321ee7 Compare June 27, 2025 16:58

mkalinin reviewed Jul 1, 2025

View reviewed changes

specs/_features/eip7805/beacon-chain.md Outdated Show resolved Hide resolved

mkalinin reviewed Jul 1, 2025

View reviewed changes

specs/_features/eip7805/beacon-chain.md Outdated Show resolved Hide resolved

jihoonsong mentioned this pull request Jul 7, 2025

FOCIL Breakout #14 ethereum/pm#1600

Closed

jihoonsong force-pushed the focil-tests branch from 9321ee7 to 39f10fd Compare July 7, 2025 06:33

jtraglia changed the base branch from dev to master July 9, 2025 19:50

jtraglia added the eip7805 FOCIL label Jul 9, 2025

jihoonsong added 2 commits July 10, 2025 15:39

Resolve TODOs and improve EIP-7805 spec

92bfe48

Reflect feedback and further improve EIP-7805 spec

5956950

jihoonsong force-pushed the focil-tests branch from 39f10fd to 5956950 Compare July 11, 2025 13:00

jtraglia approved these changes Jul 15, 2025

View reviewed changes

mkalinin reviewed Jul 16, 2025

View reviewed changes

	`get_inclusion_list_store()` and `get_inclusion_list_transactions()` to set the
	`get_inclusion_list_store` and `get_inclusion_list_transactions` to set the

Resolve TODOs and Improve EIP-7805 (FOCIL) Spec #4351

Are you sure you want to change the base?

Resolve TODOs and Improve EIP-7805 (FOCIL) Spec #4351

Uh oh!

Conversation

jihoonsong commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GrapeBaBa commented Jun 27, 2025

Uh oh!

Uh oh!

Uh oh!

jihoonsong commented Jul 8, 2025

Uh oh!

jihoonsong commented Jul 11, 2025

Uh oh!

jtraglia left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkalinin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jihoonsong commented Jun 1, 2025 •

edited

Loading