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.
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.
(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.
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.
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.
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.
ACK 000000000000000000052d314a259755ca65944e68df6b12a067ea8f1f5a7091 (assume valid block hash)
achow101
left a comment
There was a problem hiding this comment.
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
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] |
darosior
left a comment
There was a problem hiding this comment.
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.
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.
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: