Repository navigation
Bump Cardano dependencies and report Dijkstra submission errors - #17
Conversation
0434a59 to
8ddf1c8
Compare
fa91de3 to
379f58b
Compare
379f58b to
3c0b0c3
Compare
| , "it runs on a different network!" | ||
| , hint | ||
| ] | ||
| HandshakeError (Handshake.VersionMismatch _ _) -> |
There was a problem hiding this comment.
We should elaborate the versions otherwise this error message will go stale.
There was a problem hiding this comment.
Good point. The message now lists the versions the node offered and the ones this Ogmios supports, taken from the handshake, so there is no hard-coded version left to go stale.
| ValidationTagMismatch { validationTag, mismatchReason } | ||
| Cn.CollectErrors errors -> | ||
| pickPredicateFailure (encodeCollectErrors AlonzoBasedEraConway (toList errors)) | ||
| pickPredicateFailure (encodeCollectErrors era (toList errors)) |
There was a problem hiding this comment.
@carbolymer in a follow up PR can you try to remove the eons as much as possible and replace them with ShelleyBasedEra era where appropriate?
There was a problem hiding this comment.
Agreed. I replaced the eons with ShelleyBasedEra in a follow-up PR:
| let nEras = length eras | ||
|
|
||
| -- NOTE: Stake pool vectors come from Dijkstra to show the BLS key. | ||
| let isVectorEra = \case |
There was a problem hiding this comment.
Could we make the era for saved JSON examples an explicit argument, rather than checking for "stakePools" inside this helper? Which eras we test and which era supplies the saved examples should be separate decisions.
For example:
validateLedgerStateQuery ::
Int ->
SomeShelleyEra -> -- Era used for saved JSON examples
Text ->
Maybe Json.Value ->
(Json.Value -> Json.Parser (QueryInEra Gen Block)) ->
SpecWith ()The stake-pool test could explicitly select Dijkstra:
validateLedgerStateQuery
10
(SomeShelleyEra ShelleyBasedEraDijkstra)
"stakePools"
Nothing
(parseQueryLedgerStakePools
genPoolParametersResultNoStake
genPoolParametersResult)Then replace isVectorEra era with sameEra era vectorEra, and handle the list of eras being tested independently.
There was a problem hiding this comment.
Done. The era of the saved examples is now an explicit argument of validateLedgerStateQuery, and the check on the method name is gone.
| encodePoolFailure = \case | ||
| Di.StakePoolNotRegisteredOnKeyPOOL poolId -> | ||
| UnknownStakePool { poolId } | ||
| Di.StakePoolRetirementWrongEpochPOOL (Mismatch currentEpoch listedEpoch) (Mismatch _ firstInvalidEpoch) -> |
There was a problem hiding this comment.
The retirement epochs are reversed here. Mismatch stores the supplied value first and the expected value second, so this first mismatch contains the requested retirement epoch followed by the current epoch. The second mismatch's expected value is the last permitted retirement epoch, not the first invalid one.
For example, with current epoch 100, requested retirement 120, and maximum permitted retirement 110, this mapping returns currentEpoch = 120, declaredEpoch = 100, and firstInvalidEpoch = 110. Those should be 100, 120, and 111 respectively.
Please swap the first two bindings and convert the inclusive upper limit to the first invalid epoch. The older Shelley mapper has the same problem, so it would be worth correcting both rather than copying it into Dijkstra.
The pinned ledger rule shows the field ordering and the inclusive limit.
There was a problem hiding this comment.
Good catch. The mapper read both mismatches the wrong way round, so currentEpoch and declaredEpoch came out swapped, and firstInvalidEpoch reported the last valid epoch. I've fixed it in the Dijkstra mapper and in the Shelley one, which Conway also uses, so Conway submissions had the same problem. firstInvalidEpoch is now the limit plus one. The message only talked about retiring too late, but the same failure fires when the epoch is not after the current one, so I reworded it to cover both cases.
| StakePoolMetadataHashTooLarge { poolId, computedMetadataHashSize } | ||
| -- TODO: VRF-key reuse has no dedicated failure; fold into UnknownStakePool, | ||
| -- as the Shelley encoder does. | ||
| Di.VRFKeyHashAlreadyRegistered poolId _vrfKeyHash -> |
There was a problem hiding this comment.
VRF-key reuse should not become UnknownStakePool. This failure can occur when an already registered pool updates its parameters to use a VRF key belonging to another pool. Returning error 3140 tells the client that the pool is unknown and suggests checking registration/submission order, which is the wrong diagnosis.
Could we give this failure a distinct mapping and retain the offending VRF hash? The Shelley mapper has the same approximation, but copying it here preserves misleading client-visible behavior.
The pinned ledger implementation raises it for both new registrations and updates to existing pools.
There was a problem hiding this comment.
Agreed, reporting an unknown pool here sends clients the wrong way. I've added a new error, 3176 StakePoolVrfKeyAlreadyRegistered, which carries the pool id and the VRF key hash. The Shelley mapper (also used by Conway) and the Dijkstra one now both map VRFKeyHashAlreadyRegistered to it. The schema, docs and changelog are updated.
| Di.WithdrawalAddressesWithWrongNetwork expectedNetwork accounts -> | ||
| encodeWrongNetworkAccounts expectedNetwork accounts | ||
| -- TODO: missing accounts fold into IncompleteWithdrawals, as in earlier eras. | ||
| Di.WithdrawalAccountsMissing ws -> |
There was a problem hiding this comment.
Mapping missing accounts to IncompleteWithdrawals also reuses its message that withdrawals must consume the entire account balance. That statement is incorrect for Dijkstra outside legacy mode, where partial withdrawals are allowed.
Please distinguish the missing-account diagnosis from the legacy requirement to withdraw everything, either through a separate failure or a message that accounts for the actual cause. The same issue applies to WithdrawalAccountsMissingFromOriginal and the corresponding sub-transaction mappings. A client fixing a missing account should not also be told that a valid partial withdrawal is forbidden.
There was a problem hiding this comment.
Agreed on both points. Missing withdrawal accounts, in the transaction and in its sub-transactions, now map to the existing 3146 error, StakeCredentialNotRegistered, with the account's credential. When several accounts are missing, it names the first one, since Ogmios reports one error per submission. I've also reworded the 3141 message so it no longer says a withdrawal must always take the whole balance, as Dijkstra allows partial withdrawals outside legacy mode.
|
|
||
| -- | The ledger's own generator rarely yields the newest failures; add them explicitly. | ||
| genDijkstraMempoolFailure :: Gen (Di.DijkstraMempoolPredFailure DijkstraEra) | ||
| genDijkstraMempoolFailure = frequency |
There was a problem hiding this comment.
Could we add deterministic tests for the meaning of these mappings as well as the generated JSON/schema checks? Swapping currentEpoch and declaredEpoch, or mapping VRF-key reuse to UnknownStakePool, still produces schema-valid JSON.
In particular, please assert the exact error code and data for retirement epoch boundaries, VRF-key reuse, and the distinction between missing withdrawal accounts and inexact legacy withdrawals. Generating more samples alone will not catch these semantic mistakes.
There was a problem hiding this comment.
Agreed. The schema tests couldn't catch the swapped epochs, because swapped numbers still produce valid JSON. I've added unit tests that check the exact error code and data for the retirement epoch bounds, VRF key reuse, and missing versus partial withdrawals.
3c0b0c3 to
1ed2fcb
Compare
1ed2fcb to
c039310
Compare
Move to cardano-ledger-core 1.22, cardano-ledger-conway 1.24, cardano-ledger-dijkstra 0.4, ouroboros-consensus 5.2, ouroboros-network 2.0 and cardano-diffusion 1.3, at CHaP index-state 2026-10-27 and Hackage index-state 2026-10-23, with matching flake.lock inputs. Drop the cardano-node version pin and the allow-newer entries the new stack no longer needs. The ram package replaces memory. The new stack speaks only node-to-client V23, which cardano-node 10.7.1 and later offer. Ogmios now refuses a node without V23 at the handshake with a clear error, and stops. The error lists the node-to-client versions the node offers and the ones Ogmios supports. Stake pools gain an optional blsKey field. Failed Dijkstra-era transaction submissions are now reported as errors. Before, the connection closed with no reply. Existing error codes are used where they fit. New codes 3169 to 3175 cover guards, a pointer address in the collateral return, the BLS proof of possession, all inputs spent, withdrawals that exceed the balance, and balances outside their intervals. If the encoder fails while building a submitTransaction reply, Ogmios now returns an internal error instead of closing the connection. In Dijkstra, withdrawals from accounts that are not registered, including those in sub-transactions, are reported as 3146 StakeCredentialNotRegistered. The 3141 message no longer says a withdrawal must always take the full balance. New error 3176 StakePoolVrfKeyAlreadyRegistered is reported when a pool registration or update reuses the VRF key of another pool. Before, this was reported as an unknown stake pool. Fix error 3142 for pool retirements in Conway and Dijkstra. It swapped the current epoch and the listed epoch, and gave the last valid epoch as firstInvalidEpoch. It now also covers retirements that are too early, not only too late. Script-context build failures get new error messages, and the Dijkstra pointer-address message now says whether the address is an input or an output. Fix the JSON schema: add dijkstra to the Era enum, the guard clause of native scripts, and the guard script purpose. Add the Dijkstra reference script size parameters, maxReferenceScriptsSizePerBlock and maxReferenceScriptsSizePerTransaction. Key treasury withdrawals by stake address. Regenerate the TypeScript types from the schema. Check Dijkstra stake pools with blsKey and every new error code against the schema, and regenerate the test vectors. Unit tests now check the exact error code and data for these cases.
c039310 to
851726b
Compare
This PR moves ogmios to the latest Cardano packages on CHaP and maps every Dijkstra-era submission failure to an Ogmios error, with 8 new error codes, fixes for two existing errors, and a schema fix. ogmios depends on neither cardano-node nor cardano-api, so it does not have to wait for a node release built on these versions.
The switch to GHC 9.12.4 came in #16, which is merged. The new packages need it, because the new cardano-protocol-tpraos and cardano-ledger-shelley-test releases are not buildable with GHC 9.8, and ouroboros-consensus 5.2 needs both.
Dependencies
The CHaP index-state moves to 2026-10-27 and the Hackage index-state to 2026-10-23, with matching CHaP and hackage.nix inputs in
flake.lock.server/cabal.projectdrops thecardano-node == 11.1.1pin and theany.io-classes ^>= 1.8constraint, and removes theallow-newerentries the new set no longer needs (*:formatting,io-sim:time,io-classes:time).katip:Win32stays, since only a Windows build can show whether it is still needed.The package bounds change as well. ouroboros-consensus gets
^>= 5.2and ouroboros-network gets^>= 2.0. The new lower bounds aresmall-steps >= 1.2andcardano-ledger-alonzo >= 1.17.nodeToClientV_Minmoves from V17 to V23.ogmiosnow depends onraminstead ofmemory, following crypton 1.1.Client-visible changes
blsKeyfield withverificationKeyandproofOfPossession. It appears in thestakePoolsquery and in genesis stake pools when the pool has a BLS key. It will appear in pool registration certificates once the Dijkstra transaction encoder emits them.cardano.jsongains aBlsKeydefinition.RetirementTooLate) now reports the right epochs for a pool retirement. It used to swap the current epoch and the listed epoch, and gave the last valid epoch asfirstInvalidEpoch. The bug was already on master, so Conway was affected too. The message now also covers retirements that are too early, not only too late.cardano.json, and so the TypeScript types) now listsdijkstrain theEraenum. This coversEraMismatch,incompatibleEra,unsupportedEra,minimumRequiredEraandcurrentNodeEra, and theerafield of Praos blocks.Script<Native>now lists the Dijkstraguardclause ({"clause":"guard","from":<hash>}). The server already emitted it, but the schema rejected it.guardscript purpose."guard"is now allowed inRedeemerPointer.purpose, which evaluation results, invalid redeemer pointers andextraneousRedeemersuse, andmissingRedeemersaccepts a{"purpose": "guard", "script": <hash>}variant. The server already emitted both, and the CI schema test caught the gap. It also fixes two more gaps. The withdrawals of atreasuryWithdrawalsgovernance action (RewardTransfer) are now keyed by stake address, which is what Ogmios sends, where the schema expected 56 hex characters, so no response containing such a proposal matched it. That bug was already on master, and CI found it with a random seed. The Dijkstra protocol parameters now listmaxReferenceScriptsSizePerBlockandmaxReferenceScriptsSizePerTransaction.StakeCredentialNotRegistered. Earlier eras still report 3141. The 3141 message no longer says a withdrawal must always take the full balance.submitTransactionnow returns a JSON internal error instead of closing the client's connection with no reply.Existing error codes keep their meaning. The message changes are the two script-context messages, the Dijkstra pointer-address message, the 3141 message and the 3142 fix above.
Dijkstra submission errors
Before this PR, every Dijkstra-era rejection closed the connection with no reply, because the encoder was a stub. Now all 163 ledger failure constructors map to Ogmios errors. Existing codes are used where an equivalent exists, and there are 8 new codes. Code 3176 is not Dijkstra-only: it is reported in any era where the ledger rejects a pool registration or update that reuses the VRF key of another pool.
3176 carries the pool id and the VRF key hash. Before, this failure was reported as an unknown stake pool.
ogmios.json, the local-tx-submission docs page and the TypeScript types include the new codes. In the docs page, 3176 follows 3175 and 3997 and 3998 move down one place.Notes for reviewers
cardano-datadependency for it. New unit tests check the exact error code and data for retirement limits, VRF key reuse, partial withdrawals and missing accounts. The era of the saved examples is now an explicit test argument.EraMismatch, becausegenMismatchEraInfoonly makes Byron and Shelley pairs. Tests do not exercise that schema case.CertificateNotSupportedandPlutusPurposeNotSupportedconstructors ofConwayContextError. ogmios tells them apart by their JSONkindso that both keep their old messages. Deterministic tests now pin this workaround for Conway and Dijkstra.IncompleteWithdrawals(3141), and Dijkstra maps it toStakeCredentialNotRegistered(3146).NetworkMismatch;IncompleteWithdrawals;StakeCredentialNotRegistered, first account only;ValueNotConservedInLegacyModemaps toValueNotConserved;SubTreasuryValueMismatchmaps toTreasuryWithdrawalMismatch;withNumTestsinstead of the deprecatedwithMaxSuccess(18 call sites inJsonSpec.hsandBech32Spec.hs), which removes the deprecation warnings. Behaviour and vectors are unchanged.blsKey, and the schema check validates them. The evaluation vectors include Dijkstra context errors.SubmitTransactionResponseandEvaluateTransactionResponseran with 30 seeds each and found no further gaps.ogmios-unitfailure on seed 2007211148 was the treasury-withdrawal key pattern, and that seed now passes.cardano.json. The regeneration also picks upplutus:v4and aSignatorydoc change that were already incardano.jsonon master.Not included
19 new Dijkstra protocol parameters are not encoded yet, because their JSON names need a schema decision: 9 Leios parameters, 6 Peras parameters,
maxPledgeLeverage,minPoolMarginand the 2refInputsCostPer*parameters. The gap also affects governance proposals: anevaluateTransactionfailure that shows aParameterChangetouching only new fields renders"parameters": {}.liveStakeDistributiondoes not show the pool BLS key.Dijkstra submission errors do not say which sub-transaction failed, because the ledger does not report it.
server/Makefilestill enters theghc96-static-minimal-iogdevx shell, becausegithub:CardanoSolutions/devxhas no GHC 9.12 shell yet. The Makefile target cannot build this branch until devx adds one.How to trust this PR
nix build .#ogmiospasses with the production flag (-Werror).checks.x86_64-linux.ogmios-unitderivation stops before the tests because it cannot create/build, so I ran the built test binary by hand on the same tree. A standard nix sandbox, as on Hydra, has/build.latestReleasedNodeVersioncaps the node-to-client versions a node offers. For example, ouroboros-consensus 3.0.1.0 (cardano-node 10.7.1 to 11.0.1) and 4.2.1.0 (cardano-node 11.1.1 and 11.1.3) both returnNodeToClientV_23, and consensus 5.2 keeps the V23 codecs. cardano-node 10.7.0 also offers V23 but is a pre-release.