backport: bitcoin#24352, #25175, #25368, #25454, #25557, #25590, #25629, #25630

configure.ac

@@ -1999,6 +1999,9 @@ AC_CONFIG_LINKS([test/functional/test_runner.py:test/functional/test_runner.py])
 AC_CONFIG_LINKS([test/fuzz/test_runner.py:test/fuzz/test_runner.py])
 AC_CONFIG_LINKS([test/util/test_runner.py:test/util/test_runner.py])
 AC_CONFIG_LINKS([test/util/rpcauth-test.py:test/util/rpcauth-test.py])
+AC_CONFIG_LINKS([src/qt/Makefile:src/qt/Makefile])
+AC_CONFIG_LINKS([src/qt/test/Makefile:src/qt/test/Makefile])
+AC_CONFIG_LINKS([src/test/Makefile:src/test/Makefile])
 
 dnl boost's m4 checks do something really nasty: they export these vars. As a
 dnl result, they leak into secp256k1's configure and crazy things happen.

doc/README.md

@@ -58,6 +58,7 @@ The Dash Core repo's [root README](/README.md) contains relevant information on
 - [BIPS](bips.md)
 - [Dnsseed Policy](dnsseed-policy.md)
 - [Benchmarking](benchmarking.md)
+- [Internal Design Docs](design/)
 
 ### Resources
 * See the [Dash Developer Documentation](https://dashcore.readme.io/)
@@ -68,7 +69,6 @@ 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/design/libraries.md

@@ -0,0 +1,106 @@
+# Libraries
+
+| Name                     | Description |
+|--------------------------|-------------|
+| *libbitcoin_cli*         | RPC client functionality used by *dash-cli* executable |
+| *libbitcoin_common*      | Home for common functionality shared by different executables and libraries. Similar to *libbitcoin_util*, but higher-level (see [Dependencies](#dependencies)). |
+| *libdash_consensus*      | Stable, backwards-compatible consensus functionality used by *libbitcoin_node* and *libbitcoin_wallet* and also exposed as a [shared library](../shared-libraries.md). |
+| *libbitcoinconsensus*    | Shared library build of static *libdash_consensus* library |
+| *libbitcoin_kernel*      | Consensus engine and support library used for validation by *libbitcoin_node* and also exposed as a [shared library](../shared-libraries.md). |
+| *libbitcoinqt*           | GUI functionality used by *dash-qt* and *dash-gui* executables |
+| *libbitcoin_ipc*         | IPC functionality used by *dash-node*, *dash-wallet*, *dash-gui* executables to communicate when [`--enable-multiprocess`](multiprocess.md) is used. |
+| *libbitcoin_node*        | P2P and RPC server functionality used by *dashd* and *dash-qt* executables. |
+| *libbitcoin_util*        | Home for common functionality shared by different executables and libraries. Similar to *libbitcoin_common*, but lower-level (see [Dependencies](#dependencies)). |
+| *libbitcoin_wallet*      | Wallet functionality used by *dashd* and *dash-wallet* executables. |
+| *libbitcoin_wallet_tool* | Lower-level wallet functionality used by *dash-wallet* executable. |
+| *libbitcoin_zmq*         | [ZeroMQ](../zmq.md) functionality used by *dashd* and *dash-qt* executables. |
+
+Note: libbitcoin_kernel is a subject to be backported & dashified.
+
+## Conventions
+
+- Most libraries are internal libraries and have APIs which are completely unstable! There are few or no restrictions on backwards compatibility or rules about external dependencies. Exceptions are *libdash_consensus* and *libdash_kernel* which have external interfaces documented at [../shared-libraries.md](../shared-libraries.md).
+
+- Generally each library should have a corresponding source directory and namespace. Source code organization is a work in progress, so it is true that some namespaces are applied inconsistently, and if you look at [`libbitcoin_*_SOURCES`](../../src/Makefile.am) lists you can see that many libraries pull in files from outside their source directory. But when working with libraries, it is good to follow a consistent pattern like:
+
+  - *libbitcoin_node* code lives in `src/node/` in the `node::` namespace
+  - *libbitcoin_wallet* code lives in `src/wallet/` in the `wallet::` namespace
+  - *libbitcoin_ipc* code lives in `src/ipc/` in the `ipc::` namespace
+  - *libbitcoin_util* code lives in `src/util/` in the `util::` namespace
+  - *libdash_consensus* code lives in `src/consensus/` in the `Consensus::` namespace
+
+## Dependencies
+
+- Libraries should minimize what other libraries they depend on, and only reference symbols following the arrows shown in the dependency graph below:
+
+<table><tr><td>
+
+```mermaid
+
+%%{ init : { "flowchart" : { "curve" : "linear" }}}%%
+
+graph TD;
+
+dash-cli[dash-cli]-->libbitcoin_cli;
+
+dashd[dashd]-->libbitcoin_node;
+dashd[dashd]-->libbitcoin_wallet;
+
+dash-qt[dash-qt]-->libbitcoin_node;
+dash-qt[dash-qt]-->libbitcoinqt;
+dash-qt[dash-qt]-->libbitcoin_wallet;
+
+dash-wallet[dash-wallet]-->libbitcoin_wallet;
+dash-wallet[dash-wallet]-->libbitcoin_wallet_tool;
+
+libbitcoin_cli-->libbitcoin_common;
+libbitcoin_cli-->libbitcoin_util;
+
+libbitcoin_common-->libbitcoin_util;
+libbitcoin_common-->libdash_consensus;
+
+libbitcoin_kernel-->libdash_consensus;
+libbitcoin_kernel-->libbitcoin_util;
+
+libbitcoin_node-->libbitcoin_common;
+libbitcoin_node-->libdash_consensus;
+libbitcoin_node-->libbitcoin_kernel;
+libbitcoin_node-->libbitcoin_util;
+
+libbitcoinqt-->libbitcoin_common;
+libbitcoinqt-->libbitcoin_util;
+
+libbitcoin_wallet-->libbitcoin_common;
+libbitcoin_wallet-->libbitcoin_util;
+
+libbitcoin_wallet_tool-->libbitcoin_util;
+libbitcoin_wallet_tool-->libbitcoin_wallet;
+
+classDef bold stroke-width:2px, font-weight:bold, font-size: smaller;
+class dash-qt,dashd,dash-cli,dash-wallet bold
+```
+</td></tr><tr><td>
+
+**Dependency graph**. Arrows show linker symbol dependencies. *Consensus* lib depends on nothing. *Util* lib is depended on by everything. *Kernel* lib depends only on consensus and util.
+
+</td></tr></table>
+
+- The graph shows what _linker symbols_ (functions and variables) from each library other libraries can call and reference directly, but it is not a call graph. For example, there is no arrow connecting *libbitcoin_wallet* and *libbitcoin_node* libraries, because these libraries are intended to be modular and not depend on each other's internal implementation details. But wallet code still is still able to call node code indirectly through the `interfaces::Chain` abstract class in [`interfaces/chain.h`](../../src/interfaces/chain.h) and node code calls wallet code through the `interfaces::ChainClient` and `interfaces::Chain::Notifications` abstract classes in the same file. In general, defining abstract classes in [`src/interfaces/`](../../src/interfaces/) can be a convenient way of avoiding unwanted direct dependencies or circular dependencies between libraries.
+
+- *libdash_consensus* should be a standalone dependency that any library can depend on, and it should not depend on any other libraries itself.
+
+- *libbitcoin_util* should also be a standalone dependency that any library can depend on, and it should not depend on other internal libraries.
+
+- *libbitcoin_common* should serve a similar function as *libbitcoin_util* and be a place for miscellaneous code used by various daemon, GUI, and CLI applications and libraries to live. It should not depend on anything other than *libbitcoin_util* and *libdash_consensus*. The boundary between _util_ and _common_ is a little fuzzy but historically _util_ has been used for more generic, lower-level things like parsing hex, and _common_ has been used for bitcoin-specific, higher-level things like parsing base58. The difference between util and common is mostly important because *libbitcoin_kernel* is not supposed to depend on *libbitcoin_common*, only *libbitcoin_util*. In general, if it is ever unclear whether it is better to add code to *util* or *common*, it is probably better to add it to *common* unless it is very generically useful or useful particularly to include in the kernel.
+
+
+- *libbitcoin_kernel* should only depend on *libbitcoin_util* and *libdash_consensus*.
+
+- The only thing that should depend on *libbitcoin_kernel* internally should be *libbitcoin_node*. GUI and wallet libraries *libbitcoinqt* and *libbitcoin_wallet* in particular should not depend on *libbitcoin_kernel* and the unneeded functionality it would pull in, like block validation. To the extent that GUI and wallet code need scripting and signing functionality, they should be get able it from *libdash_consensus*, *libbitcoin_common*, and *libbitcoin_util*, instead of *libbitcoin_kernel*.
+
+- GUI, node, and wallet code internal implementations should all be independent of each other, and the *libbitcoinqt*, *libbitcoin_node*, *libbitcoin_wallet* libraries should never reference each other's symbols. They should only call each other through [`src/interfaces/`](`../../src/interfaces/`) abstract interfaces.
+
+## Work in progress
+
+- Validation code is moving from *libbitcoin_node* to *libbitcoin_kernel* as part of [The libbitcoinkernel Project #24303](https://github.com/bitcoin/bitcoin/issues/24303)
+- Source code organization is discussed in general in [Library source code organization #15732](https://github.com/bitcoin/bitcoin/issues/15732)

src/chain.h

@@ -121,7 +121,7 @@ enum BlockStatus : uint32_t {
      * If set, this indicates that the block index entry is assumed-valid.
      * Certain diagnostics will be skipped in e.g. CheckBlockIndex().
      * It almost certainly means that the block's full validation is pending
-     * on a background chainstate. See `doc/assumeutxo.md`.
+     * on a background chainstate. See `doc/design/assumeutxo.md`.
      */
     BLOCK_ASSUMED_VALID      =   256,
 };

src/net_processing.cpp

@@ -416,7 +416,7 @@ struct Peer {
     std::deque<CInv> m_getdata_requests GUARDED_BY(m_getdata_requests_mutex);
 
     /** Time of the last getheaders message to this peer */
-    std::atomic<std::chrono::seconds> m_last_getheaders_timestamp GUARDED_BY(NetEventsInterface::g_msgproc_mutex){0s};
+    NodeClock::time_point m_last_getheaders_timestamp GUARDED_BY(NetEventsInterface::g_msgproc_mutex){};
 
     explicit Peer(NodeId id, ServiceFlags our_services)
         : m_id(id)
@@ -611,15 +611,16 @@ class PeerManagerImpl final : public PeerManager
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     void BlockChecked(const CBlock& block, const BlockValidationState& state) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
-    void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) override EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex);
+    void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) override
+        EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex);
 
     /** Implement NetEventsInterface */
     void InitializeNode(CNode& node, ServiceFlags our_services) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override
-        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex, !m_most_recent_block_mutex);
+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex);
     bool SendMessages(CNode* pto) override
-        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex, !m_most_recent_block_mutex);
+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex);
 
     /** Implement PeerManager */
     void StartScheduledTasks(CScheduler& scheduler) override;
