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

CLType of {"ByteArray":32} returned in deploy #1437

Closed
mcdee opened this issue May 12, 2021 · 2 comments
Closed

CLType of {"ByteArray":32} returned in deploy #1437

mcdee opened this issue May 12, 2021 · 2 comments

Comments

@mcdee
Copy link
Contributor

mcdee commented May 12, 2021

When querying the deploy with hash f81d0a32890b47a5dfbb55c7bffe75d2a9301c442f7857b1908409b8e9921e83 on mainnet returns a JSON structure with the following:

        "session": {
          "Transfer": {
            "args": [
              ...
              [
                "target",
                {
                  "cl_type": {
                    "ByteArray": 32
                  },
                  "bytes": "94664ce59fa2e6eb0cc69d270fc91dd9dd2ba02a1f7964c69f036ef4a68bb96f",
                  "parsed": "94664ce59fa2e6eb0cc69d270fc91dd9dd2ba02a1f7964c69f036ef4a68bb96f"
                }
              ],
              ...
            ]
          }
        },

however cl_type is meant to be an enum rather than an object. Given the byte array size can be inferred from the bytes field can this be altered so the cl_type is just a simple ByteArray value as per other CL types?

@Fraser999
Copy link
Collaborator

That's an interesting point! I agree it's possible, and might make the ergonomics slightly better for users consuming the JSON. However, there are a couple of factors at play here.

While it's true that many of the other variants of the CLType can be represented as a single JSON string field (the simple variants), it's not true that they all can. The complex variants can be comprised of nested CLTypes (except for ByteArray which contains the length of the array rather than a nested CLType). In JSON form, these complex variants need to be represented as JSON objects rather than strings, so ByteArray isn't unique in that regard.

A more practical problem with adopting the approach you suggest is that we currently just derive the Serialize and Deserialize traits for CLType. This gives us the ability to serialize to a binary format or to JSON or TOML for free.

In its binary format, the nested length of the ByteArray needs to be included so that the bytes comprising the data can be distinguished from possible other data. Normally the length is encoded as a prefix, followed by the number of bytes specified by that prefix, followed by any other encoded data.

So, on the one hand we need the Serialize and Deserialize implementations to use the nested length in ByteArray, but on the other (for JSON encoding), we want to skip it. This is possible: we could hand-write custom implementations for these traits rather than deriving them. However, the risk vs reward doesn't seem worthwhile to me in this case, given the complexity of not deriving the traits, and considering that this is only useful for single variant of the CLType.

I'll not close this issue for now so we can discuss further, but my current inclination is to not change from the status quo.

Thanks anyway for raising the issue. It's a point I hadn't considered up until now :)

@mcdee
Copy link
Contributor Author

mcdee commented May 26, 2021

Yeah, understand. If the types can be arbitrarily complex then objects are going to be required, it's just a pain to map JSON to an internal struct when the value can have different types (string or object).

Although it would be possible to build a full JSON representation of a CLType that was well-defined and had a single defined structure, I very much doubt that it is worth your time to do so. I'll patch something up in my parser instead.

Thanks for the detailed response.

@mcdee mcdee closed this as completed May 26, 2021
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

No branches or pull requests

2 participants