Bitfinex: Support multiple wallets for Balance

passabilities / crypto-exchange

Pulls together list of crypto exchanges to interact with their API's in a uniform fashion.

MIT License

261 stars 78 forks source link

Bitfinex: Support multiple wallets for Balance #15

Closed siedi closed 6 years ago

siedi commented 6 years ago

Bitfinex has multiple wallets, i.e. for types = ['exchange', 'trading', 'funding']

Right now, only the exchange wallet is returned for the balance. In order to be compatible with all exchanges, I suggest adding the type (wallet) as a parameter for all exchanges. Of course, others might have just one wallet.

Does that make sense?

passabilities commented 6 years ago

Yes, I noticed that as I was building this out. I don't use personally use Bitfinex and was confused by the different types of wallets. Could you help me understand what each wallet type purpose is?

I can add a type parameter for Bitfinex to allow this.

passabilities commented 6 years ago

I added a fix for this. For Bitfinex, you can pass a type in the options. See the README for fetching balances and addresses.

siedi commented 6 years ago

Thanks for fixing (also another bug included in the balance function). Unfortunately, there is a new issue introduced by the refactoring of the balance method. The caching you introduced creates a class variable balances, which conflicts with the method name balances, thus, when calling node complaints: TypeError: bitfinex.balances is not a function Also, the in Line109 the alt variable is not set before when it gets assigned.

And would you mind merging it to the history branch?

Thanks again!

siedi commented 6 years ago

Just to explain the different wallet types:

exchange is for "regular" buying or selling of assets/coins (like physically so you can transfer it from/to another wallet), similar to what you do e.g. on GDAX
trading is used for real trading, like when you open long or short positions. The wallet is needed to fund these trades and "collect" you wins (or losses) when closing the open positions.
funding, hm, never used it.

Hope my explanation makes sense.

passabilities commented 6 years ago

Ok those bugs should be fixed now. Thanks for the heads up. What would you liked merged with the history branch? I'm not quite done with it; i have plenty more to add for other exchanges.

passabilities commented 6 years ago

I just rebased history on top of master so it should have all those changes, if thats what you meant.

siedi commented 6 years ago

Works. Thanks!