-
Notifications
You must be signed in to change notification settings - Fork 2
Description
There is no need to expose values through both constants & config, e.g. if you put slot_duration_ms into config, then don't make SLOT_DURATION_MS global constant. Keeping both is redundant.
Another issue is, that you're exposing DEVNET_CONFIG as constant, what makes whole config thing kinda pointless, as primary usecase for configurations is to be able to swap them easily during runtime. E.g., when lean ethereum will go to mainnet, you'll have to use MAINNET_CONFIG everywhere, instead of DEVNET_CONFIG. So best solution to this is to not expose any constants at all, but make some method for config, just like grandine does.
Last thing, please remove this test, as there is no need to test constants:
#[cfg(test)]
mod tests {
use super::*;
#[test] fn time_math_is_consistent() {
assert_eq!(SLOT_DURATION_MS, 4_000);
assert_eq!(SECONDS_PER_SLOT, 4);
assert_eq!(SECONDS_PER_INTERVAL, 1);
}
}UPDATE: forgot to mention, also please change:
// lean_client/chain/src/lib.rs file
pub mod config;To something like:
// lean_client/chain/src/lib.rs file
mod config;
pub use config::ChainConfig;So that everyone can use just ChainConfig directly, without ever knowing about this subcrate structure.