Open morleyzhi opened 5 years ago
The developer story here is: a developer reads the XDR definition, writes the transaction / asset / whatever in JSON, then is able to convert it to an XDR.
@morleyzhi Can you unpack this story a bit? Where is this JSON generated? Is it transmitted on a wire and then needs to turn into an XDR? Is the developer directly editing this JSON?
While JSON is clearly more human readable then XDR, it's still usually generated and consumed by computer programs. Perhaps instead of introducing a new JSON<->XDR mapping we can improve the generated XDR javascript classes by adding the type of accessors and mutators that are missing in our stories.
If it is meant for humans to interact with, maybe we can use TxREP (SEP11) as is? I'm not a huge fan of the grammar (would prefer using something like YAML) but it's better to have just one standard mapping.
@bartekn Can you expand what the use case for this feature is? I haven't seen the exact community request that you're relaying.
FWIW as I've said in other tickets, I'm leaning towards not introducing more than one way to solve a problem, so if it's just a matter of giving developers a different way of assembling transactions, I'd opt to not do it.
While JSON is clearly more human readable then XDR, it's still usually generated and consumed by computer programs.
I don't agree with this statement. In JS, JSON is actually a native representation of objects and is used quite often.
The biggest problem we need to solve is that a lot of boilerplate code is needed to create more complicated XDR objects. Check union example.
If it is meant for humans to interact with, maybe we can use TxREP (SEP11) as is?
FWIW as I've said in other tickets, I'm leaning towards not introducing more than one way to solve a problem, so if it's just a matter of giving developers a different way of assembling transactions, I'd opt to not do it.
It's not about transactions only. If you want to do any other task related to XDR (ex. decode Ledger contents encoded in XDR) it's actually really hard to do using existing js-xdr
APIs.
In JS, JSON is actually a native representation of objects
I don't agree with that statement :) JSON doesn't capture the prototype functions (or class system, whatever JS people have decided on) which are heavily used in the current way that js-xdr
generates these objects.
Looking at your union example I think I understand the problem better now and indeed it's not pretty. It seems like our generated xdr objects conflate the marshalling/unmarshalling code with the actual data ("OOP"). If there was a clear separation, it should be trivial to use simple JSON as input or output.
Does this make any sense? It seems like it will require a non-trivial amount of work to make that change though. @bartekn how important would you say is it?
I don't agree with that statement :) JSON doesn't capture the prototype functions (or class system, whatever JS people have decided on) which are heavily used in the current way that
js-xdr
generates these objects.
I agree with what you've said 😉. What I was trying to say is that JSON is native to JS: you don't need to import a package or library to be able to marshal/unmarshal/traverse JSON, it's part of a language syntax. Because of this JS devs often use JSON syntax to create objects. Obviously if you want to add some behaviour to the object JSON is not enough but XDR is only used to transfer data.
I think my main problem with the current API is that you need to create objects bottom-up. TransactionBuilder
in js-stellar-base is a good example. If you want to create a TransactionEnvelope
you need to first create the leaf-objects of this structure (memo, operations, time bounds, etc.) and then add them to the higher level object. This is especially awkward when you want to create a union (see create_account
operation code).
If it was possible to create XDR objects directly from JSON we could remove 50% of the code in js-stellar-base (the code responsible for building XDRs). Seriously.
It seems like it will require a non-trivial amount of work to make that change though. @bartekn how important would you say is it?
This is the issue I've been hearing very often from devs in our community. Some of them even created their own XDR packages to make this easier.
When it comes to difficulty of this task, XDR->JSON conversion is already done in XDR Viewer but instead of JSON we generate a HTML table. When it comes to JSON->XDR it will be probably some DFS-like algorithm. Maybe it's not trivial but it's also not an epic task, imho.
I think adding JSON -> XDR and XDR -> JSON converters to this library will greatly improve developer ergonomics.
@bartekn brought up a good example of how the current approach for creating XDR is awkward in JS in the union example. @vcarl also points out how awkward reading XDR in JS is in https://github.com/stellar/js-stellar-sdk/issues/209:
StellarSdk.xdr.TransactionResult.fromXDR(result.result_xdr, 'base64').result().value()[0].value().value().success().offer().switch().name
Instead, if the result were translated to JSON, I believe it could be as simple as:
result[0].success.offer.name
You can see more examples of how rough it is to read from XDR in JS in https://github.com/stellar/js-stellar-sdk/pull/247.
@bartekn @tomerweller @morleyzhi @tomquisel folks, could you give me your 5 cents on this? https://github.com/abuiles/json-xdr/issues/5 it tries to answers the question to How do you represent XDR union?
Broadly, this allows a developer to compose XDR representations in JSON and convert them into real XDR objects (and vice versa).
The developer story here is: a developer reads the XDR definition, writes the transaction / asset / whatever in JSON, then is able to convert it to an XDR.
For example, given the following XDR definition of
Transaction
:The JSON would look like:
This example is for
Transaction
object but in general you should be able to do the same thing with any XDR object, you check it's definition, write JSON and you can quickly convert it to actual XDR representation.There are some things to figure out:
An obvious way is to use union switch name
type
. So this would become:But maybe there's a better way? SEP-0011 is responsible for very similar thing but is using a different format than JSON so you can check it out.
AccountID
type (sourceAccount
inTransaction
) isPublicKey
which is really:Would be great to have a helper methods to quickly create byte arrays, ex:
Originally posted by @bartekn in https://github.com/stellar/js-stellar-base/pull/157#issuecomment-462704769