Replace INTERVALS_PER_SLOT time setting with ATTESTATION_DUE_MS style presets

[hwwhww]

Issue

One of the pushbacks of #3433 implementation is that it introduces a new timing setting scheme while the CL specs and clients have used INTERVALS_PER_SLOT in several places.

Proposal

This PR is a refactoring to use #3433 style parameters in phase0, and getting rid of INTERVALS_PER_SLOT from the specs. The goal is to provide a more flexible time-setting scheme and eliminate the legacy technical debt.

Even if we don't add #3433 to Deneb, this refactoring can give us a foundation to change the duty times setting in the future forks. e.g., we can set new ATTESTATION_DUE_MS_ELECTRA in presets easily.

/cc @arnetheduck

TBD

• Moving all new parameters to "Configurations" v.s. moving them + SECONDS_PER_SLOT to "Presets"
• Parameter naming discussion

[hwwhww]

@tersec @tbenr

Is it a nice-to-have change to you?

[tbenr]

Seems reasonable to me to introduce the params in presets, which allows to set the stage without touching configs. Clients are free to start implementing or wait until electra.

@rolfyone WDYT?

[arnetheduck]

Oh, nice! great idea!

[tbenr]

The only inconvenience is that those params must show up in the spec beacon API to be consumed by VCs, so it wont be super transparent.

(And technically they become mandatory from VC perspective, leading to potential interoperability issues)

[ppopth]

I quite agree with this PR. It makes sense to also remove INTERVALS_PER_SLOT in all the previous hard forks and it also makes sense to put the *_DUE_MS in presets rather than configurations.

But I have a concern. I saw that SECONDS_PER_SLOT is a configuration rather than a preset. Those all the *_DUE_MS numbers depend on SECONDS_PER_SLOT but SECONDS_PER_SLOT is a configuration that depends on a specific network.

I guess this fact makes the *_DUE_MS numbers should be in the configurations instead?

However, personally I think all of them, including SECONDS_PER_SLOT, should be in the presets.

[mcdee]

Not a massive fan of the new naming convention. Taking ATTESTATION_DUE_MS as an example: 4s into the slot isn't really when attestations are due, they are:

• generated at the earliest of (4s, block arrival time)
• propagated through the network
• and ideally arrive before 8s into the slot (although some clients use them earlier)

As such, I'm not sure that the name reflects the reality. Although not perfect, perhaps using deadlines instead would be better e.g. BLOCK_RECEIPT_DEADLINE_MS set to 4000 may be clearer.

Separately, if this goes through then we need to consider downstream systems and their adoption of these changes. Specifically, I'm thinking of https://ethereum.github.io/beacon-APIs/#/Config/getSpec and the how this would impact the output. Although this isn't part of the spec itself, I would like to see all client teams to commit to providing both old-style and new-style configuration values during the Deneb period (i.e. from the Deneb hard fork up to the Electra hard fork) in this API to allow dependent systems time to cut over.

[rolfyone]

Is the intent that all clients apply clock disparity to the values?

I think it's good. We had problems with the last constants moved because some clients didn't offer them and some clients had disruption, so coordination is probably worth having in rolling this out to reduce the possibility of more disruptions...

Probably mitigated by loading our preset values we ship with as 'defaults' I guess...

[ppopth]

Not a massive fan of the new naming convention. Taking ATTESTATION_DUE_MS as an example: 4s into the slot isn't really when attestations are due

ATTESTATION_DUE_MS doesn't mean the due time that the attestation must arrive, but it means the time that the attestation must be generated, so 4s quite makes sense.

[mcdee]

ATTESTATION_DUE_MS doesn't mean the due time that the attestation must arrive, but it means the time that the attestation must be generated, so 4s quite makes sense.

a) this is not correct if the block for the slot is imported earlier

b) your sentence shows that the _DUE_ part of the constant has implicit information around what is due (although even then I disagree with "must be generated" and instead would say "should start the generation process")

So I stand by my general point that using DUE is likely to be confusing to those coming fresh to the specs.

[casparschwa]

Not a massive fan of the new naming convention. Taking ATTESTATION_DUE_MS as an example: 4s into the slot isn't really when attestations are due, they are:

• generated at the earliest of (4s, block arrival time)

As per the honest validator specs an attestation is to be released as soon as a valid block is heard, or at 4s, whichever occurs first. The intention of the honest specs, as currently written, is not to target a receival time, but a release time.

• and ideally arrive before 8s into the slot (although some clients use them earlier)

The aggregation is to be released at 8s, so attestations not received until then will not be aggregated within that slot (assuming honest validator specs are followed).

As such, I'm not sure that the name reflects the reality. Although not perfect, perhaps using deadlines instead would be
better e.g. BLOCK_RECEIPT_DEADLINE_MS set to 4000 may be clearer.

In my understanding this really is not the intention of the honest validator spec. You want the block to be released at 0s, rather than received by 4s. Similarly, you want the attestations to be released at the latest at 4s, rather than received by 8s, because the aggregates are to be released at 8s and it takes time to aggregate...

This block release schedule is not strictly incentive compatible, as we model and show in our paper, but that is a separate discussion to be had in my opinion. The language around this topic is subtle, but important. While not strictly incentive compatible, I think we should not easily throw away the framing of that duties are due by some time.

Default client settings are powerful Schelling Points and we should not encourage optimizing for network latency (release consensus messages as late as possible while being early enough, as this is the rational strategy) through use of language here. Or at least this would require a larger discussion imo.

[hwwhww]

@ppopth

I quite agree with this PR. It makes sense to also remove INTERVALS_PER_SLOT in all the previous hard forks and it also makes sense to put the *_DUE_MS in presets rather than configurations.

But I have a concern. I saw that SECONDS_PER_SLOT is a configuration rather than a preset. Those all the *_DUE_MS numbers depend on SECONDS_PER_SLOT but SECONDS_PER_SLOT is a configuration that depends on a specific network.

I guess this fact makes the *_DUE_MS numbers should be in the configurations instead?

However, personally I think all of them, including SECONDS_PER_SLOT, should be in the presets.

Oh, good point! I agree that it makes more sense to put these new parameters with SECONDS_PER_SLOT. 👍

About Presets v.s. Configurations: one argument was that we could tune SECONDS_PER_SLOT to find the shorter block time if possible. e.g., when tuning PBS. Therefore, moving all timing parameters to Configurations may also make sense. We can still add *_DUE_MS_ELECTRA configuration. Moreover, it seems more potential disasters if we move SECONDS_PER_SLOT.

I now tend to move them to configurations. 😅

[arnetheduck]

The other way to express this is to call it a BLOCK_TIMOUT_MS - this corresponds to what we want should happen: we wait for the block no longer than 4s but clients MUST send attestations as soon as they observe the block.

Ditto attestations, ie ATTESTATION_TIMEOUT_MS is the timeout when we stop aggregating things and send out the aggregate so far (but all attestations up to that point must be included, per other PR on this subject).

[ppopth]

@mcdee @casparschwa We also had a serious discussion in the past on what 4s really means, i.e. whether it means the time that the attestation MUST or SHOULD be transmitted (or it should start the generation process). And we didn't have a conclusion.

Unfortunately, the spec doesn't describe it clearly.

[ppopth]

Ditto attestations, ie ATTESTATION_TIMEOUT_MS is the timeout when we stop aggregating things and send out the aggregate so far (but all attestations up to that point must be included, per other PR on this subject).

Did you mean this PR #3472 ?

[mcdee]

As per the honest validator specs an attestation is to be released as soon as a valid block is heard, or at 4s, whichever occurs first. The intention of the honest specs, as currently written, is not to target a receival time, but a release time.

I'm not sure what you mean by "released", that isn't a word used in the validator spec. And the specs target a time at which the various processes (block generation, attestation etc.) start, with the implicit assumption that they will complete within a given timeframe (or, if not, some sort of penalty applies for example the addition of proposer boost).

In my understanding this really is not the intention of the honest validator spec. You want the block to be released at 0s, rather than received by 4s.

Yes, but the point of the 4s mark at current is that it's the cutoff time by which validators give up on the idea of seeing a block for the slot and attest based on the existing head. It isn't a time at which anything is due, per say.

We also had a serious discussion in the past on what 4s really means, i.e. whether it means the time that the attestation MUST or SHOULD be transmitted (or it should start the generation process). And we didn't have a conclusion.

I'm not sure of which bit of the second paragraph at https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/validator.md#attesting is unclear, could you expand on this?

[ppopth]

I'm not sure of which bit of the second paragraph at https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/validator.md#attesting is unclear, could you expand on this?

Alright this one has a "should" word. So never mind and sorry. I misremembered that. The discussion in the past I mentioned is on the 8s not on the 4s. It's not clear if the validator MUST or SHOULD broadcast their aggregate at 8s https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/validator.md#broadcast-aggregate

Fyi, Prysm people think that it's a MUST, so they start the aggregation process in advance and try to send the aggregate exactly at 8s (without any error).