test: fully reset the state of CConnman in tests

[vasild]

Member variables of CConnman::m_private_broadcast (introduced in
#29415) could influence the tests
which creates non-determinism if the same instance of CConnman is used
for repeated test iterations.

So, reset the state of CConnman::m_private_broadcast from
ConnmanTestMsg::Reset(). Currently this affects the fuzz tests
process_message and process_messages.

Reported in #34476 (comment)

[DrahtBot]

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, Crypt-iQ, frankomosh, brunoerg

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #29418 (rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats' by vasild)

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.

[maflcko]

Thx! lgtm, but I think it would be better to keep test-only code only in the test directory

[maflcko]

Not sure about adding test-only code to the real code. This can be fixed by changing the line below from private to protected. and then moving PrivateBroadcast::Reset() to PrivateBroadcastTest::Reset() (or so) in src/test/util/net.h

[vasild]

PrivateBroadcastTest

The test cannot change the type of CConnman::m_private_broadcast or ConnmanTestMsg::m_private_broadcast from CConnman::PrivateBroadcast to something else, e.g. PrivateBroadcastTest, but can make use of a friend. Done.

[fanquake]

Could you add the details to the PR description or commit messages? This is fixing non-determinism in the process_message fuzz tests, introduced by private broadcast (#29415), but that isn't mentioned (other than linking to a comment), and the description makes it sound like a generic test change.

[vasild]

41c94739ca841d7374beab2002eee75bc9088c8c...2cb7e99deee1017a6edd94d82de556895138361d: address suggestion

Could you add the details to the PR description or commit messages?

Done.

[maflcko]

review ACK 2cb7e99 🚙

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 2cb7e99deee1017a6edd94d82de556895138361d 🚙
RKqwSjBJQ9OQHDJVnwRa8lJ45ihA+GiVVqdY0UOFQB0UjOtFhgt7OP3WQiA3AB65GDl3mmk5qYsKqmZET7D8Bw==

[Crypt-iQ]

tACK 2cb7e99

Checked that it fixes the non-determinism in process_message(s)

[bvbfan]

You may not change that line 1255 is enough.

[maflcko]

Though, it seems fine, if another test wants to derive in the future. Should be harmless; No strong opinion.

[frankomosh]

Code Review ACK 2cb7e99

Manually reviewed the reset logic; additions to ConnmanTestMsg::Reset() look correct for fixing the non-determinism.

[brunoerg]

code review ACK 2cb7e99