feat:Implement snapshot parser for snapshot delegation and pool registration #446
Conversation
* Implemented parser and type conversions for pool registration * Parsing delegations * Add logging output for new snapshot data to the test example
There was a problem hiding this comment.
Pull request overview
This PR implements parsing support for snapshot delegations and pool registration data within the Mark, Set, Go snapshot structure. The changes extend the existing snapshot parser to extract and convert pool parameters and delegation mappings from Cardano ledger state snapshots.
Key changes:
- Added parsing for delegation mappings (credential → pool key hash) and pool parameters (pool key hash → pool registration details)
- Implemented type conversions from internal CBOR types to public API types (Relay, PoolMetadata, StakeAddress, PoolRegistration)
- Enhanced test output to display sample stake data for Mark, Set, and Go snapshots
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| common/src/snapshot/streaming_snapshot.rs | Changed parse_pstate visibility from private to public to support pool parameter parsing |
| common/src/snapshot/mark_set_go.rs | Extended Snapshot struct with delegation and pool parameter fields; implemented parsing and type conversion logic for pool registration data |
| common/examples/test_streaming_parser.rs | Enhanced snapshot output formatting to display sample stakes for each snapshot type (Mark, Set, Go) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| eprintln!("Fee: {:.2} ADA", snapshots.fee as f64 / 1_000_000.0); | ||
| eprintln!(); | ||
|
|
There was a problem hiding this comment.
[nitpick] Trailing whitespace detected on this line.
| let registration = VMap( | ||
| pools | ||
| .0 | ||
| .into_iter() | ||
| .map(|(pool_id, params)| { | ||
| // Convert RewardAccount (Vec<u8>) to StakeAddress | ||
| let reward_account = | ||
| StakeAddress::from_binary(¶ms.reward_account.0) | ||
| .unwrap_or_else(|_| StakeAddress::default()); | ||
|
|
||
| // Convert Set<AddrKeyhash> to Vec<StakeAddress> | ||
| let pool_owners: Vec<StakeAddress> = params | ||
| .owners | ||
| .0 | ||
| .into_iter() | ||
| .map(|keyhash| { | ||
| StakeAddress::new( | ||
| StakeCredential::AddrKeyHash(keyhash), | ||
| NetworkId::Mainnet, | ||
| ) | ||
| }) | ||
| .collect(); | ||
|
|
||
| // Convert Vec<streaming_snapshot::Relay> to Vec<types::Relay> | ||
| let relays: Vec<types::Relay> = params | ||
| .relays | ||
| .into_iter() | ||
| .map(|relay| match relay { | ||
| streaming_snapshot::Relay::SingleHostAddr( | ||
| port, | ||
| ipv4, | ||
| ipv6, | ||
| ) => { | ||
| let port_opt = match port { | ||
| streaming_snapshot::Nullable::Some(p) => { | ||
| Some(p as u16) | ||
| } | ||
| _ => None, | ||
| }; | ||
| let ipv4_opt = match ipv4 { | ||
| streaming_snapshot::Nullable::Some(ip) | ||
| if ip.0.len() == 4 => | ||
| { | ||
| Some(std::net::Ipv4Addr::new( | ||
| ip.0[0], ip.0[1], ip.0[2], ip.0[3], | ||
| )) | ||
| } | ||
| _ => None, | ||
| }; | ||
| let ipv6_opt = match ipv6 { | ||
| streaming_snapshot::Nullable::Some(ip) | ||
| if ip.0.len() == 16 => | ||
| { | ||
| let b = &ip.0; | ||
| Some(std::net::Ipv6Addr::from([ | ||
| b[0], b[1], b[2], b[3], b[4], b[5], | ||
| b[6], b[7], b[8], b[9], b[10], b[11], | ||
| b[12], b[13], b[14], b[15], | ||
| ])) | ||
| } | ||
| _ => None, | ||
| }; | ||
| types::Relay::SingleHostAddr( | ||
| types::SingleHostAddr { | ||
| port: port_opt, | ||
| ipv4: ipv4_opt, | ||
| ipv6: ipv6_opt, | ||
| }, | ||
| ) | ||
| } | ||
| streaming_snapshot::Relay::SingleHostName( | ||
| port, | ||
| hostname, | ||
| ) => { | ||
| let port_opt = match port { | ||
| streaming_snapshot::Nullable::Some(p) => { | ||
| Some(p as u16) | ||
| } | ||
| _ => None, | ||
| }; | ||
| types::Relay::SingleHostName( | ||
| types::SingleHostName { | ||
| port: port_opt, | ||
| dns_name: hostname, | ||
| }, | ||
| ) | ||
| } | ||
| streaming_snapshot::Relay::MultiHostName(hostname) => { | ||
| types::Relay::MultiHostName(types::MultiHostName { | ||
| dns_name: hostname, | ||
| }) | ||
| } | ||
| }) | ||
| .collect(); | ||
|
|
||
| // Convert Nullable<PoolMetadata> to Option<PoolMetadata> | ||
| let pool_metadata = match params.metadata { | ||
| streaming_snapshot::Nullable::Some(meta) => { | ||
| Some(types::PoolMetadata { | ||
| url: meta.url, | ||
| hash: meta.hash.to_vec(), | ||
| }) | ||
| } | ||
| _ => None, | ||
| }; | ||
|
|
||
| ( | ||
| pool_id, | ||
| PoolRegistration { | ||
| operator: params.id, | ||
| vrf_key_hash: params.vrf, | ||
| pledge: params.pledge, | ||
| cost: params.cost, | ||
| margin: Ratio { | ||
| numerator: params.margin.numerator, | ||
| denominator: params.margin.denominator, | ||
| }, | ||
| reward_account, | ||
| pool_owners, | ||
| relays, | ||
| pool_metadata, | ||
| }, | ||
| ) | ||
| }) | ||
| .collect(), | ||
| ); |
There was a problem hiding this comment.
[nitpick] The extensive type conversion logic (130+ lines) for transforming PoolParams to PoolRegistration makes this function difficult to maintain. Consider extracting this conversion into a separate method like PoolParams::to_pool_registration() or a From trait implementation.
This would improve testability and make the parsing logic easier to follow.
| let cred_str = match cred { | ||
| acropolis_common::StakeCredential::AddrKeyHash(h) => { | ||
| format!("KeyHash({}...)", &hex::encode(&h[..4])) | ||
| } | ||
| acropolis_common::StakeCredential::ScriptHash(h) => { | ||
| format!("ScriptHash({}...)", &hex::encode(&h[..4])) | ||
| } | ||
| }; | ||
| eprintln!( | ||
| " [{}] {} -> {:.2} ADA", | ||
| i + 1, | ||
| cred_str, | ||
| *amount as f64 / 1_000_000.0 | ||
| ); | ||
| } | ||
| } | ||
| eprintln!(); | ||
|
|
||
| eprintln!("Set Snapshot:"); | ||
| eprintln!(" Delegators: {}", snapshots.set.0.len()); | ||
| eprintln!(" Total stake: {:.2} ADA", set_total as f64 / 1_000_000.0); | ||
| if !snapshots.set.0.is_empty() { | ||
| eprintln!(" Sample stakes (first 5):"); | ||
| for (i, (cred, amount)) in snapshots.set.0.iter().take(5).enumerate() { | ||
| let cred_str = match cred { | ||
| acropolis_common::StakeCredential::AddrKeyHash(h) => { | ||
| format!("KeyHash({}...)", &hex::encode(&h[..4])) | ||
| } | ||
| acropolis_common::StakeCredential::ScriptHash(h) => { | ||
| format!("ScriptHash({}...)", &hex::encode(&h[..4])) | ||
| } | ||
| }; | ||
| eprintln!( | ||
| " [{}] {} -> {:.2} ADA", | ||
| i + 1, | ||
| cred_str, | ||
| *amount as f64 / 1_000_000.0 | ||
| ); | ||
| } | ||
| } | ||
| eprintln!(); | ||
|
|
||
| eprintln!("Go Snapshot:"); | ||
| eprintln!(" Delegators: {}", snapshots.go.0.len()); | ||
| eprintln!(" Total stake: {:.2} ADA", go_total as f64 / 1_000_000.0); | ||
| if !snapshots.go.0.is_empty() { | ||
| eprintln!(" Sample stakes (first 5):"); | ||
| for (i, (cred, amount)) in snapshots.go.0.iter().take(5).enumerate() { | ||
| let cred_str = match cred { | ||
| acropolis_common::StakeCredential::AddrKeyHash(h) => { | ||
| format!("KeyHash({}...)", &hex::encode(&h[..4])) | ||
| } | ||
| acropolis_common::StakeCredential::ScriptHash(h) => { | ||
| format!("ScriptHash({}...)", &hex::encode(&h[..4])) | ||
| } | ||
| }; |
There was a problem hiding this comment.
[nitpick] There is duplicated code for formatting stake credentials across the three snapshot sections (Mark, Set, Go). The same pattern is repeated at lines 291-298, 315-322, and 339-346.
Consider extracting this into a helper function to reduce code duplication:
fn format_credential(cred: &StakeCredential) -> String {
match cred {
acropolis_common::StakeCredential::AddrKeyHash(h) => {
format!("KeyHash({}...)", &hex::encode(&h[..4]))
}
acropolis_common::StakeCredential::ScriptHash(h) => {
format!("ScriptHash({}...)", &hex::encode(&h[..4]))
}
}
}| let snapshot_stake: VMap<StakeCredential, i64> = decoder.decode()?; | ||
|
|
||
| // Skip delegations (second element) | ||
| decoder.skip().context("Failed to skip snapshot_delegations")?; | ||
| info!(" {snapshot_name} snapshot - parsing snapshot_delegations..."); | ||
|
|
||
| // Skip pool_params (third element) | ||
| decoder.skip().context("Failed to skip snapshot_pool_params")?; | ||
| let delegations: VMap<StakeCredential, Hash<28>> = | ||
| decoder.decode().context("Failed to parse snapshot_delegations")?; | ||
|
|
||
| Ok(Snapshot { snapshot_stake }) | ||
| } | ||
| other_type => { | ||
| info!( | ||
| " {snapshot_name} snapshot - first element is {other_type:?}, skipping entire array" | ||
| " {snapshot_name} snapshot - parsing snapshot_pool_registration..." | ||
| ); | ||
| // pool_registration (third element) | ||
| let pools: VMap<Hash<28>, PoolParams> = decoder | ||
| .decode() | ||
| .context("Failed to parse snapshot_pool_registration")?; | ||
| let registration = VMap( | ||
| pools | ||
| .0 | ||
| .into_iter() | ||
| .map(|(pool_id, params)| { | ||
| // Convert RewardAccount (Vec<u8>) to StakeAddress | ||
| let reward_account = | ||
| StakeAddress::from_binary(¶ms.reward_account.0) | ||
| .unwrap_or_else(|_| StakeAddress::default()); | ||
|
|
||
| // Convert Set<AddrKeyhash> to Vec<StakeAddress> | ||
| let pool_owners: Vec<StakeAddress> = params | ||
| .owners | ||
| .0 | ||
| .into_iter() | ||
| .map(|keyhash| { | ||
| StakeAddress::new( | ||
| StakeCredential::AddrKeyHash(keyhash), | ||
| NetworkId::Mainnet, | ||
| ) | ||
| }) | ||
| .collect(); | ||
|
|
||
| // Convert Vec<streaming_snapshot::Relay> to Vec<types::Relay> | ||
| let relays: Vec<types::Relay> = params | ||
| .relays | ||
| .into_iter() | ||
| .map(|relay| match relay { | ||
| streaming_snapshot::Relay::SingleHostAddr( | ||
| port, | ||
| ipv4, | ||
| ipv6, | ||
| ) => { | ||
| let port_opt = match port { | ||
| streaming_snapshot::Nullable::Some(p) => { | ||
| Some(p as u16) | ||
| } | ||
| _ => None, | ||
| }; | ||
| let ipv4_opt = match ipv4 { | ||
| streaming_snapshot::Nullable::Some(ip) | ||
| if ip.0.len() == 4 => | ||
| { | ||
| Some(std::net::Ipv4Addr::new( | ||
| ip.0[0], ip.0[1], ip.0[2], ip.0[3], | ||
| )) | ||
| } | ||
| _ => None, | ||
| }; | ||
| let ipv6_opt = match ipv6 { | ||
| streaming_snapshot::Nullable::Some(ip) | ||
| if ip.0.len() == 16 => | ||
| { | ||
| let b = &ip.0; | ||
| Some(std::net::Ipv6Addr::from([ | ||
| b[0], b[1], b[2], b[3], b[4], b[5], | ||
| b[6], b[7], b[8], b[9], b[10], b[11], | ||
| b[12], b[13], b[14], b[15], | ||
| ])) | ||
| } | ||
| _ => None, | ||
| }; | ||
| types::Relay::SingleHostAddr( | ||
| types::SingleHostAddr { | ||
| port: port_opt, | ||
| ipv4: ipv4_opt, | ||
| ipv6: ipv6_opt, | ||
| }, | ||
| ) | ||
| } | ||
| streaming_snapshot::Relay::SingleHostName( | ||
| port, | ||
| hostname, | ||
| ) => { | ||
| let port_opt = match port { | ||
| streaming_snapshot::Nullable::Some(p) => { | ||
| Some(p as u16) | ||
| } | ||
| _ => None, | ||
| }; | ||
| types::Relay::SingleHostName( | ||
| types::SingleHostName { | ||
| port: port_opt, | ||
| dns_name: hostname, | ||
| }, | ||
| ) | ||
| } | ||
| streaming_snapshot::Relay::MultiHostName(hostname) => { | ||
| types::Relay::MultiHostName(types::MultiHostName { | ||
| dns_name: hostname, | ||
| }) | ||
| } | ||
| }) | ||
| .collect(); | ||
|
|
||
| // Convert Nullable<PoolMetadata> to Option<PoolMetadata> | ||
| let pool_metadata = match params.metadata { | ||
| streaming_snapshot::Nullable::Some(meta) => { | ||
| Some(types::PoolMetadata { | ||
| url: meta.url, | ||
| hash: meta.hash.to_vec(), | ||
| }) | ||
| } | ||
| _ => None, | ||
| }; | ||
|
|
||
| ( | ||
| pool_id, | ||
| PoolRegistration { | ||
| operator: params.id, | ||
| vrf_key_hash: params.vrf, | ||
| pledge: params.pledge, | ||
| cost: params.cost, | ||
| margin: Ratio { | ||
| numerator: params.margin.numerator, | ||
| denominator: params.margin.denominator, | ||
| }, | ||
| reward_account, | ||
| pool_owners, | ||
| relays, | ||
| pool_metadata, | ||
| }, | ||
| ) | ||
| }) | ||
| .collect(), | ||
| ); | ||
| // We don't know how many elements are in this array, so just skip the first element | ||
| // and let the array parsing naturally complete | ||
| decoder.skip().context("Failed to skip first element")?; | ||
|
|
||
| // Try to skip remaining elements, but don't fail if there aren't exactly 3 | ||
| loop { | ||
| match decoder.datatype() { | ||
| Ok(minicbor::data::Type::Break) => { | ||
| // End of indefinite array | ||
| break; | ||
| } | ||
| Ok(_) => { | ||
| // More elements to skip | ||
| decoder.skip().ok(); // Don't fail on individual skips | ||
| } | ||
| Err(_) => { | ||
| // End of definite array or other error - break | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| info!(" {snapshot_name} snapshot - parse completed successfully."); | ||
|
|
||
| Ok(Snapshot { | ||
| snapshot_stake: VMap(Vec::new()), | ||
| snapshot_stake, | ||
| snapshot_delegations: delegations, | ||
| snapshot_pool_params: registration, | ||
| }) |
There was a problem hiding this comment.
The new parsing logic for delegations and pool parameters lacks test coverage. The mark_set_go.rs file has no test module, while other snapshot-related files like streaming_snapshot.rs and parser.rs include tests.
Consider adding unit tests to verify:
- Correct parsing of delegations from CBOR
- Correct conversion of
PoolParamstoPoolRegistration - Proper handling of different relay types (SingleHostAddr, SingleHostName, MultiHostName)
- Network ID extraction from reward accounts
- Error cases (invalid reward account data, missing fields, etc.)
| StakeAddress::new( | ||
| StakeCredential::AddrKeyHash(keyhash), | ||
| NetworkId::Mainnet, | ||
| ) |
There was a problem hiding this comment.
The hardcoded NetworkId::Mainnet may cause issues when processing testnet snapshots. The network ID should be extracted from the reward account binary data (first byte indicates network) or passed as a parameter to the parsing function.
Consider extracting the network from params.reward_account.0[0] & 0x01 (where 0b1 = Mainnet, 0b0 = Testnet) to ensure correct network assignment.
| let reward_account = | ||
| StakeAddress::from_binary(¶ms.reward_account.0) | ||
| .unwrap_or_else(|_| StakeAddress::default()); |
There was a problem hiding this comment.
Using unwrap_or_else(|_| StakeAddress::default()) silently ignores errors when parsing reward accounts. If the binary data is invalid, this creates a default Mainnet address which may not represent the actual pool's reward account.
Consider logging the error or returning it rather than using a default value, or at minimum add a warning log when falling back to default.
There was a problem hiding this comment.
Unless the snapshot is corrupted, I wouldn't be too concerned about this from_binary failing, but if it did fail for some reason...we would definitely want to know about it.
buddhisthead
changed the title
lowhung
left a comment
lowhung
left a comment
There was a problem hiding this comment.
Nice work @buddhisthead! Main concern is the AddrKeyHash parse.
| if !snapshots.mark.0.is_empty() { | ||
| eprintln!(" Sample stakes (first 5):"); | ||
| for (i, (cred, amount)) in snapshots.mark.0.iter().take(5).enumerate() { | ||
| let cred_str = match cred { |
There was a problem hiding this comment.
Nit: I know this is just for testing, but the StakeCredential type has a to_string() implementation that you could use instead of doing this matching on the type of credential in a few places.
impl StakeCredential {
pub fn to_string(&self) -> Result<String> {
let (hrp, data) = match &self {
Self::AddrKeyHash(data) => (Hrp::parse("stake_vkh")?, data.as_slice()),
Self::ScriptHash(data) => (Hrp::parse("script")?, data.as_slice()),
};
Ok(bech32::encode::<Bech32>(hrp, data)?)
}
}I think the whole bech-32 encoding would be more useful for debugging than using 8 hex characters in this case.
| let reward_account = | ||
| StakeAddress::from_binary(¶ms.reward_account.0) | ||
| .unwrap_or_else(|_| StakeAddress::default()); |
There was a problem hiding this comment.
Unless the snapshot is corrupted, I wouldn't be too concerned about this from_binary failing, but if it did fail for some reason...we would definitely want to know about it.
| .into_iter() | ||
| .map(|keyhash| { | ||
| StakeAddress::new( | ||
| StakeCredential::AddrKeyHash(keyhash), |
There was a problem hiding this comment.
This is a situation where as much it's most common for pool owners to use AddrKeyHash credentials, the protocol does allow ScriptHash credentials for pool owners. So we'd need to account for both here. For example, you could have a pool owned by a multi-sig / script such that a pool is jointly owned by multiple parties (where a 2-of-3 signature is required to update parameters or withdraw rewards).
So I don't think we can hard-code this to being a AddrKeyHash. I'd love for @sandtreader to chime in here if he has time.
Description
Implement snapshot parser for snapshot delegation and pool registration
Related Issue(s)
Resolves #450
How was this tested?
Tested manually with the example/test_streaming_parser executable.
From the top-level, run
make snap-test-streamingand confirm there are unique values for the Mark Set and Go snapshot output.Checklist
Impact / Side effects
Describe any potential side effects, e.g. performance, compatibility, or security concerns.
If the PR introduces a breaking change, explain migration steps for users.
Reviewer notes / Areas to focus
If you want specific feedback, list files/functions to review or aspects you are unsure about.