Skip to content

Commit 5c0cd20

Browse files
committed
Merge bitcoin/bitcoin#29625: Several randomness improvements
ce80942 random: replace construct/assign with explicit Reseed() (Pieter Wuille) 2ae392d random: use LogError for init failure (Pieter Wuille) 97e16f5 tests: make fuzz tests (mostly) deterministic with fixed seed (Pieter Wuille) 2c91330 random: cleanup order, comments, static (Pieter Wuille) 8e31cf9 net, net_processing: use existing RNG objects more (Pieter Wuille) d5fcbe9 random: improve precision of MakeExponentiallyDistributed (Pieter Wuille) cfb0dfe random: convert GetExponentialRand into rand_exp_duration (Pieter Wuille) 4eaa239 random: convert GetRand{Micros,Millis} into randrange (Pieter Wuille) 82de1b8 net: use GetRandMicros for cache expiration (Pieter Wuille) ddc184d random: get rid of GetRand by inlining (Pieter Wuille) e2d1f84 random: make GetRand() support entire range (incl. max) (Pieter Wuille) 810cdf6 tests: overhaul deterministic test randomness (Pieter Wuille) 6cfdc5b random: convert XoRoShiRo128PlusPlus into full RNG (Pieter Wuille) 8cc2f45 random: move XoRoShiRo128PlusPlus into random module (Pieter Wuille) 8f5ac0d xoroshiro128plusplus: drop comment about nonexisting copy() (Pieter Wuille) 8924f51 random: modernize XoRoShiRo128PlusPlus a bit (Pieter Wuille) ddb7d26 random: add RandomMixin::randbits with compile-known bits (Pieter Wuille) 21ce9d8 random: Improve RandomMixin::randbits (Pieter Wuille) 9b14d3d random: refactor: move rand* utilities to RandomMixin (Pieter Wuille) 40dd86f random: use BasicByte concept in randbytes (Pieter Wuille) 27cefc7 random: add a few noexcepts to FastRandomContext (Pieter Wuille) b3b382d random: move rand256() and randbytes() to .h file (Pieter Wuille) 493a2e0 random: write rand256() in function of fillrand() (Pieter Wuille) Pull request description: This PR contains a number of vaguely-related improvements to the random module. The specific changes and more detailed rationale is in the commit messages, but the highlights are: * `XoRoShiRo128PlusPlus` (previously a test-only RNG) moves to random.h and becomes `InsecureRandomContext`, which is even faster than `FastRandomContext` but non-cryptographic. It also gets all helper randomness functions (`randrange`, `fillrand`, ...), making it a lot more succinct to use. * During tests, **all** randomness is made deterministic (except for `GetStrongRandBytes`) but non-repeating (like `GetRand()` used to be when `g_mock_deterministic_tests` was used), either fixed, or from a random seed (overridden by env var). * Several infrequently used top-level functions (`GetRandMillis`, `GetRandMicros`, `GetExponentialRand`) are converted into member functions of `FastRandomContext` (and `InsecureRandomContext`). * `GetRand<T>()` (without argument) can now return the maximum value of the type (previously e.g. `GetRand<uint32_t>()` would never return 0xffffffff). ACKs for top commit: achow101: ACK ce80942 maflcko: re-ACK ce80942 🐈 hodlinator: ACK ce80942 dergoegge: utACK ce80942 Tree-SHA512: 79bc0cbafaf27e95012c1ce2947a8ca6f9a3c78af5f1f16e69354b6fc9b987a28858adf4cd356dc5baf21163e9af8dcc24e70f8d7173be870e8a3ddcdd47c02c
2 parents 3714692 + ce80942 commit 5c0cd20

Some content is hidden

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

42 files changed

+707
-533
lines changed

src/Makefile.test.include

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,7 @@ BITCOIN_TESTS =\
173173
test/validation_flush_tests.cpp \
174174
test/validation_tests.cpp \
175175
test/validationinterface_tests.cpp \
176-
test/versionbits_tests.cpp \
177-
test/xoroshiro128plusplus_tests.cpp
176+
test/versionbits_tests.cpp
178177

179178
if ENABLE_WALLET
180179
BITCOIN_TESTS += \

src/Makefile.test_util.include

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,7 @@ TEST_UTIL_H = \
2323
test/util/str.h \
2424
test/util/transaction_utils.h \
2525
test/util/txmempool.h \
26-
test/util/validation.h \
27-
test/util/xoroshiro128plusplus.h
26+
test/util/validation.h
2827

2928
if ENABLE_WALLET
3029
TEST_UTIL_H += wallet/test/util.h

