-
Notifications
You must be signed in to change notification settings - Fork 98
protocol: Bump heartbeat protocol version to disallow connections to previous versions #7138
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
protocol: Bump heartbeat protocol version to disallow connections to previous versions #7138
Conversation
…previous versions
📝 Walkthrough## Walkthrough
The changes update the P2P heartbeat protocol version identifier from "0.1.0" to "0.2.0" in the constants definition, its usage within the P2P network behavior initialization, and the tracing span label in the main P2P event loop. The package version was also incremented from 0.7.1 to 0.7.2. No other logic or control flow was modified.
## Changes
| Files | Change Summary |
|--------------------------------------------|------------------------------------------------------------------------------------------------|
| transport/p2p/src/constants.rs | Renamed and updated the heartbeat protocol version constant from "0.1.0" to "0.2.0". |
| transport/p2p/src/lib.rs | Updated usage of the heartbeat protocol constant to reference the new "0.2.0" version. |
| transport/p2p/src/swarm.rs | Replaced hardcoded heartbeat protocol string in tracing span with the updated constant. |
| transport/p2p/Cargo.toml | Bumped package version from 0.7.1 to 0.7.2. |
## Suggested labels
`crate:core-p2p`
## Suggested reviewers
- NumberFour8
- tolbrino Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (10)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Pull Request Overview
This PR bumps the heartbeat protocol version to disallow connections between incompatible node versions, ensuring only peers running the same protocol variant can communicate.
- Updated the heartbeat protocol constant from v0.1.0 to v0.2.0 in the constants file.
- Updated the corresponding usage in the network behavior implementation to utilize the new protocol version.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
transport/p2p/src/lib.rs | Updated the heartbeat protocol version constant used in the network behavior. |
transport/p2p/src/constants.rs | Replaced the old heartbeat protocol constant with the new version identifier. |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
transport/p2p/src/lib.rs (1)
139-142
: Use the bumped protocol in request-response behavior
TheStreamProtocol::new
call correctly uses the newHOPR_HEARTBEAT_PROTOCOL_V_0_2_0
. It may be valuable to add an integration test to assert that peers using/hopr/heartbeat/0.1.0
are rejected, ensuring the bump disallows old-version connections.Would you like me to draft an integration test fixture for the heartbeat handshake?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
transport/p2p/src/constants.rs
(1 hunks)transport/p2p/src/lib.rs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: hoprd / docker
- GitHub Check: hopli / docker
- GitHub Check: Docs / Rust docs
- GitHub Check: tests-smoke-hopli
- GitHub Check: tests-unit-nightly
- GitHub Check: tests-unit
- GitHub Check: zizmor
- GitHub Check: Linter
🔇 Additional comments (1)
transport/p2p/src/lib.rs (1)
45-45
: Update import to new heartbeat protocol constant
The import statement now points toHOPR_HEARTBEAT_PROTOCOL_V_0_2_0
. Confirm that all related modules reference this updated constant and remove any lingering uses of the old version.
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.
crate version bump missing
If the heartbeat protocol remained unchanged, a v2 release would collide with the current implementation on the
msg/ack
protocol level within the same network.Heartbeat maps the current network to usable peers. If v2 peers (
singapore
) can't connect to the latest release due to protocol version changes, they discard v3 peers from the usability mapping. Conversely, the latest nodes attempt to probe v2 nodes but fail to register them for message sending due to protocol differences.