Add HasCallStack constraints to functions containing error calls

[Jimbo4350]

Changelog

- description: |
    Add HasCallStack constraints to standalone functions that call error
    directly or indirectly, and propagate to their callers. Dijkstra-era
    placeholder error messages are also improved.
  type:
   - compatible
  projects:
   - cardano-api

Context

Closes #809

Phase 1: HasCallStack at error sites

Each placement is justified by a real error call in the function body. We add HasCallStack rather than replacing error with proper error handling for the reasons noted:

Function	File	Error source	Why HasCallStack rather than replacing error
generateInsecureSigningKey	Key/Internal/Class.hs	Direct error on deserialization failure	Genuinely impossible — random bytes that don't deserialize as a valid signing key indicate a broken crypto implementation, not a recoverable condition
foldBlocks	LedgerState.hs	error "Impossible! Missing Ledger state" in nested chain sync client	Return type is constrained by the ouroboros-network protocol client callback interface — can't change to Either without rearchitecting the protocol client
chainSyncClientWithLedgerState	LedgerState.hs	2× error "Impossible! History should always be non-empty"	Same — deep inside a protocol handler whose type is dictated by ouroboros-network
chainSyncClientPipelinedWithLedgerState	LedgerState.hs	2× same as above	Pipelined variant, same constraint
foldEpochState	LedgerState.hs	error "Impossible! Missing Ledger state" in nested function	Same — protocol callback interface
fromShelleyNetwork	Network/Internal/NetworkId.hs	error on wrong mainnet network magic	Could return Either, but it's called from fromShelleyGenesis and other pure conversion functions — changing the return type would ripple through the genesis/network conversion chain
toConsensusQueryShelleyBased	Query/Internal/Type/QueryInMode.hs	4× direct error for wrong-era queries	Could return Either, but feeds into the consensus query pipeline which expects a pure value — would require reworking the query infrastructure
getTxIdByron	Tx/Internal/Body.hs	fromMaybe impossible (hash size mismatch)	Genuinely impossible — hash size is fixed by the crypto algorithm
makeShelleySignature	Tx/Internal/Sign.hs	fromMaybe impossible (signature size mismatch)	Genuinely impossible — signature size is fixed by the crypto algorithm

Phase 2: Propagation to callers

After adding HasCallStack to the error sites, it was propagated up the call chain so stack traces show the full path rather than stopping at the immediate error site. Each line below is a call chain — read → as "calls". The rightmost function is where the error lives; each function to its left gets HasCallStack so its frame appears in the stack trace.

Propagation from Phase 1 error sites

Tx signing (all paths end at makeShelleySignature):

makeShelleyKeyWitness       → makeShelleyKeyWitness'            → makeShelleySignature
makeShelleyBootstrapWitness → makeShelleyBasedBootstrapWitness  → makeShelleySignature
makeKeyWitness              → makeShelleySignature
issueOperationalCertificate → makeShelleySignature

Query:

toConsensusQuery → toConsensusQueryShelleyBased

Genesis:

fromShelleyGenesis → fromShelleyNetwork

The remaining Phase 1 error sites (generateInsecureSigningKey, foldBlocks, chainSyncClientWithLedgerState, chainSyncClientPipelinedWithLedgerState, foldEpochState, getTxIdByron) are top-level entry points with no internal callers in this repo to propagate to.

Propagation from pre-existing master constraints

These functions call downstream functions that already carried HasCallStack on master but were themselves missing the constraint:

Tx body construction & output (callees pre-existing on master: mkCommonTxBody, toShelleyTxOut, toShelleyTxOutAny, createTransactionBody):

createAndValidateTransactionBody → makeShelleyTransactionBody → (pre-existing constrained callees)
fromLegacyTxOut                                               → (pre-existing constrained callees)
toShelleyUTxO                                                 → (pre-existing constrained callees)

Fee calculation (callees pre-existing on master: balanceTxOuts, makeTransactionBodyAutoBalance, estimateBalancedTxBody, calculateMinimumUTxO):

calcMinFeeRecursive               → (pre-existing constrained callees)
estimateOrCalculateBalancedTxBody → (pre-existing constrained callees)
constructBalancedTx               → (pre-existing constrained callees)

Error message improvements (no HasCallStack)

Dijkstra-era placeholder error calls had unhelpful messages (error "dijkstra", error ""). These are replaced with descriptive messages naming the function and the reason, e.g. error "getPlutusDatum: Dijkstra era not supported".

What was excluded and why

Dijkstra-era placeholders (~20 calls across Era/Internal/Case.hs, Eon/*.hs, etc.): All temporary error calls that will be replaced with real implementations when Dijkstra support lands. Adding HasCallStack would create churn across many signatures that will be rewritten anyway.

Class method instances (castVerificationKey, deterministicSigningKey): The failing cases guard impossible code paths (e.g. ed25519-bip32 extended key byte length can never mismatch ed25519). HasCallStack on instance methods via InstanceSigs doesn't propagate the caller's stack through class dispatch, and putting it on the class method would force the constraint on all instances including those that can never fail.

Certificate validation functions (toShelleyPoolParams, toShelleyDnsName, toShelleyUrl, fromShelleyPoolMetadata): These have real error calls for validation failures, but they carry TODO: proper validation comments — they should be converted to proper error handling rather than patched with HasCallStack.

How to trust this PR

• Every HasCallStack placement is justified by a real error call in the function body — see table above
• Propagation chains are visualized above so the "caller → callee" relationship is auditable
• Only standalone functions are constrained; class methods are left alone
• Dijkstra placeholder messages are improved but not constrained
• Builds clean with -Wredundant-constraints — every constraint is actually used

Checklist

• Commit sequence broadly makes sense and commits have useful messages
• New tests are added if needed and existing tests are updated
• Self-reviewed the diff

[Copilot]

Pull request overview

This PR improves runtime debuggability by adding HasCallStack constraints to selected standalone functions that can reach error, and by replacing placeholder “Dijkstra”/empty error messages with more descriptive ones across a few era-bridging/conversion helpers.

Changes:

• Add HasCallStack constraints to a small set of functions so call stacks include their callers when error is triggered.
• Improve several Dijkstra-era placeholder error messages to include the function name and a clearer reason.
• Add GHC.Stack (HasCallStack) imports where newly required.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
cardano-api/src/Cardano/Api/Tx/Internal/Sign.hs	Adds HasCallStack to makeShelleySignature and imports HasCallStack.
cardano-api/src/Cardano/Api/Tx/Internal/Body.hs	Adds HasCallStack constraint to getTxIdByron for better stacks on the “impossible” hash-size path.
cardano-api/src/Cardano/Api/Query/Internal/Type/QueryInMode.hs	Adds HasCallStack to toConsensusQueryShelleyBased to improve stacks for wrong-era query errors.
cardano-api/src/Cardano/Api/Plutus/Internal/Script.hs	Replaces empty/placeholder error messages with descriptive “Dijkstra era not supported” messages.
cardano-api/src/Cardano/Api/Network/Internal/NetworkId.hs	Adds HasCallStack to fromShelleyNetwork and imports HasCallStack.
cardano-api/src/Cardano/Api/LedgerState.hs	Adds HasCallStack to chain-sync/ledger-state folding entrypoints and imports HasCallStack.
cardano-api/src/Cardano/Api/Key/Internal/Class.hs	Adds HasCallStack to generateInsecureSigningKey and imports HasCallStack.
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/AnyWitness.hs	Improves a Dijkstra placeholder error message in getPlutusDatum.
cardano-api/src/Cardano/Api/Certificate/Internal.hs	Improves Dijkstra placeholder error messages in getAnchorDataFromCertificate.

[carbolymer]

Closes #1175

Wrong link

[carbolymer]

That's a good direction, but I think we can do better.

1. It would be nice to enhance all the callers of the modified functions here with HasCallstack for complete call stacks.

2. Currently all errors for dijkstra are not following any convention. It would be better to update all of them to follow the same pattern so they're easier to find. Here's what exists across ~26 Dijkstra placeholder errors:

   Three patterns in use

   1. Descriptive (majority, ~22 instances) -- already follows "functionName: Dijkstra era not supported":

   error "caseByronOrShelleyBasedEra: DijkstraEra is not supported"     -- Era/Internal/Case.hs
   error "shelleyBasedEraConstraints: Dijkstra is not yet supported"    -- Eon/ShelleyBasedEra.hs
   error "makeShelleyTransactionBody: Dijkstra is not  supported"       -- Tx/Internal/Body.hs (note: double space typo)
   error "estimateBalancedTxBody: DijkstraEra is not supported for fee estimation"  -- Fee.hs

   3. Bare placeholders (4 instances) -- the ones the PR fixes:

   error "dijkstra"   -- 3 places
   error ""           -- 1 place (fromShelleyMultiSig)

   5. TODO-prefixed error (1 instance):

   error "TODO: makeStakeAddressDelegationCertificate DijkstraEra"  -- Certificate/Compatible.hs:74

[palas]

Nothing controversial 👍

[carbolymer]

line breaks will be carried over to changelog. You should tell your Claude that's 2026 and we're not using 80 character line length limit anymore.

Idk why mine was insisting on doing that too...

[carbolymer]

LGTM. Do you think we could address the other remaining error stubs (in other cardano-api code) for Dijkstra era in a follow up PR?

[carbolymer]

Suggested change

	Add HasCallStack constraints to standalone functions that call error
	Add `HasCallStack` constraints to standalone functions that call error

[Jimbo4350]

LGTM. Do you think we could address the other remaining error stubs (in other cardano-api code) for Dijkstra era in a follow up PR?

Can you elaborate? Address in what sense?

[carbolymer]

@Jimbo4350 There are different error messages in error stubs, which are harder to find. Making them uniform would make they more greppable:

Pattern A - error "Dijkstra era is not active yet" (12 hits, identical message):

• OrphanInstances/Shelley.hs:482,487,492,497,502,507 - 6x ToObject instances
• Tracing/Era/Shelley.hs:1224,1229,1234,1239,1244,1249 - 6x LogFormatting instances

Pattern B - -- TODO dijkstra: (1 hit):

• Blockfrost.hs:283 - comment about missing blockfrost params

Pattern C - -- TODO: with dijkstra mentioned in the text (1 hit):

• Shutdown.hs:133 - comment about cardano-cli genesis create

The main problem: patterns A, B, and C use three different conventions. grep 'TODO.*dijkstra' misses all 12 error stubs. grep 'Dijkstra era is not active' misses the two TODO comments. No single grep
catches all 14.

Unifying them all under e.g. TODO Dijkstra in either the error string or an adjacent comment would make grep -r 'TODO Dijkstra' a one-stop search.

[Jimbo4350]

@Jimbo4350 There are different error messages in error stubs, which are harder to find. Making them uniform would make they more greppable:

Pattern A - error "Dijkstra era is not active yet" (12 hits, identical message):

* OrphanInstances/Shelley.hs:482,487,492,497,502,507 - 6x ToObject instances

* Tracing/Era/Shelley.hs:1224,1229,1234,1239,1244,1249 - 6x LogFormatting instances

Pattern B - -- TODO dijkstra: (1 hit):

* Blockfrost.hs:283 - comment about missing blockfrost params

Pattern C - -- TODO: with dijkstra mentioned in the text (1 hit):

* Shutdown.hs:133 - comment about cardano-cli genesis create

The main problem: patterns A, B, and C use three different conventions. grep 'TODO.*dijkstra' misses all 12 error stubs. grep 'Dijkstra era is not active' misses the two TODO comments. No single grep catches all 14.

Unifying them all under e.g. TODO Dijkstra in either the error string or an adjacent comment would make grep -r 'TODO Dijkstra' a one-stop search.

Will open a follow up PR

[Jimbo4350]

Thanks @carbolymer — good catch. The four files you cite are in cardano-node, not cardano-api, so I'll track them over there separately.

For cardano-api itself I audited the same thing and found 27 error stubs mentioning Dijkstra across 5+ phrasings plus 4 inconsistent -- TODO comments — none shared a single greppable token. #1187 unifies 30 of them under TODO Dijkstra:. After that PR, grep -r 'TODO Dijkstra' cardano-api/src returns every Dijkstra-era placeholder in one shot.

Two sites were intentionally excluded from the cardano-api unification, both documented in #1187's description:

• Tx/Internal/Body.hs:2858 — runtime invariant assertion for DijkstraGuarding, not a pending-implementation stub.
• Plutus/Internal/Script.hs:1302 — PlutusV4 stub that blocks on ledger exposing a PlutusV4 script constructor; tracked as PlutusV4 script construction not implemented in old API (Plutus/Internal/Script.hs:1302) #1188.