Closed laloch closed 6 years ago
laloch
commented
6 years ago It's worth noting that just an hour later the sell_stop_pct stop didn't trigger. I have the engine patched in such a way that the sell_stop triggers only if the last trade was 'buy': if (do_sell_stop && !s.buy_order && s.sell_stop && last_trade.type === 'buy' && s.period.close < s.sell_stop). So my guess is that the engine "forgot" about the last trade somewhere between 16:18 and 16:21.
laloch
commented
6 years ago I can now confirm the above speculation about the last trade being "forgotten". I have added the last trade value price and trade count to the console output:
@@ -762,6 +762,13 @@ module.exports = function container (get, set, clear) {
else {
process.stdout.write(z(9, '', ' '))
}
+ var last_trade = s.my_trades[s.my_trades.length - 1]
+ if (last_trade) {
+ process.stdout.write(z(8, last_trade.price, ' ')[last_trade.type === 'buy' ? 'green' : 'red'])
+ } else {
+ process.stdout.write(z(8, '', ' '))
+ }
+ process.stdout.write(z(3, s.my_trades.length, ' '))
var orig_capital = s.orig_capital || s.start_capital
var orig_price = s.orig_price || s.start_price
if (orig_capital) {These are the results:
2018-01-30 05:36:00 2.48260 IOT-USD +0.51% 29530 57 ++ buying 0 2.28999180 IOT 588.98 USD -25.77% -8.00%
2018-01-30 05:36:26 2.47580 IOT-USD -0.28% 30 53 + buying 0 2.28999180 IOT 588.98 USD -25.77% -7.75%
buy order completed at 2018-01-30 05:36:40:
235.04498878 IOT at 2.48260 USD
[...]
2018-01-30 05:42:00 2.47530 IOT-USD -0.04% 3630 52 + sell 2.4826 1 236.86489060 IOT 5.45 USD -26.13% -8.18%
2018-01-30 05:43:23 2.47530 IOT-USD +0.00% 1849 52 + selling 2.4826 1 236.86489060 IOT 5.45 USD -26.13% -8.18%
sell order completed at 2018-01-30 05:43:27:
234.49624169 IOT at 2.47530 USD
[...]
2018-01-30 05:45:00 2.48140 IOT-USD +0.24% 24548 56 ++ sold 2.4753 2 2.36864891 IOT 584.74 USD -26.27% -8.58%
2018-01-30 05:48:00 2.47600 IOT-USD -0.22% 26728 52 + sell 2.4753 2 2.36864891 IOT 584.74 USD -26.28% -8.38%
[...]
2018-01-30 08:42:00 2.41480 IOT-USD +0.00% 28939 - 49 buy 2.4753 2 2.36864891 IOT 584.74 USD -26.29% -6.08%
2018-01-30 08:42:01 2.41400 IOT-USD -0.04% 863 - 48 buying 2.4753 2 2.36864891 IOT 584.74 USD -26.29% -6.05%
buy order completed at 2018-01-30 08:42:06:
239.32894773 IOT at 2.41480 USD
[...]
2018-01-30 09:54:00 2.48930 IOT-USD +0.01% 4452 63 ++ +3.08% 2.4148 3 241.21893874 IOT 7.00 USD -24.17% -6.27%
2018-01-30 09:57:00 2.48490 IOT-USD -0.18% 10500 59 ++ selling 2.4826 3 241.21893874 IOT 7.00 USD -24.30% -6.27%
^^^^^^
2018-01-30 09:57:03 2.48400 IOT-USD -0.04% 431 59 ++ selling 2.4826 3 241.21893874 IOT 7.00 USD -24.33% -6.27%
sell order completed at 2018-01-30 09:57:10:
238.80674935 IOT at 2.48470 USD
[...]
2018-01-30 11:30:00 2.53000 IOT-USD +0.20% 41092 69 +++ sell 2.4847 4 2.41218939 IOT 599.03 USD -24.46% -8.13%
2018-01-30 11:33:00 2.52800 IOT-USD -0.08% 34128 67 +++ sell 2.4148 4 2.41218939 IOT 599.03 USD -24.46% -8.06%
^^^^^^
2018-01-30 11:36:00 2.49000 IOT-USD -1.51% 107.21k - 47 sell 2.4148 4 2.41218939 IOT 599.03 USD -24.47% -6.67%You can see the last trade value price jumping around the my_trades array. Any clues? Maybe a call to my_trades.slice() missing somewhere before manipulation (sorting?) with a copy of my_trades?
laloch
commented
6 years ago @defkev : The bug is in dashboard.ejs on master branch where you do _mytrades.reverse(). It is probably already fixed by using XMLHttpRequest to obtain the trade data on unstable branch.
defkev
commented
6 years ago my_trades are no longer reversed but sorted in unstable
The array is populated by the time a trade got executed but we want it to show up by date.
This of course only applies to the dashboard as the array will always be old -> new
laloch
commented
6 years ago The point is that you should never ever touch _mytrades directly unless you are perfectly sure that what you have is my_trades deep copy. If you need to reverse or sort it, do it like _mytrades.slice().sort(...) instead. If you manipulate the array directly, you are breaking the engine, because of the brain damaged way the JS arrays work. There should be some sort of API preventing extensions to fiddle with the internal structures directly.
Edit: The master branch needs a hotfix for this. In the present state the dashboard breaks pretty much everything that depends on the _mytrades state (no offense, @defkev).
defkev
commented
6 years ago As i said, this only applies to the dashboard and never touches the array.
Its sorting the object exposed by the api under /trades in ejs not the array of the backend in node.
laloch
commented
6 years ago Its sorting the object exposed by the api under /trades in ejs not the array of the backend in node.
Not on the master branch. On unstable the dashboard is decoupled via XMLHttpRequest request to /trades. On master, however, the array is accessed directly. Try for yourself. I bet, that you can even do <% my_trades.push({size:0, price:0, time:0}) %> and the new trade will be immediately visible from the engine.js.
defkev
commented
6 years ago Again, as i said
sorted in unstable
But if what you say is true this would indeed be a huge security risk as the data exposed by /trades should be read-only, at least until we have some kind of API authentication in place.
Let me take a look.
laloch
commented
6 years ago I have just tried myself. I've added <% my_trades.push({size:1, price:1, type:"buy", time:0}) %> to dashboard.ejs (master branch version) and the console immediately displayed one trade with +140.07% profit.
laloch
commented
6 years ago Btw, I can confirm that with the below patch applied, the _lasttrade stopped altering and consequently stop loss order stopped firing at "random".
diff --git a/templates/dashboard.ejs b/templates/dashboard.ejs
index 34301cd..7bd29d5 100644
--- a/templates/dashboard.ejs
+++ b/templates/dashboard.ejs
@@ -209,7 +209,7 @@
</tr>
<% } %>
<% if (my_trades) { %>
- <% my_trades.reverse().forEach(function(trade){ %>
+ <% my_trades.slice().reverse().forEach(function(trade){ %>
<tr>
<td><span class='text-<% if (trade.type == "buy") { %>success<% } else { %>danger<% } %>'><%= trade.type.toUpperCase() %></td>
<td><%= trade.size %> <%= asset.toUpperCase() %></td>This is indeed bad and absolutely not what we want.
I have zero experience with express and wonder if this is document somewhere.
Thanks for reporting it 👍
Let me prep a PR.
System information
Describe the problem
As can be seen in the below live trading log, the profit calculated since the last trade diverts from the actual price movement. In this case from +0.89% to -1.72% despite the price growing by 0.51%.
Source code / logs