[RPC] add possibility to extend the JSONRPC URI schemas

jonasschnelli commented at 12:58 pm on April 12, 2015: contributor

supersedes #5744

I recognized that this approach of extending RPC calls is more flexible and only needs a few lines of changes.

A new wallet could register for a URI (example: /wallet) and response to incoming JSON RPC calls. The signaling overhead should not affect the RPC overall performance. URI distinction could also help if it comes to process separation of core and wallet.

Working “example” will follow soon.

jonasschnelli commented at 1:02 pm on April 12, 2015: contributor

PS: bitcoin-cli is untouched for now. As discussed on IRC it could be possible to have something like bitcoin-cli -w getnewaddress (-w stands for –wallet which would send the commands to /wallet). Unclear for now is how it would handle json type conversion (minor issue IMO).

jgarzik commented at 3:09 pm on April 12, 2015: contributor

I thought about this long ago.

JSON-RPC is really built around a single endpoint through which all information is exchanged. It’s essentially a messaging tunnel. This PR changes that model, without apparent need.

It did not seem to make sense to add another level like this, when you can easily just add new methods. Convention “mining.$name” or “wallet.$name” can easily suffice, and registered services/plugins may be triggered there.

jonasschnelli force-pushed on Apr 12, 2015

jonasschnelli commented at 3:40 pm on April 12, 2015: contributor

Recently there was a discussion on IRC about this topic (gmaxwell, sipa, wumpus) http://bitcoinstats.com/irc/bitcoin-dev/logs/2015/04/10#l1428649319 It came out that a new url endpoint makes most sense.

The signaling in this PR hooks in after the JSON Parsing and before the JSONReply. It basically allows bypassing of the raw RPC function and will reuse the whole HTTP/RPC/JSON stack.

laanwj commented at 8:55 am on April 15, 2015: member

@jgarzik Right, that’s also direction I assumed we’d go in for namespacing, but @gmaxwell was arguing against that approach on IRC recently and preferred multiple endpoints.

An argument for multiple endpoints is that it makes it easier to transition to separate processes, as client software would support different URIs already.

sipa commented at 10:04 am on April 15, 2015: member

Multiple endpoints probably also makes having different authentication easier?

laanwj commented at 8:05 am on April 16, 2015: member

@sipa Yes. If requests are delegated based on base URI, different endpoints can handle authentication differently.

jonasschnelli force-pushed on Apr 16, 2015

jonasschnelli commented at 8:28 am on April 16, 2015: contributor

Slightly updated:

reduced to only one signal (ExtCmdExecute)
removed signaling while creating help message (not required when having a different entry point uri)
additional uri schemas can be added over a static function instead over signaling (while RPC server is not running to avoid locking)

sipa commented at 9:59 am on April 24, 2015: member

Hmm, I’d rather see a single mechanism, where registered modules register for a particular URI - whether that is the root or not?

jonasschnelli commented at 12:06 pm on April 24, 2015: contributor

I agree with @sipa. My goal with this PR was to not touch the current wallet implementation. Therefor i only allowed/added the signaling in non root URI spaces, in hope to get more reviews because its behavior is more save and understandable.

If – and i tried and will try again – the current wallet will become a module, we should completely modularize the JSON RPC request/response handling. But because most PR has lack of review and attention, i decided to leave the current implementation as untouched as possible and add a 2nd wallet where we have the freedom of experimenting.

Decoupling the PRC dispatch table would also be a option but looks after no interest:

#5934
#5744

There is also a commit who would modularize the whole wallet:

https://github.com/jonasschnelli/bitcoin/commit/15ccf479f190786d0fc97a0253bfdbd080074ff1 (except RPC, see PR links above).

sipa commented at 12:44 pm on April 24, 2015: member

Fair enough, I understand. If the plan is to merge the two URI interfaces later, I’m fine with it.

jonasschnelli commented at 12:49 pm on April 24, 2015: contributor

I know it somehow looks silly to add changes where there is already the plan (sometime already some code!) to change the changes. But in terms of sane change transitions and reviewability this can make sense.

sipa commented at 12:52 pm on April 24, 2015: member

I agree, and I’ve definitely used that strategy myself. But sometimes the longer plan isn’t clear from looking at the initial step.

jgarzik commented at 12:56 pm on April 24, 2015: contributor

Since consensus seems inclined towards integrating plugin/registration at the URI level, concept ACK

laanwj added the label RPC on May 6, 2015

jonasschnelli force-pushed on Jun 24, 2015

jonasschnelli commented at 7:11 am on June 24, 2015: contributor

Rebased and added proper locking for vExtURI.

[RPC] add possibility to extend the JSONRPC URI schemas f6465cb200

jonasschnelli force-pushed on Jul 6, 2015

jgarzik commented at 6:25 pm on September 15, 2015: contributor

ut ACK

jonasschnelli commented at 6:28 pm on September 15, 2015: contributor

With the current libevent httpd this does no longer work and requires a new concept. Closing for now.

jonasschnelli closed this on Sep 15, 2015

DrahtBot locked this on Sep 8, 2021

[RPC] add possibility to extend the JSONRPC URI schemas #6006