fix(configs): ensure shelley devnet config parses correctly#710
Conversation
WalkthroughThis PR adds documentation to the pallas-configs README with usage examples, extends the Pool struct in shelley.rs with two new optional fields (owners and registration_deposit) using serde defaults, and introduces comprehensive partner genesis test data in JSON format. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
pallas-configs/src/shelley.rs (1)
274-319: Test coverage missing for the newregistration_depositfield.The test thoroughly validates most Pool fields including the defaulted
ownersfield, but doesn't verify thatregistration_depositcorrectly defaults toNonewhen absent from the JSON.Add an assertion after line 308 to verify the new field:
); assert_eq!(pool.margin.numerator, 0); + assert_eq!(pool.registration_deposit, None); let stake = staking
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pallas-configs/README.md(1 hunks)pallas-configs/src/shelley.rs(2 hunks)test_data/partner-shelley-genesis.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pallas-configs/src/shelley.rs (1)
pallas-configs/src/alonzo.rs (1)
load_test_data_config(248-255)
🪛 Gitleaks (8.29.0)
test_data/partner-shelley-genesis.json
[high] 67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 99-99: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Check (ubuntu-latest, stable)
- GitHub Check: Check (macOS-latest, stable)
- GitHub Check: Check (windows-latest, stable)
- GitHub Check: Test Suite (ubuntu-latest, stable)
- GitHub Check: Test Suite (windows-latest, stable)
- GitHub Check: Check (windows-latest, stable)
- GitHub Check: Test Suite (ubuntu-latest, stable)
- GitHub Check: Test Suite (windows-latest, stable)
🔇 Additional comments (3)
test_data/partner-shelley-genesis.json (1)
67-67: Static analysis false positive: These are legitimate test data.The static analysis tool flagged the
keyHashvalues as potential API keys. These are actually Cardano credential key hashes (public identifiers) within the staking pool reward account structures, which is the expected format for Shelley genesis configurations. Since this is test data for a Testnet environment, there's no security concern.Also applies to: 83-83, 99-99
pallas-configs/README.md (1)
3-19: LGTM!The documentation clearly explains the crate's purpose and provides a practical example demonstrating how to load and access staking pool data from Shelley genesis files.
pallas-configs/src/shelley.rs (1)
167-168: LGTM! Default handling correctly implemented.The
#[serde(default)]attributes are properly applied:
owners: Vec<String>will default to an empty vector if omittedregistration_deposit: Option<u64>will default toNoneif omittedThis allows the deserializer to handle partner genesis files that may not include these fields, maintaining backward compatibility.
Also applies to: 174-175
| #[serde(default)] | ||
| pub owners: Vec<String>, | ||
| pub pledge: u64, | ||
| pub public_key: String, // pool ID | ||
| pub relays: Vec<HashMap<String, Relay>>, | ||
| pub reward_account: RewardAccount, | ||
| pub vrf: String, | ||
| #[serde(default)] | ||
| pub registration_deposit: Option<u64>, |
There was a problem hiding this comment.
The PR description doesn't match the actual changes.
The PR title states "Remove unnecessary serde aliases" and the description says "Removes unused serde alias attributes," but the actual changes show:
- Adding
#[serde(default)]to the existingownersfield (line 167) - Adding a new
registration_depositfield with#[serde(default)](lines 174-175)
These are additions, not removals. Additionally, #[serde(default)] is not a serde "alias" attribute—it provides default values for missing fields during deserialization. Please update the PR title and description to accurately reflect that this PR adds default handling for optional Pool fields to support partner genesis configurations.
Summary by CodeRabbit
Documentation
New Features
Tests