Skip to content

Conversation

@crStiv
Copy link
Contributor

@crStiv crStiv commented Dec 5, 2025

Fix interval calculation to use INTERVALS_PER_SLOT instead of the incorrect
double modulo operation. The store.time field stores intervals since genesis,
so calculating the interval within a slot should use modulo INTERVALS_PER_SLOT
directly. This matches the formula used in tests and ensures correctness
even if SECONDS_PER_SLOT and INTERVALS_PER_SLOT values diverge in the future.

@unnawut unnawut requested a review from g11tech December 5, 2025 12:34
@unnawut
Copy link
Collaborator

unnawut commented Dec 5, 2025

I'm not sure if they're equivalent. Removing % SECONDS_PER_SLOT we need to assume that SECONDS_PER_SLOT is a multiple of INTERVALS_PER_SLOT or identical. They are coincidentally both 4 now, so modulo twice will always give the same result.

For example if SECONDS_PER_SLOT is 3 and INTERVALS_PER_SLOT remains at 4, then the current interval is different between % 4 % 4 vs % 3 % 4

@crStiv
Copy link
Contributor Author

crStiv commented Jan 13, 2026

@unnawut

@unnawut
Copy link
Collaborator

unnawut commented Jan 19, 2026

@crStiv I re-read your description and sorry I think I messed up my thinking back then. You're right that since store.time stores an interval, it doesn't make sense to % SECONDS_PER_SLOT and your original proposal is correct to do % INTERVALS_PER_SLOT directly. I'm updating your PR back to your original proposal and will follow up with another PR to add tests. Thanks and sorry for the trouble!

@unnawut unnawut merged commit 1ee0829 into leanEthereum:main Jan 19, 2026
10 checks passed
@unnawut unnawut added the specs Scope: Changes to the specifications label Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

specs Scope: Changes to the specifications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants