Repository navigation
fix(specs): parse requested block on Cardano /blocks/{hash_or_number} routes - #2335
Merged
Merged
Conversation
… routes
The six Blockfrost `/blocks/{hash_or_number}` routes in the CARDANO spec
(inherited by CARDANOT) declared `DEFAULT latest` block parsing, so a
request such as `GET /blocks/1` was classified as a latest-block request.
The archive rule (8192 blocks) never fired for historical blocks, archive
providers were not selected, and the archive CU multiplier was skipped.
Switch those routes to `PARSE_DICTIONARY_OR_ORDERED` on the
`hash_or_number` path parameter, following the Aptos and Cosmos SDK
pattern, with `default_value: latest` so a block hash keeps today's
behaviour, and add a `BLOCK_HASH` generic parser so hash requests are
surfaced as requested hashes and can be resolved to heights by the
consumer cache.
Also add the Blockfrost endpoints missing from the spec:
`/assets/{asset}/utxos`, `/scripts/{script_hash}/utxos`,
`/governance/committee`, `/governance/committee/votes` and
`/governance/committee/{cc_id}/votes`.
Add chainlib tests that load the real CARDANO spec and pin both the
requested-block parsing and the archive extension activation.
Claude-Session: https://claude.ai/code/session_015Zn1TrLFU2xyth1vqGmz7Q
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
nimrod-teich
approved these changes
Sep 9, 2026
PR Summary by QodoFix Cardano block parsing and complete Blockfrost routes
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Broken Cardano routes can pass tests
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+446
to
+449
| { | ||
| "name": "/assets/{asset}/utxos", | ||
| "block_parsing": { | ||
| "parser_arg": [ |
There was a problem hiding this comment.
1. Broken cardano routes can pass tests 📘 Rule violation ☼ Reliability
The five new Cardano route definitions are never exercised by rest_test.go, whose added cases only send block, epoch, and slot paths. Invalid route names, compute units, or parser metadata on the asset, script, and governance endpoints therefore reach the specifications without a targeted parsing regression failing.
Agent Prompt
## Issue description
The PR adds five Cardano routes without tests that parse requests against those route definitions and verify their metadata.
## Fix Focus Areas
- protocol/chainlib/rest_test.go[349-367]
- specs/mainnet-1/specs/cardano.json[446-463]
- specs/mainnet-1/specs/cardano.json[1022-1075]
- specs/mainnet-1/specs/cardano.json[1742-1759]
## Recommended Fix
Add table-driven chainlib cases for every new asset, script, and governance route. Parse representative paths from the real Cardano spec and assert successful matching, requested-block behavior, and expected compute units.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+540
to
543
| "hash_or_number", | ||
| "=", | ||
| "0" | ||
| ], |
There was a problem hiding this comment.
2. Cardano behavior has no test narrative 📘 Rule violation ⚙ Maintainability
The changed hash_or_number parsing and archive-selection scenarios are covered in Go tests but have no corresponding specs/*_test.md document. Readers evaluating numeric heights, hashes, and the archive boundary therefore cannot recover the expected behavior from the required human-readable specification.
Agent Prompt
## Issue description
The Cardano requested-block and archive behavior changed without the required GIVEN–WHEN–THEN test specification under `specs/`.
## Fix Focus Areas
- specs/mainnet-1/specs/cardano.json[540-545]
- protocol/chainlib/rest_test.go[324-332]
- protocol/chainlib/rest_test.go[384-386]
## Recommended Fix
Create or update the relevant `specs/*_test.md` document with GIVEN–WHEN–THEN narratives for numeric block heights, block hashes, latest blocks, excluded epoch and slot parameters, and archive-extension activation at the configured boundary.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+540
to
+542
| "hash_or_number", | ||
| "=", | ||
| "0" |
There was a problem hiding this comment.
3. Genesis block requests use latest 🐞 Bug ≡ Correctness
PARSE_DICTIONARY_OR_ORDERED passes the extracted string "0" to appendInterfaceToInterfaceArrayWithError, which treats it as an absent value and substitutes the configured latest default. Requests for /blocks/0 and its five subroutes therefore bypass historical-block tracking and archive selection in both Cardano specs.
Agent Prompt
## Issue description
Cardano requests whose `hash_or_number` path parameter is `0` are parsed as `latest` because the shared dictionary parser treats the string `"0"` as an unset value. This affects all six changed block routes in both Cardano specs.
## Fix Focus Areas
- specs/mainnet-1/specs/cardano.json[539-545]
- specs/testnet-2/specs/cardano.json[539-545]
- protocol/parser/parser.go[832-838]
- protocol/parser/parser.go[902-907]
- protocol/chainlib/rest_test.go[349-367]
## Recommended Fix
Ensure a present dictionary value of `"0"` is returned as a valid block value rather than converted to `ValueNotSetError`, without changing genuinely absent or empty-parameter handling. Add Cardano spec tests for `/blocks/0` and representative `/blocks/0/...` routes, asserting requested block zero and archive activation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 task done
1 task done
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.
Description
Closes: N/A (reported by Stake Village during their audit of the Cardano spec, tracked on the Lava product board as "Cardano Specs Update"; needed for the Sep 17 CARDANO / CARDANOT incentivization)
The bug
The six Blockfrost
/blocks/{hash_or_number}routes in theCARDANOspec (and therefore inCARDANOT, which imports it) declared:so
GET /blocks/1was classified as a request for the latest block. The URL sent to the provider was untouched, but on the Lava side:"block": 8192) never fired, so archive providers were not guaranteed for historical blocks and a non-archive provider could return an error even when archive providers were available;cu_multiplier: 5were not applied;latestinstead of the real height.The fix
specs/mainnet-1/specs/cardano.jsonandspecs/testnet-2/specs/cardano.json(kept identical, as before):Block parsing on the six
{hash_or_number}routes —/blocks/{hash_or_number},/next,/previous,/txs,/txs/cbor,/addresses:PARSE_DICTIONARY_OR_ORDEREDon the path parameter is the existing Aptos (/blocks/by_height/{block_height}) and Cosmos SDK (/cosmos/base/tendermint/v1beta1/blocks/{height}) pattern.RestMessage.GetParams()exposes path parameters as a map keyed by the placeholder name, so a numeric value resolves to that block height.0xprefix, whichParseDefaultBlockParameterrejects.default_value: "latest"makes hash requests keep exactly today's behaviour (latest,UsedDefaultValue = true) instead of degrading toNOT_APPLICABLE.BLOCK_HASHgeneric parser surfaces the hash as a requested block hash. The consumer already ships it inBlocksHashesToHeightsand resolves it through the relay cache (resolveRequestedBlockpicks the cached height when the block was parsed from the default value), so hash requests can be routed to archive once the height is known. For a numeric value the hash parser fails the minimum-length check and falls through to the legacy parser, as inTestParseBlockFromParamsHash.Routes deliberately left on
DEFAULT latest:/epochs/{number}/...(epoch numbers, ~500 on mainnet) and/blocks/slot/{slot_number}//blocks/epoch/{epoch_number}/slot/{slot_number}(slot numbers, far above the block height). Both would be mis-classified against the archive boundary if parsed as block heights. A hash-to-height decision for those is a separate routing policy question.Spec gaps — diffed the API list against the current Blockfrost OpenAPI (
blockfrost/openapi,master). Added the chain-data endpoints that were missing, with the same CU convention as their siblings (10 for a single object, 20 for a list):/assets/{asset}/utxos/scripts/{script_hash}/utxos/governance/committee/governance/committee/votes/governance/committee/{cc_id}/votesNot added, on purpose:
/addresses/{address}/txsand/assets/{asset}/txs(markeddeprecatedin the OpenAPI, superseded by/transactions),/,/health,/health/clock,/metrics,/metrics/endpoints(Blockfrost account/service status, not chain data), and/ipfs/*(separate Blockfrost add-on product). Every endpoint currently in the spec exists in the OpenAPI and none is deprecated.Everything else in the spec (verifications,
pruningatlatest_distance: 8193vs the 8192 archive rule, finalization and block-time values, min stake,CARDANOTchain-id override) was reviewed and left unchanged.Verification
New tests in
protocol/chainlib/rest_test.goload the real spec throughCreateChainLibMocks("CARDANO", rest, ...):TestCardanoSpec_HistoricalBlockRequestsAreNotLatest—/blocks/1,/blocks/11500000and the five sub-routes resolve to the numeric height; a 64-hex hash resolves to latest with the hash captured inGetRequestedBlocksHashes();/blocks/latest,/epochs/1and/blocks/slot/1stay latest.TestCardanoSpec_HistoricalBlockActivatesArchive— with the archive extension enabled by policy andLatestBlock = 12,000,000: latest andlatest-1get no extension and 10 CU;latest-8193and/blocks/1get thearchiveextension and 50 CU.Against the original spec the first test fails on every numeric route with
expected: 1, actual: -2(LATEST_BLOCK), which reproduces the report.CI on
8e92aae: all 30 checks green (lint, CodeQL, test-protocol, test-consensus, protocol e2e, payment e2e, codecov patch, binaries, docker images).Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking change — not breakingmainbranchReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
https://claude.ai/code/session_015Zn1TrLFU2xyth1vqGmz7Q