IntersectMBO / cardano-base

Code used throughout the Cardano eco-system
Apache License 2.0
95 stars 41 forks source link

Move the `Num` instance for `EpochNo` and `EpochSize` #455

Closed lehins closed 8 months ago

lehins commented 10 months ago

Word64 is the type that both of the EpochNo and EpochSize are based on. It especially prone to integer overflow when doing arithmetic. For this reason it would be safer to remove Num instance alltogether for those types from the main library

This will be even less of an inconvenience for EpochNo, because in ledger we have introduced EpochInterval type and addEpochInterval :: EpochNo -> EpochInterval -> EpochNo function that make the Num instance less useful.

The only downside to removing the Num instance is that construction from literals requires usage of a constructor, but that is only a tiny burden for testing.

Therefore, instead of completely removing the Num instances, I suggest we move them as standalone orphans into the new testlib, which would allow us to have the cake and eat it too.

As part of this task we should also move EpochInterval and addEpochInterval from cardano-ledger into the cardano-slotting