src/addrdb.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ template <typename Data>
5353
bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data& data)
5454
{
5555
// Generate random temporary filename
56-
const uint16_t randv{GetRand<uint16_t>()};
56+
const uint16_t randv{FastRandomContext().rand<uint16_t>()};
5757
std::string tmpfn = strprintf("%s.%04x", prefix, randv);
5858

5959
// open temp output file

src/addrman.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool new_only, std::option
776776
const AddrInfo& info{it_found->second};
777777

778778
// With probability GetChance() * chance_factor, return the entry.
779-
if (insecure_rand.randbits(30) < chance_factor * info.GetChance() * (1 << 30)) {
779+
if (insecure_rand.randbits<30>() < chance_factor * info.GetChance() * (1 << 30)) {
780780
LogPrint(BCLog::ADDRMAN, "Selected %s from %s\n", info.ToStringAddrPort(), search_tried ? "tried" : "new");
781781
return {info, info.m_last_try};
782782
}

src/common/bloom.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ bool CRollingBloomFilter::contains(Span<const unsigned char> vKey) const
239239

240240
void CRollingBloomFilter::reset()
241241
{
242-
nTweak = GetRand<unsigned int>();
242+
nTweak = FastRandomContext().rand<unsigned int>();
243243
nEntriesThisGeneration = 0;
244244
nGeneration = 1;
245245
std::fill(data.begin(), data.end(), 0);

src/headerssync.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ static_assert(sizeof(CompressedHeader) == 48);
2525

2626
HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus_params,
2727
const CBlockIndex* chain_start, const arith_uint256& minimum_required_work) :
28-
m_commit_offset(GetRand<unsigned>(HEADER_COMMITMENT_PERIOD)),
28+
m_commit_offset(FastRandomContext().randrange<unsigned>(HEADER_COMMITMENT_PERIOD)),
2929
m_id(id), m_consensus_params(consensus_params),
3030
m_chain_start(chain_start),
3131
m_minimum_required_work(minimum_required_work),

src/init.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1273,11 +1273,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
12731273
node.addrman = std::move(*addrman);
12741274
}
12751275

1276+
FastRandomContext rng;
12761277
assert(!node.banman);
12771278
node.banman = std::make_unique<BanMan>(args.GetDataDirNet() / "banlist", &uiInterface, args.GetIntArg("-bantime", DEFAULT_MISBEHAVING_BANTIME));
12781279
assert(!node.connman);
1279-
node.connman = std::make_unique<CConnman>(GetRand<uint64_t>(),
1280-
GetRand<uint64_t>(),
1280+
node.connman = std::make_unique<CConnman>(rng.rand64(),
1281+
rng.rand64(),
12811282
*node.addrman, *node.netgroupman, chainparams, args.GetBoolArg("-networkactive", true));
12821283

12831284
assert(!node.fee_estimator);

src/net.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,9 +2481,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
24812481
auto start = GetTime<std::chrono::microseconds>();
24822482

24832483
// Minimum time before next feeler connection (in microseconds).
2484-
auto next_feeler = GetExponentialRand(start, FEELER_INTERVAL);
2485-
auto next_extra_block_relay = GetExponentialRand(start, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
2486-
auto next_extra_network_peer{GetExponentialRand(start, EXTRA_NETWORK_PEER_INTERVAL)};
2484+
auto next_feeler = start + rng.rand_exp_duration(FEELER_INTERVAL);
2485+
auto next_extra_block_relay = start + rng.rand_exp_duration(EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
2486+
auto next_extra_network_peer{start + rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL)};
24872487
const bool dnsseed = gArgs.GetBoolArg("-dnsseed", DEFAULT_DNSSEED);
24882488
bool add_fixed_seeds = gArgs.GetBoolArg("-fixedseeds", DEFAULT_FIXEDSEEDS);
24892489
const bool use_seednodes{gArgs.IsArgSet("-seednode")};
@@ -2642,10 +2642,10 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
26422642
// Because we can promote these connections to block-relay-only
26432643
// connections, they do not get their own ConnectionType enum
26442644
// (similar to how we deal with extra outbound peers).
2645-
next_extra_block_relay = GetExponentialRand(now, EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
2645+
next_extra_block_relay = now + rng.rand_exp_duration(EXTRA_BLOCK_RELAY_ONLY_PEER_INTERVAL);
26462646
conn_type = ConnectionType::BLOCK_RELAY;
26472647
} else if (now > next_feeler) {
2648-
next_feeler = GetExponentialRand(now, FEELER_INTERVAL);
2648+
next_feeler = now + rng.rand_exp_duration(FEELER_INTERVAL);
26492649
conn_type = ConnectionType::FEELER;
26502650
fFeeler = true;
26512651
} else if (nOutboundFullRelay == m_max_outbound_full_relay &&
@@ -2658,7 +2658,7 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
26582658
// This is not attempted if the user changed -maxconnections to a value
26592659
// so low that less than MAX_OUTBOUND_FULL_RELAY_CONNECTIONS are made,
26602660
// to prevent interactions with otherwise protected outbound peers.
2661-
next_extra_network_peer = GetExponentialRand(now, EXTRA_NETWORK_PEER_INTERVAL);
2661+
next_extra_network_peer = now + rng.rand_exp_duration(EXTRA_NETWORK_PEER_INTERVAL);
26622662
} else {
26632663
// skip to next iteration of while loop
26642664
continue;
@@ -3475,7 +3475,8 @@ std::vector<CAddress> CConnman::GetAddresses(CNode& requestor, size_t max_addres
34753475
// nodes to be "terrible" (see IsTerrible()) if the timestamps are older than 30 days,
34763476
// max. 24 hours of "penalty" due to cache shouldn't make any meaningful difference
34773477
// in terms of the freshness of the response.
3478-
cache_entry.m_cache_entry_expiration = current_time + std::chrono::hours(21) + GetRandMillis(std::chrono::hours(6));
3478+
cache_entry.m_cache_entry_expiration = current_time +
3479+
21h + FastRandomContext().randrange<std::chrono::microseconds>(6h);
34793480
}
34803481
return cache_entry.m_addrs_response_cache;
34813482
}

src/net_processing.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -936,7 +936,7 @@ class PeerManagerImpl final : public PeerManager
936936
* accurately determine when we received the transaction (and potentially
937937
* determine the transaction's origin). */
938938
std::chrono::microseconds NextInvToInbounds(std::chrono::microseconds now,
939-
std::chrono::seconds average_interval);
939+
std::chrono::seconds average_interval) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
940940

941941

942942
// All of the following cache a recent block, and are protected by m_most_recent_block_mutex
@@ -1244,7 +1244,7 @@ std::chrono::microseconds PeerManagerImpl::NextInvToInbounds(std::chrono::micros
12441244
// If this function were called from multiple threads simultaneously
12451245
// it would possible that both update the next send variable, and return a different result to their caller.
12461246
// This is not possible in practice as only the net processing thread invokes this function.
1247-
m_next_inv_to_inbounds = GetExponentialRand(now, average_interval);
1247+
m_next_inv_to_inbounds = now + m_rng.rand_exp_duration(average_interval);
12481248
}
12491249
return m_next_inv_to_inbounds;
12501250
}
@@ -1698,7 +1698,7 @@ void PeerManagerImpl::ReattemptInitialBroadcast(CScheduler& scheduler)
16981698

16991699
// Schedule next run for 10-15 minutes in the future.
17001700
// We add randomness on every cycle to avoid the possibility of P2P fingerprinting.
1701-
const std::chrono::milliseconds delta = 10min + GetRandMillis(5min);
1701+
const auto delta = 10min + FastRandomContext().randrange<std::chrono::milliseconds>(5min);
17021702
scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta);
17031703
}
17041704

@@ -2050,7 +2050,7 @@ void PeerManagerImpl::StartScheduledTasks(CScheduler& scheduler)
20502050
scheduler.scheduleEvery([this] { this->CheckForStaleTipAndEvictPeers(); }, std::chrono::seconds{EXTRA_PEER_CHECK_INTERVAL});
20512051

20522052
// schedule next run for 10-15 minutes in the future
2053-
const std::chrono::milliseconds delta = 10min + GetRandMillis(5min);
2053+
const auto delta = 10min + FastRandomContext().randrange<std::chrono::milliseconds>(5min);
20542054
scheduler.scheduleFromNow([&] { ReattemptInitialBroadcast(scheduler); }, delta);
20552055
}
20562056

@@ -2124,7 +2124,7 @@ void PeerManagerImpl::BlockDisconnected(const std::shared_ptr<const CBlock> &blo
21242124
*/
21252125
void PeerManagerImpl::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock)
21262126
{
2127-
auto pcmpctblock = std::make_shared<const CBlockHeaderAndShortTxIDs>(*pblock, GetRand<uint64_t>());
2127+
auto pcmpctblock = std::make_shared<const CBlockHeaderAndShortTxIDs>(*pblock, FastRandomContext().rand64());
21282128

21292129
LOCK(cs_main);
21302130

@@ -2522,7 +2522,7 @@ void PeerManagerImpl::ProcessGetBlockData(CNode& pfrom, Peer& peer, const CInv&
25222522
if (a_recent_compact_block && a_recent_compact_block->header.GetHash() == pindex->GetBlockHash()) {
25232523
MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, *a_recent_compact_block);
25242524
} else {
2525-
CBlockHeaderAndShortTxIDs cmpctblock{*pblock, GetRand<uint64_t>()};
2525+
CBlockHeaderAndShortTxIDs cmpctblock{*pblock, FastRandomContext().rand64()};
25262526
MakeAndPushMessage(pfrom, NetMsgType::CMPCTBLOCK, cmpctblock);
25272527
}
25282528
} else {
@@ -5617,7 +5617,7 @@ void PeerManagerImpl::MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::mic
56175617
if (pingSend) {
56185618
uint64_t nonce;
56195619
do {
5620-
nonce = GetRand<uint64_t>();
5620+
nonce = FastRandomContext().rand64();
56215621
} while (nonce == 0);
56225622
peer.m_ping_queued = false;
56235623
peer.m_ping_start = now;
@@ -5654,13 +5654,13 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
56545654
CAddress local_addr{*local_service, peer.m_our_services, Now<NodeSeconds>()};
56555655
PushAddress(peer, local_addr);
56565656
}
5657-
peer.m_next_local_addr_send = GetExponentialRand(current_time, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
5657+
peer.m_next_local_addr_send = current_time + m_rng.rand_exp_duration(AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
56585658
}
56595659

56605660
// We sent an `addr` message to this peer recently. Nothing more to do.
56615661
if (current_time <= peer.m_next_addr_send) return;
56625662

5663-
peer.m_next_addr_send = GetExponentialRand(current_time, AVG_ADDRESS_BROADCAST_INTERVAL);
5663+
peer.m_next_addr_send = current_time + m_rng.rand_exp_duration(AVG_ADDRESS_BROADCAST_INTERVAL);
56645664

56655665
if (!Assume(peer.m_addrs_to_send.size() <= MAX_ADDR_TO_SEND)) {
56665666
// Should be impossible since we always check size before adding to
@@ -5747,13 +5747,13 @@ void PeerManagerImpl::MaybeSendFeefilter(CNode& pto, Peer& peer, std::chrono::mi
57475747
MakeAndPushMessage(pto, NetMsgType::FEEFILTER, filterToSend);
57485748
peer.m_fee_filter_sent = filterToSend;
57495749
}
5750-
peer.m_next_send_feefilter = GetExponentialRand(current_time, AVG_FEEFILTER_BROADCAST_INTERVAL);
5750+
peer.m_next_send_feefilter = current_time + m_rng.rand_exp_duration(AVG_FEEFILTER_BROADCAST_INTERVAL);
57515751
}
57525752
// If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY
57535753
// until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY.
57545754
else if (current_time + MAX_FEEFILTER_CHANGE_DELAY < peer.m_next_send_feefilter &&
57555755
(currentFilter < 3 * peer.m_fee_filter_sent / 4 || currentFilter > 4 * peer.m_fee_filter_sent / 3)) {
5756-
peer.m_next_send_feefilter = current_time + GetRandomDuration<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY);
5756+
peer.m_next_send_feefilter = current_time + m_rng.randrange<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY);
57575757
}
57585758
}
57595759

