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

(feat) Adds request rate limit throttling to Injective Perpetual #353

Closed
wants to merge 4 commits into from

Conversation

petioptrv
Copy link

@petioptrv petioptrv commented May 3, 2023

Before submitting this PR, please make sure:

  • Your code builds clean without any errors or warnings
  • You are using approved title ("feat/", "fix/", "docs/", "refactor/")

A description of the changes proposed in the pull request:

This PR introduces rate limiting for Injective perpetual connector SDK calls.

The rate limits are 120 requests per minute per service (gRPC vs HTTP), and only chain calls are limited, with indexer calls being unrestricted.

Tests performed by the developer:

Manually tested that the connector works after the introduction of the changes.

Tips for QA testing:

Please test the connector's operation for several cycles (at least 5m) with several order levels. The rate limits are on 1s basis, so we really just need to make sure that the rate limits are not impairing the connector's operation during any periods of high requests, like during the status polling loop.

[ch-39661]

@petioptrv petioptrv requested review from aarmoa and a team May 3, 2023 08:48
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #39661: Add requests throttling to Injective Perpetual.

@petioptrv petioptrv self-assigned this May 3, 2023
Comment on lines 95 to 96
SECOND = 1
MINUTE = 60
Copy link

Choose a reason for hiding this comment

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

Please use the globals in hummingbot.connector.constants instead of defining them here

Copy link
Author

Choose a reason for hiding this comment

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

Addressed

@mrhzysbl mrhzysbl linked an issue May 4, 2023 that may be closed by this pull request
Copy link

@aarmoa aarmoa left a comment

Choose a reason for hiding this comment

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

Changes look good to me

Copy link

@rxlxrxsx rxlxrxsx left a comment

Choose a reason for hiding this comment

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

Tested and confirmed working by QA.

  • Connected API keys and confirmed working as expected
  • Confirmed wallet and exchange balances are reflecting correct
  • Created a perpetual_mm strategy and went through initial prompts successfully
  • Started the strategy without issues, confirmed orders are created and cancelled based on config
  • Confirmed order book prices is in sync with the exchange
  • Confirmed positions are detected whenever it’s opened or closed
  • Confirmed funding payment is being received by the bot
  • Trade information are correct based on history command output
  • Compared trades in client and exchange, no discrepancies found
  • Confirmed running on multiple order levels works as expected

@petioptrv
Copy link
Author

petioptrv commented May 6, 2023

Closing to re-open towards foundation: hummingbot#6283

@petioptrv petioptrv closed this May 6, 2023
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.

Failed Test #353 - Not cancelling orders simultaneously
4 participants