ccxt icon indicating copy to clipboard operation
ccxt copied to clipboard

okx, huobi, gate, bitmart - networks unification

Open ttodua opened this issue 2 years ago • 15 comments

Have migrated to our agreed structure. as OKX (like huobi) has unique ids for networks (i.e. USDT-ERC20), it's impossible to make direct networkCodeToId conversion without the help of currencyCode (OKX has very variative unique ids for chains, not just simple currencyCode + '-' + networkCode junction). so, here we needed quite like our previous implementation for HUOBI.

additionally, I've started to write down all popular networks with our unified uppercase NETWORK-CODE , which might be most suitable unified network-codes imo.

edit:

sorry if this seems large PR again, but i think after reviewing different cases, exchanges can be of only 2 types - [Type-1] Exchanges, that provide currencies in flat array with networks,:

[
  {
    'curr': 'USDT',
    'chain': 'USDT-erc20',
    ...
  },
  {
    'curr': 'USDT',
    'chain': 'USDT-trc20',
    ...
  },
...
]

[TYPE2] exchanges, which provide one object per currency, and inside it there is i.e. chains subkey which includes all networks for that currency:

[
  {
      'curr': 'USDT',
      'chains': {
         'erc20': {...}`
         'trc20': {...}`
       }
    }
]

This PR considers both approaches and uses the more centralized way to handle both cases with very simple flag in options & few added extra lines in derived implementations, to use TYPE1 or TYPE2 styles easily.

(fix #15987, fix #16016 , fix #16176) (related to #15789)

ttodua avatar Dec 08 '22 18:12 ttodua

@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 avatar Dec 12 '22 14:12 ttodua

can you please clarify, for ERC20 tokens network should be ERC20. And for ETH network is ERC20 or ETH?

ndubel avatar Dec 13 '22 10:12 ndubel

can you please clarify, for ERC20 tokens network should be ERC20. And for ETH network is ERC20 or ETH?

Good question. we will include that in Manual:

  • for ETH currency, the exclusive networkCode is ETH.
  • for all other ETH chain based tokens, the networkCode is ERC20.

similarly for TRX & TRC20.

@carlosmiei @kroitor

ttodua avatar Dec 13 '22 17:12 ttodua

I've merged GATE.IO currencies & network updates, because these PR's were too much related with each other because of core networks-related functionalities, and different PR would conflict with each other. I know this PR become a bit more complex, but it's better to review this together, to see the full image of how the common exchanges would be unified.

ttodua avatar Dec 14 '22 13:12 ttodua

about: https://github.com/ccxt/ccxt/pull/16061#discussion_r1048727921

@ttodua can we replace all parseFloat occurrences with parseNumber ? parseFloat does not take into consideration the precision mode (float or string) chosen by the user

the thing is that we have to exactly use parseFloat there instead of this.parseNumber because that is not parsing side of code, but the request-side, where we have to 'guaranteely' send float (numeric) in request (bcz exchange API requests float, instead of string), so if we use this.parseNumber then we possibly would convert it to string (if user set that) and thus break the implementation and user will get exceptions from exchange API.

ttodua avatar Dec 15 '22 17:12 ttodua

about: #16061 (comment)

@ttodua can we replace all parseFloat occurrences with parseNumber ? parseFloat does not take into consideration the precision mode (float or string) chosen by the user

the thing is that we have to exactly use parseFloat there instead of this.parseNumber because that is not parsing side of code, but the request-side, where we have to 'guaranteely' send float (numeric) in request (bcz exchange API requests float, instead of string), so if we use this.parseNumber then we possibly would convert it to string (if user set that) and thus break the implementation and user will get exceptions from exchange API.

@ttodua yeah sorry I was asleep when commented that out

carlosmiei avatar Dec 15 '22 17:12 carlosmiei

@carlosmiei Sorry, I was unable to push any commit today in other PR/tasks, because I have been working in networks during these days - going through each network of huobi, okx, gate - comparing network-ids with other exchange's, coinmarketcap/coingecko, then dealing with exchange specific mistakes and etc... and didn't have any time for other things. I'll now update that bitmex PR.

ttodua avatar Dec 15 '22 18:12 ttodua

@carlosmiei after spending whole week for this, I think I've completed the framework for network unifications. let me know any questions.

now few notes:

  1. from now, we no longer need to set networksById, because they are being auto generated. for example, we have:
 'networks': {
     'BEP20': 'Bep-20',
     'ERC20': 'Ether',
     'BTC': ['Bitcoin', 'Btc'],  // ! yes, from now we can now have array of possible network ids
 }

the above is effectively converted into networksById automatically :

 'networkCodesByIdsAuto': {
     'Bep-20': 'BEP20',
     'Ether': 'ERC20',
     'Bitcoin': 'BTC',
     'Btc': 'BTC',
 }

and then used/handled in network-id-to-code and other areas..

  1. about possible conflicts - lets say, huobi has such structure:
 'networks': {
     'CRC20': 'Cro',
     'CHILIZ': 'CRC20' //network ID matches to one of unified networkCode
 }

and when you make exchange.networkCodeToId ('CRC20', 'whatever'); gives overridable exception.

  1. added most common unified networkCodes list in base (which I will expand later gradually).

  2. added base method getUnfiedNetworkCodes so users will have a method to retrieve the unified networks dictionary for the specific exchange.

also other changes, might be hard to understand everything on first look, but everything done there was necessary and I don't think it could have been done simpler than this (as long as we want to have robust system with regard to networks/unification). Some technical-details can be optimized later (i.e. moving from options to class-property or etc..)

ttodua avatar Dec 22 '22 17:12 ttodua

when this will be merged, I will quicker go through all other exchanges and easily unify their all networks.

ttodua avatar Dec 22 '22 18:12 ttodua

@carlosmiei after spending whole week for this, I think I've completed the framework for network unifications. let me know any questions.

now few notes:

  1. from now, we no longer need to set networksById, because they are being auto generated. for example, we have:
 'networks': {
     'BEP20': 'Bep-20',
     'ERC20': 'Ether',
     'BTC': ['Bitcoin', 'Btc'],  // ! yes, from now we can now have array of possible network ids
 }

the above is effectively converted into networksById automatically :

 'networkCodesByIdsAuto': {
     'Bep-20': 'BEP20',
     'Ether': 'ERC20',
     'Bitcoin': 'BTC',
     'Btc': 'BTC',
 }

and then used/handled in network-id-to-code and other areas..

  1. about possible conflicts - lets say, huobi has such structure:
 'networks': {
     'CRC20': 'Cro',
     'CHILIZ': 'CRC20' //network ID matches to one of unified networkCode
 }

and when you make exchange.networkCodeToId ('CRC20', 'whatever'); gives overridable exception.

  1. added most common unified networkCodes list in base (which I will expand later gradually).
  2. added base method getUnfiedNetworkCodes so users will have a method to retrieve the unified networks dictionary for the specific exchange.

also other changes, might be hard to understand everything on first look, but everything done there was necessary and I don't think it could have been done simpler than this (as long as we want to have robust system with regard to networks/unification). Some technical-details can be optimized later (i.e. moving from options to class-property or etc..)

@ttodua sounds good thanks, will take a look soon

carlosmiei avatar Dec 22 '22 18:12 carlosmiei

@carlosmiei tnx.

ttodua avatar Dec 23 '22 14:12 ttodua

making few edits atm.

ttodua avatar Jan 10 '23 13:01 ttodua

in combination with bitmart (which has also different approach, like digifinex iirc) I've updated few things in base and also submitted bitmart here for demonstrational purposes. So, now, we can handle cases like when:

  1. exchange returns one currency-id (i.e. USDT) but with same network-ids for same network:
{
  'usdt': {
      'chains': {
           'erc20': { ... }, // where erc20 is also chain-id for other currencies
           'trc20': { ... }
      }
  }
}
  1. exchange returns one currency-id (i.e. USDT) but with different network-ids for same network i.e. HUOBI:
{
  'usdt': {
      'chains': {
           'usdt_erc20': { ... }, // network id is unique only for this currency
           'usdt_trc20': { ... }
      }
  }
}
  1. exchanges where there are multiple entries, with with different network-ids for same network i.e. okx:
[
  {
       "ccy": "USDT",
       "chain": "USDT-TRC20",
  },
  {
       "ccy": "USDT",
       "chain": "USDT-ERC20",
  },
]
  1. where exchange returns different currency-ids for same currency:
{
  {
       "currency": "usdt-erc20",
       "chain": "Ethereum",
  }, 
  {
       "currency": "xyz-erc20",
       "chain": "Ethereum",
  }, 
}

those exchanges are very much different with regard to request-response scheme and kinda complex, but with the help of the base methods and example implementations in these exchanges, it becomes easy to handle those complexities.

(in case bitmart makes any problems, we can remove bitmart.js from this PR and merge this which has absolutely same content for bitmart.js).

ttodua avatar Jan 10 '23 19:01 ttodua

networkId to networkCode

= A) crossing ids

networks: {
    'ERC20': [ 'erc-20', 'ether' ],
    'ETH': 'ether',
}
networkIdToCode ('erc-20', 'USDT'); ---> ERC20
networkIdToCode ('erc-20', 'ETH');   ---> ETH
networkIdToCode ('erc-20');             ---> returns as is, because we dont know currency.

B) conflicting id/codes

networks: {
    'CRC20': 'CRO',
    'CHILIZ': [ 'CHZCHAIN', 'CHZ20', 'CRC20' ], 
}
networkIdToCode ('CRO');     ---> CRC20
networkIdToCode ('CRC20'); ---> CHILIZ

however, it's not guarantee that the network is really for the provided currency, for example if you supply:

networks: {
    'JAMBOCHAIN': 'jmb123',
}
networkIdToCode ('jmb123', 'USDT');

