Skip to content

Commit 61849f0

Browse files
committed
Merge bitcoin/bitcoin#27918: fuzz: addrman, avoid ConsumeDeserializable when possible
025fda0 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 025fda0 Tree-SHA512: 02450bec0b084c15ba0cd1cbdfbac067c8fea4ccf27be0c86d54e020f029a6c749a16d8e0558f9d6d35a7ca9db8916f180c872f09474702b5591129e9be0d192
2 parents 7c66a4b + 025fda0 commit 61849f0

File tree

1 file changed

+7
-26
lines changed

1 file changed

+7
-26
lines changed

src/test/fuzz/addrman.cpp

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -263,40 +263,21 @@ FUZZ_TARGET(addrman, .init = initialize_addrman)
263263
[&] {
264264
std::vector<CAddress> addresses;
265265
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
266-
const std::optional<CAddress> opt_address = ConsumeDeserializable<CAddress>(fuzzed_data_provider);
267-
if (!opt_address) {
268-
break;
269-
}
270-
addresses.push_back(*opt_address);
271-
}
272-
const std::optional<CNetAddr> opt_net_addr = ConsumeDeserializable<CNetAddr>(fuzzed_data_provider);
273-
if (opt_net_addr) {
274-
addr_man.Add(addresses, *opt_net_addr, std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)});
266+
addresses.push_back(ConsumeAddress(fuzzed_data_provider));
275267
}
268+
addr_man.Add(addresses, ConsumeNetAddr(fuzzed_data_provider), std::chrono::seconds{ConsumeTime(fuzzed_data_provider, 0, 100000000)});
276269
},
277270
[&] {
278-
const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider);
279-
if (opt_service) {
280-
addr_man.Good(*opt_service, NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
281-
}
271+
addr_man.Good(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
282272
},
283273
[&] {
284-
const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider);
285-
if (opt_service) {
286-
addr_man.Attempt(*opt_service, fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
287-
}
274+
addr_man.Attempt(ConsumeService(fuzzed_data_provider), fuzzed_data_provider.ConsumeBool(), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
288275
},
289276
[&] {
290-
const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider);
291-
if (opt_service) {
292-
addr_man.Connected(*opt_service, NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
293-
}
277+
addr_man.Connected(ConsumeService(fuzzed_data_provider), NodeSeconds{std::chrono::seconds{ConsumeTime(fuzzed_data_provider)}});
294278
},
295279
[&] {
296-
const std::optional<CService> opt_service = ConsumeDeserializable<CService>(fuzzed_data_provider);
297-
if (opt_service) {
298-
addr_man.SetServices(*opt_service, ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS));
299-
}
280+
addr_man.SetServices(ConsumeService(fuzzed_data_provider), ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS));
300281
});
301282
}
302283
const AddrMan& const_addr_man{addr_man};
@@ -334,4 +315,4 @@ FUZZ_TARGET(addrman_serdeser, .init = initialize_addrman)
334315
data_stream << addr_man1;
335316
data_stream >> addr_man2;
336317
assert(addr_man1 == addr_man2);
337-
}
318+
}

0 commit comments

Comments
 (0)