Skip to content

Migrate from GRPC to JSON RPC (with binary variant for higher performance) #495

Closed
12 of 12 issues completed
Closed
Migrate from GRPC to JSON RPC (with binary variant for higher performance)#495
12 of 12 issues completed
Assignees
CMCDragonkaitegefaulkes
Labels
developmentStandard developmentepicBig issue with multiple subissuesr&d:polykey:core activity 1Secret Vault Sharing and Secret History Managementr&d:polykey:core activity 3Peer to Peer Federated Hierarchy

Description

@CMCDragonkai
CMCDragonkai
opened on Nov 25, 2022

Specification

Due to various issues with GRPC, and a combined effort to replace both the networking in #234 and transport agnostic RPC #249, this epic covers all the problems we have with GRPC and changes we will need to do to the RPC handling architecture.

We want to go with JSON RPC because it is going to be the most suitable for third party integration.

We can have a binary protocol format for increased performance when this is detected. Candidates are:

  • BSON
  • CBOR
  • Protobuf
  • MessagePack

We need to choose one based on benchmarks, and UI/UX of converting to our internal datasets, reliability of third party libraries, portability of parsing and generating libraries and wide-deployment.

Additional context

See all sub issues under this epic.

Tasks

  1. Fix all subtasks under this epic.
  2. Once this epic is satisfactorily done and specced, close all irrelevant issues.
  3. Our migration should address all the problems we've had with GRPC and what we have been using GRPC for.

Activity

  1. CMCDragonkai
    added
    developmentStandard development
    epicBig issue with multiple subissues
    on Nov 25, 2022
  2. CMCDragonkai
    assigned
    tegefaulkes
    on Nov 25, 2022

  3. CMCDragonkai commented on Nov 25, 2022

    @CMCDragonkai
    CMCDragonkai
    on Nov 25, 2022
    MemberAuthor

    This now takes priority over all GRPC problems.

  4. CMCDragonkai commented on Dec 5, 2022

    @CMCDragonkai
    CMCDragonkai
    on Dec 5, 2022
    MemberAuthor

    Because we have pre-existing experience with protobuf. And we have a large corpus of protobuf definitions already. (Even though we will be reworking those with respect to JSON RPC) Then protobuf is probably a good choice for our binary alternative encoding.

    However the choice of protobuf library is important too.

    Our existing stack is pretty complex involving protoc, the protoc plugin for generating TS code, the google-protobufruntime library... etc.

    This all seemingly can be replaced with 1 library: https://github.com/bufbuild/protobuf-es.

  5. CMCDragonkai commented on Dec 5, 2022

    @CMCDragonkai
    CMCDragonkai
    on Dec 5, 2022
    MemberAuthor

    There are some things we need further specify above the JSON RPC spec: https://www.jsonrpc.org/specification. Because this is light on the details.

    Firstly if we were to use the params, then the params should always be an array in order to match our JS functions take parameters. Meaning f({a, b}, c) would be called with params: [{a, b}, c].

    Secondly the error structure doesn't quite match our serialiation atm. We can stuff our entire error serialisation into the data property, but then copy the first level message to the message (prefixed by the exception name), and also set a generic application-error code for the code. Unless it's one of the reserved errors.

    Furthermore, we have to consider what usage is the notifications, and batching and request objects in relation to our streaming usage as we are currently doing.

  6. CMCDragonkai commented on Dec 5, 2022

    @CMCDragonkai
    CMCDragonkai
    on Dec 5, 2022
    MemberAuthor

    The request object is an open spec. We could add additional properties to a request object, in order to cover the usage of metadata we have in GRPC. In GRPC there's leading and trailing metadata: #249 (comment)

    But if we do with the request object, we could actually add metadata to every single request object.

  7. CMCDragonkai commented on Dec 5, 2022

    @CMCDragonkai
    CMCDragonkai
    on Dec 5, 2022
    MemberAuthor

    The response object can also be extended with meta.

  8. CMCDragonkai commented on Dec 6, 2022

    @CMCDragonkai
    CMCDragonkai
    on Dec 6, 2022
    MemberAuthor

    Solving this problem should also involve bringing in rxjs properly.

  9. CMCDragonkai commented on Dec 6, 2022

    @CMCDragonkai
    CMCDragonkai
    on Dec 6, 2022
    MemberAuthor

    Still to spec out:

    1. Protobuf vs JSON muxing/demuxing, should these be separated per-stream? If so, how does one identify whether it is JSON or Protobuf in the first place, how to disambiguate?
    2. The addition of the meta property to all request objects and response objects for JSON RPC to enable passing meta, but this also means there's no such thing as "leading" or "trailing" metadata, every request/response can have metadata.
    3. The removal of "id" from requests/response objects thus everything is just a notification. The entire stream itself can have an ID though, and we do have stream IDs as well connection IDs. 1 connection ID - multiple stream IDs... and we end up not following the JSON RPC spec to the letter.

  10. CMCDragonkai commented on Jan 3, 2023

    @CMCDragonkai
    CMCDragonkai
    on Jan 3, 2023
    MemberAuthor

    I've recently created a WIP QUICStream in js-quic, details in this comment: MatrixAI/js-quic#1 (comment).

    Basically web streams doesn't have a native "duplex" stream. The only closest is the ReadableWritablePair.

    So the first order of business is to develop a function that maps;

    ReadableWritablePair<Uint8Array, Uint8Array>

    To

    ReadableWritablePair<JSON, JSON>

    Where JSON is the JSON RPC message format. Although we have some requirements regarding leading headers and trailing headers discussed in #249 (#249 (comment)).

  11. CMCDragonkai commented on Jan 3, 2023

    @CMCDragonkai
    CMCDragonkai
    on Jan 3, 2023
    MemberAuthor

    @tegefaulkes start prototyping this implementation with an new PR targetting this epic. I think having a new rpc directory as opposed to the grpc directory can be useful.

  12. CMCDragonkai commented on Feb 17, 2023

    @CMCDragonkai
    CMCDragonkai
    on Feb 17, 2023
    MemberAuthor

    For collection handlers see #237.

    For most handlers see #450.

  13. CMCDragonkai commented on Mar 21, 2023

    @CMCDragonkai
    CMCDragonkai
    on Mar 21, 2023
    MemberAuthor

    @tegefaulkes is this the larger epic? Please link all new issues created involving RPC migration here including the recent issues you created.

  14. tegefaulkes commented on Mar 24, 2023

    @tegefaulkes
    tegefaulkes
    on Mar 24, 2023
    Contributor

    I linked the new RPC issues to this epic.

  15. 38 remaining items

  16. CMCDragonkai commented on Oct 11, 2023

    @CMCDragonkai
    CMCDragonkai
    on Oct 11, 2023
    MemberAuthor

    This is done with rpc and ws integrated. No more GRPC is in PK.

  17. CMCDragonkai
    closed this as completedon Oct 11, 2023

  18. CMCDragonkai commented on Oct 11, 2023

    @CMCDragonkai
    CMCDragonkai
    on Oct 11, 2023
    MemberAuthor

    Note I removed the pagination issue out of this. Those are usability issues to be applied separately.

  19. CMCDragonkai
    added sub-issues on Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

developmentStandard developmentepicBig issue with multiple subissuesr&d:polykey:core activity 1Secret Vault Sharing and Secret History Managementr&d:polykey:core activity 3Peer to Peer Federated Hierarchy

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions