fix(peerinfo): serve inbound requests on Inactive connections

[iamquang95]

Summary

peerinfo's ConnectionHandler::poll gated on connection state first, so an
Inactive connection returned before handling inbound requests. Inactive only
means the remote doesn't serve peerinfo (our outbound negotiation failed) — but
the gate order meant we also stopped answering that peer's inbound polls.

The charon relay is exactly this peer: it polls peerinfo as a client and never
serves it, so our outbound to it fails → connection goes Inactive → we stop
answering the relay.

Change

Reorder poll in crates/peerinfo/src/handler.rs: move the state gate below
inbound handling. Inbound is served unconditionally; the gate now governs only
outbound. Gate is kept (not removed) so Inactive still stops dialing and reports
Unsupported once, avoiding a tight retry loop. One file: the reorder + a test.

Impact if not fixed

Every relay→node peerinfo poll times out: logs flood with Peerinfo failed: read response warnings (the smoke-test noise) and relay-side peer metadata
(version/git-hash/RTT) never populates. Healthy node↔node exchanges unaffected.

[iamquang95]

@claude review this PR

[claude]

Claude finished @iamquang95's task in 1m 45s —— View job

---

Review: PR #542 — serve inbound peerinfo on Inactive connections

• Read the changed handler code and surrounding context
• Compare against behaviour/protocol wiring
• Assess correctness, edge cases, and test quality
• Post review

---

Verdict: LGTM. The reorder is correct, minimal, and well-reasoned, and the regression test pins exactly the behavior that was broken. Details below.

Why the fix is correct

The root cause is accurate: State::Inactive is set only in on_dial_upgrade_error on StreamUpgradeError::NegotiationFailed (handler.rs:108-111), i.e. it describes the remote's failure to serve our outbound negotiation. It says nothing about our ability to answer that peer's inbound polls. The old ordering conflated the two by returning before inbound was ever driven.

Moving inbound handling above the state gate (handler.rs:150-164) and scoping the gate to outbound only (handler.rs:169-180) fixes this cleanly. Inbound is served unconditionally; the gate still stops outbound dialing and reports Failure::Unsupported exactly once.

No regression on Unsupported reporting

Worth calling out since it's the subtle risk in a reorder: inbound handling never returns early from poll — it only advances self.inbound — so control still falls through to the gate in the same call. Inactive { reported: false } therefore still emits NotifyBehaviour(Err(Unsupported)) on the same poll and flips reported to true, avoiding the tight retry loop. The one-shot semantics are preserved. ✅

Wakeup safety

After the inbound future completes it's cleared to None and, on Inactive { reported: true }, poll returns Poll::Pending without registering a new waker in that call. This is fine: the next inbound stream arrives via on_connection_event → FullyNegotiatedInbound (handler.rs:245-253), which repopulates self.inbound and the libp2p connection machinery re-polls the handler. No lost-wakeup here. ✅

Test quality

inactive_handler_still_services_inbound is a good, targeted regression test:

• Uses Inactive { reported: true } so the only observable effect of poll is whether inbound is driven — a clean isolation of the bug.
• The future::lazy(...) -> Err(...) trick sidesteps constructing a real Stream while still exercising the completion/clear path, and asserts inbound.is_none() afterwards.
• Correctly asserts the poll still returns Pending (outbound remains gated).
• It genuinely fails on the pre-fix ordering, which is what a regression test should do.

Minor notes (non-blocking)

• No coverage for Inactive { reported: false } + inbound. The test only covers reported: true. A second case asserting that inbound is serviced and NotifyBehaviour(Err(Unsupported)) is still returned in the same poll would lock in the "no regression on reporting" property described above. Optional, but it's the other half of the behavior this PR touches.
• Inbound success is silent to the behaviour (the Ok((_stream, _request)) arm just clears self.inbound and emits no event). That's consistent with pre-existing behavior — the serving side doesn't record peer metadata — so no change needed, just confirming it's intentional.
• This handler is modeled on rust-libp2p's ping, not a direct port of Charon's Go peerinfo, so there's no Go behavior to diff against here; the fix is Rust/libp2p-state-machine-specific and internally consistent.

I could not run cargo test -p peerinfo / clippy in this environment (command approval required), so I reviewed by reading. The change is small and the logic checks out; a green CI run on the test + clippy is the only remaining confirmation.
· feat/peerinfo-inbound