diff --git a/configure.ac b/configure.ac index d216ffd04eba3..08002825e6061 100644 --- a/configure.ac +++ b/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. diff --git a/doc/README.md b/doc/README.md index 89bf34b7df652..384ba6442f660 100644 --- a/doc/README.md +++ b/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) diff --git a/doc/assumeutxo.md b/doc/design/assumeutxo.md similarity index 100% rename from doc/assumeutxo.md rename to doc/design/assumeutxo.md diff --git a/doc/design/libraries.md b/doc/design/libraries.md new file mode 100644 index 0000000000000..548e8c8618477 --- /dev/null +++ b/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: + +
+ +```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 +``` +
+ +**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. + +
+ +- 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) diff --git a/doc/multiprocess.md b/doc/design/multiprocess.md similarity index 100% rename from doc/multiprocess.md rename to doc/design/multiprocess.md diff --git a/src/chain.h b/src/chain.h index 2fdac98e85c05..47d1e3096774f 100644 --- a/src/chain.h +++ b/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, }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bee912118e31a..c80bb36c687be 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -416,7 +416,7 @@ struct Peer { std::deque m_getdata_requests GUARDED_BY(m_getdata_requests_mutex); /** Time of the last getheaders message to this peer */ - std::atomic 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& pblock) override EXCLUSIVE_LOCKS_REQUIRED(!m_most_recent_block_mutex); + void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& 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& 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& 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& 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& 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& 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(); + 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 headers; diff --git a/src/qt/Makefile b/src/qt/Makefile new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index ec4a1aae64b8b..372b0ed7c6f4c 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -100,6 +100,7 @@ class UniValue { std::vector keys; std::vector 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& getKeys() const; const std::vector& getValues() const; template - auto getInt() const - { - static_assert(std::is_integral::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 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 +Int UniValue::getInt() const +{ + static_assert(std::is_integral::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 diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp index a00c9f3481464..4c8066767a94e 100644 --- a/src/univalue/lib/univalue.cpp +++ b/src/univalue/lib/univalue.cpp @@ -103,21 +103,21 @@ 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& 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_); @@ -125,7 +125,7 @@ void UniValue::__pushKV(const std::string& key, const UniValue& 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) { diff --git a/src/univalue/lib/univalue_get.cpp b/src/univalue/lib/univalue_get.cpp index 9bbdb1fe69b92..5c58f388ddc5c 100644 --- a/src/univalue/lib/univalue_get.cpp +++ b/src/univalue/lib/univalue_get.cpp @@ -46,8 +46,7 @@ bool ParseDouble(const std::string& str, double *out) const std::vector& 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::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; } - diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 1e3cf53c52930..d0984b5264f62 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -205,15 +205,15 @@ RPCHelpMan abortrescan() RPCHelpMan importaddress() { return RPCHelpMan{"importaddress", - "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" + "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "Hint: use importmulti to import more than one address.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" "as change, and not show up in many RPCs.\n" - "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" for descriptor wallets.\n", - { + "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" with \"addr(X)\" for descriptor wallets.\n", + { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Dash address (or hex-encoded script)"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index cc795b2d74051..6ccd1d1471357 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -100,7 +100,7 @@ LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_cr spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); } if (!spk_man) { - throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); + throw JSONRPCError(RPC_WALLET_ERROR, "Only legacy wallets are supported by this command"); } return *spk_man; } @@ -109,7 +109,7 @@ const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wal { const LegacyScriptPubKeyMan* spk_man = wallet.GetLegacyScriptPubKeyMan(); if (!spk_man) { - throw JSONRPCError(RPC_WALLET_ERROR, "This type of wallet does not support this command"); + throw JSONRPCError(RPC_WALLET_ERROR, "Only legacy wallets are supported by this command"); } return *spk_man; } diff --git a/test/functional/feature_minchainwork.py b/test/functional/feature_minchainwork.py index cb8aee912baf9..9dd8655c5acb2 100755 --- a/test/functional/feature_minchainwork.py +++ b/test/functional/feature_minchainwork.py @@ -85,7 +85,7 @@ def run_test(self): msg.hashstop = 0 peer.send_and_ping(msg) time.sleep(5) - assert ("headers" not in peer.last_message or len(peer.last_message["headers"].headers) == 0) + assert "headers" not in peer.last_message or len(peer.last_message["headers"].headers) == 0 self.log.info("Generating one more block") self.generate(self.nodes[0], 1) diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 6fb6e2d1bef6b..1f9bf2c605711 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -120,7 +120,7 @@ def run_test(self): txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000' # Test `prioritisetransaction` invalid `fee_delta` - assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo') + assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", self.nodes[0].prioritisetransaction, txid=txid, fee_delta='foo') self.test_diamond() diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 6ffaaa75f0f90..cf6ef0f7aeeb3 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -263,12 +263,12 @@ def _test_getchaintxstats(self): assert_raises_rpc_error(-1, 'getchaintxstats', self.nodes[0].getchaintxstats, 0, '', 0) # Test `getchaintxstats` invalid `nblocks` - assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].getchaintxstats, '') + assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", self.nodes[0].getchaintxstats, '') assert_raises_rpc_error(-8, "Invalid block count: should be between 0 and the block's height - 1", self.nodes[0].getchaintxstats, -1) assert_raises_rpc_error(-8, "Invalid block count: should be between 0 and the block's height - 1", self.nodes[0].getchaintxstats, self.nodes[0].getblockcount()) # Test `getchaintxstats` invalid `blockhash` - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getchaintxstats, blockhash=0) + assert_raises_rpc_error(-1, "JSON value of type number is not of expected type string", self.nodes[0].getchaintxstats, blockhash=0) assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 1, for '0')", self.nodes[0].getchaintxstats, blockhash='0') assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getchaintxstats, blockhash='ZZZ0000000000000000000000000000000000000000000000000000000000000') assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0000000000000000000000000000000000000000000000000000000000000000') @@ -543,7 +543,7 @@ def assert_vin_does_not_contain_prevout(verbosity): datadir = get_datadir_path(self.options.tmpdir, 0) self.log.info("Test that getblock with invalid verbosity type returns proper error message") - assert_raises_rpc_error(-1, "JSON value is not an integer as expected", node.getblock, blockhash, "2") + assert_raises_rpc_error(-1, "JSON value of type string is not of expected type number", node.getblock, blockhash, "2") def move_block_file(old, new): old_path = os.path.join(datadir, self.chain, 'blocks', old) diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py index c473dc8707f56..795f3856b78a8 100755 --- a/test/functional/rpc_help.py +++ b/test/functional/rpc_help.py @@ -92,7 +92,7 @@ def test_categories(self): assert_raises_rpc_error(-1, 'help', node.help, 'foo', 'bar', 'foobar') # invalid argument - assert_raises_rpc_error(-1, 'JSON value is not a string as expected', node.help, 0) + assert_raises_rpc_error(-1, "JSON value of type number is not of expected type string", node.help, 0) # help of unknown command assert_equal(node.help('foo'), 'help: unknown command: foo') diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index ecbd706fc014c..338a3eb91fcc5 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -128,14 +128,14 @@ def getrawtransaction_tests(self): # 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose for value in ["True", "False"]: - assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value) - assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransactionmulti, transactions={"0":[txId]}, verbose=value) + assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransaction, txid=txId, verbose=value) + assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransactionmulti, transactions={"0":[txId]}, verbose=value) # 7. invalid parameters - supply txid and empty array - assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, []) + assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransaction, txId, []) # 8. invalid parameters - supply txid and empty dict - assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {}) + assert_raises_rpc_error(-1, "not of expected type bool", self.nodes[n].getrawtransaction, txId, {}) # Make a tx by sending, then generate 2 blocks; block1 has the tx in it tx = self.wallet.send_self_transfer(from_node=self.nodes[2])['txid'] @@ -157,7 +157,7 @@ def getrawtransaction_tests(self): # We should not get the tx if we provide an unrelated block assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2) # An invalid block hash should raise the correct errors - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True) + assert_raises_rpc_error(-1, "JSON value of type bool is not of expected type string", self.nodes[n].getrawtransaction, txid=tx, blockhash=True) assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar") assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234") foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000" @@ -187,8 +187,8 @@ def createrawtransaction_tests(self): # Test `createrawtransaction` invalid `inputs` assert_raises_rpc_error(-3, "JSON value of type string is not of expected type array", self.nodes[0].createrawtransaction, 'foo', {}) - assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {}) - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {}) + assert_raises_rpc_error(-1, "JSON value of type string is not of expected type object", self.nodes[0].createrawtransaction, ['foo'], {}) + assert_raises_rpc_error(-1, "JSON value of type null is not of expected type string", self.nodes[0].createrawtransaction, [{}], {}) assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {}) txid = "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844" assert_raises_rpc_error(-8, f"txid must be hexadecimal string (not '{txid}')", self.nodes[0].createrawtransaction, [{'txid': txid}], {}) @@ -213,7 +213,7 @@ def createrawtransaction_tests(self): # Test `createrawtransaction` invalid `outputs` address = getnewdestination()[2] - assert_raises_rpc_error(-1, "JSON value is not an array as expected", self.nodes[0].createrawtransaction, [], 'foo') + assert_raises_rpc_error(-1, "JSON value of type string is not of expected type array", self.nodes[0].createrawtransaction, [], 'foo') self.nodes[0].createrawtransaction(inputs=[], outputs={}) # Should not throw for backwards compatibility self.nodes[0].createrawtransaction(inputs=[], outputs=[]) assert_raises_rpc_error(-8, "Data must be hexadecimal string", self.nodes[0].createrawtransaction, [], {'data': 'foo'}) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 5194b06cb2cd7..d9ad16b25459d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -681,11 +681,11 @@ def stop_node(self, i, expected_stderr='', wait=0): """Stop a dashd test node""" self.nodes[i].stop_node(expected_stderr=expected_stderr, wait=wait) - def stop_nodes(self, expected_stderr='', wait=0): + def stop_nodes(self, wait=0): """Stop multiple dashd test nodes""" for node in self.nodes: # Issue RPC to stop nodes - node.stop_node(expected_stderr=expected_stderr, wait=wait, wait_until_stopped=False) + node.stop_node(wait=wait, wait_until_stopped=False) for node in self.nodes: # Wait for nodes to stop diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index c33facaa4405e..c478a6e164217 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -453,7 +453,7 @@ def run_test(self): assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "1f-4") # This will raise an exception since generate does not accept a string - assert_raises_rpc_error(-1, "not an integer", self.generate, self.nodes[0], "2") + assert_raises_rpc_error(-1, "not of expected type number", self.generate, self.nodes[0], "2") if not self.options.descriptors: diff --git a/test/functional/wallet_descriptor.py b/test/functional/wallet_descriptor.py index 4e76904d15ac5..eba75ff941083 100755 --- a/test/functional/wallet_descriptor.py +++ b/test/functional/wallet_descriptor.py @@ -72,15 +72,15 @@ def run_test(self): # Make sure things are disabled self.log.info("Test disabled RPCs") - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.importprivkey, "cVpF924EspNh8KjYsfhgY96mmxvT6DgdWiTYMtMjuM74hJaU5psW") - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.importpubkey, send_wrpc.getaddressinfo(send_wrpc.getnewaddress())) - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.importaddress, recv_wrpc.getnewaddress()) - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.importmulti, []) - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.addmultisigaddress, 1, [recv_wrpc.getnewaddress()]) - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.dumpprivkey, recv_wrpc.getnewaddress()) - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.dumpwallet, 'wallet.dump') - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.importwallet, 'wallet.dump') - assert_raises_rpc_error(-4, "This type of wallet does not support this command", recv_wrpc.rpc.sethdseed) + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importprivkey, "cVpF924EspNh8KjYsfhgY96mmxvT6DgdWiTYMtMjuM74hJaU5psW") + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importpubkey, send_wrpc.getaddressinfo(send_wrpc.getnewaddress())) + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importaddress, recv_wrpc.getnewaddress()) + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importmulti, []) + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.addmultisigaddress, 1, [recv_wrpc.getnewaddress()]) + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.dumpprivkey, recv_wrpc.getnewaddress()) + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.dumpwallet, 'wallet.dump') + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.importwallet, 'wallet.dump') + assert_raises_rpc_error(-4, "Only legacy wallets are supported by this command", recv_wrpc.rpc.sethdseed) self.log.info("Test encryption") # Get the master fingerprint before encrypt diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index b6cf513157817..665529f72662f 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -184,8 +184,8 @@ def run_test(self): # Sethdseed parameter validity assert_raises_rpc_error(-1, 'sethdseed', self.nodes[0].sethdseed, False, new_seed, 0) assert_raises_rpc_error(-5, "Invalid private key", wallet_no_seed.sethdseed, False, "not_wif") - assert_raises_rpc_error(-1, "JSON value is not a boolean as expected", wallet_no_seed.sethdseed, "Not_bool") - assert_raises_rpc_error(-1, "JSON value is not a string as expected", wallet_no_seed.sethdseed, False, True) + assert_raises_rpc_error(-1, "JSON value of type string is not of expected type bool", wallet_no_seed.sethdseed, "Not_bool") + assert_raises_rpc_error(-1, "JSON value of type bool is not of expected type string", wallet_no_seed.sethdseed, False, True) assert_raises_rpc_error(-5, "Already have this key", wallet_no_seed.sethdseed, False, non_hd_key) self.log.info('Test sethdseed restoring with keys outside of the initial keypool') diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 9be0a3031b5cd..3902129aa0884 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -353,7 +353,7 @@ def wallet_file(name): self.log.info("Test dynamic wallet unloading") # Test `unloadwallet` errors - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].unloadwallet) + assert_raises_rpc_error(-1, "JSON value of type null is not of expected type string", self.nodes[0].unloadwallet) assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", self.nodes[0].unloadwallet, "dummy") assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", node.get_wallet_rpc("dummy").unloadwallet) assert_raises_rpc_error(-8, "RPC endpoint wallet and wallet_name parameter specify different wallets", w1.unloadwallet, "w2"),