Skip to content

Conversation

kderme
Copy link
Contributor

@kderme kderme commented Sep 11, 2021

No description provided.

@kderme kderme requested a review from erikd as a code owner September 11, 2021 22:00
encodeData :: Alonzo.Data StandardAlonzo -> ByteString
encodeData dt = LBS.toStrict $ Aeson.encode $
Api.scriptDataToJson Api.ScriptDataJsonDetailedSchema $ Api.fromAlonzoData dt
Api.scriptDataToJson Api.ScriptDataJsonNoSchema $ Api.fromAlonzoData dt
Copy link
Contributor

@sorki sorki Sep 14, 2021

Choose a reason for hiding this comment

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

What's the reason for this one? Wouldn't we loose some expressiveness regarding constructors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DetailedSchema doesn't seem very user friendly

Copy link
Contributor

@sorki sorki Sep 29, 2021

Choose a reason for hiding this comment

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

I think this should keep using DetailedSchema to have the full representational capacity as outlined in https://github.com/input-output-hk/cardano-node/blob/master/cardano-api/src/Cardano/Api/ScriptData.hs#L238-L299

Quoting the most relevant part

It also means any script data can be converted into the JSON and
back without loss. That is we can round-trip the script data via the JSON and
also round-trip schema-compliant JSON via script data.

which according to my understanding means that NoSchema is lossy as in some cases you can't convert NoSchema JSON to original ScriptData.

If an explorer or some tool wants to display user-friendly NoSchema it can convert DetailedSchema to ScriptData and then build NoSchema one.

@erikd
Copy link
Contributor

erikd commented Oct 1, 2021

Have taken the "Fix foreign key" commit in #864.

Closing this.

@erikd erikd closed this Oct 1, 2021
@erikd erikd deleted the kderme/fix-foreign-key branch January 25, 2022 04:52
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.

3 participants