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

Should we stick to the JSON-RPC specification? #52

Open
rbrunner7 opened this issue Mar 12, 2023 · 8 comments
Open

Should we stick to the JSON-RPC specification? #52

rbrunner7 opened this issue Mar 12, 2023 · 8 comments

Comments

@rbrunner7
Copy link
Member

In our Matrix room we recently brainstormed about the wallet RPC interface, about things like we may want to make it possible to handle any number of concurrently open wallets over that and what this extension could mean about the interface and its design. Or the idea to offer a wallet interface that is based on JSON in a much more fundamental way than today, with HTTP / HTTPS only one possible way out of possibly several how to interact with a wallet using JSON.

In this context I came up with the following question:

Currently, Monero RPC does not shuttle back and forth any freely / custom designed JSON. It implements a specification called JSON-RPC that is e.g. described here

Should we continue to stick to that? E.g. in the interest of people who use third-party libraries implementing that spec? Or can we consider ourselves free to model our request JSON and answer JSON structures as we want and need them? Just a small example to illustrate: Instead of identifying a requested method with one simple string:

{"jsonrpc": "2.0", "method": "echo", "params": ["hello world"], "id": 0}

could we do something like (quick-and-dirty example, don't read too much into it):

{"monerorpc": "1.0", "command": { "interface": "multisig_wallet", "call": "get_info", "wallet_handle": 1234 }, ...

So far I suspect that following this spec doesn't have a really high "value", and that we wouldn't break much if we moved away from that, but I am unsure.

We could of course continue to follow the spec in a quite superficial way by implementing only a single method in the sense of JSON-RPC, called simply execute or do or similar, and then put any "extended" command info into a first parameter, but that looks a bit silly, IMHO.

@hinto-janai
Copy link

hinto-janai commented Mar 24, 2023

we wouldn't break much if we moved away from that

All code relying on the current JSON-RPC format would break, no?

I would think sticking to a known format with a spec (whether JSON-RPC or not) instead of creating a proprietary one would be beneficial for anyone building on top.

@rbrunner7
Copy link
Member Author

All code relying on the current JSON-RPC format would break, no?

Yes, sure. But that code may break anyway because the names of the calls, their parameters and their return values will probably differ.

@hbs
Copy link

hbs commented Mar 24, 2023

Does the current implementation of the JSON-RPC server support batching? This would be the real value of using JSON-RPC, i.e. having a "standard" way of batching requests which may be supported by JSON-RPC client libraries.

@rbrunner7
Copy link
Member Author

Does the current implementation of the JSON-RPC server support batching?

Not sure, but I don't think so.

@hinto-janai
Copy link

JSON-RPC is pretty much the de-facto standard across other projects (for better or for worse) which means there are already existing implementations for many languages that are performant/feature-rich.

Anyone (now, or future) who needs to interface with RPC benefits from not having to create this themselves, and can also use existing documentation.

The benefits of creating a specification that suits Monero's needs would have to be weighted against losing compatibility with all existing JSON-RPC impls, and the burden of maintaining its (presumably single C++) implementation and documentation. Although, if the flexibility gained from this is great enough, it may be worth breaking spec. It would be nice to see at least a couple implementations out the box though (willing to help here).

A question I have: is this only for the wallet? Presumably changing the daemon is outside the scope of this change? So both JSON-RPC and this new monero-rpc would be used?

@sanderfoobar
Copy link

sanderfoobar commented Dec 4, 2023

Consider skipping JSON-RPC and implement a websocket service instead. We can benefit from asynchronous communication so that we can receive updates like "money received" and "blockheight incremented" signals without polling like it is 2005.

@hbs
Copy link

hbs commented Dec 4, 2023

While websockets are nice this approach would not be ideal for applications which only occasionally interact with an RPC wallet as they would need to refactor the way they connect to support websockets instead of single request/reply interactions.

Adding a websocket to allow pipelining/streaming of interactions is rather independent from the actual format of those interactions (eg JSON-RPC messages as of now), so any replacement of JSON-RPC should be done with compatibility with a websocket approach in mind rather than limiting access to websockets.

@MrCyjaneK
Copy link

While I do agree with @sanderfoobar that it is nice, I also think that is is critical to break as few production applications as possible, I would be very happy to move to websocket, but I can imagine that if somebody already went and implemented JSON-RPC client for their site (like I imagine is the case for almost all custom payment boxes I've interacted with) they wouldn't be happy to do an entire rewrite of their logic using entirely separate stack (I believe we all have seen PHP cron script that looped over all invoices to check their status). This could possibly result in Monero getting delisted as payment option simply because the development cost of implementing something new would have been larger than a sum of Monero payments in last couple of weeks for some sites.

Yes, sure. But that code may break anyway because the names of the calls, their parameters and their return values will probably differ.

I also think that it is important to break as few apps as possible during this, while obvious things such as regexes for wallet addresses would differ, JSON-RPC could remain pretty similar (especially for actions such as creating account, getting subaddress, checking balance etc.. - basic merchant functions).

Regardless of the decision made on here I'd be happy to help and move forward, if JSON-RPC would get dropped from the codebase, or would differ significantly I'd probably just go ahead and create a proxy server that would translate JSON-RPC calls to the new implementation (websocket or entirely different JSON-RPC).

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

No branches or pull requests

5 participants