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(json schema): manually implement generic types #349

Closed
wants to merge 5 commits into from

Conversation

willemneal
Copy link
Member

@willemneal willemneal commented Mar 4, 2024

This creates a JsonSchema impl for VecM, BytesM, Frame, and StringM and ensures that they are in-lined.

@willemneal willemneal changed the title fix: manually implement VecM fix(json schema): manually implement generic types Mar 5, 2024
@willemneal
Copy link
Member Author

@leighmcculloch I updated to use base64 since there doesn't seem to be hex support, but base64 is supported.

@leighmcculloch
Copy link
Member

Do you mean that json schemas in general don't support hex, or the lib we're using doesn't?

The JSON rendered from the xdr lib is already outputting hex today, not base64.

@willemneal
Copy link
Member Author

JSON schema the spec provides a "format" property for strings. Is hex for historical reasons? We can also allow for both.

@leighmcculloch
Copy link
Member

I think contentEncoding would be more appropriate than format, but yeah neither support hex in the specification.

@leighmcculloch
Copy link
Member

A similar issue is that strings generated are printable-ASCII escaped strings, not UTF-8 encoded. For example, the following is valid XDR for an ScVal::String:

AAAADgAAAARow2V5

It contains a four character string where the hex values are for those four characters:

$ echo -n 'AAAADgAAAARow2V5' | base64 -d | hexdump -C
00000000  00 00 00 0e 00 00 00 04  68 c3 65 79              |........h.ey|
0000000c

The second character is 0xc3 which is not valid UTF-8.

It decodes to the following JSON:

$ echo -n 'AAAADgAAAARow2V5' | soroban lab xdr decode --type ScVal
{"string":"h\\xc3ey"}

@leighmcculloch
Copy link
Member

I think the above means the JSON generated can't be completely described by a JSON schema, but maybe that's okay. It can still be described sufficiently for a lot of uses? Applications will just need to figure out somehow that certain fields result in certain types of data?

Can we use a non-standard contentEncoding or format?

@leighmcculloch
Copy link
Member

Is hex for historical reasons?

Hex is because most binary values in the XDR are more commonly displayed as hex in UIs, things like hashes, or 32-byte IDs.

But a reason not to change it is this JSON is in use, so changing it would be breaking and potentially disruptive. It'd be best not to change the output unless there's a big win involved.

leighmcculloch added a commit to stellar/xdrgen that referenced this pull request Mar 7, 2024
Copied from 66a4913f1bd0de167ff50edd05c93dd958bec0eb
Copied from stellar/rs-stellar-xdr#349

Co-authored-by: Willem Wyndham <[email protected]>
@leighmcculloch
Copy link
Member

Closing as this has been merged into stellar/xdrgen#193.

leighmcculloch added a commit to stellar/xdrgen that referenced this pull request May 16, 2024
* Generate schemars derives in Rust generator

* update tsets

* move jsonschema behind own feature and add support for custom

* Add manual JsonSchema impls

Copied from 66a4913f1bd0de167ff50edd05c93dd958bec0eb
Copied from stellar/rs-stellar-xdr#349

Co-authored-by: Willem Wyndham <[email protected]>

* fix dead code

* simplified features

* gen for fixed opaque typedefs

* fix cas

* fix

* fix

* add type macro

* add simple methnod for generating json schema

* upd tests

* simplified the fns

* fix tests

* fix

* fix

* test

* fix

* defer adding macro that isn't required by this pr

---------

Co-authored-by: Willem Wyndham <[email protected]>
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.

2 participants