Age | Commit message (Collapse) | Author |
|
We currently use both. Consolidate on the former.
|
|
|
|
|
|
|
|
436ce0233c276e263dcb441255dc0b881cb39cfb sync.h: strengthen AssertLockNotHeld assertion (Anthony Towns)
7d73f58e9cea8f4b0bc16512983898fddde3d764 Increase threadsafety annotation coverage (Anthony Towns)
Pull request description:
This changes `AssertLockNotHeld` so that it is annotated with the negative capability for the mutex it refers to. clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions.
Note that this can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex. At present, the only global mutexes that use `AssertLockNotHeld` are `RecursiveMutex` so we treat that as an exception in order to avoid having to add an excessive number of negative annotations.
ACKs for top commit:
vasild:
ACK 436ce0233c276e263dcb441255dc0b881cb39cfb
MarcoFalke:
review ACK 436ce0233c276e263dcb441255dc0b881cb39cfb 🌺
Tree-SHA512: 5f16d098790a36b5277324d5ee89cdc87033c19b11c7943c2f630a41c2e3998eb39d356a763e857f4d8fefb6c0c02291f720bb6769bcbdf5e2cd765bf266ab8c
|
|
|
|
|
|
3ae7791bcaa88f5c68592673b8926ee807242ce7 refactor: use Span in random.* (pasta)
Pull request description:
~This PR does two things~
1. use a Span<unsigned char> for GetRandBytes and GetStrongRandBytes
~2. make GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided.
This simplifies a lot of code from `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()`~
MarcoFalke this was inspired by your comment here: https://github.com/bitcoin/bitcoin/pull/24185#issuecomment-1025514263 about using Span, so hopefully I'll be able to get this PR done and merged 😂
~Also, if requested I could revert the `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()` related changes if it ends up causing too many conflicts~
ACKs for top commit:
laanwj:
Thank you! Code review re-ACK 3ae7791bcaa88f5c68592673b8926ee807242ce7
Tree-SHA512: 12375a83b68b288916ba0de81cfcab4aac14389a66a36811ae850427435eb67dd55e47df9ac3ec47db4e214f4330139e548bec815fff8a3f571484ea558dca79
|
|
|
|
|
|
|
|
Following best practices on OpenBSD. The getentropy(2) man page states:
"getentropy() is not intended for regular code;
please use the arc4random(3) family of functions instead."
|
|
This distribution is used for more than just the next inv send, so make
the name more generic.
Also rename to "exponential" to avoid the confusion that this is a
poisson distribution.
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1" ./src) ; }
ren PoissonNextSend GetExponentialRand
ren "a poisson timer" "an exponential timer"
-END VERIFY SCRIPT-
|
|
Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
|
|
PoissonNextSend is used by net and net_processing and is stateless, so
place it in the utility random.cpp translation unit.
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
* 2020: fa0074e2d82928016a43ca408717154a1c70a4db
* 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
Removes the macro ARRAYLEN and also substitutes all other uses of the same
"sizeof(a)/sizeof(a[0])" pattern by std::size, available since C++17.
|
|
ca2e47437277ef6851a739f247b44e73a53f21a1 Fix a compiler warning: unused GetDevURandom() (Vasil Dimov)
Pull request description:
~~Only define GetDevURandom() if it is going to be used.~~
Silence by planting a dummy reference to the `GetDevURandom` symbol
in the places where we don't call the function.
ACKs for top commit:
practicalswift:
ACK ca2e47437277ef6851a739f247b44e73a53f21a1 -- increased signal to noise in compiler diagnostics is good
sipa:
utACK ca2e47437277ef6851a739f247b44e73a53f21a1
hebasto:
re-ACK ca2e47437277ef6851a739f247b44e73a53f21a1, tested on macOS 10.15.6 + llvm clang 10.0.0
Tree-SHA512: 03c98f00dad5d9a3c5c9f68553d72ad5489ec02f18b9769108a22003ec7be7819a731b1eab6a9f64dafb5be0efddccf6980de7e3bb90cd20d4f4d72f74124675
|
|
0000ea32656833efa3d2ffd9bab66c88c83334f0 test: Add test for GetRandMillis and GetRandMicros (MarcoFalke)
fa0e5b89cf742df56c6c8f49fe9b3c54d2970a66 Add templated GetRandomDuration<> (MarcoFalke)
Pull request description:
A naive implementation of this template is dangerous, because the call site might accidentally omit the template parameter:
```cpp
template <typename D>
D GetRandDur(const D& duration_max)
{
return D{GetRand(duration_max.count())};
}
BOOST_AUTO_TEST_CASE(util_time_GetRandTime)
{
std::chrono::seconds rand_hour = GetRandDur(std::chrono::hours{1});
// Want seconds to be in range [0..1hour), but always get zero :((((
BOOST_CHECK_EQUAL(rand_hour.count(), 0);
}
```
Luckily `std::common_type` is already specialised in the standard lib for `std::chrono::duration` (https://en.cppreference.com/w/cpp/chrono/duration/common_type). And its effect seem to be that the call site must always specify the template argument explicitly.
So instead of implementing the function for each duration type by hand, replace it with a templated version that is safe to use.
ACKs for top commit:
laanwj:
Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0
promag:
Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0.
jonatack:
ACK 0000ea3 thanks for the improved documentation. Code review, built, ran `src/test/test_bitcoin -t random_tests -l test_suite` for the new unit tests, `git diff fa05a4c 0000ea3` since previous review:
hebasto:
ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0 with non-blocking [nit](https://github.com/bitcoin/bitcoin/pull/18781#discussion_r424924671).
Tree-SHA512: e89d46e31452be6ea14269ecbbb2cdd9ae83b4412cd14dff7d1084283092722a2f847cb501e8054394e4a3eff852f9c87f6d694fd008b3f7e8458cb5a3068af7
|
|
Enable -Wconditional-uninitialized to warn on potentially uninitialized
reads.
Fix the sole such warning in Bitcoin Core in GetRdRand(): r1 would be
set to 0 on rdrand failure, so initializing it to 0 is a non-functional
change.
From "Intel 64 and IA-32 ArchitecturesSoftware Developer's Manual" [1],
page 1711: "CF=1 indicates that the data in the destination is valid.
Otherwise CF=0 and the data in the destination operand will be returned
as zeros for the specified width."
[1] https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf
|
|
|
|
wallet privacy
50fc4df6c4e8a84bdda13ade7bed7a2131796f00 [mempool] Persist unbroadcast set to mempool.dat (Amiti Uttarwar)
297a1785360c4db662a7f3d3ade7b6b503258d39 [test] Integration tests for unbroadcast functionality (Amiti Uttarwar)
6851502472d3625416f0e7796e9f2a0379d14d49 [refactor/test] Extract P2PTxInvStore into test framework (Amiti Uttarwar)
dc1da48dc5e5526215561311c184a8cbc345ecdc [wallet] Update the rebroadcast frequency to be ~1/day. (Amiti Uttarwar)
e25e42f20a3aa39651fbc1f9fa3df1a49f1f5868 [p2p] Reattempt initial send of unbroadcast transactions (Amiti Uttarwar)
7e93eecce3bc5a1b7bb0284e06f9e2e69454f5ba [util] Add method that returns random time in milliseconds (Amiti Uttarwar)
89eeb4a3335f8e871cc3f5286af4546dff66172a [mempool] Track "unbroadcast" transactions (Amiti Uttarwar)
Pull request description:
This PR introduces mempool tracking of unbroadcast transactions and periodic reattempts at initial broadcast. This is a part of the rebroadcast project, and a standalone privacy win.
The current rebroadcast logic is terrible for privacy because 1. only the source wallet rebroadcasts transactions and 2. it does so quite frequently. In the current system, if a user submits a transaction that does not immediately get broadcast to the network (eg. they are offline), this "rebroadcast" behavior is the safety net that can actually serve as the initial broadcast. So, keeping the attempts frequent is important for initial delivery within a reasonable timespan.
This PR aims to improve # 2 by reducing the wallet rebroadcast frequency to ~1/day from ~1/15 min. It achieves this by separating the notion of initial broadcast from rebroadcasts. With these changes, the mempool tracks locally submitted transactions & periodically reattempts initial broadcast. Transactions submitted via the wallet or RPC are added to an "unbroadcast" set & are removed when a peer sends a `getdata` request, or the transaction is removed from the mempool. Every 10-15 minutes, the node reattempts an initial broadcast. This enables reducing the wallet rebroadcast frequency while ensuring the transactions will be propagated to the network.
For privacy improvements around # 1, please see #16698.
Thank you to gmaxwell for the idea of how to break out this subset of functionality (https://github.com/bitcoin/bitcoin/pull/16698#issuecomment-571399346)
ACKs for top commit:
fjahr:
Code review ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00
MarcoFalke:
ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00, I think this is ready for merge now 👻
amitiuttarwar:
The current tip `50fc4df` currently has 6 ACKs on it, so I've opened #18807 to address the last bits.
jnewbery:
utACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00.
ariard:
Code Review ACK 50fc4df (minor points no need to invalid other ACKs)
robot-visions:
ACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00
sipa:
utACK 50fc4df6c4e8a84bdda13ade7bed7a2131796f00
naumenkogs:
utACK 50fc4df
Tree-SHA512: 2dd935d645d5e209f8abf87bfaa3ef0e4492705ce7e89ea64279cb27ffd37f4727fa94ad62d41be331177332f8edbebf3c7f4972f8cda10dd951b80a28ab3c0f
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
```
random.cpp:255:13: error: unused function 'GetDevURandom' [-Werror,-Wunused-function]
```
Clang 9.0.0, FreeBSD 12.1
Silence by planting a dummy reference to the `GetDevURandom` symbol
in the places where we don't call the function.
|
|
sysctl() on *BSD takes a "const int *name", whereas sysctl() on macOS
it takes an "int *name". So our configure check and sysctl() detection on
macOS currently fails:
```bash
/usr/include/sys/sysctl.h:759:9: note: candidate function not viable:
no known conversion from 'const int [2]' to 'int *' for 1st argument
int sysctl(int *, u_int, void *, size_t *, void *, size_t);
```
This change removes the name argument from the sysctl() detection check,
meaning we will detect correctly on macOS and *BSD.
For consistency we also switch to using the more generic, non-const
version of the name parameter in the rest of our usage.
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
|
|
55b2cb199c276781b6daa5438af2da57dea3ac52 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake)
461e547877da0c04db69e067c923cc4540aab03a doc: correct random.h docs after #17270 (fanquake)
Pull request description:
The usage of `MilliSleep()` in SeedPeriodic (previously SeedSleep) was
[removed](https://github.com/bitcoin/bitcoin/pull/17270/commits/d61f2bb076d8f17840a8e79f1583d7f6e3e6d09a) in #17270, meaning it, and its users can now be marked `noexcept`.
This also corrects the docs in random.h for some of the changes in #17270.
ACKs for top commit:
practicalswift:
ACK 55b2cb199c276781b6daa5438af2da57dea3ac52
laanwj:
ACK 55b2cb199c276781b6daa5438af2da57dea3ac52
sipa:
ACK 55b2cb199c276781b6daa5438af2da57dea3ac52
Tree-SHA512: 672d369796e7c4f9b4d98dc545e5454999fa1bef373871994a26041d6163c58909e2255e4f820d3ef011679aa3392754eb57477306a89f5fd3d57e2bd7f0811a
|
|
|
|
The usage of MilliSleep() in SeedPeriodic (previously SeedSleep) was
removed in #17270, meaning it, and its users can now be marked noexcept.
|
|
|
|
|
|
On the ::SLOW path we would use OpenSSL as an additional source of
random bytes. This commit removes that functionality. Note that this was
always only an additional source, and that we never checked the return
value
RAND_bytes(): https://www.openssl.org/docs/manmaster/man3/RAND_bytes.html
RAND_bytes() puts num cryptographically strong pseudo-random bytes into buf.
|
|
On the ::SLOW or ::SLEEP paths, we would feed our RNG output back into
OpenSSL using RAND_add. This commit removes that functionality.
RAND_add(): https://www.openssl.org/docs/manmaster/man3/RAND_add.html
RAND_add() mixes the num bytes at buf into the internal state of the
random generator. This function will not normally be needed, as
mentioned above. The randomness argument is an estimate of how much
randomness is contained in buf, in bytes, and should be a number
between zero and num.
|
|
|
|
* Instead of calling RandAddSeedSleep anytime the scheduler goes
idle, call its replacement (RandAddSeedPeriodic) just once per
minute. This has better guarantees of actually being run, and
helps limit how frequently the dynamic env data is gathered.
* Since this code runs once per minute regardless now, we no
longer need to keep track of the last time strengthening was
run; just do it always.
* Make strengthening time context dependent (100 ms at startup,
10 ms once per minute afterwards).
|
|
|
|
|
|
This should have been part of #17151.
|
|
Follow up to https://github.com/bitcoin/bitcoin/pull/17151 where
there were multiple calls to also remove our call to RAND_screen().
|
|
|
|
|
|
|
|
3cb9ce85d0c6d01217babf0df7efc2eabde1b12f Document strenghtening (Pieter Wuille)
1d207bc46f995ad3b5ae89bb504affaca09d10b1 Add hash strengthening to the RNG (Pieter Wuille)
Pull request description:
This patch improves the built-in RNG using hash strengthening.
At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.
ACKs for commit 3cb9ce:
pstratem:
utACK 3cb9ce85d0c6d01217babf0df7efc2eabde1b12f
Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
|
|
Once every minute, this will feed the RNG state through repeated SHA512
for 10ms. The timings of that operation are used as entropy source as
well.
|