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

Render asset codes as strings in JSON #324

Merged
merged 10 commits into from
Dec 1, 2023
Merged

Render asset codes as strings in JSON #324

merged 10 commits into from
Dec 1, 2023

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Nov 23, 2023

What

Render asset codes in JSON as UTF-8 strings, where any bytes are not printable ASCII are escaped using the rules defined by https://docs.rs/escape-bytes.

Why

Valid asset codes are some subset of ASCII, but they're currently rendered as hex. It creates a poor and difficult experience when a developer is expecting to see their asset code in the JSON and instead see hex.

The codes are rendered to a subset of UTF-8 because JSON strings must be Unicode.

XDR defines its strings to be ASCII, but can otherwise hold any byte sequences, even invalid ASCII.

Because the data can be any byte sequence, escaping any non-printable ASCII characters means that the majority of valid and meaningful data will be human readable, while all data is still roundtrippable.

Another option would have been to only escape invalid UTF-8 byte sequences, however reproducing that behavior exactly the same in a variety of SDKs would be more challenging.

Discussion at https://discord.com/channels/897514728459468821/1179194134783868988

Close stellar/xdrgen#175

Follow ups

@leighmcculloch leighmcculloch requested review from graydon and removed request for dmkozh November 23, 2023 09:42
@leighmcculloch leighmcculloch marked this pull request as ready for review November 23, 2023 09:44
@leighmcculloch leighmcculloch added this pull request to the merge queue Nov 27, 2023
@leighmcculloch leighmcculloch removed this pull request from the merge queue due to a manual request Nov 27, 2023
@leighmcculloch leighmcculloch added this pull request to the merge queue Dec 1, 2023
Merged via the queue into main with commit 9e7c662 Dec 1, 2023
@leighmcculloch leighmcculloch deleted the xdrgen-i175 branch December 1, 2023 10:32
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.

Rust: Render asset codes as strings instead of hex
2 participants