-
Notifications
You must be signed in to change notification settings - Fork 37.7k
doc: document json rpc endpoints #27225
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
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
Concept ACK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 86cff0e
Error is also helpful if someone is unaware of this and tries to use /
endpoint with 2 or more wallets for wallet based RPC commands.
{
"result": null,
"error": {
"code": -19,
"message": "Wallet file not specified (must request wallet RPC through /wallet/<filename> uri-path)."
},
"id": "test"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
concept ACK
1. `/` | ||
2. `/wallet/<walletname>/` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few ideas about these:
- could include full path
http://localhost:{port}/
- could list relevant ports per network (i.e. main=8332, regtest=18443 etc)
- not sure if this applies here but I'm used to seeing API endpoints with variables expressed like
/wallet/:walletname
, at least for REST APIs. I dunno if there is otherwise a convention for that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I feel it's clearer just to use the endpoints themselves, rather than cluttering with hosts and ports.
Also, we have examples now!
doc/JSON-RPC-interface.md
Outdated
|
||
### `/wallet/<walletname>/` endpoint | ||
|
||
This endpoint is only activated when the wallet component has been compiled in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is also always active for non-wallet requests before any wallets are loaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(but only when the wallet is compiled in).
Also I think i've covered that it handles both types of request when no wallet is loaded in the Quirks section?
86cff0e
to
36748e8
Compare
$ curl --user alice --data-binary '{"jsonrpc": "1.0", "id": "0", "method": "getblockcount", "params": []}' -H 'content-type: text/plain;' localhost:38332/ | ||
|
||
# Get balance from the /wallet/walletname endpoint when rpcuser=alice, rpcport=38332 and rpcwallet=desc-wallet | ||
$ curl --user alice --data-binary '{"jsonrpc": "1.0", "id": "0", "method": "getbalance", "params": []}' -H 'content-type: text/plain;' localhost:38332/wallet/desc-wallet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this gets merged before #27101, remind me to update to 2.0
;-)
Technically I think jsonrpc:"1.0"
isn't necessary and doesn't actually do anything.
Concept ACK |
Nit - there is typo in commit message / PR title (s/docment/document/). |
fixes bitcoin#20246 Document both JSON-RPC endpoints, when they are active and which types of requests they are able to service. Adds two example curl requests, one for each endpoint.
Thanks, i've updated this in 65e3abc |
36748e8
to
65e3abc
Compare
You should also edit PR title. It is used by github-merge.py. |
Thanks, I thought that updated from the force push, but apparently not. Now also done :) |
# Get block count from the / endpoint when rpcuser=alice and rpcport=38332 | ||
$ curl --user alice --data-binary '{"jsonrpc": "1.0", "id": "0", "method": "getblockcount", "params": []}' -H 'content-type: text/plain;' localhost:38332/ | ||
|
||
# Get balance from the /wallet/walletname endpoint when rpcuser=alice, rpcport=38332 and rpcwallet=desc-wallet | ||
$ curl --user alice --data-binary '{"jsonrpc": "1.0", "id": "0", "method": "getbalance", "params": []}' -H 'content-type: text/plain;' localhost:38332/wallet/desc-wallet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are the examples using signet port? I think we should use mainnet port (8332) same as all the RPC examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to be persuaded either way here, no particular reason for using signet (other than I was using it for testing). Although I personally don't think it matters much as the comment for each shows which variables are being used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 65e3abc - Seems fine. Can be improved if need be.
65e3abc doc: document json rpc endpoints (willcl-ark) Pull request description: fixes bitcoin#20246 This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks. ACKs for top commit: fanquake: ACK 65e3abc - Seems fine. Can be improved if need be. Tree-SHA512: d557c2caf000a1bdd7b46c9da38afe63dc28446ba4a961526f1af3cec81d994a9da663e02c81ebdc4f609b794585349cfca77a582dc1e788c120de1d3b4c7db6
fixes #20246
This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks.