-dnsseed determines whether we run ThreadDNSAddressSeed and potentially query
the DNS seeds for addresses. -forcednsseed tells the node to force querying the
DNS seeds even if we have sufficient addresses or current connections.
This commit disallows starting up with explicitly conflicting parameters.
a806647d260132a00cd633160040625c7dd17803 [validation] Always include merkle root in coinbase commitment (Dhruv Mehta)
189128c220190a588500b8e74ee7ae47671b9558 [validation] Set witness script flag with p2sh for blocks (Dhruv Mehta)
ac82b99db77ec843af82dcdf040dfdbc98c8ff26 [p2p] remove redundant NODE_WITNESS checks (Dhruv Mehta)
6f8b198b8256a6703a6f5e592dfa77fa024a7035 [p2p] remove unused segwitheight=-1 option (Dhruv Mehta)
eba5b1cd6460c98e75d0422bd394e12af7f11e4c [test] remove or move tests using `-segwitheight=-1` (Dhruv Mehta)
Pull request description:
Builds on #21009 and makes progress on remaining items in #17862
Removing `RewindBlockIndex()` in #21009 allows the following:
- removal of tests using `segwitheight=-1` in `p2p_segwit.py`.
- move `test_upgrade_after_activation()` out of `p2p_segwit.py` reducing runtime
- in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test.
- that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`.
- that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS`
ACKs for top commit:
mzumsande:
Code-Review ACK a806647d260132a00cd633160040625c7dd17803
laanwj:
Code review ACK a806647d260132a00cd633160040625c7dd17803, nice cleanup
jnewbery:
utACK a806647d260132a00cd633160040625c7dd17803
theStack:
ACK a806647d260132a00cd633160040625c7dd17803
Tree-SHA512: 73e1a69d1d7eca1f5c38558ec6672decd0b60b16c2ef6134df6f6af71bb159e6eea160f9bb5ab0eb6723c6632d29509811e29469d0d87abbe9b69a2890fbc73e
ceb7b35a39145717e2d9d356fd382bd1f95d2a5a refactor: move UpdateTip into CChainState (James O'Beirne)
4abf0779d6594e97222279110c328b75b5f3db7b refactor: no mempool arg to GetCoinsCacheSizeState (James O'Beirne)
46e3efd1e4ae2f058ecfffdaee7e882c4305eb35 refactor: move UpdateMempoolForReorg into CChainState (James O'Beirne)
617661703ac29e0744f21de74501d033fdc53ff6 validation: make CChainState::m_mempool optional (James O'Beirne)
Pull request description:
Make `CChainState::m_mempool` optional by making it a pointer instead of a reference. This will allow a simplification to assumeutxo semantics (see https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905) and help facilitate the `-nomempool` option.
ACKs for top commit:
jnewbery:
ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a
naumenkogs:
ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a
ryanofsky:
Code review ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a (just minor style and test tweaks since last review)
lsilva01:
Code review ACK and tested on Signet ACK ceb7b35a39
MarcoFalke:
review ACK ceb7b35a39145717e2d9d356fd382bd1f95d2a5a 😌
Tree-SHA512: cc445ad33439d5918cacf80a6354eea8f3d33bb7719573ed5b970fad1a0dab410bcd70be44c862b8aba1b71263b82d79876688c553e339362653dfb3d8ec81e6
Since we now have multiple chainstate objects, only one of them is active at any given
time. An active chainstate has a mempool, but there's no point to others having one.
This change will simplify proposed assumeutxo semantics. See the discussion here:
https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
4101ec9d2e05a35c35f587a28f1feee6cebcc61b doc: mention that we enforce port=0 in I2P (Vasil Dimov)
e0a2b390c144e123e2fc8a289fdff36815476964 addrman: reset I2P ports to 0 when loading from disk (Vasil Dimov)
41cda9d075ebcab1dbb950160ebe9d0ba7b5745e test: ensure I2P ports are handled as expected (Vasil Dimov)
4f432bd738c420512a86a51ab3e00323f396b89e net: do not connect to I2P hosts on port!=0 (Vasil Dimov)
1f096f091ebd88efb18154b8894a38122c39624f net: distinguish default port per network (Vasil Dimov)
aeac3bce3ead1f24ca782079ef0defa86fd8cb98 net: change I2P seeds' ports to 0 (Vasil Dimov)
38f900290cc3a839e99bef13474d35e1c02e6b0d net: change assumed I2P port to 0 (Vasil Dimov)
Pull request description:
_This is an alternative to https://github.com/bitcoin/bitcoin/pull/21514, inspired by https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-815049933. They are mutually exclusive. Just one of them should be merged._
Change assumed ports for I2P to 0 (instead of the default 8333) as this is closer to what actually happens underneath with SAM 3.1 (https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-812632520, https://github.com/bitcoin/bitcoin/pull/21514#issuecomment-816564719).
Don't connect to I2P peers with advertised port != 0 (we don't specify a port to our SAM 3.1 proxy and it always connects to port = 0).
Note, this change:
* Keeps I2P addresses with port != 0 in addrman and relays them to others via P2P gossip. There may be non-bitcoin-core-22.0 peers using SAM 3.2 and for them such addresses may be useful.
* Silently refuses to connect to I2P hosts with port != 0. This is ok for automatically chosen peers from addrman. Not so ok for peers provided via `-addnode` or `-connect` - a user who specifies `foo.b32.i2p:1234` (non zero port) may wonder why "nothing is happening".
Fixes#21389
ACKs for top commit:
laanwj:
Code review ACK 4101ec9d2e05a35c35f587a28f1feee6cebcc61b
jonatack:
re-ACK 4101ec9d2e05a35c35f587a28f1feee6cebcc61b per `git range-diff efff9c3 0b0ee03 4101ec9`, built with DDEBUG_ADDRMAN, did fairly extensive testing on mainnet both with and without a peers.dat / -dnsseeds=0 to test boostrapping.
Tree-SHA512: 0e3c019e1dc05e54f559275859d3450e0c735596d179e30b66811aad9d5b5fabe3dcc44571e8f7b99f9fe16453eee393d6e153454dd873b9ff14907d4e6354fe
2feec3ce3130961f98ceb030951d0e46d3b9096c net: don't bind on 0.0.0.0 if binds are restricted to Tor (Vasil Dimov)
Pull request description:
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.
Before this change, if no `-bind` is given then we would bind to
`0.0.0.0:8333` and to `127.0.0.1:8334` (incoming Tor) which is ok -
the user does not care to restrict the binding.
However, if only `-bind=addr:port=onion` is given (without ordinary
`-bind=`) then we would bind to `addr:port` _and_ to `0.0.0.0:8333` in
addition.
Change the above to not do the additional bind: if only
`-bind=addr:port=onion` is given (without ordinary `-bind=`) then bind
to `addr:port` (only) and consider incoming connections to that as Tor
and do not advertise it. I.e. a Tor-only node.
ACKs for top commit:
laanwj:
Code review ACK 2feec3ce3130961f98ceb030951d0e46d3b9096c
jonatack:
utACK 2feec3ce3130961f98ceb030951d0e46d3b9096c per `git diff a004833 2feec3c`
hebasto:
ACK 2feec3ce3130961f98ceb030951d0e46d3b9096c, tested on Linux Mint 20.1 (x86_64):
Tree-SHA512: a04483af601706da928958b92dc560f9cfcc78ab0bb9d74414636eed1c6f29ed538ce1fb5a17d41ed82c9c9a45ca94899d0966e7ef93da809c9bcdcdb1d1f040
* When accepting an I2P connection, assume the peer has port 0 instead
of the default 8333 (for mainnet). It is not being sent to us, so we
must assume something.
* When deriving our own I2P listen CService use port 0 instead of the
default 8333 (for mainnet). So that we later advertise it to peers
with port 0.
In the I2P protocol SAM 3.1 and older (we use 3.1) ports are not used,
so they are irrelevant. However in SAM 3.2 and newer ports are used and
from the point of view of SAM 3.2, a peer using SAM 3.1 seems to have
specified port=0.
The semantic of `-bind` is to restrict the binding only to some address.
If not specified, then the user does not care and we bind to `0.0.0.0`.
If specified then we should honor the restriction and bind only to the
specified address.
Before this change, if no `-bind` is given then we would bind to
`0.0.0.0:8333` and to `127.0.0.1:8334` (incoming Tor) which is ok -
the user does not care to restrict the binding.
However, if only `-bind=addr:port=onion` is given (without ordinary
`-bind=`) then we would bind to `addr:port` _and_ to `0.0.0.0:8333` in
addition.
Change the above to not do the additional bind: if only
`-bind=addr:port=onion` is given (without ordinary `-bind=`) then bind
to `addr:port` (only) and consider incoming connections to that as Tor
and do not advertise it. I.e. a Tor-only node.
Provides DeploymentEnabled, DeploymentActiveAt, and DeploymentActiveAfter
helpers for checking the status of buried deployments. Can be overloaded
so the same syntax works for non-buried deployments, allowing future
soft forks to be changed from signalled to buried deployments without
having to touch the implementation code.
Replaces IsWitnessEnabled and IsScriptWitnessEnabled.
fa0d9211ef87a682573aaae932c0c440acbcb8a8 refactor: Remove chainparams arg from CChainState member functions (MarcoFalke)
fa389471251f043ec25e7b01e59b37d3b921ce54 refactor: Remove ::Params() global from inside CChainState member functions (MarcoFalke)
Pull request description:
The `::Params()` global is verbose and confusing. Also it makes tests a bit harder to write because they'd have to mock a global.
Fix all issues by simply using a member variable that points to the right params.
(Can be reviewed with `--word-diff-regex=.`)
ACKs for top commit:
jnewbery:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8
kiminuo:
utACK fa0d9211
theStack:
ACK fa0d9211ef87a682573aaae932c0c440acbcb8a8 🍉
Tree-SHA512: 44676b19c9ed471ccb536331d3029bad192d7d50f394fd7b8527ec431452aeec8c4494164b9cf8e16e0123c4463b16be864366c6b599370032c17262625a0356
Save the banlist in `banlist.json` instead of `banlist.dat`.
This makes it possible to store Tor v3 entries in the banlist on disk
(and any other addresses that cannot be serialized in addrv1 format).
Only read `banlist.dat` if it exists and `banlist.json` does not
exist (first start after an upgrade).
Supersedes https://github.com/bitcoin/bitcoin/pull/20904
Resolves https://github.com/bitcoin/bitcoin/issues/19748
6f994882deafe62e97f0a889d8bdb8c96dcf913d validation: Farewell, global Chainstate! (Carl Dong)
972c5166ee685447a6d4bf5e501b07a0871fba85 qt/test: Reset chainman in ~ChainstateManager instead (Carl Dong)
6c3b5dc0c13c3ac8c6e86298f924abe99d8d6bd1 scripted-diff: tree-wide: Remove all review-only assertions (Carl Dong)
3e82abb8dd7e21ec918966105648be7ae077fd8c tree-wide: Remove stray review-only assertion (Carl Dong)
f323248aba5088c9630e5cdfe5ce980f21633fe8 qt/test: Use existing chainman in ::TestGUI (can be scripted-diff) (Carl Dong)
6c15de129cd645bf0547cb184003fae131b95b83 scripted-diff: wallet/test: Use existing chainman (Carl Dong)
ee0ab1e959e0e75e04d87fabae8334ad4656f3e5 fuzz: Initialize a TestingSetup for test_one_input (Carl Dong)
0d61634c066a7102d539e85e2b1a4ca15be9660a scripted-diff: test: Use existing chainman in unit tests (Carl Dong)
e197076219e986ede6cf924e0ea36bd723503b2d test: Pass in CoinsTip to ValidateCheckInputsForAllFlags (Carl Dong)
4d99b61014ba26eb1f3713df5528d2804edff165 test/miner_tests: Pass in chain tip to CreateBlockIndex (Carl Dong)
f0dd5e6bb4b16e69d35b648b7ef973a732229873 test/util: Use existing chainman in ::PrepareBlock (Carl Dong)
464c313e304cef04a82e14f736e3c44ed5604a4e init: Use existing chainman (Carl Dong)
Pull request description:
Based on: #21767
à la Mr. Sandman
```
Mr. Chainman, bring me a tip (bung, bung, bung, bung)
Make it the most work that I've ever seen (bung, bung, bung, bung)
Rewind old tip till we're at the fork point (bung, bung, bung, bung)
Then tell it that it's time to call Con-nectTip
Chainman, I'm so alone (bung, bung, bung, bung)
No local objects to call my own (bung, bung, bung, bung)
Please make sure I have a ref
Mr. Chainman, bring me a tip!
```
This is the last bundle in the #20158 series. Thanks everyone for their diligent review.
I would like to call attention to https://github.com/bitcoin/bitcoin/issues/21766, where a few leftover improvements were collated.
- Remove globals:
- `ChainstateManager g_chainman`
- `CChainState& ChainstateActive()`
- `CChain& ChainActive()`
- Remove all review-only assertions.
ACKs for top commit:
jamesob:
reACK 6f994882de based on the contents of
ariard:
Code Review ACK 6f99488.
jnewbery:
utACK 6f994882deafe62e97f0a889d8bdb8c96dcf913d
achow101:
Code Review ACK 6f994882deafe62e97f0a889d8bdb8c96dcf913d
ryanofsky:
Code review ACK 6f994882deafe62e97f0a889d8bdb8c96dcf913d.
Tree-SHA512: 4052ea79360cf0efd81ad0ee3f982e1d93aab1837dcec75f875a56ceda085de078bb3099a2137935d7cc2222004ad88da94b605ef5efef35cb6bc733725debe6
7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a index: refactor-only: Reuse CChain ref (Carl Dong)
db33cde80fff749c6adff9e91fca5f27f4bb6278 index: Add chainstate member to BaseIndex (Carl Dong)
f4a47a1febfa35ab077f2a841fe31a8cd9618250 bench: Use existing chainman in AssembleBlock (Carl Dong)
91226eb91769aad5a63bc671595e1353a2b2247a bench: Use existing NodeContext in DuplicateInputs (Carl Dong)
e6b4aa6eb53dc555ecab2922af35e7a2572faf4f miner: Pass in chainman to RegenerateCommitments (Carl Dong)
9ecade14252ad1972f668d2d2e4ef44fdfcb944a rest: Add GetChainman function and use it (Carl Dong)
fc1c282845f6b8436d1ea4c68eb3511034c29bea rpc/blockchain: Use existing blockman in gettxoutsetinfo (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
The first 2 commits are fixups addressing review for the last bundle: #21391
NEW note:
1. I have opened #21766 which keeps track of potential improvements where the flaws already existed before the de-globalization work, please post on that issue about these improvements, thanks!
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
jarolrod:
ACK 7a799c9
ariard:
Code Review ACK 7a799c9
fjahr:
re-ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a
MarcoFalke:
review ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a 🌠
ryanofsky:
Code review ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a. Basically no change since last review except fixed rebase conflicts and a new comment about REST Ensure()
jamesob:
conditional ACK 7a799c9c2b8652e780d1fd5e1bf7d05b026c1c1a ([`jamesob/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai`](https://github.com/jamesob/bitcoin/tree/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai))
Tree-SHA512: 531c00ddcb318817457db2812d9a9d930bc664e58e6f7f1c746350732b031dd624270bfa6b9f49d8056aeb6321d973f0e38e4ff914acd6768edd8602c017d10e
a7a43e8fe85f6247c35d7ff99f36448574f3e34a Factor feefilter logic out (amadeuszpawlik)
c0385f10a133d5d8a4c296e7b7a6d75c9c4eec12 Remove -feefilter option (amadeuszpawlik)
Pull request description:
net: Remove -feefilter option, as it is debug only and isn't used in any tests. Checking this option for every peer on every iteration of the message handler is unnecessary, as described in #21545.
refactor: Move feefilter logic out into a separate `MaybeSendFeefilter(...)` function to improve readability of the already long `SendMessages(...)`. fixes #21545
The configuration option `-feefilter` has been added in 9e072a6e66efbda7d39bf61eded21d2b324323be: _"Implement "feefilter" P2P message"_
According to the [BIP133](https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki), turning the fee filter off was ment for:
> [...] a node [...] using prioritisetransaction to accept transactions whose actual fee rates might fall below the node's mempool min fee [in order to] disable the fee filter to make sure it is exposed to all possible txid's
`-feefilter` was subsequently set as debug only in #8150, with the motivation that the help message was too difficult to translate.
ACKs for top commit:
jnewbery:
Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a
promag:
Code review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a.
MarcoFalke:
review ACK a7a43e8fe85f6247c35d7ff99f36448574f3e34a 🦁
Tree-SHA512: 8ef9a2f255597c0279d3047dcc968fd30fb7402e981b69206d08eed452c705ed568c24e646e98d06eac118eddd09205b584f45611d1c874abf38f48b08b67630
Feefilter option is debug only and it isn't used in any tests, it's wasteful
to check this option for every peer on every iteration of the message handler
loop. refs #21545
792be53d3e9e366b9f6aeee7a1eeb912fa28062e refactor: Replace std::bind with lambdas (Hennadii Stepanov)
a508f718f3e087c96a306399582a85df2e1d53ae refactor: Use appropriate thread constructor (Hennadii Stepanov)
30e44482152488a78f2c495798a75e6f553dc0c8 refactor: Make TraceThread a non-template free function (Hennadii Stepanov)
Pull request description:
This PR does not change behavior.
Its goal is to improve readability and maintainability of the code.
ACKs for top commit:
jnewbery:
utACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
jonatack:
tACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
MarcoFalke:
cr ACK 792be53d3e9e366b9f6aeee7a1eeb912fa28062e
Tree-SHA512: a03142f04f370f6bc02bd3ddfa870819b51740fcd028772241d68c84087f95a2d78207cbd5edb3f7c636fcf2d76192d9c59873f8f0af451d3b05c0cf9cf234df
fae196147bae11202c0d54543dc12ba5d92ab0cc doc: Clarify that feerates are per virtual size (MarcoFalke)
fa83e95ac6f318caa38016a08fa4e402c3b05833 scripted-diff: Clarify that feerates are per virtual size (MarcoFalke)
Pull request description:
By implementing segwit, it is already clear that all feerates in Bitcoin Core are denoted in (amount/virtual size). Though, there is inconsistency, as some places use kvB, some use kB. Thus, replace all with "kvB".
See also commit 6da3afbaee5809ebf6d88efaa3958c505c2d71c7, which did the replacement for wallet RPCs.
ACKs for top commit:
ryanofsky:
Code review ACK fae196147bae11202c0d54543dc12ba5d92ab0cc. Checked instances where units were being added in the second commit and they all looked right.
Tree-SHA512: ab70d13cde7d55c1ac931bddc2b45aa218fc75ef46cb6ea9e5a30b1d4dbf27889c2b6357299a6c5427912443a46ec3592a4809dae335e03162bd2120a0f7f8ad
d66f283ac07edce432b964f7f814631f5a5bc33b scripted-diff: Replace three dots with ellipsis in the UI strings (Hennadii Stepanov)
Pull request description:
This PR is split from #21463.
The change was suggested on [Transifex.com](https://www.transifex.com/bitcoin/bitcoin/), and it does not touch `LogPrint` and `LogPrintf` calls.
The only comment on #21463 [was](9030e4b5a6 (r597220100)):
> Mind that these messages also end up in the log. In principle the log is already UTF-8 (as are all strings and text in bitcoind). But, just noting, that it might make browsing the log a less pleasant experience on systems with misconfigured locale like some BSDs by default.
ACKs for top commit:
laanwj:
ACK d66f283ac07edce432b964f7f814631f5a5bc33b
Tree-SHA512: 5ab1cb3160f3f996f1ad7d7486662da3eb7f06a857f4a1874963ce10caed5b86b0ad6151b1b9ebeb2b8aa5f0c85efad3b768ea9cafe5db86f78f88912b756d1e
b4fcbcfb49461b96bc72fb64d6152de7c5ce00de doc: update -maxconnections config option help (Jon Atack)
79685a8992ad302833b506cc6d03aab1cc127de0 doc: update -addnode config option help (Jon Atack)
2896c6c4cc6d382d8369c037e274c08dd8e32c69 doc: update addnode rpc help (Jon Atack)
Pull request description:
Since #9319 proposed by Gregory Maxwell and released in v0.14, peers manually added through the `-addnode` config option or using the `addnode` RPC have their own separate limit of 8 connections that does not compete with other inbound or outbound connection usage and is not subject to the limitation imposed by the `-maxconnections` option.
This PR updates the `-addnode` and `-maxconnections` config options and the `addnode` RPC help docs with this information.
`-addnode` config option help
```
$ bitcoind -h | grep -A5 addnode=
-addnode=<ip>
Add a node to connect to and attempt to keep the connection open (see
the addnode RPC help for more info). This option can be specified
multiple times to add multiple nodes; connections are limited to
8 at a time and are counted separately from the -maxconnections
limit.
$ bitcoind -h | grep -A3 maxconnections=
-maxconnections=<n>
Maintain at most <n> connections to peers (default: 125). This limit
does not apply to connections manually added via -addnode or the
addnode RPC, which have a separate limit of 8.
```
`addnode` rpc help
```
$ bitcoin-cli help addnode
addnode "node" "command"
Attempts to add or remove a node from the addnode list.
Or try a connection to a node once.
Nodes added using addnode (or -connect) are protected from DoS disconnection and are not required to be
full nodes/support SegWit as other outbound peers are (though such peers will not be synced from).
Addnode connections are limited to 8 at a time and are counted separately from the -maxconnections limit.
```
ACKs for top commit:
prayank23:
ACK b4fcbcfb49
jarolrod:
ACK b4fcbcfb49461b96bc72fb64d6152de7c5ce00de
Tree-SHA512: b6d69baa6cbf6d53f91bac5b39b549d49db6c95f92ea1bdd3588a6432794a25ac2c8b3c89e2c72bb9097e61f2717c8b5ecc404745d5992b88e523db03200898f
5f96d7d22d8e05876c6fc014e70488699950fe38 rpc: gettxoutsetinfo rejects hash_serialized_2 for specific height (Fabian Jahr)
23fe50436be641d7417152adc683192649ba206a test: Add test for coinstatsindex behavior in reorgs (Fabian Jahr)
90c966b0f3cfbd6bce5883f46d8527c6853a86a2 rpc: Allow gettxoutsetinfo and getblockstats for stale blocks (Fabian Jahr)
b9362392aef2689bc106c20925859ede555d082b index, rpc: Add use_index option for gettxoutsetinfo (Fabian Jahr)
bb7788b121a30489bc81a1f46dde6a9b19ae4ec1 test: Test coinstatsindex robustness across restarts (Fabian Jahr)
e0938c29099635150014ffc9bb0cafa8049ec55a test: Add tests for block_info in gettxoutsetinfo (Fabian Jahr)
2501576eccb08af80471c7b7b843b189ad6758c0 rpc, index: Add verbose amounts tracking to Coinstats index (Fabian Jahr)
655d929836a71af23d2035d2e2e99ad8b8c340c3 test: add coinstatsindex getindexinfo coverage, improve current tests (Jon Atack)
ca01bb8d689f93e1c7669b0ba7a4994c0206dabd rpc: Add Coinstats index to getindexinfo (Fabian Jahr)
57a026c30fef3138bb8db46e6865acb9dc2674f8 test: Add unit test for Coinstats index (Fabian Jahr)
6a4c0c09ab4d073a26c3c4a02783d5dcd88f6eef test: Add functional test for Coinstats index (Fabian Jahr)
3f166ecc125fce6ccd995687fa16572090a5d099 rpc: gettxoutsetinfo can be requested for specific blockheights (Fabian Jahr)
3c914d58ff323255b32e717d0ce28209ec0abdaa index: Coinstats index can be activated with command line flag (Fabian Jahr)
dd58a4de21469d6d848ae309edc47f558628221d index: Add Coinstats index (Fabian Jahr)
a8a46c4b3cfda4b95c92a36f8cebd3606377e57d refactor: Simplify ApplyStats and ApplyHash (Fabian Jahr)
9c8a265fd21a87228c18a1661df99fedc1866baf refactor: Pass hash_type to CoinsStats in stats object (Fabian Jahr)
2e2648a9021dfbb6e17dfa81472f057dacbc34e0 crypto: Make MuHash Remove method efficient (Fabian Jahr)
Pull request description:
This is part of the coinstats index project tracked in #18000
While the review of the new UTXO set hash algorithm (MuHash) takes longer recently #19328 was merged which added the possibility to run `gettxoutsetinfo` with a specific hash type. As the first type it added `hash_type=none` which skips the hashing of the UTXO set altogether. This alone did not make `gettxoutsetinfo` much faster but it allows the use of an index for the remaining coin statistics even before a new hashing algorithm has been added. Credit to Sjors for the idea to take this intermediate step.
Features summary:
- Users can start their node with the option `-coinstatsindex` which syncs the index in the background
- After the index is synced the user can use `gettxoutsetinfo` with `hash_type=none` or `hash_type=muhash` and will get the response instantly out of the index
- The user can specify a height or block hash when calling `gettxoutsetinfo` to see coin statistics at a specific block height
ACKs for top commit:
Sjors:
re-tACK 5f96d7d22d8e05876c6fc014e70488699950fe38
jonatack:
Code review re-ACK 5f96d7d22d8e05876c6fc014e70488699950fe38 per `git range-diff 13d27b4 07201d3 5f96d7d`
promag:
Tested ACK 5f96d7d22d8e05876c6fc014e70488699950fe38. Light code review ACK 5f96d7d22d8e05876c6fc014e70488699950fe38.
Tree-SHA512: cbca78bee8e9605c19da4fbcd184625fb280200718396c694a56c7daab6f44ad23ca9fb5456d09f245d8b8d9659fdc2b3f3ce5e953c1c6cf4003dbc74c0463c2
844ad0eccaa1dbfefc30d19804d95d67c3d5f06d doc: IsSnapshotActive (James O'Beirne)
9b604c0207b526c008617cdca210f35db5e344db validation: prepare VerifyDB for assumeutxo (James O'Beirne)
7901647d722bcc5b0554ad73b14481cbe73608e4 refactor: rename active_chainstate in VerifyDB (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606)
---
~~Pretty cut and dry; parameterizes `CVerifyDB` methods so that we can run the verify procedure on multiple chainstates.~~
Two minor tweaks to ensure that `VerifyDB` can be run on multiple chainstates and a corresponding rename.
ACKs for top commit:
fjahr:
Code review re-ACK 844ad0eccaa1dbfefc30d19804d95d67c3d5f06d
MarcoFalke:
review ACK 844ad0eccaa1dbfefc30d19804d95d67c3d5f06d 🐥
Tree-SHA512: 26a398cf4dabc1aa0850743921dba0452b4813848a3c777586dc981716737e98e17b8110254a5c41af95dd236e0c00dc8b4eee891d69bef825a5e1911fc499d0
d831e711cab83c70bf2ded62fe33f484844e73dd [validation] RewindBlockIndex no longer needed (Dhruv Mehta)
Pull request description:
Closes#17862
Context from [original comment](https://github.com/bitcoin/bitcoin/issues/17862#issuecomment-744285188) (minor edits):
`RewindBlockIndex()` is a mechanism to allow nodes to be upgraded after segwit activation, while still keeping their chainstate/datadir in a consistent state. It works as follows:
- A pre-segwit (i.e. v0.13.0 or older) node is running.
- Segwit activates. The pre-segwit node remains sync'ed to the tip, but is not enforcing the new segwit rules.
- The user upgrades the node to a segwit-aware version (v0.13.1 or newer).
- On startup, in `AppInitMain()`, `RewindBlockIndex()` is called. This walks the chain backwards from the tip, disconnecting and erasing blocks that from after segwit activation that weren't validated with segwit rules.
- those blocks are then redownloaded (with witness data) and validated with segwit rules.
This logic probably isn't required any more since:
- Segwit activated at height 481824, when the block chain was 130GB and the total number of txs was 250 million. Today, we're at height 667704, the blockchain is over 315GB and the total number of txs is over 600 million. Even if 20% of that added data is witness data (a high estimate), then around 150GB of transactions would need to be rewound to get back to segwit activation height. It'd probably be faster to simply validate from genesis, especially since we won't be validating any scripts before the assumevalid block. It's also unclear whether rewinding 150GB of transactions would even work. It's certainly never been tested.
- Bitcoin Core v0.13 is hardly used any more. https://luke.dashjr.org/programs/bitcoin/files/charts/software.html shows less than 50 nodes running it. The software was EOL on Aug 1st 2018. It's very unlikely that anyone is running 0.13 and will want to upgrade to 0.22.
This PR introduces `NeedsRedownload()` which merely checks for insufficiently validated segwit blocks and requests that the user restarts the node with `-reindex`. Reindexing the block files upon restart will make the node rebuild chain state and block index from the `blk*.dat` files on disk. The node won't be able to index the blocks with `BLOCK_OPT_WITNESS`, so they will be missing from the chain and be re-downloaded, with witness data.
Removing this code allows the following (done in follow-up #21090):
- removal of tests using `segwitheight=-1` in `p2p_segwit.py`.
- in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test.
- that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`.
- that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS`
ACKs for top commit:
jnewbery:
utACK d831e711cab83c70bf2ded62fe33f484844e73dd
jamesob:
ACK d831e711ca
laanwj:
Cursory code review ACK d831e711cab83c70bf2ded62fe33f484844e73dd. Agree with the direction of the change, thanks for simplifying the logic here.
glozow:
utACK d831e711cab83c70bf2ded62fe33f484844e73dd
Tree-SHA512: 3eddf5121ccd081ad7f15a5c6478ef867083edc8ba0bf1ee759e87bc070ee3d2f0698a3feba8db8dc087987c8452887b6f72cff05b3e178f41cb10a515fb8053
785f9cc46a43661c63a5ec56a9e82f4ce9d42f44 refactor: init: mark fReset const (James O'Beirne)
Pull request description:
Small thing, but hey - it doesn't change.
ACKs for top commit:
theStack:
Code-review ACK 785f9cc46a43661c63a5ec56a9e82f4ce9d42f44
Tree-SHA512: 3cb8d7037f517162f6315d561accc4932b0f1e340162c3283871433f2e355d57b3740c9d2e953ce33fbfa3b277c8437f91955fb70331b3fe9c8e6a8589dc2b49
8c8237a4a10feb2ac9ce46f67b5d14bf879b670f net, refactor: Fix style in CConnman::StopNodes (Hennadii Stepanov)
229ac1892d807a1eea5a7c24ae0fe27dc913b1bd net: Combine two loops into one, and update comments (Hennadii Stepanov)
a3d090d1103cd6c25daf07afdf4e65febca6d3f7 net: Restrict period when cs_vNodes mutex is locked (Hennadii Stepanov)
Pull request description:
This PR restricts the period when the `cs_vNodes` mutex is locked, prevents the only case when `cs_vNodes` could be locked before the `::cs_main`.
This change makes the explicit locking of recursive mutexes in the explicit order redundant.
ACKs for top commit:
jnewbery:
utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f
vasild:
ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f
ajtowns:
utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f - logic seems sound
MarcoFalke:
review ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f 👢
Tree-SHA512: a8277924339622b188b12d260a100adf5d82781634cf974320cf6007341f946a7ff40351137c2f5369aed0d318f38aac2d32965c9b619432440d722a4e78bb73
615965cfd1ef1e0627d69970d99bdfedb9176833 Move common package version code to init/common (Russell Yanofsky)
5bed2ab42c4f1a820468f7005ce62e39001f6611 Move common logging start code to init/common (Russell Yanofsky)
1fb7fcfa52569a652d3ea55c210b725e60b7d86f Move common logging GetArgs code to init/common (Russell Yanofsky)
90469c16906ab451bb1250df5e51563870a7ef3b Move common logging AddArg code to init/common (Russell Yanofsky)
387c4cf5887bfdaf1606e1b287d901e4c449514f Move common sanity check code to init/common (Russell Yanofsky)
a67b54855b294802d52f09fa60d3f63550cbada7 Move common global init code to init/common (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
This change is move-only and can be easily reviewed with `--color-moved=dimmed_zebra`. The moves are needed to avoid duplicating common init code between different binaries (`bitcoin-node`, `bitcoin-wallet`, etc) in #10102. In #10102, each binary has it's own init file (`src/init/bitcoin-node.cpp`, `src/init/bitcoin-wallet.cpp`) so this PR moves the common code to `src/init/common.cpp`.
ACKs for top commit:
MarcoFalke:
review ACK 615965cfd1ef1e0627d69970d99bdfedb9176833 🖱
practicalswift:
cr ACK 615965cfd1ef1e0627d69970d99bdfedb9176833: dimmed zebra looks correct
Tree-SHA512: 859e1d86aee17eb50a49d806cf62d30d12f6b15018e41c096da41d7e535a9d2d088481cb340fee59e6c68e512a74b61c7146f2683465f553dc4953bf32f2a7b4
