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

id/counter for database_api::get_trade_history #448

Closed
vanissoft opened this issue Oct 30, 2017 · 5 comments
Closed

id/counter for database_api::get_trade_history #448

vanissoft opened this issue Oct 30, 2017 · 5 comments

Comments

@vanissoft
Copy link

For make use of get_trade_history could be easier if the return could have an op id or something unique and ordered.
At this moment is returned date but fall in problems where there are many ops in the same second.

@oxarbitrage
Copy link
Member

oxarbitrage commented Oct 30, 2017

Yes, we had the same issue with a new call i was trying to make very similar to this, it was get_account_history_by_date that was later discarded(not by this problem exactly but due others + this).
At that time we had a few workarounds for it but we cant apply here so easy as this is a production call, with clients using it as we speak so we can't change results format and can't add arguments to the call(like a start) without breaking some of this apps.

here are a few options to discuss, they all have some pros and cons:

  1. create new get_trade_history2, add an id to the results and add a start parameter to call.
  2. use current one and add to results all the trades until the time::second is finished bypassing the hard coded limit of 100. this haves the problem that one call can return 100 results, other 120 , etc.
  3. check the impact(what is the max number of trades per second the blockchain saw) and extend the limit.

@vanissoft
Copy link
Author

For me the limits are ok. Only have to make a loop with the calls. For id i think that it could be done adding some digits in the date when there are orders that fall in the same second.

@abitmore
Copy link
Member

abitmore commented Nov 9, 2017

create new get_trade_history2, add an id to the results and add a start parameter to call.

I like this approach. Will work on it in #455.

abitmore added a commit to abitmore/bitshares-core that referenced this issue Nov 10, 2017
* get_trade_history returns sequence
* get_ticker returns current time
abitmore added a commit to abitmore/bitshares-core that referenced this issue Nov 10, 2017
@abitmore
Copy link
Member

See #455, get_trade_history API now returns a sequence in each row, which can be used as start in the new get_trade_history_by_sequence API.

abitmore added a commit that referenced this issue Nov 11, 2017
Store price in fill_order_operation, for #449.
Also addressed a few other issues related to market history plugin and API, e.g. #448.
@abitmore
Copy link
Member

Implemented in #455.

@abitmore abitmore added this to the Next Non-Consensus-Changing Release - 201712 milestone Nov 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants