fa0758e1457552e9243bc9663408e88a8cccc8f6 test: Add diamond-shape prioritisetransaction test (MarcoFalke)
fa450c18db7326b2924db435a753620895b41c53 test: Rework create_self_transfer_multi (MarcoFalke)
Pull request description:
Looks like there is no test for diamonds, only for chains (in `mempool_packages.py`)
ACKs for top commit:
jamesob:
ACK fa0758e145
Tree-SHA512: d261184a81df77d24fc256f58ad5ed4a13b7cd4e33f74c8b79495c761ff417817602d8e5d4f63f4bb1000ac63f89bbfa54d8d8994a7b2bb2e8a484c467330984
45e67b2695852c94d5da3f9b82f2b567cbbd2cc3 test: invalid -i2psam will raise an init error (brunoerg)
Pull request description:
This PR adds test coverage (at `feature_proxy.py`) for the following init error:
2f0f056e08/src/init.cpp (L1791)
It starts the node with an invalid -i2psam (`-i2psam=invalidhere`) and test if it raises an error when initializing.
ACKs for top commit:
dunxen:
Code review ACK 45e67b2
Tree-SHA512: b24e3f6e7a9316b9ebc0b6c8bcf1315faff60a9e258d7bb3dbeb9f6695a728bb3083aea2f81114072fe13822bfca34d4a0f44f229825f7c97a81619d810010c0
89bb25d22a0e1c700dba4e3b754984c9b2b14836 test: check localaddresses in getnetworkinfo for nodes with proxy (brunoerg)
Pull request description:
This PR adds test coverage for the field `localaddresses` for `getnetworkinfo`. In this case, it verifies if this field is empty for all nodes since they are using proxy.
Reference:
515200298b/src/init.cpp (L449)
ACKs for top commit:
jonatack:
ACK 89bb25d22a0e1c700dba4e3b754984c9b2b14836
Tree-SHA512: 3c765c7060b6972c1ae5a1104734cd7669b650b5f6aa4f623f4299567732260da5083fef306a7c1e71c931f5d1396f24abad251d95c3d82b1f3ee0efee7fcd1f
fac5a51c47fba21678fb35805e40d00fe7c891a0 Move mempool RPCs to rpc/mempool (MarcoFalke)
fa0f666dd7b55a5a0250c5e35d2c3dfd565463b7 style: Add static keyword where possible in rpc/mempool (MarcoFalke)
Pull request description:
This moves the remaining mempool RPCs to `rpc/mempool`. Previously all mempool RPCs from the `blockchain` category have been moved. This patch moves the ones from the `rawtransactions` category.
In the future, as a follow-up to this refactoring patch, it could be considered whether a new `mempool` category should be introduced.
Beside a clearer code organization, this pull request should also reduce the compile time and space of the `rawtransactions.cpp` file.
ACKs for top commit:
promag:
Code review ACK fac5a51c47fba21678fb35805e40d00fe7c891a0.
Tree-SHA512: 5578b894b68d0595869a9b03ed8dceebe3366f73dec5f090ccc36ff4002b1bc4d58af77546c2d71537c1be03694d9a28c4b1bfbb3569560997879293c5c0301e
7e22d80af333b202939bcb2631082006c097bf22 addrman: fix incorrect named args (fanquake)
67f654ef612c8dbefb969e6e67c286ea2c2e82d6 doc: Document clang-tidy in dev notes (MarcoFalke)
Pull request description:
The documentation, and a single commit extracted from #24661.
Motivation:
> Incorrect named args are source of bugs, like https://github.com/bitcoin/bitcoin/pull/22979.
> To allow them being checked by clang-tidy, use a format it can understand.
ACKs for top commit:
glozow:
ACK 7e22d80af333b202939bcb2631082006c097bf22
Tree-SHA512: 4037fcea59fdf583b171bce7ad350299fe5f9feb3c398413432168f3b9a185e51884d5b30e4b4ab9c6c5bb896c178cfaee1d78d5b4f0034cd70121c9ea4184b7
9053f64fcbd26d87c26ae6b982d17756a6ea0896 [doc] release notes for random change target (glozow)
46f2fed6c5e0fa623bfeabf61ba4811d5cf8f47c [wallet] remove MIN_CHANGE (glozow)
a44236addd01cff4e4d751e0f379d399fbfc8eae [wallet] randomly generate change targets (glozow)
1e52e6bd0a8888efb4ed247d74ec7ca9dfc2e002 refactor coin selection for parameterizable change target (glozow)
Pull request description:
Closes#24458 - the wallet always chooses 1 million sats as its change target, making it easier to fingerprint transactions created by the Core wallet. Instead of using a fixed value, choose one randomly each time (within a range). Using 50ksat (around $20) as the lower bound and `min(1 million sat, 2 * average payment value)` as the upper bound.
RFC: If the payment is <25ksat, this doesn't work, so we're using the range (payment amount, 50ksat) instead.
ACKs for top commit:
achow101:
ACK 9053f64fcbd26d87c26ae6b982d17756a6ea0896
Xekyo:
reACK 9053f64fcbd26d87c26ae6b982d17756a6ea0896
Tree-SHA512: 45ce5d064697065549473347648e29935733f3deffc71a6ab995449431f60302d1f9911a0994dfdb960b48c48b5d8859f168b396ff2a62db67d535a7db041d35
da2bc865d644f6be748c305556bdd02f02d1b161 [wallet] don't create long chains by default (glozow)
Pull request description:
Default mempool policy doesn't let you have chains longer than 25 transactions. This is locally configurable of course, but it's not really safe to assume that a chain longer than 25 transactions will propagate. Thus, the wallet should probably avoid creating such transactions by default; set `DEFAULT_WALLET_REJECT_LONG_CHAINS` to true.
Closes#9752Closes#10004
ACKs for top commit:
MarcoFalke:
re-ACK da2bc865d644f6be748c305556bdd02f02d1b161 only change is fixing typos in tests 🎏
Tree-SHA512: 65d8e4ec437fe928adf554aa7e819a52e0599b403d5310895f4e371e99bbc838219b3097c4d2f775bc870ac617ef6b4227b94291f2b376f824f14e8f2b152f31
0000ff0d6b2442706a588fd906ebf1adf8ff8226 test: move-only: Move all generate* tests to a single file (MarcoFalke)
Pull request description:
Seems a bit overkill to spread tests for the `generate*` methods over several files. Combining them into a single file has also a nice side-effect of requiring less node (re)starts, which are expensive in valgrind.
ACKs for top commit:
glozow:
utACK 0000ff0d6b2442706a588fd906ebf1adf8ff8226
Tree-SHA512: 8269eb05649a871011bbfbd1838d0f7d1dac4a35b3b198fc43fe85131fda8a53803b75da78cbf422eabf086006dee4421e622fbe706f6781a3848b989024001b
3bb96274632cc914e9fe7a97f5e029bd29187db5 refactor: remove unused boost header include in bitcoin-util.cpp (Sebastian Falbesoner)
Pull request description:
This header was included since the introduction of bitcoin-util in
commit 13762bcc9618138dd28b53c2031defdc9d762d26, but boost was
actually never used (see `git log -S boost ./src/bitcoin-util.cpp`).
Cherry-picked out of #22953, which currently needs rebase. This commit could just be merged on its own.
ACKs for top commit:
MarcoFalke:
review ACK 3bb96274632cc914e9fe7a97f5e029bd29187db5
Tree-SHA512: 201ee1aa4d49074056654203db73a473479c2b92c49df8dbf8e35979f85178013c66540a665f0f6dc0a2efef88eb091e2b088bebff85d840033dffd8ae719349
fab287cedde85b21622d767d3ece65291e18b0bf Clarify that COutput is a struct, not a class (MarcoFalke)
fa61cdf464381dddd9da076b1a1cab95ff5b3baf wallet: Fix coinselection include (MarcoFalke)
Pull request description:
* Fix include (see commit message)
* `{}`-init, see https://github.com/bitcoin/bitcoin/pull/24091#discussion_r831193284
* `struct`, see https://github.com/bitcoin/bitcoin/pull/24091#discussion_r831192702
ACKs for top commit:
theStack:
Code-review ACK fab287cedde85b21622d767d3ece65291e18b0bf
Tree-SHA512: dd2cfb9c06a92295dbd8fbb6d56afcf00ebda2a0440e301d392cd183d1b9cd87626311d539e302a9e6c6521d69d6183c74a51934e3fc16e64a5dcaba60c7e3ce
This header was included since the introduction of bitcoin-util in
commit 13762bcc9618138dd28b53c2031defdc9d762d26, but boost was
actually never used (see `git log -S boost ./src/bitcoin-util.cpp`).
0346c26fcacca8abcf67f7320fd441e564aa97d1 init: add missing cs_main lock (Anthony Towns)
Pull request description:
`BlockManager::m_block_tree_db` is protected by `cs_main`, so take the
`cs_main` lock while accessing it.
ACKs for top commit:
jonatack:
Code review ACK 0346c26fcacca8abcf67f7320fd441e564aa97d1
Tree-SHA512: d6dff0b2d58871c7fbb281558b59fa9ad26fa75b3ceca9232277fc49ab795325e5ac3d266db49e7bda33da6de0b014b1bdebdf2c2c4347d43e50c0433a2cf06c
1066d10f71e6800c78012d789ff6ae19df0243fe scripted-diff: rename TxRelay members (John Newbery)
575bbd0dea6d12510fdf3220d0f0e47d969da6e9 [net processing] Move tx relay data to Peer (John Newbery)
785f55f7eeab0dedbeb8e0d0b459f3bdc538b621 [net processing] Move m_wtxid_relay to Peer (John Newbery)
36346703f8558d6781c079c29ddece5a97477beb [net] Add CNode.m_relays_txs and CNode.m_bloom_filter_loaded (John Newbery)
Pull request description:
This continues the work of moving application layer data into net_processing, by moving all tx data into the new Peer object added in #19607.
For motivation, see #19398.
ACKs for top commit:
dergoegge:
ACK 1066d10f71e6800c78012d789ff6ae19df0243fe - This is a good layer separation improvement with no behavior changes.
glozow:
utACK 1066d10f71e6800c78012d789ff6ae19df0243fe
Tree-SHA512: 0c9d6b8a0a05e2d816b6d6588b7df133842ec960ae67667813422aa7bd8eb5308599c714f3822a98ddbdf364ffab9050b055079277ba4aff24092557ff99ebcc
cccc1e70b8a14430cc94143da97936a60d6c83d3 Enforce Taproot script flags whenever WITNESS is set (MarcoFalke)
fa422994116a7a053789304d56159760081479eb Remove nullptr check in GetBlockScriptFlags (MarcoFalke)
faadc606c7644f2934de390e261d9d65a81a7592 refactor: Pass const reference instead of pointer to GetBlockScriptFlags (MarcoFalke)
Pull request description:
Now that Taproot is active, it makes sense to enforce its rules on all blocks, even historic ones, regardless of the deployment status.
### Benefits:
(With "script flags" I mean "taproot script verification flags".)
* Script flags are known ahead for all blocks (even blocks not yet created) and do not change. This may benefit static analysis, code review, and development of new script features that build on Taproot.
* Any future bugs introduced in the deployment code won't have any effect on the script flags, as they are independent of deployment.
* Enforcing the taproot rules regardless of the deployment status makes testing easier because invalid blocks after activation are also invalid before activation. So there is no need to differentiate the two cases.
* It gives belt-and-suspenders protection against a practically expensive and theoretically impossible IBD reorg attack where the node is eclipsed. While `nMinimumChainWork` already protects against this, the cost for a few months worth of POW might be lowered until a major version release of Bitcoin Core reaches EOL. The needed work for the attack is the difference between `nMinimumChainWork` and the work at block 709632.
For reference, previously the same was done for P2SH and WITNESS in commit 0a8b7b4b33c9d78574627fc606267e2d8955cd1c.
### Implementation:
I found one block which fails verification with the flags applied, so I added a `TaprootException`, similar to the `BIP16Exception`.
For reference, the debug log:
```
ERROR: ConnectBlock(): CheckInputScripts on b10c007c60e14f9d087e0291d4d0c7869697c6681d979c6639dbd960792b4d41 failed with non-mandatory-script-verify-flag (Witness program was passed an empty witness)
BlockChecked: block hash=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad state=non-mandatory-script-verify-flag (Witness program was passed an empty witness)
InvalidChainFound: invalid block=0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad height=692261 log2_work=92.988459 date=2021-07-23T08:24:20Z
InvalidChainFound: current best=0000000000000000000067b17a4c0ffd77c29941b15ad356ca8f980af137a25d height=692260 log2_work=92.988450 date=2021-07-23T07:47:31Z
ERROR: ConnectTip: ConnectBlock 0000000000000000000f14c35b2d841e986ab5441de8c585d5ffe55ea1e395ad failed, non-mandatory-script-verify-flag (Witness program was passed an empty witness)
```
Hint for testing, make sure to set `-noassumevalid`.
### Considerations
Obviously this change can lead to consensus splits on the network in light of massive reorgs. Currently the last block before Taproot activation, that is the last block without the Taproot script flags set, is only buried by a few days of POW. However, when and if this patch is included in the next major release, it will be buried by a few months of POW. BIP90 considerations apply when looking at reorgs this large.
ACKs for top commit:
Sjors:
tACK cccc1e70b8a14430cc94143da97936a60d6c83d3
achow101:
ACK cccc1e70b8a14430cc94143da97936a60d6c83d3
laanwj:
Code review ACK cccc1e70b8a14430cc94143da97936a60d6c83d3
ajtowns:
ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ; code review; wrote a "getblockscriptflags" rpc to quickly check that blocks just had bit 17 (taproot) added; review of earlier revisions had established non-exception blocks do validate with taproot rules enabled.
jamesob:
ACK cccc1e70b8a14430cc94143da97936a60d6c83d3 ([`jamesob/ackr/23536.1.MarcoFalke.enforce_taproot_script_f`](https://github.com/jamesob/bitcoin/tree/ackr/23536.1.MarcoFalke.enforce_taproot_script_f))
Tree-SHA512: 00044de68939caef6420ffd588c1291c041a8b397c80a3df1e3e3487fbeae1821d23975c51c95e44e774558db76f943b00b4e27cbd0213f64a9253116dc6edde
f8cba0d9117fe9b9ac51d7044372b28270c7838b test: Change default test logging directory (Yancy Ribbens)
Pull request description:
This PR changes the default test log location request here: https://github.com/bitcoin/bitcoin/issues/17224. Instead of using the location of the makefile [automatic variable](https://www.gnu.org/software/make/manual/make.html#Automatic-Variables) `$<` I extract just the basename and then prepend a new location `./test`. This is done because `$<` represents the variable name AND location of the prerequisite here.
Top commit has no ACKs.
Tree-SHA512: f0fbc530cf0e14c284b4bbf6671c145b1d7a2e1f5561f5c5d09f0cbe88b98e620e763bbbf2dfa9aeeec3dcc9b0127939e105e14c7e4f6660c7c19663622a393d
fa6e47d85be42b3065ad530257e5d4c23afcade3 ci: Compile fuzz binary in periodic task (MarcoFalke)
Pull request description:
There is one task that is re-run periodically by CI about once a week for all pull request to detect silent merge conflict before merge.
It would be nice if this task also checked for silent merge conflict in the fuzz binary.
ACKs for top commit:
fanquake:
ACK fa6e47d85be42b3065ad530257e5d4c23afcade3 green CI
vincenzopalazzo:
ACK fa6e47d85b
Tree-SHA512: fd5b02dc72af7a4ba973919d62c3aad11569f86a93622e2e56330ea9d379f1f015dfd5cb7efbf00718b306103028bc10c7e935579c2337fb506bd7fe3e145e1c
28f17c1a6de7b50482d8f55fe03698126862e8eb build: fix copypasta in OpenBSD C{XX} flags (fanquake)
Pull request description:
Introduced in #23998.
ACKs for top commit:
hebasto:
ACK 28f17c1a6de7b50482d8f55fe03698126862e8eb, I have reviewed the code and it looks OK, not tested on OpenBSD though.
Tree-SHA512: d905161534075f518c8924d3c42cca7ff8d4898e559f1daa9bd03dac95b109b2c3e76790fb8bc65b9e45e8a59566825afbf4dc3734ad74617dfdf797430e486b
b5ba3b5b2cbec4db94037baad960410fe98f3fdd doc: mention that BDB is for the legacy wallet in build-freebsd.md (fanquake)
Pull request description:
Re-order legacy and descriptor wallet section.
Remove prelude that pointlessly repeats the same info.
Cleanup configure examples.
FreeBSD version of https://github.com/bitcoin/bitcoin/pull/23446.
ACKs for top commit:
shaavan:
ACK b5ba3b5b2cbec4db94037baad960410fe98f3fdd
Tree-SHA512: ed85acee5c1b933b57d038d144768da1c4b1f1cfd441d9191353b82d50af16adb10aabb3b4661dc9aee54405ab3af2b800e39f9973261a041b2cf0db8675b5c4
049003fe68a4183f6f20da16f58f10079d1e02df coinselection: Remove COutput operators == and != (Andrew Chow)
f6c39c6adb6cbf9c87f04d3d667701905ef5c0a0 coinselection: Remove CInputCoin (Andrew Chow)
70f31f1a81710aa59e95770de9a84bf58cbce1e8 coinselection: Use COutput instead of CInputCoin (Andrew Chow)
14fbb57b79c664090f6a4e60d7bdfc9759ff4307 coinselection: Add effective value and fees to COutput (Andrew Chow)
f0821230b8de2eec21a869d1edf9e2b9f502de25 moveonly: move COutput to coinselection.h (Andrew Chow)
42e974e15c6deba1d9395a4da9341c9ebec6e8e5 wallet: Remove CWallet and CWalletTx from COutput's constructor (Andrew Chow)
14d04d5ad15ae56df56edee7ca9a202b52037889 wallet: Replace CWalletTx in COutput with COutPoint and CTxOut (Andrew Chow)
0ba4d1916e26e2a5d603edcdb7625463989d25b6 wallet: Provide input bytes to COutput (Andrew Chow)
d51f27d3bb0d6e3ca55bcd23ce53e4fe413a9360 wallet: Store whether a COutput is from the wallet (Andrew Chow)
b799814bbd53736b79495072f3c9e05989a465e8 wallet: Store tx time in COutput (Andrew Chow)
46022953ee2e8113167bafd1fd48a383a578b13c wallet: Remove use_max_sig default value (Andrew Chow)
10379f007fd2c18f4cd24d0a0783d6d929f45556 scripted-diff: Rename COutput member variables (Andrew Chow)
c7c64db41e1718584aa2f30ff27f60ab0966de62 wallet: cleanup COutput constructor (Andrew Chow)
Pull request description:
While working on coin selection code, it occurred to me that `CInputCoin` is really a subset of `COutput` and the conversion of a `COutput` to a `CInputCoin` does not appear to be all that useful. So this PR adds fields that are present in `CInputCoin` to `COutput` and replaces the usage of `CInputCoin` with `COutput`.
`COutput` is also moved to coinselection.h. As part of this move, the usage of `CWalletTx` is removed from `COutput`. It is instead replaced by storing a `COutPoint` and the `CTxOut` rather than the entire `CWalletTx` as coin selection does not really need the full `CWalletTx`. The `CWalletTx` was only used for figuring out whether the transaction containing the output was from the current wallet, and for the transaction's time. These are now parameters to `COutput`'s constructor.
ACKs for top commit:
ryanofsky:
Code review ACK 049003fe68a4183f6f20da16f58f10079d1e02df, just adding comments and removing == operators since last review
w0xlt:
reACK 049003f
Xekyo:
reACK 049003fe68a4183f6f20da16f58f10079d1e02df
Tree-SHA512: 048b4cd620a0415e1d9fe8597257ee4bc64656566e1d28a9bdd147d6d72dc87c3f34a3339fa9ab6acf42c388df7901fc4ee900ccaabc3de790ffad162b544c15
58a14795b89a6bd812e0b71cb8b3088b8ab55c11 test: passing -onlynet=onion with -onion=0/-noonion raises expected init error (Jon Atack)
7000f66d367123d1de303fc15ce2ce60df379c11 test: passing -onlynet=onion without -proxy/-onion raises expected init error (Jon Atack)
8332e6e4cf45455fea0bf1f7527256cdb7bb1e6d test: passing invalid -onion raises expected init error (Jon Atack)
d5edb087082a50e6f7d413c3b43fdf1e6a20d29b test: passing invalid -proxy raises expected init error (Jon Atack)
bd57dcbaf2b5e5f50833912c894a1f1239ceb25b test: hoist proxy out of 2 network loops in feature_proxy.py (Jon Atack)
afdf2de28296660fd0284453a241aece8494eea8 test: add CJDNS to LimitedAndReachable_Network unit tests (Jon Atack)
2b7a8180a94738c2fcb21232a2eca07a7b27656d net, init: assert each network reachability is true by default (Jon Atack)
Pull request description:
Adds missing network reachability test coverage and an assertion during init, noticed while reviewing #22834:
- assert during init that each network reachability is true by default
- add CJDNS to the `LimitedAndReachable_Network` unit tests
- hoist proxy out of two network loops in feature_proxy.py
- test that passing invalid `-proxy` raises expected init error
- test that passing invalid `-onion` raises expected init error
- test that passing `-onlynet=onion` without `-proxy` and `-onion` raises expected init error
- test that passing `-onlynet=onion` with `-onion=0` and with `-noonion` raises expected init error
ACKs for top commit:
vasild:
ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11
brunoerg:
ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11
dongcarl:
Code Review ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11
Tree-SHA512: bdee6dd0c12bb63591ce7c9321fe77b509ab1265123054e774adc38a187746dddafe1627cbe89e990bcc78b45e194bfef8dc782710d5b217e2e2106ab0158827
bc90b8d86916d43867762a391633664676550bd8 [move only] remove `is_wallet_compiled` checks (josibake)
0bfbf7fb2488753d795ffc1c63a8977e4fe4a3bc test: use MiniWallet in `interfaces_zmq` (josibake)
Pull request description:
While working on #24584 , `interface_zmq` started failing due to coin selection not running deterministically. The test doesn't actually need the wallet, so this PR migrates it to use MiniWallet
_Note for reviewers:_ the second commit moves large chunks of code out of an if block, so it may be helpful to review with something that ignores whitespace, e.g `git diff -w master`
ACKs for top commit:
vincenzopalazzo:
ACK bc90b8d869
Tree-SHA512: c618e23d00635d72dafdef28e68cbc88b9cc2030d4898fc5b7eac926fd621684c1958c075ed167192716b18308da5a0c1f1393396e31b99d0d3bde78b78fefc5
f44efc3e2c5664825d7bd071f9dc38b5b9111ae1 doc: update i2p.md with cjdns, improve local addresses section (Jon Atack)
3bf6f0cf2cb3a958e7cc346760009af50c2fa304 doc: update tor.md with cjdns and getnodeaddresses, fix tor grep, (Jon Atack)
ed15848475a10fcacaef4eeb38cd7f93ce038c47 doc: create initial doc/cjdns.md for cjdns how-to documentation (Jon Atack)
Pull request description:
and update and improve doc/tor.md and doc/i2p.md.
Adapted in part from the CJDNS description in https://github.com/bitcoin/bitcoin/pull/23077 and feedback by Vasil Dimov and from the CJDNS documentation and feedback by Caleb James DeLisle.
Targets backport to v23.x.
Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
ACKs for top commit:
vasild:
ACK f44efc3e2c5664825d7bd071f9dc38b5b9111ae1
lsilva01:
ACK f44efc3
Tree-SHA512: 7b7c69f76bc8a5705d324892f32bfe0eb21bcf048054748053eca167c65a2121f6332f40ac6ff98c955e6e8b53233c74c365d887c364ef1d5944f1c49675a6b4
fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46 Use CAmount for fee delta and modified fee (MarcoFalke)
fa8857c3f7863d5cfa9a9e62db7f95aad80ea48e Replace struct update_fee_delta with lambda (MarcoFalke)
Pull request description:
The same was done for another struct in e177fcab3831b6d259da5164cabedcc9e78f6957.
Also, change type of feeDelta from int64_t to CAmount.
ACKs for top commit:
hebasto:
re-ACK fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46
promag:
Code review ACK fa84a49526fcf76e98b0b2f4d4b00b34a8dddf46.
Tree-SHA512: 2b9ee449d348b0f685793a35c6dd3c57ed97fdf707a89495a0518bb332f407303b48723e667351e96f2b698e0a2ade27095517a3accd926d4ec85e58d6fd441f
b2813980b81034ff9b40bd45080fa67dea475d39 init: disallow reindex-chainstate when pruning (Martin Zumsande)
Pull request description:
The combination of `-reindex-chainstate` and `-prune` currently makes the node stuck in an endless loop:
- `LoadChainstate()` will wipe the existing chainstate (so we have no genesis block anymore). It won't clean up unusable block files by calling `CleanupBlockRevFiles()` as for full `-reindex`.
- `ThreadImport()` has [logic](91d12344b1/src/node/blockstorage.cpp (L855)) of reloading Genesis after reindexing. This is what makes full `-reindex` work with `-prune` but it's not executed for `-reindex-chainstate`.
- Since we still don't have a genesis block, init will wait for it forever in an endless loop ([code](91d12344b1/src/init.cpp (L1630-L1640))).
Fix this by disallowing `-reindex-chainstate` together with `-prune`. This is discouraged in the help for `-reindex-chainstate` anyway ("When in pruning mode or if blocks on disk might be corrupted, use full -reindex instead.") but wasn't enforced.
Fixes#24242
ACKs for top commit:
MarcoFalke:
cr ACK b2813980b81034ff9b40bd45080fa67dea475d39
Tree-SHA512: 7220842daaf9a4f972d82b13b81fdeac2833bf5e665c5b0f8eaf6a4bcd0725c8e97d19ec956ca4b730065a983475bb3a2732713d338f4caf8666ccbf63d4d988
7ac7198bbd086ad905d2a217fe64d2ebb54420ab doc: mention that BDB is for the legacy wallet in build-netbsd.md (fanquake)
Pull request description:
Re-order legacy and descriptor wallet section.
Add an additional configure example.
NetBSD version of #23446.
ACKs for top commit:
shaavan:
ACK 7ac7198bbd086ad905d2a217fe64d2ebb54420ab
Tree-SHA512: 5c8218424a6b12e9eee00b44dd93f9fe95fd9afa468563167feb255663720a84b55e75850985cfae3ca288a6a76e17c00ccce60b8180f92875eeaee2c9afa843
97af6527885c3b2dac183350f6095c98f8980e49 guix: Drop code for the unsupported `i686-linux-gnu` host (Hennadii Stepanov)
Pull request description:
Now GUIX build for the `i686-linux-gnu` host is broken, and [there are no plans to re-add it](https://github.com/bitcoin/bitcoin/pull/24448).
ACKs for top commit:
fanquake:
ACK 97af6527885c3b2dac183350f6095c98f8980e49
Tree-SHA512: 968181aff65e607a7c1a1b06ac7dfd79f6e2ce49b3c4c3828def020e925769fdbab1859d37ea924ded7632405b30539ac3ec81ac714cb9a01a2f7d5c93301dd9
999982b06ce1d1280e5ce48f9253c6c536f41a12 build: Add --enable-c++20 option (MarcoFalke)
fae679065e4ef0c6383bbdd1876aaed6c1e40104 Add CSerializedNetMsg::Copy() helper (MarcoFalke)
fabb7c4ba629ecdea80a23674e2659d3d391565f Make fs.h C++20 compliant (MarcoFalke)
fae2220f4e48934313389864d3d362f672627eb8 scheduler: Capture ‘this’ explicitly in lambda (MarcoFalke)
Pull request description:
This is for CI and devs only and doesn't change that C++17 is the standard we are currently using. The option `--enable-c++20` allows CI to check that the C++17 code in the repo is also valid C++20. (There are some cases where valid C++17 doesn't compile under C++20).
Also, it allows developers to easily play with C++20 in the codebase.
ACKs for top commit:
ryanofsky:
Code review ACK 999982b06ce1d1280e5ce48f9253c6c536f41a12. Since last review was rebased, and enum-conversion change was dropped, and CSerializedNetMsg copy workaround was added
fanquake:
utACK 999982b06ce1d1280e5ce48f9253c6c536f41a12
Tree-SHA512: afc95ba03ea2b937017fc8e2b1449379cd2b6f7093c430d2e344c665a00c51e402d6651cbcbd0be8118ea1e54c3a86e67d2021d19ba1d4da67168e9fcb6b6f83
faf37c217a408114224f91b7ada3fb6ff29b0c0a rpc: Exclude descriptor when address is excluded (MarcoFalke)
Pull request description:
I don't think output descriptors should be used to describe redeem scripts and witness scripts.
Fix this by excluding them when it doesn't make sense.
This should only affect the `decodepsbt` RPC.
Found by https://github.com/bitcoin/bitcoin/pull/23083
ACKs for top commit:
achow101:
ACK faf37c217a408114224f91b7ada3fb6ff29b0c0a
jonatack:
ACK faf37c217a408114224f91b7ada3fb6ff29b0c0a
Tree-SHA512: ebd581ad639e70080e26028723fed287caa3fa4d7b836936645020d6cd9b7586585d7113b043442c444a9dc90c23b93efd7f8b8a7d6cf5db1e42137b67c497c3
