Skip to content

Conversation

@sanity
Copy link
Collaborator

@sanity sanity commented Nov 18, 2025

Problem

  • Bootstrap join could double-dial the same gateway, leaking reserved connection slots and spamming duplicate handshakes (issue Observing high send packet rate #2092).
  • In the decoupled pipeline we were converting op results inside MessageProcessor, so errors were turned into generic client errors and host results could be dropped before reaching the session actor.

This Solution

  • Guard ConnectionManager reservations against duplicate peers, expose helpers for reserved counts/known peers, and skip join attempts when a gateway is already pending/connected or another outbound handshake is reserved.
  • Track in-flight gateway dials during initial join to avoid parallel duplicate attempts, and gate subscription child tracking so GET keeps parent linkage while PUT avoids unnecessary edges.
  • Build HostResult in the network path and route it directly to MessageProcessor; keep warnings when the session adapter is missing but quiet noisy success logs.

Testing

  • cargo test -p freenet message_processor
  • cargo test -p freenet connection_manager::tests::should_accept_does_not_leak_reservations_for_duplicate_peer

Copilot finished reviewing on behalf of sanity November 19, 2025 00:58
Copy link
Contributor

Copilot AI left a 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 addresses issues with duplicate gateway connections during bootstrap and improves the routing of operation results to the session actor system.

Key Changes:

  • Added duplicate peer detection in ConnectionManager.should_accept() to prevent reservation leaks when the same peer is processed multiple times
  • Implemented in-flight gateway tracking during bootstrap to prevent parallel duplicate connection attempts
  • Refactored host result creation to occur in the network layer before being routed to MessageProcessor, ensuring proper error handling and result delivery

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/core/src/ring/connection_manager.rs Added early duplicate peer check, helper methods get_reserved_connections() and has_known_peer(), and test coverage for reservation leak prevention
crates/core/src/operations/put.rs Updated subscription requests to use start_subscription_request_internal with track_parent=false to avoid unnecessary parent tracking edges
crates/core/src/operations/mod.rs Introduced start_subscription_request_internal to allow callers to opt out of parent tracking when spawning subscription operations
crates/core/src/operations/connect.rs Added duplicate peer check in join_ring_request and in-flight gateway tracking in initial_join_procedure to prevent concurrent duplicate dials
crates/core/src/node/mod.rs Refactored process_message_decoupled to build host results before calling MessageProcessor and cleaned up executor callback parameter threading
crates/core/src/node/message_processor.rs Updated handle_network_result to accept HostResult instead of OpEnum, simplifying the interface and improving separation of concerns
crates/core/src/contract/handler.rs Added debug logging for transaction registration and warnings when session adapter is not installed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@iduartgomez
Copy link
Collaborator

iduartgomez commented Nov 19, 2025

the op results look good but they are mixed with the changes to avoid more than one concurrent gateway connection op at the same time (which seem odd)

@sanity
Copy link
Collaborator Author

sanity commented Nov 19, 2025

@iduartgomez good call. The extra check was serialising the bootstrap loop whenever we already had a reservation, which wasn’t necessary. I’ve removed that guard so we keep dialing distinct gateways in parallel while still deduping per-peer using the in-flight map plus the connection manager’s duplicate check. CI is green again.

@sanity sanity added this pull request to the merge queue Nov 20, 2025
Merged via the queue into main with commit 60301a1 Nov 20, 2025
11 checks passed
@sanity sanity deleted the issue-2092-v3 branch November 20, 2025 18:24
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.

3 participants