9bdde3c802e1512cbcd8ae8f7db72b6fe13ea829 build: add /usr/local/ to LCOV_FILTER_PATTERN for macOS builds (eugene)
Pull request description:
With this commit, the files in /usr/local/ will not be included in
`make cov` or `make cov_fuzz` coverage reports. This behavior could
be observed when generating the reports on macOS with brew-installed
clang.
ACKs for top commit:
fanquake:
ACK 9bdde3c802e1512cbcd8ae8f7db72b6fe13ea829
Tree-SHA512: 15cbe8d514651448f3d7b8b0a00939fd6bd6f15e6812f1959fcaaab7364ca2ef4ee34f2ff8950b7fdc8ae64d043dc5f7185c0601dd94780b41331337e5e84c45
4148f55dd016f940df50a44cf03d117cdb1dd929 docs: Correct description for getblockstats's txs field (Nadav Ivgi)
Pull request description:
It does count the coinbase transaction.
Refs #19766
ACKs for top commit:
MarcoFalke:
ACK 4148f55dd016f940df50a44cf03d117cdb1dd929
theStack:
ACK 4148f55dd016f940df50a44cf03d117cdb1dd929
Tree-SHA512: ccd420f19242efbbbecfe822c825363bc89e26618834de0d805f5cdb07461c8bdc6e077c61ea8cd0d40564a96c67d8a71c68175c8543bb849909d7ae375b2a92
fa55c1d5fdd88c4bc4d361da231cd63b20255b50 build: Add Werror=range-loop-analysis (MarcoFalke)
Pull request description:
The warning is implicitly enabled for Bitcoin Core. Also explicitly since commit d92204c900d.
To avoid "fix range loop" follow-up refactors, we have two options:
* Disable the warning, so that issues never appear
* Enable it as an error, so that issues are either caught locally or by ci
ACKs for top commit:
fanquake:
ACK fa55c1d5fdd88c4bc4d361da231cd63b20255b50
practicalswift:
ACK fa55c1d5fdd88c4bc4d361da231cd63b20255b50 -- pre-review fix-up is better than post-review fix-up
hebasto:
re-ACK fa55c1d5fdd88c4bc4d361da231cd63b20255b50
Tree-SHA512: 019aa133f254af8882c1d5d10c420d9882305db0fc2aa9dad7d285168e2556306c3eedcc03bd30e63f11eae4cc82b648d83fb6e9179d6a6364651fb602d70134
35cd2da623e32b975fbc485c3605934e4aa8bdc5 test: Fix 'make cov' with clang (Hennadii Stepanov)
Pull request description:
This is a follow up of #19688.
With this PR it is possible to do the following:
```
$ ./autogen.sh
$ ./configure --enable-lcov CC=clang CXX=clang++
$ make
$ make cov
```
Currently, on master (8a85377cd0b60cb00dae4f595d628d1afbd28bd5), `make cov` fails to `Processing src/test/test_bitcoin-util_tests.gcda`.
ACKs for top commit:
vasild:
ACK 35cd2da
Crypt-iQ:
ACK 35cd2da
Tree-SHA512: aaf56118e2644064e9738a8279889c617db5805c5c804c904469b24c496bd609f9c5fc2aebcf1a422f8a5ed2eb38bd6e76b484680310b55c36d922b73a4c33cf
a51d0ad2de89b9757d158df95ddeba2bfcb23935 rpc: Improve addnode remove command error message (Fabian Jahr)
Pull request description:
The `addnode` RPC with the `remove` command parameter is used to remove a node from the "added nodes". It did not have test coverage and in case of failure to remove the node it responded with the confusing message "Error: Node has not been added.".
This PR adds test coverage and introduces a new error code as well as changes the error message to something that makes sense.
ACKs for top commit:
laanwj:
Code review ACK a51d0ad2de89b9757d158df95ddeba2bfcb23935
theStack:
Tested ACK https://github.com/bitcoin/bitcoin/commit/a51d0ad2de
Tree-SHA512: 033ef5de0d4d49d58ef4df3759b838c9d19ee9dfb0aff9f814a3a63d124ca231a442c930efa7d343fe1f65727c4b59fc23dd5e26fe6ea69f9e84fda48b5c5cc2
99993489da9bc003b823bcab10e5f5297b369431 test: Set -logthreadnames in unit tests (MarcoFalke)
fa4ea997b4da1ae0afafba223fff9efbeefaf555 init: Setup scheduler in tests and init in exactly the same way (MarcoFalke)
Pull request description:
Generally the unit tests are single threaded, with the exception of the script check threads, the schedule, and optionally indexer threads.
Like the functional tests, the thread name can serve additional debug information, so set `-logthreadnames` in unit tests.
Can be tested with
```
./src/test/test_bitcoin -l test_suite -t validation_tests/test_combiner_all -- DEBUG_LOG_OUT
ACKs for top commit:
laanwj:
ACK 99993489da9bc003b823bcab10e5f5297b369431
Tree-SHA512: 3bdbfc211da146da64b50b0826246aff5c611a84b69ab896a55b3c9d1adc92c5975da36ab92aee577df82e229c4326b477f4105bfdd1a5df4c9a0b018cf61602
498cdbb42616bc1e6c44492c907678b4dce41870 Fix improper Doxygen inline comments (Ben Woosley)
Pull request description:
The proper syntax is `//!<`
http://www.doxygen.nl/manual/docblocks.html#memberdoc
Identified via `-Wdocumentation`:
```
In file included from ./util/system.h:26:
./util/settings.h:74:41: error: not a Doxygen trailing comment [-Werror,-Wdocumentation]
const SettingsValue* begin() const; //<! Pointer to first non-negated value.
^~~~
///<
./util/settings.h:75:41: error: not a Doxygen trailing comment [-Werror,-Wdocumentation]
const SettingsValue* end() const; //<! Pointer to end of values.
^~~~
///<
./util/settings.h:76:41: error: not a Doxygen trailing comment [-Werror,-Wdocumentation]
bool empty() const; //<! True if there are any non-negated values.
^~~~
///<
./util/settings.h:77:41: error: not a Doxygen trailing comment [-Werror,-Wdocumentation]
bool last_negated() const; //<! True if the last value is negated.
^~~~
///<
./util/settings.h:78:41: error: not a Doxygen trailing comment [-Werror,-Wdocumentation]
size_t negated() const; //<! Number of negated values.
^~~~
///<
```
ACKs for top commit:
fanquake:
ACK 498cdbb42616bc1e6c44492c907678b4dce41870
Tree-SHA512: 2851fc1cbbcf700d198d82ce4923b2ef4a700f8ce19dff431ecf24f4e6fecda9fed1b4b4d148f3c1adfb6b0c6bff5d5315ee01bbcd855eb3d83e1a69b0c98893
This was previously hard-coded in Sentinel, and can be useful to refactor
future versions of Sentinel (in progress), as well as to bring superblock logic
into DashCore in the future.
* feat(qt): Let users resend stuck txes one by one via context menu
The corresponding rpc was removed due to privacy concerns (resending many txes at once can hurt privacy). On the other hand by the time users try to resend txes by hand they would be locked via IS already (normally). We don't resend IS-locked txes which means we can be pretty sure that (in most cases) no IS lock means no such txes were ever broadcasted for some reason. With this in mind resending txes one by one is probably ok-ish since it would be similar to regular tx creation/sending.
* Only allow tx resend for single row selections
* format
This commit is from 145fe95ec7 but the commit wasn't titled as such,
this to prevent confusion due to a potentially misleading commit title as the change doesn't match the pull request title's description per-se.
fa07b8beb598642655b1207afd275b801ff8cec2 test: Reset global args between test suites (MarcoFalke)
Pull request description:
Ideally there wouldn't be any globals in Bitcoin Core. However, as we still have globals, they need to be reset between runs of test cases. One way to do this is to run each suite in a different process. `make check` does that. However, `./src/test/test_bitcoin` when run manually or on appveyor is a single process, where all globals are preserved between test cases.
This leads to hard to debug issues such as https://github.com/bitcoin/bitcoin/pull/15845#pullrequestreview-310852164.
Fix that by resetting the global arg for each test suite. Note that this wont reset the arg between test cases, as the constructor/destructor is not called for them.
Addendum: This is not a general fix, only for `-segwitheight`. I don't know if clearing all args can be done with today's argsmanager. Nor do I know if it makes sense. Maybe we want datadir set to a temp path to not risk accidentally corrupting the default data dir?
ACKs for top commit:
laanwj:
ACK fa07b8beb598642655b1207afd275b801ff8cec2
practicalswift:
ACK fa07b8beb598642655b1207afd275b801ff8cec2
mzumsande:
ACK fa07b8beb598642655b1207afd275b801ff8cec2, I also tested that this fixes the issue in #15845.
Tree-SHA512: 1e30b06f0d2829144a61cc1bc9bdd6a694cbd911afff83dd3ad2a3f15b577fd30acdf9f1469f8cb724d0642ad5d297364fd5a8a2a9c8619a7a71fa9ae2837cdc
fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad bench: Remove redundant copy constructor in mempool_stress (MarcoFalke)
29f84343681831baf02a17d3af566c5c57ecf3c2 refactor: Remove redundant PSBT copy constructor (Hennadii Stepanov)
Pull request description:
I fail to see why people add these copy constructors manually without explanation, when the compiler can generate them at least as good automatically with less code.
ACKs for top commit:
promag:
ACK fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad.
hebasto:
ACK fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad, nit s/constructor/operator/ in commit fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad message, as @promag [mentioned](https://github.com/bitcoin/bitcoin/pull/17349#discussion_r341776389) above.
jonatack:
ACK fa8919889f3c1bd3e2700ecbb56493e3cd1e25ad
Tree-SHA512: ce024fdb894328f41037420b881169b8b1b48c87fbae5f432edf371a35c82e77e21468ef97cda6f54d34f1cf9bb010235d62904bb0669793457ed1c3b2a89723
facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 travis: Run unit and functional tests on native arm (MarcoFalke)
fafa064d2a8dbe24303545ab582ec84cde52ab5b ci: Remove ccache requirement on the host (MarcoFalke)
Pull request description:
This keeps the cross-compilation to make it easy to run the ci on non-arm hardware. To run this locally in qemu-user as it used to be, just `export QEMU_USER_CMD="qemu-arm -L /usr/arm-linux-gnueabihf/"`.
ACKs for top commit:
laanwj:
LGTM ACK facc0da63a8fa4bd6fc2782cbe92eb9f920f2256
practicalswift:
ACK facc0da63a8fa4bd6fc2782cbe92eb9f920f2256 -- diff looks correct and Travis seems happy
Tree-SHA512: 0dc1bc82eb93e2bd8b159e044f20fe3055f8cdfd73aaa238bd2e178397582144dfc0c6a87bd8270115dafea1a623e642bde5d5f30254f94140f1a2cdb12fc2da
ff22751417c6fbbd22f4eefd0e23431a83335c13 test: rm ascii art in rpc_fundrawtransaction (Jon Atack)
94fcc08541cf58bee864ab7c28a6c77e42472f17 test: add rpc_fundrawtransaction logging (Jon Atack)
Pull request description:
`test/functional/rpc_fundrawtransaction.py` is fairly slow to run and has no logging, so it can appear to be stalled.
This commit adds info logging at each test to provide feedback on the test run.
ACKs for top commit:
instagibbs:
utACK ff22751417
jnewbery:
tACK ff22751417c6fbbd22f4eefd0e23431a83335c13
Tree-SHA512: f4fabad8ef51c29981351bb4e66fb0c0e0517418a4a15892ef804df11d16b2d2ae1a1abc958d2b121819850278de90a2003b0edb8d7098d00360b89fa76e9062
e7ad4a2f8c07a82d6424b473f0d51dbd8f897b10 doc: rename wallet-tool references to bitcoin-wallet (Wilson Ccasihue S)
Pull request description:
Fix. text reference to executable bitcoin-wallet instead of wallet-tool, there is not a wallet-tool at bin/ folder.
ACKs for top commit:
fanquake:
ACK e7ad4a2f8c07a82d6424b473f0d51dbd8f897b10 - thanks for following up.
Tree-SHA512: aed41b08947728a4ff3a97a62858ee7c86e2e5d57dcbbd0aab492dae3d8a548bb60541924e68cf3a0aa3d53d7db0012b489462b466919cd83f05b2aa88b7fff7
a652dc5521e2caf5734ffb797c7f2fc80685fef1 qt: Normalize placeholder to avoid using "address book" in sendcoinsentry (Wladimir J. van der Laan)
67f36e0b2ce0f99b90578e7e1dd9e0624026bcfa gui: Move static placeholder texts to forms (Wladimir J. van der Laan)
Pull request description:
There was an issue around the time of Qt 4.6 when placeholder text was introduced, that caused a compile failure when it was specified in the form.
As a workaround the placeholder texts were moved to the code.
Qt 4 hasn't been relevant to us for ages. So move all (non-parametrized) placeholder texts to the form files instead.
It's better to keep this kind of text content together. Translate/no-translate status is kept as it is.
Proof that they still work:
ACKs for top commit:
hebasto:
Re-ACK a652dc5521e2caf5734ffb797c7f2fc80685fef1, `tooltip` and `placeholderText` are identical now.
MarcoFalke:
ACK a652dc5521e2caf5734ffb797c7f2fc80685fef1 🚿
fanquake:
ACK a652dc5521e2caf5734ffb797c7f2fc80685fef1 - checked that placeholder text still appears.
Tree-SHA512: 7d3c1faeef2eb5d4b195d9d78f2a3f161296d869e5059b5e8d308167e3c6c668a3ebabec93dc592762ba15bfc86d51985e20c4e17f1065c8dce84fec036ff5ee
48a5c92f9ef6634375a3f52812cf3d511c37699d ui: disable 3rd-party tx-urls when wallet disabled (Harris)
Pull request description:
This PR closes#17683 by removing 3rd-party Url-Label and -TextBox from Display Options in wallet-disabled mode.
ACKs for top commit:
laanwj:
Code review ACK 48a5c92f9ef6634375a3f52812cf3d511c37699d
fanquake:
ACK 48a5c92f9ef6634375a3f52812cf3d511c37699d - tested with and without wallet (compiled out and `-disablewallet`).
Tree-SHA512: 3cc89825409fc0a3eec501c4dab5ff1caaa4ce410746a4b6ab200222fff986f4483eab90cda53a98a144be6acf1b6ca8650ab18242c39446f3335b3a9a537066
f736f6920b160c9e7d7072500ddd0459c5181f86 lcov: filter /usr/lib64 from coverage report (nijynot)
a5a705b46dd32f93857e916311e3b71cae8be6b7 lcov: filter depends from coverage report (nijynot)
Pull request description:
If you build the binaries with the `depends` folder and then generate coverage reports with `make cov`, `depends` will be included in the coverage reports. Coverage of the dependencies are not that interesting and should be filtered.
ACKs for top commit:
laanwj:
code review ACK f736f6920b160c9e7d7072500ddd0459c5181f86
MarcoFalke:
ACK f736f6920b160c9e7d7072500ddd0459c5181f86 🐇
Tree-SHA512: 57c3e09f32e71523afff6ddc4f92bc35ab7b783f26f7a7380ae7556222954111cccce4c6dbc99305c424818f91e15bf5fe3532a7dca1daaa8ad71315d1dd857c
0ccad08fb25e00fcf41ffbad6ce2501e363a0033 Make env data logging optional (Pieter Wuille)
Pull request description:
The dynamic env feeding logging is a bit chatty, make it dependent on `-debug=rand`.
ACKs for top commit:
practicalswift:
ACK 0ccad08fb25e00fcf41ffbad6ce2501e363a0033 -- less noise is good and diff looks correct
laanwj:
ACK 0ccad08fb25e00fcf41ffbad6ce2501e363a0033
promag:
ACK 0ccad08fb25e00fcf41ffbad6ce2501e363a0033.
jonatack:
ACK 0ccad08fb25e00fcf41ffbad6ce2501e363a0033, was considering to propose this.
Tree-SHA512: 01d7f9ac134852c2c0d5f66f96ee4395f0ff7a60573e648f3d01054073624042148c8e8b9f69a29c9a41c296e1f4be77c2015a642ee4113a2fd8779b62aa137d
