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

new bitfinex code is not working correctly #341

Closed
askmike opened this issue Jun 23, 2016 · 11 comments
Closed

new bitfinex code is not working correctly #341

askmike opened this issue Jun 23, 2016 · 11 comments

Comments

@askmike
Copy link
Owner

askmike commented Jun 23, 2016

This issue is not related to the stable branch.

See #337 and #339. Ping @xhad.

Because of the above issues I had to revert to the old but working bitfinex code (see a6c7db). The current dev branch still has the bitfinex code you gave. As soon as it's fixed please submit a PR against the dev branch.

@askmike
Copy link
Owner Author

askmike commented Jun 23, 2016

@xhad you said:

Hi John! I was working on the bitfinex module and I had a similar issue. There should be atleast .015 btc available in your bfx exchange wallet. 0.01 btc is a minimum trade and there are fees per trade. Also, you may need to wait at least 10mins for the api keys to get validated.

Do you think the the error TypeError: Cannot read property 'amount' of undefined came from the fact that there were no balances on the account? In that case it's a bug in Gekko itself and we should force the currency and the asset (in this case "USD" and "BTC") on the portfolio object.

@askmike
Copy link
Owner Author

askmike commented Jun 23, 2016

@xhad you said:

This is a strange issue. It's not related to having funds available. The callback is giving an error code 'ETIMEDOUT' sometimes, but most of the it works fine. I'm not sure what's causing this error. It comes from calling #L65

Ah that is pretty easy to fix, this code is dangerous:

this.bitfinex.ticker(defaultAsset, function (err, data, body) {
  callback(err, { bid: +data.bid, ask: +data.ask })
});

If the bitfinex ticker call errors for whatever reason (network error like ETIMEDOUT) the ticker should be recalled until it works. Here is how the btce code handles that:

if(err) {
  return this.retry(this.btce.getInfo, calculate);
}

This means that if there is any error, the callback will not be called but instead the getInfo call the BTC-e exchange will be called again (and this stays happening until a succesfull call).

@xhad
Copy link
Contributor

xhad commented Jun 23, 2016

I wasn't able to reproduce this error. TypeError: Cannot read property 'amount' of undefined I emptied the exchange account and Gekko didn't give that error. Perhaps that error was related to my first PR. ^^

@askmike
Copy link
Owner Author

askmike commented Jun 23, 2016

maybe, but @jhnferraris did experience the same error (on an empty account)..

@askmike
Copy link
Owner Author

askmike commented Jun 23, 2016

I will try to look at this a little later, do you think you are able to program if checks that recall the exchange on fail? the btce exchange code has examples. That does require a deep understanding of scoping in async javascript, so if you need some help just let me know!

@xhad
Copy link
Contributor

xhad commented Jun 23, 2016

Trader.prototype.getTicker = function(callback) {
  this.bitfinex.ticker(defaultAsset, function(err, data, body) {
    if (err) {
      return this.retry(this.bitfinex
        .ticker(defaultAsset, function(err, data, body) {
                callback(err, {bid: data.bid, ask: data.ask});
          }));
        }
    callback(err, {bid: data.bid, ask: data.ask})
  });
}

@xhad
Copy link
Contributor

xhad commented Jun 23, 2016

Could you please take a look at the code above? It works, but I'm thinking there could be a more elegant way to do it. Thanks for being so kind.

@askmike
Copy link
Owner Author

askmike commented Jun 23, 2016

So how the btc-e function does it is:

  1. name the function that handles response from the API.
  2. if there is an error, just recall the API and pass the already named handler.

Example would be:

Trader.prototype.getTicker = function(callback) {
  // the function that will handle the API callback
  var process = function(err, data, body) {
    if (err) {
      // on error we need to recurse this function

      // however we don't want to hit any API ratelimits
      // so we use this.retry since this will wait first
      // before we retry.

      // the arguments we need to pass the the ticker method
      var args = [ defaultAsset, process ]
      return this.retry(this.bitfinex.ticker, args);
    }

    // whenever we reach this point we have valid
    // data, the callback is still the same since
    // we are inside the same javascript scope.
    callback(err, {bid: data.bid, ask: data.ask})

   // note that because we use `this` inside this function we
   // have to make sure this function is bound to the correct
   // context.
  }.bind(this);

  this.bitfinex.ticker(defaultAsset, process);
}

Note:

  • I am assuming defaultAsset exists and is properly set.
  • Gekko expects the returned bid and asks to be float values. They might just be strings (you can check that by logging like so console.log(typeof data.bid). If it's a string you need to cast it to a float like so var bid = +data.bid (or exactly the same as var bid = parseFloat(data.bid))

@xhad
Copy link
Contributor

xhad commented Jun 23, 2016

Thank you very much Mike.

@askmike
Copy link
Owner Author

askmike commented Jun 23, 2016

No problem, if you have more problems let me know. If you are ready please create a pull request against this branch: https://github.com/askmike/gekko/tree/new-bitfinex

The develop branch has the old bitfinex code again.

@askmike
Copy link
Owner Author

askmike commented Jun 26, 2016

Thanks @xhad! I tested your code, everything seems fine :) I merged everything into stable now.

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

No branches or pull requests

2 participants