a2a04a5abb Bugfix: Only run bitcoin-tx tests when bitcoin-tx is enabled (Luke Dashjr)
92af71cea9 configure: Make it possible to build only one of bitcoin-cli or bitcoin-tx (Luke Dashjr)
Pull request description:
Includes #5618 (which the reasons for rejecting no longer hold true)
Tree-SHA512: f30a8e4a2f70166b7cabef77c4674163b3a9da14c6a547d34f00d1056a19bf4d23e22851eea726fad2afc8735d5473ae91122c770b65ac3886663dc20e2c5b70
When receiving an islock, propagate it as islock.
When creating/receiving and isdlock, propagate it as isdlock to peers which support it and as islock to peers which don't.
Functional tests to cover both islock and isdlock scenarios.
3d0a82cff8cbb809876e82dbe62d14d2adc07d94 devtools: Accomodate block-style copyright blocks (Ben Woosley)
0ef0e51fe4bb592e67255776b5a0ba04679fb8c4 lint: Bump flake8 to 3.7.8 (Ben Woosley)
838920704ad90a71cf288b700052503db8abb17e lint: Disable flake8 W504 warning (Ben Woosley)
b21680baf5391a602b295b9d7d0ef66553661cb9 test/contrib: Fix invalid escapes in regex strings (Ben Woosley)
Pull request description:
This is a second go at #15221, fixing new lints in:
W504 line break after binary operator
W605 invalid escape sequence
F841 local variable 'e' is assigned to but never used
This time around:
* One commit per rule, for easier review
* I went with the PEP-8 style of breaking before binary operators
* I looked into the raw regex newline issue, and found that raw strings with newlines embedded do work appropriately. E.g. run `re.match(r" \n ", " \n ")` to check this for yourself. `re.MULTILINE` exists to modify `^` and `$` in multiline scenarios, but all of these searches are per-line.
ACKs for top commit:
practicalswift:
ACK 3d0a82cff8cbb809876e82dbe62d14d2adc07d94 -- diff looks correct
Tree-SHA512: bea0c144cadd72e4adf2e9a4b4ee0535dd91a8e694206924cf8a389dc9253f364a717edfe9abda88108fbb67fda19b9e823f46822d7303c0aaa72e48909a6105
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
* partial Merge #14454: Add SegWit support to importmulti
c11875c5908a17314bb38caa911507dc6401ec49 Add segwit address tests for importmulti (MeshCollider)
201451b1ca3c6db3b13f9491a81db5b120b864bb Make getaddressinfo return solvability (MeshCollider)
1753d217ead7e2de35b3df6cd6573a1c9a068f84 Add release notes for importmulti segwit change (MeshCollider)
353c064596fc2e2c149987ac3b3c11b4c90c4d5f Fix typo in test_framework/blocktools (MeshCollider)
f6ed748cf045d7f0d9a49e15cc0c0001610b9231 Add SegWit support to importmulti with some ProcessImport cleanup (MeshCollider)
Pull request description:
Add support for segwit to importmulti, supports P2WSH, P2WPKH, P2SH-P2WPKH, P2SH-P2WSH. Adds a new `witnessscript` parameter which must be used for the witness scripts in the relevant situations.
Also includes some tests for the various import types.
~Also makes the change in #14019 redundant, but cherry-picks the test from that PR to test the behavior (@achow101).~
Fixes#12253, also addresses the second point in #12703, and fixes#14407
Tree-SHA512: 775a755c524d1c387a99acddd772f677d2073876b72403dcfb92c59f9b405ae13ceedcf4dbd2ee1d7a8db91c494f67ca137161032ee3a2071282eeb411be090a
# Conflicts:
# src/wallet/rpcdump.cpp
# test/functional/test_framework/blocktools.py
# test/functional/wallet_importmulti.py
* make linter happy
Signed-off-by: pasta <pasta@dashboost.org>
* Fixes: trivial + linter + add missing consistency check + more/redo 14679
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fab98992043f47fa7240d7c1217920d0c4f783a2 test: Try once more when RPC connection fails on Windows (MarcoFalke)
faa655731eac751d4eb494268e2c815493ba9382 test: Document why connection is re-constructed on windows (MarcoFalke)
fa9f4f663c36b0824406036445e5cff0a78174e9 test: Remove python 3.4 workaround (MarcoFalke)
fae760f2b24cb26494b65c0a7ac38b92ead345af cirrus: Bump freebsd to 12.1 (MarcoFalke)
Pull request description:
Fixes: #18548
ACKs for top commit:
hebasto:
ACK fab98992043f47fa7240d7c1217920d0c4f783a2, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: c4e9ed8d995b63a820ca66984f152ac216c83ba1f318b61b15c6d375c0e936c08f6bc3d38c255dddf3ee8952f848c7ababf684854e07a7c1b1d8504e6b7208ba
369244f654 utils: Fix broken Windows filelock (Chun Kuan Lee)
Pull request description:
Fix broken filelock on Windows, also add a test for this. It's a regression introduced by #13862.
Tree-SHA512: 15665b1930cf39ec71f3ab07def8e2897659f6fd4d2de749d63a5a8ec920e4a04282f12bc262f242b1b3d14d2dd9fa191ddbcf16a46fb927b5b2b14d9f6b5d01
faa4043c66 qa: Run more tests with wallet disabled (MarcoFalke)
Pull request description:
Instead of skipping the whole test, only skip the wallet specific section of a test if the wallet is not compiled in. This is mostly an indentation change, so can be reviewed with `--ignore-all-space`.
Tree-SHA512: 5941a8b6b00dca5cf9438c5f6f010ba812115188a69e427d7ade4c1ab8cfe7a57c73daf52c66235dbb24b1cd9ab7c7a17c49bc23d931e041b605d79116a71f66
Merge #14324: qa: Run more tests with wallet disabled
faa4043c66 qa: Run more tests with wallet disabled (MarcoFalke)
Pull request description:
Instead of skipping the whole test, only skip the wallet specific section of a test if the wallet is not compiled in. This is mostly an indentation change, so can be reviewed with `--ignore-all-space`.
Tree-SHA512: 5941a8b6b00dca5cf9438c5f6f010ba812115188a69e427d7ade4c1ab8cfe7a57c73daf52c66235dbb24b1cd9ab7c7a17c49bc23d931e041b605d79116a71f66
168b6c317ca054c1287c36be532964e861f44266 add dummy file param to fix jupyter (Josiah Baker)
Pull request description:
this fixes argparse to use `parse_known_args`. previously, if an unknown argument was passed, argparse would fail with an `unrecognized arguments: %s` error.
## why
the documentation mentions being able to run `TestShell` in a REPL interpreter or a jupyter notebook. when i tried to run inside a jupyter notebook, i got the following error:
this was due to the notebook passing the filename of the notebook as an argument. this is a known problem with notebooks and argparse, documented here: https://stackoverflow.com/questions/48796169/how-to-fix-ipykernel-launcher-py-error-unrecognized-arguments-in-jupyter
## testing
to test, make sure you have jupyter notebooks installed. you can do this by running:
```
pip install notebook
```
or following instructions from [here](https://jupyterlab.readthedocs.io/en/stable/getting_started/installation.html).
once installed, start a notebook (`jupyter notebook`), launch a python3 kernel and run the following snippet:
```python
import sys
# make sure this is the path for your system
sys.path.insert(0, "/path/to/bitcoin/test/functional")
from test_framework.test_shell import TestShell
test = TestShell().setup(num_nodes=2, setup_clean_chain=True)
```
you should see the following output, without errors:
if you are unfamiliar with notebooks, here is a short guide on using them: https://jupyter.readthedocs.io/en/latest/running.html
ACKs for top commit:
MarcoFalke:
review ACK 168b6c317ca054c1287c36be532964e861f44266
jamesob:
crACK 168b6c317c
practicalswift:
cr ACK 168b6c317ca054c1287c36be532964e861f44266
Tree-SHA512: 4fee1563bf64a1cf9009934182412446cde03badf2f19553b78ad2cb3ceb0e5e085a5db41ed440473494ac047f04641311ecbba3948761c6553d0ca4b54937b4
e1fdd2963baab68bb6a77af2ad7a07fcacd4e73e Test batch rpc with params (Gregory Sanders)
Pull request description:
Useful as an example and test case.
ACKs for top commit:
laanwj:
ACK e1fdd2963baab68bb6a77af2ad7a07fcacd4e73e
theStack:
ACK e1fdd2963baab68bb6a77af2ad7a07fcacd4e73e
Tree-SHA512: 2d2ba8960916342b264a14624857d6dd10005be12efafb3e970b82656f721c8f3700ebc9b8809de1b2f887d482b772043504aeaeebc7f2e1c8203f076a451526
ff94da7887 tests: Make appveyor run with --usecli (practicalswift)
db01839361 test: Add missing call to skip_if_no_cli() (practicalswift)
Pull request description:
Add missing call to `skip_if_no_cli()` as suggested by @MarcoFalke in #14365.
Tree-SHA512: b0a2ddfad0f81cc9544f63c4e490fb983d833a47c23522549d1200ea6a8a132b2cd4bf0d66b862ef3a548d8471128b80aea3525fb5dec65221e23f32a8d46746
c7b3e487f2 tests: exclude all tests with difference parameters (Chun Kuan Lee)
Pull request description:
Fix broken exclusion list in functional tests. See https://github.com/bitcoin/bitcoin/pull/14007#pullrequestreview-158309105
Tree-SHA512: b6c2b86fef13e3c00c695adaeeb3e47ee9b48877c71bc605d24201ce931b2ef3ae9f5f199071fa1ec5de2d7aadc478410094c380cc297922e683e9b2569cda03
0ca4c8b3c6 Changed functional tests which do not require wallets to run without (sanket1729)
Pull request description:
Addresses #14216 . Changed Changed `get_deterministic_priv_key()` to return named tuple`(address, key)`
I have tried to be exhaustive as possible in maximum coverage for non-wallet mode without affecting any coverage for wallet mode.
However, I could not check the tests in wallet mode because of timeout issues. Hopefully, travis job checks those.
Tests `feature_block.py`, `feature_logging.py` and `feature_reindex.py` were skipping despite having no direct dependency on any wallet functions. So, I have also disabled the `skip_test_no_wallet()` for those files too.
Tree-SHA512: 8f84bd8400a732d4266c7518d5cbcf1eb761f623a64a74849e0470142c8ef22cb75364474ddae75d9213c3d16659a52917b5ed979a313695da6abd16c4fd7445
fac95398366f644911b58f1605e6bc37fb76782d qa: Run all tests even if wallet is not compiled (MarcoFalke)
faa669cbcd1fc799517b523b0f850e01b11bf40a qa: Premine to deterministic address with -disablewallet (MarcoFalke)
Pull request description:
Currently the test_runner would exit if the wallet was not compiled into the Bitcoin Core executable. However, a lot of the tests run without the wallet just fine and there is no need to globally require the wallet to run the tests.
Tree-SHA512: 63177260aa29126fd20f0be217a82b10b62288ab846f96f1cbcc3bd2c52702437703475d91eae3f8d821a3149fc62b725a4c5b2a7b3657b67ffcbc81532a03bb
661ac15a4a appveyor: Run functional tests on appveyor (Chun Kuan Lee)
2148c36b6e tests: Make it possible to run functional tests on Windows (Chun Kuan Lee)
Pull request description:
This PR do the following things:
- Make functional tests compatible with Windows
- Print color output in functional tests for Windows 10
- Run util and functional tests on appveyor
- Do not run symlink tests on Windows
Note:
- The wallet_multiwallet.py fail is unrelated to the test framework, it's a bug related to c++ code or maybe dependencies. `bitcoind` would exit with 0xC0000005(Access violation) during shutdown occasionally. Disable this for now.
- Not using `--failfast` because this is still in experimental. We should track if there is any other error.
- Disable ZMQ tests because the python zmq library could cause access violation sometimes.
- Disable `feature_notifications` because Bitcoin Core handles the command in different thread, whicha can cause a race condition.
Tree-SHA512: b76db137d264e62a5c130e1cbca7a2ca002a7a0f4153fa0b92c1ea6c9c09ef0533e11c49bdbd566c472d8ff59f245758feb5e5a6ec6cb6bb66a1c67bab5fa48a
82fc4017b774aaff8799c2b6e8ba5370d94dbf4d test: Catch decimal.InvalidOperation from TestNodeCLI#send_cli (Ben Woosley)
Pull request description:
`decimal.InvalidOperation` is a special case of a float parsing error, which
presumably should be handled in the same way as a general parsing error,
rather than blow up.
Alternatives include: logging the error, or re-raising with more information.
Example log output:
```
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 603, in sync_all
self.sync_blocks(nodes)
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in sync_blocks
best_hash = [x.getbestblockhash() for x in rpc_connections]
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 568, in <listcomp>
best_hash = [x.getbestblockhash() for x in rpc_connections]
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 571, in __call__
return self.cli.send_cli(self.command, *args, **kwargs)
File "/home/travis/build/bitcoin/bitcoin/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_node.py", line 639, in send_cli
return json.loads(cli_stdout, parse_float=decimal.Decimal)
File "/usr/lib64/python3.6/json/__init__.py", line 367, in loads
return cls(**kw).decode(s)
File "/usr/lib64/python3.6/json/decoder.py", line 339, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/lib64/python3.6/json/decoder.py", line 355, in raw_decode
obj, end = self.scan_once(s, idx)
decimal.InvalidOperation: [<class 'decimal.InvalidOperation'>]
```
See: https://travis-ci.org/github/bitcoin/bitcoin/jobs/713502326
ACKs for top commit:
laanwj:
ACK 82fc4017b774aaff8799c2b6e8ba5370d94dbf4d
Tree-SHA512: 8c102b8bf831b05c5ca4b2e1feb5574dcbaed8cab0b2f22b013c5dfcb81788a38839a163dd1e2c6470ccbe5874214663b84485f45467738fd850ca38d539ae25
2c6a02e0248825e205e6deea4c38409044feb4ab Clean message_count and last_message (Troy Giorshev)
Pull request description:
From #19580
This PR changes comments to clarify the intended usage of `message_count` and `last_message`. Additionally it changes the only usage of `message_count` to use `last_message` instead, bringing the code into alignment with the intended usage.
Note: Now `message_count` is completely unused. However, it is ready to be used (i.e. the supporting code works) and likely will be used in some test in the future.
ACKs for top commit:
jnewbery:
utACK 2c6a02e0248825e205e6deea4c38409044feb4ab
Tree-SHA512: 07c7684c9586de4f845e10d7aac36c1aab9fb56b409949c1c70d5ca705bc3971ca7d5943245a0472def4efd7b4e1c5dad2f713db5ead8fca08404daf4891e98b
233a886b4221190a3e53128162d708266494576e test: check that getblockfilter RPC fails without block filter index (Sebastian Falbesoner)
Pull request description:
If a node was started without compact block filter index (parameter `--blockfilterindex=0`), the `getblockfilter` RPC call should fail.
ACKs for top commit:
MarcoFalke:
review ACK 233a886b4221190a3e53128162d708266494576e
Tree-SHA512: c8824373fad7d1de2dcb43c1d9541d736b478235be243080d2b7479c2588eac0e5722337ec1307394b331e0002fbcabb368e4955c2dc98dd5fce76d8c089e8a1
9e165d0de4c3cd168137fc85b8f31b371bd4e851 test: Wait for 'cmpctblock' in p2p_compactblocks when it is expected (Ben Woosley)
Pull request description:
This is a more narrowly-construed wait which eliminates the possibility of the
wait being triggered by other messages.
Note `received_block_announcement` reflect three possible messages:
edec7f7c25/test/functional/p2p_compactblocks.py (L34-L53)
Prompted by looking into: #19449
ACKs for top commit:
laanwj:
Code review ACK 9e165d0de4c3cd168137fc85b8f31b371bd4e851
theStack:
ACK 9e165d0de4
Tree-SHA512: bc4a9c8bf031c8a7efb40d9625feaa3fd1f56f3b75da7034944af71ccea44328a6c708ab0c13fea85fb7cf4fd9043fe90eb94a25e95b2d42be44c2962b4904ce
fac3e22b18cd29053bc17065fd75db7b84ba6f40 qa: Read reject reasons from debug log, not p2p messages (MarcoFalke)
Pull request description:
For local testing we don't need to rely on p2p messages just to assert a reject reason.
Replace reading p2p messages with reading from the debug log file.
Tree-SHA512: fa59598ecf5e00cfb420ef1892d90aa415501fd882e1c608894dc577b0d00e93a442326d3a9167fef77d26aafbe345b730b49109982ccad68a5942384564a90b
bb08423d5ca866d4a139a3b57ff110d818d08b32 [doc] Add release notes for 'account' API removal (John Newbery)
1f4b865e57b4567270b1586bb1f348ab9106485d [wallet] Re-sort wallet RPC commands (John Newbery)
f0dc850bf698f7377797d7d68365d4fc79b0221c [wallet] Remove wallet account RPCs (John Newbery)
c410f415758913c933ad6c71cf50227cc85aa385 [tests] Remove wallet accounts test (John Newbery)
Pull request description:
This is the first part of #13825. It simply removes the RPC methods and tests.
#13825 touches lots of files and will require frequent rebasing.
Breaking it down for easier reviewing and fewer rebases.
Tree-SHA512: d29af8e7a035e4484e6b9bb56cb86592be0ec112d8ba4ce19c15d15366ff3086e89e99fca26b90c9d66f6d3e06894486d0f29948df0bb7dcb1e2c49c6887a85a
f13ad1cae0 modify test for memory locked in case locking pages failed at some point (Adam Jonas)
2fa85ebd1c add rpc_misc.py, mv test getmemoryinfo, add test mallocinfo (Adam Jonas)
Pull request description:
Creating the `rpc_misc.py` functional test file to add space for adding tests to a file that doesn't have a lot of coverage.
- Removing the `getmemoryinfo()` smoke test from wallet basic rather than moving it to keep the wallet decoupled. Feel like testing for reasonable memory allocation values should suffice.
- Adding coverage for `mallocinfo()`. Introduced standard lib XML parser since the function exports an XML string that describes the current state of the memory-allocation implementation in the caller.
Tree-SHA512: ced30115622916c88d1e729969ee331272ec9f2881eb36dee4bb7331bf633a6810a57fed63a0cfaf86de698edb5162e6a035efd07c89ece1df56b69d61288072
a2eb6f5405 [rpc] Add getnodeaddresses RPC command (chris-belcher)
Pull request description:
Implements issue https://github.com/bitcoin/bitcoin/issues/9463
New getnodeaddresses call gives access via RPC to the peers known by the node. It may be useful for bitcoin wallets to broadcast their transactions over tor for improved privacy without using the centralized DNS seeds. getnodeaddresses is very similar to the getaddr p2p method.
Please advise me on the best approach for writing an automated test. By my reading the getaddr p2p method also isn't really tested.
Tree-SHA512: ad03abf518847476495b76a2f5394b8030aa86654429167fa618e21460abb505c10ef9817ec1b80472320d41d0aff5dc94a8efce023aaaaf5e81386aa92b852b
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
fa852f0e8d test: Bump timeout on tests that timeout on windows (MarcoFalke)
Pull request description:
Those tests build a ton of blocks and time out for me on Windows with:
```
test_framework.authproxy.JSONRPCException: 'generatetoaddress' RPC took longer than 60.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344)
Tree-SHA512: a8fffeaddd02c051fbcc04bfac69f6ed826b8f16616e3b2e210a469d07c3e5706baab8121f1cd7ed265481de3a6197cf371513e2afbe506cf13b1dabfe3a0005
8e4b4f683a0b342cec24cd51b1e98433034ea2ea Address test todos by removing -txindex to nodes. Originally added when updating getrawtransaction to stop searching unspent utxos. (Amiti Uttarwar)
Pull request description:
Original todos added when removing getrawtransaction default behavior of searching unspent utxos.
Tree-SHA512: d080953c3b0d2e5dca2265a15966dc25985a614c9cc86271ecd6276178ce428c85e262c24df92501695c32fed7beec0339b989f03cce91b57fb2efba201b7809
fae8b8bb1a qa: Add tool-prefix to functional test readme (MarcoFalke)
faf3d22725 test_runner: Remove unused --force option (MarcoFalke)
Pull request description:
When someone calls the script they already have all intention to call it, no need to specify a redundant `--force`.
The functional tests are still disabled on the travis windows cross builds, where they'd run into issues when run under Wine.
Tree-SHA512: ada0dd9b3c0cd28c5832a12c5e04c029dc3bfe5ddf366fd0abc24fb7914d2e0f0a873fe756ade7ba780a561abe9bc731838c289accc421deda481269e08514cd
62d3f5057f2ed0c8646839f38dbe29adf4601502 qa: fix deprecated log.warn in feature_dbcrash test (Jon Atack)
Pull request description:
This clears up the following deprecation message when running test/functional/feature_dbcrash.py:
```
test/functional/feature_dbcrash.py:270:
DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead
self.log.warn("Node %d never crashed during utxo flush!", i)
```
Git grepping indicates that this was the last remaining use of `log.warn` in the functional tests.
ACKs for top commit:
fanquake:
ACK 62d3f5057f2ed0c8646839f38dbe29adf4601502 - checked that there were no more occurrences.
Tree-SHA512: 2fe87400f82488e44391f4897876003a98736013e819a7dbc3b3e87a5ffbfba8d5ccab81cf2b7577f40135c95e4db96e93bb8cb24de396efb4ad814fbda09559
a407b6fdf3 [tests] Make random seed logged and settable (John Newbery)
Pull request description:
This allows tests which use randomness to be reproducibly run on failure.
ACKs for commit a407b6:
jonatack:
re-ACK a407b6fdf34f77eb347378674da9cf80394897de
jb55:
great! utACK a407b6fdf34f77eb347378674da9cf80394897de
Tree-SHA512: e1e89e6e76d11ddec71a8f0f077227e4b46303f80461b170900d3f95d4dcc4187b0d1decfd63562ea970aaaf530ef032a3e64ed1669aac29033d95161855fda3
fa6ab8ada1 rpc: Return more specific reject reason for submitblock (MarcoFalke)
Pull request description:
The second commit in #13439 made the `TODO` in the first commit impossible to solve.
The meaning of `fNewBlock` changed from "This is the first time we process this block" to "We are about to write the new *valid* block".
So whenever `fNewBlock` is true, the block was valid. And whenever the `fNewBlock` is false, the block is either valid or invalid. If it was valid and not new, we know it is a `"duplicate"`. In all other cases, the `BIP22ValidationResult()` will return the reason why it is invalid.
Tree-SHA512: 4b6edf7a912339c3acb0fccfabbdd6d812a0321fb1639c244c2714e58dc119aa2b8c6bf8f7d61ea609a1b861bbc23f920370fcf989c48452721e259a8ce93d24
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
faee59103d test: Fix race in mempool_accept (MarcoFalke)
Pull request description:
If we happen to pick the same random coin to spend, there would be mempool conflicts in some runs of the test. Fix that by popping from a static list of coins to spend from.
Tree-SHA512: f6fd37e43d919371aa8bc3a2c93b569f9169961fe702f3641bb63180c3a88f12ca1857e9ed4d3723d5f04ca8ab5ef009a90e679580f36246a10b987620a55bee
8f5d9431a Add regtests for HTTP status codes. (Daniel Kraft)
Pull request description:
This adds explicit tests for the returned HTTP status codes to `interface_rpc.py` (for error cases) and the HTTP JSON-RPC client in general for success.
#15381 brought up discussion about the HTTP status codes in general, and the general opinion was that the current choice may not be ideal but should not be changed to preserve compatibility with existing JSON-RPC clients. Thus it makes sense to actually test the current status to ensure this desired compatibility is not broken accidentally.
ACKs for commit 8f5d94:
laanwj:
utACK 8f5d9431a36740aa12abc0acea64df48fe32d2a6
promag:
utACK 8f5d943.
jonasschnelli:
utACK 8f5d9431a36740aa12abc0acea64df48fe32d2a6
Tree-SHA512: 82503ccd134dd9145304e95cb6c61755f100bee27593d567cdd5c0c554d47e7b06d937456cab04107f46f4984930355db65d5e711008a0b05f2b8feec9f2950e
aaaa8eb1edba2a28916d5da6001d421c1b1b253b test: consensus: Check that final transactions are valid (MarcoFalke)
fae3617d79deee73dd375dc3ea5f4204a74420c5 test: Correctly deserialize without witness (MarcoFalke)
Pull request description:
There is no check that checks that final transactions are valid, i.e. the consensus rules could be changed (accidentally) with none of the tests failing.
Tree-SHA512: 48f4c24bfcc525ddbc1bfe8c37131953b464823428c1f7a278ba6d98b98827b6b84a8eb2b33396bfb5b8cc4012b7cc1cd771637f405ea20beddae001c22aa290
6440e61375 qa: Drop RPC connection if --usecli (João Barbosa)
Pull request description:
Drop the RPC connection used in `TestNode.wait_for_rpc_connection` if `--usecli` is set. If the connection is kept and not used the `Connection: close` header is never sent and so the connection only closes due to timeout (30 sec).
It might be sensible to revert e98a9eede2fb48ff33a020acc888cbcd83e24bbf in a follow up, however it changes the shutdown behavior.
Tree-SHA512: 2a8ee68b82ab612a556390aae521379e592c39ea0a7855a119282e6fe4cbf02ecafe7a5e2ee37d480f2c0600fa64791117a80fecc7bbe6bbb354107972b3b320
3fb09b9889665a24b34f25e9d1385a05058a28b7 Warn unrecognized sections in the config file (Akio Nakamura)
Pull request description:
This PR intends to resolve#14702.
In the config file, sections are specified by square bracket pair "[]"$,
or included in the option name itself which separated by a period"(.)".
Typicaly, [testnet] is not a correct section name and specified options
in that section are ignored but user cannot recognize what is happen.
So, add some log-warning messages if unrecognized section names are
present in the config file after checking section only args.
note: Currentry, followings are out of scope of this PR.
1) Empty section name or option name can describe.
e.g. [] , .a=b, =c
2) Multiple period characters can exist in the section name and option name.
e.g. [c.d.e], [..], f.g.h.i=j, ..=k
Tree-SHA512: 2cea02a0525feb40320613989a75cd7b7b1bd12158d5e6f3174ca77e6a25bb84425dd8812f62483df9fc482045c7b5402d69bc714430518b1847d055a2dc304b
bbbbb3f8850907d413db4715c10ef6df055234f6 qa: Add test to ensure node can generate all help texts at runtime (MarcoFalke)
Pull request description:
This might increase coverage, but more importantly this checks that the node doesn't crash when generating the help. (Right now the help is a static string, but in the future it might be generated at runtime)
Tree-SHA512: 0226e7c65f8a1a6fdc96c07dcf491d90559bc2355c92e9da9b1f174b09733fc349269e71da6d792f954de563a1e57c848471813eabae1a40b849a0d989520a0d
d4d70eda33 Fix listreceivedbyaddress not taking address as a string (Eric Scrivner)
Pull request description:
Fixes#14173. Add the patch in #14173 and include a regression test.
Tree-SHA512: 5a9794e0c43e90d18c899841afbaf15eb9129d7d2f6570fccf0a1793697fe170d224c3c3995b1a35c536fac19819042823d9e3bd23b019d0f03434499243d2f5
380c843217 utils: Convert Windows args to utf-8 string (Chun Kuan Lee)
Pull request description:
Create a new class `WinCmdLineArgs` when building for Windows. It converts all command line arguments to utf8 string.
Tree-SHA512: f098520fd123a8a452bc84a55dc8c0b88f0c475410efe57f2ccc393f86c396eed59ea1575ddc1b920323792e390fdb092061d80cdcd9b682f0ac79a22a22ff82
* Merge #14060: ZMQ: add options to configure outbound message high water mark, aka SNDHWM
a4edb168b635b6f5c36324e44961cd42cf9bbbaa ZMQ: add options to configure outbound message high water mark, aka SNDHWM (mruddy)
Pull request description:
ZMQ: add options to configure outbound message high water mark, aka SNDHWM
This is my attempt at https://github.com/bitcoin/bitcoin/pull/13315
Tree-SHA512: a4cc3bcf179776899261a97c8c4f31f35d1d8950fd71a09a79c5c064879b38e600b26824c89c4091d941502ed5b0255390882f7d44baf9e6dc49d685a86e8edb
* High watermark settings for Dash-specific messages
Signed-off-by: Dzutte <dzutte.tomsk@gmail.com>
Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
ef362f2773 rpc/gui: Remove 'Unknown block versions being mined' warning (Wladimir J. van der Laan)
Pull request description:
Due to miners inserting garbage into the version numbers causing false positives, the current version signalling has become completely useless. This removes the "unknown block versions" warning which has the tendency to scare users unnecessarily (and might get them to "update" to something bad).
It preserves the warning in the logs. Whether this is desirable can be a point of discussion.
Tree-SHA512: 51407ccd24a571462465d9c7180f0f28307c50b82a03284abe783e181d8ab7e0638dbb710698d883f28de8a609db70763e39be2470d956e67c833da0768e43e9
f7e9e70468 [rpc] Remove deprecated sigrawtransaction rpc method. (John Newbery)
90c834089a [RPC] Remove warning about wallet addresses in createmultisig() (John Newbery)
df905e390e [rpc] Remove deprecated validateaddress usage. (John Newbery)
Pull request description:
The following rpc features were deprecated in V0.17:
- `validateaddress` returning wallet information about an address
- `signrawtransaction`
This PR fully removes those features. It can be merged once V0.17 has been branched from master.
Tree-SHA512: 28293d218cf7e348632081e362f8775f243d091f49aed54c354f017d4a12ae92b87b99f81ee592a1bbf4aebd5d8cd5119278141edde7a0399ff82917ed68b9f6
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
* Backporting Merge #18584: test: Check that the version message does not leak the local address
fa404f1e4718e8155581f23826480086dfbcfaa6 test: Check that the version message does not leak the local address of the node (MarcoFalke)
Pull request description:
Add test for #8740
ACKs for top commit:
theStack:
ACK fa404f1e47
Tree-SHA512: 4d1c10d1c02fba4b51bd8b9eb3a0d9a682b6aac8c3f6924e295fdca3faefa5ecc3eaa87d347cfec5d2b2bc49963c10fe0a37c463f36088ed0304a2e3716b963b
* Merge #18584: test: Check that the version message does not leak the local address
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
6f6514a08090b37b5e8c086015ee4881813ef867 Correct units for "-dbcache" and "-prune" (Hennadii Stepanov)
Pull request description:
Actually, all `dbcache`-related values in the code are measured in MiB (not in megabytes, MB) or in bytes (e.g., `nTotalCache`).
See: https://github.com/bitcoin/bitcoin/blob/master/src/txdb.hba8c8b2227/src/init.cpp (L1405-L1424)
Also, "-prune" is fixed:
1. The GUI values in GB are translated to the node values in MiB correctly.
2. The maximum of the "prune" `QSpinBox` is not limited by default value of 99 (GB).
Fix: #15106
Tree-SHA512: 151ec43b31b1074db8b345fedb1dcc10bde225899a5296bfc183f57e1553d13ac27db8db100226646769ad03c9fcab29d88763065a471757c6c41ac51108459d
fa21568208 qa: Avoid race in p2p_invalid_block by waiting for the block request (MarcoFalke)
6c787d340c tests: Make feature_block pass on centos (MarcoFalke)
Pull request description:
This hopefully fixes#14661, which I believe is caused by a race in `send_blocks_and_test`. By setting `request_block=False` we only effectively check `node.getbestblockhash() != blocks[-1].hash` before returning and checking the debug.log. By setting `request_block=True` (the default) we make sure that we send the block, then sync with a ping before asserting on the debug.log.
Even if this patch doesn't fix the issue, it is good cleanup: There is no reason to not wait for the blocks to be requested, since in all these cases the header gives no indication that the block is consensus invalid. So this patch makes the test also a bit stricter and more useful.
Unrelated to this, I also include a fix that makes the tests pass on latest CentOS.
Tree-SHA512: c7abee3b7dc790a8af6c289159a7751bd962f6fa16c1537e7e21a0a0ef05b9596d1f4eb75319614603c05cb803e021314fa3596508ba443edd03046b25527e0f
9b4a36effcf642f3844c6696b757266686ece11a [qa] Test for duplicate inputs within a transaction (Suhas Daftuar)
b8f801964f59586508ea8da6cf3decd76bc0e571 Fix crash bug with duplicate inputs within a transaction (Suhas Daftuar)
Pull request description:
Tree-SHA512: 8c7ea34c7fa44188d86c04a690a7cbf8e9deda71ab1f7ca6d11de1f2abb3dd7222627071f86d0d39689a8b302ba9af142f0202466a67e30cd54aed3a08d4eb14
0385109444646561a718f34ae437b7e0e4d4d5bc Add test for rpcpassword hash error (MeshCollider)
13fe258e91e7a92326aedf151c571994166a06d4 Error if rpcpassword in conf contains a hash character (MeshCollider)
Pull request description:
Fixes#13143 now #13482 was merged
Tree-SHA512: e7d00c8df1657f6b8d0eee1e06b9ce2b1b0a2de487377699382c1b057836e1571dac313ca878b5877c862f0461ba789a50b239d2a9f34accd8a6321f126e3d2a
5a1f57646b qa: clean up assert_memory_usage_stable utility (James O'Beirne)
0cf1632f03 qa: fix p2p_invalid_messages on macOS (James O'Beirne)
Pull request description:
Infinite mea culpa for the number of problems with this test.
This change bumps the acceptable RSS increase threshold from 3% to 50% when spamming the test node with junk 4MB messages. On [@MarcoFalke's macOS test build](https://travis-ci.org/MarcoFalke/btc_nightly) we see RSS grow ~14% from ~71MB to 81MB, so a 50% increase threshold should be more than sufficient to avoid spurious failures.
Tree-SHA512: 150a7b88080fd883c7a5d0b9ffa470f61a97c4885fccc1a06fde6260aaef15640a7c1de7e89c581b245df7807d617ec3d86775330386ec5149ad567492fc5d31
3d305e3b89 Send fewer spam messages in p2p_invalid_messages (James O'Beirne)
Pull request description:
Builds on travis are failing because the test node isn't
able to drop all the bad messages sent within the given
timeout. Reduce the number of bad messages we're sending
and increase the timeout to avoid failures on travis.
Tree-SHA512: 11c389619d9590caf7eca74e0efe6d950469415d34220072770689024b350cc08a2d5ec90634237d87ff71ba8b638c1152b8a45ffbb2815a48bde6a88fbb8fc6
d20a9fa13d1c13f552e879798c0508be70190e71 tests: add tests for invalid P2P messages (James O'Beirne)
62f94d39f8de88a44bb0a8a2837d864f777aaacc tests: add P2PConnection.send_raw_message (James O'Beirne)
5aa31f6ef26f51ce461c917654dd1cfbbdd1409a tests: add utility to assert node memory usage hasn't increased (James O'Beirne)
Pull request description:
- Adds `p2p_invalid_messages.py`: tests based on behavior for dealing with invalid and malformed P2P messages. Includes a test verifying that we can't DoS a node by spamming it with large invalid messages.
- Adds `TestNode.assert_memory_usage_stable`: a context manager that allows us to ensure memory usage doesn't significantly increase on a node during some test.
- Adds `P2PConnection.send_raw_message`: which allows us to construct and send messages with tweaked headers.
Tree-SHA512: 720a4894c1e6d8f1551b2ae710e5b06c9e4f281524623957cb01599be9afea82671dc26d6152281de0acb87720f0c53b61e2b27d40434d30e525dd9e31fa671f
13782b8ba8 docs: add perf section to developer docs (James O'Beirne)
58180b5fd4 tests: add utility to easily profile node performance with perf (James O'Beirne)
Pull request description:
Adds a context manager to easily (and selectively) profile node performance during functional test execution using `perf`.
While writing some tests, I encountered some odd bitcoind slowness. I wrote up a utility (`TestNode.profile_with_perf`) that generates performance diagnostics for a node by running `perf` during the execution of a particular region of test code.
`perf` usage is detailed in the excellent (and sadly unmerged) https://github.com/bitcoin/bitcoin/pull/12649; all due props to @eklitzke.
### Example
```python
with node.profile_with_perf("large-msgs"):
for i in range(200):
node.p2p.send_message(some_large_msg)
node.p2p.sync_with_ping()
```
This generates a perf data file in the test node's datadir (`/tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data`).
Running `perf report` generates nice output about where the node spent most of its time while running that part of the test:
```bash
$ perf report -i /tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data --stdio \
| c++filt \
| less
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 135 of event 'cycles:pp'
# Event count (approx.): 1458205679493582
#
# Children Self Command Shared Object Symbol
# ........ ........ ............... ................... ........................................................................................................................................................................................................................................................................
#
70.14% 0.00% bitcoin-net bitcoind [.] CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
|
---CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
70.14% 0.00% bitcoin-net bitcoind [.] CNetMessage::readData(char const*, unsigned int)
|
---CNetMessage::readData(char const*, unsigned int)
CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
35.52% 0.00% bitcoin-net bitcoind [.] std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
|
---std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
CNetMessage::readData(char const*, unsigned int)
CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
...
```
Tree-SHA512: 9ac4ceaa88818d5eca00994e8e3c8ad42ae019550d6583972a0a4f7b0c4f61032e3d0c476b4ae58756bc5eb8f8015a19a7fc26c095bd588f31d49a37ed0c6b3e
fa5278a419 qa: Use wallet to retrieve raw transactions (MarcoFalke)
fa2198328e qa: Style-only fixes in touched files (MarcoFalke)
Pull request description:
Instead of asking the coin database and block storage about a transaction, pull it directly from the wallet in wallet related tests.
This refactoring only makes sense in light of #15159.
<sub>This product may contain minor stylistic cleanups
Tree-SHA512: ec34c7150d873da9f19fead3f7e3f758baba5ef10061942384c470a47a6f320690109be9c5160f0c8bc228272a729653d44c78471455337318f657d6c164ba23
e6c58d3b014ab8ef5cca4be68764af4b79685fcb Do not import private keys to wallets with private keys disabled (Andrew Chow)
b5c5021b644731d14a6ef04961320a99466f035a Refactor importwallet to extract data from the file and then import (Andrew Chow)
1f77f6754ce724493b0cb084ae0b35107d58605f tests: unify RPC argument to cli argument conversion and handle dicts and lists (Andrew Chow)
Pull request description:
Fixes a bug where private keys could be imported to wallets with private keys disabled. Now every RPC which can import private keys checks for whether the wallet has private keys are disabled and errors if it is. Also added an belt-and-suspenders check to `AddKeyPubkeyWithDB` to have it assert that the wallet has private keys enabled.
Tree-SHA512: 5cd04febce9aa2bd9bfd02f312c6ff8705e37278cae59efd3895f6d6e2f1b477aefd297e2dd0860791bdd3d4f3cad8eb1a404f8f3d4e2035b91314ad2c1028ae
dash changes
4999992c34 whitespace: Split ~300 char line into multiple ones (MarcoFalke)
fa71b38168 scripted-diff: Rename rpc_timewait to rpc_timeout (MarcoFalke)
fa3e5786d0 scripted-diff: Remove unused 'split' parameter to setup_network (MarcoFalke)
Pull request description:
This is a bugfix, since wallet_dump currently uses the wrong name:
18857b4c40/test/functional/wallet_dump.py (L89-L92)
Rename all to the same name with a scripted diff (and some unrelated cleanups).
Tree-SHA512: 338ddd20dae12e6cf7aa7adbcfb239cf648017a1572b373f8431fecb184bd2a65492846d81e75a023864d9e41c94afb53044c16b79651a5937d34a5a6b772f81
d6b3790d1a tests: check readability of cookie file (Chun Kuan Lee)
Pull request description:
This PR would wait until the `.cookie` file is readable
Possible fix no. 5 `PermissionError` in #14446
Tree-SHA512: e7055c7ca26a6eadbbe19e4eef08ffee61cd17de79b30af2f0d090f0ad81ca24815e3c7e034e5e30d47c580bb0b221b3955e9ff2fcec2274fbf7b9232ab0cdc7
2012d4df2 Add CScriptNum decode python implementation in functional suite (Gregory Sanders)
Pull request description:
I needed this for reasons and thought it'd be good to upsteam it.
Tree-SHA512: 6ea89fa2a5f5a7759ba722f2b4ed5cd6423ebfff4e83ac8b8b5c935e6aa479684e626c5f41fa020816d2a9079a99af5564e30808594d5c13e3b51ec9b474926d
ed2e18398b3ab657e98e3e1fe135cbf8dd94fda3 Remove fs::relative call and fix listwalletdir tests (João Barbosa)
Pull request description:
The implementation of `fs::relative` resolves symlinks which is not
intended in ListWalletDir. The replacement does what is required, and
`listwalletdir` RPC tests are fixed accordingly.
Also, `fs::recursive_directory_iterator` iteration is fixed to build
with boost 1.47.
Based on #14559
Tree-SHA512: 1da516226073f195285d10d9d9648c90cce0158c5d1eb9c31217bb4abb575cd37f07c00787c5a850554d6120bbc5a3cbc5cb47d4488b32ac6bcb52bc1882d600
4dca7d0a98 appveyor: Enable multiwallet test (Chun Kuan Lee)
Pull request description:
Based on #14320
This PR enable multiwallet test on appveyor. Also re-enable symlink
tests on Windows which is available after Windows Vista.
I disable these tests in #13964 because I suppose that Windows does
not support symlink, but I was wrong.
Tree-SHA512: 852cd4dedf36ec9c34aff8926cb34e6a560aea0bb9170c7a2264fc292dbb605622d561568d8df39aeb90d3d2bb700901d218ea7e7c5e21d84827c40d6370b369
ca6d86c322 tests: Stop node before removing the notification file (Chun Kuan Lee)
Pull request description:
Stop node before removing the notification file to make sure the
command has been terminated. After then we could removing those files
safely and do not receive any permission error. (See #14446)
The permission error is Windows specific, documented in python doc:
>On Windows, attempting to remove a file that is in use causes an
exception to be raised
See https://docs.python.org/3/library/os.html#os.remove
Tree-SHA512: fbdabf3a9a838bb59ba207dd9e9fbdd87c702a99ad66bee0b2b1537f80f8630d22d9d5e9c4ded23a82a66bfc10989227fb024b27393425abe0e5a2ad4e4cbb82
67654b6405 tests: write the notification to different files to avoid race condition (Chun Kuan Lee)
Pull request description:
This PR change the behavior that `feature_notifications.py` would
write to different files instead of writing to the same file to avoid
race condition.
Tree-SHA512: 78406167cc6a3f570134b0ee76d2be1440bc1498cd7b1be72fae16d0ab86950e26ef3bf6008796016e5418231400c6492f0e062909dd882646541ecb7a70fb30
d56a0689354fb814510c6c393f3e07ac9362dc1f docs: Add release notes for listwalletdir RPC (João Barbosa)
0cb3cad166bbeb75e9cc1512286453f8e7d4f717 qa: Add tests for listwalletdir RPC (João Barbosa)
cc3377360c417780f5cbd7bd69b438817a9d60be rpc: Add listwalletdir RPC (João Barbosa)
d1b03b8e5f04a2cc9ebb985bd9a1aebd2068f757 interfaces: Add getWalletDir and listWalletDir to Node (João Barbosa)
fc4db35bfd78d85d6b52d5da3d89696160658450 wallet: Add ListWalletDir utility (João Barbosa)
Pull request description:
`ListWalletDir` returns all available wallets in the current wallet
directory.
Based on MeshCollider work in pull #11485.
Tree-SHA512: 5843e3dbd1e0449f55bb8ea7c241a536078ff6ffcaad88ce5fcf8963971d48c78600fbc4f44919523b8a92329d5d8a5f567a3e0ccb0270fdd27366e19603a716
c82190cdb6 tests: Add Python dead code linter (vulture) (practicalswift)
590a57fdec tests: Remove unused testing code (practicalswift)
Pull request description:
Add Python dead code linter (`vulture`) to Travis.
Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`):
* Less is more :-)
* Unused code is by definition "untested"
* Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-)
* Unused code is hard to spot for humans and is thus often missed during manual review
* [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it)
Based on #14312 to make linter job pass.
Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
b168dd30cf71ac176e271bc610b0b1a79ceaf075 Bugfix: QA: Run tests with UPnP disabled (Luke Dashjr)
Pull request description:
This replaces #16560 by adding `upnp=0` to `bitcoin.conf` rather than passing it to nodes.
> Needed for builds configured with --enable-upnp-default
You can test this change using:
```bash
./configure --enable-upnp-default && make -j6 && test/functional/test_runner.py feature_config_args.py
```
on master the test will fail without this change.
ACKs for top commit:
practicalswift:
ACK b168dd30cf71ac176e271bc610b0b1a79ceaf075 -- diff looks correct
Tree-SHA512: e639dd480dda2cffa19a679018c4bd7e4bd4d0f5e3001d6b407b833e3c166bde98b201063e267b8e45f8a20b0d53ec8bc028bec806b2357f9a7ba314cc4e2d07
fac2e6a6045e4ddd6b473f4f3ddbb69d9d6921f6 test: Fail early on disconnect in mininode.wait_for_* (MarcoFalke)
Pull request description:
The node might crash or disconnect when our mininode waits for data. Due to the crash, the data is guaranteed to never arrive and we can fail early with an assert
ACKs for top commit:
laanwj:
ACK fac2e6a6045e4ddd6b473f4f3ddbb69d9d6921f6
Tree-SHA512: 32ca844eb66bd70ea49103d51c76b953242b1886e0834d96fca8840fc984ff40346d0a799adf8f76b03514a783cb9cec69d45e00bdd328c5192c31b5d8d17af2
fa7dd88b71 test: Add test for unknown args (MarcoFalke)
Pull request description:
Currently uncovered.
Further reading:
* https://marcofalke.github.io/btc_cov/total.coverage/src/util/system.cpp.gcov.html
* Fail on unknown config file options #15021
ACKs for commit fa7dd8:
promag:
ACK fa7dd88b71a1c6641bd450fae29a4a31849b1afd, tests looks good to me.
hebasto:
ACK fa7dd88b71a1c6641bd450fae29a4a31849b1afd, I have tested the code.
Tree-SHA512: 86ab370ce8e85925f945a52e81457b5678d71bbabcef01205a97782b780003f363552e0bad1ff678bccc784f82c6b511c3b88de3f8f25f62b0b713c387950564
2e5d482659 tests: Print remaining jobs in test_runner.py (Steven Roose)
Pull request description:
This helps finding out which tests fail to finish.
Tree-SHA512: d22beb82beecd33aaa50731c83075e49577842d29fd21aa63bcb859df5da99069eba9cc16eed5d91dbba8fb0fdc317fb88b3b370c4d3917e9da1cd13b0a622dc
3d2c7d6f94 Add regtest for JSON-RPC batch calls. (Daniel Kraft)
Pull request description:
This adds a new regtest file `interface_rpc.py`, containing a test for batch JSON-RPC requests. Those were previously not tested at all. Tests for basic requests are not really necessary, as those are used anyway in lots of other regtests.
The existing `interface_http.py` file is more about the underlying HTTP connection, so adding a new interface file for the JSON-RPC specific things makes sense.
Tree-SHA512: 7c7576004c8474e23c98f4bf25fb655328ba6bb73ea06744ebee1c0ffbb26bc132e621ae52955d51dab0803b322f8d711667626a777ac9b26003339c2484502f
fa0815c300 rpc: Correctly name arguments (Jon Layton)
Pull request description:
Consistently use the same name to describe arguments in the documentation and add a test that uses the name.
By splitting it up, the changes are easier to potentially backport and also make review easier when we switch to `RPCHelpMan`.
The tests should pass with or without the changes in `src`.
Partly stolen from #14459 (More RPC help description fixes by ch4ot1c)
Tree-SHA512: 1072992b1e93ac41006613523e54a0a8004f529fcb101eb9d74d91474abb0945a5a7539f249905151b904b87448f9efc0cacbd9e052fbe2ea9111e62f3e7249c
* Merge bitcoin#13399: rpc: Add submitheader
fa091b001605c4481fb4eca415929a98d3478549 qa: Add tests for submitheader (MarcoFalke)
36b1b63f20cc718084971d2cadd04497a9b72634 rpc: Expose ProcessNewBlockHeaders (MarcoFalke)
Pull request description:
This exposes `ProcessNewBlockHeaders` as an rpc called `submitheader`.
This can be used to check for invalid block headers and submission of
valid block headers via the rpc.
Tree-SHA512:
a61e850470f15465f88e450609116df0a98d5d9afadf36b2033d820933d8b6a4012f9f2b3246319c08a0e511bef517f5d808cd0f44ffca91d10895a938004f0b
* Update test/functional/mining_basic.py
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
9c5af58d51 Consolidate redundant implementations of ParseHashStr (Ben Woosley)
Pull request description:
This change:
* adds a length check to all calls to `ParseHashStr`, appropriate given its use to populate
a 256-bit number from a hex str
* allows the caller to handle the failure, which allows for the more
appropriate `JSONRPCError` on failure in `prioritisetransaction` rpc
Relative to #14288
Tree-SHA512: baa791147e5ceb3c30c70df3981aaf807bf7d4a90a0be3625540b59aa4b9a9d303a452bfef18bf167cbb833ef9591b4ef5948bf4a1ce67b421d804ae8d20ea53
e460232876 Document fixed attribute behavior in critical test framework classes. (Justin Turner Arthur)
17b42f4122 Check for specific tx acceptance failures based on script signature (Justin Turner Arthur)
3a4449e9ad Strictly enforce instance attrs in critical functional test classes. (Justin Turner Arthur)
1d0ce94a54 Fix for incorrect version attr set on functional test segwit block. (Justin Turner Arthur)
ba923e32a0 test: Fix broken segwit test (practicalswift)
Pull request description:
No extra attributes will be able to be added to instances of the C++ class ports or of other critical classes without causing an exception. Helps prevent adding or depending on attributes that aren't in the intended object structure. It may prevent issues such as the one fixed in bitcoin/bitcoin#14300.
This request fixes the erroneous version attribute used in the p2p_segwit.py functional tests. This pull includes the commit from bitcoin/bitcoin#14300.
Tree-SHA512: 1b8c58e7aa0f71075ed5ff3e5be0a5182599108d8cd9bce682feac3b1842508124288e9335432b16a43f40f159c9710899e6d84af1b5868f48c947bc6f3e07ec
98a1846b00d9c3076d6dcd96244fae6f923e26a0 tests: Support calling add_nodes more than once (Steven Roose)
Pull request description:
Ran into this while writing [a multi-chain test for Elements](https://github.com/ElementsProject/elements/pull/458) where I call this method more than once.
Tree-SHA512: f2d698fcb560552aa5d81a4c3fbf40b7269b228b34d85a118291649ef83f8c0a30cd82a28d418237b55893bcecd538046b704e64a4d8a41f2c0aef8033dc83e5
30d0f7be6e rpc: Fix for segfault if combinepsbt called with empty inputs (benthecarman)
Pull request description:
Fixes#15300
Tree-SHA512: 25e7b4e6e48d8b0d197f0ab96df308fff33e2110f8929cb48914877fa7f4c4a84f173b1378fdb2dec5d03fe7d6d1aced4b577e55f9fe180d8147d9106ebf543f
31926ee8cfc73501524dfa0fef2ccbaa786d6a00 [test] functional framework: add CScript hex() for Python 3.4 (Sjors Provoost)
74ce32683199b987e45eb16f0320ae392ff10edc [test] Travis: enforce Python 3.4 support in functional tests (Sjors Provoost)
Pull request description:
The minimum supported version of Python is 3.4 according to [dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md). This PR makes the Travis linter use this version in order to catch accidental use of modern syntax.
Tree-SHA512: 71b2c102be72b135a8ba049378d66875760f20a04a657102a399240c5c2b2ddbdfa7d5ab4c0c0242ecc3259e0ee8eb2273f331bc5eb824f4ae4c3cc58aea37ac
14a06525b2 tests: add test for 'getaddressinfo' RPC result 'ischange' field (whythat)
93d1aa9abc rpcwallet: add 'ischange' field to 'getaddressinfo' response (whythat)
Pull request description:
Implementation of proposal in #14396.
This introduces `CWallet::IsChange(CScript&)` method and replaces original `CWallet::IsChange(CTxOut&)` method with overloaded version that delegates to the new method with *txout*'s `scriptPubKey`. In this way `TODO` note from the original method can still be addressed in a single place.
Tree-SHA512: ef5dbc82d76b4b9b2fa6a70abc3385a677c55021f79e187ee2f392ee32bc6b406191f4129acae5c17b0206e72b6712e7e0cad574a4bbd966871c2e656c45e041
# Conflicts:
# doc/release-notes-14282.md
# src/wallet/rpcwallet.cpp
4fb3388db95f408566e43ebb9736842cfbff0a7d check that a separator is found for psbt inputs, outputs, and global map (Andrew Chow)
Pull request description:
Currently it doesn't make sure that a separator was found so PSBTs missing a trailing separator would still pass. This fixes that and adds a test case for it.
It really only makes sense to check for the separator for the output maps as if an input or global map was missing a separator, the fields following it would be interpreted as belonging to the previous input or global map. However I have added the check for those two anyways to be consistent.
Tree-SHA512: 50c0c08e201ba02494b369a4d36ddb73e6634eb5a4e4e201c4ef38fd2dbeea2c642b8a04d50c91615da61ecbfade37309e47431368f4b1064539c42015766b50
fa43626611 test_runner: Remove travis specific code (MarcoFalke)
Pull request description:
The tests are no longer run on travis, but in a docker, developer machines or a windows vm.
The code was essentially dead for months now. Fix that by explicitly passing in `--ci` to the test runner on our docker and appveyor windows vm.
Tree-SHA512: 5d48693c03e8eb27536658ccf9ba738fe93a72abd4b72c80caac084b5b2cdffa77a1031a671eeefe70b71d63500f55917803d4be54d01849722afdccb700a9e6
fa78a2fc67 [tests] Test that nodes respond to getdata with notfound (MarcoFalke)
Pull request description:
If a node has not announced a tx at all, then it should respond to
getdata messages for that tx with notfound, to avoid leaking tx
origination privacy.
In the future this could be adjusted such that a node responds with
notfound when a tx has not been announced to us, but that seems
to be a more involved change. See e.g.
https://github.com/jnewbery/bitcoin/commits/pr14220.1
Tree-SHA512: 6244afa5bd5d8fec9b89dfc02c9958bc370195145a0f3715f33200d6cf73a376c94193d44bf4523867196e6591c53ede8f9b6a77cb296b48c114a117b8c8b1fa
3be209d103297aaf2fe4711e237a65046488ea19 rpc: Always throw in getblockstats if -txindex is required (João Barbosa)
Pull request description:
Previously blocks with only the coinbase transaction didn't cause
the RPC error even if the requested stats required -txindex and
it wasn't enabled.
Fixes#14499.
Tree-SHA512: d3a6402889e3ce7199632e79eba66d7d471ff7de5c564d35312e2340cc6d84ef544a8172548fbc2eedf5e637b56dc57bbf7a9815ab798c7f226755f897fd8f3e
42a995ae48 [tests] Remove rpc_zmq.py (John Newbery)
Pull request description:
rpc_zmq.py is racy and fails intermittently. Remove that test file and
move the getzmqnotifications RPC test into interface_zmq.py.
Tree-SHA512: 666c8f252f8a392deda1bd531e84fdc04bdae4eab09407657ade2b5fc0aeffa247735e20314236f56e4e3402476673f3b7538d6e09f5af6976021ba2377ce63c
* Fix GetDevNetName
* Fix initialize_datadir
* Adjust peer_connect to use the same devnet name as in initialize_datadir
* Tweak p2p_connect_to_devnet.py to test mininode devnet connections
fa8ced32a60dea37ac169241cf9a1f708ef46c4b doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de79faaca2b088fcbe7f76701faa9bff236 test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e7b4540857261aed948b94b6b2bfdbc3d1 test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7329d3e74d46ab98b93772b1832a3f1819 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)
Pull request description:
This is de-facto no longer hidden
ACKs for commit fa8ced:
jamesob:
utACK fa8ced32a6
Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245
fa8433e379 qa: Remove unneded import_deterministic_coinbase_privkeys overwrite, add comments (MarcoFalke)
e413c2ddd1 qa: Fix codespell error and have lint-spelling error instead of warn (MarcoFalke)
Pull request description:
Currently the functional tests require the wallet module to be compiled into the Bitcoin Core executable. For example the premine (or datadir cache) to speed up tests when run in parallel would mine a bunch of blocks and store the private keys to sign the coinbase tx outputs in a wallet. There is no need to have the overhead of the whole wallet module by using keys that are deterministic for all runs.
Note that this change most likely requires the `./test/cache/` to be cleared.
Tree-SHA512: 9ce26036b0e10f0f888f66a1e50be6a357343f9ffb302ae24a7bb3df2f083a31702ef308b738a03b08a1b623aeddac5d6563dc1b15078c0357b7dafad7808ec3
Backporting 15654 in 4213 broke devnet connections because of SanitizeString for cleanSubVer. The real issue is using unsafe character in devnet uacomments actually, so to fix this we should replace unsafe `=` with something safe e.g. `.`.
3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb Reformat p2p_permissions.py (nicolas.dorier)
ce7eac3cb0e7d301db75de24e9a7b0af93c61311 [Fix] The default whitelistrelay should be true (nicolas.dorier)
Pull request description:
I thought `whitelistrelay` default was `false` when it is `true`.
The root of the issue come from the fact that all references to `DEFAULT_` are not in the scope of this file, so hard coding of default values are used everywhere in `net.cpp`. I think that in a separate PR we should fix that more fundamentally everywhere.
ACKs for top commit:
promag:
ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb.
Sjors:
re-ACK 3b05f0f70fbaee5b5eaa0d1b6f3b9d32f44410bb
Tree-SHA512: f4a75f986fa2adf1a5f1c91605e0d261f7ac5ac8535fb05437d83b8392dbcf5cc1a47d755adcf8ad8dc67a88de28060187200fd3ce06545261a5c7ec0fea831a
d117f4541d4717e83c9396273e92960723622030 Add test for setban (nicolas.dorier)
dc7529abf0197dccb876dc4a93cbdd2ad9f03e5c [Fix] Allow connection of a noban banned peer (nicolas.dorier)
Pull request description:
Reported by @MarcoFalke on https://github.com/bitcoin/bitcoin/pull/16248#discussion_r314026195
The bug would mean that if the peer connecting to you is banned, but whitelisted without specific permissions, it would not be able to connect to the node.
The solution is just to move the same line below.
ACKs for top commit:
Sjors:
Agree inline is more clear. utACK d117f45
MarcoFalke:
ACK d117f4541d4717e83c9396273e92960723622030
Tree-SHA512: 0fed39acb1e8db67bb0bf4c4de3ad034ae776f38d55bd661f1ae0e1a4c6becaf1824ab46ed8279f2f31df3f4b29ff56461d8b167d3e9cece62cfe58b5a912811
c5b404e8f1973afe071a07c63ba1038eefe13f0f Add functional tests for flexible whitebind/list (nicolas.dorier)
d541fa391844f658bd7035659b5b16695733dd56 Replace the use of fWhitelisted by permission checks (nicolas.dorier)
ecd5cf7ea4c3644a30092100ffc399e30e193275 Do not disconnect peer for asking mempool if it has NO_BAN permission (nicolas.dorier)
e5b26deaaa6842f7dd7c4537ede000f965ea0189 Make whitebind/whitelist permissions more flexible (nicolas.dorier)
Pull request description:
# Motivation
In 0.19, bloom filter will be disabled by default. I tried to make [a PR](https://github.com/bitcoin/bitcoin/pull/16176) to enable bloom filter for whitelisted peers regardless of `-peerbloomfilters`.
Bloom filter have non existent privacy and server can omit filter's matches. However, both problems are completely irrelevant when you connect to your own node. If you connect to your own node, bloom filters are the most bandwidth efficient way to synchronize your light client without the need of some middleware like Electrum.
It is also a superior alternative to BIP157 as it does not require to maintain an additional index and it would work well on pruned nodes.
When I attempted to allow bloom filters for whitelisted peer, my proposal has been NACKed in favor of [a more flexible approach](https://github.com/bitcoin/bitcoin/pull/16176#issuecomment-500762907) which should allow node operator to set fine grained permissions instead of a global `whitelisted` attribute.
Doing so will also make follow up idea very easy to implement in a backward compatible way.
# Implementation details
The PR propose a new format for `--white{list,bind}`. I added a way to specify permissions granted to inbound connection matching `white{list,bind}`.
The following permissions exists:
* ForceRelay
* Relay
* NoBan
* BloomFilter
* Mempool
Example:
* `-whitelist=bloomfilter@127.0.0.1/32`.
* `-whitebind=bloomfilter,relay,noban@127.0.0.1:10020`.
If no permissions are specified, `NoBan | Mempool` is assumed. (making this PR backward compatible)
When we receive an inbound connection, we calculate the effective permissions for this peer by fetching the permissions granted from `whitelist` and add to it the permissions granted from `whitebind`.
To keep backward compatibility, if no permissions are specified in `white{list,bind}` (e.g. `--whitelist=127.0.0.1`) then parameters `-whitelistforcerelay` and `-whiterelay` will add the permissions `ForceRelay` and `Relay` to the inbound node.
`-whitelistforcerelay` and `-whiterelay` are ignored if the permissions flags are explicitly set in `white{bind,list}`.
# Follow up idea
Based on this PR, other changes become quite easy to code in a trivially review-able, backward compatible way:
* Changing `connect` at rpc and config file level to understand the permissions flags.
* Changing the permissions of a peer at RPC level.
ACKs for top commit:
laanwj:
re-ACK c5b404e8f1973afe071a07c63ba1038eefe13f0f
Tree-SHA512: adfefb373d09e68cae401247c8fc64034e305694cdef104bdcdacb9f1704277bd53b18f52a2427a5cffdbc77bda410d221aed252bc2ece698ffbb9cf1b830577
b6a253337f6371e4aa27c488ad70741d2b750d01 Remove redundant BIP174 test from rpc_psbt.json (araspitzu)
Pull request description:
There was a duplicate test for SIGNER role inside 'test/functional/data/rpc_psbt.json', namely test number 2 was equal to test number 3 in the array of data for 'signer'. This pull request removes the 3rd (redundant) test.
Tree-SHA512: e2128c93183f2e0acf5247274397c77a962accf95dee3bb6f785494cf3080a3f28ea47d8209e36b3064490c821690d1742c22e0d76370cb1688dcb2ab91d8f57
ed2332aeffb071a3404be9cff8f9fb8a81a9fbfb test: Add test for config file parsing errors (MarcoFalke)
a66c0f78a941968340f030911765a84219908c4d util: Report parse errors in configuration file (Wladimir J. van der Laan)
Pull request description:
Report errors while parsing the configuration file, instead of silently ignoring them.
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: sdafsdfafs
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -
(inspired by https://github.com/bitcoin/bitcoin/pull/14100#issuecomment-417264823)
Tree-SHA512: d516342b65db2969edf200390994bbbda23654c648f85dcc99f9f2d217d3d59a72e0f58227be7b4746529dcfa54ba26d8188ba9f14a57c9ab00015d7283fade2
8dfc2f30dea6bde0f74d23691377f248966011ab Test rpc_help.py failed: Check whether ZMQ is enabled or not. (Kvaciral)
Pull request description:
/test/functional/rpc_help.py checks for the zmq-category even while zmq may be disabled (in /test/config.ini) , I have added a check function to test_framework.py that can be used whether to determine to include zmq in a test or not.
Tree-SHA512: 6819050277e2dc875f8d9bf49a02291555cb7b301379dfb9d898e6d8e14bfb8eeb6bef8af46d07b5db45b2fe281b35ea7f98af9ffba703768658a69addbc81b1
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
907f142fc7e1d35f443be076367739faf11cc2cc rpc: change no wallet loaded message to be clearer (Andrew Chow)
Pull request description:
Changes the no wallet is loaded rpc error message to be clearer that no wallet is loaded and how the user can load or create a wallet. Also changes the error code from METHOD_NOT_FOUND to RPC_WALLET_NOT_FOUND as that makes more sense.
ACKs for top commit:
MarcoFalke:
review ACK 907f142fc7e1d35f443be076367739faf11cc2cc
kristapsk:
ACK 907f142fc7e1d35f443be076367739faf11cc2cc. In addition to standard tests, just in case tested that this doesn't break anything with JoinMarket.
meshcollider:
utACK 907f142fc7e1d35f443be076367739faf11cc2cc
Tree-SHA512: 4b413e6ab5430ec75a79de9db6583f2f3f38ccdf71aa373d8386a56e64f07f92200c8107c8c82c92c7c431d739615977c208b771a24c5960fa8676789b5497a2
56b018ca7f37d25041b74f1bec305bdf54a55b9b test: Fix flaky wallet_basic test (Fabian Jahr)
Pull request description:
Fixes#19853
I investigated the issue in #19876 and I still intend to fix the underlying issue of a race when using wallet RPCs right after starting a node in that PR. However, since that is a bit more complicated than I initially thought it makes sense to merge the fix of the test so the intermittent test failures stop. This fix in the test is going to be needed, either way, #19876 will only provide an error where before it was reporting a false balance.
Top commit has no ACKs.
Tree-SHA512: 52bb2388a3e77aa20d26ab0fd45796bc1781483b1cffe49cbb44e2488a72e76998edfb1198495373f9c6fd2ec26064d4176bd1a64dd59806622d5e50a4f4e870
5067c5acc30c5cf87496c1bf8eb03712cc66b206 [test] Add test for getblockheader verboseness (Torhte Butler)
Pull request description:
Improve test coverage by adding a test for getblockheader with verbose argument set to false.
ACKs for top commit:
theStack:
ACK 5067c5acc3
Tree-SHA512: e55593f1026a89dc7b796fa985b4cbcdb596e91d80d42dfb0660bda1692aaa35749ec29f9cd7032803f6225afb323f085df1ef6a9982de87be8e098f7253cdd5
2be35725069fd4c589497b93e09e1c6db6946372 test: Fix IPv6 check on BSD systems (nthumann)
Pull request description:
I noticed that `test_ipv6_local()` always returns `False` on macOS or FreeBSD, even though IPv6 is working perfectly fine. This causes `test/functional/rpc_bind.py --ipv6` and `test/functional/feature_proxy.py` to skip their run.
Apparently, there's a check if the port number is `0` (see [here](64881da478/sys/netinet6/udp6_usrreq.c (L248)) or [here](8f02f2a044/bsd/netinet6/udp6_usrreq.c (L282))), while Linux has no problem with this.
This is fixed by specifying any other port number than `0`, e.g. `1`. Still, because of `SOCK_DGRAM`, no actual connection is made.
ACKs for top commit:
fanquake:
ACK 2be35725069fd4c589497b93e09e1c6db6946372 - nice improvement. I checked that with this change ipv6 related tests in `feature_proxy.py` are being run.
theStack:
ACK 2be35725069fd4c589497b93e09e1c6db6946372
Tree-SHA512: 8417c2d3cf71050529f3fa409a03872040fe5d249eae4172f276e62156e505a20d375b963712a186c9ad7967d8a497b5900d327c74a9693f68c33063871d4691
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
88a79cb436b30b39d37d139da723f5a31e9d161b fix converttopsbt permitsigdata arg, add basic test (Gregory Sanders)
Pull request description:
The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior.
Resolves https://github.com/bitcoin/bitcoin/issues/14355
Tree-SHA512: 5157a74b8ddebd7d836fba96765c4d7ed15a73d4289817353d3566a0f6803bd4bbc3f936735c517c7a83a6cbdb4052b9c61d23f6cc4ad00a6077278cd51adbd4
7a6627ae87b637bf32c03122865402bd71adf0d1 Fix mining to an invalid target + ensure that a new block has the correct hash internally in Python tests (Samer Afach)
Pull request description:
Test with block 47 in the `feature_block.py` creates a block with a hash higher than the target, which is supposed to fail. Now two issues exist there, and both have low probability of showing up:
1. The creation is done with `while (hash < target)`, which is wrong, because hash = target is a valid mined value based on the code in the function `CheckProofOfWork()` that validates the mining target:
```
if (UintToArith256(hash) > bnTarget)
return false;
```
2. As we know the hash stored in CBlock class in Python is stateful, unlike how it's in C++, where calling `CBlock::GetHash()` will actively calculate the hash and not cache it anywhere. With this, blocks that come out of the method `next_block` can have incorrect hash value when `solve=False`. This is because the `next_block` is mostly used with `solve=True`, and solving does call the function `rehash()` which calculates the hash of the block, but with `solve=False`, nothing calls that method. And since the work to be done in regtests is very low, the probably of this problem showing up is very low, but it practically happens (well, with much higher probability compared to issue No. 1 above).
This PR fixes both these issues.
Top commit has no ACKs.
Tree-SHA512: f3b54d18f5073d6f1c26eab89bfec78620dda4ac1e4dde4f1d69543f1b85a7989d64c907e091db63f3f062408f5ed1e111018b842819ba1a5f8348c7b01ade96
d6d2602a32251c1017da88b47c801b7283c66ce3 add: test that transactions expire from mempool (0xb10c)
Pull request description:
This adds the functional test `mempool_expiry.py` covering mempool transaction expiry. Both the default `DEFAULT_MEMPOOL_EXPIRY` of 336 hours (two weeks, set in #9312) and the user definable mempool expiry via the `-mempoolexpiry=<n>` command line option are tested. The test checks that descendants of expired transactions are removed as well.
*Notes for reviewers*
- `LimitMempoolSize()` (which is the only caller of `CTxMemPool::Expire()`) is only called when a transaction is added to the mempool. In order to test expiry of a transaction-that-should-expire, the mocktime is set and a random transaction is broadcast to trigger `LimitMempoolSize()`. The transaction-that-should-expire is then checked for expiry. LMK if there is another way, but I don't think there is.
ACKs for top commit:
MarcoFalke:
ACK d6d2602a32251c1017da88b47c801b7283c66ce3
theStack:
ACK d6d2602a32
promag:
Code review ACK d6d2602a32251c1017da88b47c801b7283c66ce3.
Tree-SHA512: eb68cd9e2d870872b8e8e1522fed8954fb99cc9e4edda4b28bb2a4e41cddbc53fe6f7d9c090f1e0e98ab49beb24bf37ff3787a9e9801a95e8ae9ca9eb34fe6f0
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
