-
Notifications
You must be signed in to change notification settings - Fork 955
Description
Description
For the upcoming Gloas/EIP7732 upgrade, we will have new inner slot timing configs which are:
`ATTESTATION_DUE_BPS_GLOAS`,
`AGGREGATE_DUE_BPS_GLOAS`,
`SYNC_MESSAGE_DUE_BPS_GLOAS`,
`CONTRIBUTION_DUE_BPS_GLOAS`,
`PAYLOAD_ATTESTATION_DUE_BPS`
and before Gloas, the spec had:
`ATTESTATION_DUE_BPS`,
`AGGREGATE_DUE_BPS`,
`SYNC_MESSAGE_DUE_BPS`,
`CONTRIBUTION_DUE_BPS`,
The issue is lighthouse currently does NOT use the above configs for slot timing. Instead it uses both hard coded magic numbers throughout code in addition to using the deprecated interval_per_slot. (see: ethereum/consensus-specs#4805)
For example, to ensure that we wait past ATTESTATION_DUE_BPS to send attestations we wait via sleep(duration_to_next_slot + slot_duration / 3).await; to start the attention validator tasks. This behaves close to spec given ATTESTATION_DUE_BPS: 3333 in mainnet.yaml config. see https://github.com/sigp/lighthouse/blob/stable/validator_client/validator_services/src/attestation_service.rs#L163
In addition, SlotClock uses interval_per_slot for other inner slot timings, see: https://github.com/sigp/lighthouse/blob/stable/common/slot_clock/src/lib.rs
Proposed Solution
To make future upgrades (including Gloas) to the inner slot timings more maintainable and readable I propose:
- Make the timing config in ChainSpec source their values from the config.yaml in
common/eth2_network_config. - right now they are hard coded directly in ChainSpec. - In ChainSpec add helper methods that get timing configs as a function of epoch, so we can return the Gloas ones when needed.
- Refactor both validator and beacon client code to use the inner slot timing config methods we just created in ChainSpec instead of using
interval_per_slotor magic numbers.
Testing
Given the current inner slot timing does not match exactly what is set in the spec config, I would assume we need to actually do some significant testing for this "refactor".
Note
I would be happy to implement this myself after sign off from maintainers and if i could get assistance on required testing.