d9c8aacce3 depends, refactor: Avoid hardcoding `host_prefix` in toolchain file (Hennadii Stepanov)
Pull request description:
This PR allows the entire `depends/<host_prefix>` directory to be relocatable.
Only `libevent` package configuration files are non-relocatable for the version `2.1.12-stable` we use now. However, this issue has been fixed upstream in 1f1593ff27 and friends.
Fixes https://github.com/bitcoin/bitcoin/issues/31050.
ACKs for top commit:
theuni:
Neat. utACK d9c8aacce3
ryanofsky:
Code review ACK d9c8aacce3
Tree-SHA512: c4c340722e63fc1da36fba2b15f025a6e1d477da1991194d678f546f2f3ea9454e2f0ff054aae6ae6c332a0781a597c3ce63b9018b46b8c258033f0d19efbef3
faf8fc5487 lint: Call lint_commit_msg from test_runner (MarcoFalke)
fa99728b0c lint: Move commit range printing to test_runner (MarcoFalke)
fa673cf344 lint: Call lint_scripted_diff from test_runner (MarcoFalke)
Pull request description:
The lint `commit-script-check.sh` can not be called from the test_runner at all and must be called manually. Also, some checks require `COMMIT_RANGE` to be set.
Fix all issues by moving two lint checks into the test_runner. Also, the proper commit range is passed to the checks by the test_runner, so that the user no longer has to do it.
ACKs for top commit:
kevkevinpal:
reACK [faf8fc5](faf8fc5487)
willcl-ark:
tACK faf8fc5487
Tree-SHA512: 78018adc618d997508c226c9eee0a4fada3899cdfd91587132ab1c0389aea69127bafc3a900e90e30aca2c6bae9dcd6e6188ef287e91413bc63ee66fb078b1af
3e97ff9c5e gui, psbt: Use SIGHASH_DEFAULT when signing PSBTs (Ava Chow)
Pull request description:
SIGHASH_DEFAULT should be used to indicate SIGHASH_DEFAULT for taproot inputs, and SIGHASH_ALL for all other input types. This avoids adding an unnecessary byte to the end of all Taproot signatures added to PSBTs signed in the GUI.
See also bitcoin/bitcoin#22514
ACKs for top commit:
Sjors:
utACK 3e97ff9c5e
pablomartin4btc:
utACK 3e97ff9c5e
hebasto:
ACK 3e97ff9c5e, I have reviewed the code and it looks OK.
Tree-SHA512: f96f26b3a6959865cf23039afb5ffb7e454fb52ee39c510583851caf00a8a383cde69bc7e90db536addbdd498a02f4b001cbaf509d6d53c5f8601b3933786f6c
9d2d9f7ce2 rpc: Include assumeutxo as a failure reason of rescanblockchain (Fabian Jahr)
595edee169 test, assumeutxo: import descriptors during background sync (Alfonso Roman Zubeldia)
d73ae603d4 rpc: Improve importdescriptor RPC error messages (Fabian Jahr)
27f99b6d63 validation: Don't assume m_chain_tx_count in GuessVerificationProgress (Fabian Jahr)
42d5d53363 interfaces: Add helper function for wallet on pruning (Fabian Jahr)
Pull request description:
A test that is added as part of #30455 uncovered this issue: The `GuessVerificationProgress` function is used during during descriptor import and relies on `m_chain_tx_count`. In #29370 an [`Assume` was added](0fd915ee6b) expecting the `m_chaint_tx_count` to be set. However, as the test uncovered, `GuessVerificationProgress` is called with background sync blocks that have `m_chaint_tx_count = 0` when they have not been downloaded and processed yet.
The simple fix is to remove the `Assume`. Users should not be thrown off by the `Internal bug detected` error. The behavior of `importdescriptor` is kept consistent with the behavior for blocks missing due to pruning.
The test by alfonsoromanz is cherry-picked here to show that the [CI errors](https://cirrus-ci.com/task/5110045812195328?logs=ci#L2535) should be fixed by this change.
This PR also improves error messages returned by the `importdescriptors` and `rescanblockchain` RPCs. The error message now changes depending on the situation of the node, i.e. if pruning is happening or an assumutxo backgroundsync is active.
ACKs for top commit:
achow101:
ACK 9d2d9f7ce2
mzumsande:
Code Review ACK 9d2d9f7ce2
furszy:
Code review ACK 9d2d9f7ce2
Tree-SHA512: b841a9b371e5eb8eb3bfebca35645ff2fdded7a3e5e06308d46a33a51ca42cc4c258028c9958fbbb6cda9bb990e07ab8d8504dd9ec6705ef78afe0435912b365
0cdddeb224 kernel: Move block tree db open to BlockManager constructor (TheCharlatan)
7fbb1bc44b kernel: Move block tree db open to block manager (TheCharlatan)
57ba59c0cd refactor: Remove redundant reindex check (TheCharlatan)
Pull request description:
Before this change the block tree db was needlessly re-opened during startup when loading a completed snapshot. Improve this by letting the block manager open it on construction. This also simplifies the test code a bit.
The change was initially motivated to make it easier for users of the kernel library to instantiate a BlockManager that may be used to read data from disk without loading the block index into a cache.
ACKs for top commit:
maflcko:
re-ACK 0cdddeb224🏪
achow101:
ACK 0cdddeb224
mzumsande:
re-ACK 0cdddeb224
Tree-SHA512: fe3d557a725367e549e6a0659f64259cfef6aaa565ec867d9a177be0143ff18a2c4a20dd57e35e15f97cf870df476d88c05b03b6a7d9e8d51c568d9eda8947ef
93747d934b test: Added coverage to the waitfornewblock rpc (kevkevinpal)
Pull request description:
Added a test for the Negative timeout error if the rpc is given a negative value for its timeout arg
This adds coverage to the `waitfornewblock` rpc
you can check to see there is no coverage for this error by doing
`grep -nri "Negative timeout" ./test/`
and nothing shows up, you can also see by manually checking where we call `waitfornewblock` in the functional tests
ACKs for top commit:
Sjors:
tACK 93747d934b
achow101:
ACK 93747d934b
brunoerg:
code review ACK 93747d934b
tdb3:
ACK 93747d934b
Tree-SHA512: 45cf34312412d3691a39f003bcd54791ea16542aa3f5a2674d7499c9cc4039550b2cbd32cc3d4c5fe100d65cb05690594b10a0c42dfab63bcca3dac121bb195b
e1676b08f7 doc: release notes (Sjors Provoost)
0082f6acc1 rpc: have mintime account for timewarp rule (Sjors Provoost)
79d45b10f1 rpc: clarify BIP94 behavior for curtime (Sjors Provoost)
0713548137 refactor: add GetMinimumTime() helper (Sjors Provoost)
Pull request description:
#30681 fixed the `curtime` field of `getblocktemplate` to take the timewarp rule into account. However I forgot to do the same for the `mintime` field, which was hardcoded to use `pindexPrev->GetMedianTimePast()+1`.
This PR adds a helper `GetMinimumTime()` and uses it for the `mintime` field.
#31376 changed the `curtime` field to always account for the timewarp rule. This PR maintains that behavior.
Note that `mintime` now always applies BIP94, including on mainnet. This makes future softfork activation safer.
It could be backported to v28.
ACKs for top commit:
fjahr:
tACK e1676b08f7
achow101:
ACK e1676b08f7
darosior:
utACK e1676b08f7 on the code changes
tdb3:
brief code review re ACK e1676b08f7
TheCharlatan:
ACK e1676b08f7
Tree-SHA512: 0e322d8cc3b8ff770849bce211edcb5b6f55d04e5e0dee0657805049663d758f27423b047ee6363bd8f6c6fead13f974760f48b3321ea86f514f446e1b23231c
8888ee4403 ci: Allow build dir on CI host (MarcoFalke)
Pull request description:
This is required to pass cross builds on to a different machine after the build.
See for example https://github.com/bitcoin/bitcoin/pull/31176, but this pull will also allow someone to implement it outside this repo.
ACKs for top commit:
davidgumberg:
lgtm ACK 8888ee4403
hebasto:
re-ACK 8888ee4403.
Tree-SHA512: a1e2c32bc1b95efbd0b48287ac5b49e0e1bacbf5a5800845be5352bbdd3e17fa478e90348b2e94e95cf3ae863cdf75ab444089376588f6f8eec438f73a4b5b97
152a2dcdef test: fix intermittent timeout in p2p_1p1c_network.py (Martin Zumsande)
Pull request description:
The timeout is due to outstanding txrequests with python peers, which have the same timeout (`60s`) as the mempool sync timeout.
I explained this in more detail in https://github.com/bitcoin/bitcoin/issues/31721#issuecomment-2620169640 and also mentioned there how to reproduce it.
Fix this by disconnecting the python peers after they send their txns, they aren't needed after this point anyway because the main goal of the test is the sync between the 4 full nodes.
Fixes #31721
ACKs for top commit:
achow101:
ACK 152a2dcdef
instagibbs:
reACK 152a2dcdef
marcofleon:
ACK 152a2dcdef
glozow:
reACK 152a2dcdef
Tree-SHA512: 908c58933d8e9fcca91425fce1b7c9c7cb7121a6d26840630e03a442356ad2a327d1e087df72a19caa97024ea827593e10f2ff93838f88939458e73df9857df0
Deduplicate the logic of adding the parents as announcements to
txrequest. The function can return a bool (indicating whether we're
attempting orphan resolution) instead of the delay.
e87429a2d0 ci: optionally use local docker build cache (0xb10c)
Pull request description:
By setting `DANGER_DOCKER_BUILD_CACHE_HOST_DIR`, the task-specific docker images built during the CI run can be cached. This allows, for example, ephemeral CI runners to reuse the docker images (or layers of it) from earlier runs, by persisting the image cache before the ephemeral CI runner is shut down. The cache keyed by `CONTAINER_NAME`.
As `--cache-to` doesn't remove old cache files, the existing cache is removed after a successful `docker build` and the newly cached image is moved to it's location to avoid the cache from growing indefinitely with old, unused layers.
When `--cache-from` doesn't find the directory, the cached version is a cache-miss, or the cache can't be imported for whatever other reason, it warns and `docker build` continues by building the docker image.
This feature is opt-in. The documentation for the docker build cache of `type=local` can be found on https://docs.docker.com/build/cache/backends/local/
This replaces https://github.com/bitcoin/bitcoin/pull/31377 - some of the discussion there might provide more context.
ACKs for top commit:
maflcko:
I haven't tested this, and it looks harmless and is easy to revert, if needed. So lgtm ACK e87429a2d0
achow101:
ACK e87429a2d0
TheCharlatan:
tACK e87429a2d0
willcl-ark:
ACK e87429a2d0
Tree-SHA512: 0887c395dee2e2020394933246d4c1bfb6dde7165219cbe93eccfe01379e05c75dce8920b6edd7df07364c703fcee7be4fba8fa45fd0e0e89da9e24759f67a71
cddcbaf81e RPC: improve SFFO arg parsing, error catching and coverage (furszy)
4f4cd35319 rpc: decouple sendtoaddress 'subtractfeefromamount' boolean parsing (furszy)
Pull request description:
Following changes were made:
1) Catch and signal error for duplicate string destinations.
2) Catch and signal error for invalid value type.
3) Catch and signal error for string destination not found in tx outputs.
4) Improved `InterpretSubtractFeeFromOutputInstructions()` code organization.
5) Added test coverage for all possible error failures.
Also, fixed two PEP 8 warnings at the 'wallet_sendmany.py' file:
- PEP 8: E302 expected 2 blank lines, found 1 at the SendmanyTest class declaration.
- PEP 8: E303 too many blank lines (2) at skip_test_if_missing_module() and set_test_params()
ACKs for top commit:
achow101:
ACK cddcbaf81e
murchandamus:
crACK cddcbaf81e
naiyoma:
TACK [cddcbaf81e)
ismaelsadeeq:
Code review and Tested ACK cddcbaf81e
Tree-SHA512: c9c15582b81101a93987458d155394ff2c9ca42864624c034ee808a31c3a7d7f55105dea98e86fce17d3c7b2c1a6b5b77942da66b287f8b8881a60cde78c1a3c
d45eb3964f test: compare BDB dumps of test framework parser and wallet tool (Sebastian Falbesoner)
01ddd9f646 test: complete BDB parser (handle internal/overflow pages, support all page sizes) (Sebastian Falbesoner)
Pull request description:
This PR adds missing features to our test framework's BDB parser with the goal of hopefully being able to read all legacy wallets that are created with current and past versions of Bitcoin Core. This could be useful both for making review of https://github.com/bitcoin/bitcoin/pull/26606 easier and to also possibly improve our functional tests for the wallet BDB-ro parser by additionally validating it with an alternative implementation. The second commits introduces a test that create a legacy wallet with huge label strings (in order to create overflow pages, i.e. pages needed for key/value data than is larger than the page size) and compares the dump outputs of wallet tool and the extended test framework BDB parser.
It can be exercised via `$ ./test/functional/tool_wallet.py --legacy`. BDB support has to be compiled in (obviously).
For some manual tests regarding different page sizes, the following patch can be used:
```diff
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 38cca32f80..1bf39323d3 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -395,6 +395,7 @@ void BerkeleyDatabase::Open()
DB_BTREE, // Database type
nFlags, // Flags
0);
+ pdb_temp->set_pagesize(1<<9); /* valid BDB pagesizes are from 1<<9 (=512) to <<16 (=65536) */
if (ret != 0) {
throw std::runtime_error(strprintf("BerkeleyDatabase: Error %d, can't open database %s", ret, strFile));
```
I verified that the newly introduced test passes with all valid page sizes between 512 and 65536.
ACKs for top commit:
achow101:
ACK d45eb3964f
furszy:
utACK d45eb3964f
brunoerg:
code review ACK d45eb3964f
Tree-SHA512: 9f8ac80452545f4fcd24a17ea6f9cf91b487cfb1fcb99a0ba9153fa4e3b239daa126454e26109fdcb72eb1c76a4ee3b46fd6af21dc318ab67bd12b3ebd26cfdd
The timeout is due to outstanding txrequests with
python peers. Fix this by disconnecting these peers
after they send their txns, they aren't needed after
this point anyway.
Previously in getblocktemplate only curtime took the timewarp rule into account.
Mining pool software could use either, though in general it should use curtime.
Before bip94 there was an assumption that the minimum permitted
timestamp is GetMedianTimePast() + 1.
This commit splits a helper function out of UpdateTime() to
obtain the minimum time in a way that takes the
timewarp attack rule into account.
fa8ade300f refactor: Avoid GCC false positive error (MarcoFalke)
fa40807fa8 ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions (MarcoFalke)
Pull request description:
It is possible that someone accidentally removes the workaround in fa9e0489f5, or more likely that someone accidentally adds new code without the workaround.
Avoid this by adding a temporary CI check.
This can be tested by reverting the workaround and observing a failure.
ACKs for top commit:
hebasto:
ACK fa8ade300f, I've tested locally on Ubuntu 24.04.
Tree-SHA512: 7ee1538fd5304a5ab91ac8c7619a573548d7e0345592a1e9d38b3b73729e09e7c77a9ee703d64cf02a8218de3148376d7836e294abb939aa7533034ba36dfb6c
faf2f2c654 test: Avoid redundant stop and error spam on shutdown (MarcoFalke)
fae3bf6b87 test: Avoid redundant stop and error spam on startup failure (MarcoFalke)
fa0dc09b90 test: Remove --noshutdown flag (MarcoFalke)
fad441fba0 test: Treat leftover process as error (MarcoFalke)
Pull request description:
The `--noshutdown` flag is brittle, confusing, and redundant:
* Someone wanting to inspect the state after a test failure will likely also want to debug the state on the python side, so the option is redundant with `--pdbonfailure`. If there was a use case to replicate `--pdbonfailure` without starting pdb, a dedicated flag could be added for that use case.
* It is brittle to use the flag for a passing test, because it will disable checks in the test. For example, on shutdown LSan will perform a leak check, and the test framework will check that the node did not crash, and it will check that the node did not print errors to stderr.
Fix all issues by removing it.
Also, tidy up startup error messages to be less confusing as a result.
ACKs for top commit:
hodlinator:
re-ACK faf2f2c654
pablomartin4btc:
re tACK faf2f2c654
Tree-SHA512: 46d7ae59c7be88b93f1f9e0b6be21af0fc101e646512e2c5e725682cb18bfec8aa010e0ebe89ce9ffe239e5caac0da5f81cc97b79e738d26ca5fa31930e8e4e3
f5883286e3 Add a fuzz test for Num3072 multiplication and inversion (Pieter Wuille)
a26ce62894 Safegcd based modular inverse for Num3072 (Pieter Wuille)
91ce8cef2d Add benchmark for MuHash finalization (Pieter Wuille)
Pull request description:
This implements a safegcd-based modular inverse for MuHash3072. It is a fairly straightforward translation of [the libsecp256k1 implementation](https://github.com/bitcoin-core/secp256k1/pull/831), with the following changes:
* Generic for 32-bit and 64-bit
* Specialized for the specific MuHash3072 modulus (2^3072 - 1103717).
* A bit more C++ish
* Far fewer sanity checks
A benchmark is also included for MuHash3072::Finalize. The new implementation is around 100x faster on x86_64 for me (from 5.8 ms to 57 μs); for 32-bit code the factor is likely even larger.
For more information:
* [Original paper](https://gcd.cr.yp.to/papers.html) by Daniel J. Bernstein and Bo-Yin Yang
* [Implementation](https://github.com/bitcoin-core/secp256k1/pull/767) for libsecp256k1 by Peter Dettman; and the [final](https://github.com/bitcoin-core/secp256k1/pull/831) version
* [Explanation](https://github.com/bitcoin-core/secp256k1/blob/master/doc/safegcd_implementation.md) of the algorithm using Python snippets
* [Analysis](https://github.com/sipa/safegcd-bounds) of the maximum number of iterations the algorithm needs
* [Formal proof in Coq](https://medium.com/blockstream/a-formal-proof-of-safegcd-bounds-695e1735a348) by Russell O'Connor (for the 256-bit version of the algorithm; here we use a 3072-bit one).
ACKs for top commit:
achow101:
ACK f5883286e3
TheCharlatan:
Re-ACK f5883286e3
dergoegge:
tACK f5883286e3
Tree-SHA512: 275872c61d30817a82901dee93fc7153afca55c32b72a95b8768f3fd464da1b09b36f952f30e70225e766b580751cfb9b874b2feaeb73ffaa6943c8062aee19a
1b51616f2e test: improve rogue calls in mining functions (i-am-yuvi)
Pull request description:
#31403 follow-up, see [comment](https://github.com/bitcoin/bitcoin/pull/31403#pullrequestreview-2498806354)
- Rename `invalid_call` parameter to `called_by_framework` in `generateblock`, `generatetoaddress` and `generatetodescriptor` mining methods to better express its intended usage.
- Add explicit assertion message clarifying that these functions should only be called by TestFramework itself to maintain proper node synchronization.
ACKs for top commit:
maflcko:
lgtm ACK 1b51616f2e
achow101:
ACK 1b51616f2e
hodlinator:
re-ACK 1b51616f2e
Prabhat1308:
ACK [1b51616](1b51616f2e)
Tree-SHA512: 56832626fe54dcaa07dacb4f9c960c0a83fad3fb12272155114ac697856c59b7f44805e1152eddeec7a5e8f7daf487382dc01b5b9ae2e74b62b2df6bd1f81f77
92787dd52c test: raise an error when target_vsize is below tx virtual size (ismaelsadeeq)
a8780c937f test: raise an error if output value is <= 0 in `create_self_transfer` (ismaelsadeeq)
f6e88931f0 test: test that `create_self_transfer_multi` respects `target_vsize` (ismaelsadeeq)
Pull request description:
This is a simple test PR that does two things:
1. Raise an exception in `_bulk_tx_` when `target_vsize` is too low, i.e., below the tx vsize.
2. Addresses some review comments from https://github.com/bitcoin/bitcoin/pull/30162, which are:
- Raise an error if the output value is less than or equal to zero in `create_self_transfer`.
This prevents creating transactions with a value of 0 or less.
- Add a test to verify that `create_self_transfer_multi` also respects the passed `target_vsize`.
ACKs for top commit:
achow101:
ACK 92787dd52c
theStack:
ACK 92787dd52c
rkrux:
reACK 92787dd52c
glozow:
ACK 92787dd52c
Tree-SHA512: 1f2767f2cf715ed65074c5fff347eec160b142685777d833d5e872cfef364d3dc1916b52ee442e99c7b9a8d514ff62bc67a9899d8854f65a4b93ac3ae300d18e
18619b4732 wallet: remove BDB dependency from wallet migration benchmark (furszy)
Pull request description:
Part of the legacy wallet removal working path #20160.
Stops creating a bdb database in the wallet migration benchmark.
Instead, the benchmark now creates the db in memory and re-uses it for the migration process.
ACKs for top commit:
achow101:
ACK 18619b4732
brunoerg:
code review ACK 18619b4732
theStack:
Code-review ACK 18619b4732
Tree-SHA512: a107deee3d2c00b980e3606be07d038ca524b98251442956d702a7996e2ac5e2901f656482018cacbac8ef6a628ac1fb03f677d1658aeaded4036d834a95d7e0
2656a5658c tests: add a test for the new blocksdir lock (Cory Fields)
bdc0a68e67 init: lock blocksdir in addition to datadir (Cory Fields)
cabb2e5c24 refactor: introduce a more general LockDirectories for init (Cory Fields)
1db331ba76 init: allow a new xor key to be written if the blocksdir is newly created (Cory Fields)
Pull request description:
This probably should've been included in #12653 when `-blocksdir` was introduced. Credit TheCharlatan for noticing that it's missing.
This guards against 2 processes running with separate datadirs but the same blocksdir. I didn't add `walletdir` as I assume sqlite has us covered there.
It's not likely to happen currently, but may be more relevant in the future with applications using the kernel. Note that the kernel does not currently do any dir locking, but it should.
ACKs for top commit:
maflcko:
review ACK 2656a5658c 🏼
kevkevinpal:
ACK [2656a56](2656a5658c)
achow101:
ACK 2656a5658c
tdb3:
Code review and light test ACK 2656a5658c
Tree-SHA512: 3ba17dc670126adda104148e14d1322ea4f67d671c84aaa9c08c760ef778ca1936832c0dc843cd6367e09939f64c6f0a682b0fa23a5967e821b899dff1fff961
8996fef8ae test: p2p: check that INV messages not matching wtxidrelay are ignored (Sebastian Falbesoner)
e0b3336822 test: p2p: fix sending of manual INVs in tx download test (Sebastian Falbesoner)
Pull request description:
The `test_inv_block` sub-test in p2p_tx_download.py has a subtle bug: the manual msg_inv announcements from peers currently have no effect, since they don't match the wtxidrelay setting (=true by default for `P2PInterface` instances) and are hence ignored by the nodes (since 2d282e0c / PR #18044):
e7c4794955/src/net_processing.cpp (L3904-L3911)
Though the test still passes on master, it does so without the intended scenario of asking an additional peer (triggering the GETDATA_TX_INTERVAL delay). Fix this by sending the INV message with MSG_WTX instead of MSG_TX. This increases the test run time by about one minute intentionally.
It might be good to avoid issues like this in the future, happy to add test framework improvements if someone has a concrete idea.
(Got into the topic of tx/wtx announcements via the discussion https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1904121487)
ACKs for top commit:
maflcko:
ACK 8996fef8ae😸
danielabrozzoni:
ACK 8996fef8ae
mzumsande:
Code Review ACK 8996fef8ae
Tree-SHA512: 3da26f9539c89d64c3b0d0579d9af2a6a4577615eed192506e1fb4318421b235f99a6672a497dea3050fba85dad32678f37fd2cda9ecb70cbf52982db37982e8
fad83e759a doc: Fix incorrect send RPC docs (MarcoFalke)
Pull request description:
It would be good to have accurate RPC docs, so that humans and machines can read them and rely on them.
This fixes one issue.
ACKs for top commit:
fjahr:
utACK fad83e759a
rkrux:
tACK fad83e759a
luke-jr:
tACK fad83e759a
Tree-SHA512: 65d0cc18a62ef44833621464d74b743d24ffe2b853596dce2c4f423df0495142d50387c02ba1b54f5ca77d4ddb083d55116a8ac92698aa6558762d841664911e
5c3e4d8b29 doc: add a section about using MSan (Antoine Poinsot)
Pull request description:
Just a couple lines in a subsection of the sanitizers section mentioning that using the memory sanitizer is a bit more involve than other sanitizers, describing the steps and pointing to an example.
ACKs for top commit:
fanquake:
ACK 5c3e4d8b29
dergoegge:
ACK 5c3e4d8b29
Tree-SHA512: 4ff73c2dd0f25cb96148e54bd867b8d340bd0fbc9b9a736a705125039352eb1d40bd724f9f262a44d3dbd1bea8f03166cf30e571d882fec02ceb1dd399ef7422
Trying to shut down a node after a test failure may fail and lead to an
RPC error.
Also, it is confusing to sidestep the existing fallback to kill any
leftover nodes on a test failure.
So just rely on the fallback.
Idea by Hodlinator.
Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
4da7bfdcc9 test: add coverage for unknown address type for `createwalletdescriptor` (brunoerg)
Pull request description:
Calling `createwalletdescriptor` RPC with an unknown address type throws an error. This PR adds test coverage for it as done for other RPCs (`getnewaddress `, `getrawchangeaddress`, etc).
ACKs for top commit:
maflcko:
lgtm ACK 4da7bfdcc9
rkrux:
tACK 4da7bfdcc9
Tree-SHA512: 490bc3ffeb70b0f26db0a44d3950d7410fef35d4056487f2e82c081fb14ca277a18943c487235e0163a29f90fc741a262c29835beb9f41936affa4e73ddad25f
