01971da9bd docs: Add productivity notes for "dummy rebases" (Carl Dong)
Pull request description:
When rebasing, we often want to do a "dummy rebase" whereby we are not rebasing over an updated master. This is because rebases can be confusing enough already, and we don't want to resolve upstream conflicts together with our local rebase conflicts due to fixup commits, commit rearrangements, and such. This productivity section details how to do such "dummy rebase"s.
ACKs for commit 01971d:
Tree-SHA512: 241a451cec01dc9a01a2286bdee1441cac6d28007f5b173345744d2abf436da916c3f2553ff0d1c5b3687055107b37872dda9529288645e4bae7b3cb46923b7e
f3b90f2e05 Run all lint scripts (Julian Fleischer)
Pull request description:
The description reads:
```
# This script runs all contrib/devtools/lint-*.sh files, and fails if any exit
# with a non-zero status code.
```
This runs all scripts and returns with a non-zero exit code if any failed.
ACKs for commit f3b90f:
Tree-SHA512: 4f1f6435855dd5074a38c5887be6f096ec66f4dbe8712bdfd5fed0c510f1b2c083b7318cf3bfbdcc85982429fb7b4309e57ce48cc11736c86376658ec7ffea8f
c01c065b9ded3399a6a480f15543827dd5e8eb4d Do not construct out-of-bound pointers in SHA512/SHA1/RIPEMD160 code (Pieter Wuille)
Pull request description:
This looks like an issue in the current SHA256/512 code, where a pointer outside of the area pointed to may be constructed (this is UB in theory, though in practice every supported platform treats pointers as integers).
I discovered this while investigating #14580. Sadly, it does not fix it.
ACKs for commit c01c06:
practicalswift:
utACK c01c065b9ded3399a6a480f15543827dd5e8eb4d
Tree-SHA512: 47660e00f164f38c36a1ab46e52dd91cd33cfda6a6048d67541c2f8e73c050d4d9d81b5c149bfad281212d52f204f57bebf5b19879dc7a6a5f48aa823fbc2c02
ccc27bdcd2 doc: Clarify -blocksdir usage (Daniel McNally)
Pull request description:
This PR attempts to clarify and correct the `-blocksdir` argument description and default value. `-blocksdir` does not refer to the full path to the actual `blocks` directory, but rather the root/parent directory which contains the `blocks` directory. Accordingly, the default value is `<datadir>` and not `<datadir>/blocks` - this behavior of defaulting to the datadir can also be seen in init.cpp:
```cpp
if (gArgs.IsArgSet("-blocksdir")) {
path = fs::system_complete(gArgs.GetArg("-blocksdir", ""));
if (!fs::is_directory(path)) {
path = "";
return path;
}
} else {
path = GetDataDir(false);
}
```
It also attempts to clarify that only the `.dat` files containing block data are impacted by `-blocksdir`, not the index files.
I believe this would close#12828.
ACKs for commit ccc27b:
hebasto:
utACK ccc27bdcd2d91fe2c023ad004019d5b970f21dbf
Tree-SHA512: 7b65f66b0579fd56e8c8cd4f9f22d6af56181817762a68deccd7fca51820ad82d9a0c48f5f1f012e746c67bcdae7af4555fad867cb620a9ca538d465c9d86c2b
d2eee87928 Lift prevector default vals to the member declaration (Ben Woosley)
Pull request description:
I overlooked this possibility in #14028
ACKs for commit d2eee8:
promag:
utACK d2eee87, change looks good because members are always initialized.
251Labs:
utACK d2eee87 nice one.
ken2812221:
utACK d2eee87928781ab3082d4279aa6f19641a45e801
practicalswift:
utACK d2eee87928781ab3082d4279aa6f19641a45e801
scravy:
utACK d2eee87928781ab3082d4279aa6f19641a45e801
Tree-SHA512: f2726bae1cf892fd680cf8571027bcdc2e42ba567eaa901fb5fb5423b4d11b29e745e0163d82cb513d8c81399cc85933a16ed66d4a30829382d4721ffc41dc97
5d35ae3326 Handle the result of posix_fallocate system call (Luca Venturini)
Pull request description:
The system call `posix_fallocate` is not supported on some filesystems.
- catches the result of posix_allocate and fall back to the default behaviour if the return value is different from 0 (success)
Fixes#15624
ACKs for commit 5d35ae:
MarcoFalke:
utACK 5d35ae3326624da3fe5dcb4047c9a7cec6665cab
sipa:
utACK 5d35ae3326624da3fe5dcb4047c9a7cec6665cab, though the Yoda condition is an uncommon style in this project.
hebasto:
utACK 5d35ae3326624da3fe5dcb4047c9a7cec6665cab
practicalswift:
utACK 5d35ae3326624da3fe5dcb4047c9a7cec6665cab
Tree-SHA512: 7ab3b35fb633926f28a58b2b07ffde8e31bb997c80a716b1b45ee716fe9ff4ddcef0a05810bd4423530e220cfc62f8925517d27a8b92b05a524272063e43f746
201393f932 Align code example with clang-format (Hennadii Stepanov)
Pull request description:
With this PR running [clang-format-diff.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/clang-format-diff.py) on the code example will not fire a format adjustment.
ACKs for commit 201393:
MarcoFalke:
trivial ACK 201393f93268f6775d1b5d54119a7210628b333d
Tree-SHA512: 825c5e8cfba1bc140c2dfc38b82c5eec268b82b528af4301f25dfacc1f4f0788e268e72e8512f7ce001be665a8b07964a0af832fd9a1c6bd1c27d252f93619bc
6d44c5ebf9 depends: Add commands for each package for each stage (Carl Dong)
80f0e05b70 depends: Preprocessing doesn't care about deps (Carl Dong)
Pull request description:
Adds make targets for each package for each stage, e.g.
```sh
make zeromq_configured
```
ACKs for commit 6d44c5:
MarcoFalke:
ACK 6d44c5ebf97af4b357079fe4bc2130f98e1d0fd2 (Haven't looked at the code changes, but adding this feature makes sense)
ryanofsky:
ACK 6d44c5ebf97af4b357079fe4bc2130f98e1d0fd2
Tree-SHA512: f1ac0aecfd2372aed09ca63603e2634552cb3f6ff9d610f958e2a66952d7d9e870b4c32b7d996886879e6d3016532272e8b1a10c13ed7b31009c6c96f786db9f
1111f0718a test: .style.yapf: Set column_limit=160 (MarcoFalke)
Pull request description:
The current style is pep8, as suggested in https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines.
generated with
```
$ yapf --version
yapf 0.24.0
$ yapf --style-help --style=pep8 > .style.yapf
```
However, we don't use the column_limit of 79 right now. Practically it is somewhere between 120-240.
Some stats:
```
column_limit=120: 115 files changed, 2423 insertions(+), 1408 deletions(-)
column_limit=160: 108 files changed, 1563 insertions(+), 1247 deletions(-)
column_limit=200: 104 files changed, 1255 insertions(+), 1178 deletions(-)
ACKs for commit 1111f0:
practicalswift:
utACK 1111f0718acea42954600a4dbd553ac40aae797f agree with @ryanofsky
ryanofsky:
utACK 1111f0718acea42954600a4dbd553ac40aae797f
Tree-SHA512: 1ce0da83b917496f4ea7d1af31b624517a78998a10091b6ba611737f2c819fa3cda1786307f15d20131a00fd95232818e3d1108950dd12b60c4674eaa447839f
f6ee177f7 Remove unused AES-128 code (practicalswift)
Pull request description:
Remove unused AES-128 code.
As far as I can tell this AES-128 code has never been in use in the project (outside of testing/benchmarking).
The AES-256 code is used in `CCrypter::Encrypt`/`CCrypter::Decrypt` (`src/wallet/crypter.cpp`).
Trivia: 0.15% of the project's C++ LOC count (excluding dependencies) is trimmed off:
```
$ LOC_BEFORE=$(git grep -I "" HEAD~1 -- "*.cpp" "*.h" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | wc -l)
$ LOC_AFTER=$(git grep -I "" -- "*.cpp" "*.h" ":(exclude)src/leveldb/" ":(exclude)src/secp256k1/" ":(exclude)src/univalue/" | wc -l)
$ bc <<< "scale=4; ${LOC_AFTER}/${LOC_BEFORE}"
.9985
```
:-)
Tree-SHA512: 9588a3cd795a89ef658b8ee7323865f57723cb4ed9560c21de793f82d35e2835059e7d6d0705e99e3d16bf6b2a444b4bf19568d50174ff3776caf8a3168f5c85
5801dd628d docs: Add more tips to productivity.md (gwillen)
Pull request description:
Add advice to productivity.md on:
- Using ccache to optimal effect
- The with-incompatible-bdb configure option
- Building less than the entire set of targets
ACKs for commit 5801dd:
promag:
utACK 5801dd6.
MarcoFalke:
utACK 5801dd6
Tree-SHA512: 2138acd4bf5a27ecaa9a02fb2141903d01ee199ba85ccf6a5ad6a0a4dabf4447d043108cdd86998801b0282e899f70892f9337b0b6dc59c6d1f0fccf61adb4e4
f7696e6183 depends: qt: Don't hardcode pwd path (Carl Dong)
89bee1bdbf depends: tar: Always extract as yourself (Carl Dong)
340ef50772 depends: Defer to Python detected by autoconf (Carl Dong)
Pull request description:
Removes some implicit assumptions that the depends system has about its environment and, as a side-effect, makes it possible to build the depends tree under severely privilege-limited environments such as containers built by Guix.
Tree-SHA512: e8618f9310a0deae864b44f9b60baa29e6225ba16817973ff7830b55798ebd4343aa06da6c1f92682a7afb709d26f80d6ee794a139d4d44c27caf4f0c8fe95fc
19a0c4af0f depends: native_protobuf: avoid system zlib (Carl Dong)
Pull request description:
I don't believe we use any zlib features in protobufs
Tree-SHA512: cd09229f3fac215f58e9ddd4871f190cf2a301e25939aaa1c6ee130d1ba5bbb00d9ebe9ca012a2894bac4c2db923259f34fe43e255ad55ccd2b11ec88afc2a8f
228e80608 Enable TLS in link to chris.beams.io (JeremyRand)
Pull request description:
This PR enables TLS in a documentation link to chris.beams.io, which improves security. This change was originally part of https://github.com/bitcoin/bitcoin/pull/13778 , which was closed for reasons unrelated to this change.
Tree-SHA512: 01f02031b39ebdaa7fc1859befde7e3bfff105892a8b9e1737ff94741599c6c5936eff53871e5e3560931512c9b7a4ea34f48d8555b583d232c90c2ca6d4d776
ff7f31e07d [doc] productivity: more advanced git range-diff (Sjors Provoost)
3a21905a4e [doc] devtools: mention clang-format dependency (Sjors Provoost)
bf12093191 [doc] productivity: fix broken link (Sjors Provoost)
Pull request description:
Fixes a broken link to `devtools/README.md`, points out the `clang-format` dependency and adds a `git range-diff` incantation that works even with rebases and squashes.
Tree-SHA512: 36e46282f1e28d1bf3f48ada995fbac548f61b7747091eb032b60919cf76c7bdad0fa8aecb0c47adbdaa9ef986d3ec7752b0bb94c63191401856e2ddeec48f3e
fa55104cb8e73c709e90fc5f81c9cb6a8a0713a6 build: use full version string in setup.exe (MarcoFalke)
Pull request description:
Fixes: #15546
Tree-SHA512: a8ccbfef6b9fdd10bd0facadb25019b9296579eee6c8f7b4e5298cc4df52bba61864135ab8f46b900f7a3888fbcc921e039412d5a8127e44d8f2dd2c8fc56f86
32da92bdf6bb55d6d312b0f85797d439cc942db5 gitian: Improve error handling (Wladimir J. van der Laan)
Pull request description:
Improve error handling in gitian builds:
- Set fail-on-error and pipefail flag, this causes a command to fail when either of the pipe stages fails, not only when the last of the stages fails, so this improves error detection.
- Also use `xargs` instead of `find -exec`, because `find` will not propagate errors in the executed command, but `xargs` will.
This will avoid some issues like #15541 where non-determinism is silently introduced due to errors caused by environment conditions (such as lack of disk space in that case).
Tree-SHA512: d5d3f22ce2d04a75e5c25e935744327c3adc704c2d303133f2918113573a564dff3d3243d5569a2b93ee7eb0e97f8e1b1ba81767e966af9015ea711a14091035
fa45123f66 test: Add .style.yapf (MarcoFalke)
Pull request description:
This can *optionally* be used to format any added code before submitting a pull. I use this heavily and wouldn't want to hold it back from others, now that yapf is referred to in https://github.com/bitcoin/bitcoin/blob/master/doc/productivity.md#format-python-diffs-with-yapf-diffpy
Tree-SHA512: 0f3d8bcbb76a710d9faa1226202073e8d967a82a05fc002cd10305ff58b382f5ff3df96a6faaec5bd01613d41f5fc2343e4999fb1217bf1f24f6da186d572ca1
43206239a8 tests: Add script checking for deterministic line coverage (practicalswift)
Pull request description:
Add script checking for deterministic line coverage in unit tests.
Context: #14343 ("coverage reports non-deterministic")
When the coverage is deterministic this script can be invoked from Travis to guard against regressions, but left inactive for now.
Output in case of determinism:
```
$ contrib/test_deterministic_coverage.sh 2
[2019-01-30 20:08:46] Measuring coverage, run #1 of 2
[2019-01-30 20:10:45] Measuring coverage, run #2 of 2
Coverage test passed: Deterministic coverage across 2 runs.
```
Output in case of non-determinism:
```
$ contrib/test_deterministic_coverage.sh 2
[2019-01-30 20:08:46] Measuring coverage, run #1 of 2
[2019-01-30 20:10:45] Measuring coverage, run #2 of 2
The line coverage is non-deterministic between runs.
The test suite must be deterministic in the sense that the set of lines executed at least
once must be identical between runs. This is a neccessary condition for meaningful coverage
measuring.
--- gcovr.run-1.txt 2019-01-30 23:14:07.419418694 +0100
+++ gcovr.run-2.txt 2019-01-30 23:15:57.998811282 +0100
@@ -471,7 +471,7 @@
test/crypto_tests.cpp 270 270 100%
test/cuckoocache_tests.cpp 142 142 100%
test/dbwrapper_tests.cpp 148 148 100%
-test/denialofservice_tests.cpp 225 225 100%
+test/denialofservice_tests.cpp 225 224 99% 363
test/descriptor_tests.cpp 116 116 100%
test/fs_tests.cpp 24 3 12% 14,16-17,19-20,23,25-26,29,31-32,35-36,39,41-42,45-46,49,51-52
test/getarg_tests.cpp 111 111 100%
@@ -585,5 +585,5 @@
zmq/zmqpublishnotifier.h 5 0 0% 12,31,37,43,49
zmq/zmqrpc.cpp 21 0 0% 16,18,20,22,33-35,38-45,49,52,56,60,62-63
------------------------------------------------------------------------------
-TOTAL 61561 27606 44%
+TOTAL 61561 27605 44%
------------------------------------------------------------------------------
```
In this case line 363 of `test/denialofservice_tests.cpp` was executed only in the second run. Non-determinism detected!
Tree-SHA512: 03f45590e70a87146f89aa7838beeff0925d7fd303697ff03e0e69f8a5861694be5f0dd10cb0020e3e3d40c9cf662f71dfcd838f6affb31bd5212314e0a4e3a9
* stop using global pointers, every run gets a unique pointer for itself
* keep tests relatively self contained, remove the need for CleanupBLSDkgTests()
* remove parallel tests, they seem to be somewhat unpredictable and seem to have no correlated increased CPU usage
* minor cleanup and refactoring, convert struct to class, rearrange variables
* correlate batch with quorum, minEpochIterations set to 1, unless tests are known unstable, then set to 10
5a05aa2db2 Add metavar to match var name in help text + Change wording for better readability (Martin Erlandsson)
Pull request description:
The help text given by `test/functional/test_runner.py -h` refers to the value `n`, which is defined as `COMBINEDLOGSLEN` in the list of commands.
To make the help text consistent, this PR changes the display name `COMBINEDLOGSLEN` to `n` by setting the argparse [`metavar`](https://docs.python.org/3/library/argparse.html#metavar) attribute. (`metavar` only changes the _displayed_ name)
Alternatively: Do the opposite and change the help text to use `COMBINEDLOGSLEN`.
---
Before PR:
```
➜ bitcoin > test/functional/test_runner.py -h | grep -A 1 combinedlogslen
--combinedlogslen COMBINEDLOGSLEN, -c COMBINEDLOGSLEN
print a combined log (of length n lines) from all test nodes and test framework to the console on failure.
```
After PR:
```
➜ bitcoin > test/functional/test_runner.py -h | grep -A 1 combinedlogslen
--combinedlogslen n, -c n
print a combined log (of length n lines) from all test nodes and test frameworks to the console on failure.
```
---
Also, fixed pluralization typo.
Tree-SHA512: a1124a4976d29fae1e8ecd7fa2ac523b7f05d541c611166532f44692995691a96faf797fa71582d78634f328b500cbee49c6ef296c8f1a898a57c050cc4e721d
086fc83571 Tests: Fix a comment (fridokus)
Pull request description:
Fix a comment that was false
Tree-SHA512: 945aa38229545e026e18c3abf53a4fbe6ec36413ce690fff7a1dd89b6e102d2b574524092e0ddf06cace82f3c040c59221b9b942be1203525814d2fbd50aaa0b
ab8c6f24d28ea1d1e6258cf316b4b97a0baf2377 Add SAFE_CHARS[SAFE_CHARS_URI]: Chars allowed in URIs (RFC 3986) (practicalswift)
991248649b76a5a071e1360a700f3e2ecf3e1e1f rpc: Make HTTP RPC debug logging more informative (practicalswift)
Pull request description:
* Make HTTP RPC debug logging more informative
* Avoid excessively large log messages (which could theoretically fill up the disk) when running with debug option `-debug=http`
Tree-SHA512: 9068862fb7d34db1e12e6b9dde78b669b86c65b4fed3ea8c9eb6c35310d77fd12b16644728fd7e9fbf25059d25114bded9e061eb3de649d8847486ec42041ce9
9605bbd315eb14690427560fd9a274fe837f59f5 Make clear function argument case in dev notes (Carl Dong)
Pull request description:
Rationale:
For new developers, they might be confused if they see that function arguments are sometimes `camelCase`'d in the codebase. This makes it clear that they _should_ be `snake_case`'d (maybe because no one's gotten to fixing them yet).
Tree-SHA512: 9db16d1fedf9761121844a0865ae3fefea94b5dbdfb36cb18f99cbc73e117f7d798a019f28a1c8bca19772502de2f9ed063f03bd911ffc4d248ec7386cd87d97
0e6de3aacb8ebbf2617e8c11b8dae61acdd79816 added details about commit messages (Martin Erlandsson)
Pull request description:
In this small PR, the gist of [this helpful and informative comment from @fanquake](https://github.com/bitcoin/bitcoin/pull/14583#issuecomment-433668211) is added to the official contributing instructions, to help future first-time contributors get their commit messages right.
Tree-SHA512: ca6e9b639e007944314690ef771470f346d6f0ad8aa398abf86e45258bc617334fdbd2ca10ff930d62669a5eca5b507a3ebae11f75971f80f8a44b0bb17ab054
8907df9e02ec47ef249a7422faa766f06aa01e94 qa: Ensure wallet unload during walletpassphrase timeout (João Barbosa)
321decffa1fbf213462d97e5372bd0c4eeb99635 rpc: Fix wallet unload during walletpassphrase timeout (João Barbosa)
Pull request description:
Replaces the raw wallet pointer in the `RPCRunLater` callback with a `std::weak_ptr` to check if the wallet is not expired.
To test:
```
bitcoind -regtest
bitcoin-cli -regtest encryptwallet foobar
bitcoin-cli -regtest walletpassphrase foobar 5 && bitcoin-cli -regtest unloadwallet ""
```
Fixes#14452.
Tree-SHA512: 311e839234f5fb7955ab5412a2cfc1903ee7132ea56a8ab992ede3614586834886bd65192b76531ae0aa3a526b38e70ca2e1cdbabe52995906ff97b49d93c268
1fb3c167c3cbd4a432a064b299439b3430157dda Add `doc/bitcoin-conf.md` (Hennadii Stepanov)
Pull request description:
From the IRC:
> 2018-10-16T05:35:03 \<wumpus\> if something can be solved by better documentation, please work on documentation!
> 2018-10-16T05:35:12 \<wumpus\> don't change the code instead
Refs:
- #14370
- #14427
- #14494
Based on the BITCOIN.CONF(5) manual page written by Micah Anderson \<micah@debian.org\> for the Debian system.
Tree-SHA512: 16393c9073c027fa1c46f8b59651e60b9a3159b3aeb9b3102040c292d2787f32b1ead5977957ac3ac0759a4bf626650a2325b68ad84320964ac089ffc2d3b4f4
a3197c5294 Disable wallet and address book Qt tests on macOS minimal platform (Russell Yanofsky)
Pull request description:
macOS minimal platform is frequently broken, and these are currently failing with Qt 5.11.1.
The tests do pass when run on the full cocoa platform (with `test_bitcoin-qt -platform cocoa`).
Stack trace from test crash: https://gist.github.com/ryanofsky/3401fb63c52d13d5585e7fc777361f1e
Tree-SHA512: a05644ef15d75ea7d7f85ea804c6a5fe78e4e7358b189cbab639d9f7dc46163a35f77f7a2b4ae2fd6be5b9fb22898386b4d88069d5ee8d5fdbd995157c6f0846
1f01fe0257 bitcoin-tx: Use constant for n pubkeys check (Antoine Le Calvez)
Pull request description:
Use the constant for the maximum number of public keys in a multisig script defined in script/script.h instead of hardcoding it.
Tree-SHA512: 83e6c46df907944d0d993159955e402784415536d61fdb5a5becba2b042e37ad2a291b27301c1b169416cb71c823a571d82257512cd4a64848a27a24c875fcc6
75ea00f391b742e435c650aae3e827aad913d552 Remove unused fsbridge::freopen (practicalswift)
cceedbc4bf1056db17e0adf76d0db45b94777671 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift)
Pull request description:
Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`).
Context: https://github.com/bitcoin/bitcoin/pull/13148#issuecomment-386288606
Thanks @ajtowns!
Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1
# Conflicts:
# src/logging.cpp
fa782a308dbe7bc579c122f63c1c65666fc85e91 qa: Use named args in some tests (MarcoFalke)
b4d33096734d787b0e1d754064039cbb64ce8d61 scripted-diff: Use named arguments in feature_block (MarcoFalke)
749ba35e7c9fbc21dbea27fd1be102b91313d132 scripted-diff: Pass node into p2p_segwit acceptance tests (MarcoFalke)
Pull request description:
It is confusing to use a list of arguments such as `False, False, 16, ...` where it is unclear what each of them means.
Run some scripted diffs to put meaning to them.
Tree-SHA512: d768df2375ea3c77145ebb1bf4c2d690581a379031449ded7ae160022d975eb13890aa8c6a44a5eebda8791cb2910a599326e431af76ed9e60afe1d182ada65c
# Conflicts:
# test/functional/feature_block.py
# test/functional/mining_basic.py
# test/functional/p2p_segwit.py
