b36e738285 MOVEONLY: Move abortrescan from backup.cpp to transactions.cpp (Samuel Dobson)
d794d0da8f Remove unused imports from rpc/wallet and reorder RPCs (Samuel Dobson)
e116b9747d MOVEONLY: Move rpcwallet to rpc/wallet (Samuel Dobson)
8e30875fde MOVEONLY: Move spending RPCs to spend.cpp (Samuel Dobson)
9ce521a61b MOVEONLY: Move balance and utxo RPCs to coins.cpp (Samuel Dobson)
7b45f5c059 MOVEONLY: Move address related functions from rpcwallet to addresses.cpp (Samuel Dobson)
f7646b407f MOVEONLY: Move transaction related wallet RPCs to transactions.cpp (Samuel Dobson)
Pull request description:
This is the rest of #23622, to split up rpcwallet into smaller, more logical parts.
This will have a lot of conflicts but let's just get it over and done with.
ACKs for top commit:
achow101:
ACK b36e738285
ryanofsky:
Code review ACK b36e738285, verified move-only again
Tree-SHA512: 6695fa23bbe9822c7497db7660f44c3dcb01dfa7276f5830a6b7c73c6b5fa04e04fcd4821bf0e6392e7659ed91a277ced85bd8f77477d785bca4e84a93fe791e
1dcba996d3 Coinbase receivedby rpcs release notes (Andrew Toth)
b5696750a9 Test including coinbase transactions in receivedby wallet rpcs (Andrew Toth)
bce20c34d6 Include coinbase transactions in receivedby wallet rpcs (Andrew Toth)
Pull request description:
The current `*receivedby*` RPCs filter out coinbase transactions. This doesn't seem correct since an output to your address in a coinbase transaction *is* receiving those coins.
This PR corrects this behaviour. Also, a new option `include_immature_coinbase` is added (default=`false`) that includes immature coinbase transactions when set to true.
However, since this is potentially a breaking change this PR introduces a hidden configuration option `-deprecatedrpc=exclude_coinbase`. This can be set to revert to previous behaviour. If no reports of broken workflow are received, then this option can be removed in a future release.
Fixes https://github.com/bitcoin/bitcoin/issues/14654.
ACKs for top commit:
jnewbery:
reACK 1dcba996d3
Tree-SHA512: bfc43b81279fea5b6770a4620b196f6bc7c818d221b228623e9f535ec75a2406bc440e3df911608a3680f11ab64c5a4103917162114f5ff7c4ca8ab07bb9d3df
275e9390e1 mining, refactor: add m_mempool.cs thread safety lock assertions (Jon Atack)
Pull request description:
in src/node/miner to
- BlockAssembler::addPackageTxs()
- BlockAssembler::SkipMapTxEntry()
- BlockAssembler::UpdatePackagesForAdded()
These functions have thread safety lock annotations in their declarations but are missing the corresponding run-time lock assertions in their definitions.
Per doc/developer-notes.md: "Combine annotations in function declarations with run-time asserts in function definitions."
ACKs for top commit:
shaavan:
ACK 275e9390e1. Thanks for catching and fixing this!
Tree-SHA512: 1c6f1ad1bbd94ff391fc8ce1e3b95d88bd3db5db804a1a5ef4636e54b29f5801f79aa9ed753d34c9a79a58cf01c7ed890e7681ff1c7b0f16335dc062bbac31cc
fa1571b156 doc: Add missing optional to MempoolEntryDescription (MarcoFalke)
Pull request description:
Needed for https://github.com/bitcoin/bitcoin/pull/23083.
Can be reviewed with `--word-diff-regex=.`.
ACKs for top commit:
josibake:
ACK fa1571b156
shaavan:
ACK fa1571b156
Tree-SHA512: b4370003d2aeadce438778e15bd9a0d6a7fef4711acbe8471a50a9d72bbf74e1705fecbaae6f7eb367ece7c795a816c4b8b6583ed6c8f91b35621ca30fd95c18
2f9515f37a rpc: move fees object to match help (josibake)
07ade7db8f doc: add release note for fee field deprecation (josibake)
2ee406ce3e test: add functional test for deprecatedrpc=fees (josibake)
35d928c632 rpc: deprecate fee fields from mempool entries (josibake)
Pull request description:
per #22682 , top level fee fields for mempool entries have been deprecated since 0.17 but are still returned. this PR properly deprecates them so that they are no longer returned unless `-deprecatedrpc=fees` is passed.
the first commit takes care of deprecation and also updates `test/functional/mempool_packages.py` to only use the `fees` object. the second commit adds a new functional test for `-deprecatedrpc=fees`
closes#22682
## questions for the reviewer
* `-deprecatedrpc=fees` made the most sense to me, but happy to change if there is a name that makes more sense
* #22682 seems to indicate that after some period of time, the fields will be removed all together. if we have a rough idea of when this will be, i can add a `TODO: fully remove in vXX` comment to `entryToJSON`
## testing
to get started on testing, compile, run the tests, and start your node with the deprecated rpcs flag:
```bash
./src/bitcoind -daemon -deprecatedrpc=fees
```
you should see entries with the deprecated fields like so:
```json
{
"<txid>": {
"fees": {
"base": 0.00000671,
"modified": 0.00000671,
"ancestor": 0.00000671,
"descendant": 0.00000671
},
"fee": 0.00000671,
"modifiedfee": 0.00000671,
"descendantfees": 671,
"ancestorfees": 671,
"vsize": 144,
"weight": 573,
...
},
```
you can also check `getmempoolentry` using any of the txid's from the output above.
next start the node without the deprecated flag, repeat the commands from above and verify that the deprecated fields are no longer present at the top level, but present in the "fees" object
ACKs for top commit:
jnewbery:
reACK 2f9515f37a
glozow:
utACK 2f9515f37a
Tree-SHA512: b175f4d39d26d96dc5bae26717d3ccfa5842d98ab402065880bfdcf4921b14ca692a8919fe4e9969acbb5c4d6e6d07dd6462a7e0a0a7342556279b381e1a004e
in src/node/miner to:
- BlockAssembler::addPackageTxs()
- BlockAssembler::SkipMapTxEntry()
- BlockAssembler::UpdatePackagesForAdded()
These functions have thread safety lock annotations in
their declarations but are missing the corresponding
run-time lock assertions in their definitions.
Per doc/developer-notes.md: "Combine annotations in function
declarations with run-time asserts in function definitions."
faa185bb3a Revert "Fixes Bug in Transaction generation in ComplexMempool benchmark" (MarcoFalke)
Pull request description:
Developers are reporting crashes (potentially OOM) on IRC, but I can't reproduce. Still, revert this for now, since one developer reported the bare metal this was running on crashed.
Top commit has no ACKs.
Tree-SHA512: 080db4fcfc682b68f4cc40dfabd9d3e0e3f6e6297ce4b782d5de2c83bc18f85f60efb1cda64c51e23c4fd2a05222a904e7a11853d9f9c052dcd26a53aa00b235
78a6bc6919 build, qt: Use Android NDK r23 LTS (Hennadii Stepanov)
Pull request description:
This is a continuation of bitcoin/bitcoin#23478, and, thanks to bitcoin/bitcoin#23489, a oneline patch is only required to be able build the `qt` package in depends with Android NDK r23 LTS.
ACKs for top commit:
fanquake:
ACK 78a6bc6919
Tree-SHA512: 09c6e8739ecbcbf5fdd6c2103577be2676eb448941f97c781f476918056c8405d2531d5cef8f240e4d1205c2d49f879edbba74dd5e77799a887b76a5c76ebe5b
1ed5681407 rpc: add missing scantxoutset examples (Sebastian Falbesoner)
Pull request description:
The scantxoutset RPC and its help text was at last improved in #16285, but it's still missing examples (see https://github.com/bitcoin/bitcoin/pull/16285#issuecomment-529313781).
~Note that the example descriptor used doesn't follow the developer guideline of using invalid bech32 addresses, as the RPC is not wallet-related and it's use-case is merely to look up state information (i.e. there is no danger of sending funds to a wrong address).~ For the sake of simplicity, the raw descriptor for an early coinbase payout address (block 9) is taken, i.e. it yields results even at an early stage of IBD. Happy to change that though if there are other suggestions.
ACKs for top commit:
shaavan:
reACK 1ed5681407
Tree-SHA512: 057ad9ac0d019035bee2332440128de0ef08580bbeae80182ff74771beead3555c4bf7008071a97bbb6a8d85fb85d0f0754fb7941db2c5b755eae1ac9aa65318
29e983386b Fixes Bug in Transaction generation in ComplexMempool benchmark (Shorya)
Pull request description:
This fixes issues with `ComplexMempool` benchmark introduced in [#17292](https://github.com/bitcoin/bitcoin/pull/17292) , this stress test benchmarks performance of ancestor and descendant tracking of mempool graph algorithms on a complex Mempool.
This Benchmark first creates 100 base transactions and stores them in `available_coins` vector. `available_coins` is used for selecting ancestor transactions while creating 800 new transactions. For this a random transaction is picked from `available_coins` and some of its outputs are mapped to the inputs of the new transaction being created.
Now in case we exhaust all the outputs of an entry in `available_coins` then we need to remove it from `available_coins` before the next iteration of choosing a potential ancestor , it is now implemented with this patch.
As the index of the entry is randomly chosen from `available_coins` , In order to remove it from the vector , if index of the selected entry is not at the end of `available_coins` vector , it is swapped with the entry at the back of the vector , then the entry at the end of `available_coins` is popped out.
Earlier the code responsible for constructing outputs of the newly created transaction was inside the loop used for assigning ancestors to the transaction , which does some unnecessary work as it creates outputs of the transaction again and again , now it is moved out of the loop so outputs of the transaction are created just once before adding it to the final list of the transactions created. This one is a minor change to save some computation.
These changes have changed the `ComplexMempool` benchmark results on `bitcoin:master` as follows :
**Before**
>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 232,881,625.00 | 4.29 | 0.7% | 2.55 | `ComplexMemPool`
**After**
>
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 497,275,135.00 | 2.01 | 0.5% | 5.49 | `ComplexMemPool`
Top commit has no ACKs.
Tree-SHA512: d6946d7e65c55f54c84cc49d7abee52e59ffc8b7668b3c80b4ce15a57690ab00a600c6241cc71a2a075def9c30792a311256fed325ef162f37aeacd2cce93624
0c85dc30e6 p2p: Don't use timestamps from inbound peers (Martin Zumsande)
Pull request description:
`GetAdjustedTime()` (used e.g. in validation and addrman) returns a time with an offset that is influenced by timestamps that our peers have sent us in their version message.
Currently, timestamps from all peers are used for this.
However, I think that it would make sense to ignore the timedata samples from inbound peers, making it much harder for others to influence the Adjusted Time in a targeted way.
With the extra feeler connections (every 2 minutes on average) and extra block-relay-only connections (every 5 minutes on average) there are also now plenty of opportunities to gather a meaningful number of timedata samples from outbound peers.
There are some measures in place to prevent abuse: the `-maxtimeadjustment` parameter with a default of 70 minutes, warnings in cases of large deviations, only using the first 200 samples ([explanation](383d350bd5/src/timedata.cpp (L57-L72))), but I think that only using samples from outbound connections in the first place would be an additional safety measure that would make sense.
See also issue #4521 for further context and links: There have been several discussions in the past about replacing or abolishing the existing timedata system.
ACKs for top commit:
jnewbery:
Concept and code review ACK 0c85dc30e6
naumenkogs:
ACK 0c85dc30e6
vasild:
ACK 0c85dc30e6
Tree-SHA512: 2d6375305bcae034d68b58b7a07777b40ac430dfed554c88e681a048c527536691e1b7d08c0ef995247d356f8e81aa0a4b983bf2674faf6a416264e5f1af0a96
cd8d156354 Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable (Luke Dashjr)
Pull request description:
Fixes a regression introduced by #22722
(Not entirely sure on the solution)
ACKs for top commit:
prayank23:
crACK cd8d156354
darosior:
utACK cd8d156354
kristapsk:
utACK cd8d156354
Tree-SHA512: eb4aa3cc345c69c44ffd5733b51b90eefe1d7854b7a2855e8cbb98268db24d43b7d0ae9fbb0eccf9b6dc01da644d19433cc77fec52ff67bf890be1fc53a67fc4
d6d402bd2b build: remove x-prefix comparisons (fanquake)
Pull request description:
Very old shells suffered from bugs which meant that prefixing variables
with an "x" to ensure that the lefthand side of a comparison always
started with an alphanumeric character was needed. Modern shells don't
suffer from this issue (i.e Bash was fixed in 1996).
In any case, we've already got unprefixed checks used in our codebase, i.e
681b25e3cd/configure.ac (L292)
and have libs (in depends) that also use unprefixed comparisons in their
configure scripts.
I think it's time that we consolidate on not using the x-prefix workaround.
At best it's mostly just confusing. Could simplify some of these checks
further in future.
More info:
https://github.com/koalaman/shellcheck/wiki/SC2268https://www.vidarholen.net/contents/blog/?p=1035
ACKs for top commit:
MarcoFalke:
Concept ACK d6d402bd2b
Tree-SHA512: 70030d61dcdb5b009823d83d73204627de53d2f628d8d6478e8e16804ac09f6bbdc53cbb1a6fb2085ebfe1a694b576e46ff381fb980cf667fab4bbadc79587d7
fa5362a9a0 rpc: Add missing BlockUntilSyncedToCurrentChain to wallet RPCs (MarcoFalke)
Pull request description:
Wallet RPCs that allow a rescan based on block-timestamp or block-height
need to sync with the active chain first, because the user might assume
the wallet is up-to-date with the latest block they got reported via a
blockchain RPC.
ACKs for top commit:
meshcollider:
utACK fa5362a9a0
Tree-SHA512: d4831f1f08f854f9a49fc969de86c438f856e41c2163c801a6ff36dc2f6299cb342b44663279c524a8b7ca9a50895db1243cd7d49bed79277ada857213f20a26
31ba1af74a Remove unused (and broken) functionality in SpanReader (Pieter Wuille)
Pull request description:
This removes the ability to set an offset in the `SpanReader::SpanReader` constructor, as the current code is broken since #23653. All call sites use `pos=0`, so it is actually unused. If future call sites need it, `SpanReader{a, b, c, d}` is equivalent to `SpanReader{a, b, c.subspan(d)}`.
It also removes the ability to deserialize from `SpanReader` directly from the constructor. This too is unused, and can be more idiomatically simulated using `(SpanReader{a, b, c} >> x >> y >> z)` instead of `SpanReader{a, b, c, x, y, z}`.
This was pointed out by achow101 in https://github.com/bitcoin/bitcoin/pull/23653#discussion_r763370432.
ACKs for top commit:
jb55:
crACK 31ba1af74a
achow101:
ACK 31ba1af74a
Tree-SHA512: 700ebcd74147628488c39168dbf3a00f8ed41709a26711695f4bf036250a9b115574923bbf96040ec7b7fee4132d6dbbcb5c6e5a2977c4beb521dc1500e6ed53
5767208504 correct rpc address_type helptext (brianddk)
Pull request description:
RPC calls `getnewaddress`/`getrawchangeaddress` support the address_type of `bech32m` but it is omitted in the `RPCHelpMan` help text.
The `createmultisig` and `addmultisigaddress` help text was not updated since `bech32m` is not yet supported in these.
ACKs for top commit:
shaavan:
ACK 5767208504
Tree-SHA512: 3c0cfb96019ca6d316c4a2fe27786d1b621c49b31b3aa61068bad737a5a0ceed89babad704b9923f9aedcabfa670d752916803bdf22236403061ddf9295a2637
fa37e798b2 wallet: Replace confusing getAdjustedTime() with GetTime() (MarcoFalke)
Pull request description:
Setting `nTimeReceived` to the adjusted time has several issues:
* `m_best_block_time` is set to the "unadjusted" time, thus a comparison of the two times is like comparing apples to oranges. In the worst case this opens up an attack vector where remote peers can force a premature re-broadcast of wallet txs.
* The RPC documentation for `"timereceived"` doesn't mention that the network adjusted time is used, possibly confusing users when the time reported by RPC is off by a few seconds compared to their local timestamp.
Fix all issues by replacing the call with `GetTime()`. Also a style fix: Use non-narrowing integer conversion in the RPC method.
ACKs for top commit:
theStack:
Code-review ACK fa37e798b2
shaavan:
crACK fa37e798b2
Tree-SHA512: 8d020ba400521246b7aed4b6c41319fc70552e8c69e929a5994500375466a9edac02a0ae64b803dbc6695df22276489561a23bd6e030c44c97d288f7b9b2b3fa
0ba98eda28 test: remove unneeded sync_all() calls in wallet_listtransactions.py (Sebastian Falbesoner)
Pull request description:
This is a small follow-up to #23659. The `self.sync_all()` calls after generating blocks can be removed, since that happens automatically per default by the test framework's generate function (if no explicit sync_fun is passed).
On the course of touching the file, imports are sorted and the grammar of a log message is fixed.
ACKs for top commit:
fanquake:
ACK 0ba98eda28 - thanks for following up.
shaavan:
ACK 0ba98eda28
Tree-SHA512: 451e733865dcb1e424d90289c8c89272837a9af6fd4b77d6c60728c84524d9c792d684b7e601b02a0efda67231183c42dd9040d96214ac7d9473b2808cabe73f
035767f54a test: add interface_bitcoin_cli.py --descriptors to test_runner.py (Sebastian Falbesoner)
e4fa28a322 test: fix test interface_bitcoin_cli.py for descriptor wallets (Sebastian Falbesoner)
Pull request description:
The functional test interface_bitcoin_cli.py currently fails on master branch, if descriptor wallets are used (argument `--descriptors`), see #23684. This is due to the fact that different change output types are used for created transactions (P2WPKH for legacy wallets, P2TR for descriptor wallets; the former doesn't have a ScriptPubKeyMan for bech32m), resulting in different tx sizes and hence also fees. Fix this by explicitely setting the output type via passing both `-addresstype=bech32` and `-changetype=bech32` as argument. The former would not be needed by now, but makes the test more deterministic and avoids a failure if bech32m becomes the default address type.
Fixes#23684, should also pave the way for #23682.
Top commit has no ACKs.
Tree-SHA512: 39b780e25e4c7094cb3378e0f10d4a8aebac1500b7b2d68de47e54e23b9b5efe5afcf8765bb8398eeaf56968e2586a1b294a0f8773c7d90f4188a0f00b8501ff
faef7e93e1 doc: Merge release note snippets (MarcoFalke)
Pull request description:
Periodic merge to make it easier to browse them in one file.
Can be reviewed with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
ACKs for top commit:
shaavan:
ACK faef7e93e1
fanquake:
ACK faef7e93e1
Tree-SHA512: 590f36af45a53413bd602fdd8810bf5733b4bad78bf257f3e91e1001f47d86e5a00db4fcbaaa2585e702c2d824e6c225da14e7ae7a90c2a150908c278bb2a911
This removes the ability to set an offset in the SpanReader constructor,
as the current code is broken. All call sites use pos=0, so it is actually
unused. If future call sites need it, SpanReader{a, b, c, d} is equivalent
to SpanReader{a, b, c.subspan(d)}.
It also removes the ability to deserialize from SpanReader directly from
the constructor. This too is unused, and can be more idiomatically
simulated using (SpanReader{a, b, c} >> x >> y >> z) instead of
SpanReader{a, b, c, x, y, z}.
a989f98d24 refactor: net: subnet lookup: use single-result LookupHost() (Sebastian Falbesoner)
Pull request description:
plus describe single IP subnet case for more clarity
ACKs for top commit:
jonatack:
utACK a989f98d24 the patch rebases cleanly to master, the debug build is green, and it is essentially the same patch as c8991f0251dd2a modulo local variable naming, braced initialization, and a comment
vasild:
ACK a989f98d24
Tree-SHA512: 082d3481b1fa5e5f3267b7c4a812954b67b36d1f94c5296fe20110699f053e5042dfa13f728ae20249e9b8d71e930c3b119410125d0faeccdfbdc259223ee3a6
99993425af rpc: Only allow specific types to be P2(W)SH wrapped in decodescript (MarcoFalke)
Pull request description:
It seems confusing to return a P2SH wrapping address that is eventually either policy- or consensus-unspendable.
ACKs for top commit:
laanwj:
Code review re-ACK 99993425af
Tree-SHA512: 3cd530442acee7c295d244995f0f17b2cae7212f1e0970bb5807621f8ff8e4308a3236b385d77087cd493d32ee524813d8edd15e91d937ef9a800094b7bc4946
7da4a8ffb3 cover DisconnectBlock with lock annotation (James O'Beirne)
Pull request description:
While reviewing #23630, I noticed that `DisconnectBlock` is uncovered by lock annotations. CoinsTip() access requires cs_main and therefore so should this function.
ACKs for top commit:
jonatack:
ACK 7da4a8ffb3
Tree-SHA512: 3e2b0247c138b31deeadcd48eb3f7bc8d32c0b6bb6d6e94ccf8ea0cbbc50b1b35d83f662eee432f2bd2d87a3fe9c94604da806ec711df93298bfb0ab34a5a05b
4740fe8212 test: Add test for block relay only eviction (Martin Zumsande)
Pull request description:
Adds a unit test for block-relay-only eviction logic added in #19858, which was not covered by any tests before. The added test is very similar to the existing `stale_tip_peer_management` unit test, which tests the analogous logic for regular outbound peers.
ACKs for top commit:
glozow:
reACK 4740fe8212
rajarshimaitra:
tACK 4740fe8212
shaavan:
ACK 4740fe8212. Great work @ mzumsande!
LarryRuane:
ACK 4740fe8212
Tree-SHA512: 5985afd7d8f7ae311903dbbf6b7d526e16309c83c88ae6dd6551960c0b186156310a6be0cf6b684f82ac1378d0fc5aa3717f0139e078471013fceb6aebe81bf6
e4a54af6b8 test: add wallet_transactiontime_rescan.py --descriptors to test_runner.py (Sebastian Falbesoner)
b60e02e993 test: fix test wallet_transactiontime_rescan.py for descriptor wallets (Sebastian Falbesoner)
a905ed1a61 test: refactor: use `set_node_times` helper in wallet_transactiontime_rescan.py (Sebastian Falbesoner)
Pull request description:
The functional test wallet_transactiontime_rescan.py currently fails on master branch, if descriptor wallets are used (argument `--descriptors`). This is due to the fact that in this case, the test framework maps the importaddress RPC calls to the importdescriptors RPC (rescan=False -> timestamp='now'), which always rescans blocks of the past 2 hours, based on the current MTP timestamp. In order to avoid importing the last address (wo3), we generate 10 more blocks with advanced time, to ensure that the balance after importing is zero:
681b25e3cd/test/functional/wallet_transactiontime_rescan.py (L125-L134)
Calling this test with descriptor wallets is also added to test runner. Fixes#23562.
ACKs for top commit:
Sjors:
tACK e4a54af
brunoerg:
tACK e4a54af6b8
Tree-SHA512: 9fd8e298d48dd7947b1218d61a1a66c1241b3dbb14451b0ec7cd30caa74ee540e7ee5a7bd10d421b9e3b6e549fa5c3e85bd02496436128b433b328118642f600
a4fe70171b Make Bech32 LocateErrors return error list rather than using out-arg (Samuel Dobson)
2fa4fd1961 Use std::iota instead of manually pushing range (Samuel Dobson)
405c96fc9f Use bounds-checked array lookups in Bech32 error detection code (Samuel Dobson)
28d9c2857f Simplify encoding of e in GF(1024) tables to (1,0) (Samuel Dobson)
14358a029d Replace GF1024 tables and syndrome constants with compile-time generated constexprs. (Samuel Dobson)
63f7b69779 Update release note for bech32 error detection (Samuel Dobson)
c8b9a224e7 Report encoding type in bech32 error message (Samuel Dobson)
92f0cafdca Improve Bech32 boost tests (Samuel Dobson)
bb4d3e9b97 Address review comments for Bech32 error validation (Samuel Dobson)
Pull request description:
A number of follow-ups and improvements to the bech32 error location code, introduced in #16807.
Notably, this removes the hardcoded GF1024 tables in favour of constexpr table generation.
ACKs for top commit:
laanwj:
Re-ACK a4fe70171b
Tree-SHA512: 6312373c20ebd6636f5797304876fa0d70fa777de2f6c507245f51a652b3d1224ebc55b236c9e11e6956c1e88e65faadab51d53587078efccb451455aa2e2276
b35942e500 tests: Add data-driven testcases to rpc_decodescript.py (Dimitri)
Pull request description:
closes#23641
Top commit has no ACKs.
Tree-SHA512: 2f494c78ad085d523fae15befaadb9e0fc382b5310e3a95395ecf06a90968b15b6d232f7564098ed0a68419e27aa2e5260fe691cf2ce84af9fb6b65634e54d77
c055f6b216 test: Remove false coinstatsindex test (Fabian Jahr)
Pull request description:
This test never actually tested the behavior that it describes in the comments. This was discovered in #21590 which seems to speed up muhash which lead to the test failing.
I can vaguely remember that the described behavior was desired by some reviewers of `coinstatsindex`: That `coinstatsindex` should be aware of stale blocks and able to return statistics on them as well. The index actually does this for blocks that it sees while the index is active, i.e. while running `coinstatsindex` all blocks will be indexed and even when they become stale the index (via `gettxoutsetinfo`) will still return a result for them when given the right hash. But this currently does not work for blocks that the node saw and that became stale _before_ the node activated `coinstatsindex`. While the index syncs initially everything but the active chain is ignored and I don't see any indication that this ever worked differently in the past.
Introducing this behavior seems non-trivial at first glance so, while I will give this a shot, I think the test should be removed so it does not confuse users and does not block #21590.
Top commit has no ACKs.
Tree-SHA512: b05f5dfeea3e453c8bb7c761501d0d896d4412a3f0c08037955951fae9fe388c63402da401792591e18da8fb67734f47f1a297d573cdb66e0ced451698718067
