Merge #6137: backport: merge bitcoin#21732, bitcoin#21762, bitcoin#21754

, bitcoin#21953, bitcoin#21850, bitcoin#22633, bitcoin#22738, bitcoin#23154, bitcoin#23721, bitcoin#24002, bitcoin#24197, merge bitcoin-core/gui#399 (auxiliary backports: part 14)

1c5ea38 merge bitcoin#24197: Replace lock with thread safety annotation in CBlockTreeDB::LoadBlockIndexGuts() (Kittywhiskers Van Gogh)
e5e3745 merge bitcoin#24002: add thread safety lock assertion to WriteBlockIndexDB() (Kittywhiskers Van Gogh)
04a3f65 merge bitcoin#23721: Move restorewallet() logic to the wallet section (Kittywhiskers Van Gogh)
e47d5ac merge bitcoin#23154: add assumeutxo notes (Kittywhiskers Van Gogh)
847d866 merge bitcoin#22738: fix failure in feature_nulldummy.py on single-core machines (Kittywhiskers Van Gogh)
ad96ef2 merge bitcoin#22633: Replace remaining binascii method calls (Kittywhiskers Van Gogh)
b37f609 merge bitcoin-core/gui#399: Fix "Load PSBT" functionality when no wallet loaded (Kittywhiskers Van Gogh)
94173f1 merge bitcoin#21850: Remove `GetDataDir(net_specific)` function (Kittywhiskers Van Gogh)
6264c7b merge bitcoin#21953: fuzz: Add utxo_snapshot target (Konstantin Akimov)
8b7ea28 merge bitcoin#21754: Run feature_cltv with MiniWallet (Kittywhiskers Van Gogh)
bd75014 merge bitcoin#21762: Speed up mempool_spend_coinbase.py (Kittywhiskers Van Gogh)
72eeb9a merge bitcoin#21732: Move common init code to init/common (Kittywhiskers Van Gogh)
3944d4e chore: resolve nit from dash#6085 (blockstorage backports) (Kittywhiskers Van Gogh)
92509e2 fix: don't suppress `-logtimestamps` help if `HAVE_THREAD_LOCAL` undef (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for #6138

  * In [bitcoin#21754](bitcoin#21754), the `scriptSig` padding multiplier (`24`) differs from upstream (`35`) as the `vsize` value it corresponds to must match what is ordinarily generated (`85` vs `96` upstream) in order to fulfill an assertion ([source](https://github.com/dashpay/dash/blob/d9835515cc1e1fd7d4fd3006b51a141fa2265613/test/functional/test_framework/wallet.py#L107)).

  * In [bitcoin#21953](bitcoin#21953), the hash associated with height `200` is generated like this (this is the same method used in [dash#5236](#5236)):
    * Add the height desired to the `CRegTestParams::m_assumeutxo_data` map with a garbage hash value (like `uint256::ONE`). This is to avoid an unrecognized metadata failure ([source](https://github.com/dashpay/dash/blob/5211886fb44c839d9197599d39908e1b707dfc7c/src/validation.cpp#L5755-L5761)) caused by looking through the map to see if the height's there.
    * Change the `LogPrintf(..)` in the serialized hash check error log message located [here](https://github.com/dashpay/dash/blob/5211886fb44c839d9197599d39908e1b707dfc7c/src/validation.cpp#L5876-L5880) to a `std::cout << strprintf(..)`
    * Edit the value of `mineBlocks` [here](https://github.com/dashpay/dash/blob/5211886fb44c839d9197599d39908e1b707dfc7c/src/test/validation_chainstatemanager_tests.cpp#L248-L253) to be 100 blocks _less_ than the desired height.
    * Compile Dash Core and run `./src/test/test_dash -t validation_chainstatemanager_tests`
    * Take the `got` value printed to your terminal window/`stdout` (the `expected` value should be our garbage value from earlier, ignore that). That's your good hash.
    * Update the `CRegTestParams::m_assumeutxo_data` map entry with the correct entry, reverse every change _except_ the map entry (for obvious reasons) and the `mineBlocks` change.
      * Remember to add/update the hash [here](https://github.com/dashpay/dash/blob/5211886fb44c839d9197599d39908e1b707dfc7c/src/test/validation_tests.cpp#L29-L31) in `validation_tests`, it simply tests the hardcoded chainparams value with its own hardcoded value. That's also why we don't use this test since it'll just regurgitate the garbage values we give it.
    * Compile and re-run the test. If it passes, your hash is good. Revert the `mineBlocks` change.
    * Profit?

  ## Breaking Changes

  None expected.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 1c5ea38
  PastaPastaPasta:
    utACK 1c5ea38

Tree-SHA512: 1ce0d4f1cef68990412e2e7046b36db7425059ee41b39e3681fa05d59fe24a0a74ad8c5d833c0e4c0686f693af665ca749e504b88ad30e708fc163045160aa58

contrib/linearize/linearize-data.py

@@ -17,7 +17,6 @@
 import time
 import glob
 from collections import namedtuple
-from binascii import unhexlify
 
 settings = {}
 
@@ -324,7 +323,7 @@ def run(self):
     settings['max_out_sz'] = int(settings['max_out_sz'])
     settings['split_timestamp'] = int(settings['split_timestamp'])
     settings['file_timestamp'] = int(settings['file_timestamp'])
-    settings['netmagic'] = unhexlify(settings['netmagic'].encode('utf-8'))
+    settings['netmagic'] = bytes.fromhex(settings['netmagic'])
     settings['out_of_order_cache_sz'] = int(settings['out_of_order_cache_sz'])
     settings['debug_output'] = settings['debug_output'].lower()
 

doc/README.md

@@ -68,6 +68,7 @@ The Dash Core repo's [root README](/README.md) contains relevant information on
 
 ### Miscellaneous
 - [Assets Attribution](assets-attribution.md)
+- [Assumeutxo design](assumeutxo.md)
 - [dash.conf Configuration File](dash-conf.md)
 - [CJDNS Support](cjdns.md)
 - [Files](files.md)

doc/assumeutxo.md

@@ -0,0 +1,138 @@
+# assumeutxo
+
+Assumeutxo is a feature that allows fast bootstrapping of a validating dashd
+instance with a very similar security model to assumevalid.
+
+The RPC commands `dumptxoutset` and `loadtxoutset` are used to respectively generate
+and load UTXO snapshots. The utility script `./contrib/devtools/utxo_snapshot.sh` may
+be of use.
+
+## General background
+
+- [assumeutxo proposal](https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal)
+- [Github issue](https://github.com/bitcoin/bitcoin/issues/15605) (Bitcoin)
+- [draft PR](https://github.com/bitcoin/bitcoin/pull/15606) (Bitcoin)
+
+## Design notes
+
+- A new block index `nStatus` flag is introduced, `BLOCK_ASSUMED_VALID`, to mark block
+  index entries that are required to be assumed-valid by a chainstate created
+  from a UTXO snapshot. This flag is mostly used as a way to modify certain
+  CheckBlockIndex() logic to account for index entries that are pending validation by a
+  chainstate running asynchronously in the background. We also use this flag to control
+  which index entries are added to setBlockIndexCandidates during LoadBlockIndex().
+
+- Indexing implementations via BaseIndex can no longer assume that indexation happens
+  sequentially, since background validation chainstates can submit BlockConnected
+  events out of order with the active chain.
+
+- The concept of UTXO snapshots is treated as an implementation detail that lives
+  behind the ChainstateManager interface. The external presentation of the changes
+  required to facilitate the use of UTXO snapshots is the understanding that there are
+  now certain regions of the chain that can be temporarily assumed to be valid (using
+  the nStatus flag mentioned above). In certain cases, e.g. wallet rescanning, this is
+  very similar to dealing with a pruned chain.
+
+  Logic outside ChainstateManager should try not to know about snapshots, instead
+  preferring to work in terms of more general states like assumed-valid.
+
+
+## Chainstate phases
+
+Chainstate within the system goes through a number of phases when UTXO snapshots are
+used, as managed by `ChainstateManager`. At various points there can be multiple
+`CChainState` objects in existence to facilitate both maintaining the network tip and
+performing historical validation of the assumed-valid chain.
+
+It is worth noting that though there are multiple separate chainstates, those
+chainstates share use of a common block index (i.e. they hold the same `BlockManager`
+reference).
+
+The subheadings below outline the phases and the corresponding changes to chainstate
+data.
+
+### "Normal" operation via initial block download
+
+`ChainstateManager` manages a single CChainState object, for which
+`m_snapshot_blockhash` is null. This chainstate is (maybe obviously)
+considered active. This is the "traditional" mode of operation for dashd.
+
+|    |    |
+| ---------- | ----------- |
+| number of chainstates | 1 |
+| active chainstate | ibd |
+
+### User loads a UTXO snapshot via `loadtxoutset` RPC
+
+`ChainstateManager` initializes a new chainstate (see `ActivateSnapshot()`) to load the
+snapshot contents into. During snapshot load and validation (see
+`PopulateAndValidateSnapshot()`), the new chainstate is not considered active and the
+original chainstate remains in use as active.
+
+|    |    |
+| ---------- | ----------- |
+| number of chainstates | 2 |
+| active chainstate | ibd |
+
+Once the snapshot chainstate is loaded and validated, it is promoted to active
+chainstate and a sync to tip begins. A new chainstate directory is created in the
+datadir for the snapshot chainstate called
+`chainstate_[SHA256 blockhash of snapshot base block]`.
+
+|    |    |
+| ---------- | ----------- |
+| number of chainstates | 2 |
+| active chainstate | snapshot |
+
+The snapshot begins to sync to tip from its base block, technically in parallel with
+the original chainstate, but it is given priority during block download and is
+allocated most of the cache (see `MaybeRebalanceCaches()` and usages) as our chief
+consideration is getting to network tip.
+
+**Failure consideration:** if shutdown happens at any point during this phase, both
+chainstates will be detected during the next init and the process will resume.
+
+### Snapshot chainstate hits network tip
+
+Once the snapshot chainstate leaves IBD, caches are rebalanced
+(via `MaybeRebalanceCaches()` in `ActivateBestChain()`) and more cache is given
+to the background chainstate, which is responsible for doing full validation of the
+assumed-valid parts of the chain.
+
+**Note:** at this point, ValidationInterface callbacks will be coming in from both
+chainstates. Considerations here must be made for indexing, which may no longer be happening
+sequentially.
+
+### Background chainstate hits snapshot base block
+
+Once the tip of the background chainstate hits the base block of the snapshot
+chainstate, we stop use of the background chainstate by setting `m_stop_use` (not yet
+committed - see bitcoin#15606), in `CompleteSnapshotValidation()`, which is checked in
+`ActivateBestChain()`). We hash the background chainstate's UTXO set contents and
+ensure it matches the compiled value in `CMainParams::m_assumeutxo_data`.
+
+The background chainstate data lingers on disk until shutdown, when in
+`ChainstateManager::Reset()`, the background chainstate is cleaned up with
+`ValidatedSnapshotShutdownCleanup()`, which renames the `chainstate_[hash]` datadir as
+`chainstate`.
+
+|    |    |
+| ---------- | ----------- |
+| number of chainstates | 2 (ibd has `m_stop_use=true`) |
+| active chainstate | snapshot |
+
+**Failure consideration:** if dashd unexpectedly halts after `m_stop_use` is set on
+the background chainstate but before `CompleteSnapshotValidation()` can finish, the
+need to complete snapshot validation will be detected on subsequent init by
+`ChainstateManager::CheckForUncleanShutdown()`.
+
+### Dashd restarts sometime after snapshot validation has completed
+
+When dashd initializes again, what began as the snapshot chainstate is now
+indistinguishable from a chainstate that has been built from the traditional IBD
+process, and will be initialized as such.
+
+|    |    |
+| ---------- | ----------- |
+| number of chainstates | 1 |
+| active chainstate | ibd |

share/rpcauth/rpcauth.py

@@ -5,15 +5,14 @@
 
 from argparse import ArgumentParser
 from base64 import urlsafe_b64encode
-from binascii import hexlify
 from getpass import getpass
 from os import urandom
 
 import hmac
 
 def generate_salt(size):
     """Create size byte hex salt"""
-    return hexlify(urandom(size)).decode()
+    return urandom(size).hex()
 
 def generate_password():
     """Create 32 byte b64 password"""

src/Makefile.am

@@ -216,6 +216,7 @@ BITCOIN_CORE_H = \
   index/txindex.h \
   indirectmap.h \
   init.h \
+  init/common.h \
   interfaces/chain.h \
   interfaces/coinjoin.h \
   interfaces/handler.h \
@@ -731,6 +732,7 @@ libbitcoin_common_a_SOURCES = \
   core_write.cpp \
   deploymentinfo.cpp \
   governance/common.cpp \
+  init/common.cpp \
   key.cpp \
   key_io.cpp \
   merkleblock.cpp \

src/Makefile.test.include

@@ -331,6 +331,7 @@ test_fuzz_fuzz_SOURCES = \
   test/fuzz/tx_in.cpp \
   test/fuzz/tx_out.cpp \
   test/fuzz/tx_pool.cpp \
+  test/fuzz/utxo_snapshot.cpp \
   test/fuzz/validation_load_mempool.cpp \
   test/fuzz/versionbits.cpp
 endif # ENABLE_FUZZ_BINARY

src/addrdb.cpp

@@ -52,7 +52,7 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data
     std::string tmpfn = strprintf("%s.%04x", prefix, randv);
 
     // open temp output file, and associate with CAutoFile
-    fs::path pathTmp = GetDataDir() / tmpfn;
+    fs::path pathTmp = gArgs.GetDataDirNet() / tmpfn;
     FILE *file = fsbridge::fopen(pathTmp, "wb");
     CAutoFile fileout(file, SER_DISK, version);
     if (fileout.IsNull()) {
@@ -172,7 +172,7 @@ bool CBanDB::Read(banmap_t& banSet)
 
 bool DumpPeerAddresses(const ArgsManager& args, const AddrMan& addr)
 {
-    const auto pathAddr = GetDataDir() / "peers.dat";
+    const auto pathAddr = gArgs.GetDataDirNet() / "peers.dat";
     return SerializeFileDB("peers", pathAddr, addr, CLIENT_VERSION);
 }
 
@@ -187,7 +187,7 @@ std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const A
     addrman = std::make_unique<AddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman);
 
     int64_t nStart = GetTimeMillis();
-    const auto path_addr{GetDataDir() / "peers.dat"};
+    const auto path_addr{gArgs.GetDataDirNet() / "peers.dat"};
     try {
         DeserializeFileDB(path_addr, *addrman, CLIENT_VERSION);
         LogPrintf("Loaded %i addresses from peers.dat  %dms\n", addrman->size(), GetTimeMillis() - nStart);

src/chainparams.cpp

@@ -901,8 +901,8 @@ class CRegTestParams : public CChainParams {
                 {AssumeutxoHash{uint256S("0x9b2a277a3e3b979f1a539d57e949495d7f8247312dbc32bce6619128c192b44b")}, 110},
             },
             {
-                210,
-                {AssumeutxoHash{uint256S("0xd4c97d32882583b057efc3dce673e44204851435e6ffcef20346e69cddc7c91e")}, 210},
+                200,
+                {AssumeutxoHash{uint256S("0x8a5bdd92252fc6b24663244bbe958c947bb036dc1f94ccd15439f48d8d1cb4e3")}, 200},
             },
         };
 

src/coinjoin/client.cpp

@@ -137,7 +137,7 @@ void CCoinJoinClientManager::ProcessMessage(CNode& peer, CChainState& active_cha
     if (!CCoinJoinClientOptions::IsEnabled()) return;
     if (!m_mn_sync.IsBlockchainSynced()) return;
 
-    if (!CheckDiskSpace(GetDataDir())) {
+    if (!CheckDiskSpace(gArgs.GetDataDirNet())) {
         ResetPool();
         StopMixing();
         WalletCJLogPrint(m_wallet, "CCoinJoinClientManager::ProcessMessage -- Not enough disk space, disabling CoinJoin.\n");
@@ -460,7 +460,7 @@ bool CCoinJoinClientSession::SendDenominate(const std::vector<std::pair<CTxDSIn,
         return false;
     }
 
-    if (!CheckDiskSpace(GetDataDir())) {
+    if (!CheckDiskSpace(gArgs.GetDataDirNet())) {
         UnlockCoins();
         keyHolderStorage.ReturnAll();
         WITH_LOCK(cs_coinjoin, SetNull());

src/evo/evodb.cpp

@@ -32,7 +32,7 @@ void CEvoDBScopedCommitter::Rollback()
 }
 
 CEvoDB::CEvoDB(size_t nCacheSize, bool fMemory, bool fWipe) :
-    db(fMemory ? "" : (GetDataDir() / "evodb"), nCacheSize, fMemory, fWipe),
+    db(fMemory ? "" : (gArgs.GetDataDirNet() / "evodb"), nCacheSize, fMemory, fWipe),
     rootBatch(db),
     rootDBTransaction(db, rootBatch),
     curDBTransaction(rootDBTransaction, rootDBTransaction)

src/flat-database.h

@@ -180,7 +180,7 @@ class CFlatDB
 public:
     CFlatDB(std::string strFilenameIn, std::string strMagicMessageIn)
     {
-        pathDB = GetDataDir() / strFilenameIn;
+        pathDB = gArgs.GetDataDirNet() / strFilenameIn;
         strFilename = strFilenameIn;
         strMagicMessage = strMagicMessageIn;
     }

src/index/blockfilterindex.cpp

@@ -103,7 +103,7 @@ BlockFilterIndex::BlockFilterIndex(BlockFilterType filter_type,
     const std::string& filter_name = BlockFilterTypeName(filter_type);
     if (filter_name.empty()) throw std::invalid_argument("unknown filter_type");
 
-    fs::path path = GetDataDir() / "indexes" / "blockfilter" / filter_name;
+    fs::path path = gArgs.GetDataDirNet() / "indexes" / "blockfilter" / filter_name;
     fs::create_directories(path);
 
     m_name = filter_name + " block filter index";

src/index/coinstatsindex.cpp

@@ -98,7 +98,7 @@ std::unique_ptr<CoinStatsIndex> g_coin_stats_index;
 
 CoinStatsIndex::CoinStatsIndex(size_t n_cache_size, bool f_memory, bool f_wipe)
 {
-    fs::path path{GetDataDir() / "indexes" / "coinstats"};
+    fs::path path{gArgs.GetDataDirNet() / "indexes" / "coinstats"};
     fs::create_directories(path);
 
     m_db = std::make_unique<CoinStatsIndex::DB>(path / "db", n_cache_size, f_memory, f_wipe);

src/index/txindex.cpp

@@ -37,7 +37,7 @@ class TxIndex::DB : public BaseIndex::DB
 };
 
 TxIndex::DB::DB(size_t n_cache_size, bool f_memory, bool f_wipe) :
-    BaseIndex::DB(GetDataDir() / "indexes" / "txindex", n_cache_size, f_memory, f_wipe)
+    BaseIndex::DB(gArgs.GetDataDirNet() / "indexes" / "txindex", n_cache_size, f_memory, f_wipe)
 {}
 
 bool TxIndex::DB::ReadTxPos(const uint256 &txid, CDiskTxPos& pos) const