-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Improve dYdX v3 resilience and reliability #3225
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
base: develop
Are you sure you want to change the base?
Conversation
- Add new error codes for account sequence mismatch (4, 32) and good till block errors (11) - Refactor ACCOUNT_SEQUENCE_MISMATCH_ERROR_CODE to ACCOUNT_SEQUENCE_MISMATCH_ERROR_CODES tuple - Update DYDX_RETRY_ERRORS_GRPC to include both error types for automatic retry - Add DYDXOrderBroadcastError exception class for dYdX-specific broadcast errors - Auto-refresh wallet sequence from chain on account sequence mismatch errors This improves resilience when submitting orders by handling transient gRPC errors and automatically recovering from sequence number mismatches.
- Add new error codes for account sequence mismatch (4, 32) and good till block errors (11) - Refactor ACCOUNT_SEQUENCE_MISMATCH_ERROR_CODE to ACCOUNT_SEQUENCE_MISMATCH_ERROR_CODES tuple - Update DYDX_RETRY_ERRORS_GRPC to include both error types for automatic retry - Add DYDXOrderBroadcastError exception class for dYdX-specific broadcast errors - Auto-refresh wallet sequence from chain on account sequence mismatch errors This improves resilience when submitting orders by handling transient gRPC errors and automatically recovering from sequence number mismatches.
|
Hi @SarunasSS Thanks for the contribution! Looks like the pre-commit failed, but this is easy to fix. Just run the following and push any changes to the same PR: or (if you're on Windows): Also consider running the following to install the git hook which will run the pre-commit automatically on commit: |
cjdsellers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @SarunasSS, great work here!
| before = wallet.sequence | ||
| account = await self.get_account(wallet.address) | ||
| wallet.sequence = account.sequence | ||
| print(f"Account sequence mismatch, refreshing from chain. Current sequence {before}, after {wallet.sequence}", flush=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should convert these three prints to self._logs if we'd like to keep them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They prob should be there at debug level, however there is no log instance in the DYDXAccountGRPCAPI. Feels like there should be one since its a pretty important component.
What should be the proper way to get one there? Pass during construction of it?
| def _stop(self) -> None: | ||
| self._retry_manager_pool.shutdown() | ||
|
|
||
| async def _run_periodic_reconciliation(self) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need additional reconciliation in the adapter itself? we should normalize the events and reports and send these back to the live execution engine.
| if TYPE_CHECKING: | ||
| from nautilus_trader.model.objects import Currency | ||
|
|
||
| # TODO. Make this configurable again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a config option for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah does make sense
Pull Request
NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.
CONTRIBUTING.mdand followed the established practicesSummary
This PR significantly improves the resilience and reliability of the dYdX adapter's order execution by implementing robust gRPC error handling and automatic retry logic. The changes handle account sequence mismatch errors (codes 4, 32) and good-til-block errors (code 11) by automatically refreshing the wallet sequence from the blockchain and retrying failed order broadcasts.
Related Issues/PRs
None
Type of change
Breaking change details (if applicable)
N/A - All changes are backward compatible improvements to error handling and resilience.
Documentation
docs/developer_guide/docs.md)No documentation changes required - internal implementation improvements only
Release notes
RELEASES.mdthat follows the existing conventions (when applicable)Suggested RELEASES.md entry:
Testing
Ensure new or changed logic is covered by tests.
Testing details:
Key Changes Summary
1. Enhanced gRPC Error Handling
Changes:
GOOD_TILL_BLOCK_ERROR_CODE(11) constant for good-til-block errorsACCOUNT_SEQUENCE_MISMATCH_ERROR_CODESto tuple (4, 32) to handle both sequence error codesDYDXOrderBroadcastErrorexception class for dYdX-specific broadcast errorsDYDX_RETRY_ERRORS_GRPCto include all retryable error codes2. Order Cancellation Tracking
Changes:
_track_order_cancel()method to trackBEST_EFFORT_CANCELEDordersOrderCanceledevent when block height exceeds good-til-block4. Order Processing Improvements
Changes:
OrderAcceptedevent when fill received for submitted order5. Retry and Broadcast Logic Refactoring
Files modified:
nautilus_trader/adapters/dydx/execution.pyChanges:
_broadcast_order()method to handle retry logic separately_attempt_order_broadcast()for single broadcast attemptDYDXOrderBroadcastErrorfor consistent error handlingMigration Guide
No migration required - all changes are backward compatible. The dYdX adapter will automatically:
Users may see additional log messages for: