fuzz: add target for local address stuff #29495

brunoerg commented at 7:41 PM on February 27, 2024: contributor

This PR adds fuzz target for local address functions - (AddLocal, RemoveLocal, SeenLocal, IsLocal)

DrahtBot commented at 7:41 PM on February 27, 2024: 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, vasild

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

DrahtBot added the label Tests on Feb 27, 2024

in src/test/fuzz/net.cpp:84 in 348a4c0ca8 outdated

  76 | @@ -77,3 +77,33 @@ FUZZ_TARGET(net, .init = initialize_net)
  77 |      (void)node.HasPermission(net_permission_flags);
  78 |      (void)node.ConnectedThroughNetwork();
  79 |  }
  80 | +
  81 | +FUZZ_TARGET(local_address, .init = initialize_net)
  82 | +{
  83 | +    FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
  84 | +    SetMockTime(ConsumeTime(fuzzed_data_provider));

dergoegge commented at 11:05 PM on February 27, 2024:

Is this really necessary?

brunoerg commented at 10:46 AM on February 28, 2024:

Not necessary, just removed it.

in src/test/fuzz/net.cpp:97 in 348a4c0ca8 outdated

  88 | +            fuzzed_data_provider,
  89 | +            [&] {
  90 | +                service = ConsumeService(fuzzed_data_provider);
  91 | +            },
  92 | +            [&] {
  93 | +                const bool added{AddLocal(service, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 5))};

dergoegge commented at 11:07 PM on February 27, 2024:

This harness will be non-deterministic and run out of memory since local addresses are stored in a global map. You'll need to reset mapLocalHost each iteration.

brunoerg commented at 10:46 AM on February 28, 2024:

Done.

in src/test/fuzz/net.cpp:114 in 348a4c0ca8 outdated

 102 | +            [&] {
 103 | +                (void)SeenLocal(service);
 104 | +            },
 105 | +            [&] {
 106 | +                (void)IsLocal(service);
 107 | +            });

dergoegge commented at 11:11 PM on February 27, 2024:

How about adding coverage for GetLocal as well?

dergoegge changes_requested

brunoerg force-pushed on Feb 28, 2024

brunoerg commented at 10:48 AM on February 28, 2024: contributor

Force-pushed addressing: #29495 (review) #29495 (review) #29495 (review)

Thanks, @dergoegge

in src/test/fuzz/net.cpp:112 in 36efe6bb55 outdated

 107 | +            },
 108 | +            [&] {
 109 | +                (void)IsLocal(service);
 110 | +            },
 111 | +            [&] {
 112 | +                auto peer{ConsumeNode(fuzzed_data_provider)};

dergoegge commented at 5:36 PM on February 28, 2024:

Probably better to not consume a whole CNode each time, maybe do what you did with the service.

brunoerg commented at 6:32 PM on February 28, 2024:

Even not being able to overload, sgtm to not consume each time, addressing it.

brunoerg force-pushed on Feb 28, 2024

brunoerg commented at 6:33 PM on February 28, 2024: contributor

Force-pushed addressing #29495 (review)

in src/test/fuzz/net.cpp:97 in fe4e38c257 outdated

  92 | +            fuzzed_data_provider,
  93 | +            [&] {
  94 | +                service = ConsumeService(fuzzed_data_provider);
  95 | +            },
  96 | +            [&] {
  97 | +                const bool added{AddLocal(service, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 5))};

vasild commented at 1:06 PM on February 29, 2024:

Maybe use LOCAL_MAX here instead of hardcoding 5:

                const bool added{AddLocal(service, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, LOCAL_MAX - 1))};

brunoerg commented at 5:17 PM on February 29, 2024:

Nice. Done.

vasild approved

vasild commented at 1:09 PM on February 29, 2024: contributor

ACK fe4e38c25738356a227dc8e84b6a24d837ba8215

fuzz: add target for local addresses 25eab52389

brunoerg force-pushed on Feb 29, 2024

brunoerg commented at 5:18 PM on February 29, 2024: contributor

Force-pushed addressing #29495 (review). Thanks, @vasild.

dergoegge approved

dergoegge commented at 5:20 PM on February 29, 2024: member

ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc

DrahtBot requested review from vasild on Feb 29, 2024

vasild approved

vasild commented at 7:29 AM on March 1, 2024: contributor

ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc

DrahtBot added the label Needs rebase on Mar 1, 2024

DrahtBot commented at 3:14 PM on March 1, 2024: contributor

🐙 This pull request conflicts with the target branch and needs rebase.

fanquake commented at 3:15 PM on March 1, 2024: member

This has been merged.

fanquake closed this on Mar 1, 2024

fanquake referenced this in commit ae4165f7bc on Mar 1, 2024

bitcoin locked this on Mar 1, 2025