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

Monero OpenRPC Specification #9186

Open
kilianmh opened this issue Feb 20, 2024 · 17 comments
Open

Monero OpenRPC Specification #9186

kilianmh opened this issue Feb 20, 2024 · 17 comments

Comments

@kilianmh
Copy link

OpenRPC is a (human and) machine-readable, programming language-agnostic JSON-RPC API interface description standard.

Having such a document could facilitate e.g. specification driven development, interactive documentation, code generation (documentation / clients), and automation of test cases.

Here is the post on bounties.monero.

Do you guys think such a specification could/should be part of the of this repository, another monero-project repository, or an external repository?

@SamsungGalaxyPlayer
Copy link
Collaborator

@kilianmh you can use https://github.com/MAGICGrants/getmonero.dev if you want

@vtnerd
Copy link
Contributor

vtnerd commented Feb 21, 2024

As an FYI, I tried to get the new serialization system I wrote to auto-generate schemas. I failed to get it working, without changing the interface a bit. Perhaps if the new serialization gets merged in (its moving very slowly), I could have some cycles on how to add this with minimal overhead (the overhead was always the problem with implementing it in the past).

For those curious, instead of writing the entire schema "by hand", this would auto-generate the initial process, with the exception of the "description" fields.

@vtnerd
Copy link
Contributor

vtnerd commented Feb 21, 2024

And to clarify - I think having the schemas would be useful, but writing one for all JSON-RPC endpoints would take a while. Certainly a big effort. I'm curious what bitcoind does, because they seem to have nice documentation with their endpoints.

@kilianmh
Copy link
Author

@SamsungGalaxyPlayer getmonero.dev seems like a good option

@vtnerd

  • I'm not well versed with c++ so auto-generation from code is out of reach for me. The schema generation should probably be handled by a third-party library. However, the description fields are quite important, especially if we use the schema for generating documentation.
  • For now my plan was to write it "by hand" since changes/additions to the JSON-RPC are not that numerous (is that true?).
  • For Bitcoin I created the schema also by hand.

One more question: Should there be a unified schema or one forwallet-rpc and one for daemon-rpc?

@vtnerd
Copy link
Contributor

vtnerd commented Feb 22, 2024

We should have different schemes for wallet-rpc and daemon-rpc. The schemas don't change often, but the developers will have to be nudged to remember to update the schema when possible.

@jeffro256
Copy link
Contributor

One of the biggest problems with the RPC documentation right now is that it is pretty outdated, and there is no mechanism to enforce that us lazy developers update the documentation (and it's in a different repo anyways). Perhaps a CI action could be added that checks WALLET_RPC_VERSION_{MAJOR/MINOR} and CORE_RPC_VERSION_{MAJOR/MINOR} against some internal version field of the corresponding OpenRPC specification. It would be nice to have a testable RPC documentation, especially for the wallet, for the upcoming Seraphis migration.

@SyntheticBird45
Copy link

hi, plowsof notified me that this issue exist. Hopefully vtnerd will be able to get his serlization system working. I'm trying to write it by hand at the moment in this repo: https://github.com/SyntheticBird45/monero-open-rpc. I plan to first write the daemon rpc, test it and verify what is outdated/deprecated. I'll then jump on the wallet rpc.

@SyntheticBird45
Copy link

I've finished first iteration of the core RPC openRPC document: https://github.com/SyntheticBird45/monero-open-rpc/blob/main/daemon/openrpc.json. To see, copy paste it in https://playground.open-rpc.org.

The document only contains the JSON-RPC interface and do not integrate nor document other /rpc methods. Since it isn't OpenAPI, there are no native way to express these endpoints. The servers field could be used.

For methods that could send JSON field in epee encoding format, the schema show a string, but description is precising that this is binary form.

A fair number of field description are not provided since I don't understand what are their meanings. Looking through monerod's core_rpc_server.cpp is time-consuming but I plan on make another trip and add more descriptions to the components/schema section.

I would like some directive on the other /rpc methods topic before jumping on the wallet RPC document

@SyntheticBird45
Copy link

SyntheticBird45 commented Feb 29, 2024

Perhaps a CI action could be added that checks WALLET_RPC_VERSION_{MAJOR/MINOR} and CORE_RPC_VERSION_{MAJOR/MINOR} against some internal version field of the corresponding OpenRPC specification.

Added :

"x-wallet-rpc-version": {
	"major":1,
	"minor":27
}

to wallet/openrpc.json. and:

"x-core-rpc-version": {
	"major":3,
	"minor":13
}

to daemon/openrpc.json

@plowsof
Copy link
Contributor

plowsof commented Mar 1, 2024

One of the biggest problems with the RPC documentation right now is that it is pretty outdated, and there is no mechanism to enforce that us lazy developers update the documentation (and it's in a different repo anyways). Perhaps a CI action could be added that checks WALLET_RPC_VERSION_{MAJOR/MINOR} and CORE_RPC_VERSION_{MAJOR/MINOR} against some internal version field of the corresponding OpenRPC specification. It would be nice to have a testable RPC documentation, especially for the wallet, for the upcoming Seraphis migration.

I went through the RPC docs a ~year ago and updated pretty much everything.. apart from a few occurrences of ringsize:7 they are kept up to date now. Automation ofcourse is preferred, but i / others attend to any issue made on -site reg docs.

@SyntheticBird45
Copy link

I went through the RPC docs a ~year ago and updated pretty much everything.

For reference, here what I found out of date in the core RPC:

  • get_connections lacked in his response field connections the fields address_type, rpc_credits_per_hash and rpc_port
  • get_version lacked in his response the fields current_height and hard_forks array (as well as its underlying object)
  • get_output_distribution lacked in his parameters the field binary (for disabling epee encoding) and compress (for disabling compression)

There are also methods like sync_info that did not contain in their outputs the top_hash, credits or untrusted fields.

@kilianmh
Copy link
Author

kilianmh commented Mar 1, 2024

@SyntheticBird45 Great to see what you did so far!
One improvement could be to use references for recurring parameters where all fields are identical (including "description" and "required"). An example would be fill_pow_hash.

@kilianmh
Copy link
Author

kilianmh commented Mar 1, 2024

One general question to the collaborators: Was/is there already an issue with the topic of replacing the other /rpc methods with conforming JSON-RPC methods? If no, then do you think such an issue should be created?

@kilianmh
Copy link
Author

kilianmh commented Mar 2, 2024

@SyntheticBird45 Also some methods use the paramStructure by-position instead of by-name. Examples are on_get_block_hash, and submit_block, but there might be more.

@yamabiiko
Copy link

One general question to the collaborators: Was/is there already an issue with the topic of replacing the other /rpc methods with conforming JSON-RPC methods? If no, then do you think such an issue should be created?

I also wonder this, as I am currently working on Cuprate's RPC server and only having JSON-RPC methods would be cleaner

@jeffro256
Copy link
Contributor

We could add those JSONRPC methods to match the behavior of the /* endpoints, but we shouldn't remove the /* endpoints.

@vtnerd
Copy link
Contributor

vtnerd commented Mar 16, 2024

A quick counter-argument to JSON-RPC: DOMless parsing is faster in the REST-style endpoints compared to the JSON-RPC endpoints. The problem is that the params field either has to be skipped then re-parsed on a second pass, OR it has to be put into a DOM (the very thing trying to be removed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants