15c69b158d wallet: Log and ignore errors in ListWalletDir and IsBerkeleyBtree (João Barbosa)
Pull request description:
Use the `noexcept` members of `boost::filesystem::recursive_directory_iterator` in order to ignore `boost::filesystem::directory_iterator::construct: Permission denied` errors. The errors are logged though.
Steps to reproduce the issue:
```sh
# 1. create directory for -walletdir without read access:
mkdir /tmp/foo && chmod a-r /tmp/foo
# 2. run bitcoin-qt and should print an error, but continues running:
/Volumes/Bitcoin-Core/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt -regtest -walletdir=/tmp/foo
/private/tmp/foo: Permission denied
# 4. go to File -> Open Wallet and should segfault:
EXCEPTION: N5boost10filesystem16filesystem_errorE
boost::filesystem::directory_iterator::construct: Permission denied: "/private/tmp/foo"
bitcoin in Runaway exception
```
Tree-SHA512: 37e8bf5a1e0defc331030fd511bf9cac2765d01dfbf23e7233f37506e85b8ad07edcde9ba6dae7a2c95700c78d28c7dd248153607381852da96273cb159c4934
335931df4a rpc: return a number for estimated_feerate in analyzepsbt (fanquake)
a4d0fd026b doc: correct analysepsbt rpc doc (fanquake)
Pull request description:
Even though `missing` is optional, all its field are also all optional.
`estimated_vsize` is optional (calculate alongside `estimated_feerate`).
Make `estimated_feerate` output numeric.
Tree-SHA512: 5e063bcfbca73d3d08d29c5d1f59bc1791757f69248da4a9c70fe4b2fe82807ec4ed9fca48d439101d66ba924916fa5da9232167d74ea2b967fc04f0a36f190e
56f1d28d9b dead code: Remove dead option in HexStr conversion (Lenny Maiorani)
Pull request description:
Problem:
- Nothing uses the `fspaces` argument to `HexStr()` besides unit
tests. This argument results in extra complexity and a small
performance decrease within the function.
Solution:
- Remove unused `fspaces` option.
- Remove associated unit tests.
Tree-SHA512: 33d00ce354bbc62a77232fa301cdef0a9ed2c5a09e792bc40e9620c2f2f88636e322a38c76b81d10d12a1768dd1b3b2b9cf180f7e33daef9b4f27afed68ccf70
c9963ae8b1 Fix overflow bug in analyzepsbt fee: CAmount instead of int (Pieter Wuille)
Pull request description:
This causes the `fee` and `estimated_feerate` values in the the analyzepsbt output to be off if the amount being spent exceed 21.47483647 BTC.
Tree-SHA512: 61c1e26894617c51cc5fc026a3677a0b759fcbac1e70efa7fdc68d57cfd484525e18c906f1b8c06fd5d846b74a3cb4bc0bbd302a6eeaade79055a47d6d0dacc2
fa55104cb8 build: use full version string in setup.exe (MarcoFalke)
Pull request description:
Fixes: #15546
Tree-SHA512: a8ccbfef6b9fdd10bd0facadb25019b9296579eee6c8f7b4e5298cc4df52bba61864135ab8f46b900f7a3888fbcc921e039412d5a8127e44d8f2dd2c8fc56f86
3f6568d66b cli: remove duplicate wallet fields from -getinfo (fanquake)
Pull request description:
`walletversion` and `balance` are both included below.
Tree-SHA512: cd9fe9739a2f492c8f7c0407b43a6fa95187f7e5318f05e080bac112f9f4333d2e9b84c505d098f8d66fa79439007d1c0b22e5a87d70bf5ea53ab647ee4c2046
890396cbd5 cli: replace testnet with chain and return network name as per BIP70. (fanquake)
Pull request description:
Related IRC discussion [here (line 151)](http://www.erisian.com.au/bitcoin-core-dev/log-2019-03-09.html).
Tree-SHA512: 8bdbacc7b8ce8bd2cc7c47aa9d73f2830a7c2e2ec43686430e3fba1a9db0e53a285467f26cde6dcc3bf948b7d6d59b9b7f184ce1a30a8970f39e5396dfc122f0
Problem:
- Nothing uses the `fspaces` argument to `HexStr()` besides unit
tests. This argument results in extra complexity and a small
performance decrease within the function for branch evalulation.
Solution:
- Remove unused `fspaces` option.
6e1aaffa98 doc: remove release note fragments (fanquake)
Pull request description:
Removes all release note fragments from prior to the 0.18.0 branch off.
All of these fragments have been merged into the WIP release-notes on the [dev wiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.18.0-Release-Notes-Draft).
Tree-SHA512: ad991134bdb04c556bf9abf8211c253a65866e20e11ed111b5672270c138c5427608a56df6b155af3f0fd5bd7e8632b322d90dffc51b5d9bf742b85cf63c0c87
Removes all release note fragments from prior to the 0.18.0 branch off. All of these fragments have been merged into the WIP release-notes on the dev wiki.
20e6ea259b [addrman] Improve collision logging and address nits (Suhas Daftuar)
f71fdda3bc [addrman] Ensure collisions eventually get resolved (Suhas Daftuar)
4991e3c813 [net] feeler connections can be made to outbound peers in same netgroup (Suhas Daftuar)
4d834018e3 [addrman] Improve tried table collision logging (Suhas Daftuar)
Pull request description:
The restriction on outbound peers sharing the same network group is not intended to apply to feeler connections, so fix this.
This fixes an issue where a tried table collision with an entry to a netgroup we already have an outbound connection to could cause feelers to stop working, because the tried collision buffer (`m_tried_collisions`) would never be drained.
Also, ensure that all entries don't linger in `m_tried_collisions` by evicting an old entry if its collisions is unresolved after 40 minutes.
Tree-SHA512: 553fe2b01b82cd7f0f62f90c6781e373455a45b254e3bec085b5e6b16690aa9f3938e8c50e7136f19dafa250ed4578a26227d944b76daf9ce4ef0c75802389b6
32da92bdf6 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
82c3b3f8e0 Remove sharp edge (uninitialized m_filter_type) when using the compiler-generated constructor for BlockFilter (practicalswift)
Pull request description:
Remove sharp edge (uninitialised member `m_filter_type`) when using the compiler-generated constructor for `BlockFilter`.
Before (but after added test):
```
$ src/test/test_bitcoin -t blockfilter_tests/blockfilter_basic_test
Running 1 test case...
test/blockfilter_tests.cpp(118): error: in "blockfilter_tests/blockfilter_basic_test": check default_ctor_block_filter_1.GetFilterType() == default_ctor_block_filter_2.GetFilterType() has failed [ != ]
*** 1 failure is detected in the test module "Bitcoin Test Suite"
```
After:
```
$ src/test/test_bitcoin -t blockfilter_tests/blockfilter_basic_test
Running 1 test case...
*** No errors detected
```
Tree-SHA512: 21d41f036b0bf12adcf1a788d84747353f2023cb85fd8ea6c97222967032e8bf54e7910cadb45dfcecd78e5b5dca86685f78cad0596b6d1a08f910ebf20d90aa
28c86de3b gui: Drop unused return values in WalletFrame (João Barbosa)
Pull request description:
This is a small cleanup since the return value of `WalletFrame` methods are not used. This is in line with the usual async slot declaration.
Tree-SHA512: ff0ca098804118bba200a58cd796ff90e853a6430e58125bd178b7bfa9b2b763c13d17b81e8f3ebd94395cac249d80379ba1529680c47682ba6a2ed81492ba33
519b0bc5dc Make last disconnected block BLOCK_FAILED_VALID, even when aborted (Pieter Wuille)
8d220417cd Optimization: don't add txn back to mempool after 10 invalidates (Pieter Wuille)
9ce9c37004 Prevent callback overruns in InvalidateBlock and RewindBlockIndex (Pieter Wuille)
9bb32eb571 Release cs_main during InvalidateBlock iterations (Pieter Wuille)
9b1ff5c742 Call InvalidateBlock without cs_main held (Pieter Wuille)
241b2c74ac Make RewindBlockIndex interruptible (Pieter Wuille)
880ce7d46b Call RewindBlockIndex without cs_main held (Pieter Wuille)
436f7d735f Release cs_main during RewindBlockIndex operation (Pieter Wuille)
1d342875c2 Merge the disconnection and erasing loops in RewindBlockIndex (Pieter Wuille)
32b2696ab4 Move erasure of non-active blocks to a separate loop in RewindBlockIndex (Pieter Wuille)
9d6dcc52c6 Abstract EraseBlockData out of RewindBlockIndex (Pieter Wuille)
Pull request description:
This PR makes a number of improvements to the InvalidateBlock (`invalidateblock` RPC) and RewindBlockIndex functions, primarily around breaking up their long-term cs_main holding. In addition:
* They're made safely interruptible (`bitcoind` can be shutdown, and no progress in either will be lost, though if incomplete, `invalidateblock` won't continue after restart and will need to be called again)
* The validation queue is prevented from overflowing (meaning `invalidateblock` on a very old block will not drive bitcoind OOM) (see #14289).
* `invalidateblock` won't bother to move transactions back into the mempool after 10 blocks (optimization).
This is not an optimal solution, as we're relying on the scheduler call sites to make sure the scheduler doesn't overflow. Ideally, the scheduler would guarantee this directly, but that needs a few further changes (moving the signal emissions out of cs_main) to prevent deadlocks.
I have manually tested the `invalidateblock` changes (including interrupting, and running with -checkblockindex and -checkmempool), but haven't tried the rewinding (which is probably becoming increasingly unnecessary, as very few pre-0.13.1 nodes remain that would care to upgrade).
Tree-SHA512: 692e42758bd3d3efc2eb701984a8cb5db25fbeee32e7575df0183a00d0c2c30fdf72ce64c7625c32ad8c8bdc56313da72a7471658faeb0d39eefe39c4b8b8474
faebd2ef40 doc: Move wallet lock annotations to header (MarcoFalke)
Pull request description:
We put the annotations in a central place (the header) as opposed to spreading them over the cpp files, where they easily get outdated.
Tree-SHA512: 18d8c7329efd3471713de18fe8d63d67c50fcb9fa99bc372294d829aa7668ea33e10d44e9e50121a04d8cc3302d5fd7759224f7935451a4693c4498a555257e6
fa38535130 bench: Benchmark MempoolToJSON (MarcoFalke)
fa5dc3534b rpc: Pass mempool into MempoolToJSON (MarcoFalke)
Pull request description:
This is used in production (e.g. https://jochen-hoenicke.de/queue/#0,24h), so add a benchmark to avoid making it even slower.
Related:
* "getrawmempool true RPC call is O(n^2)" #14765
Tree-SHA512: da09d2e54ee261af8671152f97f863cf1acd7a6adc6578e94046b1ec9e647a670c67499760ef765254f65522dfdf773c3c8729006fa2d63ccb6d53166bafc425
faa9b88199 fuzz: Link BasicTestingSetup (shared with unit tests) (MarcoFalke)
fa85468cd2 test: Move main_tests to validation_tests (MarcoFalke)
fa02b22245 test: Remove useless test_bitcoin_main.cpp (MarcoFalke)
fab2daa026 test: Add missing LIBBITCOIN_ZMQ to test_test_bitcoin_LDADD (MarcoFalke)
Pull request description:
Link against BasicTestingSetup in the fuzz tests, so we can fuzz against validation.
Also include a commit to remove test_bitcoin_main.cpp. That file may or may not overwrite globals in the link stage depending on the link order. This is confusing and useless anyway: The unit tests should never `std::exit` in the middle of the run (especially with success as exit code), since it will skip all test modules afterward.
Also include a commit to remove some unused forward declarations and move the main_tests to validation_tests, since main was long ago split into net_processing and validation.
Tree-SHA512: bdd34c87505450ec106d632f6664aadcbdac7c198172a77da55fab75b274f869ae1a8d06573ba2aff4cb186be9c7a34b7697894ab6f9c82b392f769c9135f36c
21be609b49 In lint-format-strings, open files sequentially (Glenn Willen)
Pull request description:
In lint-format-strings, we use python argparse to read our file arguments. In
this mode, argparse opens all the files simultaneously. On OS X, where the
default filehandle limit is 128, this causes the lint to fail. Instead, ask
argparse for our filename arguments as strings, and open them one at a time
using 'with open'.
Tree-SHA512: 4c7dabf98818a7c5d83ab10c61b89a26957fe399e39e933e30c561cb45c5e8ba6f6aedcde8343da0c32ee340289a8897db6a33708e35ee381334ee27e3f4d356
4d4e4c6448 Suggested interfaces::Chain cleanups from #15288 (Russell Yanofsky)
Pull request description:
Mostly documentation improvements requested in the last review of #15288 before it was merged (https://github.com/bitcoin/bitcoin/pull/15288#pullrequestreview-210241864)
Tree-SHA512: 64e912520bbec20a44032f265a8cf3f11ad7f5126c8626b5ad5e888227b1f92ecb321522fab4bbbd613230b55450abd6ace023631d0a4f357a780d65c5638bfe
fa2797808e test: Remove python3.4 workaround in feature_dbcrash (MarcoFalke)
dddd1d05d3 .python-version: Specify full version 3.5.6 (MarcoFalke)
faa7cdf764 scripted-diff: Update copyright in ./test (MarcoFalke)
fa0e65b772 scripted-diff: test: Remove brackets after assert (MarcoFalke)
fab5a1e0f4 build: Require python 3.5 (MarcoFalke)
fa6bf21f5e scripted-diff: test: Use py3.5 bytes::hex() method (MarcoFalke)
Pull request description:
Python 3.4 is EOL after March 2019, so switch to 3.5. See https://devguide.python.org/#status-of-python-branches
This pull does the following in a bunch of commits:
* scripted diff to use the `bytes::hex()` method in place of previous wrappers (`b2x`, `bytes_to_hex_str`, `hexlify`, ...)
* Update the build system (gitian and travis) to remove python2.7 and replace it with python3.5
* Another scripted-diff to remove brackets after `assert`. This is unrelated to the python3.5 switch, but a stylistic commit, so probably not worth to split up. The motivation behind it is to avoid asserting on data structures (such as tuples of length one), which never fails:
```py
>>> assert(False,) # with brackets
>>> assert False, # without brackets
SyntaxError: invalid syntax
>>> assert False # proper assertion
AssertionError
```
* And then a final scripted diff to update the copyright headers in the `test` subfolder, since I touched most of the files anyway and it wouldn't make sense to split this commit out into a separate pull.
For reference (contributed by luke-jr):
Ubuntu LTS (bionic): 3.6.5
Debian stable (stretch): 3.5.3
RHEL 8 (expected before v0.19): 3.6.x
Gentoo stable: 3.6.5
Arch: 3.7.1
Tree-SHA512: 643c28cd2d5b9543ce4bf8ad2a8b282bc79b37dc5b25c9c8358e6ce201e2a67a546463e5f3430b16652eb2489d7c3ed4b0772cd2e2bf790fe68a5e3cc8a25029
3eac2d57b1 docs: add "sections" info to example bitcoin.conf (Alistair Mann)
Pull request description:
Rebased / commit message fixed version of #15387.
This had ACKs, but just needed the commit message fixed up.
> Most bitcoin.conf options apply to all three networks,
however some apply only to mainnet unless specified in a section.
As stands, conf file has no indication that sections are now in use
or are in some circumstances mandatory (eg, changing rpcport for testnet.)
> Proposed change notifies the reader early which options are affected,
specifically adds those options affected but not already in the example,
adds brief explanation as to what's going on and provides a skeleton template for the sections themselves.
Tree-SHA512: 3331f2cac23d082bda2dcdea7d579360bc464d8e2123d634b810e9ba8edb5162bd62bd86f846b5299a04a3d77636a77e2fd3837c3272b22bc0d9a685d5156062
