-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Chainparams update for 23.x #24418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chainparams update for 23.x #24418
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
src/chainparams.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d8a8b54 update ChainTxData for testnet and signet too? (they were last updated in eeddd1c)
same for nMinimumChainWork and defaultAssumeValid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(side note, I have 41.4 GiB of /indexes on mainnet (4 GiB on testnet), wonder if that space ought to be taken into account somewhere)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could do it seperately. I don't have any testnet node at the moment, mainnet is definitely the one requiring the more thorough review/testing.
Edit: all three networks are now covered
(side note, I have 41.4 GiB of /indexes on mainnet (4 GiB on testnet), wonder if that space ought to be taken into account somewhere)
Good point, but I'm simply following the release process here. It could be updated to include indexes, but I don't have any indexes myself, I guess we assume a default setup without them? That makes some sense for new users (remember, this is for the GUI).
src/chainparams.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be 4096 blocks? The reason I changed this to 4320 is that it's what getchaintxstats returns by default, but it seems we've always explicitly used 4096.
Edit: updated to 4096 just to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder why 28 days (4096) was used instead of the getchaintxstats default of 30 (4320).
7d4d5ef to
7611fcb
Compare
|
Changed to |
|
My testnet data (I just used the current tip -- should I go back a few weeks?) Edit: going back 20000 blocks: |
|
@sdaftuar Thanks!
The release process mentions "Testnet should be set some tens of thousands back from the tip due to reorgs there" for |
Testnet
|
|
Signet: |
7611fcb to
4f87a2d
Compare
src/chainparams.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 000000000000000000052d314a259755ca65944e68df6b12a067ea8f1f5a7091 (assume valid block hash)
achow101
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 4f87a2d32f2018406f4a70d5256d635ba51696a9
|
ACK 4f87a2d Only checked mainnet: % rpc getblockhash 724466
000000000000000000052d314a259755ca65944e68df6b12a067ea8f1f5a7091
% rpc getchaintxstats 4096 000000000000000000052d314a259755ca65944e68df6b12a067ea8f1f5a7091
{
"time": 1645542140,
"txcount": 712531200,
"window_final_block_hash": "000000000000000000052d314a259755ca65944e68df6b12a067ea8f1f5a7091",
"window_final_block_height": 724466,
"window_block_count": 4096,
"window_tx_count": 6950257,
"window_interval": 2404071,
"txrate": 2.891036496010309
} |
|
I think the most important thing still to test here is, especially for mainnet's
I did not check this, not sure anyone else did. |
Launched it. Might take awhile. |
Thanks! Edit: re-pushed with the testnet assumed blockchain size change undone. |
- `m_assumed_chain_state_size` doesn't seem to need to be changed for mainnet. - No change needed for testnet/signet. Co-authored-by: Suhas Daftuar <[email protected]>
4f87a2d to
dbbacb2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK dbbacb2ac86a996c219477c5797e365112612608 for mainnet and signet, but not testnet
Why don't we use the same block for assume-valid and chainTxData on testnet and signet?
I verified the above values on an existing node, did not do a fresh IBD.
Co-authored-by: Suhas Daftuar <[email protected]>
Co-authored-by: Suhas Daftuar <[email protected]>
dbbacb2 to
dca693e
Compare
|
ACK dca693e PGP signatureOne way to fetch my key: gpg -v --auto-key-locate=clear,wkd --locate-key [email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
darosior
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK dca693e -- only checked mainnet (on muliple nodes). Didn't do a reindex.
PGP signature
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK dca693e08e66279c5497cb3d30285ed41ae6983c
Block 000000000000000000052d314a259755ca65944e68df6b12a067ea8f1f5a7091 is in main chain and has 301
confirmations at the time of writing. The chain work at this block is 00000000000000000000000000000000000000002927cdceccbd5209e81e80db.
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEEWQtykmla/6W2csuy4T/BRc0/QwQFAmIXtHEACgkQ4T/BRc0/
QwQ/JgwAxgT+Z+HaRwrmxZDr3xkH8vS6hUWgvOUMsMrJ3Vs9OsbJ/seYRN0QJzTK
Z30PuQqFKJBa7Jh9TU2EHLW7VHMLfjY+zXZywVQYH4NbDKPlhTlSzdndn84dUV5g
vKBwzvlDwzvHTTUsiI0zMWduLb67FSTkAL/SUjC3z32UkF1fPj2WwTbgrVFTnI/j
jTnRmZ+/B1VNWgVmIIqZ8cmUoBPGc8YUb6PM/j6erMPjiWXyesCL2NFEJqJxwRsz
n4NFOdzizT7ucGvetvhGQCp8OGuTEkDxWOHDsHCl+j0NcyE9ZvM5CGVojSXMQCEq
RgKjecnRtg+uYPFV1AoJpOIjDELSfrnHZ4PsC+vJ3RBK+fsqAI89Ne0w1zDgSLzc
Z73twom4OVyIAL9Jv/nVhTm/llq5mgSenh0uszIDLsNgbSsI42zxblIFQ6jgdQYd
377c/DCDe0CE73ViU/ATbS2VFuTx+C9iU2mAAO/hrOHIGHyVFr6Gha0PPOK3wFn8
ofJvxRAu
=viT5
-----END PGP SIGNATURE-----
|
ACK dca693e |
| consensus.nMinimumChainWork = uint256S("0x0000000000000000000000000000000000000000000000000000008546553c03"); | ||
| consensus.defaultAssumeValid = uint256S("0x000000187d4440e5bff91488b700a140441e089a8aaea707414982460edbfe54"); // 47200 | ||
| consensus.nMinimumChainWork = uint256S("0x000000000000000000000000000000000000000000000000000000de26b0e471"); | ||
| consensus.defaultAssumeValid = uint256S("0x00000112852484b5fe3451572368f93cfd2723279af3464e478aee35115256ef"); // 78788 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the same values for signet with reindex-chainstate and assumevalid=0
$ ./src/bitcoin-cli -signet getblockhash 78788
00000112852484b5fe3451572368f93cfd2723279af3464e478aee35115256ef
$ ./src/bitcoin-cli -signet getblockheader 00000112852484b5fe3451572368f93cfd2723279af3464e478aee35115256ef
{
"hash": "00000112852484b5fe3451572368f93cfd2723279af3464e478aee35115256ef",
"confirmations": 166,
"height": 78788,
"version": 536870912,
"versionHex": "20000000",
"merkleroot": "1ab0ba555287a8e81d9a2917d92d7c4a985f6cf455ae155dadaa9b66c80719dc",
"time": 1645631237,
"mediantime": 1645624966,
"nonce": 23153229,
"bits": "1e015f73",
"difficulty": 0.002845317866312478,
"chainwork": "000000000000000000000000000000000000000000000000000000de26b0e471",
"nTx": 8,
"previousblockhash": "000000b34d7f0abb0f4b15c59f7fc8d1c646322189cd0450fdbdbe74db75916b",
"nextblockhash": "0000003d9144c56ac110ae04a0c271a0acce2f14f426b39fdf0d938c96d2eb09"
}
FWIW after 4 days, I am only at 33% verification progress on mainnet. |
|
Post-merge ACK with
I calculated 510G instead of your 460G because I included the optional indexes (txindex, coinstats, and blockfilter). Might want to change in #24424
✔️
✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the most important thing still to test here is, especially for mainnet's
nMinimumChainWorkanddefaultAssumeValidupdate:This update should be reviewed with a reindex-chainstate with assumevalid=0 to catch any defect that causes rejection of blocks in the past history.
After 11 days my node completed reindex-chainstate with assumevalid=0 on mainnet and confirms the same result as this pull.
767d825 Update chainparams for 24.0 release (Janna) Pull request description: Update chain parameters for upcoming major release. See [doc/release-process.md](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md) and bitcoin#24418 for review instructions. fixes bitcoin#25921 ACKs for top commit: Sjors: tACK 767d825 achow101: utACK 767d825 Tree-SHA512: 153390203c76c981cc41629a27ec3e52fec089c7ce6edba3dd4d77c875c7d8afcae64be2bd9bc8af73f70c2dc0a08666f2986ac82c9fd536b0fded10fd8dec3d
Update chain parameters for upcoming major release. See doc/release-process.md for review instructions.
m_assumed_blockchain_size,m_assumed_chain_state_size:chainTxData:nMinimumChainWork,defaultAssumeValid: