fuzz: version handshake #20370

1. 
   MarcoFalke commented at 1:19 pm on November 11, 2020: member

   Not fuzzing the version handshake will limit fuzz coverage
2. fanquake added the label Tests on Nov 11, 2020
3. 
   practicalswift commented at 1:25 pm on November 11, 2020: contributor

   Concept ACK
4. 
   practicalswift commented at 4:12 pm on November 12, 2020: contributor

   Tested ACK dd8654bcd53dade1f5114d68bb51ef22e585b9ba
5. MarcoFalke added the label Waiting for author on Nov 13, 2020
6. in src/test/fuzz/process_messages.cpp:51 in dd8654bcd5 outdated

   57+        connman.SetPeerPermissionFlags(p2p_node, permission_flags);
   58         p2p_node.fSuccessfullyConnected = true;
   59-        p2p_node.fPauseSend = false;
   60         p2p_node.nVersion = PROTOCOL_VERSION;
   61         p2p_node.SetCommonVersion(PROTOCOL_VERSION);
   62+        p2p_node.fPauseSend = false;
   

   ---

   ---

   

   Crypt-iQ commented at 12:12 pm on November 15, 2020:

   Why was this moved?

   ---

   

   MarcoFalke commented at 5:17 pm on November 15, 2020:

   There are some more changes that I haven’t yet pushed to increase coverage even further. They change the seed format as well, so I am going to mark this as draft for now.

   ---

   

   Crypt-iQ commented at 11:31 pm on November 22, 2020:

   What is the procedure for when a PR needs to modify the fuzzing harness and may invalidate all of the seeds? Should something akin to a database migration be used if possible, applied to the seeds to get at least the same coverage as before?

   ---

   

   MarcoFalke commented at 8:05 am on November 23, 2020:

   The format might be arbitrarily complex (even depending on the consensus rules or other Bitcoin Core code), so I don’t think this can be done automatically in a trivial manner. I mostly do it manually if the fuzz engine can’t figure it out by itself after a bit of CPU time.
7. 
   Crypt-iQ commented at 12:16 pm on November 15, 2020: contributor

   utACK dd8654bcd53dade1f5114d68bb51ef22e585b9ba

   Running now to compare coverage before and after this patch. I like that the fuzzers now have SendMessages coverage.

8. MarcoFalke renamed this:
   fuzz: Permission flags in net_processing fuzzers
   DRAFT fuzz: Permission flags in net_processing fuzzers
   on Nov 15, 2020
9. 
   Crypt-iQ commented at 7:41 pm on November 26, 2020: contributor

   Coverage from dd8654bcd53dade1f5114d68bb51ef22e585b9ba: https://crypt-iq.github.io/20370_review/covrun1/src/net_processing.cpp.gcov.html

   Compared to a recent master commit (should have recorded this…oh well): https://crypt-iq.github.io/20370_review/covrun_master/src/net_processing.cpp.gcov.html

   Coverage goes up because of SendMessages, but ProcessMessage coverage is worse. I think it’s probably due to the seed change.

10. 
    DrahtBot commented at 10:25 pm on December 14, 2020: member

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

    Conflicts

    No conflicts as of last run.

11. MarcoFalke marked this as a draft on Dec 28, 2020
12. MarcoFalke renamed this:
    DRAFT fuzz: Permission flags in net_processing fuzzers
    fuzz: version handshake
    on Jan 15, 2021
13. MarcoFalke removed the label Waiting for author on Jan 15, 2021
14. MarcoFalke force-pushed on Jan 29, 2021
15. MarcoFalke marked this as ready for review on Jan 29, 2021
16. fuzz: version handshake fabce459bb
17. MarcoFalke force-pushed on Feb 11, 2021
18. 
    MarcoFalke commented at 12:20 pm on February 11, 2021: member

    Rebased
19. 
    practicalswift commented at 4:22 pm on February 11, 2021: contributor

    cr ACK fabce459bb44e90dc7ae9c44eeedab707435af5b: patch looks very much correct
20. MarcoFalke merged this on Feb 11, 2021
21. MarcoFalke closed this on Feb 11, 2021

    ---

22. MarcoFalke deleted the branch on Feb 11, 2021
23. sidhujag referenced this in commit 4c4327e623 on Feb 11, 2021
24. DrahtBot locked this on Aug 18, 2022

Contributors
MarcoFalke practicalswift Crypt-iQ DrahtBot

Labels
Tests