@@ -5984,7 +5984,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
59845984
CBlock block;
59855985
const bool ret{m_chainman.m_blockman.ReadBlockFromDisk(block, *pBestIndex)};
59865986
assert(ret);
5987-
CBlockHeaderAndShortTxIDs cmpctblock{block, GetRand<uint64_t>()};
5987+
CBlockHeaderAndShortTxIDs cmpctblock{block, m_rng.rand64()};
59885988
MakeAndPushMessage(*pto, NetMsgType::CMPCTBLOCK, cmpctblock);
59895989
}
59905990
state.pindexBestHeaderSent = pBestIndex;
@@ -6059,7 +6059,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
60596059
if (pto->IsInboundConn()) {
60606060
tx_relay->m_next_inv_send_time = NextInvToInbounds(current_time, INBOUND_INVENTORY_BROADCAST_INTERVAL);
60616061
} else {
6062-
tx_relay->m_next_inv_send_time = GetExponentialRand(current_time, OUTBOUND_INVENTORY_BROADCAST_INTERVAL);
6062+
tx_relay->m_next_inv_send_time = current_time + m_rng.rand_exp_duration(OUTBOUND_INVENTORY_BROADCAST_INTERVAL);
60636063
}
60646064
}
60656065

src/netaddress.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,8 @@ class CServiceHash
567567
{
568568
public:
569569
CServiceHash()
570-
: m_salt_k0{GetRand<uint64_t>()},
571-
m_salt_k1{GetRand<uint64_t>()}
570+
: m_salt_k0{FastRandomContext().rand64()},
571+
m_salt_k1{FastRandomContext().rand64()}
572572
{
573573
}
574574

0 commit comments

Comments
 (0)