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
84934bf70e11fe4cda1cfda60113a54895d4fdd5 multiprocess: Add echoipc RPC method and test (Russell Yanofsky)
7d76cf667eff512043a28d4407cc89f58796c42b multiprocess: Add comments and documentation (Russell Yanofsky)
ddf7ecc8dfc64cf121099fb047e1ac871de94f4c multiprocess: Add bitcoin-node process spawning support (Russell Yanofsky)
10afdf0280fa93bfffb0a7665c60dc155cd84514 multiprocess: Add Ipc interface implementation (Russell Yanofsky)
745c9cebd50fea1664efef571dc1ee1bddc96102 multiprocess: Add Ipc and Init interface definitions (Russell Yanofsky)
5d62d7f6cd48bbc4e9f37ecc369f38d5e1e0036c Update libmultiprocess library (Russell Yanofsky)
Pull request description:
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).
---
This PR adds basic process spawning and IPC method call support to `bitcoin-node` executables built with `--enable-multiprocess`[*].
These changes are used in https://github.com/bitcoin/bitcoin/pull/10102 to let node, gui, and wallet functionality run in different processes, and extended in https://github.com/bitcoin/bitcoin/pull/19460 and https://github.com/bitcoin/bitcoin/pull/19461 after that to allow gui and wallet processes to be started and stopped independently and connect to the node over a socket.
These changes can also be used to implement new functionality outside the `bitcoin-node` process like external indexes or pluggable transports (https://github.com/bitcoin/bitcoin/pull/18988). The `Ipc::spawnProcess` and `Ipc::serveProcess` methods added here are entry points for spawning a child process and serving a parent process, and being able to make bidirectional, multithreaded method calls between the processes. A simple example of this is implemented in commit "Add echoipc RPC method and test."
Changes in this PR aside from the echo test were originally part of #10102, but have been split and moved here for easier review, and so they can be used for other applications like external plugins.
Additional notes about this PR can be found at https://bitcoincore.reviews/19160
[*] Note: the `--enable-multiprocess` feature is still experimental, and not enabled by default, and not yet supported on windows. More information can be found in [doc/multiprocess.md](https://github.com/bitcoin/bitcoin/blob/master/doc/multiprocess.md)
ACKs for top commit:
fjahr:
re-ACK 84934bf70e11fe4cda1cfda60113a54895d4fdd5
ariard:
ACK 84934bf. Changes since last ACK fixes the silent merge conflict about `EnsureAnyNodeContext()`. Rebuilt and checked again debug command `echoipc`.
Tree-SHA512: 52a948b5e18a26d7d7a09b83003eaae9b1ed2981978c36c959fe9a55abf70ae6a627c4ff913a3428be17400a3dace30c58b5057fa75c319662c3be98f19810c6
However, keep a declaration in validation to make it possible to move
smaller chunks to blockstorage without breaking compilation.
Also, expose AbortNode in the header.
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
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
This value is no longer used and is instead specified statically
in chainparams. This change means that previously generated
snapshots will no longer be usable.
a732ee353c1922a1f9ca082775884d190893e0e9 [test] Add tests for addr relay in -blocksonly mode (Amiti Uttarwar)
a6694eaed8f86a6ad79689bbcbfdbed4d73327c9 [test] Add address relay tests involving outbound peers (Martin Zumsande)
8188b77c17d78bf608cb41978db4d0f46e91f7d8 [test] Add tests for getaddr behavior (Martin Zumsande)
d2dbfe6ff1806e5248399b28a7a1a22dab726c40 [test] Extract sending an addr message into a helper (Amiti Uttarwar)
c9919433994924931757aa7ec278c50744ede35d [test] Refactor the addr relay test to prepare for new tests (Amiti Uttarwar)
Pull request description:
This extends the functional test `p2p_addr_relay.py`.
It adds test coverage for address relay involving outbound peers, tests for both outgoing and incoming `GETADDR` requests and tests for `-blocksonly` mode.
The initial refactors and some of the new tests were taken from Amiti Uttarwar's PR #21528 - they are general test improvements not directly tied to the change proposed there.
ACKs for top commit:
amitiuttarwar:
re-ACK a732ee353c, small diff based on code review
MarcoFalke:
Concept ACK a732ee353c1922a1f9ca082775884d190893e0e9 🌊
Tree-SHA512: e80d52683808ddd6b948a5134239f002f3fecf61b60e187877b07be6251721fde847104e495c75a1a5133a09c0b41a9255a0bec82932c0b304b516fa89bce33e
5f438d66c1fbc0e524d12fef233f2ed2952e6f17 refactor, qt: Simplify SendCoinsDialog::updateCoinControlState (João Barbosa)
Pull request description:
This PR doesn't change behaviour, removes the coin control argument from `updateCoinControlState` since it's a class member.
ACKs for top commit:
hebasto:
ACK 5f438d66c1fbc0e524d12fef233f2ed2952e6f17, I have reviewed the code and it looks OK, I agree it can be merged.
jonatack:
Code review ACK 5f438d66c1fbc0e524d12fef233f2ed2952e6f17
kristapsk:
utACK 5f438d66c1fbc0e524d12fef233f2ed2952e6f17. Code looks correct.
Tree-SHA512: 14abaa3d561f8c8854fed989b6aca886dcca42135880bac76070043f61c0042ec8967f2b83e50bbbb82050ef0f074209e97fa300cb4dc51ee182316e0846506d
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
Add simple interfaces::Echo IPC interface with one method that just takes and
returns a string, to test multiprocess framework and provide an example of how
it can be used to spawn and call between processes.
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
The default clearHistory=true argument is passed in the RPCConsole ctor
only. This is needless, as the history and historyPtr members are
initialized properly.
