lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Replace `INTERVALS_PER_SLOT` with explicit slot component times

Open eserilev opened this issue 4 months ago • 8 comments

Issue Addressed

https://github.com/ethereum/consensus-specs/pull/4476

Additional Info

This change affects many critical paths in lighthouse and should not be merged until after 8.0.0

the Ethereum kurtosis package is working to support this change here: https://github.com/ethpandaops/ethereum-package/pull/1168

eserilev avatar Aug 26 '25 21:08 eserilev

Should this change consider that the slot time will change in the future? Or that's to be handled in a future PR

dapplion avatar Oct 04 '25 10:10 dapplion

Should this change consider that the slot time will change in the future? Or that's to be handled in a future PR

I think we should handle that in a future PR to keep this diff here as small as possible

eserilev avatar Oct 06 '25 16:10 eserilev

This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏

mergify[bot] avatar Nov 26 '25 06:11 mergify[bot]

🤖 General question: Are there plans to deprecate seconds_per_slot? Both fields now exist in ChainSpec. Should seconds_per_slot be marked deprecated or documented as maintained only for config backward compatibility?

jimmygchen avatar Nov 26 '25 06:11 jimmygchen

🤖 There's a stale comment reference in beacon_node/network/src/network_beacon_processor/sync_methods.rs:302 that mentions slot_clock.single_lookup_delay() which was removed in this PR. Should update or remove this reference.

jimmygchen avatar Nov 26 '25 06:11 jimmygchen

Fixed the above comments

eserilev avatar Nov 26 '25 21:11 eserilev

Thanks for addressing the feedback! The fixes look good

jimmygchen avatar Nov 27 '25 01:11 jimmygchen

This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏

mergify[bot] avatar Dec 03 '25 15:12 mergify[bot]