fuzz: apply node context reset pattern to p2p_handshake #35141

1. 
   frankomosh commented at 4:28 PM on April 22, 2026: contributor

   Follow-up to #34302. Applies the node context reset pattern from fabf8d1 to p2p_handshake.

   Previous code pattern created local AddrMan and node::Warnings objects, and passed them to PeerManager::make. connman was left holding a dangling reference_wrapper<AddrMan> across iterations, since the local objects destruct at iteration end while connman is global.

   Like in fabf8d1 , reset and reinstall node.addrman and node.peerman on each iteration. This PR also removes includes made unused by this or prior refactors.

2. fuzz: apply node context reset pattern to p2p_handshake

   Apply the node context reset pattern from fabf8d1 to p2p_handshake. Previous pattern created local AddrMan and Warnings objects, leaving connman holding dangling references across iterations. Reset and reinstall node.addrman and node.peerman each iteration so sanitizers can detect stale pointer usage.

   dfe5d6a81d
3. DrahtBot added the label Fuzzing on Apr 22, 2026
4. 
   DrahtBot commented at 4:28 PM on April 22, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

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

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	nervana21, maflcko, sedited
   Concept ACK	brunoerg

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #35114 (test: NodeClockContext follow-ups by seduless)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

5. DrahtBot added the label CI failed on Apr 22, 2026
6. DrahtBot removed the label CI failed on Apr 23, 2026
7. 
   brunoerg commented at 12:55 PM on April 29, 2026: contributor

   Concept ACK on applying same pattern of process_message.

8. 
   nervana21 commented at 2:50 PM on May 18, 2026: contributor

   Concept ACK

9. in src/test/fuzz/p2p_handshake.cpp:49 in dfe5d6a81d

     50 | -    auto netgroupman{NetGroupManager::NoAsmap()};
     51 | -    AddrMan addrman{netgroupman, /*deterministic=*/true, /*consistency_check_ratio=*/0};
     52 | -    auto peerman = PeerManager::make(connman, addrman,
     53 | +    node.banman.reset();
     54 | +    node.addrman.reset();
     55 | +    node.peerman.reset();
   

   ---

   ---

   

   nervana21 commented at 11:29 AM on May 19, 2026:

   Since peerman depends on addrman and banman, would it make sense to hoist reseting peerman to before when banman and addrman are reset?

   ---

   

   frankomosh commented at 6:02 PM on May 19, 2026:

   Ok, this matches the ordering from fabf8d1 as used in process_message.cpp and process_messages.cpp. Am not certain if the ordering matters anyway, but if it does, then maybe it should be addressed across all three harnesses and not diverge just here alone. So I prefer leaving as-is for now

   ---

   

   nervana21 commented at 2:19 PM on May 20, 2026:

   Sounds good!

10. in src/test/fuzz/p2p_handshake.cpp:42 in dfe5d6a81d

      37 | @@ -43,22 +38,26 @@ FUZZ_TARGET(p2p_handshake, .init = ::initialize)
      38 |      SeedRandomStateForTest(SeedRand::ZEROS);
      39 |      FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
      40 |  
      41 | -    auto& connman = static_cast<ConnmanTestMsg&>(*g_setup->m_node.connman);
      42 | -    auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);
      43 | +    auto& node{g_setup->m_node};
      44 | +    auto& connman{static_cast<ConnmanTestMsg&>(*node.connman)};
    

    ---

    ---

    

    nervana21 commented at 11:29 AM on May 19, 2026:

    Would it make sense to also reset connman here as is done in process_message.cpp and process_messages.cpp?

        auto& connman{static_cast<ConnmanTestMsg&>(*node.connman)};
        connman.Reset();
    

    ---

    

    frankomosh commented at 6:05 PM on May 19, 2026:

    I have tried checking but I do not see any of the states which connman.Reset clears (m_addr_response_caches, m_max_outbound_cycle_time, m_private_broadcast.m_outbound_tor_ok_at_least_once, m_private_broadcast.m_num_to_open), that is reachable from this p2p_handshake harness. Or am I missing something?

    ---

    

    nervana21 commented at 11:33 AM on May 21, 2026:

    No you're not missing anything. Just trying to get a better understanding of the code. Your response clears up my misunderstandings. Thanks :)

11. 
    nervana21 commented at 11:45 AM on May 21, 2026: contributor

    tACK dfe5d6a81dc39fb1a5a26490e931a04874c2360d

    This commit applies the same node context reset pattern found in similar areas of the codebase in order to avoid dangling AddrMan references between iterations of the fuzzer.

12. DrahtBot requested review from brunoerg on May 21, 2026
13. 
    maflcko commented at 12:52 PM on May 21, 2026: member

    review ACK dfe5d6a81dc39fb1a5a26490e931a04874c2360d 🦏

    <details><summary>Show signature</summary>

    Signature:

    untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    trusted comment: review ACK dfe5d6a81dc39fb1a5a26490e931a04874c2360d 🦏
    sTXGk1Qteccdi+rGmVqI7TOZOfdCQOpRNHz2fRBEQfHFD8s1HHOySMA/2JUprYrMp8vcXllvltDnJ4OajkznCw==
    

    </details>

14. fanquake requested review from marcofleon on May 21, 2026
15. sedited approved
16. 
    sedited commented at 11:05 AM on May 23, 2026: contributor

    ACK dfe5d6a81dc39fb1a5a26490e931a04874c2360d

17. sedited merged this on May 23, 2026
18. sedited closed this on May 23, 2026

    ---

19. frankomosh deleted the branch on May 23, 2026
20. yuvicc referenced this in commit 9df13f29ba on May 26, 2026
21. 
    marcofleon commented at 4:41 PM on May 26, 2026: contributor

    post merge ACK dfe5d6a81dc39fb1a5a26490e931a04874c2360d

Contributors

frankomoshDrahtBotbrunoergnervana21maflckoseditedmarcofleon

Review Requested
brunoerg