Skip to content
This repository has been archived by the owner on Jun 22, 2020. It is now read-only.

[#hotfix] Poloniex, Bittrex base/target swapped fix #86

Merged
merged 3 commits into from
Aug 20, 2017

Conversation

tmlee
Copy link
Member

@tmlee tmlee commented Aug 17, 2017

No description provided.

@tmlee tmlee force-pushed the hotfix/base_target_swapped branch from bc6ad55 to 9167382 Compare August 17, 2017 07:10
@Merlz
Copy link

Merlz commented Aug 18, 2017

@tmlee, can you add the smoke test to a seperate pull request and merge 9167382 & 0c6692f into the master branch?

end

def ticker_url(market_pair)
base = market_pair.base
target = market_pair.target
"#{Cryptoexchange::Exchanges::Bittrex::Market::API_URL}/public/getmarketsummary?market=#{base}-#{target}"
# Bittrex pair has BTC comes first, when BTC is typically a Target not a Base
"#{Cryptoexchange::Exchanges::Bittrex::Market::API_URL}/public/getmarketsummary?market=#{target}-#{base}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmlee Looking at the response of https://bittrex.com/api/v1.1/public/getmarketsummary?market=btc-ltc

{
    "success": true,
    "message": "",
    "result": [
        {
            "MarketName": "BTC-LTC",
            "High": 0.011225,
            "Low": 0.01054107,
            "Volume": 108671.50400962,
            "Last": 0.01082199,
            "BaseVolume": 1186.30109815,
            "TimeStamp": "2017-08-20T05:07:00.463",
            "Bid": 0.01082199,
            "Ask": 0.010822,
            "OpenBuyOrders": 1168,
            "OpenSellOrders": 6783,
            "PrevDay": 0.01100987,
            "Created": "2014-02-13T00:00:00"
        }
    ]
}

I think we should use Volume instead of BaseVolume?

base: pair['BaseCurrency'],
target: pair['MarketCurrency'],
base: base,
target: target,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we should also use Volume not BaseVolume for Bleutrade

@d0minicw0ng
Copy link
Contributor

@tmlee looks good, we will have a separate PR to fix the Volume

@d0minicw0ng d0minicw0ng merged commit 7fcff48 into coingecko:master Aug 20, 2017
@tmlee
Copy link
Member Author

tmlee commented Aug 21, 2017

@dominicwong617 Thanks for looking into that as well
@Merlz all those changes have been merged now

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

Successfully merging this pull request may close these issues.

None yet

3 participants