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

Minimum Order Size Calculation #1542

Closed
werkkrew opened this issue Dec 23, 2017 · 5 comments
Closed

Minimum Order Size Calculation #1542

werkkrew opened this issue Dec 23, 2017 · 5 comments

Comments

@werkkrew
Copy link
Contributor

Note: for support questions, please join our Discord server

  • I'm submitting a ...
    [x ] bug report
    [ ] feature request
    [ ] question about the decisions made in the repository

  • Action taken (what you did)
    Ran the trader bot on a pair of currency. (Binance - ETH/ZEC)

  • Expected result (what you hoped would happen)
    Trades occur.

  • Actual result (unexpected outcome)
    Errors out a lot trying to execute trades that are below the minimum allowed size on the exchange.

  • Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, etc)

In the binance api definition it is correctly set that for the pair ETH/ZEC the minimalOrder amount is 0.01 and type is set to asset (as it is with all other pairs). The minimum order for this pair on Binance is indeed 0.01 (I have verified this on their site).

The code in the trader plugin in the portfolioManager that decides if it can execute a trade is:

Manager.prototype.getMinimum = function(price) {
if(this.minimalOrder.unit === 'currency')
return minimum = this.minimalOrder.amount / price;
else
return minimum = this.minimalOrder.amount;
};

This means that when the api definition uses minimalOrder of type asset, it will simply use the amount being traded as the minimum and attempt to make the trade.

For example if the price in ETH/ZEC is 0.75 and I have a small amount of ETH (like 0.01) it will try to buy 0.013 ZEC (because it is above the minimum amount) even though the total of the order is only 0.009 which is below the minimum amount.

I believe the logic in getMinimum should be expanded to check for the minimalOrder value on both sides of the trade (buy/sell) as well as being able to have different minimum amounts set on the asset/currency side.

It looks like simply changing currency to asset in the above code, or asset to currency in the api definition would do the trick, but perhaps its worth expanding the function to be more robust.

For now as a workaround I have increased the minimalOrder value for my trading pair to be high enough that it wont try to place orders below the exchange minimum in the current market conditions.

@cmroche
Copy link
Contributor

cmroche commented Dec 24, 2017

@werkkrew In this case I don't think it is a good idea (or even a functional idea) to round up the purchase about to meet the minimum. What we should probably due it check to see if the purchase request meets the minimum of both asset and currency and if not stop trying to place the order.

The issue is that there is no way, with the current setup, to check both asset and currency in the same go. Also I think that this logic should be moved into the exchange implementation since it is market dependant, and even exchange dependant.

My plan is to implement a function like getLotSize which check the minimums, replaces with 0 if they fail, then round down to to the nearest acceptable precision.

@askmike Do you have any input on this approach?

This is a priority in my own work, so in the next couple of days I'll try to have a fix to test.

@werkkrew
Copy link
Contributor Author

@cmroche Yeah I wasn't suggesting to round anything, I was suggesting the logic be expanded to test the asset side or whatever needs to happen to ensure that the tool does not attempt to place orders which are below the minimum.

@rwoodr
Copy link

rwoodr commented Dec 24, 2017

To get Binance trading to work I switched to /cmroche/gekko/tree/binance_retry and ran into the MIN_NOTIONAL error after a few successful trades. I used werkkrew's hints to modify binance.js with both the currency and asset minimums for my trading pair and then edit Manager.prototype.getMinimum(), buy() and sell() in portfolioManager.js to test for those minimums. Trading is going well so far.

This solution limits me to one exchange and one trading pair, but I'm sure I can add additional pairs easily. I'll test your fix cmroche when it's ready.

@cmroche
Copy link
Contributor

cmroche commented Dec 24, 2017

@werkkrew @ms121 #1550 incorporates a change to resolve the min size problem, it needs to be tested though if you have some time.

@werkkrew
Copy link
Contributor Author

werkkrew commented Jan 3, 2018

#1550 fixes this, closing.

@werkkrew werkkrew closed this as completed Jan 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants