p2p: saturate LocalServiceInfo::nScore to prevent overflow #34028

1. 
   codeabysss commented at 8:08 pm on December 7, 2025: none

   This prevents signed integer overflow in LocalServiceInfo::nScore by saturating at std::numeric_limits<int>::max() in SeenLocal().

   The nScore field could overflow from INT_MAX to INT_MIN when incremented during version handshakes, causing undefined behavior.

   I implemented saturation in SeenLocal() to cap nScore at std::numeric_limits<int>::max() instead of allowing overflow. This prevents undefined behavior while maintaining memory efficiency - after 2 billion connections, the matter is settled.

   • Added test case LocalAddress_nScore_Overflow that verifies saturation behavior
   • Test confirms nScore remains at max value after multiple SeenLocal() calls

   Fixes #24049

2. saturate LocalServiceInfo::nScore to prevent overflow f11b165f35
3. 
   DrahtBot commented at 8:08 pm on December 7, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34028.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   LLM Linter (✨ experimental)

   Possible places where named args may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

   • UtilBuildAddress(0x002, 0x001, 0x001, 0x001) in src/test/net_tests.cpp

   2025-12-07

4. fanquake renamed this:
   [p2p] Saturate LocalServiceInfo::nScore to prevent overflow
   p2p: saturate LocalServiceInfo::nScore to prevent overflow
   on Dec 8, 2025
5. DrahtBot added the label P2P on Dec 8, 2025
6. in src/net.cpp:324 in f11b165f35

   319@@ -320,7 +320,10 @@ bool SeenLocal(const CService& addr)
   320     LOCK(g_maplocalhost_mutex);
   321     const auto it = mapLocalHost.find(addr);
   322     if (it == mapLocalHost.end()) return false;
   323-    ++it->second.nScore;
   324+    
   325+    if (it->second.nScore < std::numeric_limits<int>::max()) {
   

   ---

   ---

   

   brunoerg commented at 5:45 pm on December 10, 2025:

   I just ran a mutation test for this PR, here’s a mutant that wasn’t killed and might be worth addressing (writing tests that can detect this change):

    0diff --git a/src/net.cpp b/src/net.cpp
    1index bbe24c8d3e..ab159e0123 100644
    2--- a/src/net.cpp
    3+++ b/src/net.cpp
    4@@ -321,7 +321,7 @@ bool SeenLocal(const CService& addr)
    5     const auto it = mapLocalHost.find(addr);
    6     if (it == mapLocalHost.end()) return false;
    7
    8-    if (it->second.nScore < std::numeric_limits<int>::max()) {
    9+    if (it->second.nScore >  std::numeric_limits<int>::max()) {
   10         ++it->second.nScore;
   11     }
   12     return true;
   

7. DrahtBot added the label CI failed on Dec 15, 2025
8. 
   DrahtBot commented at 5:31 pm on December 15, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/20009656888/job/58081216641 LLM reason (✨ experimental): Lint check failed due to trailing whitespace detected (trailing_whitespace) in the source, causing the docker lint run to exit with code 1.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

Contributors
codeabysss DrahtBot brunoerg

Labels
P2P CI failed