faf45d1f1f997c316fc4c611a23c4456533eefe9 http: Avoid crash when g_thread_http was never started (MarcoFalke)
fa12a37b27f0570a551b8c103ea6537ee4a8e399 test: Replace inline-comments with logs, pep8 formatting (MarcoFalke)
fa83b39ff3ae3fbad93df002915c0e5f99c104a9 init: Remove confusing and redundant InitError (MarcoFalke)
Pull request description:
Avoid a crash during shutdown when the init sequence failed for some reason
ACKs for top commit:
promag:
Tested ACK faf45d1f1f997c316fc4c611a23c4456533eefe9.
ryanofsky:
Code review ACK faf45d1f1f997c316fc4c611a23c4456533eefe9. Thanks for updates, this is much easier to parse for me now. Since previous reviews: split out and reverted some cleanups & replaced chmod with mkdir in test
hebasto:
ACK faf45d1f1f997c316fc4c611a23c4456533eefe9, tested on Linux Mint 19.3 with the following patch:
Tree-SHA512: 59632bf01c999e65c724e2728ac103250ccd8b0b16fac19d3a2a82639ab73e4f2efb86c78e63c588a5954625d8d0cf9545e2a7e070e6e15d2a54beeb50e00b61
66fe7b1a98c03f690dcf60d359baac124658aeae test: added test for upgradewallet RPC (Harris)
Pull request description:
This PR adds tests for the newly merged *upgradewallet* RPC.
Additionally, it expands `test_framework/util.py` by adding the function `adjust_bitcoin_conf_for_pre_17` to support nodes that don't parse configuration sections.
This test uses two older node versions, v0.15.2 and v0.16.3, to create older wallet versions to be used by `upgradewallet`.
Fixes https://github.com/bitcoin/bitcoin/issues/18767
Top commit has no ACKs.
Tree-SHA512: bb72ff1e829e2c3954386cc308842820ef0828a4fbb754202b225a8748f92d4dcc5ec77fb146bfd5484a5c2f29ce95adf9f3fb4483437088ff3ea4a8d2c442c1
fa03713e133e3017112fdd5c278e0c8643054578 test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) (MarcoFalke)
Pull request description:
actually (?) fix#18561
See most recent traceback https://travis-ci.org/github/bitcoin/bitcoin/jobs/674668692#L7062
I believe the reason the error is still there is that ConnectionResetError is derived from OSError:
ConnectionResetError(ConnectionError(OSError))
And IOError is an alias for OSError since python 3.3, see https://docs.python.org/3/library/exceptions.html#IOError
So fix that by renaming IOError to the alias OSError and move the less specific catch clause down a few lines.
ACKs for top commit:
jonatack:
ACK fa03713e133e3017112fdd5c278e0c8643054578
Tree-SHA512: 6e5b214ed9101bf8ebe7472dcc1f9e9d128e2575c93ec00c8d0774ae1a9b52a8c2a653a45a0eab8d881570b08dd5ffeddf5aca88a10438c366e1f633253cb0b5
38677274f931088218eeb1f258077d3387f39c89 rpc: settxfee respects -maxtxfee wallet setting (Fabian Jahr)
bda84a08a0ac92dff6cadc99cf9bb8c3fadd7e13 rpc: Add documentation for deactivating settxfee (Fabian Jahr)
Pull request description:
~~Closes 18315~~
`settxfee` can be deactivated by passing 0 as the fee. That does not seem to be documented for the user so this PR adds it in the description. The return value of a simple boolean seems also too simplified given the multiple dimensions that this deactivation feature enables. I.e. it does not seem intuitive if the returned boolean shows that the call succeeded or if means that `settxfee` is active. My suggested solution is to change the return value to a JSON object that included the "active" state and the currently set fee rate.
Examples:
```
$ src/bitcoin-cli settxfee 0.0000000
{
"active": false,
"fee_rate": "0.00000000 BTC/kB"
}
$ src/bitcoin-cli settxfee 0.0001
{
"active": true,
"fee_rate": "0.00010000 BTC/kB"
}
```
ACKs for top commit:
MarcoFalke:
ACK 38677274f931088218eeb1f258077d3387f39c89, seems useful to error out early instead of later #16257🕍
jonatack:
ACK 38677274f931088218eeb
meshcollider:
LGTM, utACK 38677274f931088218eeb1f258077d3387f39c89
Tree-SHA512: 642813b5cf6612abb4b6cb63728081a6bd1659d809e0149c8f56060b6da7253fee989b3b202854f3051df3773c966799af30b612648c466b099f00590f356548
cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a test: move sync_blocks and sync_mempool functions to test_framework.py (Roy Shao)
Pull request description:
This PR moves `sync_blocks` and `sync_mempool` out from `test_framework/util.py` to `test_framework/test_framework.py` so they can take contextual information of test framework into account.
* Change all reference callers to call functions from `test_framework.py`
* Remove `**kwargs` which is not used
* Take into account of `timeout_factor` when respecting timeout in function implementations.
* Pass all tests by running `./test/functional/test_runner.py`
fixes#18930
ACKs for top commit:
MarcoFalke:
ACK cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a , reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space 💫
Tree-SHA512: a79b2a3fa842fc26a7aacb834bb2aea88b3049916c0b754e60002a77ce94bb5954e0ea3b436bf268e9295efb62d721dfef263a09339a55c684ac3fda388c275e
fac3716b09bb9ee121db629873d9694a95cae942 test: check that peer is connected when calling sync_* (MarcoFalke)
Pull request description:
Without a connection there is no way to sync, so we can fail early and don't have to wait for the timeout
ACKs for top commit:
jonatack:
ACK fac3716b09bb9
Tree-SHA512: 12f771473c23e152dae4bfb201fadb2c3530cf439de64fea07d048734614543080a5d05c9c36e6e398c6a69c8279f609d34706599571814172a11bcfbea4a3b9
38c3dd9c706e7e84b2a4dbaf1424a3f1c3b694fc docs: Add notes on how to diasble rpc timeout in functional tests while attatching gdb. (codeShark149)
784ae096259955ea7a294114f5c021c9489d2717 test: Add capability to disable RPC timeout in functional tests. (codeShark149)
Pull request description:
Many times, especially while debugging RPC callbacks to core using gdb, the test timeout kicks in before the response can get back. This can be annoying and requires restarting the functional test as well as gdb attachment.
This PR adds a `--notimeout` flag into `test_framework` and sets the `rpc_timeout` accordingly if the flag is set.
The same effect can be achieved with newly added `--factor` flag but keeping a separate flag that explicitly disables the timeout can be easier for new testers to find it out and separates its purpose from the `--factor` flag.
Requesting review ryanofsky jnewbery as per the IRC discussion.
Update: After initial round of review, the approach is modified to accommodate the functionality in already existing `--factor` flag. `--factor` is changed to `--timeout-factor` to express its intent better.
ACKs for top commit:
MarcoFalke:
ACK 38c3dd9c706e7e84b2a4dbaf1424a3f1c3b694fc and thanks for fixing up all my typos 😅
jnewbery:
ACK 38c3dd9c706e7e84b2a4dbaf1424a3f1c3b694fc.
Tree-SHA512: 9458dd1010288c62f8bb83f7a4893284fbbf938882dd65fc9e08810a910db07ef676e3100266028e5d4c8ce407b2267b3860595015da070c84a9d4a9816797db
2742c3428633b6ceaab6714635dc3adb74bf121b test: add factor option to adjust test timeouts (Harris)
Pull request description:
This PR adds a new option **factor** that can be used to adjust timeouts in various functional tests.
Several timeouts and functions from `authproxy`, `mininode`, `test_node` and `util` have been adapted to use this option. The factor-option definition is located in `test_framework.py`.
Fixes https://github.com/bitcoin/bitcoin/issues/18266
Also Fixes https://github.com/bitcoin/bitcoin/issues/18834
ACKs for top commit:
MarcoFalke:
Thanks! ACK 2742c3428633b6ceaab6714635dc3adb74bf121b
Tree-SHA512: 6d8421933ba2ac1b7db00b70cf2bc242d9842c48121c11aadc30b0985c4a174c86a127d6402d0cd73b993559d60d4f747872d21f9510cf4806e008349780d3ef
1abcecc40c518a98b7d17880657ec0247abdf125 Tests: Use self.chain instead of 'regtest' in almost all current tests (Jorge Timón)
Pull request description:
Simply avoiding the hardcoded string in more places for consistency.
It can also allow for more easily reusing tests for other chains other than regtest.
Separated from #8994 .
Continues #16509 .
It is still not complete (ie to be complete, we need the -chain parameter in #16680 and make whether acceptnonstdtxs is allowed for that chain or not customizable for regtest [or for custom chains like in #8994 ] ). But while being incomplete like #16509 , it's quite simple to review and another step forward IMO.
ACKs for top commit:
Sjors:
re-ACK 1abcecc. I think it's an improvement even if incomplete and if some PR's might accidentally bring "regtest" back. Subsequent improvements hopefully don't have to touch 16 files.
elichai:
Code review ACK 1abcecc40c518a98b7d17880657ec0247abdf125
ryanofsky:
Code review ACK 1abcecc40c518a98b7d17880657ec0247abdf125.
ryanofsky:
Code review ACK 1abcecc40c518a98b7d17880657ec0247abdf125
Tree-SHA512: 5620de6dab235ca8bd8670d6366c7b9f04f0e3ca9c5e7f87765b38e16ed80c17d7d1630c0d5fd7c5526f070830d94dc74cc2096d8ede87dc7180ed20569509ee
19139ee034d20ebab1b91d3ac13a8eee70b59374 Add documentation for test_shell submodule (JamesC)
f5112369cf91451d2d0bf574a9bfdaea04696939 Add TestShell class (James Chiang)
5155602a636c323424f75272ccec38588b3d71cd Move argparse() to init() (JamesC)
2ab01462f48b2d4e0d03ba842c3af8851c67c6f1 Move assert num_nodes is set into main() (JamesC)
614c645643e86c4255b98c663c10f2c227158d4b Clear TestNode objects after shutdown (JamesC)
6f40820757d25ff1ccfdfcbdf2b45b8b65308010 Add closing and flushing of logging handlers (JamesC)
6b71241291a184c9ee197bf5f0c7e1414417a0a0 Refactor TestFramework main() into setup/shutdown (JamesC)
ede8b7608e115364b5bb12e7f39d662145733de6 Remove network_event_loop instance in close() (JamesC)
Pull request description:
This PR refactors BitcoinTestFramework to encapsulate setup and shutdown logic into dedicated methods, and adds a ~~TestWrapper~~ TestShell child class. This wrapper allows the underlying BitcoinTestFramework to run _between user inputs_ in a REPL environment, such as a Jupyter notebook or any interactive Python3 interpreter.
The ~~TestWrapper~~ TestShell is motivated by the opportunity to expose the test-framework as a prototyping and educational toolkit. Examples of code prototypes enabled by ~~TestWrapper~~ TestShell can be found in the Optech [Taproot/Schnorr](https://github.com/bitcoinops/taproot-workshop) workshop repository.
Usage example:
```
>>> import sys
>>> sys.path.insert(0, "/path/to/bitcoin/test/functional")
```
```
>>> from test_framework.test_wrapper import TestShell
>>> test = TestShell()
>>> test.setup(num_nodes=2)
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Initializing test directory /path/to/bitcoin_func_test_XXXXXXX
```
```
>>> test.nodes[0].generate(101)
>>> test.nodes[0].getblockchaininfo()["blocks"]
101
```
```
>>> test.shutdown()
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Stopping nodes
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Cleaning up /path/to/bitcoin_func_test_XXXXXXX on exit
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Tests successful
```
**Overview of changes to BitcoinTestFramework:**
- Code moved to `setup()/shutdown()` methods.
- Argument parsing logic encapsulated by `parse_args` method.
- Success state moved to `BitcoinTestFramework.success`.
_During Shutdown_
- `BitcoinTestFramework` logging handlers are flushed and removed.
- `BitcoinTestFrameowork.nodes` list is cleared.
- `NetworkThread.network_event_loop` is reset. (NetworkThread class).
**Behavioural changes:**
- Test parameters can now also be set when overriding BitcoinTestFramework.setup() in addition to overriding `set_test_params` method.
- Potential exceptions raised in BitcoinTestFramework.setup() will be handled in main().
**Added files:**
- ~~test_wrapper.py~~ `test_shell.py`
- ~~test-wrapper.md~~ `test-shell.md`
ACKs for top commit:
jamesob:
ACK 19139ee034
jonatack:
ACK 19139ee034d20ebab1b91d3ac13a8eee70b59374
jnewbery:
Rather than invalidate the three ACKs for a minor nit, can you force push back to 19139ee034d20ebab1b91d3ac13a8eee70b59374 please? I think this PR was ready to merge before your last force push.
jachiang:
> Rather than invalidate the three ACKs for a minor nit, can you force push back to [19139ee](19139ee034) please? I think this PR was ready to merge before your last force push.
jnewbery:
ACK 19139ee034d20ebab1b91d3ac13a8eee70b59374
Tree-SHA512: 0c24f405f295a8580a9c8f1b9e0182b5d753eb08cc331424616dd50a062fb773d3719db4d08943365b1f42ccb965cc363b4bcc5beae27ac90b3460b349ed46b2
fa47330397 test: Speed up cache creation (MarcoFalke)
fa6ad7a5ec test: Bump MAX_NODES to 12 (MarcoFalke)
Pull request description:
When testing a combination of settings that affect the datadir (e.g. prune, blockfilter, ...) we may need a lot of datadirs.
Bump the maximum number of nodes proactively from 8 to 12, so that caches get populated with 12 node dirs, as opposed to 8.
Also, add an assert that the list of deterministic keys is exactly the number of max nodes (and not more than that.
Also, create the cache faster.
ACKs for commit fa4733:
laanwj:
utACK fa473303972b7dad600d949dc9b303d8136cb7e7
Tree-SHA512: 9803c765ed52d344102f5a3bce57b05d88a7429dcb05ed66ed6c881fda8d87c2834d02d21b95fe9f39c0efe3b8527e13cf94f006588cde22e8c2cd50b2d517a6
fa2cdc9ac2 test: Simplify create_cache (MarcoFalke)
fa25210d62 qa: Fix wallet_txn_doublespend issue (MarcoFalke)
1111aecbb5 qa: Always refresh stale cache to be out of ibd (MarcoFalke)
fab0d85802 qa: Remove mocktime unless required (MarcoFalke)
Pull request description:
When starting a test, we are always in IBD because the timestamps on cached blocks are in the past. Usually, we solve that by generating a block at the beginning of the test.
That is clumsy and might even lead to other problems such as #15360 and https://github.com/bitcoin/bitcoin/issues/14446#issuecomment-461926598
So fix that by getting rid of mocktime and always refreshing the last block of the cache when starting the test framework.
Should fix#14446
Tree-SHA512: 6af09800f9c86131349a103af617a54551f5f3f3260d38e14e3f30fdd3d91a0feb0100c56cbb12eae4aeac5571ae4b530b16345cbb831d2670237b53351a22c1
fa48405ef84985e5a9d38ec38e90d16596ea45b5 Warn on unknown rw_settings (MarcoFalke)
Pull request description:
Log a warning to debug log if unknown settings are encountered. This should probably only ever happen when the software is upgraded.
Something similar is already done for the command line and config file. See:
* test: Add test for unknown args #16234 (commit fa7dd88b71a1c6641bd450fae29a4a31849b1afd)
ACKs for top commit:
ryanofsky:
Code review ACK fa48405ef84985e5a9d38ec38e90d16596ea45b5. Looks good and I could see this being helpful for debugging. Thanks for taking suggestions
Tree-SHA512: cec7d88adf84fa0a842f56b26245157736eb50df433db951e622ea07fd145b899822b24cdab1d8b36c066415ce4f0ef09b493fa8a8d691532822a59c573aafa7
* fix(rpc): `masternode outputs` should produce an array
or it would ignore all but 1 outputs produced by the same tx
* rpc: improve `masternode outputs` help
* tests: check that `masternode outputs` show all outputs produced in the same tx
eca56f89293b74f11ca631ff2a0793e970e65841 test: replace 'regtest' leftovers by self.chain (Sebastian Falbesoner)
Pull request description:
This is a follow-up PR to #16681 (fixes#18068), replacing all remaining hardcoded `"regtest"` strings in functional tests by `self.chain`.
Top commit has no ACKs.
Tree-SHA512: 96524649b33164938e5a95215991103ed7855ebab55ef788d4816b3fa5cbc03d8f3b0d39f2247a87522f289fd7f4daf25e059900b8462b5127eb154bbee89054
* coinjoin: make CCoinJoinServer managed pointer, assign CConnman during init
* coinjoin: make CCoinJoinClientQueueManager managed pointer, assign CConnman during init
* sporks: move spork validation logic downwards after CConnman initialization
* sporks: make CSporkManager a pointer, reduce global invocations
* governance: make CGovernanceManager a pointer, reduce global invocations
* llmq: migrate LLMQ subsystem raw pointers to managed pointers
* masternode: make activeMasternodeManager a managed pointer
* masternode: make masternodeSync a managed pointer, assign CConnman during init
* refactor: make instantsend helper functions class members
* fix: send empty CDeterministicMNList if pointer isn't initialized yet
* fix: refactor governance object retrieval logic across node and ui
Update src/interfaces/node.cpp
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
1c23ea5fe67b88fd72a1ff640dd1bbb21a34fbf4 test: fix bitcoind already running warnings on macOS (fanquake)
Pull request description:
On macOS, `pidof` installed via brew returns b'' rather than None.
Account for this, to remove spurious warnings from the test_runner.
ACKs for top commit:
laanwj:
ACK 1c23ea5fe67b88fd72a1ff640dd1bbb21a34fbf4
Tree-SHA512: 640f4323d4105eac5c7abb52daf80486d5d3b4a074720490ceeb97c3dd8d73a3de9a988d2550f1e2076c620bb10d452b2959d8b723d2ee64f499878909824e31
fac942ca57dce6cfa5655a3ac8664d6a051bc01f test: Remove fragile assert_memory_usage_stable (MarcoFalke)
Pull request description:
This test fails on arm64 and a fuzz tests seems inappropriate for the functional test suite anyway, so remove it.
Example failures:
* https://travis-ci.org/bitcoin/bitcoin/jobs/611497963#L14517
* https://travis-ci.org/MarcoFalke/bitcoin-core/jobs/611029104#L3876
ACKs for top commit:
jamesob:
ACK fac942ca57
Tree-SHA512: 3577e7ce5891d221cb798454589ba796ed0c06621a26351bb919c23bc6bb46aafcd0b11cb02bbfde64b74d67cb2950da44959a7ecdc436491a34e8b045c1ccf4
49997813a4db388b2810e5e27ef771e8aa6a1f03 test: check custom ancestor limit in mempool_packages.py (Sebastian Falbesoner)
Pull request description:
The functional test `mempool_packages.py` starts one node with default ancestor/descendant limit settings and one with a custom, reduced ancestor limit (currently `-limitancestorcount=5`). The effect of the latter had not been tested yet though. This is approached in this PR by checking on the expected mempool contents of node1 after the node0 ancestor tests are done, via the following three conditions:
- the # of txs in the node1 mempool is equal to the the limit
- all txs in node1 mempool are a subset of txs in node0 mempool
- the node1 mempool txs match the start of the constructed tx-chain
Note that this still doesn't *fully* check the expected mempool of node1 (e.g. that it isn't influenced by `prioritisetransaction` RPC on node0), hence I add another TODO. In the future it would make sense to also set a custom descendant limit when the second TODO about checking node1's mempool is approached: 89e93135ae/test/functional/mempool_packages.py (L228)
ACKs for top commit:
MarcoFalke:
ACK 49997813a4db388b2810e5e27ef771e8aa6a1f03 👲
Tree-SHA512: d3a1d19fb49731238ad08ee7c02e2fa81a227e3b4ef3340d68598de42ddb62be9161134f6b8e08fa76b8c9faa02fecfa01111159642e20e9f358292a757b7608
af7bae734089f6af0029b0887932ccd9a469e12e [tests] Don't stop-start unnecessarily in rpc_fundrawtransaction.py (John Newbery)
9a8505299ba392acbab4647963113b0c29495f1d [tests] Use -whitelist in rpc_fundrawtransaction.py (John Newbery)
646b593bbd0db113c6e45ab92177b8f5251e8710 [tests] Speed up rpc_fundrawtransaction.py (John Newbery)
Pull request description:
Speed up rpc_fundrawtransaction.py
Most of the time in rpc_fundrawtransaction.py is spent waiting for
unconfirmed transactions to propagate. Net processing adds a poisson
random delay to the time it will INV transactions with a mean interval
of 5 seconds. Calls like the following:
```
self.nodes[2].sendrawtransaction(signedTx['hex'])
self.sync_all()
self.nodes[1].generate(1)
````
will therefore introduce a delay waiting for the mempools to sync.
Instead just generate the block on the node that sent the transaction:
```
self.nodes[2].sendrawtransaction(signedTx['hex'])
self.nodes[2].generate(1)
```
rpc_fundrawtransaction.py is not intended to be a test for transaction
relay, so it's ok to do this.
ACKs for top commit:
MarcoFalke:
ACK af7bae734089f6af0029b0887932ccd9a469e12e 🛴
Tree-SHA512: db3407d871bfdc99a02e7304b07239dd3585ac47f27f020f1a70608b7f6386b134343c01f3e4d1c246ce734676755897671999695068d6388602fb042d178780
fa5facd3e72b6d61374b0b93b722b55e2b090020 rpc: Remove unused boost::this_thread::interruption_point (MarcoFalke)
Pull request description:
There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points
However, the rpc threads are `std::thread`, which does not have an `std:🧵:interrupt` member function to request interruption: https://dev.visucore.com/bitcoin/doxygen/httpserver_8cpp.html#ae1a63374e18b9abd348eb74e4243ea34
Thus, the interruption points can be removed.
ACKs for top commit:
laanwj:
ACK fa5facd3e72b6d61374b0b93b722b55e2b090020, this does nothing.
practicalswift:
ACK fa5facd3e72b6d61374b0b93b722b55e2b090020
jamesob:
ACK fa5facd3e7
Tree-SHA512: 4e29a44df1f2702cbd1ffdffa559440a8bb800baab64b4116e2c3d27cd64d8d1e8aafe1dc21b1a4e3988470d03be19cae294bd5669f7abf6d487685dc8fd8d7e
436ad436434b94982bcb7dc1d13a21949263ef73 Fix issue with conflicted mempool tx in listsinceblock (Adam Jonas)
Pull request description:
Closes#8752 by bringing back abandoned #10470.
This now checks that returned transactions are not conflicting with any transactions that are filtered out by the given blockhash and add a functional test to prevent this in the future.
For more context, #8757 was closed in favor of #10470.
ACKs for top commit:
instagibbs:
utACK 436ad43643
kallewoof:
utACK 436ad436434b94982bcb7dc1d13a21949263ef73
jonatack:
I'm not qualifed to give an ACK here but 436ad436434b94982bcb7dc1d13a21949263ef73 appears reasonable. Built/ran tests/verified that this test fails without the change in rpcwallet.cpp:
Tree-SHA512: 63d75cd3d3f19fc84dc38899b200c96179b82b24db263cd0116ee5b715265be647157855c2e35912d2fbc49c7b37db9375d6aab0ac672f0f09bece8431de5ea9
72eaab073bc747425fe551777154b13a6c4c37c9 tests: functional watch-only wallet tests (William Casarin)
72ffbdc5799c1707ecad674d701b43fb80b031d0 doc: add release note for include_watchonly default changes (William Casarin)
003a3c73c0450aa18ac2ab2ca47def2b8c53a7df rpcwallet: document include_watchonly default for watchonly wallets (William Casarin)
a50d9e6c0b8e8144d3deec58ec2e3449ba081151 rpcwallet: default include_watchonly to true for watchonly wallets (William Casarin)
Pull request description:
Right now it's a bit annoying to deal with watchonly wallets, many rpc commands have an `include_watchonly` argument that needs to be explicitly set.
Wallets created with `createwallet` can have a `disable_private_keys` parameter, for those wallets we already know that they are watchonly, so there's no reason to have to explicitly ask for it for every command. Instead we check this wallet flag when the `include_watchonly` parameter isn't set.
ACKs for top commit:
achow101:
Code review ACK 72eaab073bc747425fe551777154b13a6c4c37c9
Sjors:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9
promag:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9, code review only, didn't look closely to the test.
kallewoof:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9
fanquake:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9 - I've looked over the changes, they make sense to me. Compiled and ran the tests etc.
Tree-SHA512: d3646b55e97f386594d7efc994f0712f3888475c6a5dc7f131ac9f8c49bf5d4677182b88f42b34152abe1ad101ecadd152b4c20e9d3c1267190db36f77ab8bd7
* Move wallet enums to walletutil.h
* MOVEONLY: Move key handling code out of wallet to keyman file
Start moving wallet and ismine code to scriptpubkeyman.h, scriptpubkeyman.cpp
The easiest way to review this commit is to run:
git log -p -n1 --color-moved=dimmed_zebra
And check that everything is a move (other than includes and copyrights comments).
This commit is move-only and doesn't change code or affect behavior.
* Refactor: Split up CWallet and LegacyScriptPubKeyMan and classes
This moves CWallet members and methods dealing with keys to a new
LegacyScriptPubKeyMan class, and updates calling code to reference the new
class instead of CWallet.
Most of the changes are simple text replacements and variable substitutions
easily verified with:
git log -p -n1 -U0 --word-diff-regex=.
The only nontrivial chunk of code added is the new LegacyScriptPubKeyMan class
declaration, but this code isn't new and is just selectively copied and moved
from the previous CWallet class declaration. This can be verified with:
git log -p -n1 --color-moved=dimmed_zebra src/wallet/scriptpubkeyman.h src/wallet/wallet.h
or
git diff HEAD~1:src/wallet/wallet.h HEAD:src/wallet/scriptpubkeyman.h
This commit does not change behavior.
* Renamed classes in scriptpubkeyman
* Fixes for conflicts, compilation and linkage errors due to previous commits
* Reordered methods in scriptpubkeyman to make further backports easier
* Reordered methods in scriptpubkeyman to make further backports easier (part II)
* Remove HDChain copy from SigningProvider class
* fixes/suggestions
Co-authored-by: Andrew Chow <achow101-github@achow101.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* refactor(llmq): substitute memberless class llmq::CLLMQUtils with namespace llmq::utils
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* chore: mark functions internal to `llmq::utils` as `static`
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* fix(llmq): Ensure connections between quorums
Every masternode will now "watch" a single node from _every other_ quorum in addition to intra-quorum connections. This should make propagation of recsigs produced by one quorum to other quorums much more reliable.
* fix: Do this only for masternodes which participate in IS quorums
* refactor: rename `CQuorumManager::EnsureQuorumConnections` to better match the actual behaviour
(and avoid confusion with `CLLMQUtils::EnsureQuorumConnections`)
* refactor: move IS quorums watch logic into `CQuorumManager::CheckQuorumConnections`
avoid calling slow `ScanQuorums` (no caching atm) inside the loop
* tests: check that inter-quorum connections are added
* use `ranges::any_of`
* fix(llmq): mark mns "bad" based on the failed connect attempts count
Avoid using "last success time" as a proxy
* fix(tests): tweak feature_llmq_simplepose.py
* Add HaveKey and HaveCScript to SigningProvider
* Remove CKeyStore and squash into CBasicKeyStore
* Move HaveKey static function from keystore to rpcwallet where it is used
* scripted-diff: rename CBasicKeyStore to FillableSigningProvider
-BEGIN VERIFY SCRIPT-
git grep -l "CBasicKeyStore" | xargs sed -i -e 's/CBasicKeyStore/FillableSigningProvider/g'
-END VERIFY SCRIPT-
* Move KeyOriginInfo to its own header file
* Move various SigningProviders to signingprovider.{cpp,h}
Moves all of the various SigningProviders out of sign.{cpp,h} and
keystore.{cpp,h}. As such, keystore.{cpp,h} is also removed.
Includes and the Makefile are updated to reflect this. Includes were largely
changed using:
git grep -l "keystore.h" | xargs sed -i -e 's;keystore.h;script/signingprovider.h;g'
* Remove CCryptoKeyStore and move all of it's functionality into CWallet
Instead of having a separate CCryptoKeyStore that handles the encryption
stuff, just roll it all into CWallet.
* Fixed cases of mess CWallet functions with CCryptoKeyStore and conflicts
* Move WatchOnly stuff from SigningProvider to CWallet
* Fixes for lint cirtular dependencies to calm linter
Co-authored-by: Andrew Chow <achow101-github@achow101.com>
