dac44fc06f init: disallow reindex-chainstate with optional indexes (Martin Zumsande)
62e14285f9 doc: Add note that -reindex will rebuild optional indexes (Martin Zumsande)
Pull request description:
When started together with `-reindex-chainstate`, currently coinstatsindex gets corrupted and the blockfilterindex flatfiles duplicated. See the OP of #24630 for more a more detailed explanation on why this happens.
This is an alternative to #24630 which does not wipe and rebuild the indexes but returns an `InitError` when they are activated, thus requiring the user to deactivate them temporarily until the `-reindex-chainstate` run is finished.
This also disallows `-reindex-chainstate` in combination with `-txindex`, which is not leading to corruption, but currently still rebuilds the index unnecessarily and unexpectedly.
As a long-term goal, it would be desirable to have the indexes tolerate `reindex-chainstate` by ignoring their `BlockConnected` notifications (there is discussion in #24630 about this) or possibly move `reindex-chainstate` option into a `bitcoin-chainstate` executable, which could also solve the problem. But these would be larger projects - until then, it might be better to disallow the interaction than having corrupted indexes.
The first commit adjusts the `-reindex` doc to mention that this option does rebuild all active indexes.
ACKs for top commit:
ryanofsky:
Code review ACK dac44fc06f. Just fixed IsArgSet call and edited error messages since last review
Tree-SHA512: c1abf7d350648ae227c3fd6c95d9a54c3bac9de70915275dea1c87cca6d9a76a056c0e306d95ef8cfe4df1f8525b418e0e7a4f52ded3be464041c0dc297f8930
172c2333f0 Porting lint-assertions.sh to lint-assertions.py (hiago)
Pull request description:
This PR is converting `test/lint/lint-assertions.sh` to `test/lint/lint-assertions.py`. It's an item of #24783.
ACKs for top commit:
laanwj:
Tested ACK 172c2333f0
Tree-SHA512: 94d5b03acfeaf2303fad95d489d6c3aa7bd655889ddaa807cc97e0613b8eb8f5ef094feee2a98d974606890deb554e76490a5c523d64eb5bc55afa6a43221aae
79635c79e0 lint: Convert lint-circular-dependencies.sh to Python (Smlep)
Pull request description:
Here is a port of `/test/lint/lint-circular-dependencies.sh` to a Python-script as part of the request of https://github.com/bitcoin/bitcoin/issues/24783.
It aims to provide the same output as the bash version.
ACKs for top commit:
laanwj:
Tested ACK 79635c79e0
Tree-SHA512: f18077018f1229dd933cfe2bf0cfe7dc7d6538961c96a83c7a1f05e0cec4b068ca05502d68410d2aa4b6864523424db386e38233735190525904c2a8e9d2ba13
267684ee34 lint: convert format strings linter test to python (Eunoia)
Pull request description:
Refs #24783
Attempted to keep the style and flow of implementation as it is.
### Additional Notes(Optional):
1. There is scope of improvement on how the related files are fetched. In this `git grep` with `subprocess` is still used as I found it to be the simplest. Any pointers on this are appreciated.
2. Removed sort operation on the matching files as I couldn't think of any strong arguments to have it. Any pointers on this are appreciated.
3. Not important, but one small detail is that the previous implementation was storing matched files for all the `function_names` iterated so far. Fixed that in this PR.
ACKs for top commit:
laanwj:
Code review ACK 267684ee34
Tree-SHA512: 54ceae0c3501e561fdd9c5167b2dd8dd06da1b3697a077a042210970ce7004bda8c4e19abb1905ee64cbdce635f0a078508da645846ae7e81c016091f3f02458
035eef4be6 lint: Convert lint-python-utf8-encoding.sh to Python (Dimitri)
Pull request description:
A port of `test/lint/lint-python-utf8-encoding.sh` to a Python-script as part of the request of #24783. Checked for output-consistency.
ACKs for top commit:
laanwj:
Code review ACK 035eef4be6
Tree-SHA512: a8a2f505bf7953d318837182101346c44e73cfd1bf3b5342ff1400fb1c67c5292519fa99db1035da87cf27fb5f5ac5d28871bf55a1c085b5f8a3bb33ff0fa3fb
3043a1bc9d lint: Make known violations more specific in lint-locale-dependence (Dimitri)
229917d3d4 lint: Convert lint-locale-dependence.sh to Python (Dimitri)
Pull request description:
A port of `test/lint/lint-locale-dependence.sh` to a Python-script as part of the request of #24783. Checked for output-consistency.
ACKs for top commit:
laanwj:
Tested and code review ACK 3043a1bc9d
Tree-SHA512: 80555cf7aac156bab5488f85098731d1c12a42667fe7d0df0c35487ab8fc951654a70a15351a759282eabab8319f5aabd8bdb153412b9edc3a9033bef64fd609
9f5ab670e7 tests: Use descriptor that requires both legacy and segwit (Andrew Chow)
8a04a386f7 tests: Calculate input weight more accurately (Andrew Chow)
Pull request description:
The external input tests with specifying input weight would sometimes result in a test failure because it would add 2 to the calculated byte size in order to account for some of the variation in signature and script sizes. However 1 in 128 signatures are actually 1 byte smaller than we expect, so the difference between the actual signature size and our calculated size becomes 3 bytes which is outside of the tolerance of `assert_fee_amount` and would thus cause the test failure.
To resolve this, the 2 byte buffer is reduced to 1 byte, so in the above scenario, the difference is 2 bytes which is within the tolerance of `assert_fee_amount`. Additionally, instead of putting a fixed size that we assume is the correct size for the length of the compact size length prefix of data, we actually get the length of the compact size uint.
Lastly, the size calculation for a scriptWitness was simply incorrect and used fields that did not exist. This is fixed, and the test slightly modified so that it also produces a scriptWitness.
Fixes#24151
ACKs for top commit:
jonatack:
re-ACK 9f5ab670e7
glozow:
code review ACK 9f5ab670e7
Tree-SHA512: b7c7ffe8fb0c07bc9e72fbff1f9ef57ee01a57c56bf54b8873345c8b9572c3ce9402b24dc211910b478114a9e6420faef5a4bf8866f38c299971354e54ec4745
ee02c8bd9a util/check: Add CHECK_NONFATAL identity function, NONFATAL_UNREACHABLE AND UNREACHABLE macros (Aurèle Oulès)
Pull request description:
This PR replaces the macro `CHECK_NONFATAL` with an identity function.
I simplified the usage of `CHECK_NONFATAL` where applicable in `src/rpc`.
This function is useful in sanity checks for RPC and command-line interfaces.
Context: https://github.com/bitcoin/bitcoin/pull/24804#discussion_r846182474.
Also adds `UNREACHABLE_NONFATAL` macro.
ACKs for top commit:
jonatack:
ACK ee02c8bd9a
MarcoFalke:
ACK ee02c8bd9a🍨
Tree-SHA512: 3cba09223cd7b22e62fe5d0b46c4a024c1d9957d4268ba6d3fb07fcc0a5854fc0886bb3266184e6a7df5df91373b3e84edd6adf6999c4e934aeef8c043b01aa2
bef61496ab test: compare `/mempool/contents` response with `getrawmempool` RPC (brunoerg)
5bc5cbaf31 doc: add reference to `getrawmempool` RPC in `/mempool/contents` REST doc (brunoerg)
Pull request description:
This PR is similar to #24797, it compares `/mempool/contents` REST response with `getrawmempool` RPC (verbose=True) since they use the same `MempoolToJSON` function.
Also, adds a reference to `getrawmempool` RPC help to get details about the fields from `/mempool/contents`.
ACKs for top commit:
0xB10C:
ACK bef6149
Tree-SHA512: b7e9e9c765ee837986ba167b9234a9b95c9ef0a9ebcc2a03d50f6be6d3aba1480bd77c78111d95df1e4023cde6dfc64bf1e7908d9e5b6f96ca46b76611a4a9b4
fa2153b05b test: Remove unused taproot node from wallet_taproot.py (MarcoFalke)
Pull request description:
Now that the wallet considers taproot always active after commit 064c729a96, there is no need to test for it.
ACKs for top commit:
dunxen:
Code review ACK fa2153b
brunoerg:
crACK fa2153b05b
Tree-SHA512: 24e4a66e43d1391acb63fd0c0c52677b0eef7f618b87a5b1a75224a9be58c9c3f8bba2de3b7510f25a686865b027f7f535e653d40d519d0e00ace38f0c7aba0c
67b41678c8 lint: Convert lint-includes.sh to Python (Dimitri)
Pull request description:
A port of `test/lint/lint-includes.sh` to a Python-script as part of the request of #24783. Checked for output-consistency.
ACKs for top commit:
KevinMusgrave:
Tested ACK 67b41678c8
Tree-SHA512: 05b4b114dc101e571004aee8aea1480e4dda1dc645426100649e9cb81e56e8667f88d6d5646a9860ea1c7abc36754eda2a77ec10156c54b62db00e2c00b8ceae
917a89a814 test: use MiniWallet for p2p_segwit.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (p2p_segwit.py) to be run even with the Bitcoin Code wallet by using the MiniWallet instead, as proposed in https://github.com/bitcoin/bitcoin/issues/20078.
This change only affects the subtest `test_superfluous_witness`. Note that instead of creating a raw transaction first and then signing it, we go the other direction here: MiniWallet creates a transaction spending a segwit v1 output (i.e. including a witness), then we turn it into a raw transaction by dropping the witness. Therefore, the debug log asserts are swapped.
Top commit has no ACKs.
Tree-SHA512: 163a93a527f60100487f0aff49a9d7baf392ceb4417c54521157b2678685f5728dd751a9747c6cf51666aae78252dd3bc44130e659f7a1262ec1c86e30225622
47b66ac4ac lint: Convert Python linter to Python (Fabian Jahr)
Pull request description:
The outputs provided by the Python version should be exactly the same as the ones from the shell version.
There is small improvement here: Previously only the dependency of `flake9` was checked, now all dependencies are checked before running.
I also tried to mostly follow the [recommendations here](https://github.com/bitcoin/bitcoin/pull/24766#pullrequestreview-932953476) but happy to make more changes if there is still room for improvement.
ACKs for top commit:
laanwj:
Tested ACK 47b66ac4ac
Tree-SHA512: 1630188e176c1063b8905669b76682b361a858cde6990ab17e51ad4333bf376eab796050cdb9f2967b84f1f74379d9e860c4258561b1964e1a47183c593e5bb4
f27fcd9bf4 lint: Convert lint-git-commit-check.sh to Python (Dimitri)
Pull request description:
A port of `/test/lint/lint-git-commit-check.sh` to a Python-script as part of the request of #24783 . Checked for output-consistency.
ACKs for top commit:
laanwj:
re-ACK f27fcd9bf4
Tree-SHA512: afc4a662f4aec1796c023b98a875c1591940ecdfc709eefe2df29d33e51e807c3c2e2b5c410aa3ad1cd3f6f8207f5c15b638637ff9f5659cafa7543bbe8a0bae
a75f6d86d1 lint: Convert lint-whitespace.sh to Python (Dimitri)
Pull request description:
A port of `/test/lint/lint-whitespace.sh` to a Python-script as part of the request of #24783 . Checked for output-consistency.
ACKs for top commit:
laanwj:
Code review and tested ACK a75f6d86d1
Tree-SHA512: 982041b0beb1b3866493ad523950c9a536a8b1ec79b773fe86dbc1166844c13a30b384e92025f845d45d25334f90f3abda5fa23f0f28e7c2cddc5e496f84c445
6f29409ad1 test: Add a test that creates a wallet with invalid parameters (w0xlt)
0359d9b6a3 Change wallet validation order (w0xlt)
Pull request description:
In the current code, the database is created before the last validation, which checks that passphrase is set and private keys are disabled.
Therefore, if this validation fails, it will result in an empty database and the user will not be able to recreate a wallet with the same name and with the correct parameters.
Behavior on the master branch:
```
$ ./src/bitcoin-cli -regtest -named createwallet wallet_name="invalid_wallet_01" disable_private_keys=true passphrase="passphrase"
error code: -4
error message:
Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.
$ ./src/bitcoin-cli -regtest -named createwallet wallet_name="invalid_wallet_01"
error code: -4
error message:
Wallet file verification failed. Failed to create database path '/home/w/.bitcoin/regtest/wallets/invalid_wallet'. Database already exists.
```
Behavior on the PR branch:
```
$ ./src/bitcoin-cli -regtest -named createwallet wallet_name="invalid_wallet_02" disable_private_keys=true passphrase="passphrase"
error code: -4
error message:
Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.
$ ./src/bitcoin-cli -regtest -named createwallet wallet_name="invalid_wallet_02"
{
"name": "invalid_wallet_01",
"warning": ""
}
```
ACKs for top commit:
achow101:
ACK 6f29409ad1
Tree-SHA512: d192955fc2285bf27ae5dd4c1b7cfd3d85441a7f3554b189b974aefb319c6b997543991dbb0ca2c8cb980f7058913a77cf0164c02e9b51ceb9c2cb601317c428
This change only affects the subtest `test_superfluous_witness`.
Note that instead of creating a raw transaction first and then
signing it, we go the other direction here: MiniWallet creates a
transaction spending a segwit v1 output (i.e. including a witness),
then we turn it into a raw transaction by dropping the witness.
Therefore, the debug log asserts are swapped.
88376c623c test: Test for disabling wallet flags (Andrew Chow)
17ab31aa46 rpc, wallet: setwalletflags warnings are optional (Andrew Chow)
Pull request description:
Trying to disable a wallet flag with `setwalletflag` results in `Internal bug detected: 'std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [ret](const RPCResult& res) { return res.MatchesType(ret); })'`. This occurs because the `warnings` field was not marked as optional. This PR makes `warnings` optional to avoid this error.
Also added a test case because apparently we didn't already have one.
ACKs for top commit:
w0xlt:
ACK 88376c6
Tree-SHA512: 4f5d3bebf0d022a5ad0f75d70c6562a43c7da6e39e9c3118733327d015c435e2c8d5004fdb039d42407dde5b21231a0f8827623d718abf611a1f06c15af5c806
Use raw string
Use re.search instead of grep in check_matching_test_names
Replaced bash commands in check_unique_test_names with python commands
Use set and sort output
Use set comprehension
Use .splitlines()
Call grep_boost_fixture_test_suite once
splitlines() once
Fixed copyright date
Use check_output() instead of run()
add encoding='utf8'
Use clearer code for getting duplicates
038d2a607f test: add test for signet miner script (Sebastian Falbesoner)
449b96ed97 test: add `is_bitcoin_util_compiled` helper (Sebastian Falbesoner)
dde33eca63 test: determine path to `bitcoin-util` in test framework (Sebastian Falbesoner)
Pull request description:
This PR adds a very basic test for the signet miner script (contrib/signet/miner). ~~It was based on #24553 (merged by now) which fixes a bug (and was also the motivation to write this test).~~
The test roughly follows the steps from https://en.bitcoin.it/wiki/Signet#Custom_Signet, except that the challenge key-pair is created solely with the test framework. Calibration is also skipped, the difficulty is simply set to the first mainnet target `0x1d00ffff` (see also https://bitcoin.stackexchange.com/a/57186).
ACKs for top commit:
laanwj:
re-ACK 038d2a607f
Tree-SHA512: 150698a3c0cda3679661b47688e3b932c9761e777fdd284776b867b485db6a8895960177bd02a53f838a4c9b9bbe6a9beea8d7a5b14825b38e4e43b3177821b3
The path is stored in `self.options.bitcoinutil`, points to
`src/bitcoin-util` by default and can be overrided with the
`BITCOINUTIL` environment variable.
e8e48fa82b Converted lint-python-mutable-default-parameters.sh to python (TakeshiMusgrave)
Pull request description:
This converts one of the linter scripts to Python. Reference issue: https://github.com/bitcoin/bitcoin/issues/24783
The approach is to just call git grep using subprocess.run.
Alternative approaches could be to use Python instead of git grep (I'm not sure how) or use ```pylint --disable=all --enable=W0102```, though that requires installation of pylint.
ACKs for top commit:
MarcoFalke:
review ACK e8e48fa82b
Tree-SHA512: 7f6f4887dee02c9751b225a6a131fb705868859c4a9af25bb3485cda2358650486b110f17adf89d96a20f212d7d94899922a07aab12c8dc11984cfd5feb7a076
494455f8a5 test: use MiniWallet for feature_fee_estimation.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (feature_fee_estimation.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in https://github.com/bitcoin/bitcoin/issues/20078. It takes use of the recently introduced methods `{create,send}_self_transfer_multi` (#24637) which allows to specify multiple UTXOs to be spent rather than only one. Very likely the test can still be simplified (e.g. coin selection in `small_txpuzzle_randfee`), but this is a first step.
ACKs for top commit:
ayush933:
tACK 494455f8 . The test runs successfully with the wallet disabled.
vincenzopalazzo:
tACK 494455f8a5
Tree-SHA512: 89789fc34a4374c79c4b90acd926ac69153aad655dab50450ed796f03c770bd675ad872e906f516f90e8d4cb40b83b55f3c78a94b13bfb8fe8f5e27624937748
