feat(utxorpc): add v1beta spec support alongside v1alpha#745
Merged
Conversation
Bumps utxorpc-spec to 0.19 and exposes both `pallas_utxorpc::v1alpha` and `pallas_utxorpc::v1beta` mappers. The crate root keeps re-exporting v1alpha items (Mapper, spec) under the default `v1alpha` feature, so existing call sites compile unchanged. v1beta is gated behind an opt-in `v1beta` feature; both features are additive. Shared mapping logic (~80% of the surface — all certificates, pparams, plutus data, witnesses, scripts, blocks) lives in a single `impl_cardano_mapper_shared!` macro instantiated once per version. Methods that diverge between specs (map_native_script, map_tx_datum, map_tx_output, map_asset, map_policy_assets, map_conway_gov_action, map_tx) are defined per-version. v1beta-only mappers (BootstrapWitness, Vote, VotingProcedure, VoterVotes) live next to the rest of the v1beta module. Adds cross-version wire-compatibility tests (PParams, Certificate, PoolRegistrationCert, Metadata, PlutusData) that act as a tripwire if upstream diverges any shared message in a future release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughConsolidates Cardano-to-UTxORPC mapping into a shared macro and introduces explicit, versioned mappers ChangesMapper Architecture Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Review rate limit: 0/1 reviews remaining, refill in 54 minutes and 43 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pallas-utxorpc/src/v1alpha/mod.rs (1)
231-234: 💤 Low valueClarify the placeholder value for
InfoAction.The comment explains this is a placeholder, but the magic number
6is unclear. Consider adding a brief note about why this specific value was chosen or if it aligns with the v1alpha protobuf definition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pallas-utxorpc/src/v1alpha/mod.rs` around lines 231 - 234, The mapping for conway::GovAction::Information currently uses a magic number (6) for u5c::governance_action::GovernanceAction::InfoAction; replace the literal with a named constant (e.g., INFO_ACTION_V1ALPHA or INFO_ACTION_PLACEHOLDER) and add a brief comment stating that this constant corresponds to the v1alpha protobuf enum value (or is a temporary placeholder until the protobuf mapping is updated), or, if available, map it to the generated protobuf enum variant instead of a raw integer to make the intent explicit.pallas-utxorpc/src/shared.rs (1)
10-17: 💤 Low valuePotential numeric truncation in
rational_number_to_u5c.The cast from
i64numerator toi32andu64denominator tou32may silently truncate values that exceed the target type's range. While Cardano protocol parameters typically use small rationals, edge cases could produce incorrect results.Consider defensive bounds checking
fn rational_number_to_u5c( value: pallas_primitives::RationalNumber, ) -> u5c::RationalNumber { + debug_assert!( + value.numerator >= i32::MIN as i64 && value.numerator <= i32::MAX as i64, + "RationalNumber numerator overflow" + ); + debug_assert!( + value.denominator <= u32::MAX as u64, + "RationalNumber denominator overflow" + ); u5c::RationalNumber { numerator: value.numerator as i32, denominator: value.denominator as u32, } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pallas-utxorpc/src/shared.rs` around lines 10 - 17, The cast in rational_number_to_u5c (converting pallas_primitives::RationalNumber.numerator: i64 -> i32 and denominator: u64 -> u32) can silently truncate; add defensive bounds checking and fail loudly instead of truncating: validate numerator fits i32::MIN..=i32::MAX and denominator fits 1..=u32::MAX (and non-zero if needed) before constructing u5c::RationalNumber, and change rational_number_to_u5c to return Result<u5c::RationalNumber, YourErrorType> (or propagate an existing error type) so callers can handle out-of-range values; use i32::try_from(value.numerator) and u32::try_from(value.denominator) or explicit range checks to implement this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pallas-utxorpc/src/v1alpha/mod.rs`:
- Around line 53-56: The match arm handling babbage::NativeScript::ScriptAny is
incorrectly constructing u5c::native_script::NativeScript::ScriptAll; update
that arm to construct u5c::native_script::NativeScript::ScriptAny instead,
preserving the same NativeScriptList/items mapping via Self::map_native_script
so semantics remain "any script must pass" (locate the code in the match for
babbage::NativeScript::ScriptAny within the mapping function
Self::map_native_script).
In `@pallas-utxorpc/src/v1beta/mod.rs`:
- Around line 53-56: The mapping for the babbage::NativeScript::ScriptAny branch
is incorrect: it currently constructs
u5c::native_script::NativeScript::ScriptAll. Change that branch to construct
u5c::native_script::NativeScript::ScriptAny instead, using the same
u5c::native_script::NativeScriptList with items: x.iter().map(|x|
Self::map_native_script(x)).collect() so the variant names align (update the
match arm that handles babbage::NativeScript::ScriptAny and ensure
Self::map_native_script is used for each item).
---
Nitpick comments:
In `@pallas-utxorpc/src/shared.rs`:
- Around line 10-17: The cast in rational_number_to_u5c (converting
pallas_primitives::RationalNumber.numerator: i64 -> i32 and denominator: u64 ->
u32) can silently truncate; add defensive bounds checking and fail loudly
instead of truncating: validate numerator fits i32::MIN..=i32::MAX and
denominator fits 1..=u32::MAX (and non-zero if needed) before constructing
u5c::RationalNumber, and change rational_number_to_u5c to return
Result<u5c::RationalNumber, YourErrorType> (or propagate an existing error type)
so callers can handle out-of-range values; use i32::try_from(value.numerator)
and u32::try_from(value.denominator) or explicit range checks to implement this.
In `@pallas-utxorpc/src/v1alpha/mod.rs`:
- Around line 231-234: The mapping for conway::GovAction::Information currently
uses a magic number (6) for
u5c::governance_action::GovernanceAction::InfoAction; replace the literal with a
named constant (e.g., INFO_ACTION_V1ALPHA or INFO_ACTION_PLACEHOLDER) and add a
brief comment stating that this constant corresponds to the v1alpha protobuf
enum value (or is a temporary placeholder until the protobuf mapping is
updated), or, if available, map it to the generated protobuf enum variant
instead of a raw integer to make the intent explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96a8bba1-795c-4735-8dec-1d314bff3dde
📒 Files selected for processing (10)
pallas-utxorpc/Cargo.tomlpallas-utxorpc/README.mdpallas-utxorpc/src/certs.rspallas-utxorpc/src/lib.rspallas-utxorpc/src/params.rspallas-utxorpc/src/shared.rspallas-utxorpc/src/v1alpha/mod.rspallas-utxorpc/src/v1beta/mod.rstest_data/u5c_v1alpha.jsontest_data/u5c_v1beta.json
💤 Files with no reviewable changes (2)
- pallas-utxorpc/src/params.rs
- pallas-utxorpc/src/certs.rs
Replaces v1alpha's "uncomment this fs::write block" pattern and v1beta's ad-hoc REGEN_V1BETA_SNAPSHOT var with a single REGENERATE_SNAPSHOTS=1 toggle that overwrites both JSON snapshots in place. README documents it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
map_native_script was constructing u5c NativeScript::ScriptAll for the
babbage::NativeScript::ScriptAny arm, inverting the semantics ("any"
becomes "all"). The bug was pre-existing in the original lib.rs and got
copied into both v1alpha and v1beta during the dual-spec split.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rate Mirrors pallas-utxorpc's spec-version features at the wrapper level so downstream users depending on `pallas` can opt in to v1beta without direct-deping `pallas-utxorpc`. Default keeps v1alpha enabled, matching prior behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Always compile both v1alpha and v1beta into pallas-utxorpc, and require
callers to pick a version explicitly. The crate root no longer re-exports
either Mapper or spec — `pallas_utxorpc::Mapper` becomes
`pallas_utxorpc::v1alpha::Mapper` (or v1beta::Mapper).
This is a breaking change, taken deliberately: having v1alpha implicitly
be "the" Mapper at the crate root was tech-debt that would have grown
worse as v1beta matures and v1alpha eventually deprecates. Surfacing the
choice at the use-site makes the version explicit and removes the
default-feature ambiguity.
Side effects:
* pallas-utxorpc has no Cargo features anymore.
* pallas/Cargo.toml drops the hoisted u5c-v1alpha / u5c-v1beta
features for the same reason.
* Cross-version wire-compat tests are unconditional now.
* Shared infrastructure (LedgerContext, type aliases) stays at the
crate root — they are version-agnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pallas-utxorpc/src/v1alpha/mod.rs (1)
229-231: 💤 Low valuePlaceholder value for InfoAction.
The magic number
6is used as a placeholder for the v1alphaInfoActionencoding. While this works, a named constant would improve clarity.♻️ Optional: Add a named constant
+// v1alpha encodes Information as a uint32; 6 is an arbitrary placeholder. +const INFO_ACTION_TAG: u32 = 6; + impl<C: LedgerContext> Mapper<C> { // ... conway::GovAction::Information => { - // The 6 is just a placeholder; v1alpha encodes Information as a uint32. - u5c::governance_action::GovernanceAction::InfoAction(6) + u5c::governance_action::GovernanceAction::InfoAction(INFO_ACTION_TAG) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pallas-utxorpc/src/v1alpha/mod.rs` around lines 229 - 231, Replace the magic literal 6 used when mapping conway::GovAction::Information to u5c::governance_action::GovernanceAction::InfoAction with a named constant (e.g., INFO_ACTION_CODE) to make the intent clear; declare the constant with the same numeric type expected by InfoAction (uint32/u32) near the top of the module or alongside other constants, then use INFO_ACTION_CODE in the match arm instead of the literal 6 to improve readability and maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pallas-utxorpc/src/v1alpha/mod.rs`:
- Around line 229-231: Replace the magic literal 6 used when mapping
conway::GovAction::Information to
u5c::governance_action::GovernanceAction::InfoAction with a named constant
(e.g., INFO_ACTION_CODE) to make the intent clear; declare the constant with the
same numeric type expected by InfoAction (uint32/u32) near the top of the module
or alongside other constants, then use INFO_ACTION_CODE in the match arm instead
of the literal 6 to improve readability and maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2588fc0-05f1-47b4-9955-5288b7c954bc
📒 Files selected for processing (6)
pallas-utxorpc/Cargo.tomlpallas-utxorpc/README.mdpallas-utxorpc/src/lib.rspallas-utxorpc/src/shared.rspallas-utxorpc/src/v1alpha/mod.rspallas-utxorpc/src/v1beta/mod.rs
✅ Files skipped from review due to trivial changes (2)
- pallas-utxorpc/Cargo.toml
- pallas-utxorpc/src/shared.rs
Walks back the breaking change from the previous commit. The crate root re-export of `Mapper` and `spec` returns, but now lives behind a new `u5c-v1alpha-compat` Cargo feature that is on by default. Default consumers see no change — `pallas_utxorpc::Mapper` keeps resolving to the v1alpha mapper. Consumers who want the explicit-version future can opt out with `default-features = false` and use `pallas_utxorpc::v1alpha::Mapper` / `v1beta::Mapper` directly. Both `pub mod v1alpha;` and `pub mod v1beta;` remain unconditional; the feature only gates the root re-export. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
utxorpc-specto0.19and exposes bothpallas_utxorpc::v1alphaandpallas_utxorpc::v1betamappers.pallas_utxorpc::Mapper/pallas::interop::utxorpc::Mapperkeeps working unchanged: under the defaultv1alphafeature the v1alpha items (Mapper,spec) are re-exported at the crate root.v1betaCargo feature (additive, opt-in) for users who want the new spec — including v1beta-only types (BootstrapWitness,Vote,VotingProcedure,VoterVotes).Implementation
Most of the mapping logic is wire-identical between the two specs (all certificates, pparams, plutus data, witnesses, scripts, blocks). It lives in a single
impl_cardano_mapper_shared!macro insrc/shared.rs, instantiated once per version.Methods that genuinely diverge live per-version in
v1alpha/mod.rsandv1beta/mod.rs:map_native_script— variant renameScriptPubkey→ScriptPubkeyHashmap_tx_datum—original_cborbecame optionalmap_tx_output— new optionaloriginal_cborfield in v1betamap_asset—Quantityoneof in v1alpha, scalarOption<BigInt>in v1betamap_policy_assets— per-multiassetredeemerremoved in v1beta (lifted toWitnessSet.redeemers)map_conway_gov_action—InfoActionshape changemap_tx—WitnessSetandTxgained fields in v1beta (redeemers,bootstrap_witnesses,votes)v1beta-only mappers (
map_bootstrap_witness,map_vote,map_voting_procedure,map_voter,map_votes) live alongside the rest of the v1beta module rather than in a separate file.Test plan
Notes
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests