diff options
author | W. J. van der Laan <laanwj@protonmail.com> | 2021-04-27 10:03:41 +0200 |
---|---|---|
committer | W. J. van der Laan <laanwj@protonmail.com> | 2021-04-27 10:14:52 +0200 |
commit | 19a56d1519fb493c3e1bd5cad55360b6b80fa52b (patch) | |
tree | 8b7d0039f25294f991464a6abb6c421e9344c377 /src/validation.h | |
parent | f8f55522c4697b77b1f739a2ed0c483396d62154 (diff) | |
parent | d831e711cab83c70bf2ded62fe33f484844e73dd (diff) |
Merge bitcoin/bitcoin#21009: Remove RewindBlockIndex logic
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 https://github.com/bitcoin/bitcoin/pull/21009/commits/d831e711cab83c70bf2ded62fe33f484844e73dd
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
Diffstat (limited to 'src/validation.h')
-rw-r--r-- | src/validation.h | 7 |
1 files changed, 3 insertions, 4 deletions
diff --git a/src/validation.h b/src/validation.h index de121ab46a..9ae2838ead 100644 --- a/src/validation.h +++ b/src/validation.h @@ -713,7 +713,9 @@ public: /** Replay blocks that aren't fully applied to the database. */ bool ReplayBlocks(const CChainParams& params); - bool RewindBlockIndex(const CChainParams& params) LOCKS_EXCLUDED(cs_main); + + /** Whether the chain state needs to be redownloaded due to lack of witness data */ + [[nodiscard]] bool NeedsRedownload(const CChainParams& params) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */ bool LoadGenesisBlock(const CChainParams& chainparams); @@ -760,9 +762,6 @@ private: bool RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& inputs, const CChainParams& params) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - //! Mark a block as not having block data - void EraseBlockData(CBlockIndex* index) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - void CheckForkWarningConditions() EXCLUSIVE_LOCKS_REQUIRED(cs_main); void InvalidChainFound(CBlockIndex* pindexNew) EXCLUSIVE_LOCKS_REQUIRED(cs_main); |