it will still return response, but user should not think that "that 'jmb123' network exists for USDT currency" actually, because networkIdToCode just converted the input into output, but it doesnt confirm that the newtworkCOde 'JAMBOCHAIN' is available for "USDT" currency. no, the provided currencyCode just helps the conversion from network-id to network-code format, but doesn't affirm the existence of that network for that currencyCode.

C) for exchanges where this.options['networksAreTitlesInsteadOfId']=true, which means network-ids are variable per currency and we dont have a direct map, instead we have "networkCode <---> networkTitle <---> networkId" (i.e. huobi, okx):

networkIdToCode ('usdterc20') --->  usdterc20 (if markets were not loaded)
networkIdToCode ('usdterc20') --->  ERC20     (if markets were loaded)
networkIdToCode ('trc20usdt') --->  trc20usdt (if markets were not loaded)
networkIdToCode ('trc20usdt') --->  TRC20     (if markets were loaded)

networkCode to networkId

= A) for exchanges where network-ids are directly defined and we have direct map of "networkCode<>networkId" (i.e. gate, bybit, etc.. but not OKX & huobi):

networks: {
    'BEP2': 'BNB',
    'BEP20': 'BSC',
}
networkCodeToId ('BEP2') ----> BNB

B) for exchanges where this.options['networksAreTitlesInsteadOfId']=true, which means network-ids are variable per currency and we dont have a direct map, instead we have "networkCode <---> networkTitle <---> networkId" (i.e. huobi, okx):

networks: {
    'ERC20': 'Erc20', 
}
networkCodeToId ('ERC20', 'USDT') // exception: loadMarkets() needed to determine networkId ( create a map to `ERC20 --> Erc20 --> usdterc20`)
// after market load:
networkCodeToId ('ERC20', 'USDT') ---> usdterc20

C) when multiple ids

networks: {
    'CHILIZ': [ 'CHZCHAIN', 'CHZ20' ],
}
networkCodeToId ('CHILIZ', 'ARG');             ---> arg            // correct id
networkCodeToId ('CHILIZ', 'GALFT');          ---> crc20gal    // correct id
networkCodeToId ('CHILIZ', 'USDT'); // throws exception and asks user to input networkId directly instead of unified networkCode, if for USDT there was not found a networkId match for CHILIZ (if neither CHZCHAIN and CHZ20 were found for USDT during fetchCurrencies)
networkCodeToId ('AVALACHE_X', 'AVAX');  ---> avax         // correct id
networkCodeToId ('AVALACHE_X', 'BTC');    ---> AVALACHE_X  ("as is", because no matching ID was found `AVALACHE_X` network for `BTC` currency)

D) Alias support:

networks: {
    'SOLANA': 'sln',
}
networkCodeToId ('SOL');       ---> sln (because, at first alias is detected and is converted to 'SOLANA' networkCode, then to networkId

E) "mainnet chain (i.e. ETH)" <> "token chain (i.e. ERC20)" replacements support - this is not called an "alias", instead this is "real network replacement on behalf of user's mistake" because ETH and ERC20 are actually different chains, but for users convenience, we "guess" what they want to mean. Some exchanges have different network-ids for ETH coin vs other erc20-based tokens, some exchanges have same network-id, but in CCXT we defined them as individual networks like any other networks.

networkCodeToId ('ETH', 'USDT');    ---> usdterc20  // correct id
networkCodeToId ('ERC20', 'USDT');  ---> usdterc20  // correct id
networkCodeToId ('ETH', 'ETH');     ---> eth        // correct id
networkCodeToId ('ERC20', 'ETH');   ---> eth        // correct id

I have tried to document these methods inside comments too, but will add a bit more explanations later.

ttodua avatar Jan 13 '23 23:01 ttodua

for another type of exchange - i.e. Bitmart, where we have to send request (for withdraw/fetchDepoistAddress...) without chain parameter in request, but with currency-id which is an unique junction-id:

request = {
    'currencyId' :  this.networkCodeToCurrencyId ('ERC20', 'USDT'),  // opposed to `networkCodeToId`, this method returns the unique "currencyId" needed for such exchanges. in this case, it returns something like "usdt-erc20" or  whatever needed for bitmart-like exchange
}

Note, even thought sometimes such "unique currency id" might be junction of currency-id and network-id, they are not always such, so, we cant use blind currencyId + networkId formula there, instead we need directly that unique random id, which is saved during fetchCurrencies into this.generatedNetworkData exact map, and then later inside networkCodeToCurrencyId that value is being retrieved, instead of guessing & aplyying currencyId + networkId formula.

ttodua avatar Jan 14 '23 10:01 ttodua

Added Hitbtc3 (which is very like to bitmart) , where there are no meaning for networkIds and are just currency&network junctions as currencyIds : USDTRX, USDT20, USDTBSC, etc...

So, the approach which is used in such case by adding options['networksAreIncludedInCurrencyIds'] = true which solved bitmart and now that approach (without any other modification) perfectly fitted hitbtc too, as it was meant specifically for such exchanges.

ttodua avatar Jan 30 '23 19:01 ttodua