8acd58927a614e006641384f61f8e7fca1cc68fc Fix Python Docstring to include all Args. (John Bampton)
Pull request description:
Found a Python function that had incorrect and missing arguments in its Docstring.
ACKs for top commit:
laanwj:
ACK 8acd58927a614e006641384f61f8e7fca1cc68fc
Tree-SHA512: 936f275f29a700d630bb479b5283e47b66f2df76d8b8c053f594e6aedf783cc98a29c924c3a46613f112dfc884acb50f21a0b18f96d939e887b12b921ef2e10f
d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad Add window final block height to getchaintxstats (Jonathan "Duke" Leto)
Pull request description:
This patch is motivated by the desire to make the output of `getchaintxstats` more useful and optimized for applications to consume and render the data.
Firstly, this data is already available to the RPC, no additional work is done. Currently additional RPC calls will be needed to look up the height of the final block in the window or the block height that began the window.
By adding the block height of the final block in the window, the JSON is "self-contained" and applications can calculate the exact block height range of the window with no additional RPC requests.
For example, a web application which wants to render historical information for `getchaintxstats` RPC on various window sizes might call the RPC with various window lengths, once per day, and store the JSON results somewhere. Because the final block height of each dataset is included, it's no extra work to determine the exact block window range of each JSON response.
ACKs for top commit:
promag:
ACK d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad.
Tree-SHA512: fd4952c125f81a4ad18f7c78498c6b3e265b93cb574832166ac25596321ce84957f971f3f78f37d7e42638dc65f2a5d4d760f289873c9c2f2a82eb00a0f87c3f
a47df13471e3168e2e02023fb20cdf2414141b36 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar)
4433ed0f730cfd60eeba3694ff3c283ce2c0c8ee [validation] Crash if disconnecting a block fails (Suhas Daftuar)
Pull request description:
If we're unable to disconnect a block during normal operation, then that is a
failure of our local system (such as disk failure) or the chain that we are on
(eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that
we're trying to validate.
We should abort rather than stay on a less work chain.
Fixes#14341.
ACKs for top commit:
practicalswift:
utACK a47df13471e3168e2e02023fb20cdf2414141b36
TheBlueMatt:
utACK a47df13471e3168e2e02023fb20cdf2414141b36. Didn't bother to review the test in detail, it looked fine. Debated whether invalidateblock should ever crash the node, but *not* crashing in the case of hitting a pruned block (which is the only change here) is clearly better, even if there are other cases I'd argue we should crash in.
ryanofsky:
utACK a47df13471e3168e2e02023fb20cdf2414141b36. Only change since last review is new comment.
promag:
ACK a47df1347, it takes awhile to quit (RPC connection timeouts) but that's unrelated - hope to fix that soon.
fanquake:
ACK a47df13471e3168e2e02023fb20cdf2414141b36
Tree-SHA512: 4dec8cef6e7dbbe513c138fc5821a7ceab855e603ece3c16185b51a3830ab7ebbc844a28827bf64e75326f45325991dcb672f13bd7baede53304f27289c4af8d
e142ee03e7a139168aa1dbf5910c616f60d25042 doc: describe how to pass wildcard names to test runner (Jon Atack)
6a7a70b8cf05a82737c72020fd2b0eebc97cb5e4 test: enable passing wildcards with path to test runner (Jon Atack)
Pull request description:
Currently, passing wildcard testname args to the test runner from outside the test/functional/ directory does not work, even though developers expect it to. See these recent IRC discussions for more background: http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-10.html#l-262 (lines 262 to 323) and http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-11.html#l-134.
1. [BUGFIX] Enable passing wildcards with paths. Examples:
- `test/functional/test_runner.py test/functional/wallet*`
- `functional/test_runner.py functional/wallet*`
- `test/functional/test_runner.py ./test/functional/tool* test/functional/mempool*`
- A current limitation this PR does not change: 9 test files with arguments in their filename are not picked up by wildcard search.
2. [Docs] Describe how to pass wildcard names (multiple and with paths) to the test runner in test/README.md.
ACKs for top commit:
jnewbery:
tested ACK e142ee03e7a139168aa1dbf5910c616f60d25042
jachiang:
Tested ACK e142ee03e7. Thanks a lot for this fix!
MarcoFalke:
ACK e142ee03e7a139168aa1dbf5910c616f60d25042, fine with me
Tree-SHA512: cb3d994880cdc9b8918546b573a25faa5b4c7339826ac7cfe20f076aac6e731a34271609c0cf5a7ee5e4a2d5ae205298319d24bf36ef5b5d569a1a0c57883e54
8f250ab7882a852f1b1947cef4837d2de5ca6913 TEST: Replace hard-coded hex tx with classes (Steven Roose)
Pull request description:
Came across these breaking Elements.
ACKs for top commit:
MarcoFalke:
ACK 8f250ab7882a852f1b1947cef4837d2de5ca6913
instagibbs:
utACK 8f250ab788
Tree-SHA512: e8615dad4cda0beea4b0c7d4951a467fb9882a0a64d49c9b5ecf167369ea62a3fe5348e2401153162b0ccadecdb128492c94be36ebb881c3c42659626d86eda8
9dcb6763fb [qa] Use correct python index slices in example test (Suhas Daftuar)
Pull request description:
There's an off-by-one in the list indices used in example_test.py.
Tree-SHA512: d75b77c1e0b3931d02dfa043da4cb6fe8e62864a73717ce5c184d9dbeb25579342c6365cc7bbcc7c4382d76a320a528bf3c69107854dfc6fa704133d0ba11012
# Conflicts:
# test/functional/example_test.py
fa3e874d69 test: pruning: Check that verifychain can be called when pruned (MarcoFalke)
Pull request description:
Tree-SHA512: b921e550fdad27b791665883ed9fc50210575b0f968840a45284ad0e060ddd443ea7b9b12c17916f68ad28877c58ac51a6acc76ae4ea71bf7bfb2f744b286884
5c40e7b91a test: allows test_runner command line to receive parameters for each test (marcoagner)
Pull request description:
Fixes#14791.
Seems to address the asked behaviour in a simple way, but could address more if a more complex behaviour is found to be needed (e.g. call `rpc_bind --ipv4` without the ".py" and have it added).
Tree-SHA512: d7e21dd0ada36dd97dac71196bb97702a92986b181beb3753e37e3294b899fb65129aff5f9a45fe92b06fbf7c74e605ccb87a422f874f7a85d30401f4c2228c8
68400d8b96 tests: Use explicit imports (practicalswift)
Pull request description:
Enable automatic detection of undefined names in Python tests scripts. Remove wildcard imports.
Wildcard imports make it unclear which names are present in the namespace, confusing both readers and many automated tools.
An additional benefit of not using wildcard imports in tests scripts is that readers of a test script then can infer the rough testing scope just by looking at the imports.
Before this commit:
```
$ contrib/devtools/lint-python.sh | head -10
./test/functional/feature_rbf.py:8:1: F403 'from test_framework.util import *' used; unable to detect undefined names
./test/functional/feature_rbf.py:9:1: F403 'from test_framework.script import *' used; unable to detect undefined names
./test/functional/feature_rbf.py:10:1: F403 'from test_framework.mininode import *' used; unable to detect undefined names
./test/functional/feature_rbf.py:15:12: F405 bytes_to_hex_str may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:17:58: F405 CScript may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:25:13: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:26:31: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:26:60: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:30:41: F405 satoshi_round may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
./test/functional/feature_rbf.py:30:68: F405 COIN may be undefined, or defined from star imports: test_framework.mininode, test_framework.script, test_framework.util
$
```
After this commit:
```
$ contrib/devtools/lint-python.sh | head -10
$
```
Tree-SHA512: 3f826d39cffb6438388e5efcb20a9622ff8238247e882d68f7b38609877421b2a8e10e9229575f8eb6a8fa42dec4256986692e92922c86171f750a0e887438d9
4b7091a842 Replace median fee rate with feerate percentiles (Marcin Jachymiak)
Pull request description:
Currently, the `medianfeerate` statistic is calculated from the feerate of the middle transaction of a list of transactions sorted by feerate.
This PR instead uses the value of the 50th percentile weight unit in the block, and also calculates the feerate at the 10th, 25th, 75th, and 90th percentiles. This more accurately corresponds with what is generally meant by median feerate.
Tree-SHA512: 59255e243df90d7afbe69839408c58c9723884b8ab82c66dc24a769e89c6d539db1905374a3f025ff28272fb25a0b90e92d8101103e39a6d9c0d60423a596714
5654efb187d24eb29a343c720e3937b01457c8b7 Ported usage of deprecated optparse module to argparse module (Kvaciral)
Pull request description:
The optparse module is deprecated since Python 2,7/3.2 . Recommend usage of the argparse module which improves upon optparse.
Tree-SHA512: ffd0e3e6f3babef1675226b107eeb7a6bab6e5199de572703da9d94e1f69c70d1c9abc353e9664b40670bb4976c06964bb2606deee52f5dfcc619f336ceb8cf8
fafe73a626 qa: Raise feature_help timeout to 5s (MarcoFalke)
faabd7bc47 qa: Use files for stdout/stderr to support Windows (MarcoFalke)
facb56ffaf qa: Run gen_rpcauth with sys.executable (MarcoFalke)
fada8966c5 qa: Close stdout and stderr file when node stops (MarcoFalke)
Pull request description:
### qa: Close stdout and stderr file when node stops
Since these files are potentially deleted by the test framework for cleanup, they should be closed first. Otherwise this will lead to errors on Windows when the tests finish successfully.
Side note: After the patch, it is no longer possible to reopen the file on Windows (see https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)
### qa: Run gen_rpcauth with sys.executable
Similar to `test_runner.py`, the `sys.executable` needs to be passed down into subprocesses to pass on native Windows. (Should have no effect on Linux)
### qa: Use files for stdout/stderr to support Windows
It seems that using PIPE is not supported on Windows. Also, it is easier to just use the files that capture the stdout and stderr within the test node class.
Tree-SHA512: ec675012b10705978606b7fcbdb287c39a8e6e3732aae2fa4041d963a3c6993c6eac6a9a3cbd5479514e7d8017fe74c12235d1ed6fed2e8af8f3c71981e91864
fa85c985ed qa: Add p2p_invalid_locator test (MarcoFalke)
Pull request description:
Should not be merged *before* #13907
Tree-SHA512: a67ca407854c421ed20a184d0b0dc90085aed3e3431d9652a107fa3022244767e67f67e50449b7e95721f56906836b134615875f28a21e8a012eb22cfe6a66a5
fa5587fe71 qa: wait_for_verack by default (MarcoFalke)
Pull request description:
This removes the need to do so manually every time a connection is added.
Tree-SHA512: a46c92cb4df41e30778b42b9fd3dcbd8d2d82aa7503d1213cb1c1165034f648d8caee01c292e2d87d05b0f71696996eef5be8a753f35ab49e5f66b0e3bf29f21
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
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
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
241f8b5de4 Fix typo in feature_blocksdir.py log message (Alexander Leishman)
Pull request description:
Typo I came across while writing some new tests.
Tree-SHA512: cc494553125a1e84f9238a14761e3fb76623e98d951811dd3bfb13595a03a1888d73859487a2cbb76c7ae85897bc64016a220a92c2636b35ea6356a5b5340d66
# Conflicts:
# test/functional/feature_blocksdir.py
a1a998cf24c0cf1232e44ec8eaf2ad6875ab5153 wallet: Fix backupwallet for multiwallets (Daniel Kraft)
Pull request description:
`backupwallet` was broken for multiwallets in their own directories (i.e. something like `DATADIR/wallets/mywallet/wallet.dat`). In this case, the backup would use `DATADIR/wallets/wallet.dat` as source file and not take the specific wallet's directory into account.
This led to either an error during the backup (if the wrong source file was not present) or would silently back up the wrong wallet; especially the latter behaviour can be quite bad for users.
Tree-SHA512: 7efe2450ca047e40719fcc7cc211ed94699056020ac737cada7b59e8240298675960570c45079add424d0aab520437d5050d956acd695a9c2452dd4317b4d2c4
5e1777777790e855a9f3c8604208bc9bd6c8c99f qa: Create unicode tempdir in test_runner (MarcoFalke)
Pull request description:
Now that wallet filenames are properly quoted when used for rpc (#13823), we can add some unicode symbols to the test_runner path. Thus, the "extern" wallet that uses a full path has a unicode symbol in its name.
Should add unicode coverage to
* `listwallets`
* `wallet.getwalletinfo`
* `(un)loadwallet`
Tree-SHA512: 1633fde56f8748df0cfef9c31a878c105dfaac85d1041b292261f44c4d40e96942aacbf7d6e839e8bbf979dc131d81c24ceb521e927fc8a5a71ba093f36b891b
fa5b440971a0dfdd64c1b86748a573fcd7dc65d3 qa: Extract rpc_timewait as test param (MarcoFalke)
Pull request description:
Also increase it for wallet_dump and wallet_groups
Tree-SHA512: 7367bc584228bda3010c453713a1505c54a8ef3d116be47dab9934d30594089dfeb27ffa862f7517fd0ec8b5dc07f4904d67ef2a53dd284cbe2a58982e410e2b
a13647b8bd [qa] Add test for too-large wallet output groups (Suhas Daftuar)
57ec1c97b2 [wallet] correctly limit output group size (Suhas Daftuar)
Pull request description:
Also add a test to ensure that output groups are being limited, even if a wallet has many outputs corresponding to the same scriptPubKey (the test fails without the first commit).
Tree-SHA512: 2aaa82005b0910488f5cbf40690d4c5e2f46949e299ef70b4cb6e440713811443d411dcbc6d71b1701fd82423073125e21747787d70830cd021c841afb732d51
247d5740d2 Ignore unknown config file options for now (Pieter Wuille)
04ce0d88ca Report when unknown config file options are ignored (Pieter Wuille)
Pull request description:
As reported by @satwo on IRC a few days ago, the current mechanism of treating unknown config file options as errors is problematic for options like `-rpcclienttimeout` which aren't defined for `bitcoind`.
A full solution would be to either make all binaries be aware of each other's options, or to permit config file options that only apply to specific binaries (`bitcoind`, `bitcoin-qt`, `bitcoin-cli`). Both of these seem too invasive to introduce for 0.17.
As a compromise, this PR makes it ignores those options, but still warn about it in the log file.
Tree-SHA512: dfddc771b91df3031a9c98d9f3292f8f4fcd1b97ebb7317b2f457e12d9f205dc63f42721302e7258dbb53f273d7cc041a65a0a9120972769555784e1f1cc9aef
64b9f27e0e Skip is_closing() check when not available. (Daniel Kraft)
Pull request description:
#13715 introduced a new check for `_transport.is_closing()` in mininode's `P2PConnection`'s. This function is [only available from Python 3.4.4](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing), though, while Bitcoin Core is supposed to support all Python 3.4 versions.
In this change, we make the check conditional on `is_closing` being available. If it is not, then we revert to the behaviour before the check was introduced; this means that #13579 is not fixed for old systems, but at least the tests work as they used to do before.
This includes a small refactoring from a one-line lambda to an inline function, because this makes the code easier to read with more and more conditions being added.
Fixes#13745.
Tree-SHA512: 15be03b8b49c40a946c5b354c5974858d14dc46283ad48ee25d9e269377077ce741c6b379b3f6581ab981cb65be799809afbb99da278caaa2d8d870fa4fb748f
5b82aa7352 Fix bitcoin-cli --version (Ben Woosley)
Pull request description:
By declaring the relevant option, as it is in init.cpp
2dc5ab6378/src/init.cpp (L356)
Note contrib/devtools/gen-manpages.sh relies on this version information.
Tree-SHA512: 64c1b9635e86fea42b797471f66f784ed0c7ad4d6a454a49eb433018d8936265624fa4edf2f138efe3ff938af61763bd473b7b86d558f5288687b25f59fa87ea
a3fa4d6a6acf19d640a1d5879a00aa1f059e2380 QA: Fix bug in -usecli logic that converts booleans to non-lowercase strings (Jonas Schnelli)
4704e5f074e57782d058404a594a7313cf170cf0 [QA] add createwallet disableprivatekey test (Jonas Schnelli)
c7b8f343e99d9d53ea353ddce9a977f1886caf30 [Qt] Disable creating receive addresses when private keys are disabled (Jonas Schnelli)
2f15c2bc20d583b4c1788da78c9c635c36e03ed0 Add disable privatekeys option to createwallet (Jonas Schnelli)
cebefba0855cee7fbcb9474b34e6779369e8e9ce Add option to disable private keys during internal wallet creation (Jonas Schnelli)
9995a602a639b64a749545b7c3bafbf67f97324f Add facility to store wallet flags (64 bits) (Jonas Schnelli)
Pull request description:
This mode ('createwallet {"disableprivatekeys": true}') is intended for a sane pure watch-only mode, ideal for a use-case where one likes to use Bitcoin-Core in conjunction with a hardware-wallet or another solutions for cold-storage.
Since we have support for custom change addresses in `fundrawtransaction`, pure watch-only wallets including coin-selection are possible and do make sense for some use cases.
This new mode disables all forms of private key generation and ensure that no mix between hot and cold keys are possible.
Tree-SHA512: 3ebe7e8d54c4d4e5f790c348d4c292d456f573960a5b04d69ca5ef43a9217c7e7671761c6968cdc56f9a8bc235f3badd358576651af9f10855a0eb731f3fc508
ea5340c9d2 tests: fixes mininode's P2PConnection sending messages on closing transport (marcoagner)
Pull request description:
Fixes#13579.
I think one possible solution is to check for [`_transport.is_closing()`](https://docs.python.org/3.4/library/asyncio-protocol.html#asyncio.BaseTransport.is_closing) in the lambda before sending a message (compatible with Python 3.4 too). Let me know if I missed any side effects this introduces.
Tree-SHA512: cab46f81dccfec7b4460fda478a617845564520694449a9e85bf8a5f1e75f35f52cafd7c64966712c3d6c29956344d5a9dbad8851424f061eb3748bc621b900b
fa87da2f172ae2e6dc15e9ed156a3564a8ecfbdd qa: Avoid start/stop of the network thread mid-test (MarcoFalke)
Pull request description:
This simplifies test writing by removing the need to handle the network thread in tests. E.g. start thread, join thread, restart thread mid-test, adding p2p connections at the "right" time, ...
Tree-SHA512: 533642f12fef5496f1933855edcdab1a7ed901d088d34911749cd0f9e044c8a6cb1f89985ac3a7f41a512943663e4e270a61978f6f072143ae050cd102d4eab8
98ea64cf232c34d4b1aebe738b3956191667cd76 Let wallet importmulti RPC accept labels for standard scriptPubKeys (Russell Yanofsky)
Pull request description:
Allow importmulti RPC to apply address labels when importing standard scriptPubKeys. This makes the importmulti RPC less finnicky about import formats and also simpler internally.
Tree-SHA512: 102426b21239f1fa5f38162dc3f4145572caef76e63906afd786b7aff1670d6cd93456f8d85f737588eedc49c11bef2e1e8019b8b2cbf6097c77b3501b0cab1f
6af6d9b23d test: Add tests for RPC help (João Barbosa)
Pull request description:
At the moment the new test checks for:
- invalid usages
- expected output for unknown command
- current RPC command titles (derived from command categories) — this prevents adding wrong RPC categories and new categories must be added to the test
Tree-SHA512: f987535d001b1cd300656588602b1634099ea68a1dd2282180c30fa56caf7f990be9e2dc86c7431dfcf7fd686d0299a8d4935df178a2c9f0fb6fbebcba748eb5
16e288acdd61fa5fa5e39f3936fb50499f82c085 test padding non micro timestamps (John Newbery)
995dd89d884bda3fb5ca1885c5887d989cd2cad3 [Tests] Make combine_logs.py handle multi-line logs (John Newbery)
Pull request description:
combine_logs.py currently inserts additional newlines into multi-line
log messages, and doesn't color them properly. Fix both of those.
Tree-SHA512: dbe2f3ecc7cfbc95ee4350e648d127538c79cb6555257d4aeec12fe3d159366742b68e90e620c8ed7219a44b973395c7e5929ba374fae115fbee25560db645f6
8845c8aea65897637c330f5893461c0da180eaf8 tests: Replace usage of tostring() with tobytes() (Carl Dong)
Pull request description:
tostring() is deprecated as of python 3.7 and results in stderr output
causing tests to fail
Tree-SHA512: 8c5bbd6c6127490922add98543ee7719d19e11200e081784adef2f026ddf90d7735da7d0fb41fa4307d0d3450a27e126752c2b01cbd79b0c8a695855aed080ac
4bd125fff0 tests: Print dots by default (Chun Kuan Lee)
Pull request description:
In cron job (https://travis-ci.org/bitcoin/bitcoin/builds/445823485), the functional tests would fail due to silent for 10 mins.
After applying this patch, we con't see any extra characters printed on screen but also avoid timeout (https://travis-ci.org/ken2812221/bitcoin/builds/445981698)
Tree-SHA512: c0412e171a451b27f9734311c7f063ad3fd7142087ed1e3786b4f303acaebc043f970523d6c2d4ef57ec5857040e2b6f7fd6345304353e7805d76044d317344d
# Conflicts:
# test/functional/test_runner.py
1cdb9bb51f minor p2p_sendheaders fix of height in coinbase (Gregory Sanders)
Pull request description:
> \# Now announce a header that forks the last two blocks
Doesn't effect any behavior since BIP34 isn't active in regtest for many blocks.
Tree-SHA512: 3f214b956a94250bb640f63b6ff707930e1d4cb8df1bbf0fef4012d89a94bafbde0d7b42bbe7113ec33810169281c22c6e389445921d99decb74aa56e87a0f27
38040c34e1 [tests] Remove accounts from wallet_importprunedfunds.py (John Newbery)
Pull request description:
This was split from #13075 to not block review/merge of that PR.
Tree-SHA512: 631d7139ed2bda5222ec395cc75720261e2e1f741dba04723d09fe04ef6cf92222a3679d886026ec33e2db2d1e2fa1a0f36c2451581d0f733a9939a98c7118ab
f618c58b75 Docs: Update python docs to reflect that wildcard imports are disallowed (Ben Woosley)
Pull request description:
These have been disallowed via flake8 since: #13054
Tree-SHA512: f41651fd883e3786a7e87c4aa4c54749308e576287f2f88da3f1d8d0f59e14519d99061f1efd05b8745f495f87a14286cea576f1507d10ccd226f8cf2f2b3cc8
42ff30ec6 [Docs] add short documentation for /rest/blockhashbyheight (Jonas Schnelli)
579d418f7 [QA] add rest tests for /rest/blockhashbyheight/<HEIGHT>.<FORMAT> (Jonas Schnelli)
eb9ef04c4 REST: add "blockhashbyheight" call, fetch blockhash by height (Jonas Schnelli)
Pull request description:
Completes the REST interface for trivial block exploring by adding a call that allows to fetch the blockhash in the main chain by a given height.
Tree-SHA512: 94be9e56718f857279b11cc16dfa8d04f3b5a762e87ae54281b4d87247c71c844895f4944d5a47f09056bf851f4c4761ac4fbdbaaee957265d14de5c1c73e8d2
7cf994d5cfd53dcff76ebd0e0007e3477a7570e8 qa: Improve tests of /rest/headers and /rest/block (João Barbosa)
0825b86b280c684c32c60bac9e862298c7279f27 doc: /rest/block responds with 404 if block does not exist (João Barbosa)
be625f7c5562afed517ff51d2d85268ba5ce6017 doc: Explain empty result of /rest/headers (João Barbosa)
Pull request description:
Follow up of #15107.
Tree-SHA512: a7fdeed05216e3eda9604664db529237c2d0ddf422cfac139d6345a22b6e00bfe870d4e3f177423db7d4efb295ac2dc0ca2eb20c9c27c0719b89fd5428860d03
e4a0c3547ed886871f8b3d51c6b4ffdb181a8b9c Improve blocksdir functional test. (Hennadii Stepanov)
c3f1821ac788e522e7558e3575150433450dcb8c Make blockdir always net specific (Hennadii Stepanov)
Pull request description:
The blocks directory is net specific by definition.
Also this prevents the side effect of calling `GetBlocksDir(false)` in the non-mainnet environment.
Currently a new node creates an unused `blocks\` directory in the root of the data directory when `-testnet` or `-regtest` is specified.
Refs:
- #12653
- https://github.com/bitcoin/bitcoin/pull/12653#discussion_r174784834 by @laanwj
- https://github.com/bitcoin/bitcoin/issues/14595#issuecomment-436011186
Tree-SHA512: c9957a68a4a200ebd2010823a56db7e61563afedcb7c9828e86b13f3af2990e07854b622c1f3374756f94574acb3ea32de7d2a399eef6c0623f0e11265155627
c9066f07c94e3610f7762d6406851cd135790511 Allow running rpc_bind.py --nonloopback test without IPv6 (Kristaps Kaupe)
Pull request description:
Don't see a reason why this can't be tested with IPv4 only.
Tree-SHA512: 515bdf700fad420e4b1798fd4978b53e2da3ddb26e43b16d68b43071bc912c325f1ceb10046ba3d0494dab289a53c45ddc2de9064117d8c1d6bf11e88323f490
fa6edfef358518022ee86c0abc77c1c068f106a3 qa: Remove portseed_offset from test runner (MarcoFalke)
Pull request description:
The portseed_offset is no longer needed in the test runner, since we already kill leftover processes (see #12904). This "fixes" #10869 because we deterministically pick ports starting at 11000
Tree-SHA512: 1ee22e19e02acd3afadc7c6a2b391fd3b5cfcec22c0fe194f3207251e7b1264a04e47d90a3ff8be4aca7d0ec33219a2f5855076acb3565291767939bc2f2fa17
fa4760fbb3f1099dcd3c43ebc53c2a761a2170e8 qa: Increase includeconf test coverage (MarcoFalke)
Pull request description:
This adds some missing `return false` for error conditions and adds test coverage [1] for those.
Also, extend recursion warning when the chain was set in one of the includeconfs.
[1] See the red lines in https://marcofalke.github.io/btc_cov/total.coverage/src/util.cpp.gcov.html for missing coverage.
Tree-SHA512: d32563c9bb277879895a173e699034db5ecdb4061a1ec8890c566d61e36a09efa5eda19a029baf952ff6d568f8b9684a13a0bb90827850075470975e2088fee4
fa26cf015658ac2aa52b5e5656e38af9a12160cc qa: Fixup setting of PATH env var (MarcoFalke)
Pull request description:
This was an oversight of mine in #13188
Can be trivially tested with `BITCOIND=bitcoin-qt ./test/functional/wallet_disable.py` before and after this fix.
Tree-SHA512: 06c7b2f12158855eb2b6392861943821bd7ad3152cf0dd49ac4abd878e5b937ebee55e256ce5bdc1c2a9c775a452112c34533366c934ff5f0f412b3a7e1c8118
c2dfbb4a97 Add unavailable options to hidden options category (Andrew Chow)
Pull request description:
From IRC:
```
<ossifrage> FYI, bitcoin-qt from the head I built today won't start if you have "daemon=0" in the config file, so you can't use the same config for either bitcoind or bitcoin-qt
<ossifrage> Seems like bitcoin-qt should ignore this option?
<provoostenator> ossifrage: probably caused by 13112. Another problem is disablewallet=1 will prevent a launch if you compile bitcoind without wallet. It probably needs to be relaxed slightly.
```
Adds all of the options that are unavailable due to compiling options to the hidden category so that shared config files do not break with the alternative binaries.
Tree-SHA512: 1ef43f5f7ad46ecc2865d22ee683ef22831e8f131ec99b732bb36d90381f7964bf64829595e993c2d435823fe4425a20323c8e65307cf2463a9e40b8049ab559
903055730b Test gArgs erroring on unknown args (Andrew Chow)
4f8704d57f Give an error and exit if there are unknown parameters (Andrew Chow)
174f7c8080 Use a struct for arguments and nested map for categories (Andrew Chow)
Pull request description:
Following #13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist.
Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior.
Closes#1044
Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7
6249021d1 [docs] Add release notes for HD master key -> HD seed rename (John Newbery)
79053a5f2 [rpc] [wallet] Add 'hdmasterkeyid' alias return values. (John Newbery)
c75c35141 [refactor] manually change remaining instances of master key to seed. (John Newbery)
131d4450b scripted-diff: Rename master key to seed (John Newbery)
Pull request description:
Addresses #12084 and #8684
This renames a couple of functions and members (no functional changes, expect log prints):
- Rename CKey::SetMaster to CKey::SetSeed
- Rename CHDChain::masterKeyId to CHDChain::seedID
- Rename CHDChain::hdMasterKeyID to CHDChain::hdSeedID
- Rename CWallet::GenerateNewHDMasterKey to CWallet::GenerateNewHDSeed
- Rename CWallet::SetHDMasterKey to CWallet::SetHDSeed
As well it introduces a tiny API change:
- RPC API change: Rename "hdmasterkeyid" to "hdseedid", rename "hdmaster" in wallet-dump output to "hdseed"
Fixes also a bug:
- Bugfix: use "s" instead of the incorrect "m" for the seed-key hd-keypath key metadata
Tree-SHA512: c913252636f213135a3b64df5de5d21844fb9c2d646567c1aad0ec65745188587de26119de99492c67e559bd49fdd9606b54276f00dddb84301785beba58f281
2352aa9 test: Ensure that recursive -includeconf produces appropriate warnings (Karl-Johan Alm)
c5bcc7d util: warn about recursive -includeconf arguments in configuration files (Karl-Johan Alm)
Pull request description:
This is a follow-up PR to #10267, and addresses https://github.com/bitcoin/bitcoin/pull/10267#issuecomment-387546144.
~~I am adding extra work for @jnewbery in #12755 here -- maybe I should just rebase on top of that, but not sure what the appropriate approach is here.~~
Tree-SHA512: 87f0c32436b70424e33616ffb88d7cb699f90d6a583a10237e224b28fc936d6a9df95536c8c52ee8546b3942da92b2a357e61bf87e00d1462bc10d46d3bee352
beee49b [tests] Allow stderr to be tested against specified string (John Newbery)
e503671 [Tests] Use LIBC_FATAL_STDERR_=1 in tests (John Newbery)
c22ce8a [Tests] Write stdout/stderr to datadir instead of temp file. (John Newbery)
Pull request description:
**Due to a merge conflict, this is now based on #10267. Please review that PR first!**
Subset of #12379 now that parts of that PR have been merged.
#12362 was only observed when running the functional tests locally because:
- by defatul libc logs to `/dev/tty` instead of stderr
- the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail.
This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr:
- commit *Write stdout/stderr to datadir instead of temp file* writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure.
- commit *Use LIBC_FATAL_STDERR=1 in tests* ensures that libc failures are logged to stderr instead of the terminal.
commit *Assert that bitcoind stdout is empty on shutdown* asserts that stderr is empty on bitcoind shutdown.
Tree-SHA512: 21111030e667b3b686f2a7625c2b625ebcfb6998e1cccb4f3932e8b5d21fb514b19a73ac971595d049343430e9a63155986a7f5648cad55b8f36f3c58b1c7048
232f96f5c8a3920c09db92f4dbac2ad7d10ce8cf doc: Add release notes for -avoidpartialspends (Karl-Johan Alm)
e00b4699cc6d2ee5697d38dd6607eb2631c9b77a clean-up: Remove no longer used ivars from CInputCoin (Karl-Johan Alm)
43e04d13b1ffc02b1082176e87f420198b40c7b1 wallet: Remove deprecated OutputEligibleForSpending (Karl-Johan Alm)
0128121101fb3ee82f3abd3973a967a4226ffe0e test: Add basic testing for wallet groups (Karl-Johan Alm)
59d6f7b4e2f847ec1f2ff46c84e6157655984f85 wallet: Switch to using output groups instead of coins in coin selection (Karl-Johan Alm)
87ebce25d66952f5ce565bb5130dcf5e24049872 wallet: Add output grouping (Karl-Johan Alm)
bb629cb9dc567cc819724d9f4852652926e60cbf Add -avoidpartialspends and m_avoid_partial_spends (Karl-Johan Alm)
65b3eda458221644616d0fdd6ba0fe01bdbce893 wallet: Add input bytes to CInputCoin (Karl-Johan Alm)
a443d7a0ca333b0bae63e04b5d476f9ad9c7aeac moveonly: CoinElegibilityFilter into coinselection.h (Karl-Johan Alm)
173e18a289088c6087ba6fac708e322aa63b7a94 utils: Add insert() convenience templates (Karl-Johan Alm)
Pull request description:
This PR adds an optional (off by default) `-avoidpartialspends` flag, which changes coin select to use output groups rather than outputs, where each output group corresponds to all outputs with the same destination.
It is a privacy improvement, as each time you spend some output, any other output that is publicly associated with the destination (address) will also be spent at the same time, at the cost of fee increase for cases where coin select without group restriction would find a more optimal set of coins (see example below).
For regular use without address reuse, this PR should have no effect on the user experience whatsoever; it only affects users who, for some reason, have multiple outputs with the same destination (i.e. address reuse).
Nodes with this turned off will still try to avoid partial spending, if the fee of the resulting transaction is not greater than the fee of the original transaction.
Example: a node has four outputs linked to two addresses `A` and `B`:
* 1.0 btc to `A`
* 0.5 btc to `A`
* 1.0 btc to `B`
* 0.5 btc to `B`
The node sends 0.2 btc to `C`. Without `-avoidpartialspends`, the following coin selection will occur:
* 0.5 btc to `A` or `B` is picked
* 0.2 btc is output to `C`
* 0.3 - fee is output to (unique change address)
With `-avoidpartialspends`, the following will instead happen:
* Both of (0.5, 1.0) btc to `A` or `B` is picked (one or the other pair)
* 0.2 btc is output to `C`
* 1.3 - fee is output to (unique change address)
As noted, the pro here is that, assuming nobody sends to the address after you spend from it, you will only ever use one address once. The con is that the transaction becomes slightly larger in this case, because it is overpicking outputs to adhere to the no partial spending rule.
This complements #10386, in particular it addresses @luke-jr and @gmaxwell's concerns in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-300667926 and https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381.
Together with `-avoidreuse`, this fully addresses the concerns in #10065 I believe.
Tree-SHA512: 24687a4490ba59cf4198ed90052944ff4996653a4257833bb52ed24d058b3e924800c9b3790aeb6be6385b653b49e304453e5d7ff960e64c682fc23bfc447621
# Conflicts:
# src/Makefile.am
# src/bench/coin_selection.cpp
# src/wallet/coincontrol.h
# src/wallet/coinselection.cpp
# src/wallet/coinselection.h
# src/wallet/init.cpp
# src/wallet/test/coinselector_tests.cpp
# src/wallet/wallet.cpp
# src/wallet/wallet.h
# test/functional/test_runner.py
d641c29a5a travis: Run feature_dbcrash functional tests in cron job (Chun Kuan Lee)
c0d947d725 tests: Reorder tests and move most of extended tests up to normal tests (Chun Kuan Lee)
Pull request description:
The travis should run almost all jobs unless it takes really long time, however it does not take too long for now. So it's time for moving it to normal job.
(The test sort is to see how many conflict will this cause, will drop it if there are too many)
The first commit can be reviewed by `git diff --color-moved=plain`
Tree-SHA512: db6bd5b1f19de2f729012adda6ed00ca989071fd40a20710c0ff2579b5bd008edcf7421c1ad56d5f0752354e7df408f58351129d35a1ab7f4a6caa9d315df2ec
# Conflicts:
# .travis/test_06_script.sh
# test/functional/test_runner.py
364bae5 qa: Pad scriptPubKeys to get minimum sized txs (MarcoFalke)
7485488 Policy to reject extremely small transactions (Johnson Lau)
0f8719b Add transaction tests for constant scriptCode (Johnson Lau)
9dabfe4 Add constant scriptCode policy in non-segwit scripts (Johnson Lau)
Pull request description:
This disables `OP_CODESEPARATOR` in non-segwit scripts (even in an unexecuted branch), and makes a positive `FindAndDelete` result invalid. This ensures that the `scriptCode` serialized in `SignatureHash` is always the same as the script passing to the `EvalScript`.
Tree-SHA512: a0552cb920294d130251c48053fa2ff1fbdd26332e62b52147d918837852750f0ce35ce2cd1cbdb86588943312f8154ccb4925e850dbb7c2254bc353070cd5f8
67d6ee1 remove redundant tests in p2p-segwit.py (Johnson Lau)
9260085 test segwit uncompressed key fixes (Johnson Lau)
248f3a7 Fix ismine and addwitnessaddress: no uncompressed keys in segwit (Pieter Wuille)
b811124 [qa] Add tests for uncompressed pubkeys in segwit (Suhas Daftuar)
9f0397a Make test framework produce lowS signatures (Johnson Lau)
4c0c25a Require compressed keys in segwit as policy and disable signing with uncompressed keys for segwit scripts (Johnson Lau)
3ade2f6 Add standard limits for P2WSH with tests (Johnson Lau)
fac1e1f qa: Remove unused option --srcdir (MarcoFalke)
Pull request description:
The `srcdir` option was both unused and misleading; It should have been called `builddir`. So remove it.
Tree-SHA512: 2c24dcf2aa82219158b8cbbf03dd3f0f51f805f1f5f670faa1fd59e5a8d60fda120ffddadeccb058d8d3f20583b4952be7afd2df6bbefb9367d35c0f0a9fda3c
25b7ab9 doc: Add release notes for -includeconf (Karl-Johan Alm)
0f0badd test: Test includeconf parameter. (Karl-Johan Alm)
629ff8c -includeconf=<path> support in config handler, for including external configuration files (Karl-Johan Alm)
Pull request description:
Fixes: #10071.
Done:
- adds `-includeconf=<path>`, where `<path>` is relative to `datadir` or to the path of the file being read, if in a file
- protects against circular includes
- updates help docs
~~~Thoughts:~~~
- ~~~I am not sure how to test this in a neat manner. Feedback on this would be nice. Will dig/think though.~~~
Tree-SHA512: cb31f1b2f69fbc0890d264948eb2e501ac05cf12f5e06a5942f9c1539eb15ea8dc3cae817f4073aecb2fcc21d0386747f14f89d990772003a76e2a6d25642553
a533834d50 [tests] Fix flake8 warnings in several wallet functional tests (John Newbery)
Pull request description:
Fixes flake8 warnings in several wallet functional tests.
Several wallet functional tests need rewrite to remove the accounts API (#13075). To prepare for that, I fixed all the flake8 warnings in those tests.
#13075 is blocked on a bitcoind bug. This PR is just the flake8 fixes so we're not completely blocked.
Tree-SHA512: 2dc1d589b2f8f4318083a681e487532d0f8f3d57e8bc8f37b660b728ffc33329b88e9251eb223104aea89f293c3f4074ca700fe690e645617326b859da3e93c3
9b2704777c [doc] Include txindex changes in the release notes. (Jim Posen)
ed77dd6b30 [test] Simple unit test for TxIndex. (Jim Posen)
6d772a3d44 [rpc] Public interfaces to GetTransaction block until synced. (Jim Posen)
a03f804f2a [index] Move disk IO logic from GetTransaction to TxIndex::FindTx. (Jim Posen)
e0a3b80033 [validation] Replace tx index code in validation code with TxIndex. (Jim Posen)
8181db88f6 [init] Initialize and start TxIndex in init code. (Jim Posen)
f90c3a62f5 [index] TxIndex method to wait until caught up. (Jim Posen)
70d510d93c [index] Allow TxIndex sync thread to be interrupted. (Jim Posen)
94b4f8bbb9 [index] TxIndex initial sync thread. (Jim Posen)
34d68bf3a3 [index] Create new TxIndex class. (Jim Posen)
c88bcec93f [db] Migration for txindex data to new, separate database. (Jim Posen)
0cb8303241 [db] Create separate database for txindex. (Jim Posen)
Pull request description:
I'm re-opening #11857 as a new pull request because the last one stopped loading for people
-------------------------------
This refactors the tx index code to be in it's own class and get built concurrently with validation code. The main benefit is decoupling and moving the txindex into a separate DB. The primary motivation is to lay the groundwork for other indexers that might be desired (such as the [compact filters](https://github.com/bitcoin/bips/pull/636)). The basic idea is that the TxIndex spins up its own thread, which first syncs the txindex to the current block index, then once in sync the BlockConnected ValidationInterface hook writes new blocks.
### DB changes
At the suggestion of some other developers, the txindex has been split out into a separate database. A data migration runs at startup on any nodes with a legacy txindex. Currently the migration blocks node initialization until complete.
### Open questions
- Should the migration of txindex data from the old DB to the new DB block in init or should it happen in a background thread? The downside to backgrounding it is that `getrawtransaction` would return an error message saying the txindex is syncing while the migration is running.
### Impact
In a sample size n=1 test where I synced nodes from scratch, the average time [Index writing](https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L1903) was 3.36ms in master and 1.72ms in this branch. The average time between `UpdateTip` log lines for sequential blocks between 400,000 and IBD end on mainnet was 0.297204s in master and 0.286134s in this branch. Most likely this is just variance in IBD times, but I can try with some more trials if people want.
Tree-SHA512: 451fd7d95df89dfafceaa723cdf0f7b137615b531cf5c5035cfb54e9ccc2026cec5ac85edbcf71b7f4e2f102e36e9202b8b3a667e1504a9e1a9976ab1f0079c4
fac0db0 wallet: Make fee settings non-static members (MarcoFalke)
Pull request description:
The wallet header defined some globals (they were called "settings"), that should be class members instead.
This commit is hopefully only refactoring, apart from a multiwallet bugfix: Calling the rpc `settxfee` for one wallet, would set (and change) the fee rate for all loaded wallets. (See added test case)
Tree-SHA512: 4ab6ec2f5c714742396ded5e451ec3b1ceb771e3696492de29889d866de4365b3fbe4a2784d085c8b8bd11b1ebb8a1fec99ab2c62eee716791cfc67c0cf29e1b
