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

[CMCDragonkai]

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.

• WebSocket Behaviour Alignment with Transport Layer js-ws#3 tracking websocket related transport issues

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.

[CMCDragonkai]

This now takes priority over all GRPC problems.

[CMCDragonkai]

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.

[CMCDragonkai]

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.

[CMCDragonkai]

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.

[CMCDragonkai]

The response object can also be extended with meta.

[CMCDragonkai]

Solving this problem should also involve bringing in rxjs properly.

[CMCDragonkai]

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.

[CMCDragonkai]

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)).

[CMCDragonkai]

@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.

[CMCDragonkai]

For collection handlers see #237.

For most handlers see #450.

[CMCDragonkai]

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

[tegefaulkes]

I linked the new RPC issues to this epic.

[CMCDragonkai]

Also pending is single class files should be using default export. The migration of client handlers kept the GRPC style handlers. But with agent handler migration we should switch over properly.

[CMCDragonkai]

With the latest merges, the demos shouild still work right? CLI demo and agent to agent demo we sent to Mercury.

[CMCDragonkai]

I added MatrixAI/js-rpc#1 to this epic too.

[CMCDragonkai]

Just a clarification, do we actually have a binary protocol atm using protobuf? Or do we just switch from JSONRPC to raw binary stream such as during file upload?

@tegefaulkes can you expand on the relevant handlers and where this is applied.

[tegefaulkes]

We don't have a protocol per say. But we can send anything over the raw streams. That could be protobuf, BISON or whatever.

AFAIK the demo should still work. All the relevant tests are still working.

[CMCDragonkai]

Can you explain how this works in the context of file uploading? Explain in reference to one of the client service handlers. What happens during the streaming?

It should be client streaming right? Then initial message is JSON, then subsequently it's just chunks of the raw file being uploaded? How does one signal end of file? Just by ending stream? I need more clarity on this.

[tegefaulkes]

It's down to implementation of the handlers. but you'd include metadata in the header message and then just start writing the contents of the file to the stream. and then end the writable of the RPC stream to signal the end. You can if you wanted to build an extra layer of protocol over this. You will also have to deal with Endianness if you don't serialise the data with something like protobuf but thats a normal concern for raw binary streams.

[CMCDragonkai]

Ok so how does it currently work with respect to pk secrets create or pk secrets get?

What are the handlers currently implemented to do?

Are we dealing with endianess? Is it possible that the client side may be a different endianess compared to the server side? Like if we are on an intel machine and send it to a different kind of machine could that cause problems?

[tegefaulkes]

It's possible? I'm not sure, its definitely something to look out for.

As for the handlers. I think anything that would be sending data such as secrets is implemented pretty simply right now. create and get are sending the secrets contents in a binary string and not even streaming it. So It needs to be changed to do streaming.

[CMCDragonkai]

Ok so we need to change them to streaming and we also need to change them to using protobuf which at the very least would fix any issues with endianness.

This reminds me of the HTTP style uploads which switch from http request to multipart upload format.

In this sense we should do something very similar. Have a parameter that indicates the type of request.

[tegefaulkes]

New issue at MatrixAI/js-rpc#2.

[tegefaulkes]

I think this is what you wanted to be tagged in @CMCDragonkai

[CMCDragonkai]

Can #525 address this as well? It's one big QUIC integration plus RPC migration.

There's alot of subissues under this epic that we could address, but some of it can be pushed to later.

[tegefaulkes]

I could but I'd rather not 'kitchen sink' PRs.

[CMCDragonkai]

After some discussion regarding MatrixAI/js-rpc#2. It is now understood that we don't need to embed protobuf or cbor or some binary variant into our RPC.

The RPC can already just drop down to byte level streams (this functionality is tracked in MatrixAI/js-rpc#2). Once it is dropped to the binary level, it is up to the RPC caller and handlers to decide to use whatever data format they want. If they wish to use protobuf they can, if they wish to use CBOR they can. But the RPC system itself doesn't require this functionality by default.

There are 2 usecases for using Protobuf/CBOR:

1. When you need to chunk up binary data, and expect the other side to be able to stream process the chunks. This requires a message-framing capability, and so both sides are likely to use a stream generator and stream parser to work with them. The byte stream provided by the RPC system does not have any message framing capability in it, unless you consider 1 byte to be a message frame.
2. When you are working with mixed binary and structured data. So like structured messages that have somethings that is binary. However we are currently using base64url encoded data mostly. ATM, there are actually 2 binary encodings we use with JSON. One is the the Buffer.toJSON way, where we produce a { type: 'Buffer', data: [1, 2, 3] }, and the other is the base64url encoded data that is used as part of JOSE tokens. The former is used more for database operations, while the latter is currently only used for tokens.

For usecase 1, we should prefer to use protobuf or cbor, and not mix the usage. Protobuf has more wider acceptance, but CBOR has more support for the native types of JS. CBOR is likely to be used in the DB when MatrixAI/js-db#58 is done.

For usecase 2, for simplicity I believe we should migrate to using the base64url style rather than the Buffer.toJSON way, and then it should be standardised as part of our development documentation. It's lot more space-wise efficient, and is more expected for RPC calls.

[CMCDragonkai]

As per the discussion in MatrixAI/js-ws#3

Do note that there's quite a few levels of encapsulation here for websocket transport.

1. TCP/TLS - where MTLS is available
2. HTTP handshake - GET request, and 101 response
3. Websocket binary frames
4. Payload protocol - JSON-RPC
5. IF you end up dropping to binary raw stream, at this point JSON-RPC is the handshake, and you can now use protobuf or cbor or whatever.

Error management should be specified too.

[CMCDragonkai]

There should be tests for uploading and downloading files using the raw stream from Polykey client to Polykey agent, and then synchronising those vaults to agent to agent.

In both hops, they should be dropping down to raw streams.

[CMCDragonkai]

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

[CMCDragonkai]

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