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

Bitfinex sync fix #555

Merged
merged 4 commits into from Oct 9, 2017
Merged

Bitfinex sync fix #555

merged 4 commits into from Oct 9, 2017

Conversation

rnevet
Copy link
Collaborator

@rnevet rnevet commented Oct 5, 2017

Fixes Bitfinex API synchronization
This will prevent nonce errors and might mitigate ERR_RATE_LIMIT (#490)

Description

Move API rate limit and sync to get & post calls

TESTING STAGE

In progress

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read CONTRIBUTING.md
  • I fully understand Github Flow.
  • My code adheres to the code style of this project.
  • [N/A ] I have updated the documentation in /docs if I have changed the config, arguments, logic in how the bot works, or anything that understandably needs a documentation change.
  • [N/A ] I have updated the config file accordingly if my change requires a new configuration setting or changes an existing one.
  • I have tested the bot with no issues for 24 continuous hours. If issues were experienced, they have been patched and tested again.

@ghost ghost assigned rnevet Oct 5, 2017
@ghost ghost added the Testing Phase label Oct 5, 2017
@rnevet rnevet requested a review from laxdog October 5, 2017 20:57
@rnevet
Copy link
Collaborator Author

rnevet commented Oct 5, 2017

@Grzesiek83 @utdrmac @laxdog @JCBauza would you mind testing
I suspect that the ERR_LIMIT is caused by our bot misbehaving and the root issue is unsynced calls, this PR fixes that, I have implemented a test that does 270 API calls (from 270 threads), it runs in ~3min (i.e. 90 req per sec) and O ran it multiple times without receiving any errors.
This branch is based on master and doesn't have dynamic changing rate.

@laxdog
Copy link
Collaborator

laxdog commented Oct 9, 2017

So this works well with the test, but I'm still hitting the error when I run the bot with it.

We definitely need this code, but we also need the other branch.

I'm going to merge this to master, then rebase the other one.

Copy link
Collaborator

@laxdog laxdog left a comment

Choose a reason for hiding this comment

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

Doesn't completely fix the ERR_RATE_LIMIT issue, but helps towards it.

Definitely needed anyway as the payload needs added at the time we send the request, not after the delay.

@laxdog laxdog merged commit c4209ff into master Oct 9, 2017
@ghost ghost removed the Testing Phase label Oct 9, 2017
@rnevet rnevet deleted the bitfinex_sync_fix branch June 1, 2018 18:51
Evanito pushed a commit that referenced this pull request Jul 20, 2018
* merge upstream (#1)

* Plugin Graphs: make history file configurable (#518)

* make history file configurable

* removed extra paren

* Reduce chances of nonce collisions (#520)

* Check currency is being analysed before trying to get a rate for it (#528)

* Check currency is being analysed before trying to get a rate for it

* Fix Capital C in "Lending.py", line 85 (#529)

* Resolves Issue 526 (#533)

* add dashboard link in navbar
* aggregate correct column

* Fix recorded data when no rate offers exist, don't print traceback unless ma_debug is on (#531)

* Return a dict of all currencies even if balance is 0 (#540)

* Log bot "version" on error handling (#543)

* When logging an exception also log the number of commits on current branch, will allow to easily identify if reporter is on master and updated.

* Move get_git_version to Data module

* Change fallback version number to 3.0.0

* A safer approach for handling errors in the git call

* Html response fix (#546)

* prevent exception overloading when receiving an html response error

* prevent unicode from throwing an exception

* File log using ensure_ascii

* Change min/max daily lend rate depending on exchange (and simplify xdays) (#547)

* Change min/max daily lend rate depending on exchange

* Lending.py

Simplify code with xdays and max/min daily rate

* Update Lending.py

* Update Lending.py

* Bitfinex sync fix (#555)

* Sync Bitfinex API requests completely

* Update test

* Update test

* ERR_RATE_LIMIT_fix (#549)

* Return a dict of all currencies even if balance is 0

* Sleep and reduce requests per min if we get ERR_RATE_LIMIT

* Reduce request window to 1 sec

* Move the timers to millisecond resolution in the coach, MA changes times now when it catches 429

* Comments, logs, fix exception

* Don't change api request times unless using bitfinex

* Update polo coach to milliseconds

* Catch 429 at top level and more sensible api_req_period calc

* PEP-8

* Unrelated bug I found from ERR_RATE_LIMIT_fix

* Sync Bitfinex API requests completely

* Update test

* Update test

* Messed up the merge from bitfinex_sync_fix

* Move timer change methods to base class and catch 429 at API level

* Add poloniex timer changes

* Make sure to re-raise the error, sleep in the MA module not the API.

* Converted a tab to spaces

* Added Blacklist Option & enforce whitelist (#570)

* Added Blacklist Option for Lending

* Mistake in Configuration.py

* Optimized config description

* Optimized description

* Minimized api requests by using blacklist_currencies and all_currencies parameter

* Removed BTG from all_currencies

* Changed to more simple blacklist and changed documentation

* Changed default config

* Changed configuration description

* Addition of a Slack Username parameter as a configuration option. (#576)

* Addition of a Slack Username parameter as a configuration option.

* Backward compatibility for configs which lack a slack_username

* split multiline message into multiple irc messages (#582)

the irc lib in use doesn't support sending multiline messages. this patch splits messages containing newline characters into multiple irc messgaes to resolve issue: #565

* Discover minimum amount in EUR (#594)

I think this solves #589.

* Use docker image with pandas and numpy already installed (#593)

* Fixes absolute reference

This way the webpage will work even if under a directory, otherwise it redirects to the root directory.

* Use FRR on bitfinex as min_daily_rate (#554)

* Return a dict of all currencies even if balance is 0

* Sleep and reduce requests per min if we get ERR_RATE_LIMIT

* Reduce request window to 1 sec

* Move the timers to millisecond resolution in the coach, MA changes times now when it catches 429

* Comments, logs, fix exception

* Don't change api request times unless using bitfinex

* Update polo coach to milliseconds

* Catch 429 at top level and more sensible api_req_period calc

* Use FRR for min_daily_rate

* Missing from last commit

* PEP-8

* Config and docs for frr as min

* Use config for frr and use configured min if higher

* Unrelated bug I found from ERR_RATE_LIMIT_fix

* Unrelated bug I found from ERR_RATE_LIMIT_fix

* Sync Bitfinex API requests completely

* Update test

* Update test

* Messed up the merge from bitfinex_sync_fix

* Messed up the merge from bitfinex_sync_fix

* Move timer change methods to base class and catch 429 at API level

* Add poloniex timer changes

* Make sure to re-raise the error, sleep in the MA module not the API.

* Converted a tab to spaces

* Added FRR Delta for Bitfinex

* Added FRR Delta for Bitfinex

* PEP-8

* Log when using FRR as min daily rate

* Allow config to be picked up from 'BOT' section rather than just coin_cfg

* Declare variables to allow global access

* This is why we should use classes!

* exchangeMax needs defined

* add larger sleeps to prevent API bans

* only display active coins in charts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants