fuzz: fuzz connman with non-empty addrman + ASMap #29536

1. 
   brunoerg commented at 9:18 AM on March 2, 2024: contributor

   Motivation

   Currently, we fuzz connman with an addrman from NodeContext. However, fuzzing connman with only empty addrman might not be effective, especially for functions like GetAddresses and other ones that plays with addrman. Also, we do not fuzz connman with ASMap, what would be good for functions that need GetGroup, or even for addrman. Without it, I do not see how effective would be fuzzing ASMapHealthCheck, for example.

   Changes

   • Move AddrManDeterministic and ConsumeNetGroupManager to util.
   • Use ConsumeNetGroupManager in connman target to construct a netgroupmanager and use it for ConnmanTestMsg.
   • Use AddrManDeterministic in connman target to create an addrman. It does not slow down as "filling" the addrman (e.g. with FillAddrman).
   • Add coverage for ASMapHealthCheck.
2. 
   DrahtBot commented at 9:19 AM on March 2, 2024: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, dergoegge, marcofleon
   Concept ACK	vasild

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #28584 (Fuzz: extend CConnman tests by vasild)

   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.

3. DrahtBot added the label Tests on Mar 2, 2024
4. DrahtBot added the label Needs rebase on Jul 4, 2024
5. brunoerg force-pushed on Jul 4, 2024
6. 
   brunoerg commented at 1:35 PM on July 4, 2024: contributor

   Rebased

7. DrahtBot removed the label Needs rebase on Jul 4, 2024
8. DrahtBot added the label Needs rebase on Sep 20, 2024
9. fuzz: move `AddrManDeterministic` to util 0a12cff2a8
10. fuzz: fuzz `connman` with a non-empty addrman fe624631ae
11. fuzz: move `ConsumeNetGroupManager` to util 18c8a0945b
12. fuzz: use `ConsumeNetGroupManager` in connman target 33b0f3ae96
13. fuzz: cover `ASMapHealthCheck` in connman target 552cae243a
14. brunoerg force-pushed on Sep 23, 2024
15. 
    brunoerg commented at 3:00 PM on September 23, 2024: contributor

    Rebased

16. DrahtBot removed the label Needs rebase on Sep 23, 2024
17. 
    maflcko commented at 12:43 PM on September 27, 2024: member

    review ACK 552cae243a1bf26bfec03eccd1458f3bf33e01dc 🏀

    <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 552cae243a1bf26bfec03eccd1458f3bf33e01dc 🏀
    tFXs/RUr0AB7AFd2H1aEtY6GfUxnz/1odsKLdbIAhaxGgcC+WgDLFrfuhZZi7In2em8xbfIB7QL8JmoIpjYOCA==
    

    </details>

18. 
    maflcko commented at 9:04 AM on October 11, 2024: member

    cc @vasild Looks like this conflicts with one of your pulls, so you may be qualified to ack/nack this?

19. 
    vasild commented at 11:03 AM on October 11, 2024: contributor

    Concept ACK

    The conflict with #28584 is only mechanical.

    This PR changes "the contents" of the CConnman and does not add fuzzing to more connman methods (except connman.ASMapHealthCheck()). The main point of this PR is that methods that are fuzzed even before this PR would behave differently if addrman is not empty. Nice.

    #28584 adds fuzzing to methods not fuzzed before: OpenNetworkConnection(), CreateNodeFromAcceptedSocket(), InitBinds() and SocketHandler().

20. dergoegge approved
21. 
    dergoegge commented at 1:07 PM on October 25, 2024: member

    Code review ACK 552cae243a1bf26bfec03eccd1458f3bf33e01dc

22. DrahtBot requested review from vasild on Oct 25, 2024
23. 
    marcofleon commented at 1:54 PM on October 25, 2024: contributor

    Code review ACK 552cae243a1bf26bfec03eccd1458f3bf33e01dc. Changes match the PR description.

24. fanquake merged this on Oct 25, 2024
25. fanquake closed this on Oct 25, 2024

    ---

26. brunoerg deleted the branch on Oct 25, 2024
27. 
    maflcko commented at 5:36 PM on November 6, 2024: member

    https://github.com/bitcoin/bitcoin/issues/31234

28. TheCharlatan referenced this in commit a73b2bd0f0 on Nov 14, 2024
29. bug-castercv502 referenced this in commit fdcc066ca0 on Sep 28, 2025
30. bitcoin locked this on Nov 6, 2025

Review Requested
vasild