Merge #20138: net: Assume that SetCommonVersion is called at most onc…

…e per peer

fa0f415 net: Assume that SetCommonVersion is called at most once per peer (MarcoFalke)

Pull request description:

  This restores the check removed in bitcoin/bitcoin#17785 (comment)

  Instead of using `error`, which was used previously, it uses a newly introduced `Assume()`. `error` had several issues:
  * It logs unconditionally to the debug log
  * It doesn't abort the program when the error is hit in tests

ACKs for top commit:
  practicalswift:
    cr ACK fa0f415: patch looks correct
  jnewbery:
    utACK fa0f415

Tree-SHA512: cd7424a9485775e8c7093b725f8f52a90d47485185e79bac80f7810e450d0b3fda608d8805e9239094929f7bad2dca3fe772fb78ae606c2399d15405521e136b

src/net.h

@@ -24,14 +24,15 @@
 #include <sync.h>
 #include <threadinterrupt.h>
 #include <uint256.h>
+#include <util/check.h>
 
 #include <atomic>
+#include <condition_variable>
 #include <cstdint>
 #include <deque>
 #include <map>
-#include <thread>
 #include <memory>
-#include <condition_variable>
+#include <thread>
 
 class CScheduler;
 class CNode;
@@ -1131,6 +1132,7 @@ class CNode
 
     void SetCommonVersion(int greatest_common_version)
     {
+        Assume(m_greatest_common_version == INIT_PROTO_VERSION);
         m_greatest_common_version = greatest_common_version;
     }
     int GetCommonVersion() const

src/test/fuzz/net.cpp

@@ -48,8 +48,9 @@ void test_one_input(const std::vector<uint8_t>& buffer)
                fuzzed_data_provider.ConsumeRandomLengthString(32),
                fuzzed_data_provider.PickValueInArray({ConnectionType::INBOUND, ConnectionType::OUTBOUND_FULL_RELAY, ConnectionType::MANUAL, ConnectionType::FEELER, ConnectionType::BLOCK_RELAY, ConnectionType::ADDR_FETCH}),
                fuzzed_data_provider.ConsumeBool()};
+    node.SetCommonVersion(fuzzed_data_provider.ConsumeIntegral<int>());
     while (fuzzed_data_provider.ConsumeBool()) {
-        switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 11)) {
+        switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 10)) {
         case 0: {
             node.CloseSocketDisconnect();
             break;
@@ -59,10 +60,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             break;
         }
         case 2: {
-            node.SetCommonVersion(fuzzed_data_provider.ConsumeIntegral<int>());
-            break;
-        }
-        case 3: {
             const std::vector<bool> asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider);
             if (!SanityCheckASMap(asmap)) {
                 break;
@@ -71,18 +68,18 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             node.copyStats(stats, asmap);
             break;
         }
-        case 4: {
+        case 3: {
             const CNode* add_ref_node = node.AddRef();
             assert(add_ref_node == &node);
             break;
         }
-        case 5: {
+        case 4: {
             if (node.GetRefCount() > 0) {
                 node.Release();
             }
             break;
         }
-        case 6: {
+        case 5: {
             if (node.m_addr_known == nullptr) {
                 break;
             }
@@ -93,7 +90,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             node.AddAddressKnown(*addr_opt);
             break;
         }
-        case 7: {
+        case 6: {
             if (node.m_addr_known == nullptr) {
                 break;
             }
@@ -105,27 +102,27 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             node.PushAddress(*addr_opt, fast_random_context);
             break;
         }
-        case 8: {
+        case 7: {
             const std::optional<CInv> inv_opt = ConsumeDeserializable<CInv>(fuzzed_data_provider);
             if (!inv_opt) {
                 break;
             }
             node.AddKnownTx(inv_opt->hash);
             break;
         }
-        case 9: {
+        case 8: {
             node.PushTxInventory(ConsumeUInt256(fuzzed_data_provider));
             break;
         }
-        case 10: {
+        case 9: {
             const std::optional<CService> service_opt = ConsumeDeserializable<CService>(fuzzed_data_provider);
             if (!service_opt) {
                 break;
             }
             node.SetAddrLocal(*service_opt);
             break;
         }
-        case 11: {
+        case 10: {
             const std::vector<uint8_t> b = ConsumeRandomLengthByteVector(fuzzed_data_provider);
             bool complete;
             node.ReceiveMsgBytes(b, complete);