-
Notifications
You must be signed in to change notification settings - Fork 38.6k
Update assumed chain params #20263
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
Update assumed chain params #20263
Conversation
fa5a461 to
fa0c1b5
Compare
fanquake
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 fa0c1b5820d12354d1e31bb561e73165b0adfbac - mainnet, testnet and signet output matches my node.
mainnet blocks & chainstate size:
du -sh /Users/michael/Library/Application\ Support/Bitcoin/blocks
325G /Users/michael/Library/Application Support/Bitcoin/blocks
du -sh /Users/michael/Library/Application\ Support/Bitcoin/chainstate
3.9G /Users/michael/Library/Application Support/Bitcoin/chainstate
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.
6 seems a bit aggressive (my chainstate is 3.9), but it doesn't matter that much, as this is only used for the intro dialog.
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.
Mine is almost 5 🤷♂️
$ du -sh ./chainstate/
4.8G ./chainstate/
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.
Mine is 4, fwiw.
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.
3.9 here like @fanquake
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.
Having 4.0G here for chainstate.
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.
$ du -sh .bitcoin/chainstate/
4.0G .bitcoin/chainstate/
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.
Mine is 5.2G
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 fa0c1b5820d12354d1e31bb561e73165b0adfbac mainnet and testnet params.
jonatack
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.
On mainnet, I see a blockchain size of 326.2 and a chainstate size of 3.9.
The chaintxdata for me are the same as this PR, except for signet.
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.
Mainnet chaintxdata seem good:
$ ./src/bitcoin-cli getchaintxstats 4096 0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72
{
"time": 1603995752,
"txcount": 582083445,
"window_final_block_hash": "0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72",
"window_final_block_height": 654683,
"window_block_count": 4096,
"window_tx_count": 8995148,
"window_interval": 2563468,
"txrate": 3.508976121410527
}
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.
Testnet3 chaintxdata seem good:
$ ./src/bitcoin-cli -testnet getchaintxstats 4096 000000000000006433d1efec504c53ca332b64963c425395515b01977bd7b3b0
{
"time": 1603359686,
"txcount": 58090238,
"window_final_block_hash": "000000000000006433d1efec504c53ca332b64963c425395515b01977bd7b3b0",
"window_final_block_height": 1864000,
"window_block_count": 4096,
"window_tx_count": 143148,
"window_interval": 1161080,
"txrate": 0.1232886622799463
}
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.
I have different Signet chaintxdata:
$ ./src/bitcoin-cli -signet getchaintxstats 4096 00000128807d9175c494e24d805fc7854f7d79aa965cbb128342ad8b70cecfa5
{
"time": 1601382000,
"txcount": 5435,
"window_final_block_hash": "00000128807d9175c494e24d805fc7854f7d79aa965cbb128342ad8b70cecfa5",
"window_final_block_height": 5348,
"window_block_count": 4096,
"window_tx_count": 4182,
"window_interval": 2202970,
"txrate": 0.001898346323372538
}
(and so different getblockheader data as well)
$ ./src/bitcoin-cli -signet getblockheader "00000128807d9175c494e24d805fc7854f7d79aa965cbb128342ad8b70cecfa5"
{
"hash": "00000128807d9175c494e24d805fc7854f7d79aa965cbb128342ad8b70cecfa5",
"height": 5348,
"chainwork": "0000000000000000000000000000000000000000000000000000000d145533ce",
}
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.
The comment has not been updated https://github.com/bitcoin/bitcoin/pull/20263/files#r515620099 . Nice find.
jonatack
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 for mainnet and testnet, not sure why I have different data for signet.
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.
The mainnet nMinimumChainWork and defaultAssumeValid consensus params match up for me
$ ./src/bitcoin-cli getblockheader 0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72
{
"hash": "0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72",
"height": 654683,
"chainwork": "00000000000000000000000000000000000000001533efd8d716a517fe2c5008",
}
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.
Testnet nMinimumChainWork and defaultAssumeValid consensus params match up for me
$ ./src/bitcoin-cli -testnet getblockheader 000000000000006433d1efec504c53ca332b64963c425395515b01977bd7b3b0
{
"hash": "000000000000006433d1efec504c53ca332b64963c425395515b01977bd7b3b0",
"height": 1864000,
"chainwork": "0000000000000000000000000000000000000000000001db6ec4ac88cf2272c6",
}
theStack
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.
Verified mainnet, testnet3, signet params via the following commands (starting with block height as first input feels most natural to me):
$ bitcoin-cli getblockheader $(bitcoin-cli getblockhash 654683) | grep "\"chainwork\|\"hash"
$ bitcoin-cli getchaintxstats 4096 0000000000000000000b9d2ec5a352ecba0592946514a92f14319dc2b367fc72
$ bitcoin-cli -testnet getblockheader $(bitcoin-cli -testnet getblockhash 1864000) | grep "\"chainwork\|\"hash"
$ bitcoin-cli -testnet getchaintxstats 4096 000000000000006433d1efec504c53ca332b64963c425395515b01977bd7b3b0
$ bitcoin-cli -signet getblockheader $(bitcoin-cli -signet getblockhash 9434) | grep "\"chainwork\|\"hash"
$ bitcoin-cli -signet getchaintxstats 4096 0000002a1de0f46379358c1fd09906f7ac59adf3712323ed90eb59e4c183c020
ACK (modulo the getchaintxstats signet comment that has not been updated yet (#20263 (comment)).
fa45c40 to
fa90ba3
Compare
|
I force pushed to fixup the signet comment. Should be easy to re-ACK for those that already ACKed |
dergoegge
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 fa90ba3 - mainnet and testnet data matches my node.
Followed process as outlined here: https://github.com/fanquake/core-review/blob/master/update-assumevalid.md
|
ACK fa90ba3 per |
theStack
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.
re-ACK fa90ba3 ✔️
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.
re-ACK fa90ba3 for mainnet and testnet.
See https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off