diff options
| author | fanquake <fanquake@gmail.com> | 2023-08-03 17:16:46 +0100 |
|---|---|---|
| committer | fanquake <fanquake@gmail.com> | 2023-08-03 17:32:46 +0100 |
| commit | 61849f0464f15f92814cd7408023a86e9c32567d (patch) | |
| tree | ab688c33eaf92d0cb529fec617e5ab700853f4a5 /src/test/fuzz | |
| parent | 7c66a4b6109c01b79979128ef23e63fc733f89ba (diff) | |
| parent | 025fda0a76893d09d19ec9a6c0ba86ad11c466f7 (diff) | |
Merge bitcoin/bitcoin#27918: fuzz: addrman, avoid `ConsumeDeserializable` when possible
025fda0a76893d09d19ec9a6c0ba86ad11c466f7 fuzz: addrman, avoid `ConsumeDeserializable` when possible (brunoerg)
Pull request description:
Using specific functions like `ConsumeService`, `ConsumeAddress` and `ConsumeNetAddr` may be more effective than using `ConsumeDeserializable`. They always return some value while `ConsumeDeserializable` may return `std::nullopt`.
E.g.: In this part of the code, if `op_net_addr` is `std::nullopt`, we basically generated the addresses (if so) unnecessarily, because we won't be able to use them:
```cpp
std::vector<CAddress> addresses;
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider);
if (!opt_address) {
break;
}
addresses.push_back(*opt_address);
}
const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider);
if (opt_net_addr) {
addr_man.Add(addresses, *opt_net_addr, std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)});
}
```
Also, if we are not calling `Add` effectively, it would also be affect other functions that may "depend" on it.
ACKs for top commit:
dergoegge:
Code review ACK 025fda0a76893d09d19ec9a6c0ba86ad11c466f7
Tree-SHA512: 02450bec0b084c15ba0cd1cbdfbac067c8fea4ccf27be0c86d54e020f029a6c749a16d8e0558f9d6d35a7ca9db8916f180c872f09474702b5591129e9be0d192
Diffstat (limited to 'src/test/fuzz')
| -rw-r--r-- | src/test/fuzz/addrman.cpp | 33 |
1 files changed, 7 insertions, 26 deletions
diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 8ac1330dcb..02df4590de 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -263,40 +263,21 @@ FUZZ_TARGET(addrman, .init = initialize_addrman) [&] { std::vector<CAddress> addresses; LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) { - const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider); - if (!opt_address) { - break; - } - addresses.push_back(*opt_address); - } - const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider); - if (opt_net_addr) { - addr_man.Add(addresses, *opt_net_addr, std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)}); + addresses.push_back(ConsumeAddress(fuzzed_data_provider)); } + addr_man.Add(addresses, ConsumeNetAddr(fuzzed_data_provider), std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.Good(*opt_service, NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); - } + addr_man.Good(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.Attempt(*opt_service, fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); - } + addr_man.Attempt(ConsumeService(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.Connected(*opt_service, NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); - } + addr_man.Connected(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}}); }, [&] { - const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider); - if (opt_service) { - addr_man.SetServices(*opt_service, ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS)); - } + addr_man.SetServices(ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS)); }); } const AddrMan& const_addr_man{addr_man}; @@ -334,4 +315,4 @@ FUZZ_TARGET(addrman_serdeser, .init = initialize_addrman) data_stream << addr_man1; data_stream >> addr_man2; assert(addr_man1 == addr_man2); -} +}
\ No newline at end of file |