40b17f5f9 [scripts] lint-whitespace: use perl instead of grep -P (Sjors Provoost)
Pull request description:
MacOS does not support `grep -P` out of the box. This change makes
it easier for developers to check for whitespace problems locally.
Based on [this](https://stackoverflow.com/a/16658690) and [this](https://serverfault.com/a/504387) Stack Exchange answer.
Tested with:
```sh
export TRAVIS_COMMIT_RANGE='fe78c9a...62e0453'
contrib/devtools/lint-whitespace.sh
This diff appears to have added new lines with tab characters instead of spaces.
The following changes were suspected:
diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
@@ -0,0 +1,110 @@
+ num.setint64(n);
```
Tree-SHA512: 37c342a0ca2580289cf326a278a051a7c21ba918d6b2143fd9987f159fab85f1de3d770fcf532a642cd5d1957afc8595678128196e102dc473924758f133db7f
b0fec8d623 Append scripts to new test_list array to fix bad assignment (Jeff Rade)
Pull request description:
Fixes review by @MarcoFalke in PR [#12437](https://github.com/bitcoin/bitcoin/pull/12437)
Assignment of `test_list` would point to the same array object as `BASE_SCRIPT` or `ALL_SCRIPTS` which we do not want.
Tree-SHA512: 57d6c1f4563aaffbac68e96782283799b10be687292152d70ffbbc22e7b52c11c1af0c483acb01c69fbaa99bdae01431b65a5d1d0a913d549f58dfd95d0d28d9
97bcd36811 [Trivial] Simplify if-else blocks and more descriptive variable naming (Jeff Rade)
Pull request description:
Was looking through `test_runner.py` to start work on [#11964](https://github.com/bitcoin/bitcoin/issues/11964). Made a few changes to make the file more readable and keep these separate from future PR.
Tree-SHA512: 7508f4ee39672d18718d8f80b61b89918eac7b4c75953682b812b73013f18ebd81adc7953f3b6c98c5c598adeb1998f5455f123b5566d1cc03631c7924b4103a
b7cd08b71 Add documentation to PeerLogicValidation interface and related functions (James O'Beirne)
Pull request description:
Adds docs for PeerLogicValidation's public interface and two related functions.
Tree-SHA512: b4c2f47e9baa9396d2b6faf3792e46b371c50cd91b9ac890f263f4d14eb24a71e7b40ceb4cbb41e254f5008eff357f417b842618e7ebece9039802ab2a5dd728
e2b2e48b6 doc: SIGNER can contains space inside now. (Ken Lee)
Pull request description:
SIGNER can contains space inside now. mentioned in https://github.com/bitcoin/bitcoin/pull/12527#issuecomment-370506416
Tree-SHA512: 8da1e8146751457c351058c0142fa3d474a338fe7304a31ebed4726202202724aaca94441806458512259238a703601b87961196abc3fdd57b5eb0f062ff0c12
526e28220a contrib: Add support for out-of-tree builds in gen-manpages.sh (Wladimir J. van der Laan)
Pull request description:
This adds support for setting the environment variable `BUILDDIR` to point to executables that are outside the source directory.
E.g. to invoke the tool when the build is in $PWD/build:
```bash
BUILDDIR=$PWD/build contrib/devtools/gen-manpages.sh
```
This avoids having to manually copy the generated manpages after they end up in the build instead of source path, when setting TOPDIR instead.
Tree-SHA512: 8dc6dd7a47a0c014ae7d27f0ac9d86f69238ec6bac8a3007b975bb88c9f37014755c716c5e62604dd91baad2f8a41fd1544cdca3ba4b59bc76602e6593f4a4a7
a777244e4 Build: Add a makefile target for Doxygen documentation (Andrea Comand)
Pull request description:
You can now build the doxygen documentation with `make docs` and clean it with `make clean-docs`.
Fixes: #11949
Tree-SHA512: f2361ec7f771227367dd04bba1a444b44e59f13901463a678a5f2f579a10a56d67db2e29552e754e312a1c472a31593b6af189cbaac5cd351a428c57baf5ace7
be45a67 Add some script tests related to BOOL ops and odd values like negative 0. (Richard Kiss)
Pull request description:
Add some script tests related to BOOL ops and odd values like negative 0.
Tree-SHA512: 8e633f7ea5eea39e31016994baf60f295fa1dc8cae27aa5fcfc741ea97136bfb3ddc57bb62b9c6bf9fe256fc09cdd184906ba8e611e297cf8d2d363da2bbf1d4
fafbf7f74e devtools: Exclude patches from lint-whitespace (MarcoFalke)
Pull request description:
By default, unified patches have trailing whitespace in all context lines. Thus, exclude patches from linting.
Tree-SHA512: 8f89f1584581e94dd4e34bd522cba21602bafe7933b4631a3abc5da5a8f25568810862d696618fe63c15edf3e046869ad5077d09373f09792985503c6a415538
fa27623 qt: Initialize members in WalletModel (MarcoFalke)
Pull request description:
This prevents segfaults (or errors when running qt in valgrind)
```
Conditional jump or move depends on uninitialised value(s)
WalletModel::checkBalanceChanged() (walletmodel.cpp:156)
Tree-SHA512: 38c8c03c7fa947edb3f1c13eab2ac7a62ef8f8141603c2329a7dc5821a887a349af8014dc739b762e046f410f44a9c6653b6930f08b53496cf66381cadc06246
fa795cf wallet: Disallow abandon of conflicted txes (MarcoFalke)
Pull request description:
Abandon transactions that are already conflicted is a noop, so don't try and return false/throw instead.
Tree-SHA512: fd2af4149bd2323f7f31fe18685c763790b8589319b4e467b464ab456d5e8971501ab16d124e57a22693666b06ae433ac3e59f0fd6dfbd2be2c6cae8be5bcbd8
a9d0ebc262 Enable flake8 warnings for all currently non-violated rules (practicalswift)
4cbab15e75 tests: Fix accidental redefinition of previously defined variable via list comprehension (practicalswift)
0b9207efbe Enable flake8 warning for "list comprehension redefines 'foo' from line N" (F812) (practicalswift)
Pull request description:
* Enable `flake8` warnings for all currently non-violated rules
* Fix accidental redefinition via list comprehension
Tree-SHA512: 738b87789e99d02abb2c6b8ff58f65c0cbfeb93e3bf320763e033e510ebd0a4f72861bc8faaf42c14a056a5d4659c33dc70a63730a32cc15159559427bf21193
ada1af6d8f Fix typo in test_runner.py causing error (MeshCollider)
Pull request description:
In the case that a test fails, the typo in run_tests() (introduced in #11858) will cause an error rather than printing out the combined logs, hiding the cause of the failure.
Tree-SHA512: 7d7aa406d92750320ed20610cc5f174cdc94086f630af8c0c4db2003497132e0c56d59b94312fb42ad4507904a2fa858226a4a9337450930bf206183fc35c0a0
faefd29 qa: Prepare functional tests for Windows (MarcoFalke)
Pull request description:
* Pass `sys.executable` when calling a python script via the subprocess
module
* Don't remove the log file while it is still open and written to
* Properly use os.pathsep and os.path.sep when modifying the PATH
environment variable
* util-tests: Use os.path.join for Windows compatibility
Ref: #8227
Tree-SHA512: c507a536af104b3bde4366b6634099db826532bd3e7c35d694b5883c550920643b3eab79c76703ca67e1044ed09979e855088f7324321c8d52112514e334d614
faac7a2db4f9f511c901cb1b4d4e7c599b92884f qa: Avoid checking reject code for now (MarcoFalke)
Pull request description:
The node will often disconnect before sending a reject code. A more
robust solution would be to read from the debug log. See #13006
Tree-SHA512: 1dabf8a43dabbc722f4ffe4fbc1f870090253a66290b2d1a95e7a24e14c6442b493c314480c0314587164eb65e5d468aa9eb5e107ad90bb3ca821a97ea4d373c
cfaac2a60 Add build support for 'gprof' profiling. (murrayn)
Pull request description:
Support for profiling build: `./configure --enable-profiling`
Tree-SHA512: ea983cfce385f1893bb4ab7f94ac141b7d620951dc430da3bbc92ae1357fb05521eac689216e66dc87040171a8a57e76dd7ad98036e12a2896cfe5ab544347f0
937bf4335 Use std:🧵:hardware_concurrency, instead of Boost, to determine available cores (fanquake)
Pull request description:
Following discussion on IRC about replacing Boost usage for detecting available system cores, I've opened this to collect some benchmarks + further discussion.
The current method for detecting available cores was introduced in #6361.
Recap of the IRC chat:
```
21:14:08 fanquake: Since we seem to be giving Boost removal a good shot for 0.15, does anyone have suggestions for replacing GetNumCores?
21:14:26 fanquake: There is std:🧵:hardware_concurrency(), but that seems to count virtual cores, which I don't think we want.
21:14:51 BlueMatt: fanquake: I doubt we'll do boost removal for 0.15
21:14:58 BlueMatt: shit like BOOST_FOREACH, sure
21:15:07 BlueMatt: but all of boost? doubtful, there are still things we need
21:16:36 fanquake: Yea sorry, not the whole lot, but we can remove a decent chunk. Just looking into what else needs to be done to replace some of the less involved Boost usage.
21:16:43 BlueMatt: fair
21:17:14 wumpus: yes, it makes sense to plan ahead a bit, without immediately doing it
21:18:12 wumpus: right, don't count virtual cores, that used to be the case but it makes no sense for our usage
21:19:15 wumpus: it'd create a swarm of threads overwhelming any machine with hyperthreading (+accompanying thread stack overhead), for script validation, and there was no gain at all for that
21:20:03 sipa: BlueMatt: don't worry, there is no hurry
21:59:10 morcos: wumpus: i don't think that is correct
21:59:24 morcos: suppose you have 4 cores (8 virtual cores)
21:59:24 wumpus: fanquake: indeed seems that std has no equivalent to physical_concurrency, on any standard. That's annoying as it is non-trivial to implement
21:59:35 morcos: i think running par=8 (if it let you) would be notably faster
21:59:59 morcos: jeremyrubin and i discussed this at length a while back... i think i commented about it on irc at the time
22:00:21 wumpus: morcos: I think the conclusion at the time was that it made no difference, but sure would make sense to benchmark
22:00:39 morcos: perhaps historical testing on the virtual vs actual cores was polluted by concurrency issues that have now improved
22:00:47 wumpus: I think there are not more ALUs, so there is not really a point in having more threads
22:01:40 wumpus: hyperthreads are basically just a stored register state right?
22:02:23 sipa: wumpus: yes but it helps the scheduler
22:02:27 wumpus: in which case the only speedup using "number of cores" threads would give you is, possibly, excluding other software from running on the cores on the same time
22:02:37 morcos: well this is where i get out of my depth
22:02:50 sipa: if one of the threads is waiting on a read from ram, the other can use the arithmetic unit for example
22:02:54 morcos: wumpus: i'm pretty sure though that the speed up is considerably more than what you might expect from that
22:02:59 wumpus: sipa: ok, I back down, I didn't want to argue this at all
22:03:35 morcos: the reason i haven't tested it myself, is the machine i usually use has 16 cores... so not easy due to remaining concurrency issues to get much more speedup
22:03:36 wumpus: I'm fine with restoring it to number of virtual threads if that's faster
22:03:54 morcos: we should have somene with 4 cores (and  actually test it though, i agree
22:03:58 sipa: i would expect (but we should benchmark...) that if 8 scriot validation threads instead of 4 on a quadcore hyperthreading is not faster, it's due to lock contention
22:04:20 morcos: sipa: yeah thats my point, i think lock contention isn't that bad with 8 now
22:04:22 wumpus: on 64-bit systems the additional thread overhead wouldn't be important at least
22:04:23 gmaxwell: I previously benchmarked, a long time ago, it was faster.
22:04:33 gmaxwell: (to use the HT core count)
22:04:44 wumpus: why was this changed at all then?
22:04:47 wumpus: I'm confused
22:05:04 sipa: good question!
22:05:06 gmaxwell: I had no idea we changed it.
22:05:25 wumpus: sigh 
22:05:54 gmaxwell: What PR changed it?
22:06:51 gmaxwell: In any case, on 32-bit it's probably a good tradeoff... the extra ram overhead is worth avoiding.
22:07:22 wumpus: https://github.com/bitcoin/bitcoin/pull/6361
22:07:28 gmaxwell: PR 6461 btw.
22:07:37 gmaxwell: er lol at least you got it right.
22:07:45 wumpus: the complaint was that systems became unsuably slow when using that many thread
22:07:51 wumpus: so at least I got one thing right, woohoo
22:07:55 sipa: seems i even acked it!
22:07:57 BlueMatt: wumpus: there are more alus
22:08:38 BlueMatt: but we need to improve lock contention first
22:08:40 morcos: anywya, i think in the past the lock contention made 8 threads regardless of cores a bit dicey.. now that is much better (although more still to be done)
22:09:01 BlueMatt: or we can just merge #10192, thats fee
22:09:04 gribble: https://github.com/bitcoin/bitcoin/issues/10192 | Cache full script execution results in addition to signatures by TheBlueMatt · Pull Request #10192 · bitcoin/bitcoin · GitHub
22:09:11 BlueMatt: s/fee/free/
22:09:21 morcos: no, we do not need to improve lock contention first. but we should probably do that before we increase the max beyond 16
22:09:26 BlueMatt: then we can toss concurrency issues out the window and get more speedup anyway
22:09:35 gmaxwell: wumpus: yea, well in QT I thought we also diminished the count by 1 or something? but yes, if the motivation was to reduce how heavily the machine was used, thats fair.
22:09:56 sipa: the benefit of using HT cores is certainly not a factor 2
22:09:58 wumpus: gmaxwell: for the default I think this makes a lot of sense, yes
22:10:10 gmaxwell: morcos: right now on my 24/28 physical core hosts going beyond 16 still reduces performance.
22:10:11 wumpus: gmaxwell: do we also restrict the maximum par using this? that'd make less sense
22:10:51 wumpus: if someone *wants* to use the virtual cores they should be able to by setting -par=
22:10:51 sipa: *flies to US*
22:10:52 BlueMatt: sipa: sure, but the shared cache helps us get more out of it than some others, as morcos points out
22:11:30 BlueMatt: (because it means our thread contention issues are less)
22:12:05 morcos: gmaxwell: yeah i've been bogged down in fee estimation as well (and the rest of life) for a while now.. otherwise i would have put more effort into jeremy's checkqueue
22:12:36 BlueMatt: morcos: heh, well now you can do other stuff while the rest of us get bogged down in understanding fee estimation enough to review it 
22:12:37 wumpus: [to answer my own question: no, the limit for par is MAX_SCRIPTCHECK_THREADS, or 16]
22:12:54 morcos: but to me optimizing for more than 16 cores is pretty valuable as miners could use beefy machines and be less concerned by block validation time
22:14:38 BlueMatt: morcos: i think you may be surprised by the number of mining pools that are on VPSes that do not have 16 cores 
22:15:34 gmaxwell: I assume right now most of the time block validation is bogged in the parts that are not as concurrent. simple because caching makes the concurrent parts so fast. (and soon to hopefully increase with bluematt's patch)
22:17:55 gmaxwell: improving sha2 speed, or transaction malloc overhead are probably bigger wins now for connection at the tip than parallelism beyond 16 (though I'd like that too).
22:18:21 BlueMatt: sha2 speed is big
22:18:27 morcos: yeah lots of things to do actually...
22:18:57 gmaxwell: BlueMatt: might be a tiny bit less big if we didn't hash the block header 8 times for every block. 
22:21:27 BlueMatt: ehh, probably, but I'm less rushed there
22:21:43 BlueMatt: my new cache thing is about to add a bunch of hashing
22:21:50 BlueMatt: 1 sha round per tx
22:22:25 BlueMatt: and sigcache is obviously a ton
```
Tree-SHA512: a594430e2a77d8cc741ea8c664a2867b1e1693e5050a4bbc8511e8d66a2bffe241a9965f6dff1e7fbb99f21dd1fdeb95b826365da8bd8f9fab2d0ffd80d5059c
741f0177c Add DynamicMemoryUsage() to LevelDB (Evan Klitzke)
Pull request description:
This adds a new method `CDBWrapper::DynamicMemoryUsage()` similar to Bitcoin's existing methods of the same name. It's implemented by asking LevelDB for the information, and then parsing the string response. I've also added logging to `CDBWrapper::WriteBatch()` to track this information:
```
$ tail -f ~/.bitcoin/testnet3/debug.log | grep WriteBatch
2018-03-05 19:34:55 WriteBatch memory usage: db=chainstate, before=0.0MiB, after=0.0MiB
2018-03-05 19:35:17 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
2018-03-05 19:35:17 WriteBatch memory usage: db=chainstate, before=0.0MiB, after=8.0MiB
2018-03-05 19:35:22 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
2018-03-05 19:35:22 WriteBatch memory usage: db=chainstate, before=8.0MiB, after=17.0MiB
2018-03-05 19:35:26 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
2018-03-05 19:35:27 WriteBatch memory usage: db=chainstate, before=9.0MiB, after=18.0MiB
2018-03-05 19:35:40 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
2018-03-05 19:35:41 WriteBatch memory usage: db=chainstate, before=9.0MiB, after=7.0MiB
2018-03-05 19:35:52 WriteBatch memory usage: db=index, before=0.0MiB, after=0.0MiB
2018-03-05 19:35:52 WriteBatch memory usage: db=chainstate, before=7.0MiB, after=9.0MiB
^C
```
As LevelDB doesn't seem to provide a way to get the database name, I've also added a new `m_name` field to the `CDBWrapper`. This is necessary because we have multiple LevelDB databases (two now, and possibly more later, e.g. #11857).
I am using this information in other branches where I'm experimenting with changing LevelDB buffer sizes.
Tree-SHA512: 7ea8ff5484bb07ef806af17d000c74ccca27d2e0f6c3229e12d93818f00874553335d87428482bd8acbcae81ea35aef2a243326f9fccbfac25989323d24391b4
874e81808 Allow dustrelayfee to be set to zero (Luke Dashjr)
Pull request description:
I don't see and can't think of any rationale for forbidding this configuration.
Tree-SHA512: df09441f4aec63e79bea94838b7f8e336cebaeb0a22b5e58d27937bbeb1377f229921aeae43674e0b63fc40a39ae51a264d48aa1cdb4cbd0e3339d32856698bf
9c5a4a6ed Stop special-casing phashBlock handling in validation for TBV (Matt Corallo)
Pull request description:
There is no reason to do this, really, we already have "ignore PoW" flags. Motivated by https://github.com/bitcoin/bitcoin/pull/11739#discussion_r155841721
Tree-SHA512: 37cb1ae5b11c9e8ed7a679bb07ad3b119a2a014744b26d197d67ba21beb19fe6815271df935e40f7c7bd5f2e4d7ae4dad7bd4d00fa230a8d789f37e9de31a769
7eb665fc8 [Trivial] link mentioned scripted-diff-commit (Felix Wolfsteller)
Pull request description:
Make it easier for people who do not operate on a cloned repository to access the example mentioned.
Tree-SHA512: 1c06e551c68cad03e6bd541bf0e0076cdf0b48ef9b8b4e4a61435367c3435e2e4ccb934112e8dc29d3d70217d8834924704aaf839e25d1133312df86848ca1a1
fa9461473 [doc] dev-notes: Members should be initialized (MarcoFalke)
Pull request description:
Also, remove mention of threads that were removed long ago.
Motivation:
Make it easier to spot bugs such as #11654 and #12426
Tree-SHA512: 8ca1cb54e830e9368803bd98a8b08c39bf2d46f079094ed7e070b32ae15a6e611ce98d7a614f897803309f4728575e6bc9357fab1157c53d2536417eb8271653
e7d9fc5 [qt] navigate to transaction history page after send (Sjors Provoost)
Pull request description:
Before this change QT just remained on the Send tab, which I found confusing. Now it switches to the Transactions tab. This makes it more clear to the user that the send actually succeeded, and here they can monitor progress.
Ideally I would like to highlight the transaction, e.g. by refactoring `TransactionView::focusTransaction(const QModelIndex &idx)` to accept a transaction hash, but I'm not sure how to do that.
Tree-SHA512: 8aa93e03874de8434e18951f8aec47377814c0bcaf7eda4766fc41d5a4e32806346e12e4139e4d45468dfdf0b786f5a7faa393a31b8cd6c65ccac21fb3782c33
d6f3a73 Remove redundant locks (practicalswift)
Pull request description:
Remove redundant locks:
* ~~`FindNode(...)` is locking `cs_vNodes` internally~~
* `SetAddressBook(...)` is locking `cs_wallet` internally
* `DelAddressBook(...)` is locking `cs_wallet` internally
**Note to reviewers:** From what I can tell these locks are redundantly held from a data integrity perspective (guarding specific variables), and they do not appear to be needed from a data consistency perspective (ensuring a consistent state at the right points). Review thoroughly and please let me know if I'm mistaken :-)
Tree-SHA512: 7e3ca2d52fecb16385dc65051b5b20d81b502c0025d70b0c489eb3881866bdd57947a9c96931f7b213f5a8a76b6d2c7b084dff0ef2028a1e9ca9ccfd83e5b91e
a25cb0f Use ptrdiff_t type to more precisely indicate usage and avoid compiler warnings. (murrayn)
Pull request description:
ptrdiff_t is a more strictly correct type, and gets rid of compiler warnings.
Tree-SHA512: 39718a5cdc10e698f14185f4622a9b439728bce619bd8b3a86f2b99ed5b056cf5a8545a3e5c4bc8a6a01b845fb73510036cee5e6d2629c58df26be692a957fba
4ae7d15 init: Fix help message for checkblockindex (MarcoFalke)
Pull request description:
Minor fixup for my commit fa6ab96799.
Tree-SHA512: 18f9255bf1342007be2bdc26d6f688bcd27ba8eebfc709bd9ee31dfd2e4d955d2b699686492ccf59e94eb4b1cc7bf3332376aa151a68cb0b21695b3f67d4a940
57dae3fc4a Replace boost::call_once with std::call_once (donaloconnor)
Pull request description:
This replaces boost::call_once with the C++11 std::call_once. The aim is to remove unnecessary boost code.
Tested on Windows/MSVC
Tree-SHA512: 5e98ea6e5052fffeaf29f845f4ecf1078b38cbb27671c5b7b6167e7f074a391e10020445107979d9e220d029bc9464fb8b2ccb0bea664eeb7af59a789c988b10
8b2ef27 tests: Test connecting with non-existing RPC cookie file (practicalswift)
a2b2476 tests: Test connecting to a non-existing server (practicalswift)
de04fde bitcoin-cli: Provide a better error message when bitcoind is not running (practicalswift)
Pull request description:
Provide a better `bitcoin-cli` error message when `bitcoind` is not running.
Before this patch:
```
$ killall -9 bitcoind
$ bitcoin-cli -testnet echo 'hello world'
error: Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (/root/.bitcoin/bitcoin.conf)
```
After this patch:
```
$ killall -9 bitcoind
$ bitcoin-cli -testnet echo 'hello world'
error: Could not connect to the server 127.0.0.1:18332
Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
```
Tree-SHA512: bb16e1a9a1ac110ee202c3cb99b5d7c5c1e5487a17e6cd101e12dc69e9525c14dc71f37b128c26ad615369a57547f15d0f1e29b207c1b2f2ee4b4ba7105f3433
Signed-off-by: Pasta <pasta@dashboost.org>
d843db7 Qt: remove "new" button during receive-mode in addressbook (Jonas Schnelli)
Pull request description:
There are currently two ways how to generate new receiving addresses in the GUI (which leads to code duplication or required refactoring, see #12520).
Since the address-book is probably something that should be removed in the long run, suppressing the new-button in receive-mode could be a first step in deprecating the address book.
With this PR, users can still edit existing receiving address book entries and they can still create new sending address book entries.
Tree-SHA512: abe8d1b44bc3e1b53826ccf9d2b3f764264337758d95ca1fe1ef1bac72d47608cf454055fce3720e06634f0a5841a752ce643b4505b47d6e322b6fc71296e961
bb079a0e2c Remove unused variable in SortForBlock (Drew Rasmussen)
Pull request description:
Although txiter is passed to BlockAssembler::SortForBlock, it is never used. Other than BlockAssembler::addPackageTxs, no other method ever makes a call to SortForBlock, thus making this change harmless.
Tree-SHA512: c7df948c5f75f7371844200e0227a26476437f300148d29020e01041b382f5bda31d9c520c9c5425aee88ce8f4a52cd0e594985d69ed8a081b878cda2e4de8c5