doc: release process chainparams updates

jonatack commented at 11:21 pm on February 22, 2022: member

Release process updates, fixes and clarifications regarding updating the chainparams:

add missing references to signet
specify specify blockchain/chainstate units, reduce repetition
exclude huge files for m_assumed_blockchain_size on mainnet
rewrite overhead note to be more actionable
reorganize the chainparams section to reduce repetition
specify which chains need to be updated
use 4096 blocks and getbestblockhash for getchaintxstats
clarify how to update defaultAssumeValid and nMinimumChainWork

DrahtBot added the label Docs on Feb 23, 2022

in doc/release-process.md:314 in 7e0d5ca3af outdated

317+- For `signet` -> `/signet/chainstate`
318 
319 Notes:
320 - When taking the size for `m_assumed_blockchain_size`, there's no need to exclude the `/chainstate` directory since it's a guideline value and an overhead will be added anyway.
321-- The expected overhead for growth may change over time, so it may not be the same value as last release; pay attention to that when changing the variables.
322+- The expected overhead for growth may change over time, so it may not be the same value as the previous release; pay attention to that when updating the variables.

laanwj commented at 12:50 pm on February 23, 2022:

BTW. this sentence (not introduced here) is completely un-actionable for someone executing the steps. Pay attention to what? It needs to be more specific, or maybe just left out. If the % overhead changes, the document should be updated.

jonatack commented at 8:55 pm on February 23, 2022:

Reworked to be more specific and actionable.

jonatack commented at 8:55 pm on February 23, 2022:

(Good point.)

in doc/release-process.md:31 in 7e0d5ca3af outdated

27@@ -28,8 +28,8 @@ Release Process
28 #### Before branch-off
29 
30 * Update hardcoded [seeds](/contrib/seeds/README.md), see [this pull request](https://github.com/bitcoin/bitcoin/pull/7415) for an example.
31-* Update [`src/chainparams.cpp`](/src/chainparams.cpp) m_assumed_blockchain_size and m_assumed_chain_state_size with the current size plus some overhead (see [this](#how-to-calculate-assumed-blockchain-and-chain-state-size) for information on how to calculate them).
32-* Update [`src/chainparams.cpp`](/src/chainparams.cpp) chainTxData with statistics about the transaction count and rate. Use the output of the `getchaintxstats` RPC, see
33+* Update [`src/chainparams.cpp`](/src/chainparams.cpp) m_assumed_blockchain_size and m_assumed_chain_state_size for mainnet, testnet, and signet with the current size plus some overhead (see [this](#how-to-calculate-assumed-blockchain-and-chain-state-size) for information on how to calculate them).

laanwj commented at 12:51 pm on February 23, 2022:

I’ve also thought about nesting the three concerning chainparam under one item somehow. There’s a lot of repetition.

jonatack commented at 8:56 pm on February 23, 2022:

Good point, reorganized.

jonatack force-pushed on Feb 23, 2022

jonatack commented at 8:59 pm on February 23, 2022: member

Updated per @laanwj feedback (good ideas, thanks). Edit: repushed with a couple edits after re-review.

jonatack force-pushed on Feb 24, 2022

unknown approved

unknown commented at 12:34 pm on February 24, 2022: none

ACK https://github.com/bitcoin/bitcoin/pull/24424/commits/bba0655c6fd6908edc1a43146ec52fbf95124a68

Updates improve documentation and I could not find any issues.

laanwj commented at 2:40 pm on February 24, 2022: member

Other things to add maybe:

Use 4096 blocks for getchaintxstats #24418 (review)
Try to use the same block for getchaintxstats and getblockheader (e.g. for the chainTxData and nMinimumChainWork/defaultAssumeValid steps) #24418 (review)

Sjors commented at 3:03 pm on February 24, 2022: member

It might also be worth writing a script to fetch these values based on a given height, both for reviewers and the maintainer.

fanquake commented at 1:29 pm on February 25, 2022: member

Some of the commits here can be dropped. i.e 4b5b011c1d0eba8a432e067c28278f22c18689ff adds a number of for mainnet, testnet, and signet, but then they are just deleted in 5099f23b7b29e869af463fa53a3442be011bf039.

jonatack commented at 1:40 pm on February 25, 2022: member

Some of the commits here can be dropped. i.e 4b5b011c1d0eba8a432e067c28278f22c18689ff adds a number of for mainnet, testnet, and signet, but then they are just deleted in 5099f23b7b29e869af463fa53a3442be011bf039.

Yes, the commit specifying the chains can be moved after the reorganization to reduce the diff. I’m reworking to incorporate the latest feedback here and the other items I’ve noticed. Edit: moved after.

laanwj commented at 11:44 am on February 28, 2022: member

It might also be worth writing a script to fetch these values based on a given height, both for reviewers and the maintainer.

Yes, automation would be even better. There’s quite some scope for reducing human error-proneness and pitfalls here. That said, documenting what we really want to be done is the first step.

in doc/release-process.md:311 in bba0655c6f outdated

302@@ -300,15 +303,15 @@ cat "$VERSION"/*/all.SHA256SUMS.asc > SHA256SUMS.asc
303 Both variables are used as a guideline for how much space the user needs on their drive in total, not just strictly for the blockchain.
304 Note that all values should be taken from a **fully synced** node and have an overhead of 5-10% added on top of its base value.
305 
306-To calculate `m_assumed_blockchain_size`:
307-- For `mainnet` -> Take the size of the data directory, excluding `/regtest` and `/testnet3` directories.
308-- For `testnet` -> Take the size of the `/testnet3` directory.
309+To calculate `m_assumed_blockchain_size`, take the size in GiB of these directories:
310+- For `mainnet` -> the data directory, excluding the `/regtest`, `/signet`, and `/testnet3` directories
311+- For `testnet` -> `/testnet3`

laanwj commented at 11:49 am on February 28, 2022:

We also want to exclude overly large files in the root directory of the network, e.g. a huge debug.log as tripped up in #24418 :smile: I’m not sure what is a good calculation, but maybe a simple reminder would do.

jonatack commented at 4:54 pm on February 28, 2022:

Added in reminder in 39f85b4

jonatack force-pushed on Feb 28, 2022

jonatack commented at 5:01 pm on February 28, 2022: member

Other things to add maybe:

* Use 4096 blocks for `getchaintxstats` [Chainparams update for 23.x [#24418](/bitcoin-bitcoin/24418/) (comment)](https://github.com/bitcoin/bitcoin/pull/24418#discussion_r812988911)

* Try to use the same block for `getchaintxstats` and `getblockheader` (e.g. for the chainTxData and nMinimumChainWork/defaultAssumeValid steps) [Chainparams update for 23.x [#24418](/bitcoin-bitcoin/24418/) (comment)](https://github.com/bitcoin/bitcoin/pull/24418#discussion_r813919147)

Thanks, I’ve attempted to address these along with other how-to clarifications. I wasn’t sure about #24418 (review), though.

unknown approved

unknown commented at 4:40 pm on March 6, 2022: none

reACK https://github.com/bitcoin/bitcoin/pull/24424/commits/9560757adc90938b54148e3921aacfaa4aa51be6

in doc/release-process.md:311 in 9560757adc outdated

305@@ -300,15 +306,15 @@ cat "$VERSION"/*/all.SHA256SUMS.asc > SHA256SUMS.asc
306 Both variables are used as a guideline for how much space the user needs on their drive in total, not just strictly for the blockchain.
307 Note that all values should be taken from a **fully synced** node and have an overhead of 5-10% added on top of its base value.
308 
309-To calculate `m_assumed_blockchain_size`:
310-- For `mainnet` -> Take the size of the data directory, excluding `/regtest` and `/testnet3` directories.
311-- For `testnet` -> Take the size of the `/testnet3` directory.
312+To calculate `m_assumed_blockchain_size`, take the size in GiB of these directories:
313+- For `mainnet` -> the data directory, excluding the `/regtest`, `/signet`, and `/testnet3` directories and any overly large files, e.g. a huge `debug.log`
314+- For `testnet` -> `/testnet3`

laanwj commented at 2:32 pm on April 19, 2022:

No signet for m_assumed_blockchain_size? I mean, I guess it’s way too small to be relevant at this point

0m_assumed_blockchain_size = 1;
1m_assumed_chain_state_size = 0;

But you do add it for m_assumed_chain_state_size below.

jonatack commented at 6:47 pm on April 19, 2022:

Good catch! Fixed.

laanwj commented at 2:34 pm on April 19, 2022: member

ACK 9560757adc90938b54148e3921aacfaa4aa51be6 modulo signet question

in doc/release-process.md:310 in 9560757adc outdated

305@@ -300,15 +306,15 @@ cat "$VERSION"/*/all.SHA256SUMS.asc > SHA256SUMS.asc
306 Both variables are used as a guideline for how much space the user needs on their drive in total, not just strictly for the blockchain.
307 Note that all values should be taken from a **fully synced** node and have an overhead of 5-10% added on top of its base value.
308 
309-To calculate `m_assumed_blockchain_size`:
310-- For `mainnet` -> Take the size of the data directory, excluding `/regtest` and `/testnet3` directories.
311-- For `testnet` -> Take the size of the `/testnet3` directory.
312+To calculate `m_assumed_blockchain_size`, take the size in GiB of these directories:
313+- For `mainnet` -> the data directory, excluding the `/regtest`, `/signet`, and `/testnet3` directories and any overly large files, e.g. a huge `debug.log`

brunoerg commented at 4:24 pm on April 19, 2022:

What size can be considered a huge debug.log?

jonatack commented at 6:48 pm on April 19, 2022:

See #24424 (review).

jonatack commented at 6:54 pm on April 19, 2022:

(Happy to update if you have a suggestion.)

brunoerg commented at 2:21 pm on May 3, 2022:

Np, sounds good.

brunoerg approved

brunoerg commented at 5:02 pm on April 19, 2022: member

ACK 9560757adc90938b54148e3921aacfaa4aa51be6

Add missing references to signet in the release process 318655c395

Release process: specify blockchain/chain_state units, reduce repetition b4d2d74767

Release process: exclude huge files for mainnet m_assumed_blockchain_size e538eada7c

Clarify release process overhead note to be more actionable e8f844888f

Reorganize release process chainparams section to reduce repetition 584147682a

Specify in release process which chains need to be updated fe048f7f7c

Release process: use 4096 blocks and getbestblockhash for getchaintxstats 415345d547

Clarify in release process how to update defaultAssumeValid/nMinimumChainWork 74743ad905

jonatack force-pushed on Apr 19, 2022

jonatack commented at 6:54 pm on April 19, 2022: member

Thanks for reviewing! Addressed @laanwj’s feedback, fixed an indentation, and reordered the directories to exclude in the m_assumed_blockchain_size mainnet calculation from large to small instead of alphabetical/small to large.

 0--- a/doc/release-process.md
 1+++ b/doc/release-process.md
 2@@ -31,8 +31,8 @@ Release Process
 3 * Update the following variables in [`src/chainparams.cpp`](/src/chainparams.cpp) for mainnet, testnet, and signet:
 4   - `m_assumed_blockchain_size` and `m_assumed_chain_state_size` with the current size plus some overhead (see
 5     [this](#how-to-calculate-assumed-blockchain-and-chain-state-size) for information on how to calculate them).
 6-    - The following updates should be reviewed with `reindex-chainstate` and `assumevalid=0` to catch any defect
 7-      that causes rejection of blocks in the past history.
 8+  - The following updates should be reviewed with `reindex-chainstate` and `assumevalid=0` to catch any defect
 9+    that causes rejection of blocks in the past history.
10   - `chainTxData` with statistics about the transaction count and rate. Use the output of the `getchaintxstats` RPC with an
11     `nBlocks` of 4096 (28 days) and a `bestblockhash` of RPC `getbestblockhash`; see
12     [this pull request](https://github.com/bitcoin/bitcoin/pull/20263) for an example. Reviewers can verify the results by running
13@@ -307,8 +307,9 @@ Both variables are used as a guideline for how much space the user needs on thei
14 Note that all values should be taken from a **fully synced** node and have an overhead of 5-10% added on top of its base value.
15 
16 To calculate `m_assumed_blockchain_size`, take the size in GiB of these directories:
17-- For `mainnet` -> the data directory, excluding the `/regtest`, `/signet`, and `/testnet3` directories and any overly large files, e.g. a huge `debug.log`
18+- For `mainnet` -> the data directory, excluding the `/testnet3`, `/signet`, and `/regtest` directories and any overly large files, e.g. a huge `debug.log`
19 - For `testnet` -> `/testnet3`
20+- For `signet` -> `/signet`
21 
22 To calculate `m_assumed_chain_state_size`, take the size in GiB of these directories:
23 - For `mainnet` -> `/chainstate`

brunoerg approved

brunoerg commented at 2:59 pm on May 3, 2022: member

re-ACK 74743ad90590708b46a8bc8cb9cefedb66471306

theStack commented at 12:39 pm on May 16, 2022: member

Concept ACK

laanwj commented at 1:53 pm on May 30, 2022: member

ACK 74743ad90590708b46a8bc8cb9cefedb66471306

laanwj merged this on May 30, 2022

laanwj closed this on May 30, 2022

jonatack deleted the branch on May 30, 2022

sidhujag referenced this in commit 3645b449cb on May 30, 2022

DrahtBot locked this on May 30, 2023

doc: release process chainparams updates #24424