Skip to content

Conversation

@sanity
Copy link
Collaborator

@sanity sanity commented Nov 25, 2025

Problem

The test_three_node_network_connectivity test was failing in the release/v0.1.39 CI. This failure was caused by the interaction between PR #2140 (self-connection guard) and PR #2142 (response_target fix).

Root cause: The self-connection guard in should_accept() only checked peer_key, which can be None early during node initialization. When the gateway received a connection request that referenced itself, the guard failed to trigger because get_peer_key() returned None.

Investigation findings:

Solution

Add a secondary self-connection check that compares pub_key directly, which is always available in ConnectionManager:

// Primary check: compare full PeerId (address-based equality)
if let Some(own_id) = self.get_peer_key() {
    if &own_id == peer_id {
        return false;
    }
}

// Secondary check: compare pub_key directly
// This catches self-connections even when peer_key is not yet set
if *self.pub_key == peer_id.pub_key {
    return false;
}

Testing

  • ✅ Added unit test rejects_self_connection_by_pubkey_when_peer_key_not_set
  • ✅ All existing unit tests pass
  • test_three_node_network_connectivity now passes locally

Fixes

This should unblock the release/v0.1.39 CI.

[AI-assisted - Claude]

sanity and others added 2 commits November 24, 2025 23:02
- freenet: → 0.1.39
- fdev: → 0.3.17

🤖 Automated release commit
The previous self-connection guard only checked peer_key, which can be
None early during initialization. This caused self-connections to slip
through when combined with PR #2142's response_target change.

The fix adds a secondary check that compares pub_key directly. Since
pub_key is always set in ConnectionManager, this catches self-connections
even when peer_key is not yet initialized.

This fixes the test_three_node_network_connectivity failure in the
release/v0.1.39 CI.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@sanity sanity enabled auto-merge November 25, 2025 05:33
@sanity sanity disabled auto-merge November 25, 2025 05:36
@sanity sanity closed this Nov 25, 2025
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.

2 participants