APIv2 proposal

[bddap]

Regarding the wallet API. I'm working on an update that uses jsonrpc.
The new API will be less work to maintain as it will be automatically generated using a proceedural
macro. The jsonrpc api will be isomophic to the rust api so documentation can be shared between the two.

Readers are encouraged to provide feedback.

Proposed changes

1. Edit APIOwner and APIForeign methods to accept and return owned values.

• Mutable out parmeters don't work over the network.
• Mixing reference args with owned args makes api generation via macro more complex.

2. Add member to the grin workspace, jsonrpc_derive. jsonrpc_derive will provide a macro to convert
   a trait definition into a jsonrpc api.
3. Convert APIOwner and APIForeign to traits. Split implementaions into new structs.

• The macro will take the trait definitions as input. This can possibly be done as a #[derive()].
  Trait definitions will contain api documentation including doctests.
  Implementaions may contain implementaion specific documentaion. Implementaions
  may define additional methods not accessable to jsonrpc clients.

Future steps

• Work out an authentication method for Owner API (probably keep using http with basic auth and an api key)
• Add command line options to the wallet to activate APIv2
• Automatically generate a jsonrpc client for exposed methods

External crates

There are at least two existing crates that attempt to provide the functionality needed. As of now, neither of them
matches our needs.

• https://github.com/paritytech/jsonrpc
• https://github.com/google/tarpc

API generation utilities provided by the first crate are not ready for production IMO. The second crate does not target stable rust.

Consideration on error handling

We should to condider how this api should report errors. I can think of several
options.

1. All procedures return a Result which is serialzed using serde. This could
   be a security concern as structured errors may leak sensitive data to api clients.
2. All procedures return a Result which is not serialized; instead it is
   reported as an opaque "Internal Error".
3. The jsonrpc 2.0 spec provides a mechanism for reporting internal errors
   https://www.jsonrpc.org/specification#error_object
   Use of this mechanism has pros and cons:
   pros:
   - Conforms to jsonrpc user expectations
   - More easily consumable by non-rust clients
   cons:
   - According to the spec, each error type must have an associated number id.
   Assigning ids will likely be a manual process.
   Jsonrpc errors MAY include a structured "data" field. Internal errors
   would be serialized into the field using serde. As with option 1, detailed
   error messages could leak sensitive information.
4. Use jsonrpc error reporting, but report iternal errors as an opaque
   "Internal Error".

Request for feedback

If anyone wants to comment on these plans, please do.

I intend to start a WIP pr if this proposal is well accepted.

Previous issue: https://github.com/mimblewimble/grin/issues/2048

[bddap]

@yeastplume @ignopeverell

[yeastplume]

Looks excellent, exactly the direction I was imagining going in. I had the same process of elimination you had when looking into existing options, they were either too heavyweight, incomplete, or unsuitable for some reason or another. A small, targeted lib to generate stubs from our existing APIs is definitely the best way to go here.

I'd probably use option 3 for error reporting. The errors need to be as clear as possible and properly propagated out from the deepest parts of the system. I'd imagine that the majority of clients would be non-rust.

We probably want to provide a client lib in js and/or another target language to go alongside this (that's another issue). And I think we also want to rewrite the wallet CLI to use the generated API (and that's a further issue).

Overall, great. Let me know if you need any questions or further help with this

[0xmichalis]

@yeastplume I guess this issue can be closed now and more specific issues opened in grin-wallet?

[quentinlesceller]

Closing. Feel free to ping me if you'd like me to reopen it.