FOCIL #2

terencechain · 2024-09-11T20:46:06Z

No description provided.

soispoke

General comment/question:

Just to make sure I understand: I couldn't find the Agg/Eval functions attesters have to run before casting their votes, is this expected?

soispoke · 2024-09-17T11:09:25Z

specs/_features/focil/beacon-chain.md

+- [Beacon chain state transition function](#beacon-chain-state-transition-function)
+  - [Execution engine](#execution-engine)
+    - [Engine APIs](#engine-apis)
+      - [New `verify_and_notify_new_inclusion_list`](#new-verify_and_notify_new_inclusion_list)


should it be verify_and_notify_new_inclusion_list_aggregate?

soispoke · 2024-09-17T11:10:20Z

specs/_features/focil/beacon-chain.md

+
+## Introduction
+
+This is the beacon chain specification to add a committee-based inclusion list mechanism to allow forced transaction inclusion. Refers to [Ethresearch](https://ethresear.ch/t/fork-choice-enforced-inclusion-lists-focil-a-simple-committee-based-inclusion-list-proposal/19870/1)


better to say "to add a fork-choice enforced, committee-based inclusion list (FOCIL)" here

soispoke · 2024-09-17T11:11:27Z

specs/_features/focil/beacon-chain.md

+
+| Name | Value | 
+| - | - | 
+| `IL_COMMITTEE_SIZE` | `uint64(2**9)` (=512)  # (New in FOCIL) |


this is still very much to be defined, but in the post it's 256, not 512

soispoke · 2024-09-17T11:11:59Z

specs/_features/focil/beacon-chain.md

+#### `InclusionSummary`
+
+```python
+class InclusionSummary(Container):


LocalInclusionSummary?

soispoke · 2024-09-17T11:13:22Z

specs/_features/focil/beacon-chain.md

+    sync_aggregate: SyncAggregate
+    bls_to_execution_changes: List[SignedBLSToExecutionChange, MAX_BLS_TO_EXECUTION_CHANGES]
+    # FOCIL
+    inclusion_summary_aggregates: InclusionSummaryAggregates   # [New in FOCIL]


the note mentions inclusion_summary_aggregate but here you added an "s" at the end

I think the note is wrong, inclusion_summary_aggregates is correct here. Aligns with bls_to_execution_changes and others

soispoke · 2024-09-17T11:15:30Z

specs/_features/focil/engine-api.md

+
+Warning: This file should be placed in https://github.com/ethereum/execution-apis but I'm placing it here for convenience of reviewing.
+
+## Table of contents


Should probably be LocalInclusionListSummary here too (throughout the whole section)

soispoke · 2024-09-17T11:15:45Z

specs/_features/focil/engine-api.md

+
+| Name | Value |
+| - | - |
+| `INCLUSION_LIST_MAX_GAS` |  `uint64(4194304) = 2**22` |


LOCAL_INCLUSION_LIST_MAX_GAS

soispoke · 2024-09-17T11:17:44Z

specs/_features/focil/p2p-interface.md

+  - [Topics and messages](#topics-and-messages-1)
+    - [Global topics](#global-topics)
+      - [`local_inclusion_list`](#local_inclusion_list)
+      - [`inclusion_summary_aggregate`](#inclusion_summary_aggregate)


did you mean inclusion_list_summary_aggregate?

I renamed everything from inclusion_list_summary -> inclusion_summary. It feels cleaner, happy to change but I will leave naming stuff to the last. Focusing on functionality first

soispoke · 2024-09-17T11:17:58Z

specs/_features/focil/p2p-interface.md

+class SignedInclusionListAggregates(Container):
+    slot: Slot
+    proposer_index: ValidatorIndex
+    message: InclusionSummaryAggregates


same q here

jihoonsong · 2024-09-17T13:59:43Z

specs/_features/focil/beacon-chain.md

+    validator_indices: List[ValidatorIndex] = [] 
+    for idx in range(committees_per_slot):
+        beacon_committee = get_beacon_committee(state, slot, CommitteeIndex(idx))
+        validator_indices += beacon_committee[:members_per_committee]


What happens if multiple committees assemble their members from the beginning of beacon committee at the same time? Does a validator with multiple committee roles just fulfill its duties? If so, wouldn't this be a burden at some point?

It's just one committee, size 256 or 512, if you get duplicated entries then just do it twice. I don't think it's worth optimizing this edge case at this point

Alright, it was a genuine question. Thanks for the answer!

jihoonsong · 2024-09-17T14:02:59Z

specs/_features/focil/beacon-chain.md

+    """
+    Return the indexed inclusion list aggregate corresponding to ``inclusion_summary_aggregate``.
+    """
+    indices = get_inclusion_list_aggregate_indices(state)


You need to pass inclusion_summary_aggregate.

jihoonsong · 2024-09-17T14:03:56Z

specs/_features/focil/beacon-chain.md

+#### `IndexedInclusionListAggregate`
+
+```python
+class IndexedInclusionListAggregate(Container):


Suggested change

class IndexedInclusionListAggregate(Container):

class IndexedInclusionSummaryAggregate(Container):

jihoonsong · 2024-09-17T14:05:58Z

specs/_features/focil/beacon-chain.md

+#### New `get_indexed_inclusion_list_aggregate`
+
+```python
+def get_indexed_inclusion_list_aggregate(state: BeaconState, 


Suggested change

def get_indexed_inclusion_list_aggregate(state: BeaconState,

def get_indexed_inclusion_summary_aggregate(state: BeaconState,

jihoonsong · 2024-09-17T14:06:04Z

specs/_features/focil/beacon-chain.md

+#### New `get_inclusion_list_aggregate_indices`
+
+```python
+def get_inclusion_list_aggregate_indices(state: BeaconState, 


Suggested change

def get_inclusion_list_aggregate_indices(state: BeaconState,

def get_inclusion_summary_aggregate_indices(state: BeaconState,

jihoonsong · 2024-09-17T14:11:27Z

specs/_features/focil/beacon-chain.md

+
+```python
+class InclusionSummaryAggregates(Container):
+    List[InclusionSummaryAggregate, MAX_TRANSACTIONS_PER_INCLUSION_LIST]


If I understood correctly, the maximum number of InclusionSummaryAggregate is MAX_TRANSACTIONS_PER_INCLUSION_LIST * IL_COMMITTEE_SIZE.

jihoonsong · 2024-09-17T14:14:56Z

specs/_features/focil/beacon-chain.md

+    address: ExecutionAddress
+    nonce: uint64
+    gas_limit: uint64


Do these combination always correspond to one transaction? What if an EOA sends a transaction and then sends another one with the same nonce and gas_limit but with higher priority fee in the hope that it can effectively cancel the first one?

Execution layer will filter that

jihoonsong · 2024-09-17T14:21:52Z

specs/_features/focil/beacon-chain.md

+    validator_index: ValidatorIndex
+    parent_hash: Hash32
+    summaries: List[InclusionSummary, MAX_TRANSACTIONS_PER_INCLUSION_LIST]
+    transactions: List[Transaction, MAX_TRANSACTIONS_PER_INCLUSION_LIST]


Shouldn't LocalInclusionList contain any raw transaction to prevent free DA? I think raw transactions need to be cached somewhere else.

LocalInclusionList can't be proven on chain, there's no free DA. Yea it will be cached short term, which wont be specified in the spec

You're right, I was mistaken. Only the summary is part of a block.

jihoonsong · 2024-09-17T14:27:47Z

specs/_features/focil/beacon-chain.md

+    inclusion_summary_aggregates: InclusionSummaryAggregates   # [New in FOCIL]
+```
+
+### Beacon State accessors


I think we also need a function that takes a list of SignedLocalInclusionList and outputs InclusionSummaryAggregates.

I don't think that's part of the state transition function though. I briefly mentioned this in the validator spec. I think this is an implementation detail. Nodes will cache signed local inclusion lists, aggregate them, and release them

I don't think it belongs to beacon state transition function either. I think it belongs to beacon state accessors or at least a helper. Currently, all beacon state accessors are using InclusionSummaryAggregate so I thought they need some function to derive it from the received a list of SignedLocalInclusionList. It's for attesters rather than IL committee members.

Let's add that to the fork choice spec or validator spec. Fork choice spec will use it to lock views

terencechain · 2024-09-17T15:54:43Z

Just to make sure I understand: I couldn't find the Agg/Eval functions attesters have to run before casting their votes, is this expected?

That's part of fork choice spec, which I've been procrastinating on : )

jihoonsong · 2024-09-17T16:11:40Z

specs/_features/focil/validator.md

+- The proposer signs the `inclusion_summary_aggregates` using helper `get_inclusion_summary_aggregates` and constructs a `signed_inclusion_aggregates_summary`.
+
+```python
+def get_inclusion_summary_aggregate(


Suggested change

def get_inclusion_summary_aggregate(

def get_inclusion_summary_aggregates_signature(

jihoonsong · 2024-09-17T16:17:40Z

specs/_features/focil/validator.md

+#### Constructing the new `InclusionSummaryAggregate` field in  `BeaconBlockBody`
+
+Proposer has to include a valid `inclusion_summary_aggregate` into the block body. The proposer will have to
+* Proposer uses perviously constructed `InclusionSummaryAggregate` and include it in the beacon block body.


InclusionSummaryAggregate -> InclusionSummaryAggregates

jihoonsong · 2024-09-17T16:28:46Z

specs/_features/focil/p2p-interface.md

+#### `SignedInclusionListAggregate`
+
+```python
+class SignedInclusionListAggregates(Container):


Suggested change

class SignedInclusionListAggregates(Container):

class SignedInclusionSummaryAggregates(Container):

jihoonsong · 2024-09-17T16:31:43Z

specs/_features/focil/p2p-interface.md

+- _[REJECT]_ The signature of `inclusion_list.signature` is valid with respect to the validator index. 
+- _[REJECT]_ The validator index is within the inclusion list committee in `get_inclusion_list_committee(state)`. The `state` is the head state corresponding to processing the block up to the current slot as determined by the fork choice. 
+
+###### `inclusion_summary_aggregate`


Suggested change

###### `inclusion_summary_aggregate`

###### `inclusion_summary_aggregates`

jihoonsong · 2024-09-18T09:35:27Z

specs/_features/focil/beacon-chain.md

+    sync_aggregate: SyncAggregate
+    bls_to_execution_changes: List[SignedBLSToExecutionChange, MAX_BLS_TO_EXECUTION_CHANGES]
+    # FOCIL
+    inclusion_summary_aggregates: InclusionSummaryAggregates   # [New in FOCIL]


I just realize this could add quite amount of bytes to beacon block. I'm not aware of what is the acceptable range, but here is my back-of-the-napkin math.

InclusionSummary: address + uint64 + uint64 = (20 + 8 + 8) bytes = 36 bytes.

InclusionSummaryAggregate: Bitvector[IL_COMMITTEE_SIZE] + InclusionSummary + BLSSignature = (256 / 8 + 36 + 96) bytes = 164 bytes.

InclusionSummaryAggregates: InclusionSummaryAggregate * MAX_TRANSACTIONS_PER_INCLUSION_LIST * IL_COMMITTEE_SIZE = (164 * MAX_TRANSACTIONS_PER_INCLUSION_LIST * 256) bytes >= 41,984 bytes.

Of course it depends on constants values, which are not fixed yet.

The ideal size of the new field would be (128 + 36 * MAX_TRANSACTIONS_PER_INCLUSION_LIST) bytes, which comes from:

aggregation_bits: BitVector[IL_COMMITTEE_SIZE] summaries: List[InclusionSummary, MAX_TRANSACTIONS_PER_INCLUSION_LIST] signature: BLSSignature

But this requires at least one more round from a proposer to IL committee members. I don't have a good solution at the moment but wanted to share what I've found.

jihoonsong · 2024-10-02T07:15:28Z

specs/_features/focil/beacon-chain.md

+
+## Introduction
+
+This is the beacon chain specification to add a fork-choice enforced, committee-based inclusion list (FOCIL) mechanism to allow forced transaction inclusion. Refers to [Ethresearch](https://ethresear.ch/t/fork-choice-enforced-inclusion-lists-focil-a-simple-committee-based-inclusion-list-proposal/19870/1)


Might be worth to add this one also? Could help the draft reader to follow up the concepts.. but it doesn't really matter as we can add a link to EIP once we have it.

jihoonsong · 2024-10-02T07:28:53Z

specs/_features/focil/beacon-chain.md

+    execution_payload = new_payload_request.execution_payload
+    execution_requests = new_payload_request.execution_requests
+    parent_beacon_block_root = new_payload_request.parent_beacon_block_root
+    il_transactions = new_payload_request.il_transactions


Suggested change

il_transactions = new_payload_request.il_transactions

il_transactions = new_payload_request.il_transactions # [New in FOCIL]

jihoonsong · 2024-10-02T07:34:49Z

specs/_features/focil/beacon-chain.md

+when calling `notify_new_payload` in FOCIL.
+
+```python
+def verify_and_notify_new_payload(self: ExecutionEngine,


il_transactions is in NewPayloadRequest anyway so there is no need to change the signature of verify_and_notify_new_payload of Electra.

jihoonsong · 2024-10-02T07:45:36Z

specs/_features/focil/fork-choice.md

+```python
+def validate_inclusion_lists(store: Store, inclusion_list_transactions: List[Transaction, MAX_TRANSACTIONS_PER_INCLUSION_LIST], execution_payload: ExecutionPayload) -> bool:
+    """
+    Return ``True`` if and only if the input ``inclusion_list_transactions`` satifies validation, that to verify if the `execution_payload` satisfies `inclusion_list_transactions` validity conditions either when all transactions are present in payload or when any missing transactions are found to be invalid when appended to the end of the payload.


Would it be better to explicitly describe a condition when a block is full?

jihoonsong · 2024-10-02T07:56:03Z

specs/_features/focil/fork-choice.md

+    # Verify inclusion list signature
+    assert is_valid_local_inclusion_list_signature(state, signed_inclusion_list) 
+
+    parent_hash = message.parent_hash


Where is parent_hash used?

jihoonsong · 2024-10-02T08:21:17Z

specs/_features/focil/validator.md

+This document is an extension of the [Electra -- Honest Validator](../../electra/validator.md) guide.
+All behaviors and definitions defined in this document, and documents it extends, carry over unless explicitly noted or overridden.
+
+All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [FOCIL](./beacon-chain.md) are requisite for this document and used throughout.


Suggested change

All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [FOCIL](./beacon-chain.md) are requisite for this document and used throughout.

All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [FOCIL](./beacon-chain.md) are prerequisite for this document and used throughout.

Sticking with requisite because that's what's used in all the documents, not really sure which is better

jihoonsong · 2024-10-02T08:33:18Z

specs/_features/focil/engine-api.md

+#### Specification
+
+1. Client software **MUST** respond to this method call in the following way:
+  * `{status: INVALID_INCLUSION_LISTING, latestValidHash: null, validationError: null}` if there are any leftover `inclusionListTransactions` that are not part of the `executionPayload`, they can be processed at the end of the `executionPayload`.


Suggested change

* `{status: INVALID_INCLUSION_LISTING, latestValidHash: null, validationError: null}` if there are any leftover `inclusionListTransactions` that are not part of the `executionPayload`, they can be processed at the end of the `executionPayload`.

* `{status: INVALID_INCLUSION_LIST, latestValidHash: null, validationError: null}` if there are any leftover `inclusionListTransactions` that are not part of the `executionPayload`, they can be processed at the end of the `executionPayload`.

Do we need to mention the condition that execution payload has leftover gas?

jihoonsong · 2024-10-02T08:45:08Z

specs/_features/focil/validator.md

+
+### `ExecutionEngine`
+
+*Note*: `engine_getInclusionListV1` and `engine_updateInclusionListV1` functions are added to the `ExecutionEngine` protocol for use as a validator.


engine_updateInclusionListV1 seems missing in the engine-api.md.

jihoonsong · 2024-10-02T08:49:53Z

specs/_features/focil/validator.md

+
+Some validators are selected to submit local inclusion list. Validators should call `get_ilc_assignment` at the beginning of an epoch to be prepared to submit their local inclusion list during the next epoch. 
+
+A validator should create and broadcast the `signed_inclusion_list` to the global `inclusion_list` subnet by the `LOCAL_INCLUSION_LIST_CUT_OFF` in the slot, unless a block for the current slot has been processed and is the head of the chain and broadcast to the network.


Suggested change

A validator should create and broadcast the `signed_inclusion_list` to the global `inclusion_list` subnet by the `LOCAL_INCLUSION_LIST_CUT_OFF` in the slot, unless a block for the current slot has been processed and is the head of the chain and broadcast to the network.

A validator should create and broadcast the `signed_inclusion_list` to the global `local_inclusion_list` subnet by the `LOCAL_INCLUSION_LIST_CUT_OFF` in the slot, unless a block for the current slot has been processed and is the head of the chain and broadcast to the network.

jihoonsong · 2024-10-02T08:52:23Z

specs/_features/focil/validator.md

+- Set `local_inclusion_list.parent_hash` to the block hash of the fork choice head.
+- Set `local_inclusion_list.parent_root` to the block hash of the fork choice head.


What's the difference between these two?

one is consensus layer construct, one is execution layer construct, fixing the sentences to clarify

jihoonsong · 2024-10-13T04:34:40Z

specs/_features/focil/fork-choice.md

+    """
+    message = signed_inclusion_list.message
+    # Verify inclusion list slot is bounded to the current slot
+    assert get_current_slot(store) != message.slot


Suggested change

assert get_current_slot(store) != message.slot

assert get_current_slot(store) == message.slot

jihoonsong · 2024-10-14T02:39:02Z

specs/_features/focil/fork-choice.md

+        store.inclusion_lists.remove(existing_inclusion_list)
+    else:
+        # If no such inclusion list exists, add the new one
+        store.inclusion_lists.append(message.transaction)


Suggested change

store.inclusion_lists.append(message.transaction)

store.inclusion_lists.append(message)

terencechain force-pushed the focil branch 4 times, most recently from ef9c08c to 1764816 Compare September 16, 2024 22:46

soispoke reviewed Sep 17, 2024

View reviewed changes

jihoonsong reviewed Sep 17, 2024

View reviewed changes

jihoonsong reviewed Sep 18, 2024

View reviewed changes

terencechain force-pushed the focil branch from 3efd69a to 9d0d456 Compare September 30, 2024 20:11

terencechain added 9 commits September 30, 2024 10:49

Add focil containers

3a16a0e

Add process inclusion list aggregate

fa27c8f

Add inclusion list gossip conditions

3c86963

Add execution api

8a1e940

Add validator spec

833be94

Review feedback

a2b68a7

Update forkchoice

fb4a3c3

Update based on latest discussion. No more IL aggregate

c7a056b

Update fork-choice.md

f84556c

terencechain force-pushed the focil branch from 9d0d456 to 446b803 Compare September 30, 2024 20:49

Update per https://ethresear.ch/t/focil-cl-el-workflow/20526

2787b0d

terencechain force-pushed the focil branch from 446b803 to 2787b0d Compare October 1, 2024 20:16

jihoonsong reviewed Oct 2, 2024

View reviewed changes

Address feedback

0b09f96

terencechain force-pushed the focil branch from ae2cf0e to 542cf3e Compare October 12, 2024 22:37

jihoonsong reviewed Oct 13, 2024

View reviewed changes

jihoonsong reviewed Oct 14, 2024

View reviewed changes

Clarify forkchoice and p2p

6056b69

terencechain force-pushed the focil branch from 542cf3e to 6056b69 Compare October 16, 2024 22:07

Remove local

a8aec7c

terencechain force-pushed the focil branch from aff7c0a to a8aec7c Compare October 17, 2024 14:29


		## Introduction

		This is the beacon chain specification to add a committee-based inclusion list mechanism to allow forced transaction inclusion. Refers to [Ethresearch](https://ethresear.ch/t/fork-choice-enforced-inclusion-lists-focil-a-simple-committee-based-inclusion-list-proposal/19870/1)


		Warning: This file should be placed in https://github.com/ethereum/execution-apis but I'm placing it here for convenience of reviewing.

		## Table of contents

	class IndexedInclusionListAggregate(Container):
	class IndexedInclusionSummaryAggregate(Container):

	def get_indexed_inclusion_list_aggregate(state: BeaconState,
	def get_indexed_inclusion_summary_aggregate(state: BeaconState,

	def get_inclusion_list_aggregate_indices(state: BeaconState,
	def get_inclusion_summary_aggregate_indices(state: BeaconState,

	def get_inclusion_summary_aggregate(
	def get_inclusion_summary_aggregates_signature(

	class SignedInclusionListAggregates(Container):
	class SignedInclusionSummaryAggregates(Container):

	###### `inclusion_summary_aggregate`
	###### `inclusion_summary_aggregates`

	il_transactions = new_payload_request.il_transactions
	il_transactions = new_payload_request.il_transactions # [New in FOCIL]

	All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [FOCIL](./beacon-chain.md) are requisite for this document and used throughout.
	All terminology, constants, functions, and protocol mechanics defined in the updated Beacon Chain doc of [FOCIL](./beacon-chain.md) are prerequisite for this document and used throughout.

	* `{status: INVALID_INCLUSION_LISTING, latestValidHash: null, validationError: null}` if there are any leftover `inclusionListTransactions` that are not part of the `executionPayload`, they can be processed at the end of the `executionPayload`.
	* `{status: INVALID_INCLUSION_LIST, latestValidHash: null, validationError: null}` if there are any leftover `inclusionListTransactions` that are not part of the `executionPayload`, they can be processed at the end of the `executionPayload`.


		### `ExecutionEngine`

		Note: `engine_getInclusionListV1` and `engine_updateInclusionListV1` functions are added to the `ExecutionEngine` protocol for use as a validator.


		Some validators are selected to submit local inclusion list. Validators should call `get_ilc_assignment` at the beginning of an epoch to be prepared to submit their local inclusion list during the next epoch.

		A validator should create and broadcast the `signed_inclusion_list` to the global `inclusion_list` subnet by the `LOCAL_INCLUSION_LIST_CUT_OFF` in the slot, unless a block for the current slot has been processed and is the head of the chain and broadcast to the network.

		- Set `local_inclusion_list.parent_hash` to the block hash of the fork choice head.
		- Set `local_inclusion_list.parent_root` to the block hash of the fork choice head.

	assert get_current_slot(store) != message.slot
	assert get_current_slot(store) == message.slot

	store.inclusion_lists.append(message.transaction)
	store.inclusion_lists.append(message)

FOCIL #2

Are you sure you want to change the base?

FOCIL #2

Conversation

terencechain commented Sep 11, 2024

soispoke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terencechain Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jihoonsong Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terencechain commented Sep 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terencechain Sep 17, 2024 •

edited

Loading

jihoonsong Sep 17, 2024 •

edited

Loading