-
Notifications
You must be signed in to change notification settings - Fork 3
Add timestamp
and epoch_slot
to BlockInfo
#150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Self { | ||
proposals: HashMap::new(), | ||
shelley_slots_per_epoch: MAINNET_SHELLEY_SLOTS_PER_EPOCH, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This governance_state
module was hard-coded to use mainnet slots per epoch length, so it wouldn't have worked against sanchonet. I updated it so it defaults to the mainnet value, but reads the real epoch length from protocol parameters when it sees those.
It'd be cleaner to get rid of the default, but I feel like this PR is big enough already. I only updated it in the first place so I could get hard-coded values out of common
.
I thought for some time about this commit, because it's hard to find a good foundation for the thought. Finally I have come with these two options:
I see the following variants for "simple" solution. The "automatic" solution: The variants in between have too many drawbacks to my opinion: they're technically much more complicated (compared to their outcome). So, I can be more specific on this topic if you're interested. |
download( | ||
&client, | ||
"https://raw.githubusercontent.com/Hornan7/SanchoNet-Tutorials/refs/heads/main/genesis/shelley-genesis.json", | ||
"sanchonet-shelley-genesis.json", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shelley genesis has bugs, and cannot be properly parsed by Pallas, see parameters-state module.
// Include genesis data (downloaded by build.rs) | ||
const MAINNET_BYRON_GENESIS: &[u8] = include_bytes!("../downloads/mainnet-byron-genesis.json"); | ||
const MAINNET_SHELLEY_GENESIS: &[u8] = include_bytes!("../downloads/mainnet-shelley-genesis.json"); | ||
const MAINNET_SHELLEY_START_EPOCH: u64 = 208; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is still hardcoded. Why not compute it?
let values = GenesisValues { | ||
byron_timestamp: byron_genesis.start_time, | ||
shelley_epoch: shelley_start_epoch, | ||
shelley_epoch_len: shelley_genesis.epoch_length.unwrap() as u64, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd propose to use parameter-state module output here: it should provide all these data. If it does not, then it's much easier to add additional fields there.
We discussed the solution to this briefly - I like the idea of a single source of truth of these things, ideally the actual genesis JSON files rather than having any thing hardcoded. If we can remove Pallas from the equation, we could make this a library in common, with a single build-time fetch, and any module that wants it can then query it for the configured network. Only problem is that we don't have any global config mechanism so the network would need to be configured independently, unless we add a link to global config in the Caryatid Context (which we could)... I'm going to merge this PR for now to stop it being a blocker but copy this discussion to a new issue: #155 |
Add two useful fields to the common
BlockInfo
struct attached to all Cardano messages:epoch_slot
andtimestamp
. These fields can both be computed from theslot
, but only with access to some values populated at genesis. (This is also true for the existingepoch
field).This PR also ensures that we compute these values correctly: we create a
GenesisValues
struct ingenesis-bootstrapper
, and read it in modules which need it (mithril-snapshot-fetcher
andupstream-chain-fetcher
). This'll make it easier to run in other environments down the road.@shd if you want to keep running
upstream-chain-fetcher
independently, you need to set"byron-timestamp"
in your local config. The fetcher still reads genesis values from local config, but it'll wait forgenesis-bootstrapper
to run to use values from there if any of those values are missing from local config.Fixes #147