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

Read interface for new serialization system #8970

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vtnerd
Copy link
Contributor

@vtnerd vtnerd commented Aug 11, 2023

This is the read interface for my proposed serialization system, along with the remainder of the (major) adapters/wrappers that were not in the write interface review. After this review are the implementations (epee, json, msgpack), and then the changes needed to the existing code.

@jeffro256
Copy link
Contributor

What will basic_value be used for?

@vtnerd
Copy link
Contributor Author

vtnerd commented Oct 21, 2023

basic_value is used by JSON-RPC id field, and no one where else currently.

@vtnerd
Copy link
Contributor Author

vtnerd commented Oct 27, 2023

@jeffro256 Earlier I mentioned protobuf, and I forgot completely that this interface cannot support reading of protobuf arrays (without some modifications to the interface). Write support should be possible (although I haven't tried to implement it yet). I've otherwise implemented reading/writing of everything else protobuf.

I think its better to introduce the necessary changes when/if protobuf is added as potential format. A couple of people have asked for it years ago, but its pointless unless we can auto-generate schemas from this interface (another thing I failed to figure out - at least yet).

Otherwise, epee_binary, json, msgpack are all implemented completely, and CBOR should be possible as well.


Reading Protobuf Arrays

Reading protobuf arrays is difficult because a field within an object appears multiple times, and does not have to be contiguous. The binary format looks something like:

  {"1": 100, "2": "string", "1": 300}

which specifies an integer array of length 2: [100, 300].

@vtnerd
Copy link
Contributor Author

vtnerd commented Jun 20, 2024

@jeffro256 I finally updated this PR. I thought it was done previously, but I didn't merge in the requested changes for the read interface.

Changes:

  • Rebased
  • Changes requested by @jeffro256
  • Add API support for reading integer/msgpack keys

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