Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: non-active exchange symbols #478

Merged

Conversation

habibalkhabbaz
Copy link
Contributor

Description

Based on #476 we found that the Binance API is returning all their symbols even the non-active ones.
Hence, this PR is going to squash those non-active symbols from our exchange symbols list.

Related Issue

#476

Motivation and Context

How Has This Been Tested?

Write tests to check if the symbols were filtered correctly or not.

Screenshots (if appropriate):

@chrisleekr
Copy link
Owner

Hi @habibalkhabbaz

It can cause confusion during the time when Binance is in maintenance.
It may remove all symbols as it's not TRADING during maintenance.

Exchange info returns the symbol status as the below link

And some good summary of it.

It's a bit hairy because BREAK can be just in some downtime.

I had some thought about it.
What if we simply allow selecting from the setting, but show the symbol status in the UI?

image

For example, if it's TRADING, then show this loading indicator. If not TRADING, show an exclamation mark with some notes.
You may have a better idea.

@habibalkhabbaz
Copy link
Contributor Author

habibalkhabbaz commented Aug 17, 2022

Hello @chrisleekr
You are right 💯
After reading the summary I understood that it's better to keep the symbols because there is no guarantee of a delisted symbol.

However, I think twice to make things easier :)

Besides your suggestion, what do you think about the following?

Selecting symbols from Settings (Before) Selecting symbols from Settings (After)
No indicators Blinking green dot with status badge

I think this will be easier to catch.

@chrisleekr
Copy link
Owner

@habibalkhabbaz that is much better idea!

@habibalkhabbaz
Copy link
Contributor Author

Done @chrisleekr
I also added a warning when a symbol stopped working.

@uhliksk
Copy link
Contributor

uhliksk commented Aug 18, 2022

@habibalkhabbaz It will be also useful to add new symbol to main screen immediatelly to be visible. Currently it is waiting for the first price change which will make this indication useless. If I add multiple inactive symbols then I even see the number of pages is incremented but selecting the last page I'll get empty screen without any indication what's wrong.

image

@habibalkhabbaz
Copy link
Contributor Author

@habibalkhabbaz It will be also useful to add new symbol to main screen immediatelly to be visible. Currently it is waiting for the first price change which will make this indication useless. If I add multiple inactive symbols then I even see the number of pages is incremented but selecting the last page I'll get empty screen without any indication what's wrong.

image

You are right @uhliksk
We may have to call the queue whenever new symbol added.
Give me sometime to work on this.

@habibalkhabbaz
Copy link
Contributor Author

Done @uhliksk @chrisleekr
Now it will call queue.executeFor just after populating candles to mongo DB.
This will ensure that all symbols will be inserted into the trailing-trade-cache collection even if no ticker is received and it will show up on the frontend immediately.

@uhliksk
Copy link
Contributor

uhliksk commented Aug 18, 2022

Great job @habibalkhabbaz!

habibalkhabbaz added a commit to habibalkhabbaz/binance-trading-bot that referenced this pull request Aug 18, 2022
@chrisleekr
Copy link
Owner

@habibalkhabbaz @uhliksk
Hey guys,

Is this ready to review/merge?

@chrisleekr chrisleekr added bug Something isn't working enhancement New feature or request labels Aug 19, 2022
@chrisleekr chrisleekr linked an issue Aug 19, 2022 that may be closed by this pull request
@habibalkhabbaz
Copy link
Contributor Author

@habibalkhabbaz @uhliksk

Hey guys,

Is this ready to review/merge?

Hello @chrisleekr
Sure, it's ready for review.

<Popover.Content>
{symbol} exists in your monitoring list. However, it is not
active on Binance due to emergency downtime, due to it was
actually delisted or due to market move too fast. For more
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERY GOOD!! 💯

@chrisleekr
Copy link
Owner

@habibalkhabbaz

I love it.

Merging now.

@chrisleekr chrisleekr merged commit e0471e5 into chrisleekr:master Aug 19, 2022
@habibalkhabbaz habibalkhabbaz deleted the fix/non-active-exchange-symbols branch October 7, 2022 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inactive symbols suggested in Global Settings
3 participants