feat/fix: add functionality & fix off-by-ones in SlotCalculator#20
feat/fix: add functionality & fix off-by-ones in SlotCalculator#20anna-carroll merged 4 commits intomainfrom
Conversation
| Self { start_timestamp: 1695902400, slot_offset: 0, slot_duration: 12 } | ||
| // begin slot calculation for Holesky from block number 1, slot number 2 | ||
| // because of a strange 324 second gap between block 0 and 1 which | ||
| // should have been 27 slots, but which is recorded as 2 slots in chain data |
There was a problem hiding this comment.
we should look into this more and maybe write something. genesis of new testnets seems very poorly documented
There was a problem hiding this comment.
yes, i tried googling a bit to see if there was a writeup anywhere and I couldn't find one. granted, the SEO for Holesky incidents is drowning in more recent reports 😅
There was a problem hiding this comment.
The math seems OK to me, but I think we should make triple sure our fixes are correct and account for the weird slot gaps. We could do this:
- query a CL for random slot numbers
- grab the output
- calculate the slot with the timestamp it gives us
- ensure the calculated slot matches the slot the output actually gave back
The point is to ensure that these gaps do not exist elsewhere "in the middle" of the history of the chain.
To accomplish this we can use this endpoint in the CL API.
why random? seems like latest is sufficient to tell that they've all been at the correct interval |
|
Yeah, latest is sufficient—what I meant with this is that we should get more than one test case ideally quite a few blocks apart. |
correct / agree, and I already checked it against the latest before submitting the PR :~) the test cases are in the code |
crates/types/src/slot.rs
Outdated
| // calculate slot | ||
| assert_eq!(calculator.calculate_slot(1695902424), 3); | ||
| assert_eq!(calculator.calculate_slot(1695902425), 3); | ||
| assert_eq!(calculator.calculate_slot(1742586984), 3890383); |
There was a problem hiding this comment.
this was latest on holesky when i submitted the PR
There was a problem hiding this comment.
you can also see we cover the oldest possible slots on both mainnet & holesky within the test cases too. so, oldest && latest
crates/types/src/slot.rs
Outdated
| assert_eq!(calculator.calculate_slot(1738863035), 11003252); | ||
| assert_eq!(calculator.calculate_slot(1738866239), 11003519); | ||
| assert_eq!(calculator.calculate_slot(1738866227), 11003518); | ||
| assert_eq!(calculator.calculate_slot(1742587235), 11313602); |
There was a problem hiding this comment.
this was latest on mainnet when i submitted the PR
anna-carroll
force-pushed
the
anna/slot-calculator
branch
from
3d9be2a to
29aac0f
Compare
Evalir
left a comment
Evalir
left a comment
There was a problem hiding this comment.
test cases look great. this code is already running in the node to avoid suspected off by 1s when getting slots (I've temporarily vendored the file).
|
CI should be fixed if you rebase :) |
anna-carroll
force-pushed
the
anna/slot-calculator
branch
from
0d5f158 to
5704f74
Compare
3 participants
Uh oh!
There was an error while loading. Please reload this page.