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

Fix render of AssetCode12 to JSON when shorter than 5 chars #358

Merged
merged 2 commits into from
Apr 9, 2024

Conversation

leighmcculloch
Copy link
Member

@leighmcculloch leighmcculloch commented Apr 5, 2024

What

Change render of AssetCode12 to JSON to always output at least 5 chars and show escaped zeros for any missing.

Why

AssetCode12's that have less than 5 characters are not supported by the Stellar network. However, they can be encoded in XDR which means they can be rendered to JSON. There's no pubnet usage of this because they are rejected being invalid, but it has been noted in SEP-11 that such cases can exist for testing invalid data. In any case this is one stand out place that the round trip encoding of asset codes is somewhat buggy and is worth fixing.

Today the AssetCode12 when encoded to JSON renders itself to a string stopping at the last sequential zero from the end meaning it can render itself as ABC, or AB, or A, or even an empty string "" if it is all zeroes. When an AssetCode12 has been wrapped in an AssetCode and is then decoded it is reinterpreted as an AssetCode4 because the length is equal or less than four.

For the sake of preserving the round-trippable guarantees of the XDR <> JSON conversions, this is worth fixing.

This is a breaking change to the XDR<>JSON conversion, but it is a bug fix, and any AssetCode12 already encoded using this function would never have decoded correctly anyway, so this change can be included in a patch or minor release if the opportunity arises or it is deemed necessary.

Close #356

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@leighmcculloch leighmcculloch added this pull request to the merge queue Apr 9, 2024
Merged via the queue into main with commit 31923a1 Apr 9, 2024
10 checks passed
@leighmcculloch leighmcculloch deleted the i356 branch April 9, 2024 16:23
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.

Invalid asset codes do not round trip from JSON<>XDR
2 participants