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

bt-rpc encoding for wallet3 #1477

Merged
merged 0 commits into from Nov 30, 2021
Merged

bt-rpc encoding for wallet3 #1477

merged 0 commits into from Nov 30, 2021

Conversation

jagerman
Copy link
Member

@jagerman jagerman commented Aug 5, 2021

This removes the rigidity from RPC request responses, making it easier to return useful constructs that the current epee code doesn't support (like lists of lists), and also adds support for bt-encoded RPC requests (intended to eventually replace the ".bin" endpoints with Monero NIH binary encoding).

The bt-encoding, in particular, is aimed at more efficient wallet3 rpc interactions with a daemon.

Instead endpoints now set a nlohmann::json response object with whatever arbitrary values they want without having to add a bunch of limited epee serialization macro hell.

So, for example, to set the value "foo" to 20, you use:

rpc.response["height"] = 20;

Binary values (for things like hashes) are handled by going through a proxy object:

rpc.response_hex["hash"] = hash; // hash is a crypto::hash

which, for json, is equivalent to:

rpc.response["hash"] = tools::type_to_hex(hash);

but when the response is to be bt-encoded it leaves it as binary for more efficient transfers.

There is also a response_b64 that converts json-destined values to base64 instead of hex (and again leaves bt-destined values as binary).

Parsing of incoming requests now moves to a new core_rpc_server_command_parser file, which does free-form parsing from a
nlohmann::json or bt_dict_consumer, and has various templated helper functions to make this easier.

This is preliminary: this builds the basic infrastucture for handling requests, and converts three endpoints:

  • get_info
  • get_height
  • ons_resolve

Currently only make daemon builds (there is code in the wallet that hasn't been updated yet), and the other RPC interfaces are disabled by this commit (until they get converted to the new style).

To make a JSON request:

curl -sSX POST http://localhost:22023/json_rpc -d '{"jsonrpc":"2.0","id":"0","method":"get_info"}' | jq .

or:

./utils/lmq-rpc.py ipc://$HOME/.oxen/oxend.sock rpc.get_info '{}' | jq .

bt-encoded requests are autodetected when you make an OMQ request giving it a bt-encoded dict, such as:

./utils/lmq-rpc.py ipc://$HOME/.oxen/oxend.sock rpc.get_info 'de'

Part of the conversion here is also aimed at being able to easily generate more consistent RPC API documentation as, in the changes so far, every field is documented using machine-parseable doxygen style comments.

TODO (this list is almost certainly incomplete):
[ ] finish implementing the remaining json endpoints, both parsing and result production
[ ] implement bootstrapping; currently it is commented out
[ ] integrate automated RPC descriptions, probably using doxygen
[ ] devise replacements for each of the .bin endpoints that use the new response_b64 interface to set values. We likely can't get rid of the .bin endpoints right away (the current wallet uses them) but if we have replacements we can mark them deprecated and remove them once wallet3 is ready to completely replace wallet2.

Wallet3 will likely also need some endpoints that are more stream oriented so that it can receive things faster without incurring network latency waiting for requests, but those changes can come in another PR.


res.immutable_height = 0;
uint64_t immutable_height = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

possibly unused?

src/rpc/core_rpc_server.cpp Outdated Show resolved Hide resolved
src/rpc/core_rpc_server.cpp Outdated Show resolved Hide resolved
src/rpc/core_rpc_server.cpp Outdated Show resolved Hide resolved
///
/// Output values available from a restricted/admin RPC endpoint:
///
/// \p status General RPC status string. `"OK"` means everything looks good.
Copy link
Member Author

@jagerman jagerman Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add a - on the beginning of each of these so that doxygen will pick them up as list items. I think. (Although it's also possible that we just want \p everywhere; depends on how doxygen handles these).

src/rpc/core_rpc_server.cpp Outdated Show resolved Hide resolved
src/rpc/core_rpc_server.cpp Outdated Show resolved Hide resolved
@jagerman
Copy link
Member Author

Tom accidentally merged this by rebasing wallet3 onto it (when it was already based on wallet3). Did it still need updates?

jagerman added a commit to jagerman/loki that referenced this pull request Nov 30, 2021
The wallet3 rebasing accidentally merged bt-rpc (PR oxen-io#1477) on github,
though there were still some action items.

This PR is to continue it.
@jagerman jagerman mentioned this pull request Nov 30, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants