search

stellar / rs-stellar-xdr

Rust lib for Stellar XDR.
Apache License 2.0
20 stars 27 forks source link

fix(json schema): manually implement generic types #349

Closed willemneal closed 7 months ago

willemneal commented 8 months ago

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

willemneal commented 8 months ago

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

leighmcculloch commented 8 months ago

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 commented 8 months ago

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

leighmcculloch commented 8 months ago

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

leighmcculloch commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 7 months ago

Closing as this has been merged into https://github.com/stellar/xdrgen/pull/193.