-
Notifications
You must be signed in to change notification settings - Fork 717
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
[Core] CBlockIndex cleanup - Bump client version to 4.0.99.1 #1338
[Core] CBlockIndex cleanup - Bump client version to 4.0.99.1 #1338
Conversation
Human DrahtBot: conflicts after merging 1328. Need rebase. |
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.
Code ACK, just one minor comment. Really nice work 👌
ed8591e
to
084a18c
Compare
Rebased, and moved block version before stake modifier in CDiskBlockIndex serialization, as per @furszy 's suggestion |
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.
Full sync completed on testnet without any problem, ACK 084a18c.
needs rebase |
The function GetStakeModifierChecksum is used to update the class variable nStakeModifierChecksum which is never used. - remove CBlockIndex variable - remove GetStakeModifierChecksum function in kernel
also remove vMintDenominationsInBlock after block version 6
Use a char vector to store the stake modifier being it a (uint64_t) v1 modifier or (uint256) v2 modifier. It is empty for pow blocks.
Last CLIENT_VERSION with old serialization is 4009900. First one with new serialization is 4009901. Define new serialization and bump the build version.
nMint is set and used only in ConnectBlock to verify that the block does not overmint. It can be done with a local variable, no reason to save it in the index.
- remove RecalculateZPIVSpent and RecalculateZPIVMinted main functions - remove MintedDenomination CBlockIndex method
- remove SetNull function - replace with ClearMapZcSupply used to set an empty mapZerocoinSupply
084a18c
to
3d88a47
Compare
Rebased |
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 rebase 3d88a47.
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 3d88a47
Merging... |
2b31bf4 [Tests] Check PIV and zPIV supply in mining_pos_reorg (random-zebra) 8b2b418 [Chain] Add checks for ser_action in block index serialization (random-zebra) 494ea2a [Core] Guard nMoneySupply and mapZerocoinSupply with cs_main LOCK (random-zebra) ec2f82d [Doc] Document removal of moneysupply and zpivSupply from getblock RPC (random-zebra) c30b98d [Core] Update money supply when disconnecting blocks too (random-zebra) 361e584 [Chain] Remove CBlockIndex::nMoneySupply, replace with global instance (random-zebra) 15d22a6 [RPC] remove nMoneySupply from BlockToJSON (random-zebra) 994dc1f [Cleanup] Nuke horrendous SeeSaw calculation (random-zebra) f5ee149 [zPIV] Remove extra on-time recalculation of supply (random-zebra) b370414 [zPIV] Recalculate zPIV supply including wrapped serials inflation (random-zebra) ee5c843 [zPIV] Update supply when disconnecting blocks too (random-zebra) b738103 [DB] Database new mapZerocoinSupply (random-zebra) 56b454a [Chain] remove CBlockIndex::mapZerocoinSupply and bump client version (random-zebra) 9c95c29 [Refactor] introduce global mapZerocoinSupply and move chain functions (random-zebra) 6844110 [RPC] remove mapZerocoinSupply from BlockToJSON (random-zebra) Pull request description: This completes the work started in #1338 and bumps the version to `4.0.99.2`. It gives a dramatic improvement in memory usage. On mainnet synced up to block 2281117, it takes **~926 Mb** in total (a 59% reduction over the 2.261 Gb of `v4.0.99.1`, and 64% overall reduction over the 2.570 Gb of `v4.0.2`). Changes: `mapZerocoinSupply` and `moneySupply` are removed from the block index serialization and replaced with a single cached map/value updated in `ConnectBlock`/`DisconnectBlock` and databased independently. To avoid reindex on upgrade, a utility class `CLegacyBlockIndex` is introduced, in order to marshall old (removed) block index members directly from disk. ACKs for top commit: furszy: ACK 2b31bf4 Fuzzbawls: ACK 2b31bf4 Tree-SHA512: ed191f034ab337208e1ffc8a2c6ef5a24e1aedaf20d0f7f117e2efc35480e3c6bd8052b1b85a75a40bc8220b37f2e4fe7f2857b7f2af5f6c5a22ebe28c27e00e
Removes a number of (unused or barely used) class variables/methods in CBlockIndex, and introduces a way to use a single variable to store either v1 or v2 stake modifiers.
Since
CDiskBlockIndex
serialization starts with CLIENT_VERSION, this pr bumps the CLIENT_VERSION_BUILD in order to introduce a new serialization without requiring reindex.This is a first (tiny) step towards reducing our memory usage.
On a quick test (x86_64 linux) average RAM usage on (fully synced) Mainet is reduced from 2.486 Gb to 2.190 Gb, and from 1.776 Gb to 1.566 Gb on Testnet.
Future work:
Remove mapZerocoinSupply and nMoneySupply from the blockindex as well.