fuzz: Faster wallet_notifications target #28933

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2311-fuzz-wallet-notif- changing 2 files +129 −36
  1. maflcko commented at 12:13 pm on November 24, 2023: member
    Avoid read/write from storage to speed the target up.
  2. Export assert from util/check.h 
    This avoids having to include both headers when assert and Assert are
used at the same time.
    fa971c09f2
  3. DrahtBot commented at 12:13 pm on November 24, 2023: contributor

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

    Code Coverage

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK dergoegge, brunoerg

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

  4. DrahtBot added the label Tests on Nov 24, 2023
  5. maflcko force-pushed on Nov 24, 2023
  6. DrahtBot added the label CI failed on Nov 24, 2023
  7. dergoegge approved
  8. dergoegge commented at 1:44 pm on November 24, 2023: member

    Code review ACK fa4fc99b6e2a13736521e6bca1f626ba1d2f59e3

    With the mocked database this shouldn’t be going to disk at all now, right?

  9. DrahtBot removed the label CI failed on Nov 24, 2023
  10. maflcko commented at 1:51 pm on November 24, 2023: member

    With the mocked database this shouldn’t be going to disk at all now, right?

    Yeah, I guess so. (A datadir is still created, though)

  11. dergoegge commented at 2:00 pm on November 24, 2023: member

    A datadir is still created, though

    Is that necessary or just a side effect from using TestingSetup?

  12. brunoerg commented at 2:03 pm on November 24, 2023: contributor

    Concept ACK

    will review soon!

  13. maflcko commented at 2:18 pm on November 24, 2023: member

    Is that necessary or just a side effect from using TestingSetup?

    Both. For one, validation can not exist without datadir. Also, it is created in TestingSetup. But it shouldn’t cause any issues, because it will be empty.

  14. dergoegge commented at 2:20 pm on November 24, 2023: member

    But it shouldn’t cause any issues, because it will be empty.

    Empty dirs aren’t of size zero and #28811 would still apply (given more timeouts/crashes).

    It’s a bit of a shame if it’s always empty to still create it, but not an issue for this PR in any case.

  15. maflcko commented at 2:39 pm on November 24, 2023: member
    Are there any timeouts left at this point? If yes, it may be good to report them.
  16. dergoegge commented at 3:03 pm on November 24, 2023: member

    Are there any timeouts left at this point? If yes, it may be good to report them.

    We’ll see I guess, initial exec/s looked good but they have now deteriorated to 3 exec/s (per core). No timeouts so far though and CPU utilization is at 100% on all cores, so this is a definite improvement.

  17. brunoerg commented at 3:34 pm on November 24, 2023: contributor
    Is there any benefit about using this ImportDescriptor approach compared to simply use SetupDescriptorScriptPubKeyMans?
  18. maflcko commented at 3:43 pm on November 24, 2023: member

    SetupScriptPubKeyMans

    Which function do you mean?

    0$ git grep SetupScriptPubKeyMans | wc -l
10
  19. brunoerg commented at 3:47 pm on November 24, 2023: contributor

    Which function do you mean?

    Sorry, edited that. It’s SetupDescriptorScriptPubKeyMans.

  20. brunoerg approved
  21. brunoerg commented at 5:23 pm on November 24, 2023: contributor
    ACK fa4fc99b6e2a13736521e6bca1f626ba1d2f59e3
  22. dergoegge commented at 10:48 am on November 27, 2023: member

    I’ve run into timeouts.

     0QV9BKr6un2BgAAD/S6+fn2CAYJ9gn5+f3wCABP+fn2Bg/489AGCf8AAAJn8AAAB/AJ9kYJ+fk0FB
 1QQAZAADBvkFEIiT5////IiIjIiIgvF1G/90AAOR/D/+7QUFBPb1hvkFBV0G+Qfi+4IJBQQcjByIg
 2jf4iIiISDwAAAAB///+7VBC1RCIk+f///yIiIyIiILxdRv/dAADkfw//u5gjRLW73dsG//8gIiIj
 3IgAAQeDh4eHh4eEtLUEtLQMaGgMeKSopKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSEpKSkpKSkp
 4KSkpKRgpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
 5KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKTkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
 6KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSlFKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
 7KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKdbWKSkpKSkpKSkpKSkpKSkpKSkpKSkp
 8KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
 9yBID/wAC0QMDQQNBAQBBTmRCKSkpKSkpKSkpKSkpKSkpKSkpKSkpKTIpKSkpKSkpKSkpKSkpKSkp
10KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
11KSkpKSkpKSkpKSkpKSkpKSkpHykpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSki
12KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKQ4pKSkpKSkpKSkpKSkpKSkp
13KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpDikpKSkpKSkpKSkpKSkpKSlLKSkpKSkpKSkpKSkpKSkp
14KSkpKSkpKSkpyBID/wAC0QMDQQNBAQBBTkFCCgMbAwMEABQDABADZAMDA0EAAAMDAwMKAx1DQ0Nm
15f34mQ0FkAwMDQQAAAwMDA0ED738AAEEDAuETGxTBwcHBwcHBwcHBwcHBwcHBwcEDABADQgoDZAP/
16/wNBDQMDKSkpKSkpKSkpKSkpLikpKSkpCikpKX//KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
17KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
18KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpSSkpKSkpKSkpCCkpKSkpKSkpKSkpKSkp
19KSkpKSkpKSkpKUQpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKS8pKSkpKSkpKSkpKSkpKSopKSkp
20KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
21KSkpKSkpKSkpKSkpKSkpKSkpKSkpKTkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKX8pKWkpKSkp
22KSkpKSkpKSkpKSkpGCkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
23KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
24KdbWKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSopKSkpKSkpKSkpKSkpKSkp
25SSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
26KSkpKSkpKSkpKSkpKSkpKSkpKSopKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
27KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
28KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
29KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
30KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
31KSkpKSkpKSkpKdYpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
32KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSlCKSkpKSkpKSkpKSkpKSkpKSkpKSkp
33KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
34KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
35KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
36KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKcgS/AAAAtEDA0EDQQEAQU5k
37QgoDGwMDBAAUAwAQA2QDAwNBAAADAwMDCnsdQ0NDZn9+JkNBZAMDA0EAAAMDAwNBA/9/AABBAwLh
38EwMUwcHBwcHBwcHBwcHBwcHBwcHBAwAQA0IKA5QD//8DQQ0DAykpKSkpKSkQKSkpKSkpKSkpKSkp
39KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
40KSkpKSkpKSkpKSkpKSkpKSkpSSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
41KSkpKSkpDEkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
42KSkpKSkpfwAAACkpKSkpKSkpKSkpKSkpKSkqKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
43KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkp
44KSkpKSkpKSkpKSkpKSkpKSkpKSkpKZUpKSkpKSkpKSnW1ikpKSkpKSkpKRgpKSkpKSkpKSkpKSkp
45KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkpKKkpKSkpKSkpKSkpKSkp
46KSkpKSkpKSkpKSkpKSkpKSkpKSkpKSkaAwPhqwAAAwMDAwP/5+fn5+fn

    Afl++ stability also dropped to ~70% which suggests that there is some non-determinism in the target.

  23. maflcko commented at 10:50 am on November 27, 2023: member

    Afl++ stability also dropped to ~70% which suggests that there is some non-determinism in the target.

    This is expected, because the wallet internally uses system random, similar to the p2p fuzz targets. This should be fixed in a follow-up for all fuzz targets.

  24. fuzz: Faster wallet_notifications target fa15861763
  25. maflcko force-pushed on Nov 27, 2023
  26. maflcko commented at 11:07 am on November 27, 2023: member

    I’ve run into timeouts.

    Thanks, fixed by reducing the number of new addresses that are generated each run

  27. dergoegge approved
  28. dergoegge commented at 11:46 am on November 27, 2023: member
    reACK fa15861763df71e788849b587883b3c16bb12229
  29. DrahtBot requested review from brunoerg on Nov 27, 2023
  30. brunoerg approved
  31. brunoerg commented at 11:55 am on November 27, 2023: contributor
    reACK fa15861763df71e788849b587883b3c16bb12229
  32. fanquake merged this on Nov 27, 2023
  33. fanquake closed this on Nov 27, 2023
  34. maflcko deleted the branch on Nov 28, 2023


maflcko DrahtBot dergoegge brunoerg

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-28 22:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me