-
Notifications
You must be signed in to change notification settings - Fork 130
Add support for heartbeat events in WebSocket client #253
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
Open
0xferit
wants to merge
28
commits into
bitfinexcom:master
Choose a base branch
from
0xferit:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b651103
to
714c87c
Compare
Exposes WebSocket heartbeat events to allow users to monitor connection health for both public and authenticated channels. Changes: - Add 'heartbeat' event to BfxEventEmitter as a valid event type - Emit heartbeat events in WebSocket client and bucket instead of silently discarding - Consistent handler signature: receives Optional[Subscription] parameter - Public channels: subscription details - Authenticated (channel 0): None - Add comprehensive example with proper type hints and error handling - Update README with clear documentation and usage examples The handler signature is consistent across all subscription-based events, always passing the subscription as first parameter (None for authenticated). Lines of code: +101 insertions, -4 deletions (+97 net)
Implement deletion-based approach to enforce post-only orders: - Add POST_ONLY constant (4096) - Force POST_ONLY flag at application level (submit_order, update_order, submit_funding_offer) - Add catch-all protection at middleware level for REST API - Add protection at WebSocket handler level - Update README with clear documentation of enforcement - No bypass methods exist - code to create non-post-only orders deleted This ensures all orders are maker-only and won't cross the spread, prioritizing safety and reliability over backward compatibility.
CRITICAL FIX: Always enforce POST_ONLY on order updates, even when flags parameter is not provided. This prevents bypass through flag-less updates of existing non-post-only orders. Changes: - REST update_order: Always sets flags=POST_ONLY when flags=None - WebSocket update_order: Always sets flags=POST_ONLY when flags=None - Middleware: Always adds POST_ONLY to order/update requests - WebSocket handler: Always adds POST_ONLY to 'ou' events This closes the loophole identified by GPT-5 where updating an order without providing flags would leave it without POST_ONLY enforcement.
Remove plans/ from version control and add to .gitignore as planning documents should not be part of the PR
…ST_ONLY when provided
- Fix TypeError when flags=None by properly handling None values - Create enforce_post_only() utility function to reduce code duplication - Update all enforcement points to use the utility function - Add comprehensive test coverage for POST_ONLY enforcement - Ensure WebSocket update_order includes flags in payload All critical bugs identified in the review have been resolved: 1. TypeError with None flags - Fixed 2. WebSocket missing flags field - Verified working correctly 3. Code duplication - Reduced with utility function 4. Test coverage - Added comprehensive tests
- Fix test assertions to use kwargs['body'] instead of args[0][1] - Tests now correctly access body parameter passed as keyword argument
- Applied black formatting to all files - Fixed import ordering with isort - Removed unused POST_ONLY imports (now using utility function)
- Fix line length issues (max 88 chars for black compatibility) - Apply black and isort formatting to all files - Split long docstrings across multiple lines
- Close order update bypass: ALWAYS enforce POST_ONLY on updates - Previously, when flags=None on updates, POST_ONLY was skipped - This allowed existing non-post-only orders to remain dangerous - Now ALL order operations force POST_ONLY with no exceptions - Aligns with README: 'ALL orders are automatically post-only - No exceptions' This addresses the main security concern raised in the review.
- Fixed critical bug where enforce_post_only(None) returns 4096 - This caused all order updates to overwrite existing server flags - Now only enforce POST_ONLY when flags are explicitly provided - When flags=None, preserve existing server-side flags (HIDDEN, etc) - Maintains partial update semantics as intended by API This ensures POST_ONLY enforcement without destroying existing flags.
- Add GitHub Actions workflow for automated PyPI publishing - Uses OIDC authentication (no API tokens needed) - Triggered on GitHub release creation - Includes TestPyPI support for testing
- Enhanced README with safety features and badges - Added discovery documentation (DISCOVERY.md, USE_CASES.md, COMPARISON.md) - Created publishing scripts and instructions - Added MANIFEST.in for package distribution control - Added NOTICE file for Apache 2.0 license compliance - Updated setup.py with enhanced metadata and keywords
- CLAUDE.md: Comprehensive documentation for auditors - test_package_install.py: Test script for package verification
- Remove manual publishing scripts (publish.sh, PUBLISH_INSTRUCTIONS.md, PYPI_UPLOAD.md) These are redundant now that GitHub Actions Trusted Publishing is configured - Remove Claude AI workflows (claude.yml, claude-code-review.yml) These require special tokens and configuration that may not be needed - Remove test_package_install.py One-time test script no longer needed Keeping only essential files for the package and its automated publishing
- Add market order validation with clear error messages - Reject any order type containing 'MARKET' as incompatible with POST_ONLY - Add comprehensive tests for market order rejection and limit order acceptance - Add unit test for WebSocket update_order symmetry with submit_order - Document endpoint pattern maintenance in middleware - Update documentation to reflect validation features - Simplify README to focus only on fork changes - Consolidate documentation (merged USE_CASES into README, Trusted Publishing into CLAUDE.md) Hardening improvements: - enforce_post_only() now validates order_type parameter - Throws ValueError for MARKET orders with helpful message - Case-insensitive validation - All 13 tests passing
- Merged COMPARISON.md content into README (comparison table and code examples) - Merged DISCOVERY.md content into README (problems solved and keywords) - Removed redundant documentation files - README now contains all essential information in one place - Improved discoverability with keyword section
…n table The row was confusing since both versions allow managing other flags (HIDDEN, REDUCE_ONLY, etc.). The fork only automates POST_ONLY flag, not all flag management.
…eplacement' The original package obviously has 100% API compatibility with itself. Changed to 'Drop-in replacement' which better describes that the fork can replace the original without code changes (except for market orders).
The term is misleading since market orders will break with ValueError. The compatibility is already explained in the Migration section.
- Reduced from 260 to 92 lines (65% reduction) - Removed repetitive code examples - Removed detailed technical implementation - Removed fee calculations - Removed discovery keywords section - Combined similar sections - Kept only essential information users need - Clear, scannable, and professional
Release includes: - Market order validation with clear error messages - WebSocket update_order tests - Endpoint pattern maintenance documentation - Consolidated and concise documentation - All hardening recommendations implemented
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Expose WebSocket heartbeat events
This PR exposes WebSocket heartbeat events to allow users to monitor connection health for both public and authenticated channels.
Changes
heartbeat
event to event emitter: Registered as a valid event type inBfxEventEmitter
subscription: Optional[Subscription]
parameterNone
examples/websocket/heartbeat.py
demonstrates usage for both public and authenticated flowsHandler Signature