-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Rippled API isn't compliant with the JSON RPC specification #3065
Comments
I believe that the issue you linked was resolved with #1907.
I think the rippled JSON-RPC API is actually compliant if you include
Unfortunately, it doesn't look like this is documented anywhere. (Perhaps something for @mDuo13 to have a look at.) |
Is this what you meant?
|
OK, I think you're saying that the rippled API is not compliant because it expects "id" and "jsonrpc" to be inside of the object in the |
I think you've just slipped up when writing your response? As I've written it above that'd be compliant, but looking here it seems that
is the current case? As stated here, the reference implementation written by the author of the JSON RPC 2.0 spec would reject this because extra fields in request objects aren't intended. |
It seems like that code path is only exercised when the commandline On the other hand, when calling the JSON-RPC API over HTTP, it's a different story. The example I posted earlier, with "id", "jsonrpc": "2.0", and "ripplerpc": "2.0" inside of the object in the |
hmmm, in that case you're right: if all but the method field are inside I recommend against putting
I'm unsure of the correct way to do versioning in JSON RPC. Perhaps you're supposed place it on a new endpoint? |
Existing clients would still need to be updated to be compatible. That being said a clean switch over (with no backwards compatibility between versions) would reduce code bloat and development complexity. Relevant docs would need to be updated of course.
I wrote an implementation a while back that permits extra 'header' fields, a feature I & others found useful in projects which depended on it (we used it for persistent 'session-id' & other authentication features). I understand the desire to maintain compatibility with various JSON-RPC implementations but as mentioned in the stackoverflow posted above, this behaviour (extra fields) is not defined in the spec and thus it technically is permitted |
I understand. It's also worth noting that JSON RPC 2.0 does not require that |
Oh dear ... so I tried using http://software.dzhuvinov.com/json-rpc-2.0-client.html
So I need to find an implementation of JSON-RPC 1.0? ... from 2005? sigh Any progress on this issue? Any idea when you'll support JSON-RPC 2.0? |
Nope ... I simply won't soil my modern Kotlin codebase with libraries from 15 years ago. I'm just gonna have to wait until you get this fixed. Starting down this path leads to the dark side - then I might as well abandon Event Sourcing and go back to relational databases from the 1970's (which is comparable to cars going from electric to coal engines) |
Well, cars went from electric to gas engines in the early 20th century... ;-) Are there actual errors you encounter because the JSON-RPC 2.0 spec isn't 100% followed? You could also try the new gRPC interface by the way, this should be better typed and more modern than JSON-RPC. |
Yes, I've heard of this mad try from the gas companies to fight the future ... also gas powered radios to keep the electric companies out of the households. Sorry again for the tone in my comments, but there's nothing that can get my blood up more than bad/legacy tech IT solutions. I've tried adding all the parameters discussed earlier in this thread, but all tries ended up with an HTTP 400. I'll see if I can get an elegant solution using gRPC 😌 👍 |
Is gRPC enabled on the public servers? |
Why public servers? Already in this issue it is stated that JSON-RPC 2.0 only works on command line, so you clearly have a local server running if you did your tests with the parameters discussed in this thread? |
I guess we're back to timelines then.
On the public servers ... I write client software to the ripple network, I have no benefit in running my own rippled server. |
Then you're in the wrong place I'm afraid, since this is the repository for the code of this server, not the contact point for the operations team at Ripple who are running some public servers. I'm sure you can find someone to run a server for you if you need such special configurations and don't want to run it yourself for whatever reason. The public servers operated by Ripple by the way are NOT intended (or fit...) to be used for production. You'll either need to operate your own or pay someone to do it for you anyways. |
Then I must admit I don't know how Ripple servers work. Quite embarrassing actually ... I've written quite an efficient trading robot, but don't understand the most basic concept about Rippled servers 🙃 |
https://xrpl.org/get-started-with-the-rippled-api.html#public-servers
The first sentence in the README of this repository sums it up nicely: "The XRP Ledger is a decentralized cryptographic ledger powered by a network of peer-to-peer servers." Peer-to-peer means that there are no differences between servers you run or that someone else runs, they are equals. You can just run a local instance, tell it to connect to the main network (it will even do so by default) and you'll be on your merry way. As this has nothing to do with the original issue any more though I just recommend you to read the documentation on https://xrpl.org in case there are more things unclear to you. |
You threw the book at me! |
Running rippled-1.5.0 on Debian Buster @ Scaleway Unfortunately the workaround @intelliot talked about doesn't work: Maybe if I downgrade to rippled version 1.3.1? |
An ambitious solution here might be to throw out both JSON-RPC 1.0 and gRPC ... and replace them with GraphQL. |
I definitely do NOT want GraphQL exclusively. Feel free to open an issue + PRs for a GraphQL interface in The following seems to work by the way (I don't know how a 100% valid response would look like):
or with ping:
Copy-paste into https://xrpl.org/websocket-api-tool.html#account_info and send the request. |
Thank you for your time finding a solution @MarkusTeufelberger |
Well ... I can see the writing on the wall ... this issue was opened almost a year ago and nothing has happened. |
Well, I got it to run on my local node now using the command line, with a correct RPC call and response (though it still requires adding that extra field that was already criticized in the original PR - which was merged about half a decade ago btw.). I also got it to work against |
I did a solution with a simple REST call. |
Yes, I managed with jsonrpc 2.0 (the calls look different than the ones I posted). |
Sure, looking forward to your contributions. |
Erhhh ... Ripple pays their developers, right? 😄 |
I guess? I'm not employed there. |
@SteelBRS just out of curiousity, why don't you implement the JSON 2.0 functionality that you desire and create a pull request? This is an open source project, so anyone can contribute. Or you could use the gRPC API. The gRPC API only has a subset of the functionality of the JSON API, but extending that functionality should be straightforward. The infrastructure is already there. |
I don't work for free. |
@cjcobb23 - JSON-RPC 2.0 was already implemented and added to GRPC functionality in rippled is only experimental as far as I know and doesn't seem to be actively extended to match the existing methods any further. Especially with RPC interfaces I would expect them to be used by people that are not comfortable coding in C++ by the way (otherwise they could just use libXRPL), so the "straightforward extension of functionality" might be a bigger task than you imagine. |
@MarkusTeufelberger good points. Just want to mention, we would welcome an extension to the gRPC API. The gRPC API is currently being used by Xpring to build SDK's in various languages; we implemented the minimal subset that Xpring needed for the SDKs. It would be ideal to continue to extend the gRPC API to match the existing methods; it's just a low priority task. However, the gRPC API is somewhat tangential to this issue. @SteelBRS - how you look at the work is your own perspective and I can't force a perspective upon you. However, if you need this functionality as part of a side project, you could consider the development as part of said side project. If you need this functionality for work you are doing for an employer, you could view implementing this functionality as part of your job, and implement during your normal work hours, that you are getting paid for. It's a matter of perspective though, and I'm not asserting this perspective I laid out is any more correct than your perspective. |
Closing as we have no specific plans to make changes here, but y'all please feel free to open a new issue with any particular requests or needs. |
Both JSON RPC 1.0 and 2.0 require that when a request invokes an error the "result" field is left empty and the error information is given in the "error" field.
This was noted in 2013 and the issue was closed in 2017.
#214
The text was updated successfully, but these errors were encountered: