Skip to content

Add FromJSON instance for new experimental TxOut#1179

Open
Jimbo4350 wants to merge 4 commits into
masterfrom
issue-926-experimental-fromjson-txout
Open

Add FromJSON instance for new experimental TxOut#1179
Jimbo4350 wants to merge 4 commits into
masterfrom
issue-926-experimental-fromjson-txout

Conversation

@Jimbo4350

Copy link
Copy Markdown
Contributor

Context

Implements FromJSON instances for the new TxOut era type (which wraps L.TxOut era) in the experimental API, matching the JSON format produced by the ToJSON instances added in #1176.

Closes #926

Changelog

- description: |
    Add FromJSON instance for new experimental TxOut type with per-era
    concrete instances, inline datum parsing from inlineDatumRaw with
    hash validation, and reference script support via scriptInAnyLangToLedgerScript
  type:
   - feature
  projects:
   - cardano-api

How to trust this PR

  • Concrete FromJSON instances for each era (Shelley through Conway), mirroring the ToJSON structure from Add ToJSON instance for new experimental TxOut #1176
  • Pre-Alonzo eras parse address and value only via shared txOutBaseParseJson helper
  • Alonzo adds datum hash parsing via dataHashTxOutL
  • Babbage+ adds inline datum parsing from inlineDatumRaw (hex-encoded original CBOR bytes) with hash validation against inlineDatumhash, and reference script parsing via scriptInAnyLangToLedgerScript
  • Supplemental datums deliberately unsupported — the ledger TxOut does not carry them
  • Helper functions: addrFromJson (reverse of addrToJson), scriptInAnyLangToLedgerScript (reverse of ledgerScriptToScriptInAnyLang)
  • Property test verifies JSON round-trip (fromJSON . toJSON = id) across all Shelley-based eras

Checklist

  • Commit sequence is logical and each commit has a good commit message
  • New tests are added if needed and old tests still pass
  • Code is formatted with fourmolu
  • Self-reviewed the diff

@Jimbo4350 Jimbo4350 force-pushed the issue-926-experimental-fromjson-txout branch from 87cfa8a to bc2431f Compare April 20, 2026 13:48
@Jimbo4350 Jimbo4350 force-pushed the issue-926-experimental-fromjson-txout branch from bc2431f to 9734a72 Compare April 27, 2026 20:19
@github-actions

Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 45 days with no activity.

@github-actions github-actions Bot added Stale and removed Stale labels Jun 12, 2026
@Jimbo4350 Jimbo4350 force-pushed the issue-926-experimental-fromjson-txout branch 2 times, most recently from 4c0be1f to 3467b7f Compare June 16, 2026 13:30
@Jimbo4350 Jimbo4350 marked this pull request as ready for review June 25, 2026 15:07
Add per-era FromJSON instances for the experimental TxOut type,
mirroring the ToJSON structure. Pre-Alonzo eras parse address and
value only; Alonzo adds datum hash support; Babbage+ adds inline
datum (parsed from inlineDatumRaw with hash validation) and
reference script support. Supplemental datums are deliberately
unsupported as the ledger TxOut does not carry them.
Copilot AI review requested due to automatic review settings June 25, 2026 15:07
Replace the three separate where-bound helpers (datumFields,
inlineDatumFields, refScriptFields) with a single top-level
datumAndRefScriptFields function. Simplifies alonzoOnwardsTxOutToJson
and documents the per-era field layout in one place.
@Jimbo4350 Jimbo4350 force-pushed the issue-926-experimental-fromjson-txout branch from 3467b7f to 223fb95 Compare June 25, 2026 15:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds FromJSON support for the experimental TxOut era wrapper (around L.TxOut era), intended to be the inverse of the ToJSON format introduced in #1176, and adds a property test to verify JSON round-tripping for supported Shelley-based eras.

Changes:

  • Implement FromJSON instances for experimental TxOut across Shelley→Conway, including Babbage+ inline datum parsing (from inlineDatumRaw) with hash validation and reference script support.
  • Refactor datum/reference-script JSON field emission to a shared helper (datumAndRefScriptFields).
  • Add a Hedgehog property test for encode/eitherDecode round-tripping, plus a Herald changelog fragment.

Reviewed changes

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

File Description
cardano-api/test/cardano-api-test/Test/Cardano/Api/Json.hs Adds a JSON round-trip property for the new experimental TxOut (with Dijkstra currently skipped).
cardano-api/src/Cardano/Api/Experimental/Tx/Internal/BodyContent/New.hs Adds FromJSON parsing for experimental TxOut (base fields, datum hash, inline datum raw+hash validation, reference scripts).
.changes/20260420_cardano_api_fromjson_experimental_txout.yml Adds the required changelog fragment documenting the new feature.

Comment on lines +96 to +98
-- | Verify that the new experimental 'TxOut' round-trips through JSON
-- (encode then decode) for all Shelley-based eras.
prop_new_txout_json_roundtrip :: Property
Comment thread cardano-api/test/cardano-api-test/Test/Cardano/Api/Json.hs Outdated
Comment on lines +583 to +585
Nothing -> case mDatumHash of
Just dh -> pure $ L.DatumHash dh
Nothing -> pure L.NoDatum
…rage

Fail when inlineDatumhash is present without inlineDatumRaw, symmetric with
the existing check for the opposite case. Correct the Haddock and add an
explanatory comment for the Dijkstra skip in prop_new_txout_json_roundtrip.

@palas palas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I just made a couple of small suggestions.

inlineDatumhash is really weird, both the capitalisation and it being omitted on Babbage when it inline datum is not present (instad of just being null). But it is outside of the scope of the PR

addr <- addrFromJson =<< o .: "address"
apiVal <- parseJSON =<< o .: "value"
let mv = toMaryValue apiVal
val <- case cast mv of

@palas palas Jun 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this usage of cast could use a comment explaining how it works

Comment on lines +534 to +536
Nothing -> case cast (L.coin mv) of
Just v -> pure v
Nothing -> fail "txOutBaseParseJson: value is unsupported for this era"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Nothing -> case cast (L.coin mv) of
Just v -> pure v
Nothing -> fail "txOutBaseParseJson: value is unsupported for this era"
Nothing -> do
let L.MaryValue _ ma = mv
unless (ma == mempty) $
fail "txOutBaseParseJson: ada-only era output cannot carry a multi-asset value"
case cast (L.coin mv) of
Just v -> pure v
Nothing -> fail "txOutBaseParseJson: value is unsupported for this era"

I think it would be for the best to check there are no non-ada assets in the case of pre-mary eras

Comment on lines +586 to +588
case mDatumHash of
Just dh -> pure $ L.DatumHash dh
Nothing -> pure L.NoDatum

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case mDatumHash of
Just dh -> pure $ L.DatumHash dh
Nothing -> pure L.NoDatum
maybe L.NoDatum L.DatumHash mDatumHash

Maybe a good place to use the maybe function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement JSON instances for the new experimental TxOut

3 participants