Skip to content

Commit d184fc3

Browse files
committed
Merge bitcoin/bitcoin#30571: test: [refactor] Use m_rng directly
948238a test: Remove FastRandomContext global (Ryan Ofsky) fa0fe08 scripted-diff: [test] Use g_rng/m_rng directly (MarcoFalke) fa54cab test: refactor: Accept any RandomNumberGenerator in RandMoney (MarcoFalke) 68f77dd test: refactor: Pass rng parameters to test functions (Ryan Ofsky) fa19af5 test: refactor: Move g_insecure_rand_ctx.Reseed out of the helper that calls MakeRandDeterministicDANGEROUS (MarcoFalke) 3dc527f test: refactor: Give unit test functions access to test state (Ryan Ofsky) fab023e test: refactor: Make unsigned promotion explicit (MarcoFalke) fa2cb65 test: Add m_rng alias for the global random context (MarcoFalke) fae7e37 test: Correct the random seed log on a prevector test failure (MarcoFalke) Pull request description: This is mostly a style-cleanup for the tests' random generation: 1) `g_insecure_rand_ctx` in the tests is problematic, because the name is a leftover when the generator was indeed insecure. However, now the generator is *deterministic*, because the seed is either passed in or printed (c.f. RANDOM_CTX_SEED). Stating that deterministic randomness is insecure in the tests seems redundant at best. Fix it by just using `m_rng` for the name. 2) The global random context has many one-line aliases, such as `InsecureRand32`. This is problematic, because the same line of code may use the context directly and through a wrapper at the same time. For example in net_tests (see below). This inconsistency is harmless, but confusing. Fix it by just removing the one-line aliases. ``` src/test/net_tests.cpp: auto msg_data_1 = g_insecure_rand_ctx.randbytes<uint8_t>(InsecureRandRange(100000)); ```` 3) The wrapper for randmoney has the same problem that the same unit test uses the context directly and through a wrapper at the same time. Also, it has a single type of Rng hardcoded. Fix it by accepting any type. ACKs for top commit: hodlinator: ACK 948238a ryanofsky: Code review ACK 948238a. Only changes since last review were changing a comments a little bit. marcofleon: Code review ACK 948238a. Only changes since my last review are the improvements in `prevector_tests`. Tree-SHA512: 69c6b46a42cb743138ee8c87ff26a588dbe083e3efb3dca49b8a133ba5d3b09e8bf01c590ec7e121a7d77cb1fd7dcacd927a9ca139ac65e1f7c6d1ec46f93b57
2 parents f93d555 + 948238a commit d184fc3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+514
-459
lines changed

