fix(service-disco): initialise the network size estimator

[gmelodie]

Summary

ServiceDiscovery.new built the KadDHT base object field by field and missed nsEstimator. KadDHT.new set that field, so only a node that mounts service discovery carried a nil estimator.

Every provider path reads the estimator, so a provide on such a node segfaults:

• addProvider calls kad.nsEstimator.networkSize() at libp2p/protocols/kademlia/provider.nim:314.
• maybeTrackNetsize calls kad.nsEstimator.track() at libp2p/protocols/kademlia/provider.nim:252.

The crash happens with optimisticProvide on and off, because maybeTrackNetsize runs on both branches. This is the downstream trace, from the logos-libp2p-module tutorial job, which calls startProviding through the C bindings on a node that mounts service discovery:

cbind/libp2p.nim(852) libp2pKadStartProviding
libp2p/protocols/kademlia/provider.nim(339) startProviding
libp2p/protocols/kademlia/provider.nim(314) addProvider
libp2p/protocols/kademlia/netsize.nim(89) networkSize
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Two constructors that build the same base object are the reason one field went missing, so the fix removes the duplication instead of patching one line. initKadBase in libp2p/protocols/kademlia.nim now sets every shared field, including the routing table both constructors built from the same three config values. KadDHT.new and ServiceDiscovery.new both call it, and ServiceDiscovery.new keeps only its own fields. The next base field reaches both protocols on its own.

Affected Areas

• Peer Management / Discovery
• Protocol Logic

KadDHT and ServiceDiscovery construction, and the Kademlia provider paths a service discovery node reaches.

Compatibility & Downstream Validation

The signatures of KadDHT.new and ServiceDiscovery.new do not change, and initKadBase sets the same fields to the same values the two constructors set before, so no downstream project needs an adaptation.

• Nimbus: not run. The diff changes no KadDHT.new behaviour, so a Nimbus node observes no difference.
• Waku: not run. Same reason.
• Codex: not run. Same reason.
• logos-libp2p-module: the consumer that hit the crash. Its tutorial job segfaults on the current master pin. See fix(service-disco): advertise a caller-supplied XPR logos-co/logos-libp2p-module#99.

Impact on Library Users

No API change. A ServiceDiscovery node now provides keys instead of crashing. A user who avoided startProviding and addProvider on a service discovery node to dodge the crash can drop that workaround.

Risk Assessment

Low. initKadBase assigns the fields the two constructors assigned inline, with the same expressions. The estimator starts empty, so networkSize returns "not enough data" until enough lookups converge, which is the state a plain KadDHT node already starts in. The new indirection runs one time per node, at construction.

The refactor touches KadDHT.new, which every Kademlia user reaches, so the field-by-field diff is worth a close read.

References

• fix(service-disco): advertise a caller-supplied XPR logos-co/logos-libp2p-module#99 — the PR that hits the crash.
• feat(kad): send optimistic ADD_PROVIDER during lookup #2911 — added the estimator and the optimistic provide path.

Additional Notes

tests/libp2p/service_discovery/component/test_add_provider.nim provides a key between two service discovery nodes, which is the shortest path to the same crash. It reproduces the SIGSEGV on master and passes with the fix.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.20%. Comparing base (00cab64) to head (5ad6b88).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2920      +/-   ##
==========================================
- Coverage   82.21%   82.20%   -0.02%     
==========================================
  Files         175      175              
  Lines       31559    31585      +26     
  Branches       13       12       -1     
==========================================
+ Hits        25945    25963      +18     
- Misses       5614     5622       +8     

Files with missing lines	Coverage Δ
libp2p/protocols/kademlia.nim	67.71% <100.00%> (-1.68%)	⬇️
libp2p/protocols/service_discovery.nim	84.91% <100.00%> (+0.93%)	⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vladopajic]

isServer = not isClient

in general code shouldn't really do twists like this,