Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop support for JSON encoded transactions #1213

Closed
5 of 8 tasks
ghost opened this issue Dec 13, 2023 · 5 comments · Fixed by #1240
Closed
5 of 8 tasks

Drop support for JSON encoded transactions #1213

ghost opened this issue Dec 13, 2023 · 5 comments · Fixed by #1240
Labels
api Items related to the Hydra client API green 💚 Low complexity or well understood feature 💬 feature A feature on our roadmap
Milestone

Comments

@ghost
Copy link

ghost commented Dec 13, 2023

Why

  • As new Eras come into play and Tx representation changes, storing data in non-canonical JSON runs the risk of inconsistencies as we observed when trying to migrate to Conway and found out the txid changed because canonical CBOR representation changed
  • We maintain our own JSON representation which is cumbersome and error-prone as we don't test for all the possible transaction contents and are not safe against upstream changes

What

  • Migrate how we store and share Transactions as JSON everywhere

    • All Tx JSON representations have the transaction data as cborHex (see ADR)
    • NewTx websocket command and /cardano-transaction HTTP endpoint accept transactions as objects with a cborHex field (nothing else required)
      • If an txId is present, it must match the transaction body hash
    • The /commit response and TxValid, TxInvalid etc. websocket messages are compatible with cardano-cli transaction commands and also contain a txId
    • Anything we store on disk must us CBOR encoded transactions (can use the same schema as above)
    • Documentation is updated to instruct users to use CBOR encoded transactions and how to deal with them (e.g. debugging)
  • Ensure our maintained hydra clients and integrations still work

    • hydra-tui and hydraw (in the same repo)
    • hydra-poll and hydra-chess (maintained by us, different repos)
    • kupo indexing of a hydra head (we originally contributed and is affected)
  • Out of scope: Any changes to UTxO (as initially drafted in the ADR)

How

@ghost ghost added api Items related to the Hydra client API documentation 📖 Documentation changes ux Related to user experience labels Dec 13, 2023
@ghost ghost changed the title Cardano Tx not as JSON ADR Replace JSON with CBOR in external representations of Tx and UTxO Dec 13, 2023
ghost pushed a commit that referenced this issue Dec 13, 2023
@ghost ghost mentioned this issue Dec 13, 2023
4 tasks
ch1bo pushed a commit that referenced this issue Dec 14, 2023
ch1bo pushed a commit that referenced this issue Dec 21, 2023
@ch1bo ch1bo added feedback needed and removed documentation 📖 Documentation changes labels Dec 22, 2023
@ghost
Copy link
Author

ghost commented Jan 2, 2024

After having spent some time working on a Chess game DApp for Hydra, I am now convinced of the benefits of a proper JSON API over a CBOR one, at least for UTxO-related interactions. Being able to commit a JSON-formatted UTxO through the HTTP interface, and being able to extract information from JSON-formatted UTxO from the Head protocol, is very useful as it alleviates the need for the client to be aware of all the details of the CBOR formatting of those UTxOs.

In general, a JSON API is much more versatile from a client perspective, notably because it allows those clients to pick and choose what they support, for example ignoring some parts of a data structure they don't care about and using symbolic references (eg. field names) instead of having to be aware of all the details of the structure's layout as in CBOR. There's support for JSON-schema based documentation and even code generation, which might not be stellar but is still better than what's available for CBOR. And it's always easier for a client to drop to raw JSON handling.

I think we should continue to push for first-class support of such an interface in the cardano ecosystem, and for example improve on Cardanonical, and/or propose/use canonical JSON schema for cardano-ledger data structures.

What I think is the most glaring issue right now is the lack of proper versioning of the serialised data, whether it's persisted on disk or exchanged across the network of nodes or through the interfaces we provide:

  • A starting hydra-node should be able to check what's the version of the data it's loading
  • A client should know what's the version of the data is supported by the node it connects to

This can be easily done by extending these data structures with a generic version field, and adding check/conversion code to the hydra-node. Note this realisation also dawned on the cardano-ledger team leading them to change all their serialisation formats to include a version.

@ch1bo ch1bo changed the title Replace JSON with CBOR in external representations of Tx and UTxO Replace JSON with CBOR in external representations of Tx Jan 8, 2024
@ch1bo ch1bo changed the title Replace JSON with CBOR in external representations of Tx Drop JSON support for external representations of Tx Jan 8, 2024
@ch1bo ch1bo changed the title Drop JSON support for external representations of Tx Do not accept JSON serialized transactions anywhere Jan 8, 2024
@ch1bo
Copy link
Collaborator

ch1bo commented Jan 8, 2024

Discussion from tactical:

  • Any "self-hashing" data should not be accepted as JSON, although for informational purposes it might be just fine and helpful to output this data as JSON on the API
  • UTxO for example are helpful and fine to accept as JSON on /commit endpoint

ghost pushed a commit that referenced this issue Jan 11, 2024
ghost pushed a commit that referenced this issue Jan 11, 2024
ghost pushed a commit that referenced this issue Jan 11, 2024
@ghost ghost mentioned this issue Jan 11, 2024
4 tasks
@ch1bo ch1bo reopened this Jan 16, 2024
@ch1bo
Copy link
Collaborator

ch1bo commented Jan 17, 2024

I realized that we also should contribute a PR to kupo to decode the cbor (as a follow-up)

@ch1bo
Copy link
Collaborator

ch1bo commented Jan 19, 2024

This should actually become a 💬 feature item and put on our roadmap. I'll bring it up on our next grooming session.

@ch1bo ch1bo changed the title Do not accept JSON serialized transactions anywhere Drop support for JSON encoded transactions Jan 23, 2024
@ch1bo ch1bo added 💬 feature A feature on our roadmap and removed feedback needed labels Jan 23, 2024
@ch1bo ch1bo added green 💚 Low complexity or well understood feature and removed ux Related to user experience labels Jan 23, 2024
@ch1bo ch1bo added this to the 0.16.0 milestone Jan 23, 2024
@ch1bo ch1bo moved this to Now in Hydra Head Roadmap Jan 23, 2024
@ch1bo ch1bo linked a pull request Jan 26, 2024 that will close this issue
4 tasks
@ch1bo
Copy link
Collaborator

ch1bo commented Mar 28, 2024

We decided not to include hydra-poll and hydra-chess fixes as they are targeting older versions of the released hydra-node right now and we will assist anyone who wants to put it o use when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Items related to the Hydra client API green 💚 Low complexity or well understood feature 💬 feature A feature on our roadmap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants