feat:Add decoder and callbacks for Governance Protocol Parameters and Delegation #459

buddhisthead · 2025-12-05T20:48:25Z

Description

This PR adds mini-cbor decoder and callbacks for Governance Protocol Parameters and Delegation into the Snapshot parser.

added ProtocolParameters type
converted the RationalNumber type to use decoder and other depedent types of the protocol parameters
add constitution parsing as a correctness checkpoint (not needed yet for bootstrapping) just before protocol params
add stub callbacks for the boostrapper
fixed the Anchor decoder so that reads various formatted structures (bytes and strings)

Related Issue(s)

Resolves issue #445

How was this tested?

It was tested manually by running make snap-test-streaming to demonstrate proper parsing and logging of the previous, current, and future (empty) protocol parameters as well as the delegation. You can run that command and look for the output in the logs more directly with make snap-test-streaming 2>&1 | grep -A8 "Protocol Parameters"

Checklist

My code builds and passes local tests
I added/updated tests for my changes, where applicable
I updated documentation (if applicable)
CI is green for this PR

Impact / Side effects

I've added an empty stub callback in the boostrapper as a TODO item, which can be filled as needed.

Reviewer notes / Areas to focus

Nothing specific. I've tried to build a type called ProtocolParameters that uses the existing types and fixed up their decoders, so there is no "adapter" layer required.

Copilot

Pull request overview

This PR adds comprehensive CBOR decoding support for Governance Protocol Parameters and Delegation to the snapshot parser, enabling proper parsing and handling of Conway-era governance state during bootstrapping.

Key changes:

Converted RationalNumber from a type alias to a wrapper struct with minicbor::Decode implementation, enabling CBOR deserialization
Added new ProtocolParameters type with full decoder for current, previous, and future protocol parameters from snapshots
Extended the snapshot parser with GovernanceProtocolParametersCallback trait and integration

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`common/src/rational_number.rs`	Converted from type alias to wrapper struct, added CBOR decoder, `Display`, `FromStr`, constants (ZERO, ONE), and `Deref` trait
`common/src/snapshot/protocol_parameters.rs`	New file with complete protocol parameters decoder handling Conway-era governance parameters
`common/src/snapshot/decode.rs`	New utility module for heterogeneous CBOR array/map decoding with comprehensive tests
`common/src/snapshot/mod.rs`	Added module exports for protocol_parameters and decode
`common/src/types.rs`	Added CBOR decoders for Anchor, Constitution, ExUnits, ExUnitPrices, CostModel, voting thresholds
`common/src/snapshot/streaming_snapshot.rs`	Added GovernanceProtocolParametersCallback trait, integrated protocol parameters parsing, decoder scoping improvements
`modules/snapshot_bootstrapper/src/publisher.rs`	Added stub callback implementation for protocol parameters (TODO for actual publishing)
`common/examples/test_streaming_parser.rs`	Added example callback implementation with protocol parameters logging
`modules/spo_state/src/spo_state.rs`	Updated to use `RationalNumber::ZERO` constant and clone method
`modules/spo_state/src/epochs_history.rs`	Updated to use `RationalNumber::ZERO` constant and clone method
`modules/parameters_state/src/parameters_updater.rs`	Updated to clone RationalNumber where needed
`modules/parameters_state/src/genesis_params.rs`	Updated `RationalNumber::from()` calls to use new signature (numerator, denominator)
`modules/governance_state/src/voting_state.rs`	Updated to clone RationalNumber values in vote threshold calculations
`modules/governance_state/src/alonzo_babbage_voting.rs`	Updated test to clone RationalNumber
`modules/block_vrf_validator/src/state.rs`	Updated to clone RationalNumber in protocol parameter handling and VRF validation
`modules/block_vrf_validator/src/ouroboros/vrf_validation.rs`	Updated to clone RationalNumber in error construction
`modules/block_vrf_validator/src/ouroboros/tpraos.rs`	Updated test to use `RationalNumber::ONE` constant
`modules/block_vrf_validator/src/ouroboros/overlay_schedule.rs`	Updated test to use `RationalNumber::ONE` constant
`modules/accounts_state/src/monetary.rs`	Updated to clone RationalNumber when accessing monetary expansion factor
`common/src/protocol_params.rs`	Updated `PraosParams::from()` to clone active_slots_coeff

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-05T20:52:59Z

common/src/snapshot/protocol_parameters.rs

+                    0 => plutus_v1 = Some(cost_model),
+                    1 => plutus_v2 = Some(cost_model),
+                    2 => plutus_v3 = Some(cost_model),
+                    _ => unreachable!("unexpected language version: {}", lang_id),


Using unreachable!() with a panic message suggests this case might actually be reachable. If future Plutus versions (v4, v5, etc.) are added to the protocol, this will cause a panic instead of returning a proper error. Consider using return Err(minicbor::decode::Error::message(format!("unsupported language version: {}", lang_id))) instead to handle unknown language versions gracefully.

Suggested change

_ => unreachable!("unexpected language version: {}", lang_id),

_ => return Err(minicbor::decode::Error::message(format!("unsupported language version: {}", lang_id))),

Copilot · 2025-12-05T20:52:59Z

common/src/rational_number.rs

 use std::str::FromStr;

-pub type RationalNumber = num_rational::Ratio<u64>;
+#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]


Consider deriving Copy for RationalNumber since it wraps Ratio<u64> which is essentially two u64 values (16 bytes total). This would improve performance in critical paths like block VRF validation (see modules/block_vrf_validator/src/state.rs:98,103) where RationalNumber values are currently cloned. Add #[derive(Copy)] to the struct definition alongside the existing derives.

Suggested change

#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]

#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]

Copilot · 2025-12-05T20:52:59Z

common/src/snapshot/streaming_snapshot.rs


+/// Callback invoked with Governance ProtocolParameters (previous, current, future)
+pub trait GovernanceProtocolParametersCallback {
+    /// Called once with all proposals


The documentation comment "Called once with all proposals" is incorrect for GovernanceProtocolParametersCallback. It should describe that this callback is invoked with the previous, current, and future protocol parameters from the governance state, not proposals.

Suggested change

/// Called once with all proposals

/// Called once with the previous, current, and future protocol parameters from the governance state

Copilot · 2025-12-05T20:53:00Z

common/src/snapshot/protocol_parameters.rs

+impl<'b, C> minicbor::decode::Decode<'b, C> for ProtocolParameters {
+    fn decode(d: &mut minicbor::Decoder<'b>, ctx: &mut C) -> Result<Self, minicbor::decode::Error> {
+        d.array()?;
+
+        // Check first field - for future params it might be a variant tag
+        let first_field_type = d.datatype()?;
+
+        // Peek at the value if it's U8
+        if first_field_type == minicbor::data::Type::U8 {
+            let tag_value = d.u8()?;
+
+            // future_pparams = [0] / [1, pparams_real] / [2, strict_maybe<pparams_real>]
+            if tag_value == 0 {
+                // Return default/empty protocol parameters
+                return Ok(ProtocolParameters::default());
+            } else if tag_value == 1 {
+                // Continue with normal parsing below (tag already consumed)
+            } else if tag_value == 2 {
+                // Next element might be Nothing or Just(params)
+                // For now, skip this case
+                return Err(minicbor::decode::Error::message(
+                    "Future params variant [2] not yet implemented",
+                ));
+            } else {
+                // Not a variant tag, this is the actual first field (max_block_header_size?)
+                let unknown_field = tag_value;
+                return Self::decode_real_params(d, ctx, unknown_field);
+            }
+        }
+
+        // If we get here, we have variant [1] or [2] and need to parse the real params
+        // For variant [1], the next field should be the start of pparams_real
+        // which starts with the first real field (not a tag)
+        let first_field_type = d.datatype()?;
+        if first_field_type == minicbor::data::Type::U8 {
+            let first_field = d.u8()?;
+            Self::decode_real_params(d, ctx, first_field)
+        } else {
+            Err(minicbor::decode::Error::message(
+                "Expected U8 for first field of pparams_real",
+            ))
+        }
+    }
+}
+
+impl ProtocolParameters {
+    fn decode_real_params<'b, C>(
+        d: &mut minicbor::Decoder<'b>,
+        ctx: &mut C,
+        first_field: u8,
+    ) -> Result<Self, minicbor::decode::Error> {
+        // first_field is field 0 which we already consumed (U8=44 or similar, unknown purpose)
+
+        // Read what appears to be the fee parameters
+        let min_fee_a = d.u32()? as u64;
+        let min_fee_b = d.u32()? as u64;
+
+        // Read what appears to be size limits (but check types - they might be u16 not u64)
+        let max_block_body_size = d.u16()? as u64;
+        let max_transaction_size = d.u16()? as u64;
+
+        // Deposits
+        let stake_credential_deposit = d.u32()? as u64;
+        let stake_pool_deposit = d.u32()? as u64;
+
+        // Retirement epoch
+        let stake_pool_max_retirement_epoch = d.u8()? as u64;
+
+        // Pool count
+        let optimal_stake_pools_count = d.u16()?;
+
+        // Fields 9-11 should be ratios (Tag 30)
+        let pledge_influence = decode_rationale(d)?;
+        let monetary_expansion_rate = decode_rationale(d)?;
+        let treasury_expansion_rate = decode_rationale(d)?;
+
+        // Field 12 should be protocol version array
+        let protocol_version = decode_protocol_version(d)?;
+
+        // Field 13
+        let min_pool_cost = d.u32()? as u64;
+
+        // Field 14
+        let lovelace_per_utxo_byte = d.u16()? as u64;
+
+        // Field 15: cost_models map - manually decode since CostModel format might be different
+        let mut plutus_v1 = None;
+        let mut plutus_v2 = None;
+        let mut plutus_v3 = None;
+
+        let map_len = d.map()?;
+
+        if let Some(len) = map_len {
+            for _ in 0..len {
+                let lang_id: u8 = d.decode()?;
+
+                // Try decoding as array of i64 (could be indefinite)
+                let array_len = d.array()?;
+
+                let mut costs = Vec::new();
+                if array_len.is_none() {
+                    // Indefinite array - read until break
+                    loop {
+                        match d.datatype()? {
+                            minicbor::data::Type::Break => {
+                                d.skip()?; // consume the break
+                                break;
+                            }
+                            _ => {
+                                // Decode as i64, handling different integer sizes
+                                let cost: i64 = d.decode()?;
+                                costs.push(cost);
+                            }
+                        }
+                    }
+                } else if let Some(alen) = array_len {
+                    for _ in 0..alen {
+                        let cost: i64 = d.decode()?;
+                        costs.push(cost);
+                    }
+                }
+
+                let cost_model = CostModel::new(costs);
+                match lang_id {
+                    0 => plutus_v1 = Some(cost_model),
+                    1 => plutus_v2 = Some(cost_model),
+                    2 => plutus_v3 = Some(cost_model),
+                    _ => unreachable!("unexpected language version: {}", lang_id),
+                }
+            }
+        }
+
+        // Field 16: prices - encoded as array containing two tag-30 ratios
+        d.array()?; // Outer array
+        let mem_price = decode_rationale(d)?; // First ratio (tag 30)
+        let step_price = decode_rationale(d)?; // Second ratio (tag 30)
+        let prices = ExUnitPrices {
+            mem_price: RationalNumber::from(mem_price.numerator, mem_price.denominator),
+            step_price: RationalNumber::from(step_price.numerator, step_price.denominator),
+        };
+
+        // Field 17: max_tx_ex_units
+        let max_tx_ex_units = d.decode_with(ctx)?;
+
+        // Field 18: max_block_ex_units
+        let max_block_ex_units = d.decode_with(ctx)?;
+
+        // Field 19: max_value_size
+        let max_value_size = d.u16()? as u64;
+
+        // Field 20: collateral_percentage
+        let collateral_percentage = d.u16()?;
+
+        // Field 21: max_collateral_inputs
+        let max_collateral_inputs = d.u16()?;
+
+        // Field 22: pool_voting_thresholds
+        let pool_voting_thresholds = d.decode_with(ctx)?;
+
+        // Field 23: drep_voting_thresholds
+        let drep_voting_thresholds = d.decode_with(ctx)?;
+
+        // Field 24: min_committee_size
+        let min_committee_size = d.u16()?;
+
+        // Field 25: max_committee_term_length
+        let max_committee_term_length = d.u64()?;
+
+        // Field 26: gov_action_lifetime
+        let gov_action_lifetime = d.u64()?;
+
+        // Field 27: gov_action_deposit
+        let gov_action_deposit = d.u64()?;
+
+        // Field 28: drep_deposit
+        let drep_deposit = d.u64()?;
+
+        // Field 29: drep_expiry
+        let drep_expiry = d.decode_with(ctx)?;
+
+        // Field 30: min_fee_ref_script_lovelace_per_byte
+        let min_fee_ref_script_lovelace_per_byte = decode_rationale(d)?;
+
+        // Field 0 (U8=44) - still unknown, need to determine max_block_header_size
+        let max_block_header_size = first_field as u16;
+
+        Ok(ProtocolParameters {
+            protocol_version,
+            min_fee_a,
+            min_fee_b,
+            max_block_body_size,
+            max_transaction_size,
+            max_block_header_size,
+            stake_credential_deposit,
+            stake_pool_deposit,
+            stake_pool_max_retirement_epoch,
+            optimal_stake_pools_count,
+            pledge_influence,
+            monetary_expansion_rate,
+            treasury_expansion_rate,
+            min_pool_cost,
+            lovelace_per_utxo_byte,
+            cost_models: CostModels {
+                plutus_v1,
+                plutus_v2,
+                plutus_v3,
+            },
+            prices,
+            max_tx_ex_units,
+            max_block_ex_units,
+            max_value_size,
+            collateral_percentage,
+            max_collateral_inputs,
+            pool_voting_thresholds,
+            drep_voting_thresholds,
+            min_committee_size,
+            max_committee_term_length,
+            gov_action_lifetime,
+            gov_action_deposit,
+            drep_deposit,
+            drep_expiry,
+            min_fee_ref_script_lovelace_per_byte,
+            max_ref_script_size_per_tx: 200 * 1024,
+            max_ref_script_size_per_block: 1024 * 1024,
+            ref_script_cost_stride: 25600,
+            ref_script_cost_multiplier: Ratio {
+                numerator: 12,
+                denominator: 10,
+            },
+        })
+    }
+}


