[Core] Per-txout model for chainstate database #1801
Merged
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 this to In Progress
in perpetual updating PIVX Core to BTC Core
via automation
random-zebra
force-pushed
the
202007_pertxoutcache_2
branch
2 times, most recently
from
903ca50
to
54e98d8
Compare
random-zebra
force-pushed
the
202007_pertxoutcache_2
branch
2 times, most recently
from
9c7e104
to
02b804b
Compare
|
Rebased on master. Ready for review. |
change the serialization/unserialization code, mirroring CTxInUndo class (which will be removed in next commit, and replaced with a serialization adapter for Coin)
>>> backports bitcoin/bitcoin@cb2c7fd The earlier CTxInUndo class now holds the same information as the Coin class. Instead of duplicating functionality, replace CTxInUndo with a serialization adapter for Coin.
>>> backports bitcoin/bitcoin@bd83111 This avoids a prevector copy in ApplyTxInUndo.
>>> backports bitcoin/bitcoin@0003911 The new functions are: * CCoinsViewCache::AddCoin: Add a single COutPoint/Coin pair. * CCoinsViewCache::SpendCoin: Remove a single COutPoint. * AddCoins: utility function that invokes CCoinsViewCache::AddCoin for each output in a CTransaction. * AccessByTxid: utility function that searches for any output with a given txid. * CCoinsViewCache::AccessCoin: retrieve the Coin for a COutPoint. * CCoinsViewCache::HaveCoins: check whether a non-empty Coin exists for a given COutPoint. The AddCoin and SpendCoin methods will eventually replace ModifyCoins and ModifyNewCoins, AddCoins will replace CCoins::FromTx, and the new AccessCoins and HaveCoins functions will replace their per-txid counterparts. Note that AccessCoin for now returns a copy of the Coin object. In a later commit it will be change to returning a const reference (which keeps working in all call sites).
>>> backports bitcoin/bitcoin@c87b957 This clarifies a bit more the ways in which the new script execution cache could break consensus in the future if additional data from the CCoins object were to be used as a part of script execution. After this change, any such consensus breaks should be very visible to reviewers, hopefully ensuring no such changes can be made.
>>> backports bitcoin/bitcoin@5083079 This patch makes several related changes: * Changes the CCoinsView virtual methods (GetCoins, HaveCoins, ...) to be COutPoint/Coin-based rather than txid/CCoins-based. * Changes the chainstate db to a new incompatible format that is also COutPoint/Coin based. * Implements reconstruction code for hash_serialized_2. * Adapts the coins_tests unit tests (thanks to Russell Yanofsky). A side effect of the new CCoinsView model is that we can no longer use the (unreliable) test for transaction outputs in the UTXO set to determine whether we already have a particular transaction.
>>> backports bitcoin/bitcoin@ce23efa
>>> backports bitcoin/bitcoin@97072d6
>>> backports bitcoin/bitcoin@b2af357 As the maximum amount of data that can be pulled into the cache due to a block validation is much lower now (at most one CCoin entry per input and per output), reduce the conservative estimate used to determine flushing time.
>>> backports bitcoin/bitcoin@580b023 It's only used for upgrading from the old database anymore.
They're doing the same thing now.
>>> backports bitcoin/bitcoin@5898279 Thanks to John Newberry for pointing these out. -BEGIN VERIFY SCRIPT- sed -i 's/\<GetCoins\>/GetCoin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<HaveCoins\>/HaveCoin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<HaveCoinsInCache\>/HaveCoinInCache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<IsPruned\>/IsSpent/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<FetchCoins\>/FetchCoin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<CoinsEntry\>/CoinEntry/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<vHashTxnToUncache\>/coins_to_uncache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<vHashTxToUncache\>/coins_to_uncache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<fHadTxInCache\>/had_coin_in_cache/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<coinbaseids\>/coinbase_coins/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<disconnectedids\>/disconnected_coins/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<duplicateids\>/duplicate_coins/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h sed -i 's/\<oldcoins\>/old_coin/g' src/test/coins_tests.cpp sed -i 's/\<origcoins\>/orig_coin/g' src/*.cpp src/*.h src/*/*.cpp src/*/*.h -END VERIFY SCRIPT-
random-zebra
force-pushed
the
202007_pertxoutcache_2
branch
from
02b804b
to
535d8e4
Compare
furszy
reviewed
Aug 31, 2020
1 task
perpetual updating PIVX Core to BTC Core
automation
moved this from In Progress
to Ready
Sep 5, 2020
Fuzzbawls
approved these changes
Sep 5, 2020
furszy
added a commit
that referenced
this pull request
Sep 7, 2020
cf7003b [Cleanup] Remove cacheInputAge nonsense from CMasternode (random-zebra) 7cebb9b [Refactor] Get rid of GetInputAge static function (random-zebra) ae2af54 [Refactoring] Add GetCoinDepth utility function to CCoinsViewCache (random-zebra) Pull request description: Introduce a `GetCoinDepth()` utility function in CCoinsViewCache and use that instead (with `pcoinsTip`... no need to lock/load the mempool too in the backend). This should fix the invalid lock order: ``` 2020-09-02 10:23:17 POTENTIAL DEADLOCK DETECTED 2020-09-02 10:23:17 Previous lock order was: 2020-09-02 10:23:17 cs_process_message /src/masternodeman.cpp:690 (in thread pivx-msghand) 2020-09-02 10:23:17 (1) mempool.cs /src/main.cpp:777 (in thread pivx-msghand) 2020-09-02 10:23:17 (2) cs_main /src/main.cpp:785 (in thread pivx-msghand) 2020-09-02 10:23:17 Current lock order is: 2020-09-02 10:23:17 (2) cs_main /src/main.cpp:4571 (in thread ) 2020-09-02 10:23:17 (1) cs /src/txmempool.cpp:590 (in thread ) ``` To be able to use the new per-txout interface, this is based on top of: - [x] #1801 The present PR starts with `[Refactoring] Add GetCoinDepth utility function to CCoinsViewCache` (294e177) BONUS: remove the cached "ages" saved/serialized in CMasternode class... no comment. ACKs for top commit: furszy: utACK cf7003b Tree-SHA512: 8eb73ff32d542a97aacd53ca4e0743a7facdf9055a45feea80e47822befc4ee499d4d943160e3926bd62ad287ccff546fdc3e868a0c6d15eb9e3e8b34e367c20
furszy
added a commit
that referenced
this pull request
Sep 23, 2020
979cb74 [Trivial][UI] Fix init messages (random-zebra) Pull request description: After #1801 the coins database is updated to the new format. During this time (which could take a minute or so), QT wallet reports "Loading block index..." <img src="https://user-images.githubusercontent.com/18186894/93999833-9bdb8c80-fd96-11ea-9c40-d58417a122a0.png" width="60%"> which is wrong: even without the coins DB upgrade, it loads first the sporks DB, before the block index. So: - remove the misplaced init message for BlockIndex load (there's another one in the right place). - add a new init message for the coins DB upgrade (if the DB is already upgraded, the function returns immediately so this is shown only for a fraction of a second before "Loading sporks..."). Note: As 4.3 is the first release including the new DB, we should probably backport this trivial PR. ACKs for top commit: furszy: utACK 979cb74 Fuzzbawls: ACK 979cb74 Tree-SHA512: e85a3e128907430391d47b28ed66473e2be16ec83683362ccdc8e1d4ce62d24f8a39de9ce049dce339cb575616194ee5cfb8c8f033fc0dde8f154c18a0b01a99
Fuzzbawls
added
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.
The chainstate database, and its cache, rely on a per-tx model (i.e. it is a map from txids to lists of unspent outputs).
This PR chages it to a per-txout model (i.e. a map from outpoints to single unspent outputs).
Pros:
Cons:
Adapted from bitcoin#10195 (adding a
fCoinStakeboolean memeber to the Coin class and considering BIP34 always in effect as per #1775)Based on top of:
The actual PR starts with "Introduce Coin, a single unspent output" (171bf0b)