| Age | Commit message (Collapse) | Author |
|---|
|
74bf850ac47735f2ef4306059d3e664d40cac85e faster HexStr => 13% faster blockToJSON (Martin Ankerl)
Pull request description:
`std::string`'s push_back is rather slow because it needs to check & update the string size. For
`HexStr` the output string size is already easily know, so we can initially create the string with
the correct size and then just assign the data.
`HexStr` is heavily usd in `blockToJSON`, so this change is a noticeable benefit. Benchmark on an i7-8700 @3.2GHz:
* 71,315,461.00 ns/op master
* 62,842,490.00 ns/op this commit
So this little change makes `blockToJSON` about ~13% faster.
ACKs for top commit:
laanwj:
Code review ACK 74bf850ac47735f2ef4306059d3e664d40cac85e
theStack:
re-ACK 74bf850ac47735f2ef4306059d3e664d40cac85e
Tree-SHA512: fc99105123edc11f4e40ed77aea80cf7f32e49c53369aa364b38395dcb48575e15040b0489ed30d0fe857c032a04e225c33e9d95cdfa109a3cb5a6ec9a972415
|
|
fa340b87944764ea4e8e04038fe7471fd452bc23 refactor: Avoid magic value of all-zeros in assumeutxo base_blockhash (MarcoFalke)
fae33f98e6a8d5934edbdce2eb8688112eac41a8 Fix assumeutxo crash due to invalid base_blockhash (MarcoFalke)
fa5668bfb34e2778936af30e9fb6bd3c6bcf41fd refactor: Use type-safe assumeutxo hash (MarcoFalke)
00000077098a23e0ac9781f5f799c90fd2fd97de refactor: Remove unused code (MarcoFalke)
faa921f78788be0f236c6b10ed3f92dcf148d1cb move-only: Add util/hash_type (MarcoFalke)
Pull request description:
Starting with commit d6af06d68aa, a block hash of all-zeros is invalid and will lead to a crash of the node. Can be tested by cherry-picking the test changes without the other changes.
Stack trace (copied from https://github.com/bitcoin/bitcoin/pull/21584#discussion_r612673879):
```
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007ffff583c8b1 in __GI_abort () at abort.c:79
#2 0x00007ffff582c42a in __assert_fail_base (fmt=0x7ffff59b3a38 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion@entry=0x555556c8b450 "!hashBlock.IsNull()", file=file@entry=0x555556c8b464 "txdb.cpp", line=line@entry=89,
function=function@entry=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:92
#3 0x00007ffff582c4a2 in __GI___assert_fail (assertion=0x555556c8b450 "!hashBlock.IsNull()", file=0x555556c8b464 "txdb.cpp", line=89,
function=0x555556c8b46d "virtual bool CCoinsViewDB::BatchWrite(CCoinsMap &, const uint256 &)") at assert.c:101
#4 0x000055555636738b in CCoinsViewDB::BatchWrite (this=0x5555577975c0, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at txdb.cpp:89
#5 0x00005555564a2e80 in CCoinsViewBacked::BatchWrite (this=0x5555577975f8, mapCoins=std::unordered_map with 110 elements = {...}, hashBlock=...) at coins.cpp:30
#6 0x00005555564a43de in CCoinsViewCache::Flush (this=0x55555778eaf0) at coins.cpp:223
#7 0x00005555563fc11d in ChainstateManager::PopulateAndValidateSnapshot (this=0x55555740b038 <g_chainman>, snapshot_chainstate=..., coins_file=..., metadata=...)
at validation.cpp:5422
#8 0x00005555563fab3d in ChainstateManager::ActivateSnapshot (this=0x55555740b038 <g_chainman>, coins_file=..., metadata=..., in_memory=true) at validation.cpp:5299
#9 0x0000555555e8c893 in validation_chainstatemanager_tests::CreateAndActivateUTXOSnapshot<validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12>(NodeContext&, boost::filesystem::path, validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method()::$_12) (node=...,
root=..., malleation=...) at test/validation_chainstatemanager_tests.cpp:199
#10 0x0000555555e8877a in validation_chainstatemanager_tests::chainstatemanager_activate_snapshot::test_method (this=0x7fffffffc8d0)
at test/validation_chainstatemanager_tests.cpp:262
ACKs for top commit:
laanwj:
Code review re-ACK fa340b87944764ea4e8e04038fe7471fd452bc23
jamesob:
ACK fa340b87944764ea4e8e04038fe7471fd452bc23 ([`jamesob/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due`](https://github.com/jamesob/bitcoin/tree/ackr/21584.1.MarcoFalke.fix_assumeutxo_crash_due))
Tree-SHA512: c2c4e66c1abfd400ef18a04f22fec1f302f1ff4d27a18050f492f688319deb4ccdd165ff792eee0a1f816e7b69fb64080662b79517ab669e3d26b9eb77802851
|
|
792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov)
a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov)
30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov)
Pull request description:
This PR does not change behavior.
Its goal is to improve readability and maintainability of the code.
ACKs for top commit:
jnewbery:
utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
jonatack:
tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
MarcoFalke:
cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
|
|
Can be reviewed with --color-moved=dimmed-zebra
|
|
This reverts commit eac6a3080d38cfd4eb7204ecd327df213958e51a ("refactor:
Rework asmap Interpret to avoid ptrdiff_t"), because it is UB to form a
past-the-end iterator, even if it is never dereferenced.
Then fix the compiler warning in a different way:
Instead of comparing an uint32_t against a signed ptrdiff_t, just
promote both to a type that can represent both types.
Even though in this case the ptrdiff_t should never hold a negative
value, the overhead from promotion should be negligible.
|
|
Also it is moved into its own module.
|
|
|
|
|
|
|
|
|
|
|
|
916ab0195d567fd0a9097045e73a6654c453adea remove unused class util::Ref and its unit test (Sebastian Falbesoner)
8dbb87a3932f81e23ba7afd865b9aeeb535f0c20 refactor: replace util::Ref by std::any (C++17) (Sebastian Falbesoner)
95cccf8a4b392959c1fd7ec0647e04eb13880865 util: introduce helper AnyPtr to access std::any instances (Sebastian Falbesoner)
Pull request description:
As described in `util/ref.h`: "_This implements a small subset of the functionality in C++17's std::any class, and **can be dropped when the project updates to C++17**_". For accessing the contained object of a `std::any` instance, a helper template function `AnyPtr` is introduced (thanks to ryanofsky).
ACKs for top commit:
hebasto:
re-ACK 916ab0195d567fd0a9097045e73a6654c453adea, with command
ryanofsky:
Code review ACK 916ab0195d567fd0a9097045e73a6654c453adea. Changes since last review: rebase and replacing types with `auto`. I might have used `const auto*` and `auto*` instead of plain `auto` because I think the qualifiers are useful, but this is all good.
Tree-SHA512: fe2c3e4f5726f8ad40c61128339bb24ad11d2c261f71f7b934b1efe3e3279df14046452b0d9b566917ef61d5c7e0fd96ccbf35ff810357e305710f5002c27d47
|
|
40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 test: add I2P test for a runaway SAM proxy (Vasil Dimov)
2d8ac779708322e1235e823edfc9c8f6e2dd65e4 fuzz: add tests for the I2P Session public interface (Vasil Dimov)
9947e44de0cbd79e99d883443a9ac441d8c69713 i2p: use pointers to Sock to accommodate mocking (Vasil Dimov)
82d360b5a88d9057b6c09b61cd69e426c7a2412d net: change ConnectSocketDirectly() to take a Sock argument (Vasil Dimov)
b5861100f85fef77b00f55dcdf01ffb4a2a112d8 net: add connect() and getsockopt() wrappers to Sock (Vasil Dimov)
5a887d49b2b39e59d7cce8e9d5b89c21ad694f8b fuzz: avoid FuzzedSock::Recv() repeated errors with EAGAIN (Vasil Dimov)
3088f83d016e7ebb6e6aa559e6326fa0ef0d6282 fuzz: extend FuzzedSock::Recv() to support MSG_PEEK (Vasil Dimov)
9b05c49ade729311a0f4388a109530ff8d0ed1f9 fuzz: implement unimplemented FuzzedSock methods (Vasil Dimov)
Pull request description:
Change the networking code and the I2P code to be fully mockable and use `FuzzedSocket` to fuzz the I2P methods `Listen()`, `Accept()` and `Connect()`.
Add a mocked `Sock` implementation that returns a predefined data on reads and use it for a regression unit test for the bug fixed in https://github.com/bitcoin/bitcoin/pull/21407.
ACKs for top commit:
practicalswift:
Tested ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5
MarcoFalke:
Concept ACK 40316a37cb
jonatack:
re-ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5 reviewed `git range-diff 01bb3afb 23c861d 40316a3` and the new unit test commit, debug built, ran unit tests, ran bitcoind with an I2P service and network operation with seven I2P peers (2 in, 5 out) is looking nominal
laanwj:
Code review ACK 40316a37cb02cf8a9a8b2cbd4d7153ffa57e7ec5
Tree-SHA512: 7fc4f129849e16e0c7e16662d9f4d35dfcc369bb31450ee369a2b97bdca95285533bee7787983e881e5a3d248f912afb42b4a2299d5860ace7129b0b19623cc8
|
|
|
|
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
|
|
fa4cebadcffd9112da4b13c7cc7ccf21e2bee887 util: Make Assume() usable as unary expression (MarcoFalke)
Pull request description:
Assume shouldn't behave different at the call site depending on build flags. Currently compilation fails if it is used as expression. Fix that by using the lambda approach from `Assert()` without the `assert()`.
ACKs for top commit:
jnewbery:
ACK fa4cebadcffd9112da4b13c7cc7ccf21e2bee887
practicalswift:
cr ACK fa4cebadcffd9112da4b13c7cc7ccf21e2bee887: patch looks correct and commit hash starts with `fa`
Tree-SHA512: 9ec9ac8d410cdaf5e4e28df571a89e3d23d38e05a7027bb726cae3da6e9314734277e5a218e9e090cc17e10db763da71052c229ad642077ca5824ee42022f3ed
|
|
52dd40a9febec1f4e70d777821b6764830bdec61 test: add missing netaddress include headers (Jon Atack)
6f09c0f6b57ac01a473c587a3e51e9d477866bb0 util: add missing braces and apply clang format to SplitHostPort() (Jon Atack)
2875a764f7d8b1503c7bdb2f262964f7a0cb5fc3 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack)
6423c8175fad3163c10ffdb49e0df48e4e4931f1 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack)
Pull request description:
As noticed during review today in https://github.com/bitcoin/bitcoin/pull/20685#discussion_r584873708 of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch.
ACKs for top commit:
practicalswift:
cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61: patch looks correct
MarcoFalke:
cr ACK 52dd40a9febec1f4e70d777821b6764830bdec61
vasild:
ACK 52dd40a9febec1f4e70d777821b6764830bdec61
Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432
|
|
|
|
|
|
|
|
Extend the `Sock` class with wrappers to `connect()` and `getsockopt()`.
This will make it possible to mock code which uses those.
|
|
We want `Get()` to always return the same value, otherwise it will look
like the `FuzzedSock` implementation itself is broken. So assign
`m_socket` a random number in the `FuzzedSock` constructor.
There is nothing to fuzz about the `Get()` and `Release()` methods, so
use the ones from the base class `Sock`.
`Reset()` is just setting our socket to `INVALID_SOCKET`. We don't want
to use the base `Reset()` because it will close `m_socket` and given
that our `m_socket` is just a random number it may end up closing a real
opened file descriptor if it coincides with our random `m_socket`.
|
|
7059e6d82275b44efc41675ee10760145b6c1073 test: add a test to ensure RecvUntilTerminator() limit works (Vasil Dimov)
80a5a8ea2b7ad512c74c29df5b504e9be6cf23a0 i2p: limit the size of incoming messages (Vasil Dimov)
Pull request description:
Put a limit on the amount of data `Sock::RecvUntilTerminator()` can read
if no terminator is received.
In the case of I2P this avoids a runaway (or malicious) I2P proxy
sending us tons of data without a terminator before a timeout is
triggered.
ACKs for top commit:
laanwj:
Re-ACK 7059e6d82275b44efc41675ee10760145b6c1073
Tree-SHA512: 21f3f3468c765c726cdc877eae847cdb4dbe225d94c5bd1849bd752c9761fac881c554f16ea7a685ad40312159d554e126c481e21c5fd83a6d947060b920373d
|
|
Put a limit on the amount of data `Sock::RecvUntilTerminator()` can read
if no terminator is received.
In the case of I2P this avoids a runaway (or malicious) I2P proxy
sending us tons of data without a terminator before a timeout is
triggered.
|
|
1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff doc: update developer notes for removal of MakeUnique (fanquake)
3ba2840e7ee81341b0748c0121aedc2e9305041a scripted-diff: remove MakeUnique<T>() (fanquake)
Pull request description:
Since requiring C++17, this is just pointless abstraction. I think we should just "tear the band-aid off" and remove it. Similar to the changes happening in #21366.
Also, having a comment saying this is deprecated doesn't prevent it's usage in new code. i.e : https://github.com/bitcoin/bitcoin/pull/20946#discussion_r561949731.
The repository is fairly quiet at the moment, so any potential complaints about having to rebase should be minimal. Might as well get this over and done with.
ACKs for top commit:
jnewbery:
utACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff
practicalswift:
cr ACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff: patch looks correct
ajtowns:
ACK 1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff -- code review only
glozow:
ACK https://github.com/bitcoin/bitcoin/commit/1a6323bdbe20bdb7b1c907d8fa0333ffa88b21ff looks correct
Tree-SHA512: 4a14b9611b60b9b3026b54d6f5a2dce4c5d9b63a7b93d7de1307512df736503ed84bac66e7b93372c76e3117f49bf9f29cd473d3a47cb41fb2775bc10234736f
|
|
ebc4ab721b0371c0ef217c0f5bd7d42613e951e6 refactor: post Optional<> removal cleanups (fanquake)
57e980d13ca488031bde6ef197cf34d493d36796 scripted-diff: remove Optional & nullopt (fanquake)
Pull request description:
Same rationale & motivation as #21404, which turned out to be quite low in the number of potential conflicts. Lets see what the bot has to say here.
ACKs for top commit:
practicalswift:
cr ACK ebc4ab721b0371c0ef217c0f5bd7d42613e951e6: patch looks correct
jnewbery:
utACK ebc4ab721b0371c0ef217c0f5bd7d42613e951e6
laanwj:
Code review ACK ebc4ab721b0371c0ef217c0f5bd7d42613e951e6
Tree-SHA512: 550fbeef09b9d35ddefaa805d1755c18c8fd499c4b0f77ebfece8c20296a7abd1cf6c699e2261f92fe3552deeb7555ec2a2287ffe3ab9e98bb9f8612a4d43be3
|
|
|
|
-BEGIN VERIFY SCRIPT-
git rm src/optional.h
sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src)
sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src)
sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src)
sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src)
sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src)
sed -i -e '/optional.h \\/d' src/Makefile.am
sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp
sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src)
-END VERIFY SCRIPT-
|
|
e017a913d0d78ef0766cf73586fe7a38488e1a26 bitcoind: Add -daemonwait option to wait for initialization (Wladimir J. van der Laan)
c3e6fdee6d39d3f52dec421b48a0ac8bad5006f7 shutdown: Use RAII TokenPipe in shutdown (Wladimir J. van der Laan)
612f746a8ffa265b6877bedbbe21fcbb392f1516 util: Add RAII TokenPipe (Wladimir J. van der Laan)
Pull request description:
This adds a `-daemonwait` flag that does the same as `-daemon` except that it, from a user perspective, backgrounds the process only after initialization is complete. This is similar to the behaviour of some other software such as c-lightning.
This can be useful when the process launching bitcoind wants to guarantee that either the RPC server is running, or that initialization failed, before continuing. The exit code indicates the initialization result.
The use of the libc function `daemon()` is replaced by a custom implementation which is inspired by the [glibc implementation](https://github.com/lattera/glibc/blob/master/misc/daemon.c#L44), but which also creates a pipe from the child to the parent process for communication.
An additional advantage of having our own `daemon()` implementation is that no MACOS-specific pragmas are needed anymore to silence a deprecation warning.
TODO:
- [x] Factor out `token_read` and `token_write` to an utility, and use them in `shutdown.cpp` as well—this is exactly the same kind of communication mechanism.
- [x] RAII-ify pipe endpoints.
- [x] Improve granularity of the `configure.ac` checks. This currently still checks for the function `daemon()` which makes no sense as it's not used. It should check for individual functions such as
`fork()` and `setsid()` etc—the former being required, the second optional.
- [-] ~~Signal propagation during initialization: if say, pressing Ctrl-C during `-daemonwait` it would be good to pass this SIGINT on to the child process instead of detaching the parent process and letting the child run free.~~ This is not necessary, see https://github.com/bitcoin/bitcoin/pull/21007#issuecomment-769007341.
Future:
- Consider if it makes sense to use this in the RPC tests (there would be no more need for "is RPC ready" polling loops). I think this is out of scope for this PR.
ACKs for top commit:
jonatack:
Tested ACK e017a913d0d78ef0766cf73586fe7a38488e1a26 checked change since previous review is move-only
Tree-SHA512: 53369b8ca2247e4cf3af8cb2cfd5b3399e8e0e3296423d64be987004758162a7ddc1287b01a92d7692328edcb2da4cf05d279b1b4ef61a665b71440ab6a6dbe2
|
|
-BEGIN VERIFY SCRIPT-
git rm src/util/memory.h
sed -i -e 's/MakeUnique/std::make_unique/g' $(git grep -l MakeUnique src)
sed -i -e '/#include <util\/memory.h>/d' $(git grep -l '#include <util/memory.h>' src)
sed -i -e '/util\/memory.h \\/d' src/Makefile.am
-END VERIFY SCRIPT-
|
|
|
|
0eaea66e8bfdfb23ff86c0f0924c2d75f5aca75f Make tx relay data structure use std::chrono types (Pieter Wuille)
55e82881a1503bff146970856c1474a6ea659c94 Make all Poisson delays use std::chrono types (Pieter Wuille)
c733ac4d8a597b1001555b142f488ed9fbecc405 Convert block/header sync timeouts to std::chrono types (Pieter Wuille)
4d98b401fbd821700f7a792b0a4cb52c9b71bc9f Change all ping times to std::chrono types (Pieter Wuille)
Pull request description:
(Picking up #20044. Rebased against master.)
This changes various uses of integers to represent timestamps and durations to `std::chrono` duration types with type-safe conversions, getting rid of various `.count()`, constructors, and conversion factors.
ACKs for top commit:
jnewbery:
utACK 0eaea66e8bfdfb23ff86c0f0924c2d75f5aca75f
vasild:
ACK 0eaea66e8bfdfb23ff86c0f0924c2d75f5aca75f
MarcoFalke:
re-ACK 0eaea66e8bfdfb23ff86c0f0924c2d75f5aca75f, only changes: minor rename, using C++11 member initializer, using 2min chrono literal, rebase 🤚
ajtowns:
utACK 0eaea66e8bfdfb23ff86c0f0924c2d75f5aca75f
Tree-SHA512: 2dbd8d53bf82e98f9b4611e61dc14c448e8957d1a02575b837fadfd59f80e98614d0ccf890fc351f960ade76a6fb8051b282e252e81675a8ee753dba8b1d7f57
|
|
|
|
|
|
|
|
CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min()
|
|
This will be convenient in the I2P SAM implementation.
|
|
Introduce two high level, convenience methods in the `Sock` class:
* `SendComplete()`: keep trying to send the specified data until either
successfully sent all of it, timeout or interrupted.
* `RecvUntilTerminator()`: read until a terminator is encountered (never
after it), timeout or interrupted.
These will be convenient in the I2P SAM implementation.
`SendComplete()` can also be used in the SOCKS5 implementation instead
of calling `send()` directly.
|
|
Previously `Sock::Wait()` would not have signaled to the caller whether
a timeout or one of the requested events occurred since that was not
needed by any of the callers.
Such functionality will be needed in the I2P implementation, thus extend
the `Sock::Wait()` method.
|
|
Move `maxWait` out of `InterruptibleRecv()` and rename it to
`MAX_WAIT_FOR_IO` so that it can be reused by other code.
|
|
`fclose()` is flushing any buffered data to disk, so if it fails then
that could mean that the data was not completely written to disk.
Thus, check if `fclose()` succeeds and only then claim success from
`WriteBinaryFile()`.
|
|
If an error occurs and `fread()` returns `0` (nothing was read) then the
code before this patch would have returned "success" with a partially
read contents of the file.
|
|
Extract `ReadBinaryFile()` and `WriteBinaryFile()` from `torcontrol.cpp`
to its own `readwritefile.{h,cpp}` files, so that it can be reused from
other modules.
|
|
fd6580e405699ccb051fd2a34525e48d3253673d [refactor] txmempool: split epoch logic into class (Anthony Towns)
Pull request description:
Splits the epoch logic introduced in #17925 into a separate class.
Uses clang's thread safety annotations and encapsulates the data more strongly to reduce chances of bugs from API misuse.
ACKs for top commit:
jonatack:
ACK fd6580e405699ccb051fd2a34525e48d3253673d using clang thread safety annotations looks like a very good idea, and the encapsulation this change adds should improve robustness (and possible unit test-ability) of the code. Verified that changing some of the locking duly provoked build-time warnings with Clang 9 on Debian and that small changes in the new `Epoch` class were covered by failing functional test assertions in `mempool_updatefromblock.py`, `mempool_resurrect.py`, and `mempool_reorg.py`
hebasto:
re-ACK fd6580e405699ccb051fd2a34525e48d3253673d, since my [previous](https://github.com/bitcoin/bitcoin/pull/18017#pullrequestreview-569619362) review:
Tree-SHA512: 7004623faa02b56639aa05ab7a078320a6d8d54ec62d8022876221e33f350f47df51ddff056c0de5be798f8eb39b5c03c2d3f035698555d70abc218e950f2f8c
|
|
|
|
This option replaces --with-boost-process
This prepares external signer support to be disabled by default.
It adds a configure option to enable this feature and to check
if Boost::Process is present.
This also exposes ENABLE_EXTERNAL_SIGNER to the test suite via test/config.ini
|
|
location if -logsourcelocations is set
b4511e2e2ed1a6077ae6826a9ee6b7a311293d08 log: Prefix log messages with function name if -logsourcelocations is set (practicalswift)
Pull request description:
Prefix log messages with function name if `-logfunctionnames` is set.
Yes, exactly like `-logthreadnames` but for function names instead of thread names :)
This is a small developer ergonomics improvement: I've found this to be a cheap/simple way to correlate log output and originating function.
For me it beats the ordinary cycle of 1.) try to figure out a regexp matching the static part of the dynamic log message, 2.) `git grep -E 'Using .* MiB out of .* requested for signature cache'`, 3.) `mcedit filename.cpp` (`openemacs filename.cpp` works too!) and 4.) search for log message and scroll up to find the function name :)
Without any logging parameters:
```
$ src/bitcoind -regtest
2020-08-25T03:29:04Z Using RdRand as an additional entropy source
2020-08-25T03:29:04Z Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
2020-08-25T03:29:04Z Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
2020-08-25T03:29:04Z Loaded best chain: hashBestChain=0fff88f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e22ff height=0 date=2011-02-02T23:16:42Z progress=1.000000
2020-08-25T03:29:04Z block tree size = 1
2020-08-25T03:29:04Z nBestHeight = 0
2020-08-25T03:29:04Z Imported mempool transactions from disk: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
2020-08-25T03:29:04Z 0 addresses found from DNS seeds
```
With `-logthreadnames` and `-logfunctionnames`:
```
$ src/bitcoind -regtest -logthreadnames -logfunctionnames
2020-08-25T03:29:04Z [init] [ReportHardwareRand] Using RdRand as an additional entropy source
2020-08-25T03:29:04Z [init] [InitSignatureCache] Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
2020-08-25T03:29:04Z [init] [InitScriptExecutionCache] Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
2020-08-25T03:29:04Z [init] [LoadChainTip] Loaded best chain: hashBestChain=0fff88f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e22ff height=0 date=2011-02-02T23:16:42Z progress=1.000000
2020-08-25T03:29:04Z [init] [AppInitMain] block tree size = 1
2020-08-25T03:29:04Z [init] [AppInitMain] nBestHeight = 0
2020-08-25T03:29:04Z [loadblk] [LoadMempool] Imported mempool transactions from disk: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
2020-08-25T03:29:04Z [dnsseed] [ThreadDNSAddressSeed] 0 addresses found from DNS seeds
```
ACKs for top commit:
laanwj:
Code review ACK b4511e2e2ed1a6077ae6826a9ee6b7a311293d08
MarcoFalke:
review ACK b4511e2e2ed1a6077ae6826a9ee6b7a311293d08 🌃
Tree-SHA512: d100f5364630c323f31d275259864c597f7725e462d5f4bdedcc7033ea616d7fc0d16ef1b2af557e692f4deea73c6773ccfc681589e7bf6ba970b9ec169040c7
|
|
e829c9afbf75e930db6c3fe77a269b0af5e7a3ad refactor: replace sizeof(a)/sizeof(a[0]) by std::size (C++17) (Sebastian Falbesoner)
365539c84691d470b44d35df374d8c049f8c1192 refactor: init vectors via std::{begin,end} to avoid pointer arithmetic (Sebastian Falbesoner)
63d4ee1968144cc3d115f92baef95785abf813ac refactor: iterate arrays via C++11 range-based for loops if idx is not needed (Sebastian Falbesoner)
Pull request description:
This refactoring PR picks up the idea of #19626 and replaces all occurences of `sizeof(x)/sizeof(x[0])` (or `sizeof(x)/sizeof(*x)`, respectively) with the now-available C++17 [`std::size`](https://en.cppreference.com/w/cpp/iterator/size) (as [suggested by sipa](https://github.com/bitcoin/bitcoin/pull/19626#issuecomment-666487228)), making the macro `ARRAYLEN` obsolete.
As preparation for this, two other changes are done to eliminate `sizeof(x)/sizeof(x[0])` usage:
* all places where arrays are iterated via an index are changed to use C++11 range-based for loops If the index' only purpose is to access the array element (as [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/19626#discussion_r463404541)).
* `std::vector` initializations are done via `std::begin` and `std::end` rather than using pointer arithmetic to calculate the end (also [suggested by MarcoFalke](https://github.com/bitcoin/bitcoin/pull/20429#discussion_r567418821)).
ACKs for top commit:
practicalswift:
cr ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad: patch looks correct
fanquake:
ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad
MarcoFalke:
review ACK e829c9afbf75e930db6c3fe77a269b0af5e7a3ad 🌩
Tree-SHA512: b01d32c04b9e04d562b7717cae00a651ec9a718645047a90761be6959e0cc2adbd67494e058fe894641076711bb09c3b47a047d0275c736f0b2218e1ce0d193d
|
|
9266f7497f256d780178829e0f3a29ddaeb794ba util: Use std::chrono for time getters (MarcoFalke)
3c2e16be22ae04bf56663ee5ec1554d0d569741b time: add runtime sanity check (Cory Fields)
Pull request description:
I have a followup that should remove the last of our `boost:posix_time` usage in `ParseISO8601DateTime`, but that will likely need more cross-platform testing/discussion, so have just split them up as this change is straight forward.
ACKs for top commit:
practicalswift:
Tested ACK 9266f7497f256d780178829e0f3a29ddaeb794ba
laanwj:
Code review ACK 9266f7497f256d780178829e0f3a29ddaeb794ba
Tree-SHA512: 5471a60e65e9fa8ef48320743ef637f1d162724e717e0f5509118e1e5732fc0844656a9c09d3d1300eb657dcc7a1e1e67305d8c9ef959c63be67393607dd4ceb
|
|
|
