diff options
89 files changed, 390 insertions, 371 deletions
diff --git a/build-aux/m4/ax_boost_chrono.m4 b/build-aux/m4/ax_boost_chrono.m4 index 318ecea17f..e9b0f2061c 100644 --- a/build-aux/m4/ax_boost_chrono.m4 +++ b/build-aux/m4/ax_boost_chrono.m4 @@ -1,5 +1,5 @@ # =========================================================================== -# http://www.gnu.org/software/autoconf-archive/ax_boost_chrono.html +# https://www.gnu.org/software/autoconf-archive/ax_boost_chrono.html # =========================================================================== # # SYNOPSIS @@ -8,7 +8,7 @@ # # DESCRIPTION # -# Test for System library from the Boost C++ libraries. The macro requires +# Test for Chrono library from the Boost C++ libraries. The macro requires # a preceding call to AX_BOOST_BASE. Further documentation is available at # <http://randspringer.de/boost/index.html>. # @@ -29,7 +29,7 @@ # and this notice are preserved. This file is offered as-is, without any # warranty. -#serial 1 +#serial 4 AC_DEFUN([AX_BOOST_CHRONO], [ @@ -68,7 +68,7 @@ AC_DEFUN([AX_BOOST_CHRONO], CXXFLAGS_SAVE=$CXXFLAGS AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[@%:@include <boost/chrono.hpp>]], - [[boost::chrono::system_clock::time_point time;]])], + [[boost::chrono::system_clock::time_point* time = new boost::chrono::system_clock::time_point; delete time;]])], ax_cv_boost_chrono=yes, ax_cv_boost_chrono=no) CXXFLAGS=$CXXFLAGS_SAVE AC_LANG_POP([C++]) diff --git a/build-aux/m4/ax_boost_unit_test_framework.m4 b/build-aux/m4/ax_boost_unit_test_framework.m4 index 4efd1e2f18..0cdbe752cf 100644 --- a/build-aux/m4/ax_boost_unit_test_framework.m4 +++ b/build-aux/m4/ax_boost_unit_test_framework.m4 @@ -1,6 +1,6 @@ -# ================================================================================ -# http://www.gnu.org/software/autoconf-archive/ax_boost_unit_test_framework.html -# ================================================================================ +# ================================================================================= +# https://www.gnu.org/software/autoconf-archive/ax_boost_unit_test_framework.html +# ================================================================================= # # SYNOPSIS # @@ -29,7 +29,7 @@ # and this notice are preserved. This file is offered as-is, without any # warranty. -#serial 19 +#serial 21 AC_DEFUN([AX_BOOST_UNIT_TEST_FRAMEWORK], [ @@ -66,7 +66,7 @@ AC_DEFUN([AX_BOOST_UNIT_TEST_FRAMEWORK], [AC_LANG_PUSH([C++]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[@%:@include <boost/test/unit_test.hpp>]], [[using boost::unit_test::test_suite; - test_suite* test= BOOST_TEST_SUITE( "Unit test example 1" ); return 0;]])], + test_suite* test= BOOST_TEST_SUITE( "Unit test example 1" ); if (test == NULL) { return 1; } else { return 0; }]])], ax_cv_boost_unit_test_framework=yes, ax_cv_boost_unit_test_framework=no) AC_LANG_POP([C++]) ]) diff --git a/configure.ac b/configure.ac index 18f707f0ba..4f57750a1a 100644 --- a/configure.ac +++ b/configure.ac @@ -643,22 +643,6 @@ AC_CHECK_DECLS([bswap_16, bswap_32, bswap_64],,, AC_CHECK_DECLS([__builtin_clz, __builtin_clzl, __builtin_clzll]) -dnl Check for MSG_NOSIGNAL -AC_MSG_CHECKING(for MSG_NOSIGNAL) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <sys/socket.h>]], - [[ int f = MSG_NOSIGNAL; ]])], - [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_MSG_NOSIGNAL, 1,[Define this symbol if you have MSG_NOSIGNAL]) ], - [ AC_MSG_RESULT(no)] -) - -dnl Check for MSG_DONTWAIT -AC_MSG_CHECKING(for MSG_DONTWAIT) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <sys/socket.h>]], - [[ int f = MSG_DONTWAIT; ]])], - [ AC_MSG_RESULT(yes); AC_DEFINE(HAVE_MSG_DONTWAIT, 1,[Define this symbol if you have MSG_DONTWAIT]) ], - [ AC_MSG_RESULT(no)] -) - dnl Check for malloc_info (for memory statistics information in getmemoryinfo) AC_MSG_CHECKING(for getmemoryinfo) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <malloc.h>]], @@ -771,7 +755,7 @@ dnl Check for libminiupnpc (optional) if test x$use_upnp != xno; then AC_CHECK_HEADERS( [miniupnpc/miniwget.h miniupnpc/miniupnpc.h miniupnpc/upnpcommands.h miniupnpc/upnperrors.h], - [AC_CHECK_LIB([miniupnpc], [main],[MINIUPNPC_LIBS=-lminiupnpc], [have_miniupnpc=no])], + [AC_CHECK_LIB([miniupnpc], [upnpDiscover], [MINIUPNPC_LIBS=-lminiupnpc], [have_miniupnpc=no])], [have_miniupnpc=no] ) fi diff --git a/doc/developer-notes.md b/doc/developer-notes.md index a5468c3be3..8f06ee4eca 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -710,3 +710,14 @@ A few guidelines for introducing and reviewing new RPC interfaces: client may be aware of prior to entering a wallet RPC call, we must block until the wallet is caught up to the chainstate as of the RPC call's entry. This also makes the API much easier for RPC clients to reason about. + +- Be aware of RPC method aliases and generally avoid registering the same + callback function pointer for different RPCs. + + - *Rationale*: RPC methods registered with the same function pointer will be + considered aliases and only the first method name will show up in the + `help` rpc command list. + + - *Exception*: Using RPC method aliases may be appropriate in cases where a + new RPC is replacing a deprecated RPC, to avoid both RPCs confusingly + showing up in the command list. diff --git a/src/addrman.h b/src/addrman.h index 67423c6c55..6dec3fe416 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -218,7 +218,7 @@ private: //! last time Good was called (memory only) int64_t nLastGood; - //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discpline used to resolve these collisions. + //! Holds addrs inserted into tried table that collide with existing entries. Test-before-evict discipline used to resolve these collisions. std::set<int> m_tried_collisions; protected: diff --git a/src/base58.cpp b/src/base58.cpp index 982e123a1d..feec2d4e05 100644 --- a/src/base58.cpp +++ b/src/base58.cpp @@ -12,6 +12,24 @@ /** All alphanumeric characters except for "0", "I", "O", and "l" */ static const char* pszBase58 = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"; +static const int8_t mapBase58[256] = { + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1, 0, 1, 2, 3, 4, 5, 6, 7, 8,-1,-1,-1,-1,-1,-1, + -1, 9,10,11,12,13,14,15, 16,-1,17,18,19,20,21,-1, + 22,23,24,25,26,27,28,29, 30,31,32,-1,-1,-1,-1,-1, + -1,33,34,35,36,37,38,39, 40,41,42,43,-1,44,45,46, + 47,48,49,50,51,52,53,54, 55,56,57,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, + -1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1, +}; bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch) { @@ -29,13 +47,12 @@ bool DecodeBase58(const char* psz, std::vector<unsigned char>& vch) int size = strlen(psz) * 733 /1000 + 1; // log(58) / log(256), rounded up. std::vector<unsigned char> b256(size); // Process the characters. + static_assert(sizeof(mapBase58)/sizeof(mapBase58[0]) == 256, "mapBase58.size() should be 256"); // guarantee not out of range while (*psz && !isspace(*psz)) { // Decode base58 character - const char* ch = strchr(pszBase58, *psz); - if (ch == nullptr) + int carry = mapBase58[(uint8_t)*psz]; + if (carry == -1) // Invalid b58 character return false; - // Apply "b256 = b256 * 58 + ch". - int carry = ch - pszBase58; int i = 0; for (std::vector<unsigned char>::reverse_iterator it = b256.rbegin(); (carry != 0 || i < length) && (it != b256.rend()); ++it, ++i) { carry += 58 * (*it); diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 41f1e5786c..8bdc210997 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -313,13 +313,11 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co // Get credentials std::string strRPCUserColonPass; + bool failedToGetAuthCookie = false; if (gArgs.GetArg("-rpcpassword", "") == "") { // Try fall back to cookie-based authentication if no password is provided if (!GetAuthCookie(&strRPCUserColonPass)) { - throw std::runtime_error(strprintf( - _("Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (%s)"), - GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string().c_str())); - + failedToGetAuthCookie = true; } } else { strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); @@ -358,11 +356,21 @@ static UniValue CallRPC(BaseRequestHandler *rh, const std::string& strMethod, co event_base_dispatch(base.get()); - if (response.status == 0) - throw CConnectionFailed(strprintf("couldn't connect to server: %s (code %d)\n(make sure server is running and you are connecting to the correct RPC port)", http_errorstring(response.error), response.error)); - else if (response.status == HTTP_UNAUTHORIZED) - throw std::runtime_error("incorrect rpcuser or rpcpassword (authorization failed)"); - else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR) + if (response.status == 0) { + std::string responseErrorMessage; + if (response.error != -1) { + responseErrorMessage = strprintf(" (error code %d - \"%s\")", response.error, http_errorstring(response.error)); + } + throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\nMake sure the bitcoind server is running and that you are connecting to the correct RPC port.", host, port, responseErrorMessage)); + } else if (response.status == HTTP_UNAUTHORIZED) { + if (failedToGetAuthCookie) { + throw std::runtime_error(strprintf( + _("Could not locate RPC credentials. No authentication cookie could be found, and RPC password is not set. See -rpcpassword and -stdinrpcpass. Configuration file: (%s)"), + GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string().c_str())); + } else { + throw std::runtime_error("Authorization failed: Incorrect rpcuser or rpcpassword"); + } + } else if (response.status >= 400 && response.status != HTTP_BAD_REQUEST && response.status != HTTP_NOT_FOUND && response.status != HTTP_INTERNAL_SERVER_ERROR) throw std::runtime_error(strprintf("server returned HTTP error %d", response.status)); else if (response.body.empty()) throw std::runtime_error("no response from server"); diff --git a/src/chain.h b/src/chain.h index 3728f768c4..757840bb23 100644 --- a/src/chain.h +++ b/src/chain.h @@ -91,7 +91,7 @@ struct CDiskBlockPos template <typename Stream, typename Operation> inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(VARINT(nFile)); + READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED)); READWRITE(VARINT(nPos)); } @@ -386,13 +386,13 @@ public: inline void SerializationOp(Stream& s, Operation ser_action) { int _nVersion = s.GetVersion(); if (!(s.GetType() & SER_GETHASH)) - READWRITE(VARINT(_nVersion)); + READWRITE(VARINT(_nVersion, VarIntMode::NONNEGATIVE_SIGNED)); - READWRITE(VARINT(nHeight)); + READWRITE(VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); READWRITE(VARINT(nStatus)); READWRITE(VARINT(nTx)); if (nStatus & (BLOCK_HAVE_DATA | BLOCK_HAVE_UNDO)) - READWRITE(VARINT(nFile)); + READWRITE(VARINT(nFile, VarIntMode::NONNEGATIVE_SIGNED)); if (nStatus & BLOCK_HAVE_DATA) READWRITE(VARINT(nDataPos)); if (nStatus & BLOCK_HAVE_UNDO) diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 757df518ae..28d3c4a119 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -101,7 +101,7 @@ static inline int64_t GetBlockWeight(const CBlock& block) { return ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION); } -static inline int64_t GetTransationInputWeight(const CTxIn& txin) +static inline int64_t GetTransactionInputWeight(const CTxIn& txin) { // scriptWitness size is added here because witnesses and txins are split up in segwit serialization. return ::GetSerializeSize(txin, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) * (WITNESS_SCALE_FACTOR - 1) + ::GetSerializeSize(txin, SER_NETWORK, PROTOCOL_VERSION) + ::GetSerializeSize(txin.scriptWitness.stack, SER_NETWORK, PROTOCOL_VERSION); diff --git a/src/core_write.cpp b/src/core_write.cpp index 91742b7d1b..54b18a4931 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -209,6 +209,6 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, entry.pushKV("blockhash", hashBlock.GetHex()); if (include_hex) { - entry.pushKV("hex", EncodeHexTx(tx, serialize_flags)); // the hex-encoded transaction. used the name "hex" to be consistent with the verbose output of "getrawtransaction". + entry.pushKV("hex", EncodeHexTx(tx, serialize_flags)); // The hex-encoded transaction. Used the name "hex" to be consistent with the verbose output of "getrawtransaction". } } diff --git a/src/cuckoocache.h b/src/cuckoocache.h index 947e1a7185..d1de712024 100644 --- a/src/cuckoocache.h +++ b/src/cuckoocache.h @@ -224,7 +224,7 @@ private: * * Instead we treat the 32-bit random number as a Q32 fixed-point number in the range * [0,1) and simply multiply it by the size. Then we just shift the result down by - * 32-bits to get our bucket number. The results has non-uniformity the same as a + * 32-bits to get our bucket number. The result has non-uniformity the same as a * mod, but it is much faster to compute. More about this technique can be found at * http://lemire.me/blog/2016/06/27/a-fast-alternative-to-the-modulo-reduction/ * diff --git a/src/net.cpp b/src/net.cpp index 53a0a9b180..342dfbaeb9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -42,12 +42,13 @@ // We add a random period time (0 to 1 seconds) to feeler connections to prevent synchronization. #define FEELER_SLEEP_WINDOW 1 -#if !defined(HAVE_MSG_NOSIGNAL) +// MSG_NOSIGNAL is not available on some platforms, if it doesn't exist define it as 0 +#if !defined(MSG_NOSIGNAL) #define MSG_NOSIGNAL 0 #endif // MSG_DONTWAIT is not available on some platforms, if it doesn't exist define it as 0 -#if !defined(HAVE_MSG_DONTWAIT) +#if !defined(MSG_DONTWAIT) #define MSG_DONTWAIT 0 #endif @@ -1954,7 +1955,7 @@ void CConnman::ThreadOpenAddedConnections() for (const AddedNodeInfo& info : vInfo) { if (!info.fConnected) { if (!grant.TryAcquire()) { - // If we've used up our semaphore and need a new one, lets not wait here since while we are waiting + // If we've used up our semaphore and need a new one, let's not wait here since while we are waiting // the addednodeinfo state might change. break; } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f5073fe903..61e6ae7448 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -339,7 +339,7 @@ bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlockIndex* CNodeState *state = State(nodeid); assert(state != nullptr); - // Short-circuit most stuff in case its from the same node + // Short-circuit most stuff in case it is from the same node std::map<uint256, std::pair<NodeId, std::list<QueuedBlock>::iterator> >::iterator itInFlight = mapBlocksInFlight.find(hash); if (itInFlight != mapBlocksInFlight.end() && itInFlight->second.first == nodeid) { if (pit) { @@ -518,7 +518,7 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<con } // Iterate over those blocks in vToFetch (in forward direction), adding the ones that - // are not yet downloaded and not in flight to vBlocks. In the mean time, update + // are not yet downloaded and not in flight to vBlocks. In the meantime, update // pindexLastCommonBlock as long as all ancestors are already downloaded, or if it's // already part of our chain (and therefore don't need it even if pruned). for (const CBlockIndex* pindex : vToFetch) { diff --git a/src/netbase.cpp b/src/netbase.cpp index 3ea3141d5e..92ac1c4c85 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -21,7 +21,7 @@ #include <boost/algorithm/string/case_conv.hpp> // for to_lower() #include <boost/algorithm/string/predicate.hpp> // for startswith() and endswith() -#if !defined(HAVE_MSG_NOSIGNAL) +#if !defined(MSG_NOSIGNAL) #define MSG_NOSIGNAL 0 #endif diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index 41f967c985..db1ad4f26d 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -261,5 +261,5 @@ int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost) int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost) { - return GetVirtualTransactionSize(GetTransationInputWeight(txin), nSigOpCost); + return GetVirtualTransactionSize(GetTransactionInputWeight(txin), nSigOpCost); } diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index 78dc9f81dd..f2ddbf259b 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -104,10 +104,12 @@ AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode, case SendingTab: ui->labelExplanation->setText(tr("These are your Bitcoin addresses for sending payments. Always check the amount and the receiving address before sending coins.")); ui->deleteAddress->setVisible(true); + ui->newAddress->setVisible(true); break; case ReceivingTab: ui->labelExplanation->setText(tr("These are your Bitcoin addresses for receiving payments. It is recommended to use a new receiving address for each transaction.")); ui->deleteAddress->setVisible(false); + ui->newAddress->setVisible(false); break; } @@ -211,10 +213,11 @@ void AddressBookPage::on_newAddress_clicked() if(!model) return; - EditAddressDialog dlg( - tab == SendingTab ? - EditAddressDialog::NewSendingAddress : - EditAddressDialog::NewReceivingAddress, this); + if (tab == ReceivingTab) { + return; + } + + EditAddressDialog dlg(EditAddressDialog::NewSendingAddress, this); dlg.setModel(model); if(dlg.exec()) { diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 4f9a79d654..801334483a 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -441,6 +441,8 @@ int AddressTableModel::lookupAddress(const QString &address) const } } +OutputType AddressTableModel::GetDefaultAddressType() const { return wallet->m_default_address_type; }; + void AddressTableModel::emitDataChanged(int idx) { Q_EMIT dataChanged(index(idx, 0, QModelIndex()), index(idx, columns.length()-1, QModelIndex())); diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index 11439e25d5..ed7a4e6f43 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -8,7 +8,7 @@ #include <QAbstractTableModel> #include <QStringList> -enum OutputType : int; +enum class OutputType; class AddressTablePriv; class WalletModel; @@ -76,6 +76,8 @@ public: EditStatus getEditStatus() const { return editStatus; } + OutputType GetDefaultAddressType() const; + private: WalletModel *walletModel; CWallet *wallet; diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index b83755ab30..a45e9f85c1 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -206,7 +206,7 @@ void CoinControlDialog::showMenu(const QPoint &point) contextMenuItem = item; // disable some items (like Copy Transaction ID, lock, unlock) for tree roots in context menu - if (item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means its a child node, so its not a parent node in tree mode) + if (item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode) { copyTransactionHashAction->setEnabled(true); if (model->isLockedCoin(uint256S(item->text(COLUMN_TXHASH).toStdString()), item->text(COLUMN_VOUT_INDEX).toUInt())) @@ -374,7 +374,7 @@ void CoinControlDialog::radioListMode(bool checked) // checkbox clicked by user void CoinControlDialog::viewItemChanged(QTreeWidgetItem* item, int column) { - if (column == COLUMN_CHECKBOX && item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means its a child node, so its not a parent node in tree mode) + if (column == COLUMN_CHECKBOX && item->text(COLUMN_TXHASH).length() == 64) // transaction hash is 64 characters (this means it is a child node, so it is not a parent node in tree mode) { COutPoint outpt(uint256S(item->text(COLUMN_TXHASH).toStdString()), item->text(COLUMN_VOUT_INDEX).toUInt()); diff --git a/src/qt/editaddressdialog.cpp b/src/qt/editaddressdialog.cpp index a945fc6aa0..38411c499f 100644 --- a/src/qt/editaddressdialog.cpp +++ b/src/qt/editaddressdialog.cpp @@ -11,7 +11,6 @@ #include <QDataWidgetMapper> #include <QMessageBox> -extern OutputType g_address_type; EditAddressDialog::EditAddressDialog(Mode _mode, QWidget *parent) : QDialog(parent), @@ -26,10 +25,6 @@ EditAddressDialog::EditAddressDialog(Mode _mode, QWidget *parent) : switch(mode) { - case NewReceivingAddress: - setWindowTitle(tr("New receiving address")); - ui->addressEdit->setEnabled(false); - break; case NewSendingAddress: setWindowTitle(tr("New sending address")); break; @@ -74,13 +69,12 @@ bool EditAddressDialog::saveCurrentRow() switch(mode) { - case NewReceivingAddress: case NewSendingAddress: address = model->addRow( - mode == NewSendingAddress ? AddressTableModel::Send : AddressTableModel::Receive, + AddressTableModel::Send, ui->labelEdit->text(), ui->addressEdit->text(), - g_address_type); + model->GetDefaultAddressType()); break; case EditReceivingAddress: case EditSendingAddress: diff --git a/src/qt/editaddressdialog.h b/src/qt/editaddressdialog.h index ddb67ece72..41c5d1708a 100644 --- a/src/qt/editaddressdialog.h +++ b/src/qt/editaddressdialog.h @@ -25,7 +25,6 @@ class EditAddressDialog : public QDialog public: enum Mode { - NewReceivingAddress, NewSendingAddress, EditReceivingAddress, EditSendingAddress diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 7c3c68bfef..16ef27a36a 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -209,14 +209,6 @@ bool parseBitcoinURI(const QUrl &uri, SendCoinsRecipient *out) bool parseBitcoinURI(QString uri, SendCoinsRecipient *out) { - // Convert bitcoin:// to bitcoin: - // - // Cannot handle this later, because bitcoin:// will cause Qt to see the part after // as host, - // which will lower-case it (and thus invalidate the address). - if(uri.startsWith("bitcoin://", Qt::CaseInsensitive)) - { - uri.replace(0, 10, "bitcoin:"); - } QUrl uriInstance(uri); return parseBitcoinURI(uriInstance, out); } diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index 71a69483f5..4b856986da 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -141,7 +141,7 @@ namespace GUIUtil * Makes a QTableView last column feel as if it was being resized from its left border. * Also makes sure the column widths are never larger than the table's viewport. * In Qt, all columns are resizable from the right, but it's not intuitive resizing the last column from the right. - * Usually our second to last columns behave as if stretched, and when on strech mode, columns aren't resizable + * Usually our second to last columns behave as if stretched, and when on stretch mode, columns aren't resizable * interactively or programmatically. * * This helper object takes care of this issue. diff --git a/src/qt/modaloverlay.cpp b/src/qt/modaloverlay.cpp index b573dbe226..249418213f 100644 --- a/src/qt/modaloverlay.cpp +++ b/src/qt/modaloverlay.cpp @@ -81,7 +81,7 @@ void ModalOverlay::tipUpdate(int count, const QDateTime& blockDate, double nVeri // keep a vector of samples of verification progress at height blockProcessTime.push_front(qMakePair(currentDate.toMSecsSinceEpoch(), nVerificationProgress)); - // show progress speed if we have more then one sample + // show progress speed if we have more than one sample if (blockProcessTime.size() >= 2) { double progressDelta = 0; double progressPerHour = 0; diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp index 4b6fdc8d57..05d3d36a74 100644 --- a/src/qt/paymentserver.cpp +++ b/src/qt/paymentserver.cpp @@ -404,7 +404,12 @@ void PaymentServer::handleURIOrFile(const QString& s) return; } - if (s.startsWith(BITCOIN_IPC_PREFIX, Qt::CaseInsensitive)) // bitcoin: URI + if (s.startsWith("bitcoin://", Qt::CaseInsensitive)) + { + Q_EMIT message(tr("URI handling"), tr("'bitcoin://' is not a valid URI. Use 'bitcoin:' instead."), + CClientUIInterface::MSG_ERROR); + } + else if (s.startsWith(BITCOIN_IPC_PREFIX, Qt::CaseInsensitive)) // bitcoin: URI { #if QT_VERSION < 0x050000 QUrl uri(s); @@ -643,7 +648,7 @@ void PaymentServer::fetchPaymentACK(CWallet* wallet, const SendCoinsRecipient& r // use for change. Despite an actual payment and not change, this is a close match: // it's the output type we use subject to privacy issues, but not restricted by what // other software supports. - const OutputType change_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type; + const OutputType change_type = wallet->m_default_change_type != OutputType::NONE ? wallet->m_default_change_type : wallet->m_default_address_type; wallet->LearnRelatedScripts(newKey, change_type); CTxDestination dest = GetDestinationForKey(newKey, change_type); wallet->SetAddressBook(dest, strAccount, "refund"); diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index 7fd5285467..132fb54d66 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -95,13 +95,13 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) columnResizingFixer = new GUIUtil::TableViewLastColumnResizingFixer(tableView, AMOUNT_MINIMUM_COLUMN_WIDTH, DATE_COLUMN_WIDTH, this); // configure bech32 checkbox, disable if launched with legacy as default: - if (model->getDefaultAddressType() == OUTPUT_TYPE_BECH32) { + if (model->getDefaultAddressType() == OutputType::BECH32) { ui->useBech32->setCheckState(Qt::Checked); } else { ui->useBech32->setCheckState(Qt::Unchecked); } - ui->useBech32->setVisible(model->getDefaultAddressType() != OUTPUT_TYPE_LEGACY); + ui->useBech32->setVisible(model->getDefaultAddressType() != OutputType::LEGACY); } } @@ -145,8 +145,8 @@ void ReceiveCoinsDialog::on_receiveButton_clicked() QString label = ui->reqLabel->text(); /* Generate new receiving address */ OutputType address_type = model->getDefaultAddressType(); - if (address_type != OUTPUT_TYPE_LEGACY) { - address_type = ui->useBech32->isChecked() ? OUTPUT_TYPE_BECH32 : OUTPUT_TYPE_P2SH_SEGWIT; + if (address_type != OutputType::LEGACY) { + address_type = ui->useBech32->isChecked() ? OutputType::BECH32 : OutputType::P2SH_SEGWIT; } address = model->getAddressTableModel()->addRow(AddressTableModel::Receive, label, "", address_type); SendCoinsRecipient info(address, label, diff --git a/src/qt/splashscreen.cpp b/src/qt/splashscreen.cpp index 5df1282f73..66e9dd0465 100644 --- a/src/qt/splashscreen.cpp +++ b/src/qt/splashscreen.cpp @@ -91,7 +91,7 @@ SplashScreen::SplashScreen(Qt::WindowFlags f, const NetworkStyle *networkStyle) pixPaint.setFont(QFont(font, 15*fontFactor)); - // if the version string is to long, reduce size + // if the version string is too long, reduce size fm = pixPaint.fontMetrics(); int versionTextWidth = fm.width(versionText); if(versionTextWidth > titleTextWidth+paddingRight-10) { diff --git a/src/qt/test/uritests.cpp b/src/qt/test/uritests.cpp index 8415250630..59938f704a 100644 --- a/src/qt/test/uritests.cpp +++ b/src/qt/test/uritests.cpp @@ -51,7 +51,7 @@ void URITests::uriTests() QVERIFY(rv.address == QString("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W")); QVERIFY(rv.label == QString()); - QVERIFY(GUIUtil::parseBitcoinURI("bitcoin://175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W?message=Wikipedia Example Address", &rv)); + QVERIFY(GUIUtil::parseBitcoinURI("bitcoin:175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W?message=Wikipedia Example Address", &rv)); QVERIFY(rv.address == QString("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W")); QVERIFY(rv.label == QString()); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 976aadc0af..c9898e52ca 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -150,9 +150,6 @@ void BumpFee(TransactionView& view, const uint256& txid, bool expectDisabled, st // src/qt/test/test_bitcoin-qt -platform cocoa # macOS void TestGUI() { - g_address_type = OUTPUT_TYPE_P2SH_SEGWIT; - g_change_type = OUTPUT_TYPE_P2SH_SEGWIT; - // Set up wallet and chain with 105 blocks (5 mature blocks for spending). TestChain100Setup test; for (int i = 0; i < 5; ++i) { @@ -163,7 +160,7 @@ void TestGUI() wallet.LoadWallet(firstRun); { LOCK(wallet.cs_wallet); - wallet.SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), g_address_type), "", "receive"); + wallet.SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet.m_default_address_type), "", "receive"); wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); } { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 39ef20c835..d092ebedfb 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -698,7 +698,7 @@ bool WalletModel::bumpFee(uint256 hash) confirmationDialog.exec(); QMessageBox::StandardButton retval = static_cast<QMessageBox::StandardButton>(confirmationDialog.result()); - // cancel sign&broadcast if users doesn't want to bump the fee + // cancel sign&broadcast if user doesn't want to bump the fee if (retval != QMessageBox::Yes) { return false; } @@ -736,7 +736,7 @@ bool WalletModel::hdEnabled() const OutputType WalletModel::getDefaultAddressType() const { - return g_address_type; + return wallet->m_default_address_type; } int WalletModel::getDefaultConfirmTarget() const diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 811996b98f..64813e0f5a 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -20,7 +20,7 @@ #include <QObject> -enum OutputType : int; +enum class OutputType; class AddressTableModel; class OptionsModel; diff --git a/src/random.h b/src/random.h index ea88670a37..5fe8f2fd04 100644 --- a/src/random.h +++ b/src/random.h @@ -32,7 +32,7 @@ void RandAddSeedSleep(); /** * Function to gather random data from multiple sources, failing whenever any - * of those source fail to provide a result. + * of those sources fail to provide a result. */ void GetStrongRandBytes(unsigned char* buf, int num); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 2077723af5..169caddc59 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -834,18 +834,18 @@ static void ApplyStats(CCoinsStats &stats, CHashWriter& ss, const uint256& hash, { assert(!outputs.empty()); ss << hash; - ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase); + ss << VARINT(outputs.begin()->second.nHeight * 2 + outputs.begin()->second.fCoinBase ? 1u : 0u); stats.nTransactions++; for (const auto output : outputs) { ss << VARINT(output.first + 1); ss << output.second.out.scriptPubKey; - ss << VARINT(output.second.out.nValue); + ss << VARINT(output.second.out.nValue, VarIntMode::NONNEGATIVE_SIGNED); stats.nTransactionOutputs++; stats.nTotalAmount += output.second.out.nValue; stats.nBogoSize += 32 /* txid */ + 4 /* vout index */ + 4 /* height + coinbase */ + 8 /* amount */ + 2 /* scriptPubKey len */ + output.second.out.scriptPubKey.size() /* scriptPubKey */; } - ss << VARINT(0); + ss << VARINT(0u); } //! Calculate statistics about the unspent transaction output set diff --git a/src/rpc/server.h b/src/rpc/server.h index 8b32924fbc..d25268a8ab 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -165,8 +165,17 @@ public: /** * Appends a CRPCCommand to the dispatch table. + * * Returns false if RPC server is already running (dump concurrency protection). + * * Commands cannot be overwritten (returns false). + * + * Commands with different method names but the same callback function will + * be considered aliases, and only the first registered method name will + * show up in the help text command listing. Aliased commands do not have + * to have the same behavior. Server and client code can distinguish + * between calls based on method name, and aliased commands can also + * register different names, types, and numbers of parameters. */ bool appendCommand(const std::string& name, const CRPCCommand* pcmd); }; diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 927b0267ca..ffaba393c0 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -110,7 +110,7 @@ bool static IsValidSignatureEncoding(const std::vector<unsigned char> &sig) { // excluding the sighash byte. // * R-length: 1-byte length descriptor of the R value that follows. // * R: arbitrary-length big-endian encoded R value. It must use the shortest - // possible encoding for a positive integers (which means no null bytes at + // possible encoding for a positive integer (which means no null bytes at // the start, except a single one when the next byte has its highest bit set). // * S-length: 1-byte length descriptor of the S value that follows. // * S: arbitrary-length big-endian encoded S value. The same rules apply. diff --git a/src/serialize.h b/src/serialize.h index c454ba16b7..91da6b0f80 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -296,9 +296,31 @@ uint64_t ReadCompactSize(Stream& is) * 2^32: [0x8E 0xFE 0xFE 0xFF 0x00] */ -template<typename I> +/** + * Mode for encoding VarInts. + * + * Currently there is no support for signed encodings. The default mode will not + * compile with signed values, and the legacy "nonnegative signed" mode will + * accept signed values, but improperly encode and decode them if they are + * negative. In the future, the DEFAULT mode could be extended to support + * negative numbers in a backwards compatible way, and additional modes could be + * added to support different varint formats (e.g. zigzag encoding). + */ +enum class VarIntMode { DEFAULT, NONNEGATIVE_SIGNED }; + +template <VarIntMode Mode, typename I> +struct CheckVarIntMode { + constexpr CheckVarIntMode() + { + static_assert(Mode != VarIntMode::DEFAULT || std::is_unsigned<I>::value, "Unsigned type required with mode DEFAULT."); + static_assert(Mode != VarIntMode::NONNEGATIVE_SIGNED || std::is_signed<I>::value, "Signed type required with mode NONNEGATIVE_SIGNED."); + } +}; + +template<VarIntMode Mode, typename I> inline unsigned int GetSizeOfVarInt(I n) { + CheckVarIntMode<Mode, I>(); int nRet = 0; while(true) { nRet++; @@ -312,9 +334,10 @@ inline unsigned int GetSizeOfVarInt(I n) template<typename I> inline void WriteVarInt(CSizeComputer& os, I n); -template<typename Stream, typename I> +template<typename Stream, VarIntMode Mode, typename I> void WriteVarInt(Stream& os, I n) { + CheckVarIntMode<Mode, I>(); unsigned char tmp[(sizeof(n)*8+6)/7]; int len=0; while(true) { @@ -329,9 +352,10 @@ void WriteVarInt(Stream& os, I n) } while(len--); } -template<typename Stream, typename I> +template<typename Stream, VarIntMode Mode, typename I> I ReadVarInt(Stream& is) { + CheckVarIntMode<Mode, I>(); I n = 0; while(true) { unsigned char chData = ser_readdata8(is); @@ -351,7 +375,7 @@ I ReadVarInt(Stream& is) } #define FLATDATA(obj) CFlatData((char*)&(obj), (char*)&(obj) + sizeof(obj)) -#define VARINT(obj) WrapVarInt(REF(obj)) +#define VARINT(obj, ...) WrapVarInt<__VA_ARGS__>(REF(obj)) #define COMPACTSIZE(obj) CCompactSize(REF(obj)) #define LIMITED_STRING(obj,n) LimitedString< n >(REF(obj)) @@ -395,7 +419,7 @@ public: } }; -template<typename I> +template<VarIntMode Mode, typename I> class CVarInt { protected: @@ -405,12 +429,12 @@ public: template<typename Stream> void Serialize(Stream &s) const { - WriteVarInt<Stream,I>(s, n); + WriteVarInt<Stream,Mode,I>(s, n); } template<typename Stream> void Unserialize(Stream& s) { - n = ReadVarInt<Stream,I>(s); + n = ReadVarInt<Stream,Mode,I>(s); } }; @@ -461,8 +485,8 @@ public: } }; -template<typename I> -CVarInt<I> WrapVarInt(I& n) { return CVarInt<I>(n); } +template<VarIntMode Mode=VarIntMode::DEFAULT, typename I> +CVarInt<Mode, I> WrapVarInt(I& n) { return CVarInt<Mode, I>{n}; } /** * Forward declarations diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index d92ab02d6b..1401188a2b 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -184,7 +184,7 @@ void Win32LockedPageAllocator::FreeLocked(void* addr, size_t len) size_t Win32LockedPageAllocator::GetLimit() { - // TODO is there a limit on windows, how to get it? + // TODO is there a limit on Windows, how to get it? return std::numeric_limits<size_t>::max(); } #endif diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 0d8bd90119..6b188a06b4 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -491,7 +491,7 @@ BOOST_AUTO_TEST_CASE(caddrinfo_get_new_bucket) // this test could be a security issue. BOOST_CHECK(info1.GetNewBucket(nKey1) != info1.GetNewBucket(nKey2)); - // Test: Ports should not effect bucket placement in the addr + // Test: Ports should not affect bucket placement in the addr CAddrInfo info2 = CAddrInfo(addr2, source1); BOOST_CHECK(info1.GetKey() != info2.GetKey()); BOOST_CHECK_EQUAL(info1.GetNewBucket(nKey1), info2.GetNewBucket(nKey1)); diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 36e271295a..de7d8f7b90 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -313,7 +313,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) auto utxod = FindRandomFrom(coinbase_coins); // Reuse the exact same coinbase tx = std::get<0>(utxod->second); - // shouldn't be available for reconnection if its been duplicated + // shouldn't be available for reconnection if it's been duplicated disconnected_coins.erase(utxod->first); duplicate_coins.insert(utxod->first); diff --git a/src/test/cuckoocache_tests.cpp b/src/test/cuckoocache_tests.cpp index 51ebfc3800..ccd5caacd5 100644 --- a/src/test/cuckoocache_tests.cpp +++ b/src/test/cuckoocache_tests.cpp @@ -163,7 +163,7 @@ void test_cache_erase(size_t megabytes) for (uint32_t i = (n_insert / 2); i < n_insert; ++i) set.insert(hashes_insert_copy[i]); - /** elements that we marked erased but that are still there */ + /** elements that we marked as erased but are still there */ size_t count_erased_but_contained = 0; /** elements that we did not erase but are older */ size_t count_stale = 0; @@ -303,7 +303,7 @@ void test_cache_generations() local_rand_ctx = FastRandomContext(true); // block_activity models a chunk of network activity. n_insert elements are - // adde to the cache. The first and last n/4 are stored for removal later + // added to the cache. The first and last n/4 are stored for removal later // and the middle n/2 are not stored. This models a network which uses half // the signatures of recently (since the last block) added transactions // immediately and never uses the other half. diff --git a/src/test/data/base58_encode_decode.json b/src/test/data/base58_encode_decode.json index 9448f256d9..1a4bd7f458 100644 --- a/src/test/data/base58_encode_decode.json +++ b/src/test/data/base58_encode_decode.json @@ -10,5 +10,7 @@ ["572e4794", "3EFU7m"], ["ecac89cad93923c02321", "EJDM8drfXA6uyA"], ["10c8511e", "Rt5zm"], -["00000000000000000000", "1111111111"] +["00000000000000000000", "1111111111"], +["000111d38e5fc9071ffcd20b4a763cc9ae4f252bb4e48fd66a835e252ada93ff480d6dd43dc62a641155a5", "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"], +["000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f404142434445464748494a4b4c4d4e4f505152535455565758595a5b5c5d5e5f606162636465666768696a6b6c6d6e6f707172737475767778797a7b7c7d7e7f808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9fa0a1a2a3a4a5a6a7a8a9aaabacadaeafb0b1b2b3b4b5b6b7b8b9babbbcbdbebfc0c1c2c3c4c5c6c7c8c9cacbcccdcecfd0d1d2d3d4d5d6d7d8d9dadbdcdddedfe0e1e2e3e4e5e6e7e8e9eaebecedeeeff0f1f2f3f4f5f6f7f8f9fafbfcfdfeff", "1cWB5HCBdLjAuqGGReWE3R3CguuwSjw6RHn39s2yuDRTS5NsBgNiFpWgAnEx6VQi8csexkgYw3mdYrMHr8x9i7aEwP8kZ7vccXWqKDvGv3u1GxFKPuAkn8JCPPGDMf3vMMnbzm6Nh9zh1gcNsMvH3ZNLmP5fSG6DGbbi2tuwMWPthr4boWwCxf7ewSgNQeacyozhKDDQQ1qL5fQFUW52QKUZDZ5fw3KXNQJMcNTcaB723LchjeKun7MuGW5qyCBZYzA1KjofN1gYBV3NqyhQJ3Ns746GNuf9N2pQPmHz4xpnSrrfCvy6TVVz5d4PdrjeshsWQwpZsZGzvbdAdN8MKV5QsBDY"] ] diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 754a86344f..6694401a29 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -237,7 +237,7 @@ BOOST_AUTO_TEST_CASE(iterator_ordering) } struct StringContentsSerializer { - // Used to make two serialized objects the same while letting them have a different lengths + // Used to make two serialized objects the same while letting them have different lengths // This is a terrible idea std::string str; StringContentsSerializer() {} diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index 1766c6a093..19cd3b0963 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -523,7 +523,7 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest) pool.addUnchecked(tx6.GetHash(), entry.Fee(1100LL).FromTx(tx6)); pool.addUnchecked(tx7.GetHash(), entry.Fee(9000LL).FromTx(tx7)); - // we only require this remove, at max, 2 txn, because its not clear what we're really optimizing for aside from that + // we only require this to remove, at max, 2 txn, because it's not clear what we're really optimizing for aside from that pool.TrimToSize(pool.DynamicMemoryUsage() - 1); BOOST_CHECK(pool.exists(tx4.GetHash())); BOOST_CHECK(pool.exists(tx6.GetHash())); diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index 42fd59380a..7a79a77e8b 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -177,8 +177,8 @@ BOOST_AUTO_TEST_CASE(varints) CDataStream ss(SER_DISK, 0); CDataStream::size_type size = 0; for (int i = 0; i < 100000; i++) { - ss << VARINT(i); - size += ::GetSerializeSize(VARINT(i), 0, 0); + ss << VARINT(i, VarIntMode::NONNEGATIVE_SIGNED); + size += ::GetSerializeSize(VARINT(i, VarIntMode::NONNEGATIVE_SIGNED), 0, 0); BOOST_CHECK(size == ss.size()); } @@ -191,7 +191,7 @@ BOOST_AUTO_TEST_CASE(varints) // decode for (int i = 0; i < 100000; i++) { int j = -1; - ss >> VARINT(j); + ss >> VARINT(j, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_MESSAGE(i == j, "decoded:" << j << " expected:" << i); } @@ -205,21 +205,21 @@ BOOST_AUTO_TEST_CASE(varints) BOOST_AUTO_TEST_CASE(varints_bitpatterns) { CDataStream ss(SER_DISK, 0); - ss << VARINT(0); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear(); - ss << VARINT(0x7f); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); - ss << VARINT((int8_t)0x7f); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); - ss << VARINT(0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); + ss << VARINT(0, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "00"); ss.clear(); + ss << VARINT(0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); + ss << VARINT((int8_t)0x7f, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "7f"); ss.clear(); + ss << VARINT(0x80, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); ss << VARINT((uint8_t)0x80); BOOST_CHECK_EQUAL(HexStr(ss), "8000"); ss.clear(); - ss << VARINT(0x1234); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); - ss << VARINT((int16_t)0x1234); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); - ss << VARINT(0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); + ss << VARINT(0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); + ss << VARINT((int16_t)0x1234, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "a334"); ss.clear(); + ss << VARINT(0xffff, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); ss << VARINT((uint16_t)0xffff); BOOST_CHECK_EQUAL(HexStr(ss), "82fe7f"); ss.clear(); - ss << VARINT(0x123456); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); - ss << VARINT((int32_t)0x123456); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); + ss << VARINT(0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); + ss << VARINT((int32_t)0x123456, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "c7e756"); ss.clear(); ss << VARINT(0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear(); ss << VARINT((uint32_t)0x80123456U); BOOST_CHECK_EQUAL(HexStr(ss), "86ffc7e756"); ss.clear(); ss << VARINT(0xffffffff); BOOST_CHECK_EQUAL(HexStr(ss), "8efefefe7f"); ss.clear(); - ss << VARINT(0x7fffffffffffffffLL); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear(); + ss << VARINT(0x7fffffffffffffffLL, VarIntMode::NONNEGATIVE_SIGNED); BOOST_CHECK_EQUAL(HexStr(ss), "fefefefefefefefe7f"); ss.clear(); ss << VARINT(0xffffffffffffffffULL); BOOST_CHECK_EQUAL(HexStr(ss), "80fefefefefefefefe7f"); ss.clear(); } diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index 9ec9d6cba3..1aa54189b6 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -35,7 +35,7 @@ ToMemPool(CMutableTransaction& tx) BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) { - // Make sure skipping validation of transctions that were + // Make sure skipping validation of transactions that were // validated going into the memory pool does not allow // double-spends in blocks to pass validation when they should not. diff --git a/src/tinyformat.h b/src/tinyformat.h index d34cfaa94f..14b7cd3026 100644 --- a/src/tinyformat.h +++ b/src/tinyformat.h @@ -155,7 +155,7 @@ namespace tfm = tinyformat; #endif #ifdef __APPLE__ -// Workaround OSX linker warning: xcode uses different default symbol +// Workaround OSX linker warning: Xcode uses different default symbol // visibilities for static libs vs executables (see issue #25) # define TINYFORMAT_HIDDEN __attribute__((visibility("hidden"))) #else @@ -592,7 +592,7 @@ inline const char* printFormatStringLiteral(std::ostream& out, const char* fmt) // Formatting options which can't be natively represented using the ostream // state are returned in spacePadPositive (for space padded positive numbers) // and ntrunc (for truncating conversions). argIndex is incremented if -// necessary to pull out variable width and precision . The function returns a +// necessary to pull out variable width and precision. The function returns a // pointer to the character after the end of the current format spec. inline const char* streamStateFromFormat(std::ostream& out, bool& spacePadPositive, int& ntrunc, const char* fmtStart, diff --git a/src/txdb.cpp b/src/txdb.cpp index 7a1d920117..91d6c98430 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -324,7 +324,7 @@ public: void Unserialize(Stream &s) { unsigned int nCode = 0; // version - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); // header code ::Unserialize(s, VARINT(nCode)); @@ -351,7 +351,7 @@ public: ::Unserialize(s, CTxOutCompressor(vout[i])); } // coinbase height - ::Unserialize(s, VARINT(nHeight)); + ::Unserialize(s, VARINT(nHeight, VarIntMode::NONNEGATIVE_SIGNED)); } }; diff --git a/src/txmempool.h b/src/txmempool.h index c6a1bf08ce..08a3421015 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -689,7 +689,7 @@ private: }; /** - * CCoinsView that brings transactions from a memorypool into view. + * CCoinsView that brings transactions from a mempool into view. * It does not check for spendings by memory pool transactions. * Instead, it provides access to all Coins which are either unspent in the * base CCoinsView, or are outputs from any mempool transaction! diff --git a/src/undo.h b/src/undo.h index 7aae034de2..f292924165 100644 --- a/src/undo.h +++ b/src/undo.h @@ -15,7 +15,7 @@ * * Contains the prevout's CTxOut being spent, and its metadata as well * (coinbase or not, height). The serialization contains a dummy value of - * zero. This is be compatible with older versions which expect to see + * zero. This is compatible with older versions which expect to see * the transaction version there. */ class TxInUndoSerializer @@ -25,7 +25,7 @@ class TxInUndoSerializer public: template<typename Stream> void Serialize(Stream &s) const { - ::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1 : 0))); + ::Serialize(s, VARINT(txout->nHeight * 2 + (txout->fCoinBase ? 1u : 0u))); if (txout->nHeight > 0) { // Required to maintain compatibility with older undo format. ::Serialize(s, (unsigned char)0); @@ -51,7 +51,7 @@ public: // Old versions stored the version number for the last spend of // a transaction's outputs. Non-final spends were indicated with // height = 0. - int nVersionDummy; + unsigned int nVersionDummy; ::Unserialize(s, VARINT(nVersionDummy)); } ::Unserialize(s, CTxOutCompressor(REF(txout->out))); diff --git a/src/util.cpp b/src/util.cpp index 94f829ad32..69ceefc8cd 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -159,10 +159,10 @@ instance_of_cinit; * the mutex). */ -static boost::once_flag debugPrintInitFlag = BOOST_ONCE_INIT; +static std::once_flag debugPrintInitFlag; /** - * We use boost::call_once() to make sure mutexDebugLog and + * We use std::call_once() to make sure mutexDebugLog and * vMsgsBeforeOpenLog are initialized in a thread-safe manner. * * NOTE: fileout, mutexDebugLog and sometimes vMsgsBeforeOpenLog @@ -171,7 +171,7 @@ static boost::once_flag debugPrintInitFlag = BOOST_ONCE_INIT; * tested, explicit destruction of these objects can be implemented. */ static FILE* fileout = nullptr; -static boost::mutex* mutexDebugLog = nullptr; +static std::mutex* mutexDebugLog = nullptr; static std::list<std::string>* vMsgsBeforeOpenLog; static int FileWriteStr(const std::string &str, FILE *fp) @@ -182,7 +182,7 @@ static int FileWriteStr(const std::string &str, FILE *fp) static void DebugPrintInit() { assert(mutexDebugLog == nullptr); - mutexDebugLog = new boost::mutex(); + mutexDebugLog = new std::mutex(); vMsgsBeforeOpenLog = new std::list<std::string>; } @@ -194,8 +194,8 @@ fs::path GetDebugLogPath() bool OpenDebugLog() { - boost::call_once(&DebugPrintInit, debugPrintInitFlag); - boost::mutex::scoped_lock scoped_lock(*mutexDebugLog); + std::call_once(debugPrintInitFlag, &DebugPrintInit); + std::lock_guard<std::mutex> scoped_lock(*mutexDebugLog); assert(fileout == nullptr); assert(vMsgsBeforeOpenLog); @@ -350,8 +350,8 @@ int LogPrintStr(const std::string &str) } else if (fPrintToDebugLog) { - boost::call_once(&DebugPrintInit, debugPrintInitFlag); - boost::mutex::scoped_lock scoped_lock(*mutexDebugLog); + std::call_once(debugPrintInitFlag, &DebugPrintInit); + std::lock_guard<std::mutex> scoped_lock(*mutexDebugLog); // buffer if we haven't opened the log yet if (fileout == nullptr) { diff --git a/src/util.h b/src/util.h index e4170d8aa2..aebd2fd590 100644 --- a/src/util.h +++ b/src/util.h @@ -136,7 +136,7 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c // Be conservative when using LogPrintf/error or other things which // unconditionally log to debug.log! It should not be the case that an inbound -// peer can fill up a users disk with debug.log entries. +// peer can fill up a user's disk with debug.log entries. #ifdef USE_COVERAGE #define LogPrintf(...) do { MarkUsed(__VA_ARGS__); } while(0) diff --git a/src/validation.cpp b/src/validation.cpp index bee890437e..614876ea49 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -949,7 +949,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // This transaction should only count for fee estimation if: // - it isn't a BIP 125 replacement transaction (may not be widely supported) - // - it's not being readded during a reorg which bypasses typical mempool fee limits + // - it's not being re-added during a reorg which bypasses typical mempool fee limits // - the node is not behind // - the transaction is not dependent on any other transactions in the mempool bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx); @@ -1852,7 +1852,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl // Once BIP34 activated it was not possible to create new duplicate coinbases and thus other than starting // with the 2 existing duplicate coinbase pairs, not possible to create overwriting txs. But by the // time BIP34 activated, in each of the existing pairs the duplicate coinbase had overwritten the first - // before the first had been spent. Since those coinbases are sufficiently buried its no longer possible to create further + // before the first had been spent. Since those coinbases are sufficiently buried it's no longer possible to create further // duplicate transactions descending from the known pairs either. // If we're on the known chain at height greater than where BIP34 activated, we can save the db accesses needed for the BIP30 check. diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 458e770e03..52d6a291c9 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -18,8 +18,8 @@ class CCoinControl public: //! Custom change destination, if not set an address is generated CTxDestination destChange; - //! Custom change type, ignored if destChange is set, defaults to g_change_type - OutputType change_type; + //! Override the default change type if set, ignored if destChange is set + boost::optional<OutputType> m_change_type; //! If false, allows unselected inputs, but requires all selected inputs be used bool fAllowOtherInputs; //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria @@ -43,7 +43,7 @@ public: void SetNull() { destChange = CNoDestination(); - change_type = g_change_type; + m_change_type.reset(); fAllowOtherInputs = false; fAllowWatchOnly = false; setSelected.clear(); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index e028cf4210..61481e01b6 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -17,7 +17,7 @@ std::string GetWalletHelpString(bool showDebug) { std::string strUsage = HelpMessageGroup(_("Wallet options:")); - strUsage += HelpMessageOpt("-addresstype", strprintf("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")", FormatOutputType(OUTPUT_TYPE_DEFAULT))); + strUsage += HelpMessageOpt("-addresstype", strprintf("What type of addresses to use (\"legacy\", \"p2sh-segwit\", or \"bech32\", default: \"%s\")", FormatOutputType(DEFAULT_ADDRESS_TYPE))); strUsage += HelpMessageOpt("-changetype", "What type of change to use (\"legacy\", \"p2sh-segwit\", or \"bech32\"). Default is same as -addresstype, except when -addresstype=p2sh-segwit a native segwit output is used when sending to a native segwit address)"); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); strUsage += HelpMessageOpt("-discardfee=<amt>", strprintf(_("The fee rate (in %s/kB) that indicates your tolerance for discarding change by adding it to the fee (default: %s). " @@ -181,18 +181,6 @@ bool WalletParameterInteraction() bSpendZeroConfChange = gArgs.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE); fWalletRbf = gArgs.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF); - g_address_type = ParseOutputType(gArgs.GetArg("-addresstype", "")); - if (g_address_type == OUTPUT_TYPE_NONE) { - return InitError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); - } - - // If changetype is set in config file or parameter, check that it's valid. - // Default to OUTPUT_TYPE_NONE if not set. - g_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), OUTPUT_TYPE_NONE); - if (g_change_type == OUTPUT_TYPE_NONE && !gArgs.GetArg("-changetype", "").empty()) { - return InitError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); - } - return true; } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 01125dd618..1721bc6df6 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -83,7 +83,7 @@ bool GetWalletAddressesForKey(CWallet * const pwallet, const CKeyID &keyid, std: } } if (!fLabelFound) { - strAddr = EncodeDestination(GetDestinationForKey(key.GetPubKey(), g_address_type)); + strAddr = EncodeDestination(GetDestinationForKey(key.GetPubKey(), pwallet->m_default_address_type)); } return fLabelFound; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7ad9efff70..0dc6de9564 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -162,10 +162,10 @@ UniValue getnewaddress(const JSONRPCRequest& request) if (!request.params[0].isNull()) strAccount = AccountFromValue(request.params[0]); - OutputType output_type = g_address_type; + OutputType output_type = pwallet->m_default_address_type; if (!request.params[1].isNull()) { - output_type = ParseOutputType(request.params[1].get_str(), g_address_type); - if (output_type == OUTPUT_TYPE_NONE) { + output_type = ParseOutputType(request.params[1].get_str(), pwallet->m_default_address_type); + if (output_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[1].get_str())); } } @@ -259,10 +259,10 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); } - OutputType output_type = g_change_type != OUTPUT_TYPE_NONE ? g_change_type : g_address_type; + OutputType output_type = pwallet->m_default_change_type != OutputType::NONE ? pwallet->m_default_change_type : pwallet->m_default_address_type; if (!request.params[0].isNull()) { output_type = ParseOutputType(request.params[0].get_str(), output_type); - if (output_type == OUTPUT_TYPE_NONE) { + if (output_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[0].get_str())); } } @@ -1221,10 +1221,10 @@ UniValue addmultisigaddress(const JSONRPCRequest& request) } } - OutputType output_type = g_address_type; + OutputType output_type = pwallet->m_default_address_type; if (!request.params[3].isNull()) { output_type = ParseOutputType(request.params[3].get_str(), output_type); - if (output_type == OUTPUT_TYPE_NONE) { + if (output_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown address type '%s'", request.params[3].get_str())); } } @@ -2015,7 +2015,7 @@ UniValue listsinceblock(const JSONRPCRequest& request) " ],\n" " \"removed\": [\n" " <structure is the same as \"transactions\" above, only present if include_removed=true>\n" - " Note: transactions that were readded in the active chain will appear as-is in this array, and may thus have a positive confirmation count.\n" + " Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count.\n" " ],\n" " \"lastblock\": \"lastblockhash\" (string) The hash of the block (target_confirmations-1) from the best block on the main chain. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones\n" "}\n" @@ -2363,8 +2363,6 @@ UniValue walletpassphrase(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrase was called."); } @@ -2429,8 +2427,6 @@ UniValue walletpassphrasechange(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called."); } @@ -2485,8 +2481,6 @@ UniValue walletlock(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletlock was called."); } @@ -2532,8 +2526,6 @@ UniValue encryptwallet(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); - if (request.fHelp) - return true; if (pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called."); } @@ -3183,8 +3175,8 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) if (options.exists("changeAddress")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot specify both changeAddress and address_type options"); } - coinControl.change_type = ParseOutputType(options["change_type"].get_str(), coinControl.change_type); - if (coinControl.change_type == OUTPUT_TYPE_NONE) { + coinControl.m_change_type = ParseOutputType(options["change_type"].get_str(), pwallet->m_default_change_type); + if (coinControl.m_change_type == OutputType::NONE) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Unknown change type '%s'", options["change_type"].get_str())); } } @@ -3578,7 +3570,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); } else if (pindexStop->nHeight < pindexStart->nHeight) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater then start_height"); + throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater than start_height"); } } } diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 77ccd0b8d8..5c550742c8 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -12,8 +12,6 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName): TestingSetup(chainName), m_wallet("mock", CWalletDBWrapper::CreateMock()) { bool fFirstRun; - g_address_type = OUTPUT_TYPE_DEFAULT; - g_change_type = OUTPUT_TYPE_DEFAULT; m_wallet.LoadWallet(fFirstRun); RegisterValidationInterface(&m_wallet); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index d6b0daf4fe..808f8b8838 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -114,9 +114,6 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // than or equal to key birthday. BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { - g_address_type = OUTPUT_TYPE_DEFAULT; - g_change_type = OUTPUT_TYPE_DEFAULT; - // Create two blocks with same timestamp to verify that importwallet rescan // will pick up both blocks, not just the first. const int64_t BLOCK_TIME = chainActive.Tip()->GetBlockTimeMax() + 5; @@ -272,8 +269,6 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - g_address_type = OUTPUT_TYPE_DEFAULT; - g_change_type = OUTPUT_TYPE_DEFAULT; wallet = MakeUnique<CWallet>("mock", CWalletDBWrapper::CreateMock()); bool firstRun; wallet->LoadWallet(firstRun); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bd5094085e..d63c40d2ff 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -42,8 +42,6 @@ CFeeRate payTxFee(DEFAULT_TRANSACTION_FEE); unsigned int nTxConfirmTarget = DEFAULT_TX_CONFIRM_TARGET; bool bSpendZeroConfChange = DEFAULT_SPEND_ZEROCONF_CHANGE; bool fWalletRbf = DEFAULT_WALLET_RBF; -OutputType g_address_type = OUTPUT_TYPE_NONE; -OutputType g_change_type = OUTPUT_TYPE_NONE; bool g_wallet_allow_fallback_fee = true; //<! will be defined via chainparams const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; @@ -823,7 +821,7 @@ bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount bForceNew = true; else { // Check if the current key has been used (TODO: check other addresses with the same key) - CScript scriptPubKey = GetScriptForDestination(GetDestinationForKey(account.vchPubKey, g_address_type)); + CScript scriptPubKey = GetScriptForDestination(GetDestinationForKey(account.vchPubKey, m_default_address_type)); for (std::map<uint256, CWalletTx>::iterator it = mapWallet.begin(); it != mapWallet.end() && account.vchPubKey.IsValid(); ++it) @@ -840,12 +838,12 @@ bool CWallet::GetAccountDestination(CTxDestination &dest, std::string strAccount if (!GetKeyFromPool(account.vchPubKey, false)) return false; - LearnRelatedScripts(account.vchPubKey, g_address_type); - dest = GetDestinationForKey(account.vchPubKey, g_address_type); + LearnRelatedScripts(account.vchPubKey, m_default_address_type); + dest = GetDestinationForKey(account.vchPubKey, m_default_address_type); SetAddressBook(dest, strAccount, "receive"); walletdb.WriteAccount(strAccount, account); } else { - dest = GetDestinationForKey(account.vchPubKey, g_address_type); + dest = GetDestinationForKey(account.vchPubKey, m_default_address_type); } return true; @@ -1269,7 +1267,7 @@ void CWallet::BlockUntilSyncedToCurrentChain() { // chainActive.Tip()... // We could also take cs_wallet here, and call m_last_block_processed // protected by cs_wallet instead of cs_main, but as long as we need - // cs_main here anyway, its easier to just call it cs_main-protected. + // cs_main here anyway, it's easier to just call it cs_main-protected. LOCK(cs_main); const CBlockIndex* initialChainTip = chainActive.Tip(); @@ -2427,21 +2425,21 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -bool CWallet::OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibilty_filter) const +bool CWallet::OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibility_filter) const { if (!output.fSpendable) return false; - if (output.nDepth < (output.tx->IsFromMe(ISMINE_ALL) ? eligibilty_filter.conf_mine : eligibilty_filter.conf_theirs)) + if (output.nDepth < (output.tx->IsFromMe(ISMINE_ALL) ? eligibility_filter.conf_mine : eligibility_filter.conf_theirs)) return false; - if (!mempool.TransactionWithinChainLimit(output.tx->GetHash(), eligibilty_filter.max_ancestors)) + if (!mempool.TransactionWithinChainLimit(output.tx->GetHash(), eligibility_filter.max_ancestors)) return false; return true; } -bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector<COutput> vCoins, +bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { setCoinsRet.clear(); @@ -2462,7 +2460,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil // Filter by the min conf specs and add to utxo_pool and calculate effective value for (const COutput &output : vCoins) { - if (!OutputEligibleForSpending(output, eligibilty_filter)) + if (!OutputEligibleForSpending(output, eligibility_filter)) continue; CInputCoin coin(output.tx->tx, output.i); @@ -2482,7 +2480,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil // Filter by the min conf specs and add to utxo_pool for (const COutput &output : vCoins) { - if (!OutputEligibleForSpending(output, eligibilty_filter)) + if (!OutputEligibleForSpending(output, eligibility_filter)) continue; CInputCoin coin = CInputCoin(output.tx->tx, output.i); @@ -2649,14 +2647,14 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vector<CRecipient>& vecSend) { // If -changetype is specified, always use that change type. - if (change_type != OUTPUT_TYPE_NONE) { + if (change_type != OutputType::NONE) { return change_type; } - // if g_address_type is legacy, use legacy address as change (even + // if m_default_address_type is legacy, use legacy address as change (even // if some of the outputs are P2WPKH or P2WSH). - if (g_address_type == OUTPUT_TYPE_LEGACY) { - return OUTPUT_TYPE_LEGACY; + if (m_default_address_type == OutputType::LEGACY) { + return OutputType::LEGACY; } // if any destination is P2WPKH or P2WSH, use P2WPKH for the change @@ -2666,12 +2664,12 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec int witnessversion = 0; std::vector<unsigned char> witnessprogram; if (recipient.scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) { - return OUTPUT_TYPE_BECH32; + return OutputType::BECH32; } } - // else use g_address_type for change - return g_address_type; + // else use m_default_address_type for change + return m_default_address_type; } bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, @@ -2768,7 +2766,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransac return false; } - const OutputType change_type = TransactionChangeType(coin_control.change_type, vecSend); + const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend); LearnRelatedScripts(vchPubKey, change_type); scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type)); @@ -4002,8 +4000,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } walletInstance->SetBestChain(chainActive.GetLocator()); - } - else if (gArgs.IsArgSet("-usehd")) { + } else if (gArgs.IsArgSet("-usehd")) { bool useHD = gArgs.GetBoolArg("-usehd", true); if (walletInstance->IsHDEnabled() && !useHD) { InitError(strprintf(_("Error loading %s: You can't disable HD on an already existing HD wallet"), walletFile)); @@ -4015,6 +4012,20 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& } } + walletInstance->m_default_address_type = ParseOutputType(gArgs.GetArg("-addresstype", ""), DEFAULT_ADDRESS_TYPE); + if (walletInstance->m_default_address_type == OutputType::NONE) { + InitError(strprintf("Unknown address type '%s'", gArgs.GetArg("-addresstype", ""))); + return nullptr; + } + + // If changetype is set in config file or parameter, check that it's valid. + // Default to OutputType::NONE if not set. + walletInstance->m_default_change_type = ParseOutputType(gArgs.GetArg("-changetype", ""), OutputType::NONE); + if (walletInstance->m_default_change_type == OutputType::NONE && !gArgs.GetArg("-changetype", "").empty()) { + InitError(strprintf("Unknown change type '%s'", gArgs.GetArg("-changetype", ""))); + return nullptr; + } + LogPrintf(" wallet %15dms\n", GetTimeMillis() - nStart); // Try to top up keypool. No-op if the wallet is locked. @@ -4186,8 +4197,8 @@ bool CWalletTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& // We must set fInMempool here - while it will be re-set to true by the // entered-mempool callback, if we did not there would be a race where a // user could call sendmoney in a loop and hit spurious out of funds errors - // because we think that the transaction they just generated's change is - // unavailable as we're not yet aware its in mempool. + // because we think that this newly generated transaction's change is + // unavailable as we're not yet aware that it is in the mempool. bool ret = ::AcceptToMemoryPool(mempool, state, tx, nullptr /* pfMissingInputs */, nullptr /* plTxnReplaced */, false /* bypass_limits */, nAbsurdFee); fInMempool |= ret; @@ -4203,29 +4214,29 @@ OutputType ParseOutputType(const std::string& type, OutputType default_type) if (type.empty()) { return default_type; } else if (type == OUTPUT_TYPE_STRING_LEGACY) { - return OUTPUT_TYPE_LEGACY; + return OutputType::LEGACY; } else if (type == OUTPUT_TYPE_STRING_P2SH_SEGWIT) { - return OUTPUT_TYPE_P2SH_SEGWIT; + return OutputType::P2SH_SEGWIT; } else if (type == OUTPUT_TYPE_STRING_BECH32) { - return OUTPUT_TYPE_BECH32; + return OutputType::BECH32; } else { - return OUTPUT_TYPE_NONE; + return OutputType::NONE; } } const std::string& FormatOutputType(OutputType type) { switch (type) { - case OUTPUT_TYPE_LEGACY: return OUTPUT_TYPE_STRING_LEGACY; - case OUTPUT_TYPE_P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; - case OUTPUT_TYPE_BECH32: return OUTPUT_TYPE_STRING_BECH32; + case OutputType::LEGACY: return OUTPUT_TYPE_STRING_LEGACY; + case OutputType::P2SH_SEGWIT: return OUTPUT_TYPE_STRING_P2SH_SEGWIT; + case OutputType::BECH32: return OUTPUT_TYPE_STRING_BECH32; default: assert(false); } } void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type) { - if (key.IsCompressed() && (type == OUTPUT_TYPE_P2SH_SEGWIT || type == OUTPUT_TYPE_BECH32)) { + if (key.IsCompressed() && (type == OutputType::P2SH_SEGWIT || type == OutputType::BECH32)) { CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); // Make sure the resulting program is solvable. @@ -4236,20 +4247,20 @@ void CWallet::LearnRelatedScripts(const CPubKey& key, OutputType type) void CWallet::LearnAllRelatedScripts(const CPubKey& key) { - // OUTPUT_TYPE_P2SH_SEGWIT always adds all necessary scripts for all types. - LearnRelatedScripts(key, OUTPUT_TYPE_P2SH_SEGWIT); + // OutputType::P2SH_SEGWIT always adds all necessary scripts for all types. + LearnRelatedScripts(key, OutputType::P2SH_SEGWIT); } CTxDestination GetDestinationForKey(const CPubKey& key, OutputType type) { switch (type) { - case OUTPUT_TYPE_LEGACY: return key.GetID(); - case OUTPUT_TYPE_P2SH_SEGWIT: - case OUTPUT_TYPE_BECH32: { + case OutputType::LEGACY: return key.GetID(); + case OutputType::P2SH_SEGWIT: + case OutputType::BECH32: { if (!key.IsCompressed()) return key.GetID(); CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); - if (type == OUTPUT_TYPE_P2SH_SEGWIT) { + if (type == OutputType::P2SH_SEGWIT) { return CScriptID(witprog); } else { return witdest; @@ -4275,10 +4286,10 @@ CTxDestination CWallet::AddAndGetDestinationForScript(const CScript& script, Out { // Note that scripts over 520 bytes are not yet supported. switch (type) { - case OUTPUT_TYPE_LEGACY: + case OutputType::LEGACY: return CScriptID(script); - case OUTPUT_TYPE_P2SH_SEGWIT: - case OUTPUT_TYPE_BECH32: { + case OutputType::P2SH_SEGWIT: + case OutputType::BECH32: { WitnessV0ScriptHash hash; CSHA256().Write(script.data(), script.size()).Finalize(hash.begin()); CTxDestination witdest = hash; @@ -4287,7 +4298,7 @@ CTxDestination CWallet::AddAndGetDestinationForScript(const CScript& script, Out if (!IsSolvable(*this, witprog)) return CScriptID(script); // Add the redeemscript, so that P2WSH and P2SH-P2WSH outputs are recognized as ours. AddCScript(witprog); - if (type == OUTPUT_TYPE_BECH32) { + if (type == OutputType::BECH32) { return witdest; } else { return CScriptID(witprog); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index dc0e8d07d7..45d9762bde 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -43,6 +43,7 @@ extern bool bSpendZeroConfChange; extern bool fWalletRbf; extern bool g_wallet_allow_fallback_fee; +//! Default for -keypool static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; //! -paytxfee default static const CAmount DEFAULT_TRANSACTION_FEE = 0; @@ -96,18 +97,15 @@ enum WalletFeature FEATURE_LATEST = FEATURE_COMPRPUBKEY // HD is optional, use FEATURE_COMPRPUBKEY as latest version }; -enum OutputType : int -{ - OUTPUT_TYPE_NONE, - OUTPUT_TYPE_LEGACY, - OUTPUT_TYPE_P2SH_SEGWIT, - OUTPUT_TYPE_BECH32, - - OUTPUT_TYPE_DEFAULT = OUTPUT_TYPE_P2SH_SEGWIT +enum class OutputType { + NONE, + LEGACY, + P2SH_SEGWIT, + BECH32, }; -extern OutputType g_address_type; -extern OutputType g_change_type; +//! Default for -addresstype +constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::P2SH_SEGWIT}; /** A key pool entry */ @@ -855,7 +853,7 @@ public: * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector<COutput> vCoins, + bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; bool IsSpent(const uint256& hash, unsigned int n) const; @@ -989,6 +987,8 @@ public: static CFeeRate minTxFee; static CFeeRate fallbackFee; static CFeeRate m_discard_rate; + OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE}; + OutputType m_default_change_type{OutputType::NONE}; // Default to OutputType::NONE if not set by -changetype bool NewKeyPool(); size_t KeypoolCountExternalKeys(); @@ -1167,7 +1167,7 @@ public: CTxDestination AddAndGetDestinationForScript(const CScript& script, OutputType); /** Whether a given output is spendable by this wallet */ - bool OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibilty_filter) const; + bool OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibility_filter) const; }; /** A key allocated from the key pool. */ @@ -1232,7 +1232,7 @@ public: } }; -OutputType ParseOutputType(const std::string& str, OutputType default_type = OUTPUT_TYPE_DEFAULT); +OutputType ParseOutputType(const std::string& str, OutputType default_type); const std::string& FormatOutputType(OutputType type); /** diff --git a/test/functional/example_test.py b/test/functional/example_test.py index 12be685ecf..05d1c1bf4e 100755 --- a/test/functional/example_test.py +++ b/test/functional/example_test.py @@ -38,7 +38,7 @@ class BaseNode(P2PInterface): def __init__(self): """Initialize the P2PInterface - Used to inialize custom properties for the Node that aren't + Used to initialize custom properties for the Node that aren't included by default in the base class. Be aware that the P2PInterface base class already stores a counter for each P2P message type and the last received message of each type, which should be sufficient for the diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 8a56d3eefa..f434b6682b 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -99,7 +99,7 @@ def split_inputs(from_node, txins, txouts, initial_split=False): txouts.append({"txid": txid, "vout": 0, "amount": half_change}) txouts.append({"txid": txid, "vout": 1, "amount": rem_change}) -def check_estimates(node, fees_seen, max_invalid): +def check_estimates(node, fees_seen): """Call estimatesmartfee and verify that the estimates meet certain invariants.""" delta = 1.0e-6 # account for rounding error @@ -219,13 +219,13 @@ class EstimateFeeTest(BitcoinTestFramework): self.log.info("Creating transactions and mining them with a block size that can't keep up") # Create transactions and mine 10 small blocks with node 2, but create txs faster than we can mine self.transact_and_mine(10, self.nodes[2]) - check_estimates(self.nodes[1], self.fees_per_kb, 14) + check_estimates(self.nodes[1], self.fees_per_kb) self.log.info("Creating transactions and mining them at a block size that is just big enough") # Generate transactions while mining 10 more blocks, this time with node1 # which mines blocks with capacity just above the rate that transactions are being created self.transact_and_mine(10, self.nodes[1]) - check_estimates(self.nodes[1], self.fees_per_kb, 2) + check_estimates(self.nodes[1], self.fees_per_kb) # Finish by mining a normal-sized block: while len(self.nodes[1].getrawmempool()) > 0: @@ -233,7 +233,7 @@ class EstimateFeeTest(BitcoinTestFramework): sync_blocks(self.nodes[0:3], wait=.1) self.log.info("Final estimates after emptying mempools") - check_estimates(self.nodes[1], self.fees_per_kb, 2) + check_estimates(self.nodes[1], self.fees_per_kb) if __name__ == '__main__': EstimateFeeTest().main() diff --git a/test/functional/feature_maxuploadtarget.py b/test/functional/feature_maxuploadtarget.py index 45336ee801..abe4cb7c8e 100755 --- a/test/functional/feature_maxuploadtarget.py +++ b/test/functional/feature_maxuploadtarget.py @@ -6,7 +6,7 @@ * Verify that getdata requests for old blocks (>1week) are dropped if uploadtarget has been reached. -* Verify that getdata requests for recent blocks are respecteved even +* Verify that getdata requests for recent blocks are respected even if uploadtarget has been reached. * Verify that the upload counters are reset after 24 hours. """ @@ -17,7 +17,7 @@ from test_framework.mininode import * from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * -class TestNode(P2PInterface): +class TestP2PConn(P2PInterface): def __init__(self): super().__init__() self.block_receive_map = defaultdict(int) @@ -55,7 +55,7 @@ class MaxUploadTest(BitcoinTestFramework): p2p_conns = [] for _ in range(3): - p2p_conns.append(self.nodes[0].add_p2p_connection(TestNode())) + p2p_conns.append(self.nodes[0].add_p2p_connection(TestP2PConn())) network_thread_start() for p2pc in p2p_conns: @@ -147,7 +147,7 @@ class MaxUploadTest(BitcoinTestFramework): self.start_node(0, ["-whitelist=127.0.0.1", "-maxuploadtarget=1", "-blockmaxsize=999000"]) # Reconnect to self.nodes[0] - self.nodes[0].add_p2p_connection(TestNode()) + self.nodes[0].add_p2p_connection(TestP2PConn()) network_thread_start() self.nodes[0].p2p.wait_for_verack() diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py index 49ad7f838c..bb162e6e97 100755 --- a/test/functional/feature_pruning.py +++ b/test/functional/feature_pruning.py @@ -11,7 +11,6 @@ This test takes 30 mins or more (up to 2 hours) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * -import time import os MIN_BLOCKS_TO_KEEP = 288 @@ -23,7 +22,7 @@ TIMESTAMP_WINDOW = 2 * 60 * 60 def calc_usage(blockdir): - return sum(os.path.getsize(blockdir+f) for f in os.listdir(blockdir) if os.path.isfile(blockdir+f)) / (1024. * 1024.) + return sum(os.path.getsize(blockdir+f) for f in os.listdir(blockdir) if os.path.isfile(os.path.join(blockdir, f))) / (1024. * 1024.) class PruneTest(BitcoinTestFramework): def set_test_params(self): @@ -70,7 +69,7 @@ class PruneTest(BitcoinTestFramework): sync_blocks(self.nodes[0:5]) def test_height_min(self): - if not os.path.isfile(self.prunedir+"blk00000.dat"): + if not os.path.isfile(os.path.join(self.prunedir, "blk00000.dat")): raise AssertionError("blk00000.dat is missing, pruning too early") self.log.info("Success") self.log.info("Though we're already using more than 550MiB, current usage: %d" % calc_usage(self.prunedir)) @@ -79,11 +78,8 @@ class PruneTest(BitcoinTestFramework): for i in range(25): mine_large_block(self.nodes[0], self.utxo_cache_0) - waitstart = time.time() - while os.path.isfile(self.prunedir+"blk00000.dat"): - time.sleep(0.1) - if time.time() - waitstart > 30: - raise AssertionError("blk00000.dat not pruned when it should be") + # Wait for blk00000.dat to be pruned + wait_until(lambda: not os.path.isfile(os.path.join(self.prunedir, "blk00000.dat")), timeout=30) self.log.info("Success") usage = calc_usage(self.prunedir) @@ -190,8 +186,8 @@ class PruneTest(BitcoinTestFramework): # Verify that we have enough history to reorg back to the fork point # Although this is more than 288 blocks, because this chain was written more recently - # and only its other 299 small and 220 large block are in the block files after it, - # its expected to still be retained + # and only its other 299 small and 220 large blocks are in the block files after it, + # it is expected to still be retained self.nodes[2].getblock(self.nodes[2].getblockhash(self.forkheight)) first_reorg_height = self.nodes[2].getblockcount() @@ -218,11 +214,8 @@ class PruneTest(BitcoinTestFramework): goalbestheight = first_reorg_height + 1 self.log.info("Verify node 2 reorged back to the main chain, some blocks of which it had to redownload") - waitstart = time.time() - while self.nodes[2].getblockcount() < goalbestheight: - time.sleep(0.1) - if time.time() - waitstart > 900: - raise AssertionError("Node 2 didn't reorg to proper height") + # Wait for Node 2 to reorg to proper height + wait_until(lambda: self.nodes[2].getblockcount() >= goalbestheight, timeout=900) assert(self.nodes[2].getbestblockhash() == goalbesthash) # Verify we can now have the data for a block previously pruned assert(self.nodes[2].getblock(self.forkhash)["height"] == self.forkheight) @@ -262,7 +255,7 @@ class PruneTest(BitcoinTestFramework): assert_equal(ret, expected_ret) def has_block(index): - return os.path.isfile(self.options.tmpdir + "/node{}/regtest/blocks/blk{:05}.dat".format(node_number, index)) + return os.path.isfile(os.path.join(self.nodes[node_number].datadir, "regtest", "blocks", "blk{:05}.dat".format(index))) # should not prune because chain tip of node 3 (995) < PruneAfterHeight (1000) assert_raises_rpc_error(-1, "Blockchain is too short for pruning", node.pruneblockchain, height(500)) diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py index ac67e6e9ba..d1d3f1d7f1 100755 --- a/test/functional/feature_reindex.py +++ b/test/functional/feature_reindex.py @@ -10,8 +10,7 @@ """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal -import time +from test_framework.util import wait_until class ReindexTest(BitcoinTestFramework): @@ -25,9 +24,7 @@ class ReindexTest(BitcoinTestFramework): self.stop_nodes() extra_args = [["-reindex-chainstate" if justchainstate else "-reindex", "-checkblockindex=1"]] self.start_nodes(extra_args) - while self.nodes[0].getblockcount() < blockcount: - time.sleep(0.1) - assert_equal(self.nodes[0].getblockcount(), blockcount) + wait_until(lambda: self.nodes[0].getblockcount() == blockcount) self.log.info("Success") def run_test(self): diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index d8c80ab34f..e29fdc84e7 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -29,11 +29,17 @@ class TestBitcoinCli(BitcoinTestFramework): self.log.info("Test -stdinrpcpass option") assert_equal(0, self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input=password).getblockcount()) - assert_raises_process_error(1, "incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input="foo").echo) + assert_raises_process_error(1, "Incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input="foo").echo) self.log.info("Test -stdin and -stdinrpcpass") assert_equal(["foo", "bar"], self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input=password + "\nfoo\nbar").echo()) - assert_raises_process_error(1, "incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input="foo").echo) + assert_raises_process_error(1, "Incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input="foo").echo) + + self.log.info("Test connecting to a non-existing server") + assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcport=1').echo) + + self.log.info("Test connecting with non-existing RPC cookie file") + assert_raises_process_error(1, "Could not locate RPC credentials", self.nodes[0].cli('-rpccookiefile=does-not-exist', '-rpcpassword=').echo) self.log.info("Make sure that -getinfo with arguments fails") assert_raises_process_error(1, "-getinfo takes no arguments", self.nodes[0].cli('-getinfo').help) diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 8440f13a0d..c6cb4c54cd 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -107,7 +107,7 @@ class RESTTest (BitcoinTestFramework): #check chainTip response assert_equal(json_obj['chaintipHash'], bb_hash) - #make sure there is no utox in the response because this oupoint has been spent + #make sure there is no utxo in the response because this oupoint has been spent assert_equal(len(json_obj['utxos']), 0) #check bitmap diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 53748df915..76e5e02c01 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -29,7 +29,7 @@ Test is as follows: transactions in its mempool. This tests that -persistmempool=0 does not overwrite a previously valid mempool stored on disk. - Remove node0 mempool.dat and verify savemempool RPC recreates it - and verify that node1 can load it and has 5 transaction in its + and verify that node1 can load it and has 5 transactions in its mempool. - Verify that savemempool throws when the RPC is called if node1 can't write to disk. diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 32e2b47fc9..e754dd31ad 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -124,7 +124,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): assert(tx_id not in self.nodes[0].getrawmempool()) # This is a less than 1000-byte transaction, so just set the fee - # to be the minimum for a 1000 byte transaction and check that it is + # to be the minimum for a 1000-byte transaction and check that it is # accepted. self.nodes[0].prioritisetransaction(txid=tx_id, fee_delta=int(self.relayfee*COIN)) diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index d9f461a049..1657d97281 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -14,8 +14,8 @@ from test_framework.util import * from test_framework.blocktools import create_block, create_coinbase, add_witness_commitment from test_framework.script import CScript, OP_TRUE -# TestNode: A peer we use to send messages to bitcoind, and store responses. -class TestNode(P2PInterface): +# TestP2PConn: A peer we use to send messages to bitcoind, and store responses. +class TestP2PConn(P2PInterface): def __init__(self): super().__init__() self.last_sendcmpct = [] @@ -548,7 +548,7 @@ class CompactBlocksTest(BitcoinTestFramework): # Note that it's possible for bitcoind to be smart enough to know we're # lying, since it could check to see if the shortid matches what we're # sending, and eg disconnect us for misbehavior. If that behavior - # change were made, we could just modify this test by having a + # change was made, we could just modify this test by having a # different peer provide the block further down, so that we're still # verifying that the block isn't marked bad permanently. This is good # enough for now. @@ -788,9 +788,9 @@ class CompactBlocksTest(BitcoinTestFramework): def run_test(self): # Setup the p2p connections and start up the network thread. - self.test_node = self.nodes[0].add_p2p_connection(TestNode()) - self.segwit_node = self.nodes[1].add_p2p_connection(TestNode(), services=NODE_NETWORK|NODE_WITNESS) - self.old_node = self.nodes[1].add_p2p_connection(TestNode(), services=NODE_NETWORK) + self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn()) + self.segwit_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK|NODE_WITNESS) + self.old_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK) network_thread_start() diff --git a/test/functional/p2p_feefilter.py b/test/functional/p2p_feefilter.py index 47d9c55160..7c954cdca2 100755 --- a/test/functional/p2p_feefilter.py +++ b/test/functional/p2p_feefilter.py @@ -22,7 +22,7 @@ def allInvsMatch(invsExpected, testnode): time.sleep(1) return False -class TestNode(P2PInterface): +class TestP2PConn(P2PInterface): def __init__(self): super().__init__() self.txinvs = [] @@ -48,7 +48,7 @@ class FeeFilterTest(BitcoinTestFramework): sync_blocks(self.nodes) # Setup the p2p connections and start up the network thread. - self.nodes[0].add_p2p_connection(TestNode()) + self.nodes[0].add_p2p_connection(TestP2PConn()) network_thread_start() self.nodes[0].p2p.wait_for_verack() diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index ce4e6e9144..198dcc1490 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -7,7 +7,7 @@ A node should never send anything other than VERSION/VERACK/REJECT until it's received a VERACK. -This test connects to a node and sends it a few messages, trying to intice it +This test connects to a node and sends it a few messages, trying to entice it into sending us something it shouldn't. Also test that nodes that send unsupported service bits to bitcoind are disconnected diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py index 81a41d6a97..301d8c181a 100755 --- a/test/functional/p2p_node_network_limited.py +++ b/test/functional/p2p_node_network_limited.py @@ -64,7 +64,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework): blocks = self.nodes[1].generate(292) sync_blocks([self.nodes[0], self.nodes[1]]) - self.log.info("Make sure we can max retrive block at tip-288.") + self.log.info("Make sure we can max retrieve block at tip-288.") node.send_getdata_for_block(blocks[1]) # last block in valid range node.wait_for_block(int(blocks[1], 16), timeout=3) diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 20e4805df0..8b226c2e9d 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -59,7 +59,7 @@ def test_witness_block(rpc, p2p, block, accepted, with_witness=True): p2p.sync_with_ping() assert_equal(rpc.getbestblockhash() == block.hash, accepted) -class TestNode(P2PInterface): +class TestP2PConn(P2PInterface): def __init__(self): super().__init__() self.getdataset = set() @@ -1511,7 +1511,7 @@ class SegWitTest(BitcoinTestFramework): # Make sure that this peer thinks segwit has activated. assert(get_bip9_status(self.nodes[node_id], 'segwit')['status'] == "active") - # Make sure this peers blocks match those of node0. + # Make sure this peer's blocks match those of node0. height = self.nodes[node_id].getblockcount() while height >= 0: block_hash = self.nodes[node_id].getblockhash(height) @@ -1878,11 +1878,11 @@ class SegWitTest(BitcoinTestFramework): def run_test(self): # Setup the p2p connections and start up the network thread. # self.test_node sets NODE_WITNESS|NODE_NETWORK - self.test_node = self.nodes[0].add_p2p_connection(TestNode(), services=NODE_NETWORK|NODE_WITNESS) + self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK|NODE_WITNESS) # self.old_node sets only NODE_NETWORK - self.old_node = self.nodes[0].add_p2p_connection(TestNode(), services=NODE_NETWORK) + self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK) # self.std_node is for testing node1 (fRequireStandard=true) - self.std_node = self.nodes[1].add_p2p_connection(TestNode(), services=NODE_NETWORK|NODE_WITNESS) + self.std_node = self.nodes[1].add_p2p_connection(TestP2PConn(), services=NODE_NETWORK|NODE_WITNESS) network_thread_start() diff --git a/test/functional/p2p_timeouts.py b/test/functional/p2p_timeouts.py index 6d21095cc6..6a21b693b4 100755 --- a/test/functional/p2p_timeouts.py +++ b/test/functional/p2p_timeouts.py @@ -27,7 +27,7 @@ from test_framework.mininode import * from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * -class TestNode(P2PInterface): +class TestP2PConn(P2PInterface): def on_version(self, message): # Don't send a verack in response pass @@ -39,9 +39,9 @@ class TimeoutsTest(BitcoinTestFramework): def run_test(self): # Setup the p2p connections and start up the network thread. - no_verack_node = self.nodes[0].add_p2p_connection(TestNode()) - no_version_node = self.nodes[0].add_p2p_connection(TestNode(), send_version=False) - no_send_node = self.nodes[0].add_p2p_connection(TestNode(), send_version=False) + no_verack_node = self.nodes[0].add_p2p_connection(TestP2PConn()) + no_version_node = self.nodes[0].add_p2p_connection(TestP2PConn(), send_version=False) + no_send_node = self.nodes[0].add_p2p_connection(TestP2PConn(), send_version=False) network_thread_start() diff --git a/test/functional/p2p_unrequested_blocks.py b/test/functional/p2p_unrequested_blocks.py index 672626f15b..53b2856eb5 100755 --- a/test/functional/p2p_unrequested_blocks.py +++ b/test/functional/p2p_unrequested_blocks.py @@ -166,7 +166,7 @@ class AcceptBlockTest(BitcoinTestFramework): self.log.info("Unrequested more-work block accepted") # 4c. Now mine 288 more blocks and deliver; all should be processed but - # the last (height-too-high) on node (as long as its not missing any headers) + # the last (height-too-high) on node (as long as it is not missing any headers) tip = block_h3 all_blocks = [] for i in range(288): diff --git a/test/functional/rpc_bind.py b/test/functional/rpc_bind.py index d43c2cd5d0..414da02072 100755 --- a/test/functional/rpc_bind.py +++ b/test/functional/rpc_bind.py @@ -55,7 +55,7 @@ class RPCBindTest(BitcoinTestFramework): def run_test(self): # due to OS-specific network stats queries, this test works only on Linux if not sys.platform.startswith('linux'): - raise SkipTest("This test can only be run on linux.") + raise SkipTest("This test can only be run on Linux.") # find the first non-loopback interface for testing non_loopback_ip = None for name,ip in all_interfaces(): diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 5f34b35bfb..d8348432aa 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -7,8 +7,6 @@ Tests correspond to code in rpc/net.cpp. """ -import time - from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -62,12 +60,8 @@ class NetTest(BitcoinTestFramework): self.nodes[0].setnetworkactive(False) assert_equal(self.nodes[0].getnetworkinfo()['networkactive'], False) - timeout = 3 - while self.nodes[0].getnetworkinfo()['connections'] != 0: - # Wait a bit for all sockets to close - assert timeout > 0, 'not all connections closed in time' - timeout -= 0.1 - time.sleep(0.1) + # Wait a bit for all sockets to close + wait_until(lambda: self.nodes[0].getnetworkinfo()['connections'] == 0, timeout=3) self.nodes[0].setnetworkactive(True) connect_nodes_bi(self.nodes, 0, 1) diff --git a/test/functional/test_framework/comptool.py b/test/functional/test_framework/comptool.py index 61ea2280e2..e0ca78e5d1 100755 --- a/test/functional/test_framework/comptool.py +++ b/test/functional/test_framework/comptool.py @@ -8,7 +8,7 @@ To use, create a class that implements get_tests(), and pass it in as the test generator to TestManager. get_tests() should be a python generator that returns TestInstance objects. See below for definition. -TestNode behaves as follows: +TestP2PConn behaves as follows: Configure with a BlockStore and TxStore on_inv: log the message but don't request on_headers: log the chain tip @@ -39,7 +39,7 @@ class RejectResult(): def __repr__(self): return '%i:%s' % (self.code,self.reason or '*') -class TestNode(P2PInterface): +class TestP2PConn(P2PInterface): def __init__(self, block_store, tx_store): super().__init__() @@ -170,7 +170,7 @@ class TestManager(): def add_all_connections(self, nodes): for i in range(len(nodes)): # Create a p2p connection to each node - node = TestNode(self.block_store, self.tx_store) + node = TestP2PConn(self.block_store, self.tx_store) node.peer_connect('127.0.0.1', p2p_port(i)) self.p2p_connections.append(node) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index e032be1337..ee573e01cc 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -4,7 +4,7 @@ # Copyright (c) 2010-2017 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. -"""Bitcoin test framework primitive and message strcutures +"""Bitcoin test framework primitive and message structures CBlock, CTransaction, CBlockHeader, CTxIn, CTxOut, etc....: data structures that should map to corresponding structures in diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index 99d0abc3f9..f1f7d0c0cd 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -405,7 +405,7 @@ class P2PInterface(P2PConnection): # Keep our own socket map for asyncore, so that we can track disconnects -# ourselves (to workaround an issue with closing an asyncore socket when +# ourselves (to work around an issue with closing an asyncore socket when # using select) mininode_socket_map = dict() @@ -424,7 +424,7 @@ class NetworkThread(threading.Thread): def run(self): while mininode_socket_map: # We check for whether to disconnect outside of the asyncore - # loop to workaround the behavior of asyncore when using + # loop to work around the behavior of asyncore when using # select disconnected = [] for fd, obj in mininode_socket_map.items(): diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index aeb335edb3..1b5e1a594b 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -314,7 +314,7 @@ class BitcoinTestFramework(): blockchain. If the cached version of the blockchain is used without mocktime then the mempools will not sync due to IBD. - For backwared compatibility of the python scripts with previous + For backward compatibility of the python scripts with previous versions of the cache, this helper function sets mocktime to Jan 1, 2014 + (201 * 10 * 60)""" self.mocktime = 1388534400 + (201 * 10 * 60) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 8d6ec618d5..8257219ffd 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -62,7 +62,7 @@ class TestNode(): if extra_conf != None: append_config(dirname, i, extra_conf) # Most callers will just need to add extra args to the standard list below. - # For those callers that need more flexibity, they can just set the args property directly. + # For those callers that need more flexibility, they can just set the args property directly. # Note that common args are set in the config file (see initialize_datadir) self.extra_args = extra_args self.args = [self.binary, "-datadir=" + self.datadir, "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(mocktime), "-uacomment=testnode%d" % i] diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 7be695550b..68ac97d755 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -340,20 +340,15 @@ def disconnect_nodes(from_connection, node_num): for peer_id in [peer['id'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']]: from_connection.disconnectnode(nodeid=peer_id) - for _ in range(50): - if [peer['id'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']] == []: - break - time.sleep(0.1) - else: - raise AssertionError("timed out waiting for disconnect") + # wait to disconnect + wait_until(lambda: [peer['id'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']] == [], timeout=5) def connect_nodes(from_connection, node_num): ip_port = "127.0.0.1:" + str(p2p_port(node_num)) from_connection.addnode(ip_port, "onetry") # poll until version handshake complete to avoid race conditions # with transaction relaying - while any(peer['version'] == 0 for peer in from_connection.getpeerinfo()): - time.sleep(0.1) + wait_until(lambda: all(peer['version'] != 0 for peer in from_connection.getpeerinfo())) def connect_nodes_bi(nodes, a, b): connect_nodes(nodes[a], b) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 082191098e..09f7f50de0 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -52,6 +52,9 @@ if os.name == 'posix': TEST_EXIT_PASSED = 0 TEST_EXIT_SKIPPED = 77 +# 20 minutes represented in seconds +TRAVIS_TIMEOUT_DURATION = 20 * 60 + BASE_SCRIPTS= [ # Scripts that are run by the travis build process. # Longest test should go first, to favor running tests in parallel @@ -233,29 +236,27 @@ def main(): sys.exit(0) # Build list of tests + test_list = [] if tests: # Individual tests have been specified. Run specified tests that exist # in the ALL_SCRIPTS list. Accept the name with or without .py extension. - tests = [re.sub("\.py$", "", t) + ".py" for t in tests] - test_list = [] - for t in tests: - if t in ALL_SCRIPTS: - test_list.append(t) + tests = [re.sub("\.py$", "", test) + ".py" for test in tests] + for test in tests: + if test in ALL_SCRIPTS: + test_list.append(test) else: - print("{}WARNING!{} Test '{}' not found in full test list.".format(BOLD[1], BOLD[0], t)) + print("{}WARNING!{} Test '{}' not found in full test list.".format(BOLD[1], BOLD[0], test)) + elif args.extended: + # Include extended tests + test_list += ALL_SCRIPTS else: - # No individual tests have been specified. - # Run all base tests, and optionally run extended tests. - test_list = BASE_SCRIPTS - if args.extended: - # place the EXTENDED_SCRIPTS first since the three longest ones - # are there and the list is shorter - test_list = EXTENDED_SCRIPTS + test_list + # Run base tests only + test_list += BASE_SCRIPTS # Remove the test cases that the user has explicitly asked to exclude. if args.exclude: - tests_excl = [re.sub("\.py$", "", t) + ".py" for t in args.exclude.split(',')] - for exclude_test in tests_excl: + exclude_tests = [re.sub("\.py$", "", test) + ".py" for test in args.exclude.split(',')] + for exclude_test in exclude_tests: if exclude_test in test_list: test_list.remove(exclude_test) else: @@ -320,7 +321,7 @@ def run_tests(test_list, src_dir, build_dir, exeext, tmpdir, jobs=1, enable_cove #Run Tests job_queue = TestHandler(jobs, tests_dir, tmpdir, test_list, flags) - time0 = time.time() + start_time = time.time() test_results = [] max_len_name = len(max(test_list, key=len)) @@ -346,7 +347,7 @@ def run_tests(test_list, src_dir, build_dir, exeext, tmpdir, jobs=1, enable_cove combined_logs, _ = subprocess.Popen([sys.executable, os.path.join(tests_dir, 'combine_logs.py'), '-c', testdir], universal_newlines=True, stdout=subprocess.PIPE).communicate() print("\n".join(deque(combined_logs.splitlines(), combined_logs_len))) - print_results(test_results, max_len_name, (int(time.time() - time0))) + print_results(test_results, max_len_name, (int(time.time() - start_time))) if coverage: coverage.report_rpc_coverage() @@ -403,15 +404,15 @@ class TestHandler: while self.num_running < self.num_jobs and self.test_list: # Add tests self.num_running += 1 - t = self.test_list.pop(0) + test = self.test_list.pop(0) portseed = len(self.test_list) + self.portseed_offset portseed_arg = ["--portseed={}".format(portseed)] log_stdout = tempfile.SpooledTemporaryFile(max_size=2**16) log_stderr = tempfile.SpooledTemporaryFile(max_size=2**16) - test_argv = t.split() + test_argv = test.split() testdir = "{}/{}_{}".format(self.tmpdir, re.sub(".py$", "", test_argv[0]), portseed) tmpdir_arg = ["--tmpdir={}".format(testdir)] - self.jobs.append((t, + self.jobs.append((test, time.time(), subprocess.Popen([sys.executable, self.tests_dir + test_argv[0]] + test_argv[1:] + self.flags + portseed_arg + tmpdir_arg, universal_newlines=True, @@ -425,15 +426,14 @@ class TestHandler: while True: # Return first proc that finishes time.sleep(.5) - for j in self.jobs: - (name, time0, proc, testdir, log_out, log_err) = j - if os.getenv('TRAVIS') == 'true' and int(time.time() - time0) > 20 * 60: - # In travis, timeout individual tests after 20 minutes (to stop tests hanging and not - # providing useful output. + for job in self.jobs: + (name, start_time, proc, testdir, log_out, log_err) = job + if os.getenv('TRAVIS') == 'true' and int(time.time() - start_time) > TRAVIS_TIMEOUT_DURATION: + # In travis, timeout individual tests (to stop tests hanging and not providing useful output). proc.send_signal(signal.SIGINT) if proc.poll() is not None: log_out.seek(0), log_err.seek(0) - [stdout, stderr] = [l.read().decode('utf-8') for l in (log_out, log_err)] + [stdout, stderr] = [log_file.read().decode('utf-8') for log_file in (log_out, log_err)] log_out.close(), log_err.close() if proc.returncode == TEST_EXIT_PASSED and stderr == "": status = "Passed" @@ -442,9 +442,9 @@ class TestHandler: else: status = "Failed" self.num_running -= 1 - self.jobs.remove(j) + self.jobs.remove(job) - return TestResult(name, status, int(time.time() - time0)), testdir, stdout, stderr + return TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr print('.', end='', flush=True) class TestResult(): @@ -490,7 +490,7 @@ def check_script_list(src_dir): Check that there are no scripts in the functional tests directory which are not being run by pull-tester.py.""" script_dir = src_dir + '/test/functional/' - python_files = set([t for t in os.listdir(script_dir) if t[-3:] == ".py"]) + python_files = set([test_file for test_file in os.listdir(script_dir) if test_file.endswith(".py")]) missed_tests = list(python_files - set(map(lambda x: x.split()[0], ALL_SCRIPTS + NON_SCRIPTS))) if len(missed_tests) != 0: print("%sWARNING!%s The following scripts are not being run: %s. Check the test lists in test_runner.py." % (BOLD[1], BOLD[0], str(missed_tests))) @@ -526,7 +526,7 @@ class RPCCoverage(): if uncovered: print("Uncovered RPC commands:") - print("".join((" - %s\n" % i) for i in sorted(uncovered))) + print("".join((" - %s\n" % command) for command in sorted(uncovered))) else: print("All RPC commands covered.") @@ -550,8 +550,8 @@ class RPCCoverage(): if not os.path.isfile(coverage_ref_filename): raise RuntimeError("No coverage reference found") - with open(coverage_ref_filename, 'r') as f: - all_cmds.update([i.strip() for i in f.readlines()]) + with open(coverage_ref_filename, 'r') as coverage_ref_file: + all_cmds.update([line.strip() for line in coverage_ref_file.readlines()]) for root, dirs, files in os.walk(self.dir): for filename in files: @@ -559,8 +559,8 @@ class RPCCoverage(): coverage_filenames.add(os.path.join(root, filename)) for filename in coverage_filenames: - with open(filename, 'r') as f: - covered_cmds.update([i.strip() for i in f.readlines()]) + with open(filename, 'r') as coverage_file: + covered_cmds.update([line.strip() for line in coverage_file.readlines()]) return all_cmds - covered_cmds diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 7e0635d80f..d5ef08d782 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -109,7 +109,7 @@ class AbandonConflictTest(BitcoinTestFramework): assert_equal(len(self.nodes[0].getrawmempool()), 0) assert_equal(self.nodes[0].getbalance(), balance) - # But if its received again then it is unabandoned + # But if it is received again then it is unabandoned # And since now in mempool, the change is available # But its child tx remains abandoned self.nodes[0].sendrawtransaction(signed["hex"]) @@ -117,7 +117,7 @@ class AbandonConflictTest(BitcoinTestFramework): assert_equal(newbalance, balance - Decimal("20") + Decimal("14.99998")) balance = newbalance - # Send child tx again so its unabandoned + # Send child tx again so it is unabandoned self.nodes[0].sendrawtransaction(signed2["hex"]) newbalance = self.nodes[0].getbalance() assert_equal(newbalance, balance - Decimal("10") - Decimal("14.99998") + Decimal("24.9996")) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index f686cb6ea5..0436aca6a4 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -283,7 +283,7 @@ class WalletTest(BitcoinTestFramework): sync_blocks(self.nodes[0:3]) node_2_bal += 2 - #tx should be added to balance because after restarting the nodes tx should be broadcastet + #tx should be added to balance because after restarting the nodes tx should be broadcast assert_equal(self.nodes[2].getbalance(), node_2_bal) #send a tx with value in a string (PR#6380 +) @@ -379,9 +379,9 @@ class WalletTest(BitcoinTestFramework): self.start_node(0, [m, "-limitancestorcount="+str(chainlimit)]) self.start_node(1, [m, "-limitancestorcount="+str(chainlimit)]) self.start_node(2, [m, "-limitancestorcount="+str(chainlimit)]) - while m == '-reindex' and [block_count] * 3 != [self.nodes[i].getblockcount() for i in range(3)]: + if m == '-reindex': # reindex will leave rpc warm up "early"; Wait for it to finish - time.sleep(0.1) + wait_until(lambda: [block_count] * 3 == [self.nodes[i].getblockcount() for i in range(3)]) assert_equal(balance_nodes, [self.nodes[i].getbalance() for i in range(3)]) # Exercise listsinceblock with the last two blocks diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 25e2716661..0f2434ff0d 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -211,7 +211,7 @@ class ListSinceBlockTest (BitcoinTestFramework): 1. tx1 is listed in listsinceblock. 2. It is included in 'removed' as it was removed, even though it is now present in a different block. - 3. It is listed with a confirmations count of 2 (bb3, bb4), not + 3. It is listed with a confirmation count of 2 (bb3, bb4), not 3 (aa1, aa2, aa3). ''' diff --git a/test/util/data/bitcoin-util-test.json b/test/util/data/bitcoin-util-test.json index 89b28bba6c..a115aa30d9 100644 --- a/test/util/data/bitcoin-util-test.json +++ b/test/util/data/bitcoin-util-test.json @@ -144,12 +144,12 @@ { "exec": "./bitcoin-tx", "args": ["02000000000100000000000000000000000000"], "output_cmp": "txcreate2.hex", - "description": "Parses a transation with no inputs and a single output script" + "description": "Parses a transaction with no inputs and a single output script" }, { "exec": "./bitcoin-tx", "args": ["-json", "02000000000100000000000000000000000000"], "output_cmp": "txcreate2.json", - "description": "Parses a transation with no inputs and a single output script (output in json)" + "description": "Parses a transaction with no inputs and a single output script (output in json)" }, { "exec": "./bitcoin-tx", "args": ["-create", "outscript=0:OP_DROP", "nversion=1"], |