diff options
66 files changed, 1021 insertions, 311 deletions
diff --git a/.travis/lint_06_script.sh b/.travis/lint_06_script.sh index 6191d82571..701e6d8005 100755 --- a/.travis/lint_06_script.sh +++ b/.travis/lint_06_script.sh @@ -19,6 +19,7 @@ test/lint/check-rpc-mappings.py . test/lint/lint-all.sh if [ "$TRAVIS_REPO_SLUG" = "bitcoin/bitcoin" -a "$TRAVIS_EVENT_TYPE" = "cron" ]; then + git log --merges --before="2 days ago" -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit while read -r LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys && - travis_wait 50 contrib/verify-commits/verify-commits.py; + travis_wait 50 contrib/verify-commits/verify-commits.py --clean-merge=2; fi diff --git a/Makefile.am b/Makefile.am index 7eb4ea2f52..9a6e15f0dd 100644 --- a/Makefile.am +++ b/Makefile.am @@ -19,6 +19,7 @@ endif BITCOIND_BIN=$(top_builddir)/src/$(BITCOIN_DAEMON_NAME)$(EXEEXT) BITCOIN_QT_BIN=$(top_builddir)/src/qt/$(BITCOIN_GUI_NAME)$(EXEEXT) BITCOIN_CLI_BIN=$(top_builddir)/src/$(BITCOIN_CLI_NAME)$(EXEEXT) +BITCOIN_TX_BIN=$(top_builddir)/src/$(BITCOIN_TX_NAME)$(EXEEXT) BITCOIN_WIN_INSTALLER=$(PACKAGE)-$(PACKAGE_VERSION)-win$(WINDOWS_BITS)-setup$(EXEEXT) empty := @@ -74,6 +75,7 @@ $(BITCOIN_WIN_INSTALLER): all-recursive STRIPPROG="$(STRIP)" $(INSTALL_STRIP_PROGRAM) $(BITCOIND_BIN) $(top_builddir)/release STRIPPROG="$(STRIP)" $(INSTALL_STRIP_PROGRAM) $(BITCOIN_QT_BIN) $(top_builddir)/release STRIPPROG="$(STRIP)" $(INSTALL_STRIP_PROGRAM) $(BITCOIN_CLI_BIN) $(top_builddir)/release + STRIPPROG="$(STRIP)" $(INSTALL_STRIP_PROGRAM) $(BITCOIN_TX_BIN) $(top_builddir)/release @test -f $(MAKENSIS) && $(MAKENSIS) -V2 $(top_builddir)/share/setup.nsi || \ echo error: could not build $@ @echo built $@ @@ -167,6 +169,9 @@ $(BITCOIND_BIN): FORCE $(BITCOIN_CLI_BIN): FORCE $(MAKE) -C src $(@F) +$(BITCOIN_TX_BIN): FORCE + $(MAKE) -C src $(@F) + if USE_LCOV LCOV_FILTER_PATTERN=-p "/usr/include/" -p "/usr/lib/" -p "src/leveldb/" -p "src/bench/" -p "src/univalue" -p "src/crypto/ctaes" -p "src/secp256k1" diff --git a/build-aux/m4/bitcoin_qt.m4 b/build-aux/m4/bitcoin_qt.m4 index 05df8621d2..90a2cd9875 100644 --- a/build-aux/m4/bitcoin_qt.m4 +++ b/build-aux/m4/bitcoin_qt.m4 @@ -276,7 +276,7 @@ AC_DEFUN([_BITCOIN_QT_CHECK_QT5],[ #endif ]], [[ - #if QT_VERSION < 0x050000 || QT_VERSION_MAJOR < 5 + #if QT_VERSION < 0x050200 || QT_VERSION_MAJOR < 5 choke #endif ]])], diff --git a/doc/developer-notes.md b/doc/developer-notes.md index fd5cc78297..bb477d4be0 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -827,7 +827,16 @@ To create a scripted-diff: - `-BEGIN VERIFY SCRIPT-` - `-END VERIFY SCRIPT-` -The scripted-diff is verified by the tool `test/lint/commit-script-check.sh` +The scripted-diff is verified by the tool `test/lint/commit-script-check.sh`. The tool's default behavior when supplied +with a commit is to verify all scripted-diffs from the beginning of time up to said commit. Internally, the tool passes +the first supplied argument to `git rev-list --reverse` to determine which commits to verify script-diffs for, ignoring +commits that don't conform to the commit message format described above. + +For development, it might be more convenient to verify all scripted-diffs in a range `A..B`, for example: + +```bash +test/lint/commit-script-check.sh origin/master..HEAD +``` Commit [`bb81e173`](https://github.com/bitcoin/bitcoin/commit/bb81e173) is an example of a scripted-diff. diff --git a/doc/release-notes-13152.md b/doc/release-notes-13152.md deleted file mode 100644 index ace56f4d1b..0000000000 --- a/doc/release-notes-13152.md +++ /dev/null @@ -1,5 +0,0 @@ -New RPC methods ------------- - -- `getnodeaddresses` returns peer addresses known to this node. It may be used to connect to nodes over TCP without using the DNS seeds. -- `listwalletdir` returns a list of wallets in the wallet directory which is configured with `-walletdir` parameter. diff --git a/doc/release-notes-14023.md b/doc/release-notes-14023.md deleted file mode 100644 index 18ea6f26d0..0000000000 --- a/doc/release-notes-14023.md +++ /dev/null @@ -1,8 +0,0 @@ -Account API removed -------------------- - -The 'account' API was deprecated in v0.17 and has been fully removed in v0.18. -The 'label' API was introduced in v0.17 as a replacement for accounts. - -See the release notes from v0.17 for a full description of the changes from the -'account' API to the 'label' API. diff --git a/doc/release-notes-14060.md b/doc/release-notes-14060.md new file mode 100644 index 0000000000..2cc5ab49fb --- /dev/null +++ b/doc/release-notes-14060.md @@ -0,0 +1,21 @@ +Configuration +------------- + +The outbound message high water mark of the ZMQ PUB sockets are now +configurable via the options: + +`-zmqpubhashtxhwm=n` + +`-zmqpubhashblockhwm=n` + +`-zmqpubrawblockhwm=n` + +`-zmqpubrawtxhwm=n` + +Each high water mark value must be an integer greater than or equal to 0. +The high water mark limits the maximum number of messages that ZMQ will +queue in memory for any single subscriber. A value of 0 means no limit. +When not specified, the default value continues to be 1000. +When a ZMQ PUB socket reaches its high water mark for a subscriber, then +additional messages to the subscriber are dropped until the number of +queued messages again falls below the high water mark value. diff --git a/doc/release-notes-14282.md b/doc/release-notes-14282.md deleted file mode 100644 index 900ca04324..0000000000 --- a/doc/release-notes-14282.md +++ /dev/null @@ -1,9 +0,0 @@ -Low-level RPC changes ----------------------- - -`-usehd` was removed in version 0.16. From that version onwards, all new -wallets created are hierarchical deterministic wallets. Version 0.18 makes -specifying `-usehd` invalid config. - -`ischange` field of boolean type that shows if an address was used for change -output was added to `getaddressinfo` method response. diff --git a/doc/release-notes-14296.md b/doc/release-notes-14296.md deleted file mode 100644 index f7c52baace..0000000000 --- a/doc/release-notes-14296.md +++ /dev/null @@ -1,5 +0,0 @@ -addwitnessaddress RPC method removed ------------------------------------- - -The `addwitnessaddress` RPC was added for segwit testing in version 0.13.0. It -was deprecated in version 0.16.0. This version fully removes the RPC method. diff --git a/doc/release-notes-14454.md b/doc/release-notes-14454.md deleted file mode 100644 index dd2c6c7ced..0000000000 --- a/doc/release-notes-14454.md +++ /dev/null @@ -1,6 +0,0 @@ -Low-level RPC changes ----------------------- - -The `importmulti` RPC has been updated to support P2WSH, P2WPKH, P2SH-P2WPKH, -P2SH-P2WSH. Each request now accepts an additional `witnessscript` to be used -for P2WSH or P2SH-P2WSH. diff --git a/doc/release-notes-14468.md b/doc/release-notes-14468.md deleted file mode 100644 index fb0243aba8..0000000000 --- a/doc/release-notes-14468.md +++ /dev/null @@ -1,15 +0,0 @@ -Wallet `generate` RPC method deprecated ---------------------------------------- - -The wallet's `generate` RPC method has been deprecated and will be fully -removed in v0.19. - -`generate` is only used for testing. The RPC call reaches across multiple -subsystems (wallet and mining), so is deprecated to simplify the wallet-node -interface. Projects that are using `generate` for testing purposes should -transition to using the `generatetoaddress` call, which does not require or use -the wallet component. Calling `generatetoaddress` with an address returned by -`getnewaddress` gives the same functionality as the old `generate` method. - -To continue using `generate` in v0.18, restart bitcoind with the -`-deprecatedrpc=generate` configuration. diff --git a/doc/release-notes-pr13381.md b/doc/release-notes-pr13381.md new file mode 100644 index 0000000000..75faad9906 --- /dev/null +++ b/doc/release-notes-pr13381.md @@ -0,0 +1,29 @@ +RPC importprivkey: new label behavior +------------------------------------- + +Previously, `importprivkey` automatically added the default empty label +("") to all addresses associated with the imported private key. Now it +defaults to using any existing label for those addresses. For example: + +- Old behavior: you import a watch-only address with the label "cold + wallet". Later, you import the corresponding private key using the + default settings. The address's label is changed from "cold wallet" + to "". + +- New behavior: you import a watch-only address with the label "cold + wallet". Later, you import the corresponding private key using the + default settings. The address's label remains "cold wallet". + +In both the previous and current case, if you directly specify a label +during the import, that label will override whatever previous label the +addresses may have had. Also in both cases, if none of the addresses +previously had a label, they will still receive the default empty label +(""). Examples: + +- You import a watch-only address with the label "temporary". Later you + import the corresponding private key with the label "final". The + address's label will be changed to "final". + +- You use the default settings to import a private key for an address that + was not previously in the wallet. Its addresses will receive the default + empty label (""). diff --git a/doc/release-notes.md b/doc/release-notes.md index 2044a50098..f5c139e3f1 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -53,8 +53,14 @@ the Linux kernel, macOS 10.10+, and Windows 7 and newer (Windows XP is not suppo Bitcoin Core should also work on most other Unix-like systems but is not frequently tested on them. -From 0.17.0 onwards macOS <10.10 is no longer supported. 0.17.0 is built using Qt 5.9.x, which doesn't -support versions of macOS older than 10.10. +From 0.17.0 onwards, macOS <10.10 is no longer supported. 0.17.0 is +built using Qt 5.9.x, which doesn't support versions of macOS older than +10.10. Additionally, Bitcoin Core does not yet change appearance when +macOS "dark mode" is activated. + +In addition to previously-supported CPU platforms, this release's +pre-compiled distribution also provides binaries for the RISC-V +platform. Notable changes =============== @@ -69,9 +75,137 @@ nodes. The option will now by default be off for improved privacy and security as well as reduced upload usage. The option can explicitly be turned on for local-network debugging purposes. -Example item +Documentation +------------- + +- A new short + [document](https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md) + about the JSON-RPC interface describes cases where the results of an + RPC might contain inconsistencies between data sourced from different + subsystems, such as wallet state and mempool state. A note is added + to the [REST interface documentation](https://github.com/bitcoin/bitcoin/blob/master/doc/REST-interface.md) + indicating that the same rules apply. + +- A new [document](https://github.com/bitcoin/bitcoin/blob/master/doc/bitcoin-conf.md) + about the `bitcoin.conf` file describes how to use it to configure + Bitcoin Core. + +- A new document introduces Bitcoin Core's BIP174 + [Partially-Signed Bitcoin Transactions (PSBT)](https://github.com/bitcoin/bitcoin/blob/master/doc/psbt.md) + interface, which is used to allow multiple programs to collaboratively + work to create, sign, and broadcast new transactions. This is useful + for offline (cold storage) wallets, multisig wallets, coinjoin + implementations, and many other cases where two or more programs need + to interact to generate a complete transaction. + +- The [output script descriptor](https://github.com/bitcoin/bitcoin/blob/master/doc/descriptors.md) + documentation has been updated with information about new features in + this still-developing language for describing the output scripts that + a wallet or other program wants to receive notifications for, such as + which addresses it wants to know received payments. The language is + currently used in the `scantxoutset` RPC and is expected to be adapted + to other RPCs and to the underlying wallet structure. + +Build system changes +-------------------- + +- A new `--disable-bip70` option may be passed to `./configure` to + prevent Bitcoin-Qt from being built with support for the BIP70 payment + protocol or from linking libssl. As the payment protocol has exposed + Bitcoin Core to libssl vulnerabilities in the past, builders who don't + need BIP70 support are encouraged to use this option to reduce their + exposure to future vulnerabilities. + +Deprecated or removed RPCs +-------------------------- + +- The `signrawtransaction` RPC is removed after being deprecated and + hidden behind a special configuration option in version 0.17.0. + +- The 'account' API is removed after being deprecated in v0.17. The + 'label' API was introduced in v0.17 as a replacement for accounts. + See the [release notes from v0.17](https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.17.0.md#label-and-account-apis-for-wallet) + for a full description of the changes from the 'account' API to the + 'label' API. + +- The `addwitnessaddress` RPC is removed after being deprecated in + version 0.13.0. + +- The wallet's `generate` RPC method is deprecated and will be fully + removed in a subsequent major version. This RPC is only used for + testing, but its implementation reached across multiple subsystems + (wallet and mining), so it is being deprecated to simplify the + wallet-node interface. Projects that are using `generate` for testing + purposes should transition to using the `generatetoaddress` RPC, which + does not require or use the wallet component. Calling + `generatetoaddress` with an address returned by the `getnewaddress` + RPC gives the same functionality as the old `generate` RPC. To + continue using `generate` in this version, restart bitcoind with the + `-deprecatedrpc=generate` configuration option. + +New RPCs +-------- + +- A new `getnodeaddresses` RPC returns peer addresses known to this + node. It may be used to find nodes to connect to without using a DNS + seeder. + +- A new `listwalletdir` RPC returns a list of wallets in the wallet + directory (either the default wallet directory or the directory + configured by the `-walletdir` parameter). + +Updated RPCs ------------ +Note: some low-level RPC changes mainly useful for testing are described +in the Low-level Changes section below. + +- The `getpeerinfo` RPC now returns an additional "minfeefilter" field + set to the peer's BIP133 fee filter. You can use this to detect that + you have peers that are willing to accept transactions below the + default minimum relay fee. + +- The mempool RPCs, such as `getrawmempool` with `verbose=true`, now + return an additional "bip125-replaceable" value indicating whether the + transaction (or its unconfirmed ancestors) opts-in to asking nodes and + miners to replace it with a higher-feerate transaction spending any of + the same inputs. + +- The `settxfee` RPC previously silently ignored attempts to set the fee + below the allowed minimums. It now prints a warning. The special + value of "0" may still be used to request the minimum value. + +- The `getaddressinfo` RPC now provides an `ischange` field indicating + whether the wallet used the address in a change output. + +- The `importmulti` RPC has been updated to support P2WSH, P2WPKH, + P2SH-P2WPKH, and P2SH-P2WSH. Requests for P2WSH and P2SH-P2WSH accept + an additional `witnessscript` parameter. + +Low-level changes +================= + +RPC +--- + +- The `submitblock` RPC previously returned the reason a rejected block + was invalid the first time it processed that block but returned a + generic "duplicate" rejection message on subsequent occasions it + processed the same block. It now always returns the fundamental + reason for rejecting an invalid block and only returns "duplicate" for + valid blocks it has already accepted. + +- A new `submitheader` RPC allows submitting block headers independently + from their block. This is likely only useful for testing. + +Configuration +------------- + +- The `-usehd` configuration option was removed in version 0.16. From + that version onwards, all new wallets created are hierarchical + deterministic wallets. This release makes specifying `-usehd` an + invalid configuration option. + Credits ======= diff --git a/share/setup.nsi.in b/share/setup.nsi.in index b58a84e02d..6542370f97 100644 --- a/share/setup.nsi.in +++ b/share/setup.nsi.in @@ -80,6 +80,7 @@ Section -Main SEC0000 SetOutPath $INSTDIR\daemon File @abs_top_srcdir@/release/@BITCOIN_DAEMON_NAME@@EXEEXT@ File @abs_top_srcdir@/release/@BITCOIN_CLI_NAME@@EXEEXT@ + File @abs_top_srcdir@/release/@BITCOIN_TX_NAME@@EXEEXT@ SetOutPath $INSTDIR\doc File /r /x Makefile* @abs_top_srcdir@/doc\*.* SetOutPath $INSTDIR diff --git a/src/Makefile.am b/src/Makefile.am index 8dd0d31839..09daaebd23 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -95,6 +95,7 @@ endif BITCOIN_CORE_H = \ addrdb.h \ addrman.h \ + attributes.h \ base58.h \ bech32.h \ bloom.h \ diff --git a/src/attributes.h b/src/attributes.h new file mode 100644 index 0000000000..45099bd8b8 --- /dev/null +++ b/src/attributes.h @@ -0,0 +1,22 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2018 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_ATTRIBUTES_H +#define BITCOIN_ATTRIBUTES_H + +#if defined(__has_cpp_attribute) +# if __has_cpp_attribute(nodiscard) +# define NODISCARD [[nodiscard]] +# endif +#endif +#ifndef NODISCARD +# if defined(_MSC_VER) && _MSC_VER >= 1700 +# define NODISCARD _Check_return_ +# else +# define NODISCARD __attribute__((warn_unused_result)) +# endif +#endif + +#endif // BITCOIN_ATTRIBUTES_H diff --git a/src/base58.h b/src/base58.h index 9d3f90652e..d6e0299a1e 100644 --- a/src/base58.h +++ b/src/base58.h @@ -14,6 +14,8 @@ #ifndef BITCOIN_BASE58_H #define BITCOIN_BASE58_H +#include <attributes.h> + #include <string> #include <vector> @@ -33,13 +35,13 @@ std::string EncodeBase58(const std::vector<unsigned char>& vch); * return true if decoding is successful. * psz cannot be nullptr. */ -bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet); +NODISCARD bool DecodeBase58(const char* psz, std::vector<unsigned char>& vchRet); /** * Decode a base58-encoded string (str) into a byte vector (vchRet). * return true if decoding is successful. */ -bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet); +NODISCARD bool DecodeBase58(const std::string& str, std::vector<unsigned char>& vchRet); /** * Encode a byte vector into a base58-encoded string, including checksum @@ -50,12 +52,12 @@ std::string EncodeBase58Check(const std::vector<unsigned char>& vchIn); * Decode a base58-encoded string (psz) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet); +NODISCARD bool DecodeBase58Check(const char* psz, std::vector<unsigned char>& vchRet); /** * Decode a base58-encoded string (str) that includes a checksum into a byte * vector (vchRet), return true if decoding is successful */ -bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet); +NODISCARD bool DecodeBase58Check(const std::string& str, std::vector<unsigned char>& vchRet); #endif // BITCOIN_BASE58_H diff --git a/src/bench/base58.cpp b/src/bench/base58.cpp index a555376e40..e7702ec461 100644 --- a/src/bench/base58.cpp +++ b/src/bench/base58.cpp @@ -49,7 +49,7 @@ static void Base58Decode(benchmark::State& state) const char* addr = "17VZNX1SN5NtKa8UQFxwQbFeFc3iqRYhem"; std::vector<unsigned char> vch; while (state.KeepRunning()) { - DecodeBase58(addr, vch); + (void) DecodeBase58(addr, vch); } } diff --git a/src/blockencodings.h b/src/blockencodings.h index fad1f56f54..0c2b83ebcf 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -52,12 +52,12 @@ public: } } - uint16_t offset = 0; + int32_t offset = 0; for (size_t j = 0; j < indexes.size(); j++) { - if (uint64_t(indexes[j]) + uint64_t(offset) > std::numeric_limits<uint16_t>::max()) + if (int32_t(indexes[j]) + offset > std::numeric_limits<uint16_t>::max()) throw std::ios_base::failure("indexes overflowed 16 bits"); indexes[j] = indexes[j] + offset; - offset = indexes[j] + 1; + offset = int32_t(indexes[j]) + 1; } } else { for (size_t i = 0; i < indexes.size(); i++) { @@ -186,6 +186,9 @@ public: READWRITE(prefilledtxn); + if (BlockTxCount() > std::numeric_limits<uint16_t>::max()) + throw std::ios_base::failure("indexes overflowed 16 bits"); + if (ser_action.ForRead()) FillShortTxIDSelector(); } diff --git a/src/core_io.h b/src/core_io.h index 2c3b64d81e..6f87161f46 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -6,6 +6,7 @@ #define BITCOIN_CORE_IO_H #include <amount.h> +#include <attributes.h> #include <string> #include <vector> @@ -22,8 +23,8 @@ class UniValue; // core_read.cpp CScript ParseScript(const std::string& s); std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false); -bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true); -bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); +NODISCARD bool DecodeHexTx(CMutableTransaction& tx, const std::string& hex_tx, bool try_no_witness = false, bool try_witness = true); +NODISCARD bool DecodeHexBlk(CBlock&, const std::string& strHexBlk); bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); /** @@ -36,7 +37,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string& hex_header); */ bool ParseHashStr(const std::string& strHex, uint256& result); std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::string& strName); -bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error); +NODISCARD bool DecodePSBT(PartiallySignedTransaction& psbt, const std::string& base64_tx, std::string& error); int ParseSighashString(const UniValue& sighash); // core_write.cpp diff --git a/src/miner.cpp b/src/miner.cpp index feb86cab66..96c9cd6d2a 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -70,9 +70,8 @@ static BlockAssembler::Options DefaultOptions() // If -blockmaxweight is not given, limit to DEFAULT_BLOCK_MAX_WEIGHT BlockAssembler::Options options; options.nBlockMaxWeight = gArgs.GetArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT); - if (gArgs.IsArgSet("-blockmintxfee")) { - CAmount n = 0; - ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n); + CAmount n = 0; + if (gArgs.IsArgSet("-blockmintxfee") && ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n)) { options.blockMinFeeRate = CFeeRate(n); } else { options.blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); diff --git a/src/outputtype.h b/src/outputtype.h index 4c4d93bc8b..6c30fd1950 100644 --- a/src/outputtype.h +++ b/src/outputtype.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_OUTPUTTYPE_H #define BITCOIN_OUTPUTTYPE_H +#include <attributes.h> #include <keystore.h> #include <script/standard.h> @@ -26,7 +27,7 @@ enum class OutputType { CHANGE_AUTO, }; -bool ParseOutputType(const std::string& str, OutputType& output_type); +NODISCARD bool ParseOutputType(const std::string& str, OutputType& output_type); const std::string& FormatOutputType(OutputType type); /** diff --git a/src/qt/askpassphrasedialog.cpp b/src/qt/askpassphrasedialog.cpp index a0270daf99..821c23c467 100644 --- a/src/qt/askpassphrasedialog.cpp +++ b/src/qt/askpassphrasedialog.cpp @@ -152,14 +152,15 @@ void AskPassphraseDialog::accept() } } break; case Unlock: - if(!model->setWalletLocked(false, oldpass)) - { - QMessageBox::critical(this, tr("Wallet unlock failed"), - tr("The passphrase entered for the wallet decryption was incorrect.")); - } - else - { - QDialog::accept(); // Success + try { + if (!model->setWalletLocked(false, oldpass)) { + QMessageBox::critical(this, tr("Wallet unlock failed"), + tr("The passphrase entered for the wallet decryption was incorrect.")); + } else { + QDialog::accept(); // Success + } + } catch (const std::runtime_error& e) { + QMessageBox::critical(this, tr("Wallet unlock failed"), e.what()); } break; case Decrypt: diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 7508f596e6..de236a016f 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -570,10 +570,8 @@ int main(int argc, char *argv[]) Q_INIT_RESOURCE(bitcoin_locale); BitcoinApplication app(*node, argc, argv); -#if QT_VERSION > 0x050100 // Generate high-dpi pixmaps QApplication::setAttribute(Qt::AA_UseHighDpiPixmaps); -#endif #if QT_VERSION >= 0x050600 QGuiApplication::setAttribute(Qt::AA_EnableHighDpiScaling); #endif diff --git a/src/qt/bitcoinamountfield.cpp b/src/qt/bitcoinamountfield.cpp index b68f3a439b..558fcf50ba 100644 --- a/src/qt/bitcoinamountfield.cpp +++ b/src/qt/bitcoinamountfield.cpp @@ -23,9 +23,7 @@ class AmountSpinBox: public QAbstractSpinBox public: explicit AmountSpinBox(QWidget *parent): - QAbstractSpinBox(parent), - currentUnit(BitcoinUnits::BTC), - singleStep(100000) // satoshis + QAbstractSpinBox(parent) { setAlignment(Qt::AlignRight); @@ -44,10 +42,19 @@ public: void fixup(QString &input) const { - bool valid = false; - CAmount val = parse(input, &valid); - if(valid) - { + bool valid; + CAmount val; + + if (input.isEmpty() && !m_allow_empty) { + valid = true; + val = m_min_amount; + } else { + valid = false; + val = parse(input, &valid); + } + + if (valid) { + val = qBound(m_min_amount, val, m_max_amount); input = BitcoinUnits::format(currentUnit, val, false, BitcoinUnits::separatorAlways); lineEdit()->setText(input); } @@ -64,12 +71,27 @@ public: Q_EMIT valueChanged(); } + void SetAllowEmpty(bool allow) + { + m_allow_empty = allow; + } + + void SetMinValue(const CAmount& value) + { + m_min_amount = value; + } + + void SetMaxValue(const CAmount& value) + { + m_max_amount = value; + } + void stepBy(int steps) { bool valid = false; CAmount val = value(&valid); val = val + steps * singleStep; - val = qMin(qMax(val, CAmount(0)), BitcoinUnits::maxMoney()); + val = qBound(m_min_amount, val, m_max_amount); setValue(val); } @@ -125,9 +147,12 @@ public: } private: - int currentUnit; - CAmount singleStep; + int currentUnit{BitcoinUnits::BTC}; + CAmount singleStep{CAmount(100000)}; // satoshis mutable QSize cachedMinimumSizeHint; + bool m_allow_empty{true}; + CAmount m_min_amount{CAmount(0)}; + CAmount m_max_amount{BitcoinUnits::maxMoney()}; /** * Parse a string into a number of base monetary units and @@ -174,11 +199,10 @@ protected: StepEnabled rv = 0; bool valid = false; CAmount val = value(&valid); - if(valid) - { - if(val > 0) + if (valid) { + if (val > m_min_amount) rv |= StepDownEnabled; - if(val < BitcoinUnits::maxMoney()) + if (val < m_max_amount) rv |= StepUpEnabled; } return rv; @@ -275,6 +299,21 @@ void BitcoinAmountField::setValue(const CAmount& value) amount->setValue(value); } +void BitcoinAmountField::SetAllowEmpty(bool allow) +{ + amount->SetAllowEmpty(allow); +} + +void BitcoinAmountField::SetMinValue(const CAmount& value) +{ + amount->SetMinValue(value); +} + +void BitcoinAmountField::SetMaxValue(const CAmount& value) +{ + amount->SetMaxValue(value); +} + void BitcoinAmountField::setReadOnly(bool fReadOnly) { amount->setReadOnly(fReadOnly); diff --git a/src/qt/bitcoinamountfield.h b/src/qt/bitcoinamountfield.h index f93579c492..650481e30d 100644 --- a/src/qt/bitcoinamountfield.h +++ b/src/qt/bitcoinamountfield.h @@ -31,6 +31,15 @@ public: CAmount value(bool *value=0) const; void setValue(const CAmount& value); + /** If allow empty is set to false the field will be set to the minimum allowed value if left empty. **/ + void SetAllowEmpty(bool allow); + + /** Set the minimum value in satoshis **/ + void SetMinValue(const CAmount& value); + + /** Set the maximum value in satoshis **/ + void SetMaxValue(const CAmount& value); + /** Set single step in satoshis **/ void setSingleStep(const CAmount& step); diff --git a/src/qt/forms/sendcoinsdialog.ui b/src/qt/forms/sendcoinsdialog.ui index 6b31ddea90..386d559281 100644 --- a/src/qt/forms/sendcoinsdialog.ui +++ b/src/qt/forms/sendcoinsdialog.ui @@ -878,28 +878,15 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <item> <layout class="QHBoxLayout" name="horizontalLayoutFee8"> <item> - <widget class="QCheckBox" name="checkBoxMinimumFee"> - <property name="toolTip"> - <string>Paying only the minimum fee is just fine as long as there is less transaction volume than space in the blocks. But be aware that this can end up in a never confirming transaction once there is more demand for bitcoin transactions than the network can process.</string> - </property> - <property name="text"> - <string/> - </property> - </widget> - </item> - <item> - <widget class="QLabel" name="labelMinFeeWarning"> + <widget class="QLabel" name="labelCustomFeeWarning"> <property name="enabled"> <bool>true</bool> </property> <property name="toolTip"> - <string>Paying only the minimum fee is just fine as long as there is less transaction volume than space in the blocks. But be aware that this can end up in a never confirming transaction once there is more demand for bitcoin transactions than the network can process.</string> + <string>When there is less transaction volume than space in the blocks, miners as well as relaying nodes may enforce a minimum fee. Paying only this minimum fee is just fine, but be aware that this can result in a never confirming transaction once there is more demand for bitcoin transactions than the network can process.</string> </property> <property name="text"> - <string>(read the tooltip)</string> - </property> - <property name="margin"> - <number>5</number> + <string>A too low fee might result in a never confirming transaction (read the tooltip)</string> </property> </widget> </item> @@ -992,9 +979,6 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <property name="text"> <string/> </property> - <property name="margin"> - <number>2</number> - </property> </widget> </item> <item> @@ -1009,9 +993,6 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <property name="text"> <string>(Smart fee not initialized yet. This usually takes a few blocks...)</string> </property> - <property name="margin"> - <number>2</number> - </property> </widget> </item> <item> @@ -1038,24 +1019,8 @@ Note: Since the fee is calculated on a per-byte basis, a fee of "100 satoshis p <property name="text"> <string>Confirmation time target:</string> </property> - <property name="margin"> - <number>2</number> - </property> </widget> </item> - <item> - <spacer name="verticalSpacer_3"> - <property name="orientation"> - <enum>Qt::Vertical</enum> - </property> - <property name="sizeHint" stdset="0"> - <size> - <width>1</width> - <height>1</height> - </size> - </property> - </spacer> - </item> </layout> </item> <item> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index b1cd2f77d0..0e9aca21b1 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -47,6 +47,7 @@ #include <QDoubleValidator> #include <QFileDialog> #include <QFont> +#include <QFontDatabase> #include <QKeyEvent> #include <QLineEdit> #include <QSettings> @@ -55,11 +56,6 @@ #include <QUrlQuery> #include <QMouseEvent> - -#if QT_VERSION >= 0x50200 -#include <QFontDatabase> -#endif - #if defined(Q_OS_MAC) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" @@ -82,13 +78,7 @@ QString dateTimeStr(qint64 nTime) QFont fixedPitchFont() { -#if QT_VERSION >= 0x50200 return QFontDatabase::systemFont(QFontDatabase::FixedFont); -#else - QFont font("Monospace"); - font.setStyleHint(QFont::Monospace); - return font; -#endif } // Just some dummy data to generate a convincing random-looking (but consistent) address diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 858128f9f9..65db0280b7 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -119,13 +119,11 @@ SendCoinsDialog::SendCoinsDialog(const PlatformStyle *_platformStyle, QWidget *p settings.setValue("nSmartFeeSliderPosition", 0); if (!settings.contains("nTransactionFee")) settings.setValue("nTransactionFee", (qint64)DEFAULT_PAY_TX_FEE); - if (!settings.contains("fPayOnlyMinFee")) - settings.setValue("fPayOnlyMinFee", false); ui->groupFee->setId(ui->radioSmartFee, 0); ui->groupFee->setId(ui->radioCustomFee, 1); ui->groupFee->button((int)std::max(0, std::min(1, settings.value("nFeeRadio").toInt())))->setChecked(true); + ui->customFee->SetAllowEmpty(false); ui->customFee->setValue(settings.value("nTransactionFee").toLongLong()); - ui->checkBoxMinimumFee->setChecked(settings.value("fPayOnlyMinFee").toBool()); minimizeFeeSection(settings.value("fFeeSectionMinimized").toBool()); } @@ -174,14 +172,15 @@ void SendCoinsDialog::setModel(WalletModel *_model) connect(ui->groupFee, static_cast<void (QButtonGroup::*)(int)>(&QButtonGroup::buttonClicked), this, &SendCoinsDialog::updateFeeSectionControls); connect(ui->groupFee, static_cast<void (QButtonGroup::*)(int)>(&QButtonGroup::buttonClicked), this, &SendCoinsDialog::coinControlUpdateLabels); connect(ui->customFee, &BitcoinAmountField::valueChanged, this, &SendCoinsDialog::coinControlUpdateLabels); - connect(ui->checkBoxMinimumFee, &QCheckBox::stateChanged, this, &SendCoinsDialog::setMinimumFee); - connect(ui->checkBoxMinimumFee, &QCheckBox::stateChanged, this, &SendCoinsDialog::updateFeeSectionControls); - connect(ui->checkBoxMinimumFee, &QCheckBox::stateChanged, this, &SendCoinsDialog::coinControlUpdateLabels); connect(ui->optInRBF, &QCheckBox::stateChanged, this, &SendCoinsDialog::updateSmartFeeLabel); connect(ui->optInRBF, &QCheckBox::stateChanged, this, &SendCoinsDialog::coinControlUpdateLabels); - ui->customFee->setSingleStep(model->wallet().getRequiredFee(1000)); + CAmount requiredFee = model->wallet().getRequiredFee(1000); + ui->customFee->SetMinValue(requiredFee); + if (ui->customFee->value() < requiredFee) { + ui->customFee->setValue(requiredFee); + } + ui->customFee->setSingleStep(requiredFee); updateFeeSectionControls(); - updateMinFeeLabel(); updateSmartFeeLabel(); // set default rbf checkbox state @@ -210,7 +209,6 @@ SendCoinsDialog::~SendCoinsDialog() settings.setValue("nFeeRadio", ui->groupFee->checkedId()); settings.setValue("nConfTarget", getConfTargetForIndex(ui->confTargetSelector->currentIndex())); settings.setValue("nTransactionFee", (qint64)ui->customFee->value()); - settings.setValue("fPayOnlyMinFee", ui->checkBoxMinimumFee->isChecked()); delete ui; } @@ -542,7 +540,6 @@ void SendCoinsDialog::updateDisplayUnit() { setBalance(model->wallet().getBalances()); ui->customFee->setDisplayUnit(model->getOptionsModel()->getDisplayUnit()); - updateMinFeeLabel(); updateSmartFeeLabel(); } @@ -642,11 +639,6 @@ void SendCoinsDialog::useAvailableBalance(SendCoinsEntry* entry) } } -void SendCoinsDialog::setMinimumFee() -{ - ui->customFee->setValue(model->wallet().getRequiredFee(1000)); -} - void SendCoinsDialog::updateFeeSectionControls() { ui->confTargetSelector ->setEnabled(ui->radioSmartFee->isChecked()); @@ -654,10 +646,9 @@ void SendCoinsDialog::updateFeeSectionControls() ui->labelSmartFee2 ->setEnabled(ui->radioSmartFee->isChecked()); ui->labelSmartFee3 ->setEnabled(ui->radioSmartFee->isChecked()); ui->labelFeeEstimation ->setEnabled(ui->radioSmartFee->isChecked()); - ui->checkBoxMinimumFee ->setEnabled(ui->radioCustomFee->isChecked()); - ui->labelMinFeeWarning ->setEnabled(ui->radioCustomFee->isChecked()); - ui->labelCustomPerKilobyte ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked()); - ui->customFee ->setEnabled(ui->radioCustomFee->isChecked() && !ui->checkBoxMinimumFee->isChecked()); + ui->labelCustomFeeWarning ->setEnabled(ui->radioCustomFee->isChecked()); + ui->labelCustomPerKilobyte ->setEnabled(ui->radioCustomFee->isChecked()); + ui->customFee ->setEnabled(ui->radioCustomFee->isChecked()); } void SendCoinsDialog::updateFeeMinimizedLabel() @@ -672,14 +663,6 @@ void SendCoinsDialog::updateFeeMinimizedLabel() } } -void SendCoinsDialog::updateMinFeeLabel() -{ - if (model && model->getOptionsModel()) - ui->checkBoxMinimumFee->setText(tr("Pay only the required fee of %1").arg( - BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->wallet().getRequiredFee(1000)) + "/kB") - ); -} - void SendCoinsDialog::updateCoinControlState(CCoinControl& ctrl) { if (ui->radioCustomFee->isChecked()) { diff --git a/src/qt/sendcoinsdialog.h b/src/qt/sendcoinsdialog.h index 7009855f17..e1ebc77d59 100644 --- a/src/qt/sendcoinsdialog.h +++ b/src/qt/sendcoinsdialog.h @@ -92,9 +92,7 @@ private Q_SLOTS: void coinControlClipboardBytes(); void coinControlClipboardLowOutput(); void coinControlClipboardChange(); - void setMinimumFee(); void updateFeeSectionControls(); - void updateMinFeeLabel(); void updateSmartFeeLabel(); Q_SIGNALS: diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index b6235e91f7..df38285d08 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -37,9 +37,7 @@ SplashScreen::SplashScreen(interfaces::Node& node, Qt::WindowFlags f, const Netw float fontFactor = 1.0; float devicePixelRatio = 1.0; -#if QT_VERSION > 0x050100 devicePixelRatio = static_cast<QGuiApplication*>(QCoreApplication::instance())->devicePixelRatio(); -#endif // define text to place QString titleText = tr(PACKAGE_NAME); @@ -53,10 +51,8 @@ SplashScreen::SplashScreen(interfaces::Node& node, Qt::WindowFlags f, const Netw QSize splashSize(480*devicePixelRatio,320*devicePixelRatio); pixmap = QPixmap(splashSize); -#if QT_VERSION > 0x050100 // change to HiDPI if it makes sense pixmap.setDevicePixelRatio(devicePixelRatio); -#endif QPainter pixPaint(&pixmap); pixPaint.setPen(QColor(100,100,100)); diff --git a/src/qt/test/util.h b/src/qt/test/util.h index 5363c94547..377f07dcba 100644 --- a/src/qt/test/util.h +++ b/src/qt/test/util.h @@ -1,6 +1,8 @@ #ifndef BITCOIN_QT_TEST_UTIL_H #define BITCOIN_QT_TEST_UTIL_H +#include <QString> + /** * Press "Ok" button in message box dialog. * diff --git a/src/rest.cpp b/src/rest.cpp index 6c7e0384cb..4988e6ed26 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -3,20 +3,21 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <attributes.h> #include <chain.h> #include <chainparams.h> #include <core_io.h> +#include <httpserver.h> #include <index/txindex.h> #include <primitives/block.h> #include <primitives/transaction.h> -#include <validation.h> -#include <httpserver.h> #include <rpc/blockchain.h> #include <rpc/server.h> #include <streams.h> #include <sync.h> #include <txmempool.h> #include <util/strencodings.h> +#include <validation.h> #include <version.h> #include <boost/algorithm/string.hpp> diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e9bc6d988d..c48224b245 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -12,8 +12,8 @@ #include <checkpoints.h> #include <coins.h> #include <consensus/validation.h> -#include <validation.h> #include <core_io.h> +#include <hash.h> #include <index/txindex.h> #include <key_io.h> #include <policy/feerate.h> @@ -21,14 +21,15 @@ #include <policy/rbf.h> #include <primitives/transaction.h> #include <rpc/server.h> +#include <rpc/util.h> #include <script/descriptor.h> #include <streams.h> #include <sync.h> #include <txdb.h> #include <txmempool.h> -#include <util/system.h> #include <util/strencodings.h> -#include <hash.h> +#include <util/system.h> +#include <validation.h> #include <validationinterface.h> #include <versionbitsinfo.h> #include <warnings.h> @@ -285,12 +286,12 @@ static UniValue waitforblockheight(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) throw std::runtime_error( - "waitforblockheight <height> (timeout)\n" + "waitforblockheight height ( timeout )\n" "\nWaits for (at least) block height and returns the height and hash\n" "of the current tip.\n" "\nReturns the current block on timeout or exit.\n" "\nArguments:\n" - "1. height (required, int) Block height to wait for (int)\n" + "1. height (int, required) Block height to wait for.\n" "2. timeout (int, optional, default=0) Time in milliseconds to wait for a response. 0 indicates no timeout.\n" "\nResult:\n" "{ (json object)\n" @@ -694,11 +695,11 @@ static UniValue getblockheader(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) throw std::runtime_error( - "getblockheader \"hash\" ( verbose )\n" + "getblockheader \"blockhash\" ( verbose )\n" "\nIf verbose is false, returns a string that is serialized, hex-encoded data for blockheader 'hash'.\n" "If verbose is true, returns an Object with information about blockheader <hash>.\n" "\nArguments:\n" - "1. \"hash\" (string, required) The block hash\n" + "1. \"blockhash\" (string, required) The block hash\n" "2. verbose (boolean, optional, default=true) true for a json object, false for the hex-encoded data\n" "\nResult (for verbose = true):\n" "{\n" @@ -924,7 +925,7 @@ static UniValue pruneblockchain(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) throw std::runtime_error( - "pruneblockchain\n" + "pruneblockchain height\n" "\nArguments:\n" "1. \"height\" (numeric, required) The block height to prune up to. May be set to a discrete height, or a unix timestamp\n" " to prune blocks whose block time is at least 2 hours older than the provided timestamp.\n" @@ -1561,7 +1562,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() > 2) throw std::runtime_error( - "getchaintxstats ( nblocks blockhash )\n" + "getchaintxstats ( nblocks \"blockhash\" )\n" "\nCompute statistics about the total number and rate of transactions in the chain.\n" "\nArguments:\n" "1. nblocks (numeric, optional) Size of the window in number of blocks (default: one month).\n" @@ -2038,7 +2039,7 @@ UniValue scantxoutset(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) throw std::runtime_error( - "scantxoutset <action> ( <scanobjects> )\n" + "scantxoutset \"action\" [scanobjects,...]\n" "\nEXPERIMENTAL warning: this call may be removed or changed in future releases.\n" "\nScans the unspent transaction output set for entries that match certain output descriptors.\n" "Examples of output descriptors are:\n" diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 58b54c66de..4f314ef215 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -18,10 +18,11 @@ #include <rpc/blockchain.h> #include <rpc/mining.h> #include <rpc/server.h> +#include <rpc/util.h> #include <shutdown.h> #include <txmempool.h> -#include <util/system.h> #include <util/strencodings.h> +#include <util/system.h> #include <validation.h> #include <validationinterface.h> #include <versionbitsinfo.h> @@ -231,7 +232,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 3) throw std::runtime_error( - "prioritisetransaction <txid> <dummy value> <fee delta>\n" + "prioritisetransaction \"txid\" dummy fee_delta\n" "Accepts the transaction into mined blocks at a higher (or lower) priority\n" "\nArguments:\n" "1. \"txid\" (string, required) The transaction id.\n" @@ -294,7 +295,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() > 1) throw std::runtime_error( - "getblocktemplate ( TemplateRequest )\n" + "getblocktemplate ( \"template_request\" )\n" "\nIf the request parameters include a 'mode' key, that is used to explicitly select between the default 'template' request or a 'proposal'.\n" "It returns data needed to construct a block to work on.\n" "For full specification, see BIPs 22, 23, 9, and 145:\n" diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 4d893bb838..fc1498a224 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -7,17 +7,18 @@ #include <chainparams.h> #include <clientversion.h> #include <core_io.h> -#include <validation.h> #include <net.h> #include <net_processing.h> #include <netbase.h> #include <policy/policy.h> #include <rpc/protocol.h> +#include <rpc/util.h> #include <sync.h> #include <timedata.h> #include <ui_interface.h> -#include <util/system.h> #include <util/strencodings.h> +#include <util/system.h> +#include <validation.h> #include <version.h> #include <warnings.h> @@ -200,7 +201,7 @@ static UniValue addnode(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 2 || (strCommand != "onetry" && strCommand != "add" && strCommand != "remove")) throw std::runtime_error( - "addnode \"node\" \"add|remove|onetry\"\n" + "addnode \"node\" \"command\"\n" "\nAttempts to add or remove a node from the addnode list.\n" "Or try a connection to a node once.\n" "Nodes added using addnode (or -connect) are protected from DoS disconnection and are not required to be\n" @@ -243,13 +244,13 @@ static UniValue disconnectnode(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() == 0 || request.params.size() >= 3) throw std::runtime_error( - "disconnectnode \"[address]\" [nodeid]\n" + "disconnectnode ( \"address\" nodeid )\n" "\nImmediately disconnects from the specified peer node.\n" "\nStrictly one out of 'address' and 'nodeid' can be provided to identify the node.\n" "\nTo disconnect by nodeid, either set 'address' to the empty string, or call using the named 'nodeid' argument only.\n" "\nArguments:\n" "1. \"address\" (string, optional) The IP address/port of the node\n" - "2. \"nodeid\" (number, optional) The node ID (see getpeerinfo for node IDs)\n" + "2. nodeid (number, optional) The node ID (see getpeerinfo for node IDs)\n" "\nExamples:\n" + HelpExampleCli("disconnectnode", "\"192.168.0.6:8333\"") + HelpExampleCli("disconnectnode", "\"\" 1") @@ -499,7 +500,7 @@ static UniValue setban(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 2 || (strCommand != "add" && strCommand != "remove")) throw std::runtime_error( - "setban \"subnet\" \"add|remove\" (bantime) (absolute)\n" + "setban \"subnet\" \"command\" ( bantime absolute )\n" "\nAttempts to add or remove an IP/Subnet from the banned list.\n" "\nArguments:\n" "1. \"subnet\" (string, required) The IP/Subnet (see getpeerinfo for nodes IP) with an optional netmask (default is /32 = single IP)\n" @@ -610,7 +611,7 @@ static UniValue setnetworkactive(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) { throw std::runtime_error( - "setnetworkactive true|false\n" + "setnetworkactive state\n" "\nDisable/enable all p2p network activity.\n" "\nArguments:\n" "1. \"state\" (boolean, required) true to enable networking, false to disable\n" diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index c27d5d142b..480cb0b9d2 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -10,10 +10,8 @@ #include <core_io.h> #include <index/txindex.h> #include <init.h> -#include <keystore.h> -#include <validation.h> -#include <validationinterface.h> #include <key_io.h> +#include <keystore.h> #include <merkleblock.h> #include <net.h> #include <policy/policy.h> @@ -29,6 +27,8 @@ #include <txmempool.h> #include <uint256.h> #include <util/strencodings.h> +#include <validation.h> +#include <validationinterface.h> #include <future> #include <stdint.h> @@ -206,7 +206,16 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) { if (request.fHelp || (request.params.size() != 1 && request.params.size() != 2)) throw std::runtime_error( - "gettxoutproof [\"txid\",...] ( blockhash )\n" + RPCHelpMan{"gettxoutproof", + { + {"txids", RPCArg::Type::ARR, + { + {"txid", RPCArg::Type::STR_HEX, false}, + }, + false}, + {"blockhash", RPCArg::Type::STR_HEX, true}, + }} + .ToString() + "\nReturns a hex-encoded proof that \"txid\" was included in a block.\n" "\nNOTE: By default this function only works sometimes. This is when there is an\n" "unspent output in the utxo for this transaction. To make it always work,\n" @@ -673,10 +682,17 @@ static void TxInErrorToJSON(const CTxIn& txin, UniValue& vErrorsRet, const std:: static UniValue combinerawtransaction(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) throw std::runtime_error( - "combinerawtransaction [\"hexstring\",...]\n" + RPCHelpMan{"combinerawtransaction", + { + {"txs", RPCArg::Type::ARR, + { + {"hexstring", RPCArg::Type::STR_HEX, false}, + }, + false}, + }} + .ToString() + "\nCombine multiple partially signed transactions into one transaction.\n" "The combined transaction may be another partially signed transaction or a \n" "fully signed transaction." @@ -899,7 +915,30 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 2 || request.params.size() > 4) throw std::runtime_error( - "signrawtransactionwithkey \"hexstring\" [\"privatekey1\",...] ( [{\"txid\":\"id\",\"vout\":n,\"scriptPubKey\":\"hex\",\"redeemScript\":\"hex\"},...] sighashtype )\n" + RPCHelpMan{"signrawtransactionwithkey", + { + {"hexstring", RPCArg::Type::STR, false}, + {"privkyes", RPCArg::Type::ARR, + { + {"privatekey", RPCArg::Type::STR_HEX, false}, + }, + false}, + {"prevtxs", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"txid", RPCArg::Type::STR_HEX, false}, + {"vout", RPCArg::Type::NUM, false}, + {"scriptPubKey", RPCArg::Type::STR_HEX, false}, + {"redeemScript", RPCArg::Type::STR_HEX, false}, + {"amount", RPCArg::Type::AMOUNT, false}, + }, + true}, + }, + true}, + {"sighashtype", RPCArg::Type::STR, true}, + }} + .ToString() + "\nSign inputs for raw transaction (serialized, hex-encoded).\n" "The second argument is an array of base58-encoded private\n" "keys that will be the only keys used to sign the transaction.\n" @@ -1454,7 +1493,15 @@ UniValue combinepsbt(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 1) throw std::runtime_error( - "combinepsbt [\"psbt\",...]\n" + RPCHelpMan{"combinepsbt", + { + {"txs", RPCArg::Type::ARR, + { + {"psbt", RPCArg::Type::STR_HEX, false}, + }, + false}, + }} + .ToString() + "\nCombine multiple partially signed Bitcoin transactions into one transaction.\n" "Implements the Combiner role.\n" "\nArguments:\n" @@ -1571,7 +1618,37 @@ UniValue createpsbt(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() < 2 || request.params.size() > 4) throw std::runtime_error( - "createpsbt [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime ) ( replaceable )\n" + RPCHelpMan{"createpsbt", + { + {"inputs", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"txid", RPCArg::Type::STR_HEX, false}, + {"vout", RPCArg::Type::NUM, false}, + {"sequence", RPCArg::Type::NUM, true}, + }, + false}, + }, + false}, + {"outputs", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"address", RPCArg::Type::AMOUNT, false}, + }, + true}, + {"", RPCArg::Type::OBJ, + { + {"data", RPCArg::Type::STR_HEX, false}, + }, + true}, + }, + false}, + {"locktime", RPCArg::Type::NUM, true}, + {"replaceable", RPCArg::Type::BOOL, true}, + }} + .ToString() + "\nCreates a transaction in the Partially Signed Transaction format.\n" "Implements the Creator role.\n" "\nArguments:\n" diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 78abd7b610..f890baba51 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -8,11 +8,12 @@ #include <fs.h> #include <key_io.h> #include <random.h> +#include <rpc/util.h> #include <shutdown.h> #include <sync.h> #include <ui_interface.h> -#include <util/system.h> #include <util/strencodings.h> +#include <util/system.h> #include <boost/bind.hpp> #include <boost/signals2/signal.hpp> @@ -234,7 +235,7 @@ UniValue stop(const JSONRPCRequest& jsonRequest) static UniValue uptime(const JSONRPCRequest& jsonRequest) { - if (jsonRequest.fHelp || jsonRequest.params.size() > 1) + if (jsonRequest.fHelp || jsonRequest.params.size() > 0) throw std::runtime_error( "uptime\n" "\nReturns the total uptime of the server.\n" diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index ef2d14b90e..6f2450708a 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -128,3 +128,95 @@ UniValue DescribeAddress(const CTxDestination& dest) { return boost::apply_visitor(DescribeAddressVisitor(), dest); } + +std::string RPCHelpMan::ToString() const +{ + std::string ret; + + ret += m_name; + bool is_optional{false}; + for (const auto& arg : m_args) { + ret += " "; + if (arg.m_optional) { + if (!is_optional) ret += "( "; + is_optional = true; + } else { + // Currently we still support unnamed arguments, so any argument following an optional argument must also be optional + // If support for positional arguments is deprecated in the future, remove this line + assert(!is_optional); + } + ret += arg.ToString(); + } + if (is_optional) ret += " )"; + ret += "\n"; + + return ret; +} + +std::string RPCArg::ToStringObj() const +{ + std::string res = "\"" + m_name + "\":"; + switch (m_type) { + case Type::STR: + return res + "\"str\""; + case Type::STR_HEX: + return res + "\"hex\""; + case Type::NUM: + return res + "n"; + case Type::AMOUNT: + return res + "amount"; + case Type::BOOL: + return res + "bool"; + case Type::ARR: + res += "["; + for (const auto& i : m_inner) { + res += i.ToString() + ","; + } + return res + "...]"; + case Type::OBJ: + case Type::OBJ_USER_KEYS: + // Currently unused, so avoid writing dead code + assert(false); + + // no default case, so the compiler can warn about missing cases + } + assert(false); +} + +std::string RPCArg::ToString() const +{ + switch (m_type) { + case Type::STR_HEX: + case Type::STR: { + return "\"" + m_name + "\""; + } + case Type::NUM: + case Type::AMOUNT: + case Type::BOOL: { + return m_name; + } + case Type::OBJ: + case Type::OBJ_USER_KEYS: { + std::string res; + for (size_t i = 0; i < m_inner.size();) { + res += m_inner[i].ToStringObj(); + if (++i < m_inner.size()) res += ","; + } + if (m_type == Type::OBJ) { + return "{" + res + "}"; + } else { + return "{" + res + ",...}"; + } + } + case Type::ARR: { + std::string res; + for (const auto& i : m_inner) { + res += i.ToString() + ","; + } + return "[" + res + "...]"; + } + + // no default case, so the compiler can warn about missing cases + } + assert(false); +} diff --git a/src/rpc/util.h b/src/rpc/util.h index c125739c20..6d1e591f12 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -28,4 +28,53 @@ CScript CreateMultisigRedeemscript(const int required, const std::vector<CPubKey UniValue DescribeAddress(const CTxDestination& dest); +struct RPCArg { + enum class Type { + OBJ, + ARR, + STR, + NUM, + BOOL, + OBJ_USER_KEYS, //!< Special type where the user must set the keys e.g. to define multiple addresses; as opposed to e.g. an options object where the keys are predefined + AMOUNT, //!< Special type representing a floating point amount (can be either NUM or STR) + STR_HEX, //!< Special type that is a STR with only hex chars + }; + const std::string m_name; //!< The name of the arg (can be empty for inner args) + const Type m_type; + const std::vector<RPCArg> m_inner; //!< Only used for arrays or dicts + const bool m_optional; + + RPCArg(const std::string& name, const Type& type, const bool optional) + : m_name{name}, m_type{type}, m_optional{optional} + { + assert(type != Type::ARR && type != Type::OBJ); + } + + RPCArg(const std::string& name, const Type& type, const std::vector<RPCArg>& inner, const bool optional) + : m_name{name}, m_type{type}, m_inner{inner}, m_optional{optional} + { + assert(type == Type::ARR || type == Type::OBJ); + } + + std::string ToString() const; + +private: + std::string ToStringObj() const; +}; + +class RPCHelpMan +{ +public: + RPCHelpMan(const std::string& name, const std::vector<RPCArg>& args) + : m_name{name}, m_args{args} + { + } + + std::string ToString() const; + +private: + const std::string m_name; + const std::vector<RPCArg> m_args; +}; + #endif // BITCOIN_RPC_UTIL_H diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 90c4ddcc11..d343972c40 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -478,7 +478,7 @@ std::vector<Span<const char>> Split(const Span<const char>& sp, char sep) } /** Parse a key path, being passed a split list of elements (the first element is ignored). */ -bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out) +NODISCARD bool ParseKeyPath(const std::vector<Span<const char>>& split, KeyPath& out) { for (size_t i = 1; i < split.size(); ++i) { Span<const char> elem = split[i]; diff --git a/src/script/sign.h b/src/script/sign.h index e71f43f96d..a478f49789 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -206,6 +206,9 @@ template<typename Stream> void SerializeHDKeypaths(Stream& s, const std::map<CPubKey, KeyOriginInfo>& hd_keypaths, uint8_t type) { for (auto keypath_pair : hd_keypaths) { + if (!keypath_pair.first.IsValid()) { + throw std::ios_base::failure("Invalid CPubKey being serialized"); + } SerializeToVector(s, type, MakeSpan(keypath_pair.first)); WriteCompactSize(s, (keypath_pair.second.path.size() + 1) * sizeof(uint32_t)); s << keypath_pair.second.fingerprint; diff --git a/src/test/blockencodings_tests.cpp b/src/test/blockencodings_tests.cpp index 5131fe8235..309b8d2d06 100644 --- a/src/test/blockencodings_tests.cpp +++ b/src/test/blockencodings_tests.cpp @@ -344,4 +344,49 @@ BOOST_AUTO_TEST_CASE(TransactionsRequestSerializationTest) { BOOST_CHECK_EQUAL(req1.indexes[3], req2.indexes[3]); } +BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationMaxTest) { + // Check that the highest legal index is decoded correctly + BlockTransactionsRequest req0; + req0.blockhash = InsecureRand256(); + req0.indexes.resize(1); + req0.indexes[0] = 0xffff; + CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); + stream << req0; + + BlockTransactionsRequest req1; + stream >> req1; + BOOST_CHECK_EQUAL(req0.indexes.size(), req1.indexes.size()); + BOOST_CHECK_EQUAL(req0.indexes[0], req1.indexes[0]); +} + +BOOST_AUTO_TEST_CASE(TransactionsRequestDeserializationOverflowTest) { + // Any set of index deltas that starts with N values that sum to (0x10000 - N) + // causes the edge-case overflow that was originally not checked for. Such + // a request cannot be created by serializing a real BlockTransactionsRequest + // due to the overflow, so here we'll serialize from raw deltas. + BlockTransactionsRequest req0; + req0.blockhash = InsecureRand256(); + req0.indexes.resize(3); + req0.indexes[0] = 0x7000; + req0.indexes[1] = 0x10000 - 0x7000 - 2; + req0.indexes[2] = 0; + CDataStream stream(SER_NETWORK, PROTOCOL_VERSION); + stream << req0.blockhash; + WriteCompactSize(stream, req0.indexes.size()); + WriteCompactSize(stream, req0.indexes[0]); + WriteCompactSize(stream, req0.indexes[1]); + WriteCompactSize(stream, req0.indexes[2]); + + BlockTransactionsRequest req1; + try { + stream >> req1; + // before patch: deserialize above succeeds and this check fails, demonstrating the overflow + BOOST_CHECK(req1.indexes[1] < req1.indexes[2]); + // this shouldn't be reachable before or after patch + BOOST_CHECK(0); + } catch(std::ios_base::failure &) { + // deserialize should fail + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 0432ede3e0..14ddf4d10e 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -28,7 +28,7 @@ static void ResetArgs(const std::string& strArg) vecChar.push_back(s.c_str()); std::string error; - gArgs.ParseParameters(vecChar.size(), vecChar.data(), error); + BOOST_CHECK(gArgs.ParseParameters(vecChar.size(), vecChar.data(), error)); } static void SetupArgs(const std::vector<std::string>& args) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index ff19b12a9c..f461c7496f 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -210,13 +210,13 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters) std::string error; testArgs.SetupArgs(4, avail_args); - testArgs.ParseParameters(0, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(0, (char**)argv_test, error)); BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); - testArgs.ParseParameters(1, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error)); BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); - testArgs.ParseParameters(7, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error)); // expectation: -ignored is ignored (program name argument), // -a, -b and -ccc end up in map, -d ignored because it is after // a non-option argument (non-GNU option parsing) @@ -242,7 +242,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg) "ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"}; std::string error; testArgs.SetupArgs(6, avail_args); - testArgs.ParseParameters(7, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error)); // Each letter should be set. for (const char opt : "abcdef") @@ -278,7 +278,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"}; testArgs.SetupArgs(2, avail_args); std::string error; - testArgs.ParseParameters(4, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(4, (char**)argv_test, error)); // This was passed twice, second one overrides the negative setting. BOOST_CHECK(!testArgs.IsArgNegated("-foo")); @@ -290,7 +290,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) // Config test const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n"; - testArgs.ParseParameters(1, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error)); testArgs.ReadConfigString(conf_test); // This was passed twice, second one overrides the negative setting, @@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) // Combined test const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"}; const char *combo_test_conf = "foo=1\nnobar=1\n"; - testArgs.ParseParameters(3, (char**)combo_test_args, error); + BOOST_CHECK(testArgs.ParseParameters(3, (char**)combo_test_args, error)); testArgs.ReadConfigString(combo_test_conf); // Command line overrides, but doesn't erase old setting @@ -557,38 +557,38 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) const char* testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1"; std::string error; - test_args.ParseParameters(0, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "main"); - test_args.ParseParameters(2, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_regtest, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest"); - test_args.ParseParameters(3, (char**)argv_test_no_reg, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_test_no_reg, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char**)argv_both, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error)); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(0, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_regtest, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(3, (char**)argv_test_no_reg, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_test_no_reg, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char**)argv_both, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); @@ -596,23 +596,23 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) // [test] regtest=1 potentially relevant) doesn't break things test_args.SelectConfigNetwork("test"); - test_args.ParseParameters(0, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_regtest, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(2, (char**)argv_test_no_reg, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_test_no_reg, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char**)argv_both, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); } diff --git a/src/util/moneystr.h b/src/util/moneystr.h index 9133f46d5d..b8e2812a96 100644 --- a/src/util/moneystr.h +++ b/src/util/moneystr.h @@ -9,16 +9,17 @@ #ifndef BITCOIN_UTIL_MONEYSTR_H #define BITCOIN_UTIL_MONEYSTR_H -#include <stdint.h> -#include <string> - #include <amount.h> +#include <attributes.h> + +#include <cstdint> +#include <string> /* Do not use these functions to represent or parse monetary amounts to or from * JSON but use AmountFromValue and ValueFromAmount for that. */ std::string FormatMoney(const CAmount& n); -bool ParseMoney(const std::string& str, CAmount& nRet); -bool ParseMoney(const char* pszIn, CAmount& nRet); +NODISCARD bool ParseMoney(const std::string& str, CAmount& nRet); +NODISCARD bool ParseMoney(const char* pszIn, CAmount& nRet); #endif // BITCOIN_UTIL_MONEYSTR_H diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 2a2df43337..46146be66f 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -263,7 +263,7 @@ std::string DecodeBase32(const std::string& str) return std::string((const char*)vchRet.data(), vchRet.size()); } -static bool ParsePrechecks(const std::string& str) +NODISCARD static bool ParsePrechecks(const std::string& str) { if (str.empty()) // No empty string allowed return false; diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 87ccf40a1b..7d16d7dcfd 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -9,7 +9,9 @@ #ifndef BITCOIN_UTIL_STRENCODINGS_H #define BITCOIN_UTIL_STRENCODINGS_H -#include <stdint.h> +#include <attributes.h> + +#include <cstdint> #include <string> #include <vector> @@ -92,35 +94,35 @@ constexpr inline bool IsSpace(char c) noexcept { * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -bool ParseInt32(const std::string& str, int32_t *out); +NODISCARD bool ParseInt32(const std::string& str, int32_t *out); /** * Convert string to signed 64-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -bool ParseInt64(const std::string& str, int64_t *out); +NODISCARD bool ParseInt64(const std::string& str, int64_t *out); /** * Convert decimal string to unsigned 32-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -bool ParseUInt32(const std::string& str, uint32_t *out); +NODISCARD bool ParseUInt32(const std::string& str, uint32_t *out); /** * Convert decimal string to unsigned 64-bit integer with strict parse error feedback. * @returns true if the entire string could be parsed as valid integer, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -bool ParseUInt64(const std::string& str, uint64_t *out); +NODISCARD bool ParseUInt64(const std::string& str, uint64_t *out); /** * Convert string to double with strict parse error feedback. * @returns true if the entire string could be parsed as valid double, * false if not the entire string could be parsed or when overflow or underflow occurred. */ -bool ParseDouble(const std::string& str, double *out); +NODISCARD bool ParseDouble(const std::string& str, double *out); template<typename T> std::string HexStr(const T itbegin, const T itend, bool fSpaces=false) @@ -173,7 +175,7 @@ bool TimingResistantEqual(const T& a, const T& b) * @returns true on success, false on error. * @note The result must be in the range (-10^18,10^18), otherwise an overflow error will trigger. */ -bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); +NODISCARD bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out); /** Convert from one power-of-2 number base to another. */ template<int frombits, int tobits, bool pad, typename O, typename I> @@ -200,7 +202,7 @@ bool ConvertBits(const O& outfn, I it, I end) { } /** Parse an HD keypaths like "m/7/0'/2000". */ -bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath); +NODISCARD bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath); /** * Converts the given character to its lowercase equivalent. diff --git a/src/util/system.h b/src/util/system.h index 5634b8dd61..bebb089a2a 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -14,6 +14,7 @@ #include <config/bitcoin-config.h> #endif +#include <attributes.h> #include <compat.h> #include <fs.h> #include <logging.h> @@ -159,7 +160,7 @@ public: */ void SelectConfigNetwork(const std::string& network); - bool ParseParameters(int argc, const char* const argv[], std::string& error); + NODISCARD bool ParseParameters(int argc, const char* const argv[], std::string& error); bool ReadConfigFiles(std::string& error, bool ignore_invalid_keys = false); /** diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp index de320b4e9e..f5ac6e98b2 100644 --- a/src/wallet/crypter.cpp +++ b/src/wallet/crypter.cpp @@ -202,7 +202,7 @@ bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) if (keyPass && keyFail) { LogPrintf("The wallet is probably corrupted: Some keys decrypt but not all.\n"); - assert(false); + throw std::runtime_error("Error unlocking wallet: some keys decrypt but not all. Your wallet file may be corrupt."); } if (keyFail || !keyPass) return false; diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index e1e4fc51fe..6269bc9d3c 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -3,18 +3,19 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <chain.h> +#include <core_io.h> #include <interfaces/chain.h> #include <key_io.h> +#include <merkleblock.h> #include <rpc/server.h> -#include <validation.h> +#include <rpc/util.h> #include <script/script.h> #include <script/standard.h> #include <sync.h> #include <util/system.h> #include <util/time.h> +#include <validation.h> #include <wallet/wallet.h> -#include <merkleblock.h> -#include <core_io.h> #include <wallet/rpcwallet.h> @@ -113,7 +114,7 @@ UniValue importprivkey(const JSONRPCRequest& request) "Hint: use importmulti to import more than one private key.\n" "\nArguments:\n" "1. \"privkey\" (string, required) The private key (see dumpprivkey)\n" - "2. \"label\" (string, optional, default=\"\") An optional label\n" + "2. \"label\" (string, optional, default=current label if address exists, otherwise \"\") An optional label\n" "3. rescan (boolean, optional, default=true) Rescan the wallet for transactions\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 key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" @@ -163,9 +164,14 @@ UniValue importprivkey(const JSONRPCRequest& request) CKeyID vchAddress = pubkey.GetID(); { pwallet->MarkDirty(); - // We don't know which corresponding address will be used; label them all + + // We don't know which corresponding address will be used; + // label all new addresses, and label existing addresses if a + // label was passed. for (const auto& dest : GetAllDestinationsForKey(pubkey)) { - pwallet->SetAddressBook(dest, strLabel, "receive"); + if (!request.params[1].isNull() || pwallet->mapAddressBook.count(dest) == 0) { + pwallet->SetAddressBook(dest, strLabel, "receive"); + } } // Don't throw error in case a key is already there @@ -342,7 +348,7 @@ UniValue importprunedfunds(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 2) throw std::runtime_error( - "importprunedfunds\n" + "importprunedfunds \"rawtransaction\" \"txoutproof\"\n" "\nImports funds without rescan. Corresponding address or script must previously be included in wallet. Aimed towards pruned wallets. The end-user is responsible to import additional transactions that subsequently spend the imported outputs or rescan after the point in the blockchain the transaction is included.\n" "\nArguments:\n" "1. \"rawtransaction\" (string, required) A raw transaction in hex funding an already-existing address in wallet\n" @@ -1003,8 +1009,9 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con throw JSONRPCError(RPC_WALLET_ERROR, "Error adding scriptPubKey script to wallet"); } - // add to address book or update label - if (IsValidDestination(scriptpubkey_dest)) { + // if not internal add to address book or update label + if (!internal) { + assert(IsValidDestination(scriptpubkey_dest)); pwallet->SetAddressBook(scriptpubkey_dest, label, "receive"); } @@ -1079,8 +1086,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) // clang-format off if (mainRequest.fHelp || mainRequest.params.size() < 1 || mainRequest.params.size() > 2) throw std::runtime_error( - "importmulti \"requests\" ( \"options\" )\n\n" - "Import addresses/scripts (with private or public keys, redeem script (P2SH)), rescanning all addresses in one-shot-only (rescan can be disabled via options). Requires a new wallet backup.\n\n" + "importmulti \"requests\" ( \"options\" )\n" + "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), rescanning all addresses in one-shot-only (rescan can be disabled via options). Requires a new wallet backup.\n\n" "Arguments:\n" "1. requests (array, required) Data to be imported\n" " [ (array of json objects)\n" @@ -1096,7 +1103,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) " \"witnessscript\": \"<script>\" , (string, optional) Allowed only if the scriptPubKey is a P2SH-P2WSH or P2WSH address/scriptPubKey\n" " \"pubkeys\": [\"<pubKey>\", ... ] , (array, optional) Array of strings giving pubkeys that must occur in the output or redeemscript\n" " \"keys\": [\"<key>\", ... ] , (array, optional) Array of strings giving private keys whose corresponding public keys must occur in the output or redeemscript\n" - " \"internal\": <true> , (boolean, optional, default: false) Stating whether matching outputs should be treated as not incoming payments\n" + " \"internal\": <true> , (boolean, optional, default: false) Stating whether matching outputs should be treated as not incoming payments aka change\n" " \"watchonly\": <true> , (boolean, optional, default: false) Stating whether matching outputs should be considered watched even when they're not spendable, only allowed if keys are empty\n" " \"label\": <label> , (string, optional, default: '') Label to assign to the address, only allowed with internal=false\n" " }\n" diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index bfac990639..0e2f8864b1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -693,7 +693,7 @@ static UniValue getbalance(const JSONRPCRequest& request) if (request.fHelp || (request.params.size() > 3 )) throw std::runtime_error( - "getbalance ( \"(dummy)\" minconf include_watchonly )\n" + "getbalance ( \"dummy\" minconf include_watchonly )\n" "\nReturns the total available balance.\n" "The available balance is what the wallet considers currently spendable, and is\n" "thus affected by options which limit spendability such as -spendzeroconfchange.\n" @@ -1261,25 +1261,26 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) /** * List transactions based on the given criteria. * - * @param pwallet The wallet. - * @param wtx The wallet transaction. - * @param nMinDepth The minimum confirmation depth. - * @param fLong Whether to include the JSON version of the transaction. - * @param ret The UniValue into which the result is stored. - * @param filter The "is mine" filter bool. + * @param pwallet The wallet. + * @param wtx The wallet transaction. + * @param nMinDepth The minimum confirmation depth. + * @param fLong Whether to include the JSON version of the transaction. + * @param ret The UniValue into which the result is stored. + * @param filter_ismine The "is mine" filter flags. + * @param filter_label Optional label string to filter incoming transactions. */ -static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter) +static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) { CAmount nFee; std::list<COutputEntry> listReceived; std::list<COutputEntry> listSent; - wtx.GetAmounts(listReceived, listSent, nFee, filter); + wtx.GetAmounts(listReceived, listSent, nFee, filter_ismine); bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY); // Sent - if ((!listSent.empty() || nFee != 0)) + if (!filter_label) { for (const COutputEntry& s : listSent) { @@ -1311,6 +1312,9 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con if (pwallet->mapAddressBook.count(r.destination)) { label = pwallet->mapAddressBook[r.destination].name; } + if (filter_label && label != *filter_label) { + continue; + } UniValue entry(UniValue::VOBJ); if (involvesWatchonly || (::IsMine(*pwallet, r.destination) & ISMINE_WATCH_ONLY)) { entry.pushKV("involvesWatchonly", true); @@ -1352,10 +1356,12 @@ UniValue listtransactions(const JSONRPCRequest& request) if (request.fHelp || request.params.size() > 4) throw std::runtime_error( - "listtransactions (dummy count skip include_watchonly)\n" + "listtransactions ( \"label\" count skip include_watchonly )\n" + "\nIf a label name is provided, this will return only incoming transactions paying to addresses with the specified label.\n" "\nReturns up to 'count' most recent transactions skipping the first 'from' transactions.\n" "\nArguments:\n" - "1. \"dummy\" (string, optional) If set, should be \"*\" for backwards compatibility.\n" + "1. \"label\" (string, optional) If set, should be a valid label name to return only incoming transactions\n" + " with the specified label, or \"*\" to disable filtering and return all transactions.\n" "2. count (numeric, optional, default=10) The number of transactions to return\n" "3. skip (numeric, optional, default=0) The number of transactions to skip\n" "4. include_watchonly (bool, optional, default=false) Include transactions to watch-only addresses (see 'importaddress')\n" @@ -1400,8 +1406,12 @@ UniValue listtransactions(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now pwallet->BlockUntilSyncedToCurrentChain(); + const std::string* filter_label = nullptr; if (!request.params[0].isNull() && request.params[0].get_str() != "*") { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Dummy value must be set to \"*\""); + filter_label = &request.params[0].get_str(); + if (filter_label->empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Label argument must be a valid label name or \"*\"."); + } } int nCount = 10; if (!request.params[1].isNull()) @@ -1431,7 +1441,7 @@ UniValue listtransactions(const JSONRPCRequest& request) for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) { CWalletTx *const pwtx = (*it).second; - ListTransactions(*locked_chain, pwallet, *pwtx, 0, true, ret, filter); + ListTransactions(*locked_chain, pwallet, *pwtx, 0, true, ret, filter, filter_label); if ((int)ret.size() >= (nCount+nFrom)) break; } } @@ -1568,7 +1578,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) CWalletTx tx = pairWtx.second; if (depth == -1 || tx.GetDepthInMainChain(*locked_chain) < depth) { - ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter); + ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); } } @@ -1585,7 +1595,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) if (it != pwallet->mapWallet.end()) { // We want all transactions regardless of confirmation count to appear here, // even negative confirmation ones, hence the big negative. - ListTransactions(*locked_chain, pwallet, it->second, -100000000, true, removed, filter); + ListTransactions(*locked_chain, pwallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */); } } paltindex = paltindex->pprev; @@ -1688,7 +1698,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) WalletTxToJSON(pwallet->chain(), *locked_chain, wtx, entry); UniValue details(UniValue::VARR); - ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter); + ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */); entry.pushKV("details", details); std::string strHex = EncodeHexTx(*wtx.tx, RPCSerializationFlags()); @@ -2075,7 +2085,21 @@ static UniValue lockunspent(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) throw std::runtime_error( - "lockunspent unlock ([{\"txid\":\"txid\",\"vout\":n},...])\n" + RPCHelpMan{"lockunspent", + { + {"unlock", RPCArg::Type::BOOL, false}, + {"transactions", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"txid", RPCArg::Type::STR_HEX, false}, + {"vout", RPCArg::Type::NUM, false}, + }, + true}, + }, + true}, + }} + .ToString() + "\nUpdates list of temporarily unspendable outputs.\n" "Temporarily lock (unlock=false) or unlock (unlock=true) specified transaction outputs.\n" "If no transaction outputs are specified when unlocking then all current locked transaction outputs are unlocked.\n" @@ -2620,7 +2644,26 @@ static UniValue listunspent(const JSONRPCRequest& request) if (request.fHelp || request.params.size() > 5) throw std::runtime_error( - "listunspent ( minconf maxconf [\"addresses\",...] [include_unsafe] [query_options])\n" + RPCHelpMan{"listunspent", + { + {"minconf", RPCArg::Type::NUM, true}, + {"maxconf", RPCArg::Type::NUM, true}, + {"addresses", RPCArg::Type::ARR, + { + {"address", RPCArg::Type::STR_HEX, true}, + }, + true}, + {"include_unsafe", RPCArg::Type::BOOL, true}, + {"query_options", RPCArg::Type::OBJ, + { + {"minimumAmount", RPCArg::Type::AMOUNT, true}, + {"maximumAmount", RPCArg::Type::AMOUNT, true}, + {"maximumCount", RPCArg::Type::NUM, true}, + {"minimumSumAmount", RPCArg::Type::AMOUNT, true}, + }, + true}, + }} + .ToString() + "\nReturns array of unspent transaction outputs\n" "with between minconf and maxconf (inclusive) confirmations.\n" "Optionally filter to only include txouts paid to specified addresses.\n" @@ -2995,7 +3038,25 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) throw std::runtime_error( - "signrawtransactionwithwallet \"hexstring\" ( [{\"txid\":\"id\",\"vout\":n,\"scriptPubKey\":\"hex\",\"redeemScript\":\"hex\"},...] sighashtype )\n" + RPCHelpMan{"signrawtransactionwithwallet", + { + {"hexstring", RPCArg::Type::STR, false}, + {"prevtxs", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"txid", RPCArg::Type::STR_HEX, false}, + {"vout", RPCArg::Type::NUM, false}, + {"scriptPubKey", RPCArg::Type::STR_HEX, false}, + {"redeemScript", RPCArg::Type::STR_HEX, false}, + {"amount", RPCArg::Type::AMOUNT, false}, + }, + false}, + }, + true}, + {"sighashtype", RPCArg::Type::STR, true}, + }} + .ToString() + "\nSign inputs for raw transaction (serialized, hex-encoded).\n" "The second optional argument (may be null) is an array of previous transaction outputs that\n" "this transaction depends on but may not yet be in the block chain.\n" @@ -3902,7 +3963,55 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 2 || request.params.size() > 5) throw std::runtime_error( - "walletcreatefundedpsbt [{\"txid\":\"id\",\"vout\":n},...] [{\"address\":amount},{\"data\":\"hex\"},...] ( locktime ) ( replaceable ) ( options bip32derivs )\n" + RPCHelpMan{"walletcreatefundedpsbt", + { + {"inputs", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"txid", RPCArg::Type::STR_HEX, false}, + {"vout", RPCArg::Type::NUM, false}, + {"sequence", RPCArg::Type::NUM, false}, + }, + false}, + }, + false}, + {"outputs", RPCArg::Type::ARR, + { + {"", RPCArg::Type::OBJ, + { + {"address", RPCArg::Type::AMOUNT, true}, + }, + true}, + {"", RPCArg::Type::OBJ, + { + {"data", RPCArg::Type::STR_HEX, true}, + }, + true}, + }, + false}, + {"locktime", RPCArg::Type::NUM, true}, + {"options", RPCArg::Type::OBJ, + { + {"changeAddress", RPCArg::Type::STR_HEX, true}, + {"changePosition", RPCArg::Type::NUM, true}, + {"change_type", RPCArg::Type::STR, true}, + {"includeWatching", RPCArg::Type::BOOL, true}, + {"lockUnspents", RPCArg::Type::BOOL, true}, + {"feeRate", RPCArg::Type::NUM, true}, + {"subtractFeeFromOutputs", RPCArg::Type::ARR, + { + {"int", RPCArg::Type::NUM, true}, + }, + true}, + {"replaceable", RPCArg::Type::BOOL, true}, + {"conf_target", RPCArg::Type::NUM, true}, + {"estimate_mode", RPCArg::Type::STR, true}, + }, + true}, + {"bip32derivs", RPCArg::Type::BOOL, true}, + }} + .ToString() + "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n" "Implements the Creator and Updater roles.\n" "\nArguments:\n" @@ -4044,7 +4153,7 @@ static const CRPCCommand commands[] = { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} }, { "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} }, { "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} }, - { "wallet", "listtransactions", &listtransactions, {"dummy","count","skip","include_watchonly"} }, + { "wallet", "listtransactions", &listtransactions, {"label|dummy","count","skip","include_watchonly"} }, { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, { "wallet", "listwalletdir", &listwalletdir, {} }, { "wallet", "listwallets", &listwallets, {} }, diff --git a/src/zmq/zmqrpc.cpp b/src/zmq/zmqrpc.cpp index 0e0b745375..66b491427d 100644 --- a/src/zmq/zmqrpc.cpp +++ b/src/zmq/zmqrpc.cpp @@ -5,6 +5,7 @@ #include <zmq/zmqrpc.h> #include <rpc/server.h> +#include <rpc/util.h> #include <zmq/zmqabstractnotifier.h> #include <zmq/zmqnotificationinterface.h> diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 628cefb76d..e386915ada 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -824,7 +824,7 @@ class FullBlockTest(BitcoinTestFramework): tx.vin.append(CTxIn(COutPoint(b64a.vtx[1].sha256, 0))) b64a = self.update_block("64a", [tx]) assert_equal(len(b64a.serialize()), MAX_BLOCK_BASE_SIZE + 8) - self.sync_blocks([b64a], success=False, reject_reason='non-canonical ReadCompactSize():') + self.sync_blocks([b64a], success=False, reject_reason='non-canonical ReadCompactSize()') # bitcoind doesn't disconnect us for sending a bloated block, but if we subsequently # resend the header message, it won't send us the getdata message again. Just diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index c820ca33e2..c162f46d63 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -249,7 +249,7 @@ class PruneTest(BitcoinTestFramework): return index def prune(index, expected_ret=None): - ret = node.pruneblockchain(height(index)) + ret = node.pruneblockchain(height=height(index)) # Check the return value. When use_timestamp is True, just check # that the return value is less than or equal to the expected # value, because when more than one block is generated per second, diff --git a/test/functional/mining_basic.py b/test/functional/mining_basic.py index ff55ea5528..9f01be0646 100755 --- a/test/functional/mining_basic.py +++ b/test/functional/mining_basic.py @@ -30,9 +30,10 @@ from test_framework.util import ( def assert_template(node, block, expect, rehash=True): if rehash: block.hashMerkleRoot = block.calc_merkle_root() - rsp = node.getblocktemplate({'data': b2x(block.serialize()), 'mode': 'proposal'}) + rsp = node.getblocktemplate(template_request={'data': b2x(block.serialize()), 'mode': 'proposal'}) assert_equal(rsp, expect) + class MiningTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 67f24d6bff..1b11a2a294 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -22,7 +22,7 @@ class DisconnectBanTest(BitcoinTestFramework): self.log.info("setban: successfully ban single IP address") assert_equal(len(self.nodes[1].getpeerinfo()), 2) # node1 should have 2 connections to node0 at this point - self.nodes[1].setban("127.0.0.1", "add") + self.nodes[1].setban(subnet="127.0.0.1", command="add") wait_until(lambda: len(self.nodes[1].getpeerinfo()) == 0, timeout=10) assert_equal(len(self.nodes[1].getpeerinfo()), 0) # all nodes must be disconnected at this point assert_equal(len(self.nodes[1].listbanned()), 1) diff --git a/test/functional/p2p_invalid_block.py b/test/functional/p2p_invalid_block.py index 0678b1a651..1e0b876593 100755 --- a/test/functional/p2p_invalid_block.py +++ b/test/functional/p2p_invalid_block.py @@ -77,9 +77,9 @@ class InvalidBlockRequestTest(BitcoinTestFramework): block2.vtx.append(tx2) assert_equal(block2.hashMerkleRoot, block2.calc_merkle_root()) assert_equal(orig_hash, block2.rehash()) - assert(block2_orig.vtx != block2.vtx) + assert block2_orig.vtx != block2.vtx - node.p2p.send_blocks_and_test([block2], node, success=False, request_block=False, reject_reason='bad-txns-duplicate') + node.p2p.send_blocks_and_test([block2], node, success=False, reject_reason='bad-txns-duplicate') # Check transactions for duplicate inputs self.log.info("Test duplicate input block.") @@ -89,7 +89,7 @@ class InvalidBlockRequestTest(BitcoinTestFramework): block2_orig.hashMerkleRoot = block2_orig.calc_merkle_root() block2_orig.rehash() block2_orig.solve() - node.p2p.send_blocks_and_test([block2_orig], node, success=False, request_block=False, reject_reason='bad-txns-inputs-duplicate') + node.p2p.send_blocks_and_test([block2_orig], node, success=False, reject_reason='bad-txns-inputs-duplicate') self.log.info("Test very broken block.") @@ -102,7 +102,8 @@ class InvalidBlockRequestTest(BitcoinTestFramework): block3.rehash() block3.solve() - node.p2p.send_blocks_and_test([block3], node, success=False, request_block=False, reject_reason='bad-cb-amount') + node.p2p.send_blocks_and_test([block3], node, success=False, reject_reason='bad-cb-amount') + if __name__ == '__main__': InvalidBlockRequestTest().main() diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 92b690176d..31e60f1cea 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -133,7 +133,7 @@ class BlockchainTest(BitcoinTestFramework): assert_raises_rpc_error(-8, "Block is not in main chain", self.nodes[0].getchaintxstats, blockhash=blockhash) self.nodes[0].reconsiderblock(blockhash) - chaintxstats = self.nodes[0].getchaintxstats(1) + chaintxstats = self.nodes[0].getchaintxstats(nblocks=1) # 200 txs plus genesis tx assert_equal(chaintxstats['txcount'], 201) # tx rate should be 1 per 10 minutes, or 1/600 @@ -211,7 +211,7 @@ class BlockchainTest(BitcoinTestFramework): besthash = node.getbestblockhash() secondbesthash = node.getblockhash(199) - header = node.getblockheader(besthash) + header = node.getblockheader(blockhash=besthash) assert_equal(header['hash'], besthash) assert_equal(header['height'], 200) @@ -287,7 +287,7 @@ class BlockchainTest(BitcoinTestFramework): def assert_waitforheight(height, timeout=2): assert_equal( - node.waitforblockheight(height, timeout)['height'], + node.waitforblockheight(height=height, timeout=timeout)['height'], current_height) assert_waitforheight(0) diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 1e525214fa..0affddcf05 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -74,12 +74,12 @@ class NetTest(BitcoinTestFramework): assert_equal(self.nodes[0].getnetworkinfo()['networkactive'], True) assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2) - self.nodes[0].setnetworkactive(False) + self.nodes[0].setnetworkactive(state=False) assert_equal(self.nodes[0].getnetworkinfo()['networkactive'], False) # Wait a bit for all sockets to close wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 0, timeout=3) - self.nodes[0].setnetworkactive(True) + self.nodes[0].setnetworkactive(state=True) connect_nodes_bi(self.nodes, 0, 1) assert_equal(self.nodes[0].getnetworkinfo()['networkactive'], True) assert_equal(self.nodes[0].getnetworkinfo()['connections'], 2) @@ -88,7 +88,7 @@ class NetTest(BitcoinTestFramework): assert_equal(self.nodes[0].getaddednodeinfo(), []) # add a node (node2) to node0 ip_port = "127.0.0.1:{}".format(p2p_port(2)) - self.nodes[0].addnode(ip_port, 'add') + self.nodes[0].addnode(node=ip_port, command='add') # check that the node has indeed been added added_nodes = self.nodes[0].getaddednodeinfo(ip_port) assert_equal(len(added_nodes), 1) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 90b333f45a..5541b44690 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -155,6 +155,7 @@ BASE_SCRIPTS = [ 'feature_nulldummy.py', 'mempool_accept.py', 'wallet_import_rescan.py', + 'wallet_import_with_label.py', 'rpc_bind.py --ipv4', 'rpc_bind.py --ipv6', 'rpc_bind.py --nonloopback', @@ -365,7 +366,7 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage= tmpdir=tmpdir, test_list=test_list, flags=flags, - timeout_duration=20 * 60 if runs_ci else float('inf'), # in seconds + timeout_duration=40 * 60 if runs_ci else float('inf'), # in seconds ) start_time = time.time() test_results = [] diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 08809a688a..46462a16f3 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -46,11 +46,11 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")): if self.call == Call.single: if self.data == Data.address: - response = self.try_rpc(self.node.importaddress, address=self.address["address"], rescan=rescan) + response = self.try_rpc(self.node.importaddress, address=self.address["address"], label=self.label, rescan=rescan) elif self.data == Data.pub: - response = self.try_rpc(self.node.importpubkey, pubkey=self.address["pubkey"], rescan=rescan) + response = self.try_rpc(self.node.importpubkey, pubkey=self.address["pubkey"], label=self.label, rescan=rescan) elif self.data == Data.priv: - response = self.try_rpc(self.node.importprivkey, privkey=self.key, rescan=rescan) + response = self.try_rpc(self.node.importprivkey, privkey=self.key, label=self.label, rescan=rescan) assert_equal(response, None) elif self.call in (Call.multiaddress, Call.multiscript): @@ -61,18 +61,32 @@ class Variant(collections.namedtuple("Variant", "call data rescan prune")): "timestamp": timestamp + TIMESTAMP_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0), "pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [], "keys": [self.key] if self.data == Data.priv else [], + "label": self.label, "watchonly": self.data != Data.priv }], {"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)}) assert_equal(response, [{"success": True}]) def check(self, txid=None, amount=None, confirmations=None): - """Verify that listreceivedbyaddress returns expected values.""" + """Verify that listtransactions/listreceivedbyaddress return expected values.""" + + txs = self.node.listtransactions(label=self.label, count=10000, include_watchonly=True) + assert_equal(len(txs), self.expected_txs) addresses = self.node.listreceivedbyaddress(minconf=0, include_watchonly=True, address_filter=self.address['address']) if self.expected_txs: assert_equal(len(addresses[0]["txids"]), self.expected_txs) if txid is not None: + tx, = [tx for tx in txs if tx["txid"] == txid] + assert_equal(tx["label"], self.label) + assert_equal(tx["address"], self.address["address"]) + assert_equal(tx["amount"], amount) + assert_equal(tx["category"], "receive") + assert_equal(tx["label"], self.label) + assert_equal(tx["txid"], txid) + assert_equal(tx["confirmations"], confirmations) + assert_equal("trusted" not in tx, True) + address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) assert_equal(address["amount"], self.expected_balance) @@ -134,7 +148,8 @@ class ImportRescanTest(BitcoinTestFramework): # Create one transaction on node 0 with a unique amount for # each possible type of wallet import RPC. for i, variant in enumerate(IMPORT_VARIANTS): - variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress()) + variant.label = "label {} {}".format(i, variant) + variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress(variant.label)) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) variant.initial_amount = 1 - (i + 1) / 64 variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py new file mode 100755 index 0000000000..95acaa752e --- /dev/null +++ b/test/functional/wallet_import_with_label.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test the behavior of RPC importprivkey on set and unset labels of +addresses. + +It tests different cases in which an address is imported with importaddress +with or without a label and then its private key is imported with importprivkey +with and without a label. +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +class ImportWithLabel(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + """Main test logic""" + + self.log.info( + "Test importaddress with label and importprivkey without label." + ) + self.log.info("Import a watch-only address with a label.") + address = self.nodes[0].getnewaddress() + label = "Test Label" + self.nodes[1].importaddress(address, label) + address_assert = self.nodes[1].getaddressinfo(address) + + assert_equal(address_assert["iswatchonly"], True) + assert_equal(address_assert["ismine"], False) + assert_equal(address_assert["label"], label) + + self.log.info( + "Import the watch-only address's private key without a " + "label and the address should keep its label." + ) + priv_key = self.nodes[0].dumpprivkey(address) + self.nodes[1].importprivkey(priv_key) + + assert_equal(label, self.nodes[1].getaddressinfo(address)["label"]) + + self.log.info( + "Test importaddress without label and importprivkey with label." + ) + self.log.info("Import a watch-only address without a label.") + address2 = self.nodes[0].getnewaddress() + self.nodes[1].importaddress(address2) + address_assert2 = self.nodes[1].getaddressinfo(address2) + + assert_equal(address_assert2["iswatchonly"], True) + assert_equal(address_assert2["ismine"], False) + assert_equal(address_assert2["label"], "") + + self.log.info( + "Import the watch-only address's private key with a " + "label and the address should have its label updated." + ) + priv_key2 = self.nodes[0].dumpprivkey(address2) + label2 = "Test Label 2" + self.nodes[1].importprivkey(priv_key2, label2) + + assert_equal(label2, self.nodes[1].getaddressinfo(address2)["label"]) + + self.log.info("Test importaddress with label and importprivkey with label.") + self.log.info("Import a watch-only address with a label.") + address3 = self.nodes[0].getnewaddress() + label3_addr = "Test Label 3 for importaddress" + self.nodes[1].importaddress(address3, label3_addr) + address_assert3 = self.nodes[1].getaddressinfo(address3) + + assert_equal(address_assert3["iswatchonly"], True) + assert_equal(address_assert3["ismine"], False) + assert_equal(address_assert3["label"], label3_addr) + + self.log.info( + "Import the watch-only address's private key with a " + "label and the address should have its label updated." + ) + priv_key3 = self.nodes[0].dumpprivkey(address3) + label3_priv = "Test Label 3 for importprivkey" + self.nodes[1].importprivkey(priv_key3, label3_priv) + + assert_equal(label3_priv, self.nodes[1].getaddressinfo(address3)["label"]) + + self.log.info( + "Test importprivkey won't label new dests with the same " + "label as others labeled dests for the same key." + ) + self.log.info("Import a watch-only legacy address with a label.") + address4 = self.nodes[0].getnewaddress() + label4_addr = "Test Label 4 for importaddress" + self.nodes[1].importaddress(address4, label4_addr) + address_assert4 = self.nodes[1].getaddressinfo(address4) + + assert_equal(address_assert4["iswatchonly"], True) + assert_equal(address_assert4["ismine"], False) + assert_equal(address_assert4["label"], label4_addr) + + self.log.info("Asserts address has no embedded field with dests.") + + assert_equal(address_assert4.get("embedded"), None) + + self.log.info( + "Import the watch-only address's private key without a " + "label and new destinations for the key should have an " + "empty label while the 'old' destination should keep " + "its label." + ) + priv_key4 = self.nodes[0].dumpprivkey(address4) + self.nodes[1].importprivkey(priv_key4) + address_assert4 = self.nodes[1].getaddressinfo(address4) + + assert address_assert4.get("embedded") + + bcaddress_assert = self.nodes[1].getaddressinfo( + address_assert4["embedded"]["address"] + ) + + assert_equal(address_assert4["label"], label4_addr) + assert_equal(bcaddress_assert["label"], "") + + self.stop_nodes() + + +if __name__ == "__main__": + ImportWithLabel().main() diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 9ba6860306..5c789b1c03 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -54,7 +54,7 @@ class ImportMultiTest(BitcoinTestFramework): # RPC importmulti ----------------------------------------------- - # Bitcoin Address + # Bitcoin Address (implicit non-internal) self.log.info("Should import an address") address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) result = self.nodes[1].importmulti([{ @@ -68,6 +68,7 @@ class ImportMultiTest(BitcoinTestFramework): assert_equal(address_assert['iswatchonly'], True) assert_equal(address_assert['ismine'], False) assert_equal(address_assert['timestamp'], timestamp) + assert_equal(address_assert['ischange'], False) watchonly_address = address['address'] watchonly_timestamp = timestamp @@ -95,6 +96,7 @@ class ImportMultiTest(BitcoinTestFramework): assert_equal(address_assert['iswatchonly'], True) assert_equal(address_assert['ismine'], False) assert_equal(address_assert['timestamp'], timestamp) + assert_equal(address_assert['ischange'], True) # ScriptPubKey + internal + label self.log.info("Should not allow a label to be specified when internal is true") @@ -126,7 +128,7 @@ class ImportMultiTest(BitcoinTestFramework): assert_equal('timestamp' in address_assert, False) - # Address + Public key + !Internal + # Address + Public key + !Internal(explicit) self.log.info("Should import an address with public key") address = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) result = self.nodes[1].importmulti([{ @@ -134,7 +136,8 @@ class ImportMultiTest(BitcoinTestFramework): "address": address['address'] }, "timestamp": "now", - "pubkeys": [ address['pubkey'] ] + "pubkeys": [ address['pubkey'] ], + "internal": False }]) assert_equal(result[0]['success'], True) address_assert = self.nodes[1].getaddressinfo(address['address']) @@ -152,7 +155,7 @@ class ImportMultiTest(BitcoinTestFramework): "pubkeys": [ address['pubkey'] ], "internal": True }] - result = self.nodes[1].importmulti(request) + result = self.nodes[1].importmulti(requests=request) assert_equal(result[0]['success'], True) address_assert = self.nodes[1].getaddressinfo(address['address']) assert_equal(address_assert['iswatchonly'], True) @@ -167,7 +170,7 @@ class ImportMultiTest(BitcoinTestFramework): "timestamp": "now", "pubkeys": [ address['pubkey'] ] }] - result = self.nodes[1].importmulti(request) + result = self.nodes[1].importmulti(requests=request) assert_equal(result[0]['success'], False) assert_equal(result[0]['error']['code'], -8) assert_equal(result[0]['error']['message'], 'Internal must be set to true for nonstandard scriptPubKey imports.') diff --git a/test/functional/wallet_importprunedfunds.py b/test/functional/wallet_importprunedfunds.py index 26b181db33..78426018ef 100755 --- a/test/functional/wallet_importprunedfunds.py +++ b/test/functional/wallet_importprunedfunds.py @@ -81,7 +81,7 @@ class ImportPrunedFundsTest(BitcoinTestFramework): # Import with affiliated address with no rescan self.nodes[1].importaddress(address=address2, rescan=False) - self.nodes[1].importprunedfunds(rawtxn2, proof2) + self.nodes[1].importprunedfunds(rawtransaction=rawtxn2, txoutproof=proof2) assert [tx for tx in self.nodes[1].listtransactions(include_watchonly=True) if tx['txid'] == txnid2] # Import with private key with no rescan diff --git a/test/functional/wallet_listtransactions.py b/test/functional/wallet_listtransactions.py index 5a17395abd..8ca0387268 100755 --- a/test/functional/wallet_listtransactions.py +++ b/test/functional/wallet_listtransactions.py @@ -97,9 +97,10 @@ class ListTransactionsTest(BitcoinTestFramework): txid = self.nodes[1].sendtoaddress(multisig["address"], 0.1) self.nodes[1].generate(1) self.sync_all() - assert not [tx for tx in self.nodes[0].listtransactions(dummy="*", count=100, skip=0, include_watchonly=False) if "label" in tx and tx["label"] == "watchonly"] - txs = [tx for tx in self.nodes[0].listtransactions(dummy="*", count=100, skip=0, include_watchonly=True) if "label" in tx and tx['label'] == 'watchonly'] - assert_array_result(txs, {"category": "receive", "amount": Decimal("0.1")}, {"txid": txid}) + assert len(self.nodes[0].listtransactions(label="watchonly", count=100, include_watchonly=False)) == 0 + assert_array_result(self.nodes[0].listtransactions(label="watchonly", count=100, include_watchonly=True), + {"category": "receive", "amount": Decimal("0.1")}, + {"txid": txid, "label": "watchonly"}) self.run_rbf_opt_in_test() |