Skip to content

Fix: ipv4/ipv6_contactable metrics never updated#300

Open
ackintosh wants to merge 6 commits into
sigp:masterfrom
ackintosh:fix/connectivity-state-is-not-updated
Open

Fix: ipv4/ipv6_contactable metrics never updated#300
ackintosh wants to merge 6 commits into
sigp:masterfrom
ackintosh:fix/connectivity-state-is-not-updated

Conversation

@ackintosh
Copy link
Copy Markdown
Member

@ackintosh ackintosh commented Apr 26, 2026

Description

  • Fix METRICS.ipv4_contactable and METRICS.ipv6_contactable never being updated due to an inverted condition in Config::build()
  • Add regression tests for both metrics
  • Fix cargo clippy errors
    • Suppress clippy::collapsible_match with explanatory comments where PendingEntry::value()requires&mut self` and cannot be used in a pattern guard
    • (Edit) The clippy fixes have been split into Fix clippy errors #301

Background

Config::build() contained an inverted boolean condition:

// Intended: disable auto-NAT when enr_update is false
if self.config.enr_update {   // ← should be `if !self.config.enr_update`
    self.config.auto_nat_listen_duration = None;
}

Because enr_update defaults to true, this condition was always true, so auto_nat_listen_duration was unconditionally overwritten with None. As a result:

  • ConnectivityState never started an incoming-connection timer
  • received_incoming_connection() always returned early
  • METRICS.ipv4_contactable and METRICS.ipv6_contactable remained false permanently regardless of actual reachability

Notes & open questions

Change checklist

  • Self-review
  • Documentation updates if relevant
  • Tests if relevant

…pdated

Reproduces a bug in Config::build() where the inverted condition
`if self.config.enr_update` always sets auto_nat_listen_duration to None
(since enr_update defaults to true), preventing the auto-NAT timer from
ever starting and leaving ipv4_contactable/ipv6_contactable permanently false.
@ackintosh ackintosh force-pushed the fix/connectivity-state-is-not-updated branch from 1e82d6c to cb3a6f6 Compare April 26, 2026 21:45
@ackintosh ackintosh marked this pull request as ready for review April 26, 2026 22:39
@ackintosh ackintosh added the bug Something isn't working label Apr 26, 2026
@ackintosh ackintosh changed the title Fix: connectivity state is not updated Fix: ipv4/ipv6_contactable metrics never updated Apr 26, 2026
Copy link
Copy Markdown
Member

@jxs jxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, Akihito, thanks for looking into this! Can we separate the clippy PR into a separate PR? I am also wondering if we can test in a more isolated way as this test is quite complex and needs flags to not affect other tests

@jxs
Copy link
Copy Markdown
Member

jxs commented May 11, 2026

can you solve the merge conflicts @ackintosh? Should be good to go after

@ackintosh
Copy link
Copy Markdown
Member Author

@jxs I've updated this PR, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants