XRPLF / rippled

Decentralized cryptocurrency blockchain daemon implementing the XRP Ledger protocol in C++
https://xrpl.org
ISC License
4.49k stars 1.45k forks source link

book_offers returns limited offers without marker #3089

Open jpsgio opened 4 years ago

jpsgio commented 4 years ago

The book_offers method seems to return a max of 300 offers (or one page) per request, without returning a marker that allows for querying the remaining pages.

jpsgio commented 4 years ago

It appears that the line of code which returns the marker is commented out: https://github.com/ripple/rippled/blob/master/src/ripple/app/misc/NetworkOPs.cpp#L3211

Any reason for this?

scotttolton commented 4 years ago

I have also run into this issue. I am using the command line interface.

Based on the example query from the documentation: https://xrpl.org/book_offers.html, it is unclear from the syntax how one would specify a limit, and in instances where I am not specifying a limit, I am getting no more than 300 results, without any marker in the response.

Is there anyway that I can edit my request by adding more parameters, or is this an issue in the source code as the OP suggests?

scotttolton commented 4 years ago

It looks like this issue is noted as unresolved in the websocket source code as well: https://github.com/rubblelabs/ripple/blob/2881d83321683abf79199054788dcb329b9f3106/websockets/remote.go#L434

JoelKatz commented 4 years ago

This would take a pretty significant effort to add the desired behavior. When we first discussed this issue, we thought of a few ways to do it, but they're all quite complicated.

Remember, a person with 1,000 XRP can place 20 offers to trade that same 1,000 XRP. Only the first one is funded. This means that rendering the order book requires tracking the remaining available balance of all affected accounts assuming the order book gets taken to that depth.

One would be to have a huge marker that contains the balance deductions for all affected accounts. Another would be to have the server assign the query an ID and store the balance deductions for a few seconds, returning the ID in the marker. Lastly, the server could build order books locally, as deeply as needed, and store them locally for sharing between clients. But each order book would be ledger specific.

As none of these approaches are particularly satisfying, we didn't implement any of them.

The only really good approach is to require the client to track the balances. That would mean that for the first time an account is encountered in the reply, we'd report the current balance. It would be up to the client to determine the funded portion, caching deductions across queries.

MarkusTeufelberger commented 4 years ago

Only the first one is funded.

As far as I understand it, it is even "worse": Only the first one actually taken is funded, so at the time these offers are still up, there's no real way to know which one will be taken first in the future. Even if more than one valid transaction out there would take the XRP, it depends on the ordering of transactions making it into the ledger which one is the one that actually gets the funds - and that depends on all other transactions in that block.

To get an actual "state of the world" one probably would need to get a full ledger state and then re-apply all transactions since downloading that state, so it is possible to implement your own logic on top of this (e.g. how would one treat an account offering the same 1000 XRP in 20 different markets). This also isn't really feasible or easy though...

JoelKatz commented 4 years ago

It's not that complicated. The question we want to answer is what rate one or more transactions will take if they convert various amounts of the source asset into the destination asset. Offers are necessarily taken in order of quality and, for offers at the same quality, in the order they were placed. So we can always tell which offers to consider funded and which offers to consider unfunded.