DeviaVir / zenbot

Zenbot is a command-line cryptocurrency trading bot using Node.js and MongoDB.
MIT License
8.24k stars 2.03k forks source link

Bug: zenbot sells in loss even if configured otherwise #1141

Closed firepol closed 6 years ago

firepol commented 6 years ago

There is an option called max_sell_loss_pct.

In the conf-sample.js I read:

// avoid selling at a loss below this pct set to 0 to ensure selling at a higher price...
c.max_sell_loss_pct = 25

so I configured as follows:

c.max_sell_loss_pct = 0

I did set it to 0. I did run a zenbot session in live and still zenbot bought e.g. at 0.00015 and sold some hours later at 0.00012. This is quite a big percent of loss. For a volatile crypto it's ok for the price to go up and down, what I definitively don't want is the bot to sell if I configured c.max_sell_loss_pct = 0.

This looks like a critical bug to me.

To reproduce I guess you can backfill some data and test with the sim. You should not get any losses if configured to 0, else the option is misleading and should be documented otherwise.

Shawn8901 commented 6 years ago

Yeah its not working like documented.

Following is checked in line 498 in engine.js if (so.max_sell_loss_pct && sell_loss > so.max_sell_loss_pct) { if max_sell_los_pct = 0 it will be false every time. So the code should check if (so.max_sell_loss_pct == 0 || sell_loss > so.max_sell_loss_pct) { or just if (sell_loss > so.max_sell_loss_pct) { to work like documented.

firepol commented 6 years ago

@Shawn8901 please commit ;) and I'll test it.

defkev commented 6 years ago

Checking a var which is set to 0 (zero) will always yield false.

Instead check if the var is not undefined and not null using so.max_sell_loss_pct != null

the sell_loss logic itself is fine:

sell_loss is either null (if s.last_buy_price isn't set aka there was no previous buy) or (Number(price) - s.last_buy_price) / s.last_buy_price * -100 which either returns a positive (=loss) or or negative (=no loss) float.

So as long as sell_loss is positive sell_loss > so.max_sell_loss_pct will always return true and refuse to sell.

Taking @firepol's numbers sell_loss would be 20 (or +20%)

You should not get any losses if configured to 0

Only if the very first order would be a buy, as long as the bot keeps placing sell orders without a buy sell_loss is null and sell loss protection won't work, see above.

firepol commented 6 years ago

@defkev I just pulled the change and tried it on the simulator, the simulator still sells at loss:

Try e.g ./zenbot.sh sim cexio.XRP-USD --start 2018-01-15 --end "2018-01-16T23:00:00" --period 5m --currency_capital 1000

It buys at the beginning of the simulation and sells around the end. See result below...

{ 
  "asset_capital": 0,
  "avg_slippage_pct": 0.045,
  "buy_pct": 99,
  "buy_stop_pct": 0,
  "currency_capital": 1000,
  "days": 1,
  "end": 1516140000000,
  "markdown_buy_pct": 0,
  "markup_sell_pct": 0,
  "max_sell_loss_pct": 0,
  "max_slippage_pct": 5,
  "min_periods": 52,
  "mode": "sim",
  "neutral_rate": "auto",
  "order_adjust_time": 5000,
  "order_type": "taker",
  "oversold_rsi": 10,
  "oversold_rsi_periods": 14,
  "period": "5m",
  "period_length": "5m",
  "profit_stop_enable_pct": 10,
  "profit_stop_pct": 1,
  "rsi_periods": 14,
  "selector": {
    "exchange_id": "cexio",
    "product_id": "XRP-USD",
    "asset": "XRP",
    "currency": "USD",
    "normalized": "cexio.XRP-USD"
  },
  "sell_pct": 99,
  "sell_stop_pct": 0,
  "show_options": true,
  "start": 1515970800000,
  "stats": false,
  "strategy": "trend_ema",
  "trend_ema": 26,
  "verbose": false
}
end balance: 565.59288225 (-43.45%)
buy hold: 577.21139430 (-42.28%)
vs. buy hold: -2.02%
11 trades over 3 days (avg 3.66 trades/day)
win/loss: 1/1
error rate: 50.00%
wrote simulations/sim_result_cexio.XRP-USD_180116_221806_UTC.html

My config is the same as conf-sample.js, except:

c.max_sell_loss_pct = 0

Or can it be that this feature working only on live trading, but not in the simulator?

defkev commented 6 years ago

Now sell loss protection is always working as long as c.max_sell_loss_pct isn't set to 100

win/loss: 1/1 error rate: 50.00%

The bot sold 2 times (win = # of sells - # of losses) and lost 1 time (loss = # of losses) thus a error rate of 50% A loss occurs whenever the current trade price is below your last_buy. Keep in mind that this aren't live stats, they are calculated after the sim is finished and the price can differ significant compared to your last_buy by now.

firepol commented 6 years ago

@defkev alright now I see, that means, if it takes in consideration only the last buy, if I set the bot to buy at 99% (as in the config-sample), starting with a balance of 1000 USD, the bot make a purchase of 990 (99% of 1000) at price 12'000, then say the crypto value is crashing, like during these day. The bot after several hours thinks it's a good moment to buy, at 10'700, so now it triggers an order for 99% of the remaining balance, which in our case is just 10 USD. So 99% would be 9.90 USD.

Cool, now the crypto is going up, say at 11'800, but then encounters some resistance and start crashing again, so the bot triggers a sell order, because compared to the last order of 9.90 USD bought at 10'700, 11'800 is a win, but the bot was configured to sell 99% of the asset capital. So now the bot is selling everything at that price, which is below the price used to buy the biggest chunk of crypto.

So this is a big loss now.

To save us from such a situation, I guess that the best is to use 100% for buy and sell, to keep things easy.

If it keeps track only of the price of the last purchase, then it forgets all previous purchases? Or does it save every single trade in a LIFO way, so when a trade is completed it's removed from the stack and it can compare to the next one (the one at the top of the stack)?

If it's really like this, I believe we should have the feature for the bot to track every single purchase and code it to try to do trades using the same amounts of every single trade, not using the percent. That would be a cool feature.