The new ProtocolParameters decoder lacks unit tests. Given the complexity of the decoder (handling variants, tag 30 rationals, cost models, etc.) and the critical nature of protocol parameters, consider adding unit tests to verify correct parsing of various protocol parameter formats, especially edge cases like empty future parameters (variant [0]), indefinite/definite arrays for cost models, and the handling of the mysterious first_field value.

Copilot · 2025-12-05T20:53:00Z

common/src/snapshot/streaming_snapshot.rs

 //! These snapshots represent the internal `NewEpochState` type and are not formally
 //! specified - see: https://github.com/IntersectMBO/cardano-ledger/blob/33e90ea03447b44a389985ca2b158568e5f4ad65/eras/shelley/impl/src/Cardano/Ledger/Shelley/LedgerState/Types.hs#L121-L131
-//!
+//! and ttps://github.com/rrruko/nes-cddl-hs/blob/main/nes.cddl


Typo in URL: missing 'h' in 'https'. Should be "https://github.com/rrruko/nes-cddl-hs/blob/main/nes.cddl"

Suggested change

//! and ttps://github.com/rrruko/nes-cddl-hs/blob/main/nes.cddl

//! and https://github.com/rrruko/nes-cddl-hs/blob/main/nes.cddl

That's a good catch.

Copilot · 2025-12-05T20:53:00Z

common/src/snapshot/protocol_parameters.rs

+            max_ref_script_size_per_tx: 200 * 1024,
+            max_ref_script_size_per_block: 1024 * 1024,
+            ref_script_cost_stride: 25600,
+            ref_script_cost_multiplier: Ratio {
+                numerator: 12,
+                denominator: 10,
+            },


Hardcoded values for reference script parameters should be documented or sourced from a specification. The values for max_ref_script_size_per_tx (200 * 1024), max_ref_script_size_per_block (1024 * 1024), ref_script_cost_stride (25600), and ref_script_cost_multiplier (12/10) are not parsed from the snapshot but are hardcoded. If these fields are not present in the snapshot format, this should be documented with a comment explaining why these specific default values are used.

It's the default, co-pilot!

lowhung

Approved with some minor comments

lowhung · 2025-12-09T16:34:19Z

common/src/types.rs

 pub struct ExUnitPrices {
-    pub mem_price: RationalNumber,
-    pub step_price: RationalNumber,
+    pub mem_price: RationalNumber,  // why not use Ratio here?


Should we remove these comments? Or still relevant?

Yep, will do.

lowhung · 2025-12-09T17:08:27Z

Makefile


 fmt:
-	$(CARGO) fmt --all
+	$(CARGO) fmt --all -- --check


I think we should add a new target or rename this.

Suggested change

$(CARGO) fmt --all -- --check

fmt:

$(CARGO) fmt --all

fmt-check:

$(CARGO) fmt --all -- --check

On the fence about this, as it now matches what CI/CD does, which is my intent. One-stop shopping! I like a simple name when possible. I don't know if anyone else was even using it.

Thought about it some more and made the change, though you may not like it. I used "check" for the new name. It wasn't taken yet, and it can be my one-stop-shopping target for CI/CD compliance.

…elgation * converted the RationalNumber type to use decoder and other depedent types of the protocol parameters * add constitution parsing * add stub callbacks for the boostrapper * fixed the Anchor decoder so that reads various formatted structures (bytes and strings) Address clippy and fmt errors Apply some PR feedback chage requests

buddhisthead requested review from Copilot and lowhung December 5, 2025 20:48

Copilot started reviewing on behalf of buddhisthead December 5, 2025 20:48 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

github-actions bot mentioned this pull request Dec 8, 2025

📊 Weekly Status - Week of 2025-12-08 #460

Open

lowhung approved these changes Dec 9, 2025

View reviewed changes

buddhisthead force-pushed the cet/snapshot-protocol-params-445 branch from d92c322 to 622125f Compare December 9, 2025 23:08

buddhisthead merged commit c863a6d into main Dec 9, 2025
2 checks passed

buddhisthead deleted the cet/snapshot-protocol-params-445 branch December 9, 2025 23:23

github-actions bot mentioned this pull request Dec 15, 2025

📊 Weekly Status - Week of 2025-12-15 #496

Open

	_ => unreachable!("unexpected language version: {}", lang_id),
	_ => return Err(minicbor::decode::Error::message(format!("unsupported language version: {}", lang_id))),

	#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
	#[derive(serde::Deserialize, serde::Serialize, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]

	/// Called once with all proposals
	/// Called once with the previous, current, and future protocol parameters from the governance state

	//! and ttps://github.com/rrruko/nes-cddl-hs/blob/main/nes.cddl
	//! and https://github.com/rrruko/nes-cddl-hs/blob/main/nes.cddl

feat:Add decoder and callbacks for Governance Protocol Parameters and Delegation #459

feat:Add decoder and callbacks for Governance Protocol Parameters and Delegation #459

Uh oh!

Conversation

buddhisthead commented Dec 5, 2025

Description

Related Issue(s)

How was this tested?

Checklist

Impact / Side effects

Reviewer notes / Areas to focus

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

buddhisthead Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

buddhisthead Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

lowhung left a comment

Choose a reason for hiding this comment

Uh oh!

lowhung Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

buddhisthead Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lowhung Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

buddhisthead Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

buddhisthead Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

buddhisthead Dec 9, 2025 •

edited

Loading