PR 1838 issues

[JMoli]

System information

• Have I written custom code (as opposed to using zenbot vanilla): yea
• OS Platform and Distribution (e.g., Linux Ubuntu 16.04): osX
• Zenbot version (commit ref, or version): 4.1.0
• Zenbot branch: unstable
• NodeJS version: 9.9.0
• Did I make any changes to conf-sample.js?: yea

https://github.com/DeviaVir/zenbot/pull/1838

This merge introduces a bug and or features that are otherwise unhelpful.

Interval Trade https://github.com/DeviaVir/zenbot/blob/cabea7b8e00da686894d306609fa13895cc82b46/lib/engine.js#L921

This essentially attempts to add a new feature that if you set an interval_trade if will check on that interval. In theory this is a good concept as it would allow for an 1 hour interval bot to check on a 5 minute interval. However, the way it is implemented it is instead creating a new candle on the set interval_trade instead of simply just running the strategy, which means interval_trade is overriding whatever period / period_length is set as.

Quarantine Time

This is a non helpful feature. If my strategy makes a mistake and buys or sells at a loss I do not want some code at the engine level preventing my strategy from working on the next interval. If this is a desired feature it should be handled in the strategy logic not at the engine level, as it is an issue with the strategy not the engine. It essentially introduces more problems than it solves. Not to mention as the code is today it is not disabled by default.

[grooveuser]

FWIW, I agree wholeheartedly with @JMoli on the quarantine issue. This is a strategy issue, not an engine issue. Also, it annoys me that quarantine is spelled wrong. :)

I'm poring through the code again to confirm the findings on interval_time. This would explain a few issues I'm seeing.

Because these were implemented as individual PRs, I guess it would be easy enough to roll them back. Perhaps we could get an opinion from the PR author, or @DeviaVir ?

[mmdiego]

I also agree with this discussion, as that feature should be part of the strategy and not of the engine. Since the very first moment, I have disabled the feature making the following change in engine.js in my local copy. if (so.quarentine_time > 0 && s.buy_quarentine_time && moment.duration(moment(now()).diff(s.buy_quarentine_time)).asMinutes() < so.quarentine_time){ So setting quarentine_time to 0 should ignore that and behave as before.