Anything left to do here? :) The changes are limited to src/test/fuzz/ and should hopefully be trivial to review.

I cannot build this branch with either clang/clang++ or afl-gcc/afl-g++:

clang version: 6.0.0-1ubuntu2 (tags/RELEASE_600/final)
afl-gcc/afl-g++ built from afl master

Make output:

Making all in src
  2 make[1]: Entering directory '/root/bitcoin/src'
  3 make[2]: Entering directory '/root/bitcoin/src'
  4   CXX      test/fuzz/test_fuzz_addition_overflow-addition_overflow.o
  5 In file included from test/fuzz/addition_overflow.cpp:7:
  6 ./test/fuzz/util.h:62:18: error: no template named 'optional' in namespace 'std'; did you mean 'Optional'?
  7 NODISCARD inline std::optional<T> ConsumeDeserializable(FuzzedDataProvider& fuzzed_data_provider, const si    ze_t max_length = 4096) noexcept
  8                  ^~~~~~~~~~~~~
  9                  Optional
 10 ./optional.h:14:1: note: 'Optional' declared here
 11 using Optional = boost::optional<T>;
 12 ^
 13 In file included from test/fuzz/addition_overflow.cpp:7:
 14 ./test/fuzz/util.h:70:16: error: no member named 'nullopt' in namespace 'std'; did you mean simply 'nullop    t'?
 15         return std::nullopt;
 16                ^~~~~~~~~~~~
 17                nullopt
 18 ./optional.h:24:14: note: 'nullopt' declared here
 19 static auto& nullopt = boost::none;
 20              ^
 21 2 errors generated.
 22 Makefile:14081: recipe for target 'test/fuzz/test_fuzz_addition_overflow-addition_overflow.o' failed
 23 make[2]: *** [test/fuzz/test_fuzz_addition_overflow-addition_overflow.o] Error 1
 24 make[2]: Leaving directory '/root/bitcoin/src'
 25 Makefile:17268: recipe for target 'all-recursive' failed
 26 make[1]: *** [all-recursive] Error 1
 27 make[1]: Leaving directory '/root/bitcoin/src'
 28 Makefile:785: recipe for target 'all-recursive' failed
 29 make: *** [all-recursive] Error 1

Am I doing something wrong? I have C++17 features and can build on master.

@Crypt-iQ So to clarify, you can build current master, but not this pull request merged into current master?

@MarcoFalke I can build the current master branch, but not this branch as it is. I checkout this branch via:

git fetch origin pull/19065/head:addrmanfuzz

@Crypt-iQ Oh, that's weird! Thanks for reporting. I've now rebased this PR on current master. Would you mind testing again? :)

@practicalswift I think you may have forgotten to push it up :) And will test once it's up

@Crypt-iQ Oh, thanks for letting me know! Now pushed for real :)

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• net: Assume that SetCommonVersion is called at most once per peer #20138 (net: Assume that SetCommonVersion is called at most once per peer by MarcoFalke)

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.

@practicalswift Able to build this branch with afl and with libfuzzer (clang-10).

Coverage from 24 hours of AFL fuzzing here: https://crypt-iq.github.io/addrman_fuzz_outs/src/index.html
With more time it should hit the collision code but it was very slow ~4-5 execs/s.

@Crypt-iQ Thanks a lot for reviewing! Feedback addressed. Would you mind re-reviewing? :)

Ready for merge? :)

PR touches code only in src/test/fuzz/ and has two Tested ACK:s.

Edit: Noticed that I had unaddressed feedback from @Crypt-iQ in #19065 (comment). Will address. Sorry.

@practicalswift Just had that one comment remaining about GetGroup. I tried with @MarcoFalke suggested patch of insecure_rand being deterministic and after about a week the fuzzer still did not hit the collision code according to lcov. Granted, I am not throwing very many cores at it (2), and some input could probably be engineered to trigger it - just haven't had the time. As a newcomer to the codebase, I don't know what is considered out of scope PR-wise, but test determinism is always nice-to-have.

@Crypt-iQ Thanks for clarifying! I've now moved the AS-map code to allow for use via GetGroup. I'm now also mocking the time, but I skipped the suggested insecure_rand change since I don't want to change any code outside of src/test/fuzz/ in this PR. That can be taken in a follow-up if deemed necessary.

I think this PR should be ready for final review now. Many thanks for your patience: may I ask you to review for the n:th time? :)

Concept ACK. Will review

@MarcoFalke Thanks for reviewing. Addressed your optimization request and dropped all changes that are unrelated to CAddrMan. Should hopefully be ready for merge now :)

@MarcoFalke Thanks for re-reviewing! Addressed all feedback. Should hopefully be ready for final review now :)

@MarcoFalke Thanks for re-re-reviewing! Addressed all feedback. Should hopefully be ready for final final review now :)

review ACK d04a17a