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

transition vault.proto to rev3? #3

Open
drew-512 opened this issue Oct 18, 2020 · 4 comments
Open

transition vault.proto to rev3? #3

drew-512 opened this issue Oct 18, 2020 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@drew-512
Copy link
Member

With the anticipated roadmap of plan-vault-libp2p consuming repo.proto in the long run, I propose to do I rev on vault.proto that narrows the gap a little more. Namely: instead of FeedMsg used for messages both ways, it becomes more specialized. Modeled after repo.proto:

FeedReq is what a client sends the vault, and

FeedMsg becomes what the vault sends in response.

As you see in repo.proto, this lets each become a lil more specialized making things more self-documenting.

In the end, it's not much of change, but I wanted to raise it since @tgross will be focusing on the libp2p of the vault.

@drew-512 drew-512 self-assigned this Oct 18, 2020
@drew-512 drew-512 added the enhancement New feature or request label Oct 18, 2020
@drew-512
Copy link
Member Author

While we're talking about vault.proto revs, there were two other areas of interest (that could warrant us having a meeting about)...

In short, it's adopting the TID concept as a way to name/identify Feed entries. Aren't they just hash IDs? Well yes, but they also have a time-based prefix, allowing the handlers of these opaque objects to handle, sort, and prioritize their storage and access just by having the TID in hand. As you can imagine, a vault (and pnode) can manage large collections of (opaque) TID blobs WAY more effectively and intelligently. A TID for each object also makes a common standard for entry naming, allowing vaults to address a specific stream starting point

The extended version of what's at hand here is going route of having a header object (where a feed msg blob is accompanied by a simple pb header). This would give us a place to put the TID and other helpful data. For example, we could opt to also have a ParentTID and ChildTID would could be a way for the vault to track/follow feed stream forks. I personally am a fan of that path (as it's basically free and doesn't even have to be used by the vault). It also future proofs the vault with an open-ended header.

@tgross
Copy link
Collaborator

tgross commented Oct 18, 2020

With the anticipated roadmap of plan-vault-libp2p consuming repo.proto in the long run, I propose to do I rev on vault.proto that narrows the gap a little more. Namely: instead of FeedMsg used for messages both ways, it becomes more specialized. Modeled after repo.proto:

Sounds good to me.

In short, it's adopting the TID concept as a way to name/identify Feed entries. Aren't they just hash IDs? Well yes, but they also have a time-based prefix, allowing the handlers of these opaque objects to handle, sort, and prioritize their storage and access just by having the TID in hand.

I'm game for this as well, including the extension into a header. With the caveat that you should not assume you'll have a useful time prefix between vaults.

While we're at it, another vault proto change I'd recommend is not serializing the OpenFeedReq to a byte slice that we stuff into MsgData:

  • Deserializing it out of the MsgData only if the FeedMsgOp is set is a second pass through deserialization. This moves error handling for corrupt data away from the "boundary" where we initially get the proto.
  • Fields are optional by default.
  • The default-value for a pointer/message field is safely meaningless (i.e. it's not an int where the default 0 could be confused with data).
  • An unused field is 0-bytes on the wire.

We could use oneof for OpenFeedReq and MsgData (or some collection of different message body types).

@drew-512
Copy link
Member Author

While we're at it, another vault proto change I'd recommend is not serializing the OpenFeedReq to a byte slice that we stuff into MsgData:

Yeah, this is basically what I mean by creating a FeedReq message type (like in repo.proto). Big fan of pb b/c of it's awesomeness with default fields.

We could use oneof for OpenFeedReq and MsgData (or some collection of different message body types).

I am not a big fan of oneof for a couple reasons and just like to put in fields that are there or not. Not a oneof fan mainly b/c it causes overhead code for each target language to be inserted.

@tgross
Copy link
Collaborator

tgross commented Oct 18, 2020

Works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants