Bot buys coins, even though orderLimitMaxOpenTrades is exceeded

[Rayn0r]

Today I ran into the situation where all open grid trades for my 3 traded coins fired their last grid trade. "Max. Open Trades" is set to 3 here. Shortly after, the bot tried to purchase three additional coins, that met the buy criteria.
Is this behaviour intentional? One can easily run into the situation, where you run out of coins on a big drop while the bot keeps buying new coins.
I now set my last grid trade to a ridiculously low percentage of 0.01 that will never be reached in order to (hopefully) overcome this issue.

Is there some other measure to prevent above described situation?

Thank you in advance.

[Rayn0r]

I faced the same situation again this morning. Last night I had 3 open trades with "Max. Open Trades" set to 4 and
"Max. Buy Open Orders" set to 2.
This morning I suddenly had 6 open trades. If I had set "Max. Buy Open Orders" to 3 I could come up with a logical explanation why I ended up at 6. But with above setup I could only have gotten to 5.
When the bot creates a buy-order for a coin not yet in the wallet, does this count into "Max. Open Trades" already?
If not, then the bot would always be able to buy more coins than "Max. Open Trades" permits, when "Max. Buy Open Orders" is above 1.

Curiously, when one of the 6 coins dropped low enough, so the bot would trigger the next grid, it complained that the maximum number of open trades had been reached.

The behaviour seem odd, and I cannot quite fathom the cause of it. Could my installation be messed up?
Could there by an consistency in the mongodb?

How exactly is the bot to behave in a situation with "Max. Open Trades" at 4, "Max. Open Trades" at 2 and already 3 open trades?

[Rayn0r]

@chrisleekr
I think the code does not handle what I think it should. If I understood this correctly then the code does not check whether or not "currently open trades" + "currently open orders" will exceed "max open trades".

I hope this is the right place to look at...
I found the function isExceedingMaxOpenTrades() that seem to check whether or not it is allowed to buy more coins.
I don't comprehend why you deduct 1 from currentOpenTrades but I'd say the code should look similar to this:

  // get this value for currentBuyOpenOrders from logger passed to the function
  const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);

  // do this when the coin is already in your wallet
  if (lastBuyPrice) { 
    currentOpenTrades -= 1;
  }
  // do this if you are about to buy a new coin, but don't want to exceed orderLimitMaxOpenTrades after coin is purchased
  else { 
    if (currentBuyOpenOrders + currentOpenTrades >= orderLimitMaxOpenTrades) {
      return false;
    }
  }

PS: This is my first attempt doing anything in node.js.

[Rayn0r]

My above code doesn't seem quite right.
I think this is closer to the truth:

  // If the last buy price is recorded, this is one of open trades.
  if (lastBuyPrice) {
    return false;
  }
  // If the last buy price is not recorded, this is a new trade.
  // Do not place additional open orders if the resulting number
  // of trades would exceed max open trades.
  else {
    const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
    if (currentBuyOpenOrders + currentOpenTrades >= orderLimitMaxOpenTrades) {
      return true;
    }
  }

  return false;
};

This would be the corresponding diff against current head:

diff --git a/app/cronjob/trailingTrade/step/determine-action.js b/app/cronjob/trailingTrade/step/determine-action.js
index 2be1ca5..2454fe1 100644
--- a/app/cronjob/trailingTrade/step/determine-action.js
+++ b/app/cronjob/trailingTrade/step/determine-action.js
@@ -149,13 +149,17 @@ const isExceedingMaxOpenTrades = async (logger, data) => {
   let currentOpenTrades = await getNumberOfOpenTrades(logger);

   // If the last buy price is recorded, this is one of open trades.
-  // Deduct 1 from the current open trades and calculate it.
   if (lastBuyPrice) {
-    currentOpenTrades -= 1;
+    return false;
   }
-
-  if (currentOpenTrades >= orderLimitMaxOpenTrades) {
-    return true;
+  // If the last buy price is not recorded, this is a new trade.
+  // Do not place additional open orders if the resulting number
+  // of trades would exceed max open trades.
+  else {
+    const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
+    if (currentBuyOpenOrders + currentOpenTrades >= orderLimitMaxOpenTrades) {
+      return true;
+    }
   }

   return false;

Comments are greatly appreciated.

[Rayn0r]

I found a case where above code did not do what it should. So here is a new version:

  // If the last buy price is recorded, this is one of open trades.
  if (lastBuyPrice) {
    // Usually it should always be allowed to buy more coins once the bot initially purchased them
    // and there are grid trades left.
    // If you lower your orderLimitMaxOpenTrades or if you manually add a buy price to a coin you already own,
    // we must still check whether or not we have reached orderLimitMaxOpenTrades.
    if (currentOpenTrades > orderLimitMaxOpenTrades) {
      return true;
    }
  }
  // If the last buy price is not recorded, this is a new trade.
  else {
    const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
  // Do no place an order when we have already reached orderLimitMaxOpenTrades
    if (currentOpenTrades => orderLimitMaxOpenTrades) {
      return true;
    }
  // Do not place additional open orders if the resulting number
  // of trades would exceed max open trades.
    if (currentBuyOpenOrders + currentOpenTrades > orderLimitMaxOpenTrades) {
      return true;
    }
  }
  // In all other cases return false
  return false;
};

This is the diff again:

diff --git a/app/cronjob/trailingTrade/step/determine-action.js b/app/cronjob/trailingTrade/step/determine-action.js
index 598110a..062e5f3 100644
--- a/app/cronjob/trailingTrade/step/determine-action.js
+++ b/app/cronjob/trailingTrade/step/determine-action.js
@@ -149,20 +149,29 @@ const isExceedingMaxOpenTrades = async (logger, data) => {
   let currentOpenTrades = await getNumberOfOpenTrades(logger);

   // If the last buy price is recorded, this is one of open trades.
-  // Deduct 1 from the current open trades and calculate it.
   if (lastBuyPrice) {
-    return false;
+    // Usually it should always be allowed to buy more coins once the bot initially purchased them
+    // and there are grid trades left.
+    // If you lower your orderLimitMaxOpenTrades or if you manually add a buy price to a coin you already own,
+    // we must still check whether or not we have reached orderLimitMaxOpenTrades.
+    if (currentOpenTrades > orderLimitMaxOpenTrades) {
+      return true;
+    }
   }
   // If the last buy price is not recorded, this is a new trade.
-  // Do not place additional open orders if the resulting number
-  // of trades would exceed max open trades.
   else {
     const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
-    if (currentBuyOpenOrders + currentOpenTrades >= orderLimitMaxOpenTrades) {
+  // Do no place an order when we have already reached orderLimitMaxOpenTrades
+    if (currentOpenTrades => orderLimitMaxOpenTrades) {
+      return true;
+    }
+  // Do not place additional open orders if the resulting number
+  // of trades would exceed max open trades.
+    if (currentBuyOpenOrders + currentOpenTrades > orderLimitMaxOpenTrades) {
       return true;
     }
   }
-
+  // In all other cases return false
   return false;
 };

[Rayn0r]

This condition will prevent the bot from opening number of trades greater than orderLimitMaxOpenTrades,

    if (currentBuyOpenOrders + currentOpenTrades > orderLimitMaxOpenTrades) {
      return true;
    }

but it will not completely solve the problem.
If you currentOpenTrades < orderLimitMaxOpenTrades but currentBuyOpenOrders > orderLimitMaxOpenTrades - currentOpenTrades then you won't be able to by a new coin even though orderLimitMaxOpenTrades is not reached.

For this to work correctly, we must be able to lower orderLimitMaxBuyOpenOrders to orderLimitMaxOpenTrades - currentOpenTrades .
I think this should done in isExceedingMaxOpenTrades.
I'll see if I can come up with an idea.

[Rayn0r]

Could this already do the trick:

const isExceedingMaxBuyOpenOrders = async (logger, data) => {
  const {
    symbolConfiguration: {
      botOptions: {
        orderLimit: {
          enabled: orderLimitEnabled,
          maxBuyOpenOrders: orderLimitMaxBuyOpenOrders
          maxOpenTrades: orderLimitMaxOpenTrades
        }
      }
    }
  } = data;

  if (orderLimitEnabled === false) {
    return false;
  }

  const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
  const currentOpenTrades = await getNumberOfOpenTrades(logger);

  if (currentBuyOpenOrders >= orderLimitMaxBuyOpenOrders) {
    return true;
  }
  if (currentBuyOpenOrders > orderLimitMaxOpenTrades - currentOpenTrades){
    return true;
  }

  return false;
};

?

diff --git a/app/cronjob/trailingTrade/step/determine-action.js b/app/cronjob/trailingTrade/step/determine-action.js
index 598110a..54a2d60 100644
--- a/app/cronjob/trailingTrade/step/determine-action.js
+++ b/app/cronjob/trailingTrade/step/determine-action.js
@@ -104,6 +104,7 @@ const isExceedingMaxBuyOpenOrders = async (logger, data) => {
         orderLimit: {
           enabled: orderLimitEnabled,
           maxBuyOpenOrders: orderLimitMaxBuyOpenOrders
+          maxOpenTrades: orderLimitMaxOpenTrades
         }
       }
     }
@@ -114,10 +115,14 @@ const isExceedingMaxBuyOpenOrders = async (logger, data) => {
   }

   const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
+  const currentOpenTrades = await getNumberOfOpenTrades(logger);

   if (currentBuyOpenOrders >= orderLimitMaxBuyOpenOrders) {
     return true;
   }
+  if (currentBuyOpenOrders > orderLimitMaxOpenTrades - currentOpenTrades){
+    return true;
+  }

   return false;
 };

[Rayn0r]

I think this could work.
I'm not sure how to use NOT correctly with lastBuyPrice. So this may not do what I expect.
Any help is very much appreciated.

const isExceedingMaxBuyOpenOrders = async (logger, data) => {
  const {
    symbolConfiguration: {
      botOptions: {
        orderLimit: {
          enabled: orderLimitEnabled,
          maxBuyOpenOrders: orderLimitMaxBuyOpenOrders,
          maxOpenTrades: orderLimitMaxOpenTrades
        }
      }
    },
    sell: { lastBuyPrice }
  } = data;

  if (orderLimitEnabled === false) {
    return false;
  }

  const currentBuyOpenOrders = await getNumberOfBuyOpenOrders(logger);
  const currentOpenTrades = await getNumberOfOpenTrades(logger);

  if (currentBuyOpenOrders < orderLimitMaxBuyOpenOrders) {
    if (!lastBuyPrice) {
      if (currentBuyOpenOrders > orderLimitMaxOpenTrades - currentOpenTrades) {
        return true;
      }
    }
  else {
     return true;
  }

  return false;
};

[Rayn0r]

I have put everything into a separate branch for testing: https://github.com/Rayn0r/binance-trading-bot/commits/max_open_trades_fix
Let me know what you think.

[eth-man]

Thank you @Rayn0r
I have the same issue
Will find time to try it and let u know

[Rayn0r]

I have pushed another little fix, but I don't think all is yet working the way it should.
If two coins drop simultaneously, then there is still a probability that both are bought. I have not been able to figure out why.
I believe that someone with more in-depth knowledge of how node-js works and this code here is required.

With my current code the occasion on when it bought more coins than it was allowed has decreased, but it still happens from time to time.

[uhliksk]

I think the better idea will be to keep opening orders without change but as soon as max. open trades is reached when any buy order at Grid Trade #1 is executed then all other buy orders at Grid Trade #1 have to be cancelled immediatelly. I'll create PR for this.