@@ -639,7 +640,7 @@ class PeerManagerImpl final : public PeerManager
     void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = "") override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex);
     void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv,
                         const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) override
-        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, g_msgproc_mutex, !m_most_recent_block_mutex);
+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex);
     void UpdateLastBlockAnnounceTime(NodeId node, int64_t time_in_seconds) override;
     bool IsBanned(NodeId pnode) override EXCLUSIVE_LOCKS_REQUIRED(cs_main, !m_peer_mutex);
     size_t GetRequestedObjectCount(NodeId nodeid) const override EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
@@ -1034,7 +1035,8 @@ class PeerManagerImpl final : public PeerManager
     /** Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). */
     CTransactionRef FindTxForGetData(const CNode* peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main);
 
-    void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(peer.m_getdata_requests_mutex, !m_most_recent_block_mutex) LOCKS_EXCLUDED(::cs_main);
+    void ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic<bool>& interruptMsgProc)
+        EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex, peer.m_getdata_requests_mutex) LOCKS_EXCLUDED(::cs_main);
 
     /** Process a new block. Perform any post-processing housekeeping */
     void ProcessBlock(CNode& from, const std::shared_ptr<const CBlock>& pblock, bool force_processing);
@@ -3017,10 +3019,11 @@ bool PeerManagerImpl::MaybeSendGetHeaders(CNode& pfrom, const std::string& msg_t
 
     const CNetMsgMaker msgMaker(pfrom.GetCommonVersion());
 
-    const auto current_time = GetTime<std::chrono::seconds>();
+    const auto current_time = NodeClock::now();
+
     // Only allow a new getheaders message to go out if we don't have a recent
     // one already in-flight
-    if (peer.m_last_getheaders_timestamp.load() < current_time - HEADERS_RESPONSE_TIME) {
+    if (current_time - peer.m_last_getheaders_timestamp > HEADERS_RESPONSE_TIME) {
         m_connman.PushMessage(&pfrom, msgMaker.Make(msg_type, locator, uint256()));
         peer.m_last_getheaders_timestamp = current_time;
         return true;
@@ -4994,7 +4997,7 @@ void PeerManagerImpl::ProcessMessage(
 
         // Assume that this is in response to any outstanding getheaders
         // request we may have sent, and clear out the time of our last request
-        peer->m_last_getheaders_timestamp = 0s;
+        peer->m_last_getheaders_timestamp = {};
 
         std::vector<CBlockHeader> headers;
 

src/univalue/include/univalue.h

@@ -100,6 +100,7 @@ class UniValue {
     std::vector<std::string> keys;
     std::vector<UniValue> values;
 
+    void checkType(const VType& expected) const;
     bool findKey(const std::string& key, size_t& retIdx) const;
     void writeArray(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const;
     void writeObject(unsigned int prettyIndent, unsigned int indentLevel, std::string& s) const;
@@ -110,19 +111,7 @@ class UniValue {
     const std::vector<std::string>& getKeys() const;
     const std::vector<UniValue>& getValues() const;
     template <typename Int>
-    auto getInt() const
-    {
-        static_assert(std::is_integral<Int>::value);
-        if (typ != VNUM) {
-            throw std::runtime_error("JSON value is not an integer as expected");
-        }
-        Int result;
-        const auto [first_nonmatching, error_condition] = std::from_chars(val.data(), val.data() + val.size(), result);
-        if (first_nonmatching != val.data() + val.size() || error_condition != std::errc{}) {
-            throw std::runtime_error("JSON integer out of range");
-        }
-        return result;
-    }
+    Int getInt() const;
     bool get_bool() const;
     const std::string& get_str() const;
     double get_real() const;
@@ -136,10 +125,23 @@ class UniValue {
 template <class It>
 void UniValue::push_backV(It first, It last)
 {
-    if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
+    checkType(VARR);
     values.insert(values.end(), first, last);
 }
 
+template <typename Int>
+Int UniValue::getInt() const
+{
+    static_assert(std::is_integral<Int>::value);
+    checkType(VNUM);
+    Int result;
+    const auto [first_nonmatching, error_condition] = std::from_chars(val.data(), val.data() + val.size(), result);
+    if (first_nonmatching != val.data() + val.size() || error_condition != std::errc{}) {
+        throw std::runtime_error("JSON integer out of range");
+    }
+    return result;
+}
+
 enum jtokentype {
     JTOK_ERR        = -1,
     JTOK_NONE       = 0,                           // eof

src/univalue/lib/univalue.cpp

@@ -103,29 +103,29 @@ void UniValue::setObject()
 
 void UniValue::push_back(const UniValue& val_)
 {
-    if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
+    checkType(VARR);
 
     values.push_back(val_);
 }
 
 void UniValue::push_backV(const std::vector<UniValue>& vec)
 {
-    if (typ != VARR) throw std::runtime_error{"JSON value is not an array as expected"};
+    checkType(VARR);
 
     values.insert(values.end(), vec.begin(), vec.end());
 }
 
 void UniValue::__pushKV(const std::string& key, const UniValue& val_)
 {
-    if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
+    checkType(VOBJ);
 
     keys.push_back(key);
     values.push_back(val_);
 }
 
 void UniValue::pushKV(const std::string& key, const UniValue& val_)
 {
-    if (typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
+    checkType(VOBJ);
 
     size_t idx;
     if (findKey(key, idx))
@@ -136,7 +136,8 @@ void UniValue::pushKV(const std::string& key, const UniValue& val_)
 
 void UniValue::pushKVs(const UniValue& obj)
 {
-    if (typ != VOBJ || obj.typ != VOBJ) throw std::runtime_error{"JSON value is not an object as expected"};
+    checkType(VOBJ);
+    obj.checkType(VOBJ);
 
     for (size_t i = 0; i < obj.keys.size(); i++)
         __pushKV(obj.keys[i], obj.values.at(i));
@@ -206,6 +207,14 @@ const UniValue& UniValue::operator[](size_t index) const
     return values.at(index);
 }
 
+void UniValue::checkType(const VType& expected) const
+{
+    if (typ != expected) {
+        throw std::runtime_error{"JSON value of type " + std::string{uvTypeName(typ)} + " is not of expected type " +
+                                 std::string{uvTypeName(expected)}};
+    }
+}
+
 const char *uvTypeName(UniValue::VType t)
 {
     switch (t) {

src/univalue/lib/univalue_get.cpp

@@ -46,8 +46,7 @@ bool ParseDouble(const std::string& str, double *out)
 
 const std::vector<std::string>& UniValue::getKeys() const
 {
-    if (typ != VOBJ)
-        throw std::runtime_error("JSON value is not an object as expected");
+    checkType(VOBJ);
     return keys;
 }
 
@@ -60,22 +59,19 @@ const std::vector<UniValue>& UniValue::getValues() const
 
 bool UniValue::get_bool() const
 {
-    if (typ != VBOOL)
-        throw std::runtime_error("JSON value is not a boolean as expected");
+    checkType(VBOOL);
     return getBool();
 }
 
 const std::string& UniValue::get_str() const
 {
-    if (typ != VSTR)
-        throw std::runtime_error("JSON value is not a string as expected");
+    checkType(VSTR);
     return getValStr();
 }
 
 double UniValue::get_real() const
 {
-    if (typ != VNUM)
-        throw std::runtime_error("JSON value is not a number as expected");
+    checkType(VNUM);
     double retval;
     if (!ParseDouble(getValStr(), &retval))
         throw std::runtime_error("JSON double out of range");
@@ -84,15 +80,12 @@ double UniValue::get_real() const
 
 const UniValue& UniValue::get_obj() const
 {
-    if (typ != VOBJ)
-        throw std::runtime_error("JSON value is not an object as expected");
+    checkType(VOBJ);
     return *this;
 }
 
 const UniValue& UniValue::get_array() const
 {
-    if (typ != VARR)
-        throw std::runtime_error("JSON value is not an array as expected");
+    checkType(VARR);
     return *this;
 }
-