refactor(udp-protocol): remove PeerId/PeerClient re-exports#1917
Merged
josecelano merged 3 commits intoJun 17, 2026
Merged
Conversation
The `torrust-tracker-udp-tracker-protocol` crate no longer re-exports `PeerId` and `PeerClient`. All consumers now import directly from `torrust-peer-id`. This removes the #1 thin dependency identified in the workspace coupling report (2026-06-10): `axum-http-server` no longer depends on `udp-protocol` entirely — the dependency was removed from `packages/axum-http-server/Cargo.toml`. Also fixes `console/tracker-client` which was an additional consumer beyond the original 4 listed in the issue scope. Closes torrust#1907.
There was a problem hiding this comment.
Pull request overview
This PR targets issue #1907 by decoupling workspace crates from torrust-tracker-udp-tracker-protocol’s PeerId/PeerClient surface area, updating consumers to import these types directly from torrust-peer-id.
Changes:
- Updated workspace consumers (servers/clients/tests/console client) to import
PeerId/PeerClientfromtorrust_peer_id. - Added
torrust-peer-iddependencies where needed and removed the now-unnecessary udp-protocol dependency fromaxum-http-server. - Adjusted udp-protocol exports and updated the issue spec verification checklist.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/udp-server/src/statistics/event/handler/error.rs | Switches PeerClient import to torrust_peer_id. |
| packages/udp-server/Cargo.toml | Adds torrust-peer-id dependency. |
| packages/udp-protocol/src/lib.rs | Removes direct crate-root PeerId/PeerClient re-export line. |
| packages/udp-protocol/src/common.rs | Changes PeerId/PeerClient exposure point (currently still re-exported publicly). |
| packages/tracker-client/src/peer_id.rs | Switches PeerId import to torrust_peer_id. |
| packages/tracker-client/src/http/client/requests/announce.rs | Switches PeerId import to torrust_peer_id. |
| packages/tracker-client/Cargo.toml | Adds torrust-peer-id dependency. |
| packages/axum-http-server/tests/server/v1/contract.rs | Updates test code to use torrust_peer_id::PeerId. |
| packages/axum-http-server/tests/server/requests/announce.rs | Updates request builder to use torrust_peer_id::PeerId. |
| packages/axum-http-server/Cargo.toml | Removes udp-protocol dependency and adds torrust-peer-id. |
| docs/issues/open/1907-1669-si-26-remove-udp-protocol-peer-id-re-export.md | Marks verification items as completed (needs alignment with actual udp-protocol exports). |
| console/tracker-client/src/console/clients/unified/http.rs | Switches PeerId import to torrust_peer_id. |
| console/tracker-client/src/console/clients/http/app.rs | Switches PeerId import to torrust_peer_id. |
| console/tracker-client/Cargo.toml | Adds torrust-peer-id dependency. |
| Cargo.lock | Updates lockfile for the new dependency edges. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1917 +/- ##
========================================
Coverage 79.09% 79.10%
========================================
Files 326 326
Lines 22806 22806
Branches 22806 22806
========================================
+ Hits 18038 18040 +2
+ Misses 4505 4504 -1
+ Partials 263 262 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The `pub use torrust_peer_id::{PeerClient, PeerId}` in `common.rs` was
still publicly re-exporting these types, which meant the crate's public
API was still coupled to `torrust-peer-id` despite the lib.rs re-exports
being removed.
Change summary:
- Made the re-exports in `common.rs` `pub(crate)` so they're only
available internally, not in crate's public API
- Fixed all remaining consumers that imported `PeerId` from
`udp-protocol` to import from `torrust_peer_id` instead
Consumers fixed:
- `packages/udp-server/src/handlers/announce.rs` (5 occurrences)
- `packages/udp-server/src/handlers/scrape.rs`
- `packages/udp-server/tests/server/contract.rs`
- `console/tracker-client/src/console/clients/udp/checker.rs`
- Updated doc link in `packages/udp-server/src/lib.rs`
Refs torrust#1907.
josecelano
commented
Jun 17, 2026
josecelano
left a comment
josecelano
left a comment
Member
Author
There was a problem hiding this comment.
The fix commit 72bac14 addresses both Copilot suggestions. The re-export in common.rs is now pub(crate) (not pub), so it is no longer part of the crate's public API. All remaining consumers that imported PeerId from torrust_tracker_udp_tracker_protocol have been updated to import from torrust_peer_id instead. I've also replied to each suggestion thread with details.
The AquaticPeerId alias was a legacy from when the code was forked from aquatic_udp_protocol. Now that we use our own torrust_peer_id::PeerId, the alias is unnecessary.
Member
Author
|
ACK 98d8979 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
packages/udp-server/src/lib.rs:336
- The AnnounceRequest field table has incorrect type/link entries:
bytes_uploadedis labeled asTransactionIdeven though it links toNumberOfBytes, andip_addresslinks toConnectionIdand showsNoneeven thoughAnnounceRequest.ip_addressis anIpv4AddrBytes(see udp-protocol AnnounceRequest). This can mislead readers of the UDP server docs.
//! `peer_id` | [`PeerId`](torrust_peer_id::PeerId) | `[45,113,66,52,52,49,48,45,41,83,100,126,100,101,52,120,77,112,54,68]`
//! `bytes_downloaded` | [`NumberOfBytes`](torrust_tracker_udp_tracker_protocol::common::NumberOfBytes) | `0`
//! `bytes_uploaded` | [`TransactionId`](torrust_tracker_udp_tracker_protocol::common::NumberOfBytes) | `0`
//! `event` | [`AnnounceEvent`](torrust_tracker_udp_tracker_protocol::AnnounceEvent) | `Started`
//! `ip_address` | [`Ipv4Addr`](torrust_tracker_udp_tracker_protocol::common::ConnectionId) | `None`
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
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.
Closes #1907