-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add EIP-7805 (FOCIL) specs #4003
base: dev
Are you sure you want to change the base?
Changes from 1 commit
3a16a0e
fa27c8f
3c86963
8a1e940
833be94
a2b68a7
fb4a3c3
c7a056b
f84556c
2787b0d
0b09f96
6056b69
a8aec7c
cbe6129
0efcf13
f151c7c
b181655
0681f8c
2861684
e678deb
e9fdfc6
6ce0077
be471b8
c443db0
c8b6296
7fa44f4
84acfbb
702b9e5
561aed7
c762a33
20b0c50
aca38ea
1c6761a
d63042e
86c86d0
7d8e6fb
19b50a6
f25efbf
5a37c91
c411d45
dcd31d6
3652904
2781459
7a2cb7a
91054e6
23e33cc
5be24fc
984be8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ This document contains the consensus-layer networking specification for EIP-7805 | |
|
||
| Name | Value | Unit | Duration | | ||
| - | - | :-: | :-: | | ||
| `attestation_deadline` | `uint64(4)` | seconds | 4 seconds | | ||
| `ATTESTATION_DEADLINE` | `uint64(4)` | seconds | 4 seconds | | ||
|
||
### Configuration | ||
|
||
|
@@ -50,9 +50,9 @@ The following validations MUST pass before forwarding the `inclusion_list` on th | |
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't we also limiting IL size by |
||
- _[REJECT]_ The size of `message` is within upperbound `MAX_BYTES_PER_INCLUSION_LIST`. | ||
- _[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 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 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -89,8 +89,7 @@ Proposers are still expected to propose `SignedBeaconBlock` at the beginning of | |
|
||
#### Update execution client with inclusion lists | ||
|
||
The proposer should call `engine_updateInclusionListV1` at `PROPOSER_INCLUSION_LIST_CUT_OFF` into the slot with the list of the inclusion lists that gathered since `inclusion_list_CUT_OFF`. | ||
|
||
The proposer should call `engine_updateInclusionListV1` at `PROPOSER_INCLUSION_LIST_CUT_OFF` into the slot with the list of the inclusion lists that gathered since `PROPOSER_INCLUSION_LIST_CUT_OFF`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jtraglia shouldn't this say something like "up to" instead of "since"? I think this is related to https://github.com/ethereum/consensus-specs/pull/4003/files#r1912551318, there is no specific time when we start to build the inclusion list as it depends on when the node receives the block or some other cutoff we might wanna define if the block for current slot is late. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, "up to" is correct. Thanks I'll make that change. And hmm. Would a late block change this? Wouldn't we always build the IL at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, the cutoff for the proposer is always the same. The other cutoff I mean is for IL committee members but this one does not matter for the proposer as it would just collect all ILs it received for the slot until There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah got it. Well I will leave it to someone else (maybe @terencechain) to clarify. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify: The proposer cutoff is always the same (11sec), but for IL committee members:
The rest of validators (not the proposer, and not IL committee members) have the freeze deadline cutoff 1sec later, at |
||
|
||
## New inclusion list committee duty | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should time parameters be defined as a fraction of
SECONDS_PER_SLOT
instead, or do we just define them as part of presets to work with minimal as well?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh you're right, this wouldn't work with minimal... I'll look into the best way to handle this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the solution something as simple as this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would work here. But not for other time values like:
consensus-specs/specs/_features/eip7805/validator.md
Line 45 in 2781459
Maybe that could be
SECONDS_PER_SLOT - 1
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bleesherman. I've done just that. Please see: 7a2cb7a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this more, not sure if having those values defined in preset will get us where we want as during testing we might use mainnet preset and just reduce seconds per slot. This would also make reducing seconds per slot in the future harder as you need to update a bunch of time parameters. But this might be something to worry about later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear, these are configuration values now. As of that commit I shared.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be specific to how we handle this in Lodestar but even if it's part of config we can't derive the value from
SECONDS_PER_SLOT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah dang. Well I suppose clients could use the values without deriving them.
consensus-specs/configs/mainnet.yaml
Lines 187 to 189 in 7a2cb7a
consensus-specs/configs/minimal.yaml
Lines 186 to 188 in 7a2cb7a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I did for now but will break if we change seconds per slot with mainnet config but it's possible to adapt these values as well now since we have them in config