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

Liqui: fetchBalance() should return only free balance #235

Closed
mkutny opened this issue Sep 25, 2017 · 4 comments
Closed

Liqui: fetchBalance() should return only free balance #235

mkutny opened this issue Sep 25, 2017 · 4 comments
Assignees

Comments

@mkutny
Copy link
Contributor

mkutny commented Sep 25, 2017

According to Liqui's API they return only free balance:
"Your account balance available for trading. Doesn’t include funds on your open orders".

At the moment ccxt not only puts this amount into free field (which is correct) but also copies the same value intototal (which is wrong).

@kroitor kroitor self-assigned this Sep 25, 2017
@kroitor
Copy link
Member

kroitor commented Sep 25, 2017

Hi, @mkutny !

Thanks for opening this issue! Really appreciate your involvement!

It's not very clear from their docs, how we should calculate totals without fetching currently open orders. So we basically have these options for total:

  1. put a zero there
  2. put a free + used there (where used equals 0, therefore total == free + 0 == free)
  3. put an undefined/None/null value there

It's not very clear, which one is better... Or, maybe, you can advise another way of summing the total for liqui? What do you think of it?

@mkutny
Copy link
Contributor Author

mkutny commented Sep 26, 2017

Indeed, it seems that with Liqui you can't calculate totals without fetching open orders.

At the same time ccxt Manual says:

some or all of the free, used and total amounts may be undefined, None or null. You need to account for that when working with returned balances

which aligns with common sense and I think it's a quite reasonable approach.

At least in my case, I had to go through the following steps:

  1. Fetched total.
  2. Got surprised it was zero (I knew it was not zero!).
  3. Dug into the issue, printed Liqui response, read their docs, ccxt source code, ccxt Manual.
  4. Eventually switched to free + used (and used is calculated by fetching open orders (thanks you added them for Liqui recently!)).
  5. Opened this issue.

If total was undefined from the beginning then I wouldn't have needed to go through 1-3 and 5 )

And for the sake of completeness I would comment on your options:

  1. put a zero there

If you put zero there unconditionally then when I stumble upon it I never know if it's indeed zero or I need to fetch open orders to decide. Zero is quite misleading.

  1. put a free + used there (where used equals 0, therefore total == free + 0 == free)

BTW, just noticed that you explicitly set used to zero! I believe it also should be undefined for the reason below.

put an undefined/None/null value there

Putting undefined is exactly what would reflect the reality: "has been declared but has not yet been assigned a value". One would need to assign a value to it by fetching open orders.

But what would be really nice if ccxt checked for open orders (which Liqui returns in the same getInfo response) and set zero explicitly only if open orders == 0. This way one could:

  • given zero in total/used be positive that it's indeed zero (no further checks needed)
  • given undefined in total/used be uncertain about their true value (fetch of open orders needed to determine)

@kroitor
Copy link
Member

kroitor commented Sep 26, 2017

I totally agree with you here, it was undefined in the beginning, but then there were some complaints, like this one here: #166, the guy asked for zeroes, when fields are unknown... I guess we will return it back to how it was before (like you suggested). Most of users still seem to want it explicit. Thx for such a comprehensive reply!

@kroitor
Copy link
Member

kroitor commented Sep 26, 2017

Now it's going to be like this:

https://github.com/ccxt-dev/ccxt/blob/33139c3355c45b9f8f2149dc50015308f976b432/ccxt.js#L14782

Here's a short snippet from there:

            let total = undefined;
            let used = undefined;
            if (balances['open_orders'] == 0) {
                total = funds[currency];
                used = 0.0;
            }
            let account = {
                'free': funds[currency],
                'used': used,
                'total': total,
            };

So, I, guess, it solves this particular issue and, if you don't mind, I'm closing it now. Please upgrade to fixed version 1.8.22+ (in several minutes) and see if all fields correspond to your expectations. They should ) Let us know if you have any difficulties or questions. Thx again!

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

No branches or pull requests

2 participants