test: fully reset the state of CConnman in tests

vasild commented at 8:52 am on February 5, 2026: contributor

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)

test: add ConnmanTestMsg convenience method Reset() 91b7c874e2

DrahtBot added the label Tests on Feb 5, 2026

DrahtBot commented at 8:53 am on February 5, 2026: contributor

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

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.

in src/net.h:1245 in 41c94739ca

1240@@ -1241,6 +1241,9 @@ class CConnman
1241         /// Wait for the number of needed connections to become greater than 0.
1242         void NumToOpenWait() const;
1243 
1244+        /// Reset the state of this.
1245+        void Reset();

maflcko commented at 9:27 am on February 5, 2026:

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 commented at 10:47 am on February 5, 2026:

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.

maflcko commented at 9:27 am on February 5, 2026: member

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

fanquake commented at 10:00 am on February 5, 2026: member

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.

test: also reset CConnman::m_private_broadcast in tests

Member variables of `CConnman::m_private_broadcast` (introduced in
https://github.com/bitcoin/bitcoin/pull/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
https://github.com/bitcoin/bitcoin/issues/34476#issuecomment-3849088794

2cb7e99dee

vasild force-pushed on Feb 5, 2026

vasild commented at 10:44 am on February 5, 2026: contributor

41c94739ca841d7374beab2002eee75bc9088c8c...2cb7e99deee1017a6edd94d82de556895138361d: address suggestion

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

Done.

in src/net.h:1255 in 2cb7e99dee

1250@@ -1251,6 +1251,8 @@ class CConnman
1251 
1252         /// Number of `ConnectionType::PRIVATE_BROADCAST` connections to open.
1253         std::atomic_size_t m_num_to_open{0};
1254+
1255+        friend struct ConnmanTestMsg;

maflcko commented at 10:47 am on February 5, 2026:

Not sure about adding test-only code to real code. This can be avoided by defining a

0struct PrivateBroadcastTest : public PrivateBroadcast {
1void Reset(){
2m_outbound_tor_ok_at_least_once.store(false);
3    m_num_to_open.store(0);
4}
5};

And then calling static_cast<PrivateBroadcastTest&>(m_private_broadcast).Reset() in the ConnmanTestMsg::Reset().

vasild commented at 11:00 am on February 5, 2026:

I do not like casting a base class to a derived class. Also, I do not like adding a new class just for that. I prefer the friend declaration because it is just one line of non-executable code and we already use that same friend struct ConnmanTestMsg; in net.h.

maflcko commented at 11:13 am on February 5, 2026: member

review ACK 2cb7e99deee1017a6edd94d82de556895138361d 🚙

Signature:

0untrusted 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}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 2cb7e99deee1017a6edd94d82de556895138361d 🚙
3RKqwSjBJQ9OQHDJVnwRa8lJ45ihA+GiVVqdY0UOFQB0UjOtFhgt7OP3WQiA3AB65GDl3mmk5qYsKqmZET7D8Bw==

DrahtBot added the label CI failed on Feb 5, 2026

in src/test/util/net.cpp:88 in 2cb7e99dee

83@@ -84,6 +84,8 @@ void ConnmanTestMsg::Reset()
84 {
85     ResetAddrCache();
86     ResetMaxOutboundCycle();
87+    m_private_broadcast.m_outbound_tor_ok_at_least_once.store(false);
88+    m_private_broadcast.m_num_to_open.store(0);

Crypt-iQ commented at 2:57 pm on February 5, 2026:

nit: I don’t think m_num_to_open needs to be reset, at least for fixing the non-determinism in the fuzz tests because afaict it won’t change from 0

vasild commented at 5:46 am on February 7, 2026:

I will leave it as it is, better to reset all.

Crypt-iQ commented at 2:58 pm on February 5, 2026: contributor

tACK 2cb7e99deee1017a6edd94d82de556895138361d

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

maflcko added this to the milestone 31.0 on Feb 6, 2026

in src/net.h:1244 in 2cb7e99dee

1240@@ -1241,7 +1241,7 @@ class CConnman
1241         /// Wait for the number of needed connections to become greater than 0.
1242         void NumToOpenWait() const;
1243 
1244-    private:
1245+    protected:

bvbfan commented at 8:24 am on February 8, 2026:

You may not change that line 1255 is enough.

DrahtBot removed the label CI failed on Feb 9, 2026

test: fully reset the state of CConnman in tests #34511

Reviews

Conflicts