test: Enforce comparison of signed types at compile time

Author: rustaceanrob

src/test/addrman_tests.cpp

@@ -389,7 +389,7 @@ FIXTURE_TEST_CASE(addrman_new_multiplicity, BasicTestingSetup)
         addrman->Add({addr}, source);
     }
     AddressPosition addr_pos = addrman->FindAddressEntry(addr).value();
-    CHECK(addr_pos.multiplicity == 1U);
+    CHECK(addr_pos.multiplicity == 1);
     CHECK(addrman->Size() == 1U);
 
     // if nTime increases, an addr can occur in up to 8 buckets
@@ -402,7 +402,7 @@ FIXTURE_TEST_CASE(addrman_new_multiplicity, BasicTestingSetup)
         addrman->Add({addr}, source);
     }
     AddressPosition addr_pos_multi = addrman->FindAddressEntry(addr).value();
-    CHECK(addr_pos_multi.multiplicity == 8U);
+    CHECK(addr_pos_multi.multiplicity == 8);
     // multiplicity doesn't affect size
     CHECK(addrman->Size() == 1U);
 }
@@ -644,7 +644,7 @@ FIXTURE_TEST_CASE(caddrinfo_get_new_bucket_legacy, BasicTestingSetup)
     }
     // Test: IP addresses in the different source groups should map to MORE
     //  than 64 buckets.
-    CHECK(buckets.size() > 64);
+    CHECK(buckets.size() > 64U);
 }
 
 // The following three test cases use asmap.raw
@@ -696,7 +696,7 @@ FIXTURE_TEST_CASE(caddrinfo_get_tried_bucket, BasicTestingSetup)
     }
     // Test: IP addresses in the different /16 prefix MAY map to more than
     // 8 buckets.
-    CHECK(buckets.size() > 8);
+    CHECK(buckets.size() > 8U);
 
     buckets.clear();
     for (int j = 0; j < 255; j++) {
@@ -773,7 +773,7 @@ FIXTURE_TEST_CASE(caddrinfo_get_new_bucket, BasicTestingSetup)
     }
     // Test: IP addresses in the different source /16 prefixes usually map to MORE
     // than 1 bucket.
-    CHECK(buckets.size() > 1);
+    CHECK(buckets.size() > 1U);
 
     buckets.clear();
     for (int p = 0; p < 255; p++) {
@@ -867,7 +867,7 @@ FIXTURE_TEST_CASE(remove_invalid, BasicTestingSetup)
     addrman->Add({new1, tried1, new2, tried2}, CNetAddr{});
     addrman->Good(tried1);
     addrman->Good(tried2);
-    REQUIRE(addrman->Size() == 4);
+    REQUIRE(addrman->Size() == 4U);
 
     stream << *addrman;
 
@@ -890,7 +890,7 @@ FIXTURE_TEST_CASE(remove_invalid, BasicTestingSetup)
 
     addrman = std::make_unique<AddrMan>(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node));
     stream >> *addrman;
-    CHECK(addrman->Size() == 2);
+    CHECK(addrman->Size() == 2U);
 }
 
 FIXTURE_TEST_CASE(addrman_selecttriedcollision, BasicTestingSetup)

src/test/banman_tests.cpp

@@ -34,7 +34,7 @@ FIXTURE_TEST_CASE(file, BasicTestingSetup)
             BanMan banman{banlist_path, /*client_interface=*/nullptr, /*default_ban_time=*/0};
             banmap_t entries_read;
             banman.GetBanned(entries_read);
-            CHECK(entries_read.size() == 1);
+            CHECK(entries_read.size() == 1U);
         }
     }
 }

src/test/blockmanager_tests.cpp

@@ -163,7 +163,7 @@ FIXTURE_TEST_CASE(blockmanager_block_data_part, TestChain100Setup)
 
     auto block{blockman.ReadRawBlock(tip_block_pos)};
     REQUIRE(block);
-    REQUIRE(block->size() >= 200);
+    REQUIRE(block->size() >= 200U);
 
     const auto expect_part{[&](size_t offset, size_t size) {
         auto res{blockman.ReadRawBlock(tip_block_pos, std::pair{offset, size})};
@@ -191,7 +191,7 @@ FIXTURE_TEST_CASE(blockmanager_block_data_part_error, TestChain100Setup)
 
     auto block{blockman.ReadRawBlock(tip_block_pos)};
     REQUIRE(block);
-    REQUIRE(block->size() >= 200);
+    REQUIRE(block->size() >= 200U);
 
     const auto expect_part_error{[&](size_t offset, size_t size) {
         auto res{blockman.ReadRawBlock(tip_block_pos, std::pair{offset, size})};
@@ -257,7 +257,7 @@ FIXTURE_TEST_CASE(blockmanager_flush_block_file, BasicTestingSetup)
     constexpr int TEST_BLOCK_SIZE{81};
 
     // Blockstore is empty
-    CHECK(blockman.CalculateCurrentUsage() == 0);
+    CHECK(blockman.CalculateCurrentUsage() == 0U);
 
     // Write the first block to a new location.
     FlatFilePos pos1{blockman.WriteBlock(block1, /*nHeight=*/1)};
@@ -288,10 +288,10 @@ FIXTURE_TEST_CASE(blockmanager_flush_block_file, BasicTestingSetup)
     // Verify this behavior by attempting (and failing) to write block 3 data
     // to block 2 location.
     CBlockFileInfo* block_data = blockman.GetBlockFileInfo(0);
-    CHECK(block_data->nBlocks == 2);
+    CHECK(block_data->nBlocks == 2U);
     blockman.UpdateBlockInfo(block3, /*nHeight=*/3, /*pos=*/pos2);
     // Metadata is updated...
-    CHECK(block_data->nBlocks == 3);
+    CHECK(block_data->nBlocks == 3U);
     // ...but there are still only two blocks in the file
     CHECK(blockman.CalculateCurrentUsage() == (TEST_BLOCK_SIZE + STORAGE_HEADER_BYTES) * 2);
 

src/test/btcsignals_tests.cpp

@@ -187,7 +187,7 @@ FIXTURE_TEST_CASE(thread_safety, BasicTestingSetup)
     // sig will have been called 2000 times, and at least 1000 of those will
     // have been executing multiple incrementing callbacks. So while val is
     // probably MUCH bigger, it's guaranteed to be at least 3000.
-    CHECK(val.load() >= 3000);
+    CHECK(val.load() >= 3000U);
 }
 
 /* Test that connection and disconnection works from within signal

src/test/coins_tests.cpp

@@ -1139,15 +1139,15 @@ FIXTURE_TEST_CASE(ccoins_reset_guard, BasicTestingSetup)
         const auto reset_guard{cache.CreateResetGuard()};
         CHECK((cache.AccessCoin(outpoint) == coin));
         CHECK(!cache.AccessCoin(outpoint).IsSpent());
-        CHECK(cache.GetCacheSize() == 1);
-        CHECK(cache.GetDirtyCount() == 1);
+        CHECK(cache.GetCacheSize() == 1U);
+        CHECK(cache.GetDirtyCount() == 1U);
         CHECK(cache.GetBestBlock() == cache_best_block);
         CHECK(!root_cache.HaveCoinInCache(outpoint));
     }
 
     CHECK(cache.AccessCoin(outpoint).IsSpent());
-    CHECK(cache.GetCacheSize() == 0);
-    CHECK(cache.GetDirtyCount() == 0);
+    CHECK(cache.GetCacheSize() == 0U);
+    CHECK(cache.GetDirtyCount() == 0U);
     CHECK(cache.GetBestBlock() == base_best_block);
     CHECK(!root_cache.HaveCoinInCache(outpoint));
 
@@ -1157,7 +1157,7 @@ FIXTURE_TEST_CASE(ccoins_reset_guard, BasicTestingSetup)
     }
 
     CHECK(cache.AccessCoin(outpoint).IsSpent());
-    CHECK(cache.GetCacheSize() == 0);
+    CHECK(cache.GetCacheSize() == 0U);
     CHECK(cache.GetDirtyCount() == 0U);
     CHECK(cache.GetBestBlock() == base_best_block);
     CHECK(!root_cache.HaveCoinInCache(outpoint));

src/test/coinsviewoverlay_tests.cpp

@@ -140,7 +140,7 @@ TEST_CASE(fetch_no_double_spend)
         }
     }
     // Coins are not added to the view, even though they exist unspent in the parent db
-    CHECK(view.GetCacheSize() == 0);
+    CHECK(view.GetCacheSize() == 0U);
 }
 
 TEST_CASE(fetch_no_inputs)
@@ -157,7 +157,7 @@ TEST_CASE(fetch_no_inputs)
             CHECK(!view.GetCoin(in.prevout));
         }
     }
-    CHECK(view.GetCacheSize() == 0);
+    CHECK(view.GetCacheSize() == 0U);
 }
 
 TEST_SUITE_END()

src/test/dbwrapper_tests.cpp

@@ -355,18 +355,18 @@ FIXTURE_TEST_CASE(iterator_ordering, BasicTestingSetup)
     for (const int seek_start : {0x00, 0x80}) {
         it->Seek((uint8_t)seek_start);
         for (unsigned int x=seek_start; x<255; ++x) {
-            uint8_t key;
-            uint32_t value;
+            uint8_t key{};
+            uint32_t value{};
             CHECK(it->Valid());
             if (!it->Valid()) // Avoid spurious errors about invalid iterator's key and value in case of failure
                 break;
             CHECK(it->GetKey(key));
             if (x & 1) {
-                CHECK(key == x + 1);
+                CHECK(static_cast<unsigned>(key) == x + 1);
                 continue;
             }
             CHECK(it->GetValue(value));
-            CHECK(key == x);
+            CHECK(static_cast<unsigned>(key) == x);
             CHECK(value == x*x);
             it->Next();
         }

src/test/descriptor_tests.cpp

@@ -126,7 +126,7 @@ std::set<CPubKey> GetKeyData(const FlatSigningProvider& provider, int flags) {
     for (const auto& [_, pubkey] : provider.pubkeys) {
         if (flags & XONLY_KEYS) {
             unsigned char bytes[33];
-            CHECK(pubkey.size() == 33);
+            CHECK(pubkey.size() == 33U);
             std::copy(pubkey.begin(), pubkey.end(), bytes);
             bytes[0] = 0x02;
             CPubKey norm_pubkey{bytes};
@@ -153,7 +153,7 @@ std::set<std::pair<CPubKey, KeyOriginInfo>> GetKeyOriginData(const FlatSigningPr
         if (ignored.contains(keyid)) continue;
         if (flags & XONLY_KEYS) {
             unsigned char bytes[33];
-            CHECK(data.first.size() == 33);
+            CHECK(data.first.size() == 33U);
             std::copy(data.first.begin(), data.first.end(), bytes);
             bytes[0] = 0x02;
             CPubKey norm_pubkey{bytes};
@@ -339,22 +339,22 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
                 // For ranged, unhardened derivation, None of the keys in origins should appear in the cache but the cache should have parent keys
                 // But we can derive one level from each of those parent keys and find them all
                 CHECK(der_xpub_cache.empty());
-                CHECK(parent_xpub_cache.size() > 0);
+                CHECK(parent_xpub_cache.size() > 0U);
                 std::set<CPubKey> pubkeys;
                 for (const auto& xpub_pair : parent_xpub_cache) {
                     const CExtPubKey& xpub = xpub_pair.second;
                     CExtPubKey der;
                     CHECK(xpub.Derive(der, i));
                     pubkeys.insert(der.pubkey);
                 }
-                int count_pks = 0;
+                size_t count_pks = 0;
                 for (const auto& origin_pair : script_provider_cached.origins) {
                     const CPubKey& pk = origin_pair.second.first;
                     count_pks += pubkeys.count(pk);
                 }
                 if (flags & MUSIG_DERIVATION) {
                     if (!(flags & MIXED_MUSIG)) {
-                        CHECK(count_pks == 1);
+                        CHECK(count_pks == 1U);
                     }
                     CHECK(num_xpubs == pubkeys.size());
                 } else {
@@ -391,7 +391,7 @@ void DoCheck(std::string prv, std::string pub, const std::string& norm_pub, int
                     CHECK(xpub.Derive(der, i));
                     pubkeys.insert(der.pubkey);
                 }
-                int count_pks = 0;
+                size_t count_pks = 0;
                 for (const auto& origin_pair : script_provider_cached.origins) {
                     const CPubKey& pk = origin_pair.second.first;
                     count_pks += pubkeys.count(pk);

src/test/disconnected_transactions.cpp

@@ -16,7 +16,7 @@ FIXTURE_TEST_CASE(disconnectpool_memory_limits, TestChain100Setup)
     // transactions would realistically be in a block together, they just need distinct txids and
     // uniform size for this test to work.
     std::vector<CTransactionRef> block_vtx(m_coinbase_txns);
-    CHECK(block_vtx.size() == 100);
+    CHECK(block_vtx.size() == 100U);
 
     // Roughly estimate sizes to sanity check that DisconnectedBlockTransactions::DynamicMemoryUsage
     // is within an expected range.
@@ -52,7 +52,7 @@ FIXTURE_TEST_CASE(disconnectpool_memory_limits, TestChain100Setup)
         CHECK((disconnectpool.DynamicMemoryUsage() <= MAP_1 + ENTRY_USAGE_ESTIMATE));
 
         // Only 1 transaction can be kept
-        CHECK(1 == evicted_txns.size());
+        CHECK(1U == evicted_txns.size());
         // Transactions are added from back to front and eviction is FIFO.
         CHECK(block_vtx.at(1) == evicted_txns.front());
 
@@ -66,7 +66,7 @@ FIXTURE_TEST_CASE(disconnectpool_memory_limits, TestChain100Setup)
         const size_t USAGE_100_OVERESTIMATE{MAP_100 + ENTRY_USAGE_ESTIMATE * 100};
         DisconnectedBlockTransactions disconnectpool{USAGE_100_OVERESTIMATE};
         auto evicted_txns{disconnectpool.AddTransactionsFromBlock(block_vtx)};
-        CHECK(evicted_txns.size() == 0);
+        CHECK(evicted_txns.size() == 0U);
         CHECK((disconnectpool.DynamicMemoryUsage() <= USAGE_100_OVERESTIMATE));
 
         usage_full = disconnectpool.DynamicMemoryUsage();
@@ -82,7 +82,7 @@ FIXTURE_TEST_CASE(disconnectpool_memory_limits, TestChain100Setup)
         CHECK((disconnectpool.DynamicMemoryUsage() <= MAX_MEMUSAGE_99));
 
         // Only 1 transaction needed to be evicted
-        CHECK(1 == evicted_txns.size());
+        CHECK(1U == evicted_txns.size());
 
         // Transactions are added from back to front and eviction is FIFO.
         // The last transaction of block_vtx should be the first to be evicted.

src/test/fs_tests.cpp

@@ -31,7 +31,7 @@ FIXTURE_TEST_CASE(fsbridge_pathtostring, BasicTestingSetup)
     // and these functions do encoding and decoding, so the behavior of this
     // test would be undefined.
     std::string invalid_u8_str = "\xf0";
-    CHECK(invalid_u8_str.size() == 1);
+    CHECK(invalid_u8_str.size() == 1U);
     CHECK(fs::PathToString(fs::PathFromString(invalid_u8_str)) == invalid_u8_str);
 #endif
 }