d31e5c1 Fix shutdown in case of errors during initialization (Wladimir J. van der Laan)
Pull request description:
PR #10286 introduced a few steps which are not robust to early shutdown in initialization.
Stumbled upon this with #11781, not sure if there are other scenarios that can trigger it, but it's good to harden against this in any case.
E.g.
```
$ src/bitcoind -debuglogfile=/dfdf
Error: Could not open debug log file /dfdf
Program received signal SIGSEGV, Segmentation fault.
UnregisterValidationInterface (pwalletIn=0x0) at /.../bitcoin/src/validationinterface.cpp:82
82 g_signals.m_internals->BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
(gdb) bt
#0 UnregisterValidationInterface (pwalletIn=0x0) at /.../bitcoin/src/validationinterface.cpp:82
#1 0x00005555555a11fc in Shutdown () at /.../bitcoin/src/init.cpp:196
#2 0x00005555555961cc in AppInit (argc=<optimized out>, argv=<optimized out>) at /.../bitcoin/src/bitcoind.cpp:183
#3 0x0000555555596249 in main (argc=0, argv=0x555555ecf200) at /.../bitcoin/src/bitcoind.cpp:19
```
Tree-SHA512: 7dd9570a9803514a17781bfadf1edde47e96df4e852cce2f423cab422e005fb94d44e777af1a6ea5167b04a4d889e848ae7a61a7e0e94232247ddea32ee70fc8
543ab40 Docs: Bump OSX version to 10.13 in build-osx.md (Varunram Ganesh)
Pull request description:
Core works fine on macOS 10.13
Tree-SHA512: 44807920e3d5518c98d68191ed614019934bb702df9695389f8178a00a7c5afccd90fad68a07568cdc3e3d04068bddf434942fb81dceb6cbde9bb4ff4e49e425
c60c49b Improve help text and behavior of RPC-logging (Akio Nakamura)
Pull request description:
1. It is allowed `libevent` logging to be updated during runtime,
but still described that restriction in the help text.
So we delete these text.
2. Add a descrption about the evaluation order of `<include>` and
`<exclude>` to clarify how debug loggig categories to be set.
3. Add a description about the available logging category `"all"`
which is not explained.
4. Add `"optional"` to the help text of `<include>` and `<exclude>`.
5. Add missing new lines before `"Argument:"`.
6. `"0"`,`"1"` are allowed in both array of `<include>` and `<exclude>`.
`"0"` is **ignored** and `"1"` is treated **same as** `"all"`.
It is confusing, so forbid them.
7. It always returns all logging categories with status.
Fix the help text to match this behavior.
Tree-SHA512: c2142da1a9bf714af8ebc38ac0d82394e2073fc0bd56f136372e3db7b2af3b6746f8d6b0241fe66c1698c208c124deb076be83f07dec0d0a180ad150593af415
Add leveldb: prefix to leveldb debug logging lines.
leveldb debug messages come in various scary flavors such as:
2017-11-30 08:26:31 leveldb: Recovering log #26
2017-11-30 08:26:31 leveldb: Level-0 table #28: started
2017-11-30 08:26:31 leveldb: Level-0 table #28: 597 bytes OK
2017-11-30 08:26:31 leveldb: Delete type=0 #26
2017-11-30 08:26:31 leveldb: Delete type=3 #24
so it's reasonably important to mark them as coming from leveldb
internals and not from consensus validation wallet or such.
(this only affects `-debug=leveldb` or `-debug=1` otherwise you won't
see them in the first place)
126999d depends: fix zmq build with mingw < 4.0 (Cory Fields)
387879d [depends] ZeroMQ 4.2.2 (fanquake)
Pull request description:
Update depends ZeroMQ to 4.2.2, the release notes are available [here](https://github.com/zeromq/libzmq/releases).
We can drop both patches, as they have both been merged upstream (they actually had been for some time but just hadn't yet made it into a release).
`--without-documentation` is deprecated and has been replaced with `--without-docs`.
`--disable-perf` disables building performance measuring tools, which are enabled by default, see the libzmq [configure.ac](https://github.com/zeromq/libzmq/blob/master/configure.ac#L367).
Updated dependencies.md.
`--disable-curve-keygen` disable building the curve key generation tool. See [here](https://github.com/zeromq/libzmq/blob/master/configure.ac#L405).
Can someone on windows test that this is still working correctly. Maybe @achow101 ?
Tree-SHA512: c6c4b15f545b6de21648f05027b5500fca0e6b5b72e791ac9a0aa523c57f2feb5aae94e42531275dddd922e11e462a52f08be1118ba1629c3cae765b18e5d720
33698c94b Trivial: fix comments for ZeroMQ bitcoind args (aaron-hanson)
Pull request description:
The ZeroMQ command-line args suggested here had the "-zmqpubhashblock" arg duplicated and the "-zmqpubrawblock" arg missing.
Tree-SHA512: ef3a90f6d82367104aa751778074f6f56d2aaa9cbbf980731f60ee77393db256551423f5414c5b41f09707d08828afc663d06661152ef504a690ceb13c80bc0d
e9dfa9bcc [tests] Move version message sending from NodeConn to NodeConnCB (John Newbery)
dad596fc3 [tests] Make NodeConnCB a subclass of NodeConn (John Newbery)
e30d40438 [tests] Move only: move NodeConnCB below NodeConn (John Newbery)
4d5059856 [tests] Tidy up mininode (John Newbery)
f2ae6f32a [tests] Remove mininode periodic (half-hour) ping messages (John Newbery)
ec59523c5 [tests] Remove rpc property from TestNode in p2p-segwit.py. (John Newbery)
Pull request description:
This is the final step in #11518, except for possibly renaming - for motivation, please see that PR.
If this is merged, then migrating the test framework from asyncore to asyncio should be easier (I say should because I haven't dug too deeply into what would be required).
Requesting review from @ryanofsky , since he always has good feedback on these refactor PRs, and I'd appreciate his take on this refactor. Note particularly that I've reverted the change suggested here: https://github.com/bitcoin/bitcoin/pull/11182#discussion_r148859555 . The idea, as always, is to present a simple interface to the test writer.
Tree-SHA512: 94dd467a13ec799b101108cf47d4dccb6f6240b601e375e3d785313333bbb389c26072a50759aca663bbf3d6c8b867b99e36ae8800ab8ea115e0496c151926ce
Instead of calling combine_logs from test_runner when the environment variable
PYTHON_DEBUG is set, add a combinedlogslen argument. This makes it easier
to call when running locally, and allows a different length of combined logs
to be printed.
8b2c733 clarify abortrescan rpc use (Gregory Sanders)
Pull request description:
`-rescan` during startup doesn't apply since RPC has not warmed up by that point
Tree-SHA512: 0b6738ed9429ca41edbfa23bb3e2da23bb16fed9e55c1803ab140b2efc49ef3295bb8b7eeb8c24df94204c5b3fb25d0d57cbc8d1be11856a5763c9fde71bceba
9d811dc Document partial validation in ConnectBlock() (Suhas Daftuar)
Pull request description:
`ConnectBlock()` relies on validation that happens in `ContextualCheckBlock()` and
`ContextualCheckBlockHeader()`. This has implications for implementing consensus
changes and handling software upgrade to ensure that nodes upgrading their
software end up enforcing all the consensus rules.
Tree-SHA512: 36a252af2221b0e5d5d6f8d5f8b16f8b566ca0db2d56242130a5523302c8757599ac234594a6a946c1689b260d18a32c2c7f8c3831304e78b9832e2ce5ac435a
e1a8ec5 Fix: Open files read only if requested (Andras Elso)
Tree-SHA512: 047951bf3fa1272764596d2efed0a541105a6d3057789ba4218cd1bbce8a802cab0feac0ed3d28f6eab6b2e54e294d46f729dd0162d6d209e319d716c0567352
This commit moves the logic that sends a version message
on connection from NodeConn to NodeConnCB. NodeConn should
not be aware of the semantics or meaning of the P2P payloads.
This makes NodeConnCB a subclass of NodeConn, and
removes the need for the client code to know
anything about the implementation details of NodeConnCB.
NodeConn can now be swapped out for any other implementation
of a low-level connection without changing client code.
PR #10286 introduced a few steps which are not robust to early shutdown
in initialization.
Stumbled upon this with #11781, not sure if there are other scenarios
that can trigger it, but it's harden against this in any case.
abbd230 Move RPC registration out of AppInitParameterInteraction (Russell Yanofsky)
Pull request description:
Move to AppInitServers. This doesn't have any effects on bitcoin behavior. It was just strange to have this unrelated code in the middle of parameter interaction.
Tree-SHA512: 373e18f2ef8d21999ad36295d69326128a3086044acfc8ed537abd5497c8d3620b9832f7f6aa87c0c0939bb5e0d92be8a3c006b5997e9e6fa20334f5610c89bc
fae98f6 [Docs] Bump minimum required version of GCC to 4.8 (fanquake)
Pull request description:
Make it explicit that the minimum required version of GCC is 4.8. Discussion in #11732.
Tree-SHA512: 85ade5a4a63ea1cb6b5e20756bc96f25518c394e00a8d3ed5b9c2927b55ea3155645ca74bb1c7e7eda69d7e42d440a42bde839f281e57fc3e20390b2a19885d6
blockchain.cpp has low unit test coverage. This commit is intended
to start improving its code coverage to reasonable levels. One or more
follow up commits will complete the task that this commit is starting
(though the usefulness of this commit is not dependent upon later
commits).
Note that these tests were not written based upon a specification of how
GetDifficulty *should* work, but rather how it actually *does* work. As
a result, if there are any bugs in the current GetDifficulty
implementation, these unit tests serve to lock them in rather than
expose them.
-- Why has blockchain.cpp been modified if this is a unit testing change?
Since the existing GetDifficulty function relies on a global variable,
chainActive, it was not suitable for unit testing purposes. Both the
existing GetDifficulty function and the unit tests now call through to
a new, more modular version of GetDifficulty that can work on any chain,
not just chainActive.
-- Why does blockchain_tests.cpp directly include blockchain.cpp instead
of blockchain.h?
While the new GetDifficulty function's signature is arguably better than
the old one's, it still isn't great, and doesn't seem to warrant inclusion
as part of the blockchain.h API, especially since only test code is
directly using it. If a better way of exposing the new GetDifficulty
function to unit tests exists, please mention it and the commit will be
updated accordingly.
-- Why is the test fixture named blockchain_difficulty_tests rather than
blockchain_tests?
The Bitcoin Core policy for naming unit test files is to match the the
file under test ("blockchain" becomes "blockchain_tests"). While this
commit complies with that, blockchain.cpp is a massive file, such that
having all of the unit tests in one file will tend towards disorder.
Since there will be a lot more tests added to this file, the intention
is to divide up different types of tests into different test fixtures
within the same file.
d9340ce Fix sendrawtransaction hang when sending a tx already in mempool (Matt Corallo)
Pull request description:
I assume this is what #11721 actually hit.
Tree-SHA512: 1da4088bbda64c5527233de9ec4d03f9e0c1eacddb2ed3deab3cb99eac0293ee6fb846830f97b5e10e230307b6d7fd18013043173aa4f27ef171d9da626e2c88
65e91f5ed [tests] Test that mempool rejects coinbase transactions (James O'Beirne)
Pull request description:
Neither the unit nor functional tests appear to cover rejecting a transaction from acceptance to the mempool on the basis of it being a coinbase. Seems like a decent thing to have a test for.
Tree-SHA512: 53af53c975cad5d7a21c443d71a1c0ced5c70a7799b75bb44d9b7dd6ab2afbcdcaab14571540efeb848f3a1daee5e1dd856530d8f2b50582595219a1c17555ff
fe27a7236 [docs] links to code style guides (Sjors Provoost)
Pull request description:
I always have difficulty finding these, probably because they're not actually called "code style guideline".
Tree-SHA512: a2716c5e7e6778ce71b4451091e3899c0f9e04d07e95d1a0bdb07eb0319c083e3d9aa94bc2e33fc8546f82a1441e73c37218fdc7f228ee8273c7a87466958973
ConnectBlock() relies on validation that happens in ContextualCheckBlock() and
ContextualCheckBlockHeader(). This has implications for implementing consensus
changes and handling software upgrade to ensure that nodes upgrading their
software end up enforcing all the consensus rules.
A) The changes in behavior are as follows:
1. Introduce logging category "none" as alias of "0" for
both RPC-logging and bitcoind "-debug" parameter.
2. Same as "0" is given to argument of "-debug",
if "none" or "0" is given to <include>, all other given logging
categories are ignored. The same is true for <exclude>.
(Before this PR, "0" was accepted but just be ignored itself.)
B) The changes in the help text are as follows:
1. Add a descrption about the evaluation order of <include> and
<exclude> to clarify how debug loggig categories to be set.
2. Delete text that describe restriction about libevent because
it's already allowed libevent logging to be updated during runtime.
3. Add a description for category "all", "1", "none" and "0".
4. Add "optional" to the help text of <include> and <exclude>.
5. Add missing new lines before "Argument:".
6. This RPC always returns all logging categories with status.
Fix the help text to match this behavior.
Wladimir J. van der Laan
