-
Notifications
You must be signed in to change notification settings - Fork 5
wip: block vrf validation #288
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
base: main
Are you sure you want to change the base?
Conversation
- add test case to validate shelley's first block using validate_vrf_tpraos
…r validation error
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.
Pull Request Overview
This pull request adds VRF (Verifiable Random Function) validation for Cardano block headers, implementing validation for both TPraos and Praos consensus protocols. This is a significant enhancement to ensure block headers contain valid cryptographic proofs that the block producer was legitimately selected as the slot leader.
Key changes:
- Introduces a new
BlockVrfValidatormodule to validate VRF proofs in block headers - Adds support for genesis delegations to handle overlay schedule validation during the decentralization phase
- Implements epoch nonces publishing to provide randomness for VRF validation
Reviewed Changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| processes/omnibus/src/main.rs | Registers the new BlockVrfValidator module |
| processes/omnibus/Cargo.toml | Adds dependency on block_vrf_validator module |
| processes/omnibus/.gitignore | Ignores blocks directory (likely for testing) |
| modules/block_vrf_validator/* | New module implementing VRF validation logic |
| modules/epochs_state/* | Adds epoch nonces publishing and refactors message publishers |
| common/src/ouroboros/* | New VRF validation logic for TPraos and Praos protocols |
| common/src/genesis_values.rs | Adds genesis delegation data structures |
| common/src/protocol_params.rs | Adds nonce utility methods and Display trait |
| common/src/messages.rs | Adds EpochNoncesMessage for publishing epoch nonces |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…set, use active stake(set) instead of go snapshot
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.
Pull Request Overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull Request Overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Looks good overall! I think we should consider moving this logic into the consensus module. Also could you please break up future PRs into smaller sections? Its quite difficult to do a thorough review with this much non trivial code.
| pallas = { workspace = true } | ||
| pallas-math = { workspace = true } |
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.
Lets try to keep pallas limited to specific modules which need it.
common/src/genesis_values.rs
Outdated
| const MAINNET_SHELLEY_GENESIS_HASH: &str = | ||
| "1a3be38bcbb7911969283716ad7aa550250226b76a61fc51cc9a9a35d9276d81"; | ||
|
|
||
| pub type GenesisKey = Hash<28>; |
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.
Matt recently added GenesisKeyhash and we already have GenesisDelegate in common/types.rs, please consolidate.
| pub byron_timestamp: u64, | ||
| pub shelley_epoch: u64, | ||
| pub shelley_epoch_len: u64, | ||
| pub shelley_genesis_hash: [u8; 32], |
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 should be GenesisKeyhash type.
| EpochActivity(EpochActivityMessage), // Total fees and VRF keys for an epoch | ||
| DRepState(DRepStateMessage), // Active DReps at epoch end | ||
| SPOState(SPOStateMessage), // Active SPOs at epoch end | ||
| EpochNonce(EpochNonceMessage), // Epoch Nonce for Epoch N (published after the first block of Epoch N) |
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 message isn't needed.
| #[derive(Debug, Default)] | ||
| pub struct Snapshot { | ||
| /// Map of pool_id to its vrf_key_hash | ||
| pub active_spos: HashMap<KeyHash, KeyHash>, |
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 would prefer HashMap<PoolId, VRFKeyHash>.
| pub struct EpochSnapshots { | ||
| pub mark: Arc<Snapshot>, | ||
| pub set: Arc<Snapshot>, | ||
| pub go: Arc<Snapshot>, | ||
| } |
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.
Do we need the mark and set snapshots here or only the current snapshot?
| pub shelley_params: Option<ShelleyParams>, | ||
|
|
||
| /// protocol parameter for Praos and TPraos | ||
| pub praos_params: Option<PraosParams>, |
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.
instead of storing the entire era's parameters we should probably only store the specific parameters this module needs access to such as decentralization param.
| Self { context, topic } | ||
| } | ||
|
|
||
| /// Publish the SPDD |
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.
Left over comment here.
| tracing = { workspace = true } | ||
| serde_json = { workspace = true } | ||
| serde = { workspace = true } | ||
| pallas = { workspace = true } |
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.
What specifically from pallas do we need in this module?
Overview
This PR introduces a block VRF (Verifiable Random Function) validation system for Cardano blocks which is implemented as
block_vrf_validatormodule.Key Changes
1. Ouroboros Module Integration
ouroborosmodule in thecommoncrate, making Ouroboros protocol primitives publicly accessibleTPraosandPraosvalidation sub modulesVrfValidationErrorsfor all kinds of errors caused by VRFoverlay_scheduleutilities to check blocks which are produced by genesis keys2. Epoch Nonces Messaging Infrastructure
EpochNoncesMessageto theCardanoMessageenum for communicating epoch nonces across the systemEpochNoncesPublishermodule to encapsulate the logic for publishing epoch nonces to the message bus3. Enhanced Nonce Utilities
Displaytrait forNonce(shows hex hash or "NeutralNonce")from_number(u64)- create a nonce from a numberneutral()- create a neutral nonceseed_eta()- seed nonce for randomness computationseed_l()- seed nonce for leader computation4. VRF Validation State Management
modules/block_vrf_validator/src/state.rsfor managing VRF validation stateEpochSnapshotsstruct to manage a sliding window of three epoch snapshots (mark, set, go)5. Improved Error Handling
ValidationError::BadVRFto encapsulateVrfValidationError6. Publishing Vrf Validation Status
CardanoMessage::BlockValidation(ValidationStatus)to encapsulateVrfValidationErrorvrf_validation_publishertoblock_vrf_validatormodule to publish Block's Vrf Validation Result with detailed errorNOTE:
I have a few blocks invalidated even they are valid, because of
VrfLeaderValueTooBig.I am looking into
active_stakesandtotal_active_stakes.