cardano-rpc: Refactoring in preparation for FetchBlock transaction bodies#1253
Merged
Merged
Conversation
28cd690 to
2daffe7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs preparatory refactoring in cardano-rpc ahead of adding full transaction bodies to FetchBlock responses, while also tightening proto conversion and test coverage (notably for rational conversions) and returning a parsed BlockInMode from fetchBlock for downstream inspection.
Changes:
- Fix
Rational↔RationalNumberconversion behavior by approximating out-of-range values and add new property tests to pin the semantics. - Refactor UTxO RPC conversion code by splitting
UtxoRpc.Typeinto focusedType.*modules and standardizing onProto-wrapped messages. - Change
fetchBlockto return(rawBytes, BlockInMode)and update the Sync handler and related tests accordingly.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cardano-rpc/test/cardano-rpc-test/Test/Cardano/Rpc/Type.hs | Adds property tests for in-range exact and out-of-range approximate rational conversions. |
| cardano-rpc/test/cardano-rpc-test/Test/Cardano/Rpc/TxOutput.hs | Updates TxOutput roundtrip test to pass an explicit ShelleyBasedEra witness. |
| cardano-rpc/test/cardano-rpc-test/Test/Cardano/Rpc/Predicate.hs | Updates predicate tests to use Proto-wrapped message types. |
| cardano-rpc/test/cardano-rpc-test/Test/Cardano/Rpc/Eval.hs | Aligns eval tests with Proto-wrapped redeemer purpose representation. |
| cardano-rpc/src/Cardano/Rpc/Server/NodeKernelAccess.hs | Changes fetchBlock to return BlockInMode alongside raw bytes. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/TxOutput.hs | Moves TxOutput/UTxO conversion logic into a dedicated module; generalizes over ShelleyBasedEra. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/TxEval.hs | Moves Tx evaluation/redeemer mapping logic into a dedicated module using Proto wrappers. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/Script.hs | Moves script conversion logic into a dedicated module using Proto wrappers. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/ProtocolParameters.hs | Moves protocol-params conversion logic into a dedicated module. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/PlutusData.hs | Moves Plutus data conversion logic into a dedicated module. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/ChainPoint.hs | Moves ChainPoint conversion logic into a dedicated module. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type/BigInt.hs | Moves BigInt decoding helper into a dedicated module. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Type.hs | Becomes an umbrella re-export module for Type.* submodules; exports txoRefUtxoRpcToTxIn. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Sync.hs | Updates FetchBlock handler to use the parsed block to compute height. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Query.hs | Removes getProto shim usage for predicates (now stays Proto-wrapped). |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Predicate.hs | Switches predicate APIs to Proto-wrapped types and adds exactAddressPredicate. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/UtxoRpc/Eval.hs | Removes getProto shim for updating Proto.errors. |
| cardano-rpc/src/Cardano/Rpc/Server/Internal/Orphans.hs | Implements out-of-range rational approximation for Rational -> Proto RationalNumber. |
| cardano-rpc/src/Cardano/Rpc/Proto/Api/UtxoRpc/Sync.hs | Re-exports Cardano field lenses module (with some names hidden). |
| cardano-rpc/cardano-rpc.cabal | Adds new Type.* modules to the library stanza. |
| cardano-api/src/Cardano/Api/Block.hs | Removes the deprecated exported pattern Block synonym. |
| .changes/20260709_cardano_rpc_fetchblock_refactor.yml | Adds changelog fragment for the refactor/bugfix work. |
Comment on lines
34
to
35
| instance Inject (Proto U5c.RationalNumber) Rational where | ||
| inject r = r ^. U5c.numerator . to fromIntegral % r ^. U5c.denominator . to fromIntegral |
Comment on lines
+252
to
+254
| , (?! "Invalid value in drepVotingThresholds: ppNetworkGroup") | ||
| , (?! "Invalid value in drepVotingThresholds: ppEcomonicGroup") | ||
| , (?! "Invalid value in drepVotingThresholds: ppTechnicalGroup") |
Comment on lines
+1
to
+7
| project: cardano-rpc | ||
| pr: 1253 | ||
| kind: | ||
| - bugfix | ||
| - refactoring | ||
| description: | | ||
| Approximate out-of-range rationals in RationalNumber conversions instead of wrapping. Preparatory refactoring for FetchBlock transaction bodies: split UtxoRpc.Type into Type.* modules, use Proto-wrapped messages in conversion functions, generalise txOutToUtxoRpcTxOutput over eras, expose request helpers, and return the parsed block from fetchBlock. |
Comment on lines
+42
to
+45
| -- their fields, the conversion is exact. Otherwise the result is the best | ||
| -- representable approximation of the value: values beyond the numerator | ||
| -- bounds are clamped to them, and other values are approximated using | ||
| -- continued fractions, keeping both components within their field bounds. |
Jimbo4350
approved these changes
Jul 9, 2026
534dce1 to
15431b7
Compare
2daffe7 to
ecc1f35
Compare
15431b7 to
f4c0241
Compare
Base automatically changed from
mgalazyn/feature/rpc-fetchblock-add-timestamp
to
master
July 11, 2026 00:08
ecc1f35 to
a5413ed
Compare
The Rational to RationalNumber conversion used fromIntegral into the int32 numerator and uint32 denominator, which wraps modulo 2^32: a pool margin of 1 % 2^33 emitted denominator 0. The conversion now returns the best representable approximation via a continued-fraction walk when either component exceeds its field, and converts exactly otherwise. The comment claiming the conversion clips was wrong and is gone.
Handler boundary types, the Inject orphans and nested fields of Proto messages all expect Proto values, so wrapping throughout eliminates the getProto and Proto shims at every seam. The predicate matchers now take Proto-wrapped patterns and scriptWitnessIndexToRedeemerPurpose returns a Proto-wrapped enum, since grapesy rewraps enum fields too.
…pers txOutToUtxoRpcTxOutput now takes an explicit ShelleyBasedEra witness instead of IsEra, so it can convert outputs of blocks from any Shelley-based era rather than only the current one. Export txoRefUtxoRpcToTxIn and add exactAddressPredicate so RPC clients and tests build requests from library code instead of duplicating it.
fetchBlock now yields the block in era context via fromConsensusBlock instead of just the block number, so handlers can inspect the block content. The Sync proto wrapper re-exports the whole generated Cardano_Fields module (hiding the four lenses that clash with Sync_Fields), giving FetchBlock consumers access to all cardano message field lenses.
Move the conversions between cardano-api types and their UTxO RPC protobuf counterparts from the monolithic UtxoRpc.Type module into dedicated submodules: BigInt, ChainPoint, PlutusData, ProtocolParameters, Script, TxEval and TxOutput. UtxoRpc.Type becomes an umbrella module which re-exports the same names it exported before, so the import surface is unchanged and existing importers keep compiling without modification.
a5413ed to
9396d3b
Compare
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
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.
Context
Preparatory work for #1247, which populates full transaction bodies in
FetchBlockresponses.Splitting these commits out keeps the implementation PR reviewable as a single change on top.
This PR is stacked on #1242 and #1252, so their two commits appear at the base of the branch and will drop out on rebase once those merge.
The five commits to review:
The
RationaltoRationalNumberconversion usedfromIntegralinto the int32 numerator and uint32 denominator, which wraps modulo 2^32: a pool margin of1 % 2^33emitted denominator 0.The conversion now returns the best representable approximation via a continued-fraction walk when either component exceeds its field, and converts exactly otherwise.
Conversion functions take and return
Proto-wrapped messages throughout, eliminating thegetProto/Protoshims at handler boundaries.txOutToUtxoRpcTxOutputand expose request helpers.It now takes an explicit
ShelleyBasedErawitness instead ofIsEra, so it can convert outputs of blocks from any Shelley-based era rather than only the current one.txoRefUtxoRpcToTxInis exported andexactAddressPredicateadded, so RPC clients and tests build requests from library code instead of duplicating it.fetchBlock.fetchBlockyields the block in era context viafromConsensusBlockalongside the raw bytes, so handlers can inspect block content.The Sync proto wrapper re-exports the generated
Cardano_Fieldsmodule, givingFetchBlockconsumers access to all cardano message field lenses.UtxoRpc.TypeintoType.*modules.The former ~1000-line module becomes seven focused submodules (
BigInt,ChainPoint,PlutusData,ProtocolParameters,Script,TxEval,TxOutput) withTypeas a re-export umbrella.How to trust this PR
Cardano.Rpc.Server.Internal.UtxoRpc.Typeis unchanged, so importers are unaffected.Checklist
.changes/