[Core][RPC][BUG] Use sum of utxo values as money supply #1878
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
random-zebra
added
RPC
Bug
Needs Release Notes
random-zebra
force-pushed
the
202009_moneysupply
branch
from
b2f7647
to
a34a348
Compare
to get the sum of the nValue of all unspent coins.
Now the money supply does not include the zerocoin supply
random-zebra
force-pushed
the
202009_moneysupply
branch
from
a34a348
to
0e7921e
Compare
|
Rebased. |
|
looking good so far. With this, can't we also get rid of the |
|
Oh yep, the ability to remove these special cases was, in fact, one of the pros. |
Fuzzbawls
approved these changes
Oct 6, 2020
1 task
furszy
approved these changes
Oct 10, 2020
furszy
added a commit
that referenced
this pull request
Oct 13, 2020
ae4069e [Core] Skip MoneySupply update during IBD (random-zebra) 5fc1f30 [Tests] Use new rpc getsupplyinfo in mining_pos_reorg.py (random-zebra) 244c608 [RPC] Use getsupplyinfo from within getinfo (random-zebra) 31d26c7 [RPC] Introduce getsupplyinfo RPC function (random-zebra) 8c6e4c7 [Validation] Update money supply when the chainstate is flushed (random-zebra) 809e580 [Init] Initialize global MoneySupply at startup (random-zebra) ed591d6 [Core] Introduce CMoneySupply class (random-zebra) Pull request description: With #1878 the RPC call `getinfo` becomes awfully slow (it triggers a flush of the chainstate to disk and recalculates the amount). In this PR we introduce a in-memory global object (instance of a dumb storage class) to cache the money supply, as well as the height of the chain when it was last updated. The cache is updated in 3 cases: - at startup - after periodic full flushes to disk (blocks and chainstate), except during shutdown or IBD - on-demand via RPC (see below) A new RPC `getsupplyinfo` is introduced. It returns the contents of the cached object, after an optional flush/recalculation (if the `forceupdate` argument is set to true). The money supply is still part of `getinfo` output, for backward compatibility, but the fetching is redirected to the new `getsupplyinfo` (with default value, false, for the `forceupdate` argument, thus without flush/recalculation). Examples: ``` > getblockcount 2542258 > getsupplyinfo { "updateheight": 2542240, "supply": 64090963.91257160 } > getinfo { ... "moneysupply": 64090963.91257160 ... } > getmoneysupplyinfo true { "updateheight": 2542258, "supply": 64091053.91230406 } > getinfo { ... "moneysupply": 64091053.91230406 ... } ``` Based on top of - [x] #1878 Starts with `[Core] Introduce CMoneySupply class` (6554757) ACKs for top commit: furszy: The new RPC command will need to be added to the release notes, other than that ACK ae4069e. Nice improvement Fuzzbawls: ACK ae4069e Tree-SHA512: d88d78a21d23ce63228bf1eb27e02453007dc401cb250dcc7b0a2cc412db7604b555cb915ed1e6d7ea1d333cbe860c5d5857c176b9db19e73be17146a04ae690
random-zebra
removed
the
Needs Release Notes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of updating and databasing independently
nMoneySupply, just use the sum of all unspent tx outputs, returned by the coins cache.This has the following advantages:
nMoneySupplyis databased at every block and does not follow the chainstate flushing strategy, so it's very easy to get an inconsistent state (see [Bug] getinfo returns wrong moneysupply amount #1873).Even more so, considering the absence of any consistency check between the actual chain height and the height at which the supply was updated.
Those outputs, in fact, are effectively "burnt" coins and should not be part of the total supply calculation.
Disadvantage:
getinfo.This PR removes the global variable
nMoneySupplyand all related code (Closes #1873) and gets rid of the startup flag--reindexmoneysupplyno longer needed (#1864 introducesreindex-chainstate).Currently, on master,
gettxoutsetinforeports a total amount of about 811 million PIV!This is due to improper handling of zerocoin-related "utxos".
Basically mints are added to the coins cache (
AddCoinonly skips OP_RETURN coins, not mints) and never pruned/spent (as they are skipped insideUpdateCoins), except inDisconnectBlock.Here we completely remove zerocoin mint outputs from the coins DB.
This makes sense, as originally they were not meant to be linked when spending, so should have been considered unspendable outputs, and not included in the first place. With the introduction of Public Spends, mint outputs are now referenced when spending, but we don't need to keep them in the cache either, as double spending is checked only via the zerocoin serial (and, as said, the coins aren't even spent in
UpdateCoins).We also remove all those utxos flagged as invalid.
Removing zerocoin mints from the unspent coins DB also gives a nice reduction to the utxoset size on disk (about 37%, from 428 Mb to 270 Mb).