feat: Customise the last buy price removing threshold #206

pedrohusky · 2021-05-29T21:27:56Z

As you asked, here is only the last buy function. I appreciate any critics.

You can set threshold amount in settings and in symbol settings. The bot will only remove the last price if it is below the threshold.

.vs/VSWorkspaceState.json

app/cronjob/trailingTrade/step/remove-last-buy-price.js

app/cronjob/trailingTradeHelper/configuration.js

config/default.json

chrisleekr · 2021-05-30T02:59:27Z

@pedrohusky
I just did a quick review. Only minor changes.

I think the main issue with the code was not using VSCode which gives you auto-formatting.
And you need to write tests to cover the changes.
Then it could detect the issue like https://github.com/chrisleekr/binance-trading-bot/pull/206/files#r642009869

Once you correct all comments, then needs to test whether that works or not.
Since frontend does not have any tests, need to test thoroughly.

-Indenting -Renaming to lastBuyPriceRemoveThreshold -Fixing ${maxPurchaseAmount} where it shouldn't be -Setting -1 to lastBuyPriceRemoveThreshold default value. -Removing non-sense comments that I've made

.gitignore

app/cronjob/trailingTrade/step/remove-last-buy-price.js

app/cronjob/trailingTradeHelper/configuration.js

chrisleekr · 2021-05-31T12:30:09Z

You haven't written any tests. Could you write test cases as well?

pedrohusky · 2021-05-31T12:33:29Z

@chrisleekr Of course. I'm working on them. Never wrote a test file. I'm using reverse engineering on yours. Sorry for the wait.

public/js/SettingIconLastBuyPriceRemoveThreshold.js

chrisleekr · 2021-05-31T12:42:13Z

And can you also update public/js/CoinWrapperSetting.js to display Last buy price remove threshold like Max purchase amount as it can be configured for each symbol?

chrisleekr · 2021-05-31T12:49:09Z

I just tested manually, and I found you need to update one more file app/cronjob/trailingTrade/step/determine-action.js

It has a function call canSell, which calculates the minimum notional value. You will need to update this function to use lastBuyPriceRemoveThreshold.

pedrohusky · 2021-06-02T01:25:30Z

And can you also update public/js/CoinWrapperSetting.js to display Last buy price remove threshold like Max purchase amount as it can be configured for each symbol?

Done. Will send the commit.

I just tested manually, and I found you need to update one more file app/cronjob/trailingTrade/step/determine-action.js

It has a function call canSell, which calculates the minimum notional value. You will need to update this function to use lastBuyPriceRemoveThreshold.

@chrisleekr
But why? I can't understand.

   return (
     lastBuyPrice > 0 &&
     baseAssetTotalBalance * sellCurrentPrice >  parseFloat(minNotional)
   );
 };

Why last price threshold would be useful here? Just to verify if the coin value is higher than the last price threshold?

Coin actual value: 10$
Last buy price: 1,25$
Last buy threshold: 1$**

Then we calculate if there is a last buy price, and see if is higher than minimum notional value (which can't be less than 10$ in usd). Then we verify if it is higher than the last buy threshold?

** = don't want the bot messing up with last buy until the coin value reaches 1$, which can be only two cases:
1 - on selling the coin, then the dust will not be higher than 1$ and the bot will remove the last buy price successfully
2 - the coin price dropped really a lot. But it would be impossible because of stop-loss function and a brain.

I'm a little confused.

@edit

I still didn't made the test files. Sorry. I'm a little busy.

- description, - last buy remove threshold now appears in symbol settings (frontend arrow), -Using destructing assignment, -removed .vscode from gitignore, sorry

…/binance-trading-bot into last-buy-under-amount

chrisleekr · 2021-06-03T14:11:43Z

Why last price threshold would be useful here? Just to verify if the coin value is higher than the last price threshold?

There is a scenario that removeLastBuyPriceThreshold does not work.

You have the last buy price.
The estimated balance is more than the minimum notional value. Let's assume you had $15.
You configure removeLastBuyPriceThreshold as $20.

As you configured $20, you would expect the bot removes the last buy price because the balance is less than configured threshold.

However, the bot will determine action as sell-wait - https://github.com/chrisleekr/binance-trading-bot/blob/master/app/cronjob/trailingTrade/step/determine-action.js#L267

In that case, removeLastBuyPriceThreshold is not working as expected.

I still didn't made the test files. Sorry. I'm a little busy.

It's ok. Thanks for your contribution. Take your time. :)

Now last buy remove threshold appears in symbol 'configuration arrow' in frontend; Sorry for the wait.

pedrohusky · 2021-06-08T16:59:52Z

Why last price threshold would be useful here? Just to verify if the coin value is higher than the last price threshold?

There is a scenario that removeLastBuyPriceThreshold does not work.

You have the last buy price.

The estimated balance is more than the minimum notional value. Let's assume you had $15.

You configure removeLastBuyPriceThreshold as $20.

As you configured $20, you would expect the bot removes the last buy price because the balance is less than configured threshold.

However, the bot will determine action as sell-wait - https://github.com/chrisleekr/binance-trading-bot/blob/master/app/cronjob/trailingTrade/step/determine-action.js#L267

In that case, removeLastBuyPriceThreshold is not working as expected.

I still didn't made the test files. Sorry. I'm a little busy.

It's ok. Thanks for your contribution. Take your time. :)

Done! Sorry for the delay, had returned from my vacation yesterday.

@chrisleekr

Can you tell me how can I setup apprise? (It is python)

Already tried python-shell, but didn't had success (maybe I've done something wrong)...

…/binance-trading-bot into last-buy-under-amount

pedrohusky · 2021-06-08T17:45:02Z

Dammit. VS couldn't commit or push, then when it done it, it pushed that all. lol I just changed one phrase. Sorry for that. Don't know why it did it.

chrisleekr · 2021-06-11T12:25:45Z

Done! Sorry for the delay, had returned from my vacation yesterday.

Welcome back :)

Can you tell me how can I setup apprise? (It is python)
Already tried python-shell, but didn't had success (maybe I've done something wrong)...

Oh, damn. Sorry, to be honest, I didn't check in detail.
I will do some research as well when I get time.

chrisleekr · 2021-06-11T12:39:51Z

app/cronjob/trailingTrade/step/determine-action.js

    baseAssetBalance: { total: baseAssetTotalBalance },
    sell: { currentPrice: sellCurrentPrice, lastBuyPrice }
  } = data;

  return (
    lastBuyPrice > 0 &&
-    baseAssetTotalBalance * sellCurrentPrice > parseFloat(minNotional)
+    baseAssetTotalBalance * sellCurrentPrice > parseFloat(minNotional) &&


I have tested manually, this line does not work. You gotta remove the below line because the bot is now relying on the lastBuyPriceRemoveThreshold

baseAssetTotalBalance * sellCurrentPrice > parseFloat(minNotional) &&

You can see from the below screenshot, the bot didn't remove the price, it still determines the action as sell-wait.

chrisleekr · 2021-06-11T12:44:06Z

public/js/SettingIconLastBuyPriceRemoveThreshold.js

+            controlId={'field-min-last-buy-remove-threshold-limit-percentage-' + quoteAsset}
+            className='mb-2'>
+            <Form.Label className='mb-0'>
+              Last Buy Price Remove Threshold for {quoteAsset}{' '}


As mentioned earlier, the global setting screen does not match with the symbol setting screen.

The symbol setting screen has Remove last buy price when under but in the global setting screen has Last Buy Price Remove Threshold for USDT. I think Remove last buy price when under is easier to understand. So let's go with it.

One more suggestion is let's display the field with the quote asset like the below screenshot. | 10 | USDT |

I've already edited the description and name in my other fork that I would like to open a pull request soon. I think you will like it.

I'll make those changes here.

Sure! Can you send me the code to do this? Don't forget I'm newbie in js.

@edit

Don't need to send me anything.
I can use the manual trade code to do that. Thanks for the heads up!

chrisleekr · 2021-06-11T12:45:10Z

@pedrohusky I just left some comments. I think almost there, but I will definitely need tests to make sure all working as expected.
If you need help writing test cases, please let me know.

I really appreciate your effort and time in this PR.

pedrohusky · 2021-06-14T12:01:01Z

Done! Sorry for the delay, had returned from my vacation yesterday.

Welcome back :)

Can you tell me how can I setup apprise? (It is python)
Already tried python-shell, but didn't had success (maybe I've done something wrong)...

Oh, damn. Sorry, to be honest, I didn't check in detail.
I will do some research as well when I get time.

@chrisleekr

Chris, I need help setting up the tests.

When. I run npm test I get something about "spawn error", don't know what to do.

Btw: I've set up the multilanguage in frontend. But I need to write the tests from lastbuy to open a pull request.

chrisleekr · 2021-06-14T13:11:45Z

Hey @pedrohusky

Chris, I need help setting up the tests.
When. I run npm test I get something about "spawn error", don't know what to do.

Have you run npm install to install all node modules?

Try

$ npm install
$ npm run test

And some more note, if you open the project folder, you will see a new folder/file coverage/lcov-report/index.html. This will give you the list of files and coverage of the test.

If you still need help, please let me know. :)

Btw: I've set up the multilanguage in frontend. But I need to write the tests from lastbuy to open a pull request.

Sure, that is good. Thanks for all your contributions. 👍

…ount

chrisleekr · 2021-07-06T11:30:07Z

Hey @pedrohusky

Sorry for the delay.

I have updated your repo including Setting UI and tests.
I am currently testing the feature.

Can you take a look and let me know whether you are happy with the changes?

Thank you for your contributions. :)

pedrohusky · 2021-07-06T16:50:17Z

Hi @chrisleekr . All seems good to me. Thanks for it. I think we can merge.

Btw, I'm gonna have a look on how I will split my changes.

Right now I've covered this issues:

Add sudden drop buy strategy - [#67 ]

Support Grid strategy for buy/sell to mitigate loss/increasing profit - [#158 ]

Add minimum required order amount - [#84]

Somewhat the #106 too, but I can't find a way to implement Apprise. Right now it is just telegram and slack.

Support cool time after hitting the lowest price before buy - [#105] <- My indicator covers it I think.

Filter symbols in the frontend - [#120]

Reset global configuration to initial configuration - [#97]

Support multilingual frontend - [#56]

Develop simple setup screen for secrets

With the other changes that I did like theme and such. My readme is updated at my language-telegram-lastbuy branch.

Right now I'm working in these:

Backtest. Need help loading the .csv into an array.

Add frontend option to disable sorting or improve sorting

And this I think is my priority right now:

Manage setting profiles (save/change/load?/export?) - [#151]

All time profit or All trades executed (successfully executed trades or trades at loss).

But this we can merge.

pedrohusky added 2 commits May 29, 2021 18:26

Last buy now have been reworked to work like max purchase amount.

6de3986

didn't see the need to calculate minimum amount there.

474ee24

chrisleekr mentioned this pull request May 30, 2021

Telegram, initial language setup (for future changes in frontend), don't remove last buy when under amount, reworked notifications. #204

Closed