src/bench/sign_transaction.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,12 +75,13 @@ static void SignTransactionSchnorr(benchmark::Bench& bench) { SignTransactionSin
7575

7676
static void SignSchnorrTapTweakBenchmark(benchmark::Bench& bench, bool use_null_merkle_root)
7777
{
78+
FastRandomContext rng;
7879
ECC_Context ecc_context{};
7980

8081
auto key = GenerateRandomKey();
81-
auto msg = InsecureRand256();
82-
auto merkle_root = use_null_merkle_root ? uint256() : InsecureRand256();
83-
auto aux = InsecureRand256();
82+
auto msg = rng.rand256();
83+
auto merkle_root = use_null_merkle_root ? uint256() : rng.rand256();
84+
auto aux = rng.rand256();
8485
std::vector<unsigned char> sig(64);
8586

8687
bench.minEpochIterations(100).run([&] {

src/test/base58_tests.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,14 @@ BOOST_AUTO_TEST_CASE(base58_DecodeBase58)
8484
BOOST_AUTO_TEST_CASE(base58_random_encode_decode)
8585
{
8686
for (int n = 0; n < 1000; ++n) {
87-
unsigned int len = 1 + InsecureRandBits(8);
88-
unsigned int zeroes = InsecureRandBool() ? InsecureRandRange(len + 1) : 0;
89-
auto data = Cat(std::vector<unsigned char>(zeroes, '\000'), g_insecure_rand_ctx.randbytes(len - zeroes));
87+
unsigned int len = 1 + m_rng.randbits(8);
88+
unsigned int zeroes = m_rng.randbool() ? m_rng.randrange(len + 1) : 0;
89+
auto data = Cat(std::vector<unsigned char>(zeroes, '\000'), m_rng.randbytes(len - zeroes));
9090
auto encoded = EncodeBase58Check(data);
9191
std::vector<unsigned char> decoded;
92-
auto ok_too_small = DecodeBase58Check(encoded, decoded, InsecureRandRange(len));
92+
auto ok_too_small = DecodeBase58Check(encoded, decoded, m_rng.randrange(len));
9393
BOOST_CHECK(!ok_too_small);
94-
auto ok = DecodeBase58Check(encoded, decoded, len + InsecureRandRange(257 - len));
94+
auto ok = DecodeBase58Check(encoded, decoded, len + m_rng.randrange(257 - len));
9595
BOOST_CHECK(ok);
9696
BOOST_CHECK(data == decoded);
9797
}

src/test/bip324_tests.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
namespace {
2323

24+
struct BIP324Test : BasicTestingSetup {
2425
void TestBIP324PacketVector(
2526
uint32_t in_idx,
2627
const std::string& in_priv_ours_hex,
@@ -116,7 +117,7 @@ void TestBIP324PacketVector(
116117

117118
// Seek to the numbered packet.
118119
if (in_idx == 0 && error == 12) continue;
119-
uint32_t dec_idx = in_idx ^ (error == 12 ? (1U << InsecureRandRange(16)) : 0);
120+
uint32_t dec_idx = in_idx ^ (error == 12 ? (1U << m_rng.randrange(16)) : 0);
120121
for (uint32_t i = 0; i < dec_idx; ++i) {
121122
unsigned use_idx = i < in_idx ? i : 0;
122123
bool dec_ignore{false};
@@ -128,7 +129,7 @@ void TestBIP324PacketVector(
128129
// Decrypt length
129130
auto to_decrypt = ciphertext;
130131
if (error >= 2 && error <= 9) {
131-
to_decrypt[InsecureRandRange(to_decrypt.size())] ^= std::byte(1U << (error - 2));
132+
to_decrypt[m_rng.randrange(to_decrypt.size())] ^= std::byte(1U << (error - 2));
132133
}
133134

134135
// Decrypt length and resize ciphertext to accommodate.
@@ -139,7 +140,7 @@ void TestBIP324PacketVector(
139140
auto dec_aad = in_aad;
140141
if (error == 10) {
141142
if (in_aad.size() == 0) continue;
142-
dec_aad[InsecureRandRange(dec_aad.size())] ^= std::byte(1U << InsecureRandRange(8));
143+
dec_aad[m_rng.randrange(dec_aad.size())] ^= std::byte(1U << m_rng.randrange(8));
143144
}
144145
if (error == 11) dec_aad.push_back({});
145146

@@ -156,10 +157,11 @@ void TestBIP324PacketVector(
156157
}
157158
}
158159
}
160+
}; // struct BIP324Test
159161

160162
} // namespace
161163

162-
BOOST_FIXTURE_TEST_SUITE(bip324_tests, BasicTestingSetup)
164+
BOOST_FIXTURE_TEST_SUITE(bip324_tests, BIP324Test)
163165

164166
BOOST_AUTO_TEST_CASE(packet_test_vectors) {
165167
// BIP324 key derivation uses network magic in the HKDF process. We use mainnet params here

src/test/blockencodings_tests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(ReceiveWithExtraTransactions) {
356356

357357
BOOST_AUTO_TEST_CASE(TransactionsRequestSerializationTest) {
358358
BlockTransactionsRequest req1;
359-
req1.blockhash = InsecureRand256();
359+
req1.blockhash = m_rng.rand256();
360360
req1.indexes.resize(4);
361361
req1.indexes[0] = 0;
362362
req1.indexes[1] = 1;
@@ -380,7 +380,7 @@ BOOST_AUTO_TEST_CASE(TransactionsRequestSerializationTest) {
380380
BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationMaxTest) {
381381
// Check that the highest legal index is decoded correctly
382382
BlockTransactionsRequest req0;
383-
req0.blockhash = InsecureRand256();
383+
req0.blockhash = m_rng.rand256();
384384
req0.indexes.resize(1);
385385
req0.indexes[0] = 0xffff;
386386
DataStream stream{};
@@ -398,7 +398,7 @@ BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationOverflowTest) {
398398
// a request cannot be created by serializing a real BlockTransactionsRequest
399399
// due to the overflow, so here we'll serialize from raw deltas.
400400
BlockTransactionsRequest req0;
401-
req0.blockhash = InsecureRand256();
401+
req0.blockhash = m_rng.rand256();
402402
req0.indexes.resize(3);
403403
req0.indexes[0] = 0x7000;
404404
req0.indexes[1] = 0x10000 - 0x7000 - 2;

src/test/bloom_tests.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@
2222

2323
#include <boost/test/unit_test.hpp>
2424

25-
BOOST_FIXTURE_TEST_SUITE(bloom_tests, BasicTestingSetup)
25+
namespace bloom_tests {
26+
struct BloomTest : public BasicTestingSetup {
27+
std::vector<unsigned char> RandomData();
28+
};
29+
} // namespace bloom_tests
30+
31+
BOOST_FIXTURE_TEST_SUITE(bloom_tests, BloomTest)
2632

2733
BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize)
2834
{
@@ -455,9 +461,9 @@ BOOST_AUTO_TEST_CASE(merkle_block_4_test_update_none)
455461
BOOST_CHECK(!filter.contains(COutPoint(Txid::FromHex("02981fa052f0481dbc5868f4fc2166035a10f27a03cfd2de67326471df5bc041").value(), 0)));
456462
}
457463

458-
static std::vector<unsigned char> RandomData()
464+
std::vector<unsigned char> BloomTest::RandomData()
459465
{
460-
uint256 r = InsecureRand256();
466+
uint256 r = m_rng.rand256();
461467
return std::vector<unsigned char>(r.begin(), r.end());
462468
}
463469

src/test/checkqueue_tests.cpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@ struct NoLockLoggingTestingSetup : public TestingSetup {
3434
#endif
3535
};
3636

37-
BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, NoLockLoggingTestingSetup)
37+
struct CheckQueueTest : NoLockLoggingTestingSetup {
38+
void Correct_Queue_range(std::vector<size_t> range);
39+
};
3840

3941
static const unsigned int QUEUE_BATCH_SIZE = 128;
4042
static const int SCRIPT_CHECK_THREADS = 3;
@@ -156,7 +158,7 @@ typedef CCheckQueue<FrozenCleanupCheck> FrozenCleanup_Queue;
156158
/** This test case checks that the CCheckQueue works properly
157159
* with each specified size_t Checks pushed.
158160
*/
159-
static void Correct_Queue_range(std::vector<size_t> range)
161+
void CheckQueueTest::Correct_Queue_range(std::vector<size_t> range)
160162
{
161163
auto small_queue = std::make_unique<Correct_Queue>(QUEUE_BATCH_SIZE, SCRIPT_CHECK_THREADS);
162164
// Make vChecks here to save on malloc (this test can be slow...)
@@ -168,7 +170,7 @@ static void Correct_Queue_range(std::vector<size_t> range)
168170
CCheckQueueControl<FakeCheckCheckCompletion> control(small_queue.get());
169171
while (total) {
170172
vChecks.clear();
171-
vChecks.resize(std::min<size_t>(total, InsecureRandRange(10)));
173+
vChecks.resize(std::min<size_t>(total, m_rng.randrange(10)));
172174
total -= vChecks.size();
173175
control.Add(std::move(vChecks));
174176
}
@@ -177,6 +179,8 @@ static void Correct_Queue_range(std::vector<size_t> range)
177179
}
178180
}
179181

182+
BOOST_FIXTURE_TEST_SUITE(checkqueue_tests, CheckQueueTest)
183+
180184
/** Test that 0 checks is correct
181185
*/
182186
BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Zero)
@@ -207,7 +211,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Correct_Random)
207211
{
208212
std::vector<size_t> range;
209213
range.reserve(100000/1000);
210-
for (size_t i = 2; i < 100000; i += std::max((size_t)1, (size_t)InsecureRandRange(std::min((size_t)1000, ((size_t)100000) - i))))
214+
for (size_t i = 2; i < 100000; i += std::max((size_t)1, (size_t)m_rng.randrange(std::min((size_t)1000, ((size_t)100000) - i))))
211215
range.push_back(i);
212216
Correct_Queue_range(range);
213217
}
@@ -221,7 +225,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Catches_Failure)
221225
CCheckQueueControl<FailingCheck> control(fail_queue.get());
222226
size_t remaining = i;
223227
while (remaining) {
224-
size_t r = InsecureRandRange(10);
228+
size_t r = m_rng.randrange(10);
225229

226230
std::vector<FailingCheck> vChecks;
227231
vChecks.reserve(r);
@@ -268,7 +272,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_UniqueCheck)
268272
{
269273
CCheckQueueControl<UniqueCheck> control(queue.get());
270274
while (total) {
271-
size_t r = InsecureRandRange(10);
275+
size_t r = m_rng.randrange(10);
272276
std::vector<UniqueCheck> vChecks;
273277
for (size_t k = 0; k < r && total; k++)
274278
vChecks.emplace_back(--total);
@@ -300,7 +304,7 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_Memory)
300304
{
301305
CCheckQueueControl<MemoryCheck> control(queue.get());
302306
while (total) {
303-
size_t r = InsecureRandRange(10);
307+
size_t r = m_rng.randrange(10);
304308
std::vector<MemoryCheck> vChecks;
305309
for (size_t k = 0; k < r && total; k++) {
306310
total--;

0 commit comments

Comments
 (0)