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

networks unification - okx, huobi, hitbtc #16024

Closed
wants to merge 121 commits into from

Conversation

ttodua
Copy link
Member

@ttodua ttodua commented Dec 8, 2022

this PR has been vastly transformed and migrated ino #18495

@ttodua ttodua assigned ttodua and carlosmiei and unassigned carlosmiei Dec 9, 2022
@ttodua ttodua marked this pull request as ready for review December 9, 2022 12:26
[ci-skip]
@@ -1685,6 +1686,17 @@ module.exports = class Exchange {
* @param {string|undefined} currencyCode unified currency code, but this argument is not required by default, unless there is an exchange (like huobi) that needs an override of the method to be able to pass currencyCode argument additionally
* @returns {[string|undefined]} exchange-specific network id
*/
if (this.options['hasUniqueNetworkIds']) {
Copy link
Member Author

Choose a reason for hiding this comment

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

so, this change only affects if exchange has hasUniqueNetworkIds to true in options. thus, this change doesn't affect any existing implementations (other than HUOBI & OKX)

js/okx.js Outdated Show resolved Hide resolved
js/okx.js Outdated
@@ -3566,36 +3632,11 @@ module.exports = class okx extends Exchange {
* @param {object} params extra parameters specific to the okx api endpoint
* @returns {object} an [address structure]{@link https://docs.ccxt.com/en/latest/manual.html#address-structure}
*/
const rawNetwork = this.safeStringUpper (params, 'network');
Copy link
Member Author

@ttodua ttodua Dec 9, 2022

Choose a reason for hiding this comment

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

all these lines below are similar as it was in huobi/bybit/cexio in old version, and like we replaced in huobi/bybit/cexio, here also replaced by our base network methods

@ttodua
Copy link
Member Author

ttodua commented Dec 12, 2022

@carlosmiei this is the final state of my planned version. So, both HUOBI & OKX have currency-specific ids for networks, and they do share very similar approaches, thus, I've moved the code in base.

additional problem with OKX is that it doesn't provide the correct data from fetchCurrencies (as mentioned in comments) and I had to add extra method (getCommonNetworkNameFromId) to help parsing the incoming network id (which was not defined from fetchCurrencies)

@ttodua ttodua changed the title Okx - networks unification Okx - networks unification (& huobi rework) Dec 12, 2022
@ttodua ttodua changed the title Okx - networks unification (& huobi rework) Okx - networks unification (& huobi networks ) Dec 12, 2022
@ttodua ttodua mentioned this pull request Dec 12, 2022
@ttodua ttodua changed the title networks unification - okx, huobi, gate, bitmart, hitbtc networks unification - okx, huobi,bitmart, hitbtc Jul 7, 2023
@ttodua ttodua changed the title networks unification - okx, huobi,bitmart, hitbtc networks unification - okx, huobi, hitbtc Jul 7, 2023
@ttodua ttodua closed this Jul 22, 2023
@ttodua ttodua removed the request for review from carlosmiei July 24, 2023 09:21
@ttodua ttodua removed bug important Higher priority labels Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants