77a733a99 [tests] Add additional unit tests for -nofoo edge cases (Anthony Towns)
af173c2be [tests] Check GetChainName works with config entries (Anthony Towns)
fa27f1c23 [tests] Add unit tests for ReadConfigStream (Anthony Towns)
087c5d204 ReadConfigStream: assume the stream is good (Anthony Towns)
6d5815aad Separate out ReadConfigStream from ReadConfigFile (Anthony Towns)
834d30341 [tests] Add unit tests for GetChainName (Anthony Towns)
11b6b5b86 Move ChainNameFromCommandLine into ArgsManager and rename to GetChainName (Anthony Towns)
Pull request description:
This does a bit of refactoring of the configuration handling code in order to add additional tests to make adding support for [test]/[regtest] sections in the config file in #11862 easier. Should not cause any behaviour changes.
Tree-SHA512: 8d2ce1449fc180de03414e7e569d1a21ba1e9f6564e13d3faf3961f710adc725fa0d4ab49b89ebd2baa11ea36ac5018377f693a84037d386a8b8697c9d6db3e9
0e7c52d Shut down if trying to connect a corrupted block (Suhas Daftuar)
Pull request description:
(Updated OP after reworking the approach)
Shut down if a corrupted block is found in ConnectBlock(). This prevents an infinite loop trying to connect such a block, and alerts the node operator that there may be potential hardware failure.
Tree-SHA512: f20d56aa9d36d6eeff4c3d13c0fbd14f06a57701bd13c2416d36f0cc4235f81f752139e336a073617e8e803782c5096c960108af122b19a51227de512e9095ee
d207207 [logging] add lint-logs.sh to check for newline termination. (John Newbery)
5c21e6c [logging] Comment all continuing logs. (John Newbery)
Pull request description:
Check that all calls to LogPrintf() are terminated by a newline,
except those that are explicitly marked as 'continued' logs.
Tree-SHA512: fe5162b2b2df1e8a4c807da87584fa9af97a6b8377e4090fe0caa136d90bf29a487a123cde94569bdce7101fee3478196d99aa13f1212e24bfe5f41c773604fc
d1b622b tests: Add check for test suite name uniqueness in lint-tests.sh (practicalswift)
dc8067b tests: Add note about uniqueness requirement for test suite names (practicalswift)
3ebfb2d tests: Avoid test suite name collision in wallet crypto_tests (MarcoFalke)
Pull request description:
* Add documentation: Add note about test suite name uniqueness requirement in developer notes
* Add regression test: Update `lint-tests.sh` to make it check also for test suite name uniqueness
Context: #12894 (`tests: Avoid test suite name collision in wallet crypto_tests`)
Tree-SHA512: 3c8502db069ef3d753f534976a86a997b12bac539e808a7285193bf81c9dd8c1b06821c3dd1bdf870ab87722b02c8aa9574c62ace70c2a1b8091785cb8c9aace
d54874d Set SCHED_BATCH priority on the loadblk thread. (Evan Klitzke)
Pull request description:
Today I came across #10271, and while reading the discussion #6358 was linked to. Linux systems have a `SCHED_BATCH` scheduler priority that is useful for threads like loadblk. You can find the full details at [sched(7)](http://man7.org/linux/man-pages/man7/sched.7.html), but I'll quote the relevant part of the man page below:
> ...this policy will cause the scheduler to always assume that the thread is
CPU-intensive. Consequently, the scheduler will apply a small scheduling penalty
with respect to wakeup behavior, so that this thread is mildly disfavored in
scheduling decisions.
>
> This policy is useful for workloads that are noninteractive, but do not want to
lower their nice value, and for workloads that want a deterministic scheduling
policy without interactivity causing extra preemptions (between the workload's
tasks).
I think this change is useful independently of #10271 and irrespective of whether that change is merged. Under normal operation the loadblk thread will just import `mempool.dat`. However, if Bitcoin is started with `-reindex` or `-reindex-chainstate` this thread will use a great deal of CPU while it rebuilds the chainstate database (and the block database in the case of `-reindex`). By setting `SCHED_BATCH` this thread is less likely to interfere with interactive tasks (e.g. the user's web browser, text editor, etc.).
I'm leaving the nice value unchanged (which also affects scheduling decisions) because I think that's better set by the user. Likewise I'm not using [ioprio_set(2)](http://man7.org/linux/man-pages/man2/ioprio_set.2.html) because it can cause the thread to become completely I/O starved (and knowledgeable users can use `ionice(1)` anyway).
Tree-SHA512: ea8f7d3921ed5708948809da771345cdc33efd7ba3323e9dfec07a25bc21e8612e2676f9c178e2710c7bc437e8c9cafc5e0463613688fea5699b6e8e2fec6cff
fafcad3 doc: Add testmempoolaccept to release-notes (MarcoFalke)
Pull request description:
Some fixups for #11742:
* Add release notes for the new rpc
* Fix a typo in the original pull
* Make the mempool reference passed to `CheckInputsFromMempoolAndCache` const, since that function is called before we return from ATMP and we must not modify the mempool.
Tree-SHA512: 72c459ba69f7698a69c91d2592f10f7fb1864846c7d8c525050d48286f92ba5ec5fe554c54235b52fbd9a8f00226c526ad84584641ec39084e1a1310a261510d
Most logs should terminated with a '\n'. Some logs
are built up over multiple calls to logPrintf(), so
do not need a newline terminater. Comment all of
these 'continued' logs as a linter hing.
f63bc5e wallet: Initialize m_last_block_processed to nullptr. Initialize fields where defined. (practicalswift)
Pull request description:
Initialize `m_last_block_processed` to `nullptr`.
`m_last_block_processed` was introduced in 5ee3172636.
Tree-SHA512: 6e4a807e5b02115cbd80460761056f2eb22043203212d88dd0cd44c28dc0abce30ab29b078ca2c612232e76af4886f4fdbf2b0ff75e2df19b4d1a801b236cc13
Based on suggestion by Pieter Wuille <pieter.wuille@gmail.com> at
https://github.com/bitcoin/bitcoin/pull/12119#issuecomment-357982763
After #12119, the NONE output type was overloaded to refer to either an output
type that couldn't be parsed, or to an automatic change output mode. This
change drops the NONE enum and uses a simple bool indicate parse failure, and a
new CHANGE_AUTO enum to refer the change output type.
This change is almost a pure refactoring except it makes RPCs reject empty
string ("") address types instead of treating them like they were unset. This
simplifies the parsing code a little bit and could prevent RPC usage mistakes.
It's noted in the release notes.
9960137 Add developer notes about blocking GUI code (Russell Yanofsky)
9a61eed Use WalletBalances struct in Qt (Russell Yanofsky)
56f33ca Remove direct bitcoin calls from qt/sendcoinsdialog.cpp (Russell Yanofsky)
e872c93 Remove direct bitcoin access from qt/guiutil.cpp (Russell Yanofsky)
5884558 Remove direct bitcoin calls from qt transaction table files (Russell Yanofsky)
3cab2ce Remove direct bitcoin calls from qt/paymentserver.cpp (Russell Yanofsky)
3ec2ebc Remove direct bitcoin calls from qt/addresstablemodel.cpp (Russell Yanofsky)
827de03 Remove direct bitcoin calls from qt/coincontroldialog.cpp (Russell Yanofsky)
a0704a8 Remove most direct bitcoin calls from qt/walletmodel.cpp (Russell Yanofsky)
90d4640 Remove direct bitcoin calls from qt/optionsdialog.cpp (Russell Yanofsky)
582daf6 Remove direct bitcoin calls from qt/rpcconsole.cpp (Russell Yanofsky)
3034a46 Remove direct bitcoin calls from qt/bantablemodel.cpp (Russell Yanofsky)
e0b66a3 Remove direct bitcoin calls from qt/peertablemodel.cpp (Russell Yanofsky)
d7c2c95 Remove direct bitcoin calls from qt/intro.cpp (Russell Yanofsky)
fe6f27e Remove direct bitcoin calls from qt/clientmodel.cpp (Russell Yanofsky)
5fba3af Remove direct bitcoin calls from qt/splashscreen.cpp (Russell Yanofsky)
c2f672f Remove direct bitcoin calls from qt/utilitydialog.cpp (Russell Yanofsky)
3d619e9 Remove direct bitcoin calls from qt/bitcoingui.cpp (Russell Yanofsky)
c0f2756 Remove direct bitcoin calls from qt/optionsmodel.cpp (Russell Yanofsky)
71e0d90 Remove direct bitcoin calls from qt/bitcoin.cpp (Russell Yanofsky)
ea73b84 Add src/interface/README.md (Russell Yanofsky)
Pull request description:
This is a refactoring PR that does not change behavior in any way. This change:
1. Creates abstract [`Node`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-local/src/interface/node.h) and [`Wallet`](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-local/src/interface/wallet.h) interfaces in [`src/interface/`](https://github.com/ryanofsky/bitcoin/tree/pr/ipc-local/src/interface)
1. Updates Qt code to call the new interfaces. This largely consists of diffs of the form:
```diff
- InitLogging();
- InitParameterInteraction();
+ node.initLogging();
+ node.initParameterInteraction();
```
This change allows followup PR #10102 (makes `bitcoin-qt` control `bitcoind` over an IPC socket) to work without any significant updates to Qt code. Additionally:
* It provides a single place to describe the interface between GUI and daemon code.
* It can make better GUI testing possible, because Node and Wallet objects have virtual methods that can be overloaded for mocking.
* It can be used to help make the GUI more responsive (see https://github.com/bitcoin/bitcoin/issues/10504)
Other notes:
* I used python scripts [hide-globals.py](https://github.com/ryanofsky/home/blob/master/src/2017/hide-globals/hide-globals.py) and [replace-syms.py](https://github.com/ryanofsky/home/blob/master/src/2017/hide-globals/replace-syms.py) to identify all the places where Qt code was accessing libbitcoin global variables and calling functions accessing those global variables.
* These changes were originally part of #10102. Thanks to @JeremyRubin for the suggestion of splitting them out.
Commits:
- [`ea73b84d2d` Add src/interface/README.md](ea73b84d2d)
- [`71e0d90876` Remove direct bitcoin calls from qt/bitcoin.cpp](71e0d90876)
- [`c0f2756be5` Remove direct bitcoin calls from qt/optionsmodel.cpp](c0f2756be5)
- [`3d619e9d36` Remove direct bitcoin calls from qt/bitcoingui.cpp](3d619e9d36)
- [`c2f672fb19` Remove direct bitcoin calls from qt/utilitydialog.cpp](c2f672fb19)
- [`5fba3af21e` Remove direct bitcoin calls from qt/splashscreen.cpp](5fba3af21e)
- [`fe6f27e6ea` Remove direct bitcoin calls from qt/clientmodel.cpp](fe6f27e6ea)
- [`d7c2c95948` Remove direct bitcoin calls from qt/intro.cpp](d7c2c95948)
- [`e0b66a3b7c` Remove direct bitcoin calls from qt/peertablemodel.cpp](e0b66a3b7c)
- [`3034a462a5` Remove direct bitcoin calls from qt/bantablemodel.cpp](3034a462a5)
- [`582daf6d22` Remove direct bitcoin calls from qt/rpcconsole.cpp](582daf6d22)
- [`90d4640b7e` Remove direct bitcoin calls from qt/optionsdialog.cpp](90d4640b7e)
- [`a0704a8996` Remove most direct bitcoin calls from qt/walletmodel.cpp](a0704a8996)
- [`827de038ab` Remove direct bitcoin calls from qt/coincontroldialog.cpp](827de038ab)
- [`3ec2ebcd9b` Remove direct bitcoin calls from qt/addresstablemodel.cpp](3ec2ebcd9b)
- [`3cab2ce5f9` Remove direct bitcoin calls from qt/paymentserver.cpp](3cab2ce5f9)
- [`58845587e1` Remove direct bitcoin calls from qt transaction table files](58845587e1)
- [`e872c93ee8` Remove direct bitcoin access from qt/guiutil.cpp](e872c93ee8)
- [`56f33ca349` Remove direct bitcoin calls from qt/sendcoinsdialog.cpp](56f33ca349)
- [`9a61eed1fc` Use WalletBalances struct in Qt](9a61eed1fc)
- [`9960137697` Add developer notes about blocking GUI code](9960137697)
Tree-SHA512: 7b9eff2f37d4ea21972d7cc6a3dbe144248595d6c330524396d867f3cd2841d666cdc040fd3605af559dab51b075812402f61d628d16cf13719335c1d8bf8ed3
a5bca13 Bugfix: Include <memory> for std::unique_ptr (Luke Dashjr)
Pull request description:
Not sure why all these includes were missing, but it's breaking builds for some users:
https://bugs.gentoo.org/show_bug.cgi?id=652142
(Added to all files with a reference to `std::unique_ptr`)
Tree-SHA512: 8a2c67513ca07b9bb52c34e8a20b15e56f8af2530310d9ee9b0a69694dd05e02e7a3683f14101a2685d457672b56addec591a0bb83900a0eb8e2a43d43200509
5b10ab0 [trivial] Add newlines to end of log messages. (John Newbery)
Pull request description:
Log messages should terminate with a '\n', or the following log will be
written to the same line without a timestamp. Fix a couple of cases
where the message is not terminated with a \n.
Tree-SHA512: 88677afe85c88ce9f58312430e8881916bd76bbc8cd353ff81c97b3de8356680503160992c0ef3ea192b4694e848e9ca2480dbc38fea1776903b3784497f1af6
1e747e3c1e Make segwit failure due to CLEANSTACK violation return a SCRIPT_ERR_CLEANSTACK error code. (Mark Friedenbach)
Pull request description:
If a segwit script terminates with a stack size not equal to one, the current error code is EVAL_FALSE. This is semantically wrong, and prevents explicitly checking CLEANSTACK violations in the unit tests. This PR changes the error code (and affected unit tests) to use SCRIPT_ERROR_CLEANSTACK instead of SCRIPT_ERROR_EVAL_FALSE.
Tree-SHA512: 8f7b1650f7a23a942cde1070e3e56420be456b4a7be42515b237e95557bf2bd5e7ba9aabd213c8092bea28c165dbe73f5a3486300089aeb01e698151b42484b1
f8c249ab91 Assert CPubKey::ValidLength to the pubkey's header-relevent size (Ben Woosley)
Pull request description:
A pubkey's length is specific to its type which is indicated by its header value. GetLen returns the header-indicated length, so this change ensures that a key matches its header-indicated length.
And replace some magic values with their constant equivalents.
Tree-SHA512: b727b39a631babe0932326396fc4d796ade8ec1e37454ff0c709ae9b78ecbd0cfdf59d84089ba8415e6efa7bc180e3cd39a14ddaf0871cbac54b96851e1b7b44
4e05687153 [wallet] [rpc] [doc] importprivkey: hint about importmulti (Karl-Johan Alm)
Pull request description:
From #12701, a hint about `importmulti` inside the help for `importprivkey` seems useful.
Tree-SHA512: 09ddfd384062b4365f678167076cb9f5af1eb8f083714a20c2a9bb14fef1c886d1666196272bf09862537166d15ae89c3330cdc6836eee76cb54d137e53301df
