Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion specs/_features/focil/beacon-chain.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ This is the beacon chain specification to add a fork-choice enforced, committee-
class InclusionList(Container):
slot: Slot
validator_index: ValidatorIndex
parent_root: Root
inclusion_list_committee_root: Root
transactions: List[Transaction, MAX_TRANSACTIONS_PER_INCLUSION_LIST]
```

Expand Down
43 changes: 29 additions & 14 deletions specs/_features/focil/fork-choice.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@

This is the modification of the fork choice accompanying the FOCIL upgrade.

## Configuration

| Name | Value | Unit | Duration |
| - | - | :-: | :-: |
| `VIEW_FREEZE_DEADLINE` | `uint64(9)` | seconds | 9 seconds | # [New in FOCIL]

## Helpers

### New `validate_inclusion_lists`
Expand Down Expand Up @@ -50,44 +56,53 @@ class Store(object):
latest_messages: Dict[ValidatorIndex, LatestMessage] = field(default_factory=dict)
unrealized_justifications: Dict[Root, Checkpoint] = field(default_factory=dict)
inclusion_lists: Dict[Tuple[Slot, Root], List[InclusionList]] = field(default_factory=dict) # [New in FOCIL]
inclusion_list_equivocators: Dict[Tuple[Slot, Root], Set[ValidatorIndex]] = field(default_factory=dict)# [New in FOCIL]
inclusion_list_equivocators: Dict[Tuple[Slot, Root], Set[ValidatorIndex]] = field(default_factory=dict) # [New in FOCIL]


### New `on_inclusion_list`

`on_inclusion_list` is called to import `signed_inclusion_list` to the fork choice store.
```python
def on_inclusion_list(
store: Store, signed_inclusion_list: SignedInclusionList) -> None:
store: Store,
signed_inclusion_list: SignedInclusionList,
inclusion_list_committee: Vector[ValidatorIndex, IL_COMMITTEE_SIZE]]) -> None:

Choose a reason for hiding this comment

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

Suggested change
inclusion_list_committee: Vector[ValidatorIndex, IL_COMMITTEE_SIZE]]) -> None:
inclusion_list_committee: List[ValidatorIndex, IL_COMMITTEE_SIZE]]) -> None:

Copy link
Owner

Choose a reason for hiding this comment

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

I believe Vector is right here

Choose a reason for hiding this comment

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

What's difference between Vector and List? Is the Vector from Numpy?

Copy link
Owner

Choose a reason for hiding this comment

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

nope, these are ssz types: https://github.com/ethereum/consensus-specs/blob/dev/ssz/simple-serialize.md
we want a fixed list here

"""
``on_inclusion_list`` verify the inclusion list before importing it to fork choice store.
If there exists more than 1 inclusion list in store with the same slot and validator index, remove the original one.
"""
message = signed_inclusion_list.message
# Verify inclusion list slot is bounded to the current slot
assert get_current_slot(store) == message.slot
# Verify inclusion list slot is either from the current or previous slot
assert get_current_slot(store) in [message.slot, message.slot + 1]

Choose a reason for hiding this comment

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

wouldn't it be message.slot - 1 if it's from either the current or the previous slot?

Choose a reason for hiding this comment

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

If message is from previous slot, message.slot + 1 == get_current_slot(store).

Choose a reason for hiding this comment

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

ah right that makes sense :)


assert message.beacon_block_root in store.block_states
# Get the inclusion list committee for this slot
state = copy(store.block_states[message.beacon_block_root])
if state.slot < message.slot:
process_slots(state, message.slot)
inclusion_list_committee = get_inclusion_list_committee(state, message.slot)
time_into_slot = (store.time - store.genesis_time) % SECONDS_PER_SLOT
is_before_attesting_interval = time_into_slot < SECONDS_PER_SLOT // INTERVALS_PER_SLOT

Choose a reason for hiding this comment

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

wouldn't it be easier to define the attestation deadline like this?

ATTESTATION_DEADLINE = uint64(4)  # seconds
is_before_attestation_deadline = time_into_slot < ATTESTATION_DEADLINE

# If the inclusion list is from the previous slot, ignore it if already past the attestation deadline
if get_current_slot(store) == message.slot + 1:
    assert is_before_attestation_deadline

Copy link
Owner

Choose a reason for hiding this comment

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

I dont think we want to hard code uint64(4), we want to use INTERVALS_PER_SLOT so it's not a breaking change to the spec if we were to adjust the interval deadlines later

# If the inclusion list is from the previous slot, ignore it if already past the attestation deadline
if get_current_slot(store) == message.slot + 1:
assert is_before_attesting_interval

# Sanity check that the given `inclusion_list_committee` matches the root in the inclusion list
root = message.inclusion_list_committee_root
assert hash_tree_root(inclusion_list_committee) == root

# Verify inclusion list validator is part of the committee
validator_index = message.validator_index
assert validator_index.validator_index in inclusion_list_committee
assert validator_index in inclusion_list_committee

# Verify inclusion list signature
assert is_valid_inclusion_list_signature(state, signed_inclusion_list)

root = hash_tree_root(inclusion_list_committee)
is_before_freeze_deadline = get_current_slot(store) == message.slot and time_into_slot < VIEW_FREEZE_DEADLINE

# Do not process inclusion lists from known equivocators
if validator_index not in inclusion_list_equivocators[(message.slot, root)]:
Copy link

@soispoke soispoke Oct 23, 2024

Choose a reason for hiding this comment

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

I think here you might need to introduce a time window, so you only record equivocation evidence only if before the attestation deadline (t=4s)?

if validator_index not in inclusion_list_equivocators[(message.slot, root)]:
    if validator_index in [il.validator_index for il in inclusion_lists[(message.slot, root)]]:
        il = next(
            il for il in inclusion_lists[(message.slot, root)] if il.validator_index == validator_index
        )
        if il != message:
            # Record equivocation evidence if within the allowed time window
            if get_current_slot(store) <= message.slot + 1 and (
                get_current_slot(store) != message.slot + 1 or is_before_attestation_deadline
            ):
                inclusion_list_equivocators[(message.slot, root)].add(validator_index)
    else:
        # This inclusion list is not an equivocation. Store it if prior to the view freeze deadline
        if is_before_freeze_deadline:
            inclusion_lists[(message.slot, root)].append(message)

Copy link
Author

Choose a reason for hiding this comment

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

If it's after the attestation deadline, we won't even get here because of line 82

if validator_index in [il.validator_index for il in inclusion_lists[(message.slot, root)]]
if validator_index in [il.validator_index for il in inclusion_lists[(message.slot, root)]]:
il = [il for il in inclusion_lists[(message.slot, root)] if il.validator_index == validator_index][0]
if not il == message:
# We have equivocation evidence for `validator_index`, record it as equivocator

Choose a reason for hiding this comment

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

Do we want to remove il from inclusion_lists[(message.slot, root)] following the description at L72?

Copy link
Owner

Choose a reason for hiding this comment

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

The comment is outdated, as that's the old design. This new design uses inclusion_list_equivocators so no longer required to remove inclusion_lists

inclusion_list_equivocators[(message.slot, root)].add(validator_index)
else:
# This inclusion list is not an equivocation. Store it if prior to the view freeze deadline
elif is_before_freeze_deadline:
inclusion_lists[(message.slot, root)].append(message)
```

Expand Down
13 changes: 8 additions & 5 deletions specs/_features/focil/p2p-interface.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ This document contains the consensus-layer networking specification for FOCIL.

| Name | Value | Unit | Duration |
| - | - | :-: | :-: |
| `inclusion_list_CUT_OFF` | `uint64(9)` | seconds | 9 seconds |
| `attestation_deadline` | `uint64(4)` | seconds | 4 seconds |

Choose a reason for hiding this comment

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

Is this better to be capitalized? Besides, do we want to rename PROPOSER_INCLUSION_LIST_CUT_OFF using DEADLINE for better consistency?


### Configuration

Expand All @@ -39,18 +39,21 @@ The new topics along with the type of the `data` field of a gossipsub message ar

##### Global topics

FOCIL introduces new global topics for inclusion list.
FOCIL introduces a new global topic for inclusion lists.

###### `inclusion_list`

This topic is used to propagate signed inclusion list as `SignedInclusionList`.
The following validations MUST pass before forwarding the `inclusion_list` on the network, assuming the alias `message = signed_inclusion_list.message`:

- _[REJECT]_ The slot `message.slot` is equal to current slot.
- _[REJECT]_ The slot `message.slot` is equal to the previous or current slot.
- _[IGNORE]_ The slot `message.slot` is equal to the current slot, or it is equal to the previous slot and the current time is less than `attestation_deadline` seconds into the slot.
- _[IGNORE]_ The `inclusion_list_committee` for slot `message.slot` on the current branch corresponds to `message.inclusion_list_committee_root`, as determined by `hash_tree_root(inclusion_list_committee) == message.inclusion_list_committee_root`.
- _[REJECT]_ The validator index `message.validator_index` is within the `inclusion_list_committee` corresponding to `message.inclusion_list_committee_root`.
- _[REJECT]_ The transactions `message.transactions` length is within upperbound `MAX_TRANSACTIONS_PER_INCLUSION_LIST`.
- _[IGNORE]_ The `message` is either the first or second valid message received from the validator with index `message.validator_index`.
- _[REJECT]_ The signature of `inclusion_list.signature` is valid with respect to the validator index.
- _[REJECT]_ The validator index `message.validator_index` is within the inclusion list committee given by `get_inclusion_list_committee(state, message.slot)`, where the `state` is based on `message.parent_block_root` and processed up to `message.slot`. If the validator index cannot immediately be verified against the expected committee, the inclusion list MAY be queued for later processing while the committee for the branch of `message.parent_block_root` is calculated -- in such a case do not REJECT, instead IGNORE this message.
- _[REJECT]_ The signature of `inclusion_list.signature` is valid with respect to the validator index.


### The Req/Resp domain

Expand Down