search

bitshares / bitshares1-webwallet

Web Interface for BitShares Wallets 0.x (before 2015-10-13)
The Unlicense
35 stars 53 forks source link

Incorrect data in price history chart #689

Closed abitmore closed 9 years ago

abitmore commented 9 years ago

BTS:BitUSD pair, default time interval (30m), Sat May 02 2015 04:30:00 GMT, the data shows: O:0.003414 H:0.003414 L:0.003364 C:0.003165, Both H and L are greater than C. Just like https://github.com/BitShares/bitshares/issues/1300

With 1h interval it's correct.

By comparing it with result of blockchain_market_price_history USD BTS "2015-05-02T04:30:00" 1800, it seem that the L value in 30m chart is incorrectly set to the L value of the first block.

abitmore commented 9 years ago

I think the market page would load slightly faster if the default interval is one hour. Thoughts?

abitmore commented 9 years ago

And it would be faster to load dailyHigh and dailyLow from blockchain_market_price_history (with one hour interval or so) than from blockchain_market_order_history. However, if we have to loop results of both API's for other purpose, the slight improvement here may not make much sense. Another reason of current implementation (using blockchain_market_order_history) is probably blockchain_market_price_history had different behavior in the past.

svk31 commented 9 years ago

I doubt there's much difference between 30 minute and 1 hour intervals, the timespan loaded is adjusted anyway.

We loop over the order history anyway so the low and highs use the existing loop. Either way the loops aren't the problem in general, it's the api calls themselves that are slow.

abitmore commented 9 years ago

blockchain_market_price_history with parameter per_hour or bigger would scale better in the future when we have more transactions, since the output size of price history is constant. For now, since the order history is hidden by default, but the price chart is shown by default, I think it's better to defer loading order history (load even later than what it does now) , so it's better to load daily high and low data from price history.

svk31 commented 9 years ago

Why do you say the order history is hidden by default? It shouldn't be, it's in the third column at the bottom. The order depth chart is hidden, but it's constructed from the open asks, bids, shorts and covers.

Everything is called for at the same time and I don't think we can really change that as we need all that information, at least the way the exchange interface is right now.

I agree it might be a little faster to use 1 hour by default, but I don't see why it would scale better in the future tbh, we're already asking only for a portion of the price history and that won't change.

abitmore commented 9 years ago

Ah, my fault, the order history is not hidden.

abitmore commented 9 years ago

Why I think it will scale better? If one day we have 10 transactions in every block in the future, loop the order history for daily high and daily low means we need to loop 86400 records, but loop the price history with per_hour we need to loop 24 records only.

abitmore commented 9 years ago

And yes, as I've said, it doesn't make much sense right now since we still have to loop order history for other purpose. So it's maybe some space for future improvement.

abitmore commented 9 years ago

@svk31 Currently BTS:BitUSD market shows 24h low as 0.001602 BitUSD/BTS, it's possibly wrong. Probably caused by this line: https://github.com/BitShares/web_wallet/blob/82c2bd6eda8f4935c34038df4f3f0c801f33e9f1/app/js/services/market_service.coffee#L731