fa8fe5b69669b4d86e0d0970d68502abee8785f3 scripted-diff: Use new python 3.7 keywords (MarcoFalke)
fa2a23548aa9656e397189d8da844657709fb831 Revert "contrib: Fix capture_output in getcoins.py" (MarcoFalke)
dddd462137a85225955d4c2bcdb52e1e4235bff0 Bump minimum python version to 3.7 (MarcoFalke)
Pull request description:
While there is nothing that requires a bump, it may require less maintenance to drop python3.6 support. Python3.7 is available through the package manager on all currently supported operating systems.
ACKs for top commit:
jamesob:
ACK fa8fe5b696
hebasto:
ACK fa8fe5b69669b4d86e0d0970d68502abee8785f3
Tree-SHA512: f6e080d8751948bb0e01c87be601363158f345e8037b70ce7e1bc507c611eb61600e4f24f1d2f8a6e7e44877ab09319302869e33ce8118c4c4f71fc89c0a1198
fad56f7dd6ecac772e1bf590b38cd34ba67db5d7 doc: Properly report optional RPC args (MarcoFalke)
fa09cb60865a20cc3bdc84b4001fa503f1061319 refactor: Introduce is_top_level_arg (MarcoFalke)
fab92a5a5ab45227dbb0aa6d2cf4557b157b17e6 refactor: Remove const to fix performance-move-const-arg clang-tidy errors (MarcoFalke)
Pull request description:
`OMITTED_NAMED_ARG` and `OMITTED` are a confusing burden:
* It puts the burden on developers to pick the right one of the two
* They can be interchanged without introducing a compile failure or other error
* Picking the wrong one is leading to incorrect docs
* They are redundant, because the correct one can already be determined by the surrounding type
Fix all issues by making them an alias of each other; Pick the right one based on the outer type.
ACKs for top commit:
fanquake:
ACK fad56f7dd6ecac772e1bf590b38cd34ba67db5d7
Tree-SHA512: 6e7193a05a852ba1618a9cb3261220c7ad3282bc5595325c04437aa811f529a88e2851e9c7dbf9878389b8aa42e98f8817b7eb0260fbb9e123da0893cbae6ca2
376e01b382679b49d5c8464c869d14eca23ab4b9 doc: add databases/py-sqlite3 to FreeBSD test suite deps (fanquake)
Pull request description:
Adds missing documentation. See also https://cirrus-ci.com/task/5639240319500288.
ACKs for top commit:
john-moffett:
ACK 376e01b382679b49d5c8464c869d14eca23ab4b9
Tree-SHA512: a7b8d0bae00c3538934d23eae207b7927a64e748eb228ac6c5754aee0e9b6796c0f39066c7d81cc009bf442c8b1a0c31739adde87d1ebc3445aed54dda47c9ce
Adds a Dockerfile configuration
that allows straightforward running of linters with compatible versions
locally. This removes a ton of annoyance when trying to appease CI,
because many of the linter versions are quite old and difficult to
maintain locally.
I realize that people may not be thrilled to more ancillary tooling to
the repo, but I think this makes a lot of sense given the linter
versions listed in this container configuration are dictated by this
repo (within the CI configuration), so having these things live in
two separate places is a recipe for version mismatches.
Eventually we can likely just use this container on CI directly to avoid
any chance of inconsistencies between local dev experience and CI.
faa05cd8ce9312b13553a7fea98072e700d4748a doc: Clarify debian copyright comment (MarcoFalke)
Pull request description:
Seems fragile to link to an external site for a list of "current" devs. Also, current devs shouldn't matter in this context. It might be better to explain where *all* contributors are found, so do that instead.
ACKs for top commit:
fanquake:
ACK faa05cd8ce9312b13553a7fea98072e700d4748a
john-moffett:
ACK faa05cd8ce9312b13553a7fea98072e700d4748a
Tree-SHA512: 0695c8da86b7c3efbd13e7c1d645528dbd402c18ddd63d8bc532d42de9aa4e40d0f04f3c1e1208806e74ca9fbe3cb967d5206ca59fec20c35cafd22271deecf4
6d0ab07e817725369df699791b9c5b2fae204990 refactor: use convenience fn to auto parse non-string parameters (stickies-v)
Pull request description:
Minimizes code duplication and improves function naming by having a single (overloaded) convenience function `ParseIfNonString` that both checks if the parameter is a non-string parameter and automatically parses the value if so.
ACKs for top commit:
aureleoules:
ACK 6d0ab07e817725369df699791b9c5b2fae204990
Tree-SHA512: 8cbf68a17cfbdce1e31a19916f447a2965c139fdef00c19e32a9b679f4a4015dfe69ceea0bbe1723711e1c5033ea8d4005d1f4485dfbeea22226140f8cbe8aa3
1dc0e4bc6f3bd156b3fcd942b80820e60b83fa61 rpc: remove optional from fStateStats fields (fanquake)
Pull request description:
These are no-longer optional after #26515, so remove the documentation, and no-op `fStateStats` checks.
ACKs for top commit:
dergoegge:
Code review ACK 1dc0e4bc6f3bd156b3fcd942b80820e60b83fa61
Tree-SHA512: 06d4550e866341b379bfdbc72d67d71a3b7ceceec06ebd4c5e6f178b75fe40cbf4aff51adba1bc52590e69e818cbdecb0366bf1528c59c5c3dff5bbdba8eac68
fa87d71872f1a6f85f5ac699290c45a6e3c8933a ci: Add missing lint dependency (MarcoFalke)
Pull request description:
Also, document each dependency.
Adding `gpg` avoids errors when running a release or dev branch in the CI:
01ec5308bf/ci/lint/06_script.sh (L30-L42)
```
bash: line 1: gpg: command not found
```
https://cirrus-ci.com/task/4582854860996608?logs=lint#L185
ACKs for top commit:
hebasto:
ACK fa87d71872f1a6f85f5ac699290c45a6e3c8933a
Tree-SHA512: 869a3d2feab764b2c8d47d481359680a1d2c54a33b13ca26c5f8ce56cf2f368d4c74637dcbc53fdbf323f10940965c1c0e592e2fb4ce725d5cd467e77e62b6e5
faddb7373ad3ff22f88ccc97913168ec76073618 ci: Bump --combinedlogslen to debug intermittent issues (MarcoFalke)
Pull request description:
May help to debug intermittent issues such as https://github.com/bitcoin/bitcoin/issues/26808
ACKs for top commit:
fanquake:
ACK faddb7373ad3ff22f88ccc97913168ec76073618 - if this is going to improve the chance of tracking down intermittent failures.
Tree-SHA512: f844856ede71b9fb816c39bfba6241e35480db71bdc2e534d4746a666114bfc82f9dea804f70201fbf8af32eb579b7eab3c164a0bb2f77268b5554467ff6e97d
87a08cba43f8dc427efccbd45d28acc652db2cb6 build: move rpc/request from util lib to common (fanquake)
Pull request description:
This is JSON RPC related code that doesn't need to be in util, and should not be required by the kernel.
ACKs for top commit:
TheCharlatan:
ACK 87a08cba43f8dc427efccbd45d28acc652db2cb6
Tree-SHA512: 5f335be9f0f9ff02eff073af47558ecf505c1392c05f18ca24a065b12b8d92529ec3942d84978cc5028c38369c496ed0243653e1fa26d4db2fae26dfe55c3d65
4bb91be1245666ff62b6a1e683c6ba5a5651b544 debian: remove nonexistent files from copyright (fanquake)
Pull request description:
The removed files were dropped during a secp256k1 subtree update.
Top commit has no ACKs.
Tree-SHA512: 19ef1cf76908b5468265cc25b76abf8cf3a1dd0d5f7390f9cf4c5cd4c421c8cb04b5991ded7102add896d06555696a8059df37fd1d8f7374487a12dfa594c9cd
The previous logic would call it once for serializing into the filestream,
and then again for serializing into the hasher. If AddrMan was changed
in between these calls by another thread, the resulting peers.dat would
be corrupt with non-matching checksum and data.
Fix this by using HashedSourceWriter, which writes the data
to the underlying stream and keeps track of the hash in one go.
123043e99cf3aab9eef7e381b133477b518ac4d0 ci: Bump lint task image to Ubuntu Jammy (Hennadii Stepanov)
9b86114058b3931c5b4ddc931a8a2cfdc8b085b6 ci: Use pyenv's `python-build` to install Python in lint task (Hennadii Stepanov)
Pull request description:
This PR:
- is an alternative to bitcoin/bitcoin#26581 and bitcoin/bitcoin#26637
- closesbitcoin/bitcoin#26548
Key advantages of this PR over others:
- it uses pyenv's `python-build` [standalone](https://github.com/pyenv/pyenv/tree/master/plugins/python-build#using-python-build-standalone)
- requires no additional computational resources
Note for testing. The lint task must success regardless of whether the `python_cache` is populated or invalidated.
ACKs for top commit:
MarcoFalke:
ACK 123043e99cf3aab9eef7e381b133477b518ac4d0
fanquake:
ACK 123043e99cf3aab9eef7e381b133477b518ac4d0
Tree-SHA512: ba0fcdd4f2939a59692b173dcd1f5704444cfcfbb8111538c6f8160056d0536bba250e4f9b0f8c66f8b454e52034bf36ffe6afae76cdc0f7cc5b58b576d790ba
fa6b4021142154f52fc35360e409907360808801 test: Run mempool_packages.py with MiniWallet (MarcoFalke)
fa448c27d2cdd70698a4188c7478996ff8804556 test: Return fee from MiniWallet (MarcoFalke)
faec09f240213e8540e8559a3b4396ee93950cb4 test: Return chain of MiniWallet txs from MiniWallet chain method (MarcoFalke)
faa12d4ccd2ed14a4b892816b4771d7ff8d0d8a0 test: Refactor MiniWallet sign_tx (MarcoFalke)
fa2d82103fcd3b9084edd2268443f61ca5af969c test: Return wtxid from create_self_transfer_multi (MarcoFalke)
Pull request description:
This allows to run the test even when no wallet is compiled in.
Also, it is a lot nicer to read now.
ACKs for top commit:
glozow:
reACK fa6b402
Tree-SHA512: de0338068fd51db01d64ce270f94fd2982a63a6de597325cd1e1f11127e9075bd4aeacace0ed76d09a2db8b962b27237cf11edb4c1fe1a01134d397f8a11bd05
06fc29326b6c17204b3ed179cdb07071ac18183d refactor: Remove duplication of clang-tidy's check names (Hennadii Stepanov)
Pull request description:
This PR removes duplication of `clang-tidy`'s check names.
No behavior change.
Split up from https://github.com/bitcoin/bitcoin/pull/26642 as [requested](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1385351923).
ACKs for top commit:
fanquake:
ACK 06fc29326b6c17204b3ed179cdb07071ac18183d
Tree-SHA512: a21bef3d7d7201e14565b526af2eae7a90cf0f792803704a80a70a4c78f07ef2a2eef6a8dced80361efbf13291ecccb0977378b9532fc30970a2070426e4d82c
fa95f2033a26fb18914c2aa5e5e18650d32e6e4b doc: Fix incorrect sendmany RPC doc (MarcoFalke)
fa96f93f056efffe3de5e5141f0e0927f8eb0abd test: Add test for missing and omitted required arg (MarcoFalke)
Pull request description:
This enables the skipped type check for `sendmany` and fixes the resulting error.
Also, there is an unrelated test-only commit.
ACKs for top commit:
fanquake:
ACK fa95f2033a26fb18914c2aa5e5e18650d32e6e4b
Tree-SHA512: 6f9992786472d3927485a34e918db76824cfb60fa96f42cc9c3cdba7074fe08c657bd77cb3e748432161a290f2dcf90bb0ece279904bd274c529119e65fa0959
The warnings look like:
src/rpc/util.h:192:19: error: std::move of the const variable 'name' has no effect; remove std::move() or make the variable non-const [performance-move-const-arg,-warnings-as-errors]
: m_names{std::move(name)},
^~~~~~~~~~ ~
This enables the type check and fixes the wrong docs.
Otherwise the enabled check would lead to test errors, such as:
> "wallet_labels.py", line 96, in run_test
> node.sendmany(
>
> test_framework.authproxy.JSONRPCException:
> JSON value of type null is not of expected type string (-3)
603d29519931e6ab6a58db09b9d3eb9b355fe566 test: wallet: add coverage for `-spendzeroconfchange` setting (Sebastian Falbesoner)
50112034bcdd177311679610ef2f3c2c42d6819e test: remove `-spendzeroconfchange` setting from mempool_limit.py (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `-spendzeroconfchange` setting (in particular the non-default case `=0`). Note that in contrast to the name, the setting does not only apply to change outputs, but in fact to _all_ unconfirmed outputs that we sent to ourselves, i.e. we can trigger the testing path simply with a single recipient address. The first commit removes the setting from the functional test mempool_limit.py, where it doesn't have any effect, since the test was changed to use MiniWallet in commit dddca3899c4738e512313a85aeb006310e34e31f.
ACKs for top commit:
brunoerg:
crACK 603d29519931e6ab6a58db09b9d3eb9b355fe566
Tree-SHA512: 15d9c8bd5eb37c6b228bf887eb27debee0a391c82356662785da4553ee2558e611834c3936ef7136812b46f877bab7aa5f3088bbd278b81f296bdda96cc8e1c3
7b7cd112444b996a8ae6a6edfed00bcee67546c8 clang-tidy, qt: Force checks for headers in `src/qt` (Hennadii Stepanov)
69eacf2c5ee1c84e92153b525fd4302aec0f5f2a clang-tidy, qt: Fix `modernize-use-default-member-init` in headers (Hennadii Stepanov)
Pull request description:
This PR split from bitcoin/bitcoin#26705 and contains only changes in `src/qt`.
Effectively, it fixes the clang-tidy's `modernize-use-default-member-init` errors, and forces clang-tidy checks for all headers in the `src/qt` directory.
ACKs for top commit:
jarolrod:
ACK 7b7cd112444b996a8ae6a6edfed00bcee67546c8
Tree-SHA512: 79525bb0f31ae7cad88c781e55091a21467c0485ddc1ed03ad62e051480fda3b3710619ea11af480437edba3c6e038f7c40edc6b373e3a37408c006d11b34686
7fdeb80441f451c8983632329594f4903af92729 build: allow NO_LIBEVENT=1 in depends (fanquake)
0cee156eee98686e04ac9910b8b7b5ba5b66ee2d build: allow NO_BOOST=1 in depends (fanquake)
Pull request description:
Prerequisite for removing `install_db4.sh`. So we can invoke `make -C depends/ NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_ZMQ=1 NO_UPNP=1 NO_USDT=1` and get a prefix with only bdb headers/libs.
ACKs for top commit:
hebasto:
ACK 7fdeb80441f451c8983632329594f4903af92729, tested on Ubuntu 22.04:
Tree-SHA512: 9dd595a61b3178f0296b98d780be2ac37a0d498a75ed222fb1408b84c3e6d179ea18fd96025ea6ce250216be6843dfa46621021e8bc4601a4d1bb8b20387b9e4
fa9f6d7bcdba5f18c46fff1dcc0ac6d3dd8db75d rpc: Run type check against RPCArgs (MarcoFalke)
faf96721a66dcc215ea9d6affb30f9a00cc37000 test: Fix wrong types passed to RPCs (MarcoFalke)
Pull request description:
It seems brittle to require `RPCTypeCheck` being called inside the code logic. Without compile-time enforcement this will lead to places where it is forgotten and thus to inconsistencies and bugs. Fix this by removing the calls to `RPCTypeCheck` and doing the check internally.
The changes should be reviewed as refactoring changes. However, if they change behavior, it will be a bugfix. For example the changes here happen to also detect/fix bugs like the one fixed in commit 3b5fb6e77a93f58b3d03b1eec3595f5c45e633a9.
ACKs for top commit:
ajtowns:
ACK fa9f6d7bcdba5f18c46fff1dcc0ac6d3dd8db75d
Tree-SHA512: fb2c0981fe6e24da3ca7dbc06898730779ea4e02ea485458505a281cf421015e44dad0221a04023fc547ea2c660d94657909843fc85d92b847611ec097532439
6bd098a838881ba615c01354560a70d47256f90f test: simplify tests by using the pre-mined chain (kouloumos)
42029a7fd42e93426bdfb40d1918d739d20962af test: remove redundant blocks generation logic (kouloumos)
0377d6bb420b9a2fb79af0ebe088d2b20ec808a5 test: add `rescan_utxos` in MiniWallet's initialization (kouloumos)
Pull request description:
When a pre-mined blockchain is used (default behavior), it [contains coinbase outputs in blocks 76-10](07c54de550/test/functional/test_framework/test_framework.py (L809-L813)) to [the MiniWallet's default address](07c54de550/test/functional/test_framework/wallet.py (L99-L101)). That's why we always* `rescan_utxos()` after initializing the MiniWallet, in order for the MiniWallet to account for those mature UTXOs.
> The tests following this usage pattern can be seen with:
> ```git grep -n "MiniWallet(" $(git grep -le "rescan_utxos()" $(git grep -Le "self.setup_clean_chain = True"))```
**This PR adds `rescan_utxos()` inside MiniWallet's initialization to simplify usage when the MiniWallet is used with a pre-mined chain.**
### secondary changes
- *There are a few tests that use the pre-mined blockchain but do not `rescan_utxos()`, they instead generate new blocks to create mature UTXOs.
> Those were written before the `rescan_utxos()` method was introduced with https://github.com/bitcoin/bitcoin/pull/22955 (fac66d0a39cb0b4bc565b57087ba84dd932e9b6d) and can be seen with:
> `git grep -n "MiniWallet(" $(git grep -Le "rescan_utxos()" $(git grep -Le "self.setup_clean_chain = True"))`
>
After including `rescan_utxos()` inside MiniWallets initilization, this blocks generation logic is not needed as the MiniWallet already accounts for enough mature UTXOs to perform the tests. **Therefore the now redundant blocks generation logic is removed from those tests with the second commit.**
- The rest of the MiniWallet tests use a clean chain (`self.setup_clean_chain = True`) and can be seen with
`git grep -n "MiniWallet(" $(git grep -le "self.setup_clean_chain = True")`
From those, there are a few that start from a clean chain and then create enough mature UTXOs for the MiniWallet with this kind of logic:
07c54de550/test/functional/mempool_expiry.py (L36-L40)
**Those tests are simplified in the third commit to instead utilize the mature UTXOs of the pre-mined chain.**
ACKs for top commit:
MarcoFalke:
ACK 6bd098a838881ba615c01354560a70d47256f90f 🕷
theStack:
re-ACK 6bd098a838881ba615c01354560a70d47256f90f
Tree-SHA512: 7f9361e36910e4000c33a32efdde4449f4a8a763bb42df96de826fcde469f9362f701b8c99e2a2c482d2d5a42a83ae5ae3844fdbed187ed0ff231f386c222493
5ca7a7be76f2521dca895daa70949fd11df0844c rpc: Return accurate results for scanblocks (Aurèle Oulès)
Pull request description:
Implements #26322.
Adds a `filter_false_positives` mode to `scanblocks` to accurately verify results from blockfilters.
If the option is enabled, pre-results given by blockfilters will be filtered out again by checking vouts and vins of all transactions of the relevant blocks against the given descriptors.
### Master
```bash
./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]'
{
"from_height": 0,
"to_height": 2376055,
"relevant_blocks": [
"000000000001bc35077dec4104e0ab1f667ae27059bd907f9a8fac55c802ae36",
"00000000000120a9c50542d73248fb7c37640c252850f0cf273134ad9febaf61",
"0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed",
"0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3"
]
}
```
### PR (without `filter_false_positives` mode)
Same as master
```bash
./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' filter_false_positives=false
{
"from_height": 0,
"to_height": 2376055,
"relevant_blocks": [
"000000000001bc35077dec4104e0ab1f667ae27059bd907f9a8fac55c802ae36",
"00000000000120a9c50542d73248fb7c37640c252850f0cf273134ad9febaf61",
"0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed",
"0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3"
]
}
```
### PR (with `filter_false_positives` mode)
```bash
./src/bitcoin-cli -testnet -named scanblocks action=start scanobjects='["addr(tb1qcxf2gv93c26s6mqz7y6etpqdf70zmn67dualgr)"]' filter_false_positives=true
{
"from_height": 0,
"to_height": 2376058,
"relevant_blocks": [
"0000000000000082f7af3835da8b6146b0bfb243b8842f09c495fa1e74d454ed",
"0000000000000094c32651728193bfbe91f6789683b8d6ac6ae2d22ebd3cb5d3"
]
}
```
Also adds a test to check that the blockhash of a transaction will be included in the `relevant_blocks` whether the `filter_false_positives` mode is enabled or not.
ACKs for top commit:
achow101:
ACK 5ca7a7be76f2521dca895daa70949fd11df0844c
theStack:
re-ACK 5ca7a7be76f2521dca895daa70949fd11df0844c
furszy:
Code review ACK 5ca7a7be
Tree-SHA512: e8f3cceddddd66f59509717b6314d89e2fef241e13cee81b18fd95e8362cbb95cc40f884342ce6cf892a86febd9e2d434afce05d51892240e67f72ae991852e7
cfe5aebc79c510bd2156e199c3324d7ee1f8d2ad rpc: add minconf and maxconf options to sendall (ishaanam)
a07a413466a0edd47eab9189b46a70aafbbe22b7 Wallet/RPC: Allow specifying min & max chain depth for inputs used by fund calls (Juan Pablo Civile)
Pull request description:
This PR adds a "minconf" option to `fundrawtransaction`, `walletcreatefundedpsbt`, and `sendall`.
Alternative implementation of #14641Fixes#14542
Edit: This PR now also adds this option to `send`
ACKs for top commit:
achow101:
ACK cfe5aebc79c510bd2156e199c3324d7ee1f8d2ad
Xekyo:
ACK cfe5aebc79c510bd2156e199c3324d7ee1f8d2ad
furszy:
diff ACK cfe5aebc, only a non-blocking nit.
Tree-SHA512: 836e610926eec3a62308fba88ddbd6a13d8f4dac37352d0309599f893cde9c1df5e9c298fda6e076493068e4d213e4afa7290a9e3bdb5a95a5d507da3f7b59e8
ApplyArgsManOptions does not need to set default values for missing
arguments, these are already defined in the BlockAssembler::Options.
This commit changes the interface of ApplyArgsManOptions(). If
ApplyArgsManOptions() is called again after a option is changed,
this option will no longer be reset to the default value.
There is no observed behaviour change due to how
ApplyArgsManOptions() is currently used, and the new interface is
consistent with e.g. ValidationCacheSizes and MemPoolLimits.
Add Options as a member to BlockAssembler to avoid having to assign
all the options individually.
Additionally brings the struct more in line with how we typically
define default and ArgManager values, as e.g. with
ChainstateManager::Options and and CTxMemPool::Options
