40fdb2a212d0a0e775114e4766d065e6d234c155 test: Fix Comment Typo in BitcoinTestFramework (Joel Klabo)
Pull request description:
Missing "override" in comment describing use of set_test_params
ACKs for top commit:
michaelfolkson:
ACK 40fdb2a212d0a0e775114e4766d065e6d234c155
Tree-SHA512: bf893a0d5f8dc86a3ec2eaf48cd7c0f0f832f3b3d254b3d99953336db7e294571b1d2c8686030bf8a27cbe67b1a85a54e53ebefb2e57d6d8d6ac864a15dce4e7
8b2f471a1bff753cc4df29805ef38c3623f64f6e qa: Fix double-negative arg test (Hennadii Stepanov)
Pull request description:
Commit 67518f7cc61bf59ddfa0fd7c8dbbdec3653b9556 tests do not catch that a pointer is returned instead of a value.
This PR makes test to not accept trailing characters after 0.
From [IRC](http://www.erisian.com.au/bitcoin-core-dev/log-2020-01-07.html#l-358):
> \<hebasto\> ryanofsky: hmm, why test/functional/feature_config_args.py passed on 67518f7cc61bf59ddfa0fd7c8dbbdec3653b9556 ?
> \<hebasto\> I see now: test is broken.
> \<ryanofsky\> test should be unaffected by that change, do you see a break somewhere?
> \<hebasto\> yes: "-connect=0x7fff50369968" != "-connect=0"
> ...
> \<ryanofsky\> Oh I see how that would happen, it should not be a problem in the current PR.
> \<hebasto\> going to submit a pr to fix test
> \<ryanofsky\> in the commit you mentioned, value is a pointer to a string, and it was printing the pointer address instead of the string on: LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key, value);
> \<hebasto\> correct
> \<ryanofsky\> oh I see, test could be fixed to more robust and not accept trailing characters after 0
ACKs for top commit:
ryanofsky:
Code review ACK 8b2f471a1bff753cc4df29805ef38c3623f64f6e. I don't know how you found this but it's a nice catch! This change should make the test more reliable.
Tree-SHA512: 454b3d4415771d353a2da766f6ae6e0bfae7bdf485aaa7bfdd323595282356eeaf3f40e556b39f753bc35f578cbe9684368887eef2d63c5d7f0d7d9fa971697a
8f2d7737cc236b6122f30e31856eb3181960fba1 test: add functional test for non-standard txs with too large scriptSig (Sebastian Falbesoner)
Pull request description:
Approaches another missing functional test of issue #17394 (counterpart to unit test in PR #17480, Commit 5e8a56348b): A transaction is rejected by the mempool with reason `"scriptsig-size"` if any of the inputs' scriptSig is larger than 1650 bytes.
ACKs for top commit:
MarcoFalke:
ACK 8f2d7737cc236b6122f30e31856eb3181960fba1
instagibbs:
ACK 8f2d7737cc
Tree-SHA512: 7a45b8a4181158be3e3b91756783ddf032f132ca8780dc35fac91b2df2149268f784d28ac56005135c4d86a357c57805c5a54b8155f0d049932844b18dc03992
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
