Skip to content

Conversation

@sanity
Copy link
Collaborator

@sanity sanity commented Dec 2, 2025

Problem

The test_put_contract_three_hop_returns_response and test_three_node_network_connectivity tests were flaky, timing out intermittently on CI. Investigation revealed that PUT responses were failing to route back through intermediate hops in NAT scenarios.

Root Cause

The build_op_result function in put.rs used msg.target_addr() to determine where to route response messages. However, in NAT scenarios, target_addr() returns None because peers behind NAT don't know their external addresses - their PeerKeyLocation has an unknown socket address.

When target_addr is None, the response has no destination and silently fails to route, causing the test to time out waiting for a response that will never arrive.

Why Tests Were Flaky (Not Always Failing)

The tests simulated NAT scenarios using mock connections where addresses may or may not be fully resolved depending on timing and connection establishment order. When the address happened to be known, the test passed; when unknown, it timed out.

This Solution

The fix is a one-line change: prefer upstream_addr over msg.target_addr() for routing:

// Before: 
let target_addr = msg.as_ref().and_then(|m| m.target_addr());

// After:
let target_addr = upstream_addr.or_else(|| msg.as_ref().and_then(|m| m.target_addr()));

upstream_addr is the actual connection address from which we received the message - it's always available when we need to send a response back. This ensures responses route correctly even when the peer's address isn't recorded in the message metadata.

Testing

  • Unit tests pass locally: cargo test -p freenet --lib
  • The specific flaky tests should now be reliable

[AI-assisted - Claude]

The build_op_result function used msg.target_addr() to determine where
to route response messages. However, in NAT scenarios, the target's
address may be unknown (None) because peers behind NAT don't know their
external addresses.

The upstream_addr parameter is already available and contains the actual
connection address from which we received the message. By preferring
upstream_addr over msg.target_addr(), we ensure responses are routed
correctly even when the peer's address isn't recorded in the message.

This fixes flaky failures in test_put_contract_three_hop_returns_response
and test_three_node_network_connectivity where responses would fail to
route back through intermediate hops.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@sanity sanity enabled auto-merge December 2, 2025 02:28
@sanity sanity added this pull request to the merge queue Dec 2, 2025
@sanity sanity removed this pull request from the merge queue due to a manual request Dec 2, 2025
@sanity sanity merged commit a1b161d into main Dec 2, 2025
8 checks passed
@sanity sanity deleted the fix/put-routing-nat branch December 2, 2025 03:04
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