diff options
89 files changed, 545 insertions, 372 deletions
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/bench/lockedpool.cpp b/src/bench/lockedpool.cpp index ca30d81e59..55a00318c1 100644 --- a/src/bench/lockedpool.cpp +++ b/src/bench/lockedpool.cpp @@ -43,4 +43,4 @@ static void BenchLockedPool(benchmark::State& state) addr.clear(); } -BENCHMARK(BenchLockedPool, 530); +BENCHMARK(BenchLockedPool, 1300); 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/init.cpp b/src/init.cpp index e0efe5c0a2..30ee0e694c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1165,6 +1165,9 @@ static bool LockDataDirectory(bool probeOnly) { // Make sure only a single Bitcoin process is using the data directory. fs::path datadir = GetDataDir(); + if (!DirIsWritable(datadir)) { + return InitError(strprintf(_("Cannot write to data directory '%s'; check permissions."), datadir.string())); + } if (!LockDirectory(datadir, ".lock", probeOnly)) { return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), datadir.string(), _(PACKAGE_NAME))); } diff --git a/src/net.cpp b/src/net.cpp index 4849e067aa..342dfbaeb9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1955,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/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/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 ba2f7d0d1d..38411c499f 100644 --- a/src/qt/editaddressdialog.cpp +++ b/src/qt/editaddressdialog.cpp @@ -25,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; @@ -73,10 +69,9 @@ 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(), model->GetDefaultAddressType()); 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 1d0b66dfd8..65ef250440 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); 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/walletmodel.cpp b/src/qt/walletmodel.cpp index 5e7aefb187..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; } diff --git a/src/random.h b/src/random.h index ea88670a37..1d6b13a537 100644 --- a/src/random.h +++ b/src/random.h @@ -11,6 +11,7 @@ #include <uint256.h> #include <stdint.h> +#include <limits> /* Seed OpenSSL PRNG with additional entropy data */ void RandAddSeed(); @@ -32,7 +33,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); @@ -121,6 +122,12 @@ public: /** Generate a random boolean. */ bool randbool() { return randbits(1); } + + // Compatibility with the C++11 UniformRandomBitGenerator concept + typedef uint64_t result_type; + static constexpr uint64_t min() { return 0; } + static constexpr uint64_t max() { return std::numeric_limits<uint64_t>::max(); } + inline uint64_t operator()() { return rand64(); } }; /* Number of random bytes returned by GetOSRand. 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..f10fd07c63 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -47,7 +47,9 @@ Arena::Arena(void *base_in, size_t size_in, size_t alignment_in): base(static_cast<char*>(base_in)), end(static_cast<char*>(base_in) + size_in), alignment(alignment_in) { // Start with one free chunk that covers the entire arena - chunks_free.emplace(base, size_in); + auto it = size_to_free_chunk.emplace(size_in, base); + chunks_free.emplace(base, it); + chunks_free_end.emplace(base + size_in, it); } Arena::~Arena() @@ -63,26 +65,30 @@ void* Arena::alloc(size_t size) if (size == 0) return nullptr; - // Pick a large enough free-chunk - auto it = std::find_if(chunks_free.begin(), chunks_free.end(), - [=](const std::map<char*, size_t>::value_type& chunk){ return chunk.second >= size; }); - if (it == chunks_free.end()) + // Pick a large enough free-chunk. Returns an iterator pointing to the first element that is not less than key. + // This allocation strategy is best-fit. According to "Dynamic Storage Allocation: A Survey and Critical Review", + // Wilson et. al. 1995, http://www.scs.stanford.edu/14wi-cs140/sched/readings/wilson.pdf, best-fit and first-fit + // policies seem to work well in practice. + auto size_ptr_it = size_to_free_chunk.lower_bound(size); + if (size_ptr_it == size_to_free_chunk.end()) return nullptr; // Create the used-chunk, taking its space from the end of the free-chunk - auto alloced = chunks_used.emplace(it->first + it->second - size, size).first; - if (!(it->second -= size)) - chunks_free.erase(it); - return reinterpret_cast<void*>(alloced->first); -} - -/* extend the Iterator if other begins at its end */ -template <class Iterator, class Pair> bool extend(Iterator it, const Pair& other) { - if (it->first + it->second == other.first) { - it->second += other.second; - return true; + const size_t size_remaining = size_ptr_it->first - size; + auto alloced = chunks_used.emplace(size_ptr_it->second + size_remaining, size).first; + chunks_free_end.erase(size_ptr_it->second + size_ptr_it->first); + if (size_ptr_it->first == size) { + // whole chunk is used up + chunks_free.erase(size_ptr_it->second); + } else { + // still some memory left in the chunk + auto it_remaining = size_to_free_chunk.emplace(size_remaining, size_ptr_it->second); + chunks_free[size_ptr_it->second] = it_remaining; + chunks_free_end.emplace(size_ptr_it->second + size_remaining, it_remaining); } - return false; + size_to_free_chunk.erase(size_ptr_it); + + return reinterpret_cast<void*>(alloced->first); } void Arena::free(void *ptr) @@ -97,16 +103,30 @@ void Arena::free(void *ptr) if (i == chunks_used.end()) { throw std::runtime_error("Arena: invalid or double free"); } - auto freed = *i; + std::pair<char*, size_t> freed = *i; chunks_used.erase(i); - // Add space to free map, coalescing contiguous chunks - auto next = chunks_free.upper_bound(freed.first); - auto prev = (next == chunks_free.begin()) ? chunks_free.end() : std::prev(next); - if (prev == chunks_free.end() || !extend(prev, freed)) - prev = chunks_free.emplace_hint(next, freed); - if (next != chunks_free.end() && extend(prev, *next)) + // coalesce freed with previous chunk + auto prev = chunks_free_end.find(freed.first); + if (prev != chunks_free_end.end()) { + freed.first -= prev->second->first; + freed.second += prev->second->first; + size_to_free_chunk.erase(prev->second); + chunks_free_end.erase(prev); + } + + // coalesce freed with chunk after freed + auto next = chunks_free.find(freed.first + freed.second); + if (next != chunks_free.end()) { + freed.second += next->second->first; + size_to_free_chunk.erase(next->second); chunks_free.erase(next); + } + + // Add/set space with coalesced free chunk + auto it = size_to_free_chunk.emplace(freed.second, freed.first); + chunks_free[freed.first] = it; + chunks_free_end[freed.first + freed.second] = it; } Arena::Stats Arena::stats() const @@ -115,7 +135,7 @@ Arena::Stats Arena::stats() const for (const auto& chunk: chunks_used) r.used += chunk.second; for (const auto& chunk: chunks_free) - r.free += chunk.second; + r.free += chunk.second->first; r.total = r.used + r.free; return r; } @@ -184,7 +204,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/support/lockedpool.h b/src/support/lockedpool.h index fc85e6c73c..ccfae16701 100644 --- a/src/support/lockedpool.h +++ b/src/support/lockedpool.h @@ -10,6 +10,7 @@ #include <map> #include <mutex> #include <memory> +#include <unordered_map> /** * OS-dependent allocation and deallocation of locked/pinned memory pages. @@ -88,11 +89,19 @@ public: */ bool addressInArena(void *ptr) const { return ptr >= base && ptr < end; } private: - /** Map of chunk address to chunk information. This class makes use of the - * sorted order to merge previous and next chunks during deallocation. - */ - std::map<char*, size_t> chunks_free; - std::map<char*, size_t> chunks_used; + typedef std::multimap<size_t, char*> SizeToChunkSortedMap; + /** Map to enable O(log(n)) best-fit allocation, as it's sorted by size */ + SizeToChunkSortedMap size_to_free_chunk; + + typedef std::unordered_map<char*, SizeToChunkSortedMap::const_iterator> ChunkToSizeMap; + /** Map from begin of free chunk to its node in size_to_free_chunk */ + ChunkToSizeMap chunks_free; + /** Map from end of free chunk to its node in size_to_free_chunk */ + ChunkToSizeMap chunks_free_end; + + /** Map from begin of used chunk to its size */ + std::unordered_map<char*, size_t> chunks_used; + /** Base address of arena */ char* base; /** End address of arena */ 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/random_tests.cpp b/src/test/random_tests.cpp index 1ca5a53d72..623ed239f0 100644 --- a/src/test/random_tests.cpp +++ b/src/test/random_tests.cpp @@ -8,6 +8,9 @@ #include <boost/test/unit_test.hpp> +#include <random> +#include <algorithm> + BOOST_FIXTURE_TEST_SUITE(random_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(osrandom_tests) @@ -57,4 +60,23 @@ BOOST_AUTO_TEST_CASE(fastrandom_randbits) } } +/** Does-it-compile test for compatibility with standard C++11 RNG interface. */ +BOOST_AUTO_TEST_CASE(stdrandom_test) +{ + FastRandomContext ctx; + std::uniform_int_distribution<int> distribution(3, 9); + for (int i = 0; i < 100; ++i) { + int x = distribution(ctx); + BOOST_CHECK(x >= 3); + BOOST_CHECK(x <= 9); + + std::vector<int> test{1,2,3,4,5,6,7,8,9,10}; + std::shuffle(test.begin(), test.end(), ctx); + for (int j = 1; j <= 10; ++j) { + BOOST_CHECK(std::find(test.begin(), test.end(), j) != test.end()); + } + } + +} + BOOST_AUTO_TEST_SUITE_END() 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/test/util_tests.cpp b/src/test/util_tests.cpp index 84b61bea86..b6f3cbe2b7 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -817,4 +817,20 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory) fs::remove_all(dirname); } +BOOST_AUTO_TEST_CASE(test_DirIsWritable) +{ + // Should be able to write to the system tmp dir. + fs::path tmpdirname = fs::temp_directory_path(); + BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true); + + // Should not be able to write to a non-existent dir. + tmpdirname = fs::temp_directory_path() / fs::unique_path(); + BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), false); + + fs::create_directory(tmpdirname); + // Should be able to write to it now. + BOOST_CHECK_EQUAL(DirIsWritable(tmpdirname), true); + fs::remove(tmpdirname); +} + BOOST_AUTO_TEST_SUITE_END() 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..494d5c4eaf 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) { @@ -419,6 +419,19 @@ void ReleaseDirectoryLocks() dir_locks.clear(); } +bool DirIsWritable(const fs::path& directory) +{ + fs::path tmpFile = directory / fs::unique_path(); + + FILE* file = fsbridge::fopen(tmpFile, "a"); + if (!file) return false; + + fclose(file); + remove(tmpFile); + + return true; +} + /** Interpret string as boolean, for argument parsing */ static bool InterpretBool(const std::string& strValue) { diff --git a/src/util.h b/src/util.h index e4170d8aa2..04ff44f218 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) @@ -174,6 +174,7 @@ int RaiseFileDescriptorLimit(int nMinFD); void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length); bool RenameOver(fs::path src, fs::path dest); bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false); +bool DirIsWritable(const fs::path& directory); /** Release all directory locks. This is used for unit testing only, at runtime * the global destructor will take care of the locks. 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/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7c173278c5..dbc48834ff 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -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."); } @@ -3580,7 +3572,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/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index f05c81cd4c..6c36e2e965 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -4,6 +4,7 @@ #include "wallet/wallet.h" #include "wallet/coinselection.h" +#include "wallet/coincontrol.h" #include "amount.h" #include "primitives/transaction.h" #include "random.h" @@ -27,7 +28,7 @@ std::vector<std::unique_ptr<CWalletTx>> wtxn; typedef std::set<CInputCoin> CoinSet; static std::vector<COutput> vCoins; -static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); +static CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); @@ -72,6 +73,7 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa } COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); vCoins.push_back(output); + testWallet.AddToWallet(*wtx.get()); wtxn.emplace_back(std::move(wtx)); } @@ -222,6 +224,18 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(1); vCoins.at(0).nInputBytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + + // Make sure that we aren't using BnB when there are preset inputs + empty_wallet(); + add_coin(5 * CENT); + add_coin(3 * CENT); + add_coin(2 * CENT); + CCoinControl coin_control; + coin_control.fAllowOtherInputs = true; + coin_control.Select(COutPoint(vCoins.at(0).tx->GetHash(), vCoins.at(0).i)); + BOOST_CHECK(testWallet.SelectCoins(vCoins, 10 * CENT, setCoinsRet, nValueRet, coin_control, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!bnb_used); + BOOST_CHECK(!coin_selection_params_bnb.use_bnb); } BOOST_AUTO_TEST_CASE(knapsack_solver_test) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4cd083a9c4..b37a411ef4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1267,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(); @@ -2425,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(); @@ -2460,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); @@ -2480,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); @@ -2491,7 +2491,7 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil } } -bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const +bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const { std::vector<COutput> vCoins(vAvailableCoins); @@ -2521,6 +2521,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm { // For now, don't use BnB if preset inputs are selected. TODO: Enable this later bnb_used = false; + coin_selection_params.use_bnb = false; std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(outpoint.hash); if (it != mapWallet.end()) @@ -4197,8 +4198,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; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 34902ec85e..3ef5bfbb65 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -665,15 +665,6 @@ private: std::mutex mutexScanning; friend class WalletRescanReserver; - - /** - * Select a set of coins such that nValueRet >= nTargetValue and at least - * all coins from coinControl are selected; Never select unconfirmed coins - * if they are not ours - */ - bool SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, - const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; - CWalletDB *pwalletdbEncryption; //! the current wallet version: clients below this version are not able to load the wallet @@ -766,6 +757,14 @@ public: return *dbw; } + /** + * Select a set of coins such that nValueRet >= nTargetValue and at least + * all coins from coinControl are selected; Never select unconfirmed coins + * if they are not ours + */ + bool SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, + const CCoinControl& coin_control, CoinSelectionParams& coin_selection_params, bool& bnb_used) const; + /** Get a name for this wallet for logging/debugging purposes. */ const std::string& GetName() const { return m_name; } @@ -853,7 +852,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; @@ -1167,7 +1166,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. */ 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_bip9_softforks.py b/test/functional/feature_bip9_softforks.py index 71d3d04002..ac6176e976 100755 --- a/test/functional/feature_bip9_softforks.py +++ b/test/functional/feature_bip9_softforks.py @@ -241,7 +241,7 @@ class BIP9SoftForksTest(ComparisonTestFramework): self.test.clear_all_connections() self.stop_nodes() self.nodes = [] - shutil.rmtree(self.options.tmpdir + "/node0") + shutil.rmtree(get_datadir_path(self.options.tmpdir, 0)) self.setup_chain() self.setup_network() self.test.add_all_connections(self.nodes) diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index c6cec0596b..6b1e473aa2 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -5,9 +5,10 @@ """Test various command line arguments and configuration file parameters.""" import os +import re from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import get_datadir_path + class ConfArgsTest(BitcoinTestFramework): def set_test_params(self): @@ -19,19 +20,19 @@ class ConfArgsTest(BitcoinTestFramework): # Remove the -datadir argument so it doesn't override the config file self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")] - default_data_dir = get_datadir_path(self.options.tmpdir, 0) + default_data_dir = self.nodes[0].datadir new_data_dir = os.path.join(default_data_dir, 'newdatadir') new_data_dir_2 = os.path.join(default_data_dir, 'newdatadir2') # Check that using -datadir argument on non-existent directory fails self.nodes[0].datadir = new_data_dir - self.assert_start_raises_init_error(0, ['-datadir='+new_data_dir], 'Error: Specified data directory "' + new_data_dir + '" does not exist.') + self.nodes[0].assert_start_raises_init_error(['-datadir=' + new_data_dir], 'Error: Specified data directory "' + re.escape(new_data_dir) + '" does not exist.') # Check that using non-existent datadir in conf file fails conf_file = os.path.join(default_data_dir, "bitcoin.conf") with open(conf_file, 'a', encoding='utf8') as f: f.write("datadir=" + new_data_dir + "\n") - self.assert_start_raises_init_error(0, ['-conf='+conf_file], 'Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.') + self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error reading configuration file: specified data directory "' + re.escape(new_data_dir) + '" does not exist.') # Create the directory and ensure the config file now works os.mkdir(new_data_dir) 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_logging.py b/test/functional/feature_logging.py index da4e7b0398..a4ebc7cca3 100755 --- a/test/functional/feature_logging.py +++ b/test/functional/feature_logging.py @@ -30,8 +30,8 @@ class LoggingTest(BitcoinTestFramework): invdir = os.path.join(self.nodes[0].datadir, "regtest", "foo") invalidname = os.path.join("foo", "foo.log") self.stop_node(0) - self.assert_start_raises_init_error(0, ["-debuglogfile=%s" % (invalidname)], - "Error: Could not open debug log file") + exp_stderr = "Error: Could not open debug log file \S+$" + self.nodes[0].assert_start_raises_init_error(["-debuglogfile=%s" % (invalidname)], exp_stderr) assert not os.path.isfile(os.path.join(invdir, "foo.log")) # check that invalid log (relative) works after path exists @@ -44,8 +44,7 @@ class LoggingTest(BitcoinTestFramework): self.stop_node(0) invdir = os.path.join(self.options.tmpdir, "foo") invalidname = os.path.join(invdir, "foo.log") - self.assert_start_raises_init_error(0, ["-debuglogfile=%s" % invalidname], - "Error: Could not open debug log file") + self.nodes[0].assert_start_raises_init_error(["-debuglogfile=%s" % invalidname], exp_stderr) assert not os.path.isfile(os.path.join(invdir, "foo.log")) # check that invalid log (absolute) works after path exists 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..ad305a6e80 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): @@ -45,7 +44,7 @@ class PruneTest(BitcoinTestFramework): def setup_network(self): self.setup_nodes() - self.prunedir = self.options.tmpdir + "/node2/regtest/blocks/" + self.prunedir = os.path.join(self.nodes[2].datadir, 'regtest', 'blocks', '') connect_nodes(self.nodes[0], 1) connect_nodes(self.nodes[1], 2) @@ -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/feature_uacomment.py b/test/functional/feature_uacomment.py index bc3791508a..c73bdcfbb8 100755 --- a/test/functional/feature_uacomment.py +++ b/test/functional/feature_uacomment.py @@ -4,9 +4,12 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the -uacomment option.""" +import re + from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal + class UacommentTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 @@ -23,13 +26,14 @@ class UacommentTest(BitcoinTestFramework): self.log.info("test -uacomment max length") self.stop_node(0) - expected = "exceeds maximum length (256). Reduce the number or size of uacomments." - self.assert_start_raises_init_error(0, ["-uacomment=" + 'a' * 256], expected) + expected = "Error: Total length of network version string \([0-9]+\) exceeds maximum length \(256\). Reduce the number or size of uacomments." + self.nodes[0].assert_start_raises_init_error(["-uacomment=" + 'a' * 256], expected) self.log.info("test -uacomment unsafe characters") for unsafe_char in ['/', ':', '(', ')']: - expected = "User Agent comment (" + unsafe_char + ") contains unsafe characters" - self.assert_start_raises_init_error(0, ["-uacomment=" + unsafe_char], expected) + expected = "Error: User Agent comment \(" + re.escape(unsafe_char) + "\) contains unsafe characters." + self.nodes[0].assert_start_raises_init_error(["-uacomment=" + unsafe_char], expected) + if __name__ == '__main__': UacommentTest().main() 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..75eb9b1784 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. @@ -93,8 +93,8 @@ class MempoolPersistTest(BitcoinTestFramework): self.start_node(0) wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5) - mempooldat0 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'mempool.dat') - mempooldat1 = os.path.join(self.options.tmpdir, 'node1', 'regtest', 'mempool.dat') + mempooldat0 = os.path.join(self.nodes[0].datadir, 'regtest', 'mempool.dat') + mempooldat1 = os.path.join(self.nodes[1].datadir, 'regtest', 'mempool.dat') self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") os.remove(mempooldat0) self.nodes[0].savemempool() 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..5b50520d3f 100755 --- a/test/functional/rpc_bind.py +++ b/test/functional/rpc_bind.py @@ -48,14 +48,14 @@ class RPCBindTest(BitcoinTestFramework): self.nodes[0].rpchost = None self.start_nodes([base_args]) # connect to node through non-loopback interface - node = get_rpc_proxy(rpc_url(get_datadir_path(self.options.tmpdir, 0), 0, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir) + node = get_rpc_proxy(rpc_url(self.nodes[0].datadir, 0, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir) node.getnetworkinfo() self.stop_nodes() 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/rpc_users.py b/test/functional/rpc_users.py index 01f68344ae..0ce412f74a 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -5,13 +5,18 @@ """Test multiple RPC users.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import str_to_b64str, assert_equal +from test_framework.util import ( + assert_equal, + get_datadir_path, + str_to_b64str, +) import os import http.client import urllib.parse -class HTTPBasicsTest (BitcoinTestFramework): + +class HTTPBasicsTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 @@ -22,10 +27,10 @@ class HTTPBasicsTest (BitcoinTestFramework): rpcauth2 = "rpcauth=rt2:f8607b1a88861fac29dfccf9b52ff9f$ff36a0c23c8c62b4846112e50fa888416e94c17bfd4c42f88fd8f55ec6a3137e" rpcuser = "rpcuser=rpcuser💻" rpcpassword = "rpcpassword=rpcpassword🔑" - with open(os.path.join(self.options.tmpdir+"/node0", "bitcoin.conf"), 'a', encoding='utf8') as f: + with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f: f.write(rpcauth+"\n") f.write(rpcauth2+"\n") - with open(os.path.join(self.options.tmpdir+"/node1", "bitcoin.conf"), 'a', encoding='utf8') as f: + with open(os.path.join(get_datadir_path(self.options.tmpdir, 1), "bitcoin.conf"), 'a', encoding='utf8') as f: f.write(rpcuser+"\n") f.write(rpcpassword+"\n") @@ -54,7 +59,7 @@ class HTTPBasicsTest (BitcoinTestFramework): resp = conn.getresponse() assert_equal(resp.status, 200) conn.close() - + #Use new authpair to confirm both work headers = {"Authorization": "Basic " + str_to_b64str(authpairnew)} 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 8efac9c475..d427f62856 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -228,7 +228,7 @@ class BitcoinTestFramework(): assert_equal(len(extra_args), num_nodes) assert_equal(len(binary), num_nodes) for i in range(num_nodes): - self.nodes.append(TestNode(i, self.options.tmpdir, rpchost=rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli)) + self.nodes.append(TestNode(i, get_datadir_path(self.options.tmpdir, i), rpchost=rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, extra_conf=extra_confs[i], extra_args=extra_args[i], use_cli=self.options.usecli)) def start_node(self, i, *args, **kwargs): """Start a bitcoind""" @@ -281,27 +281,6 @@ class BitcoinTestFramework(): self.stop_node(i) self.start_node(i, extra_args) - def assert_start_raises_init_error(self, i, extra_args=None, expected_msg=None, *args, **kwargs): - with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr: - try: - self.start_node(i, extra_args, stderr=log_stderr, *args, **kwargs) - self.stop_node(i) - except Exception as e: - assert 'bitcoind exited' in str(e) # node must have shutdown - self.nodes[i].running = False - self.nodes[i].process = None - if expected_msg is not None: - log_stderr.seek(0) - stderr = log_stderr.read().decode('utf-8') - if expected_msg not in stderr: - raise AssertionError("Expected error \"" + expected_msg + "\" not found in:\n" + stderr) - else: - if expected_msg is None: - assert_msg = "bitcoind should have exited with an error" - else: - assert_msg = "bitcoind should have exited with expected error " + expected_msg - raise AssertionError(assert_msg) - def wait_for_node_exit(self, i, timeout): self.nodes[i].process.wait(timeout) @@ -335,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) @@ -400,7 +379,7 @@ class BitcoinTestFramework(): args = [os.getenv("BITCOIND", "bitcoind"), "-datadir=" + datadir] if i > 0: args.append("-connect=127.0.0.1:" + str(p2p_port(0))) - self.nodes.append(TestNode(i, self.options.cachedir, extra_conf=["bind=127.0.0.1"], extra_args=[],rpchost=None, timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None)) + self.nodes.append(TestNode(i, get_datadir_path(self.options.cachedir, i), extra_conf=["bind=127.0.0.1"], extra_args=[],rpchost=None, timewait=None, binary=None, stderr=None, mocktime=self.mocktime, coverage_dir=None)) self.nodes[i].args = args self.start_node(i) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 86e44e4c97..583d07deec 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -12,6 +12,7 @@ import logging import os import re import subprocess +import tempfile import time from .authproxy import JSONRPCException @@ -43,9 +44,9 @@ class TestNode(): To make things easier for the test writer, any unrecognised messages will be dispatched to the RPC connection.""" - def __init__(self, i, dirname, rpchost, timewait, binary, stderr, mocktime, coverage_dir, extra_conf=None, extra_args=None, use_cli=False): + def __init__(self, i, datadir, rpchost, timewait, binary, stderr, mocktime, coverage_dir, extra_conf=None, extra_args=None, use_cli=False): self.index = i - self.datadir = os.path.join(dirname, "node" + str(i)) + self.datadir = datadir self.rpchost = rpchost if timewait: self.rpc_timeout = timewait @@ -59,9 +60,9 @@ class TestNode(): self.stderr = stderr self.coverage_dir = coverage_dir if extra_conf != None: - append_config(dirname, i, extra_conf) + append_config(datadir, 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] @@ -165,6 +166,41 @@ class TestNode(): def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT): wait_until(self.is_node_stopped, timeout=timeout) + def assert_start_raises_init_error(self, extra_args=None, expected_msg=None, partial_match=False, *args, **kwargs): + """Attempt to start the node and expect it to raise an error. + + extra_args: extra arguments to pass through to bitcoind + expected_msg: regex that stderr should match when bitcoind fails + + Will throw if bitcoind starts without an error. + Will throw if an expected_msg is provided and it does not match bitcoind's stdout.""" + with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr: + try: + self.start(extra_args, stderr=log_stderr, *args, **kwargs) + self.wait_for_rpc_connection() + self.stop_node() + self.wait_util_stopped() + except Exception as e: + assert 'bitcoind exited' in str(e) # node must have shutdown + self.running = False + self.process = None + # Check stderr for expected message + if expected_msg is not None: + log_stderr.seek(0) + stderr = log_stderr.read().decode('utf-8').strip() + if partial_match: + if re.search(expected_msg, stderr, flags=re.MULTILINE) is None: + raise AssertionError('Expected message "{}" does not partially match stderr:\n"{}"'.format(expected_msg, stderr)) + else: + if re.fullmatch(expected_msg, stderr) is None: + raise AssertionError('Expected message "{}" does not fully match stderr:\n"{}"'.format(expected_msg, stderr)) + else: + if expected_msg is None: + assert_msg = "bitcoind should have exited with an error" + else: + assert_msg = "bitcoind should have exited with expected error " + expected_msg + raise AssertionError(assert_msg) + def node_encrypt_wallet(self, passphrase): """"Encrypts the wallet. diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 7be695550b..041e2b86e8 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -284,7 +284,7 @@ def rpc_url(datadir, i, rpchost=None): ################ def initialize_datadir(dirname, n): - datadir = os.path.join(dirname, "node" + str(n)) + datadir = get_datadir_path(dirname, n) if not os.path.isdir(datadir): os.makedirs(datadir) with open(os.path.join(datadir, "bitcoin.conf"), 'w', encoding='utf8') as f: @@ -300,8 +300,7 @@ def initialize_datadir(dirname, n): def get_datadir_path(dirname, n): return os.path.join(dirname, "node" + str(n)) -def append_config(dirname, n, options): - datadir = get_datadir_path(dirname, n) +def append_config(datadir, options): with open(os.path.join(datadir, "bitcoin.conf"), 'a', encoding='utf8') as f: for option in options: f.write(option + "\n") @@ -340,20 +339,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/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_backup.py b/test/functional/wallet_backup.py index b4be7debb5..46a72d7e28 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -90,9 +90,9 @@ class WalletBackupTest(BitcoinTestFramework): self.stop_node(2) def erase_three(self): - os.remove(self.options.tmpdir + "/node0/regtest/wallets/wallet.dat") - os.remove(self.options.tmpdir + "/node1/regtest/wallets/wallet.dat") - os.remove(self.options.tmpdir + "/node2/regtest/wallets/wallet.dat") + os.remove(os.path.join(self.nodes[0].datadir, 'regtest', 'wallets', 'wallet.dat')) + os.remove(os.path.join(self.nodes[1].datadir, 'regtest', 'wallets', 'wallet.dat')) + os.remove(os.path.join(self.nodes[2].datadir, 'regtest', 'wallets', 'wallet.dat')) def run_test(self): self.log.info("Generating initial blockchain") @@ -116,13 +116,13 @@ class WalletBackupTest(BitcoinTestFramework): self.do_one_round() self.log.info("Backing up") - tmpdir = self.options.tmpdir - self.nodes[0].backupwallet(tmpdir + "/node0/wallet.bak") - self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.dump") - self.nodes[1].backupwallet(tmpdir + "/node1/wallet.bak") - self.nodes[1].dumpwallet(tmpdir + "/node1/wallet.dump") - self.nodes[2].backupwallet(tmpdir + "/node2/wallet.bak") - self.nodes[2].dumpwallet(tmpdir + "/node2/wallet.dump") + + self.nodes[0].backupwallet(os.path.join(self.nodes[0].datadir, 'wallet.bak')) + self.nodes[0].dumpwallet(os.path.join(self.nodes[0].datadir, 'wallet.dump')) + self.nodes[1].backupwallet(os.path.join(self.nodes[1].datadir, 'wallet.bak')) + self.nodes[1].dumpwallet(os.path.join(self.nodes[1].datadir, 'wallet.dump')) + self.nodes[2].backupwallet(os.path.join(self.nodes[2].datadir, 'wallet.bak')) + self.nodes[2].dumpwallet(os.path.join(self.nodes[2].datadir, 'wallet.dump')) self.log.info("More transactions") for i in range(5): @@ -150,13 +150,13 @@ class WalletBackupTest(BitcoinTestFramework): self.erase_three() # Start node2 with no chain - shutil.rmtree(self.options.tmpdir + "/node2/regtest/blocks") - shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate") + shutil.rmtree(os.path.join(self.nodes[2].datadir, 'regtest', 'blocks')) + shutil.rmtree(os.path.join(self.nodes[2].datadir, 'regtest', 'chainstate')) # Restore wallets from backup - shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallets/wallet.dat") - shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallets/wallet.dat") - shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallets/wallet.dat") + shutil.copyfile(os.path.join(self.nodes[0].datadir, 'wallet.bak'), os.path.join(self.nodes[0].datadir, 'regtest', 'wallets', 'wallet.dat')) + shutil.copyfile(os.path.join(self.nodes[1].datadir, 'wallet.bak'), os.path.join(self.nodes[1].datadir, 'regtest', 'wallets', 'wallet.dat')) + shutil.copyfile(os.path.join(self.nodes[2].datadir, 'wallet.bak'), os.path.join(self.nodes[2].datadir, 'regtest', 'wallets', 'wallet.dat')) self.log.info("Re-starting nodes") self.start_three() @@ -171,8 +171,8 @@ class WalletBackupTest(BitcoinTestFramework): self.erase_three() #start node2 with no chain - shutil.rmtree(self.options.tmpdir + "/node2/regtest/blocks") - shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate") + shutil.rmtree(os.path.join(self.nodes[2].datadir, 'regtest', 'blocks')) + shutil.rmtree(os.path.join(self.nodes[2].datadir, 'regtest', 'chainstate')) self.start_three() @@ -180,9 +180,9 @@ class WalletBackupTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(), 0) assert_equal(self.nodes[2].getbalance(), 0) - self.nodes[0].importwallet(tmpdir + "/node0/wallet.dump") - self.nodes[1].importwallet(tmpdir + "/node1/wallet.dump") - self.nodes[2].importwallet(tmpdir + "/node2/wallet.dump") + self.nodes[0].importwallet(os.path.join(self.nodes[0].datadir, 'wallet.dump')) + self.nodes[1].importwallet(os.path.join(self.nodes[1].datadir, 'wallet.dump')) + self.nodes[2].importwallet(os.path.join(self.nodes[2].datadir, 'wallet.dump')) sync_blocks(self.nodes) @@ -192,10 +192,10 @@ class WalletBackupTest(BitcoinTestFramework): # Backup to source wallet file must fail sourcePaths = [ - tmpdir + "/node0/regtest/wallets/wallet.dat", - tmpdir + "/node0/./regtest/wallets/wallet.dat", - tmpdir + "/node0/regtest/wallets/", - tmpdir + "/node0/regtest/wallets"] + os.path.join(self.nodes[0].datadir, 'regtest', 'wallets', 'wallet.dat'), + os.path.join(self.nodes[0].datadir, 'regtest', '.', 'wallets', 'wallet.dat'), + os.path.join(self.nodes[0].datadir, 'regtest', 'wallets', ''), + os.path.join(self.nodes[0].datadir, 'regtest', 'wallets')] for sourcePath in sourcePaths: assert_raises_rpc_error(-4, "backup failed", self.nodes[0].backupwallet, sourcePath) 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_hd.py b/test/functional/wallet_hd.py index 91f77dd5ba..eb6747c6f4 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -4,13 +4,15 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test Hierarchical Deterministic wallet function.""" +import os +import shutil + from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, connect_nodes_bi, ) -import shutil -import os + class WalletHDTest(BitcoinTestFramework): def set_test_params(self): @@ -18,12 +20,10 @@ class WalletHDTest(BitcoinTestFramework): self.num_nodes = 2 self.extra_args = [[], ['-keypool=0']] - def run_test (self): - tmpdir = self.options.tmpdir - + def run_test(self): # Make sure can't switch off usehd after wallet creation self.stop_node(1) - self.assert_start_raises_init_error(1, ['-usehd=0'], 'already existing HD wallet') + self.nodes[1].assert_start_raises_init_error(['-usehd=0'], "Error: Error loading : You can't disable HD on an already existing HD wallet") self.start_node(1) connect_nodes_bi(self.nodes, 0, 1) @@ -41,8 +41,8 @@ class WalletHDTest(BitcoinTestFramework): self.nodes[1].importprivkey(self.nodes[0].dumpprivkey(non_hd_add)) # This should be enough to keep the master key and the non-HD key - self.nodes[1].backupwallet(tmpdir + "/hd.bak") - #self.nodes[1].dumpwallet(tmpdir + "/hd.dump") + self.nodes[1].backupwallet(os.path.join(self.nodes[1].datadir, "hd.bak")) + #self.nodes[1].dumpwallet(os.path.join(self.nodes[1].datadir, "hd.dump")) # Derive some HD addresses and remember the last # Also send funds to each add @@ -71,9 +71,9 @@ class WalletHDTest(BitcoinTestFramework): self.stop_node(1) # we need to delete the complete regtest directory # otherwise node1 would auto-recover all funds in flag the keypool keys as used - shutil.rmtree(os.path.join(tmpdir, "node1/regtest/blocks")) - shutil.rmtree(os.path.join(tmpdir, "node1/regtest/chainstate")) - shutil.copyfile(os.path.join(tmpdir, "hd.bak"), os.path.join(tmpdir, "node1/regtest/wallets/wallet.dat")) + shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "blocks")) + shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate")) + shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_node(1) # Assert that derivation is deterministic @@ -94,9 +94,9 @@ class WalletHDTest(BitcoinTestFramework): # Try a RPC based rescan self.stop_node(1) - shutil.rmtree(os.path.join(tmpdir, "node1/regtest/blocks")) - shutil.rmtree(os.path.join(tmpdir, "node1/regtest/chainstate")) - shutil.copyfile(os.path.join(tmpdir, "hd.bak"), os.path.join(tmpdir, "node1/regtest/wallet.dat")) + shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "blocks")) + shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate")) + shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat")) self.start_node(1, extra_args=self.extra_args[1]) connect_nodes_bi(self.nodes, 0, 1) self.sync_all() diff --git a/test/functional/wallet_keypool_topup.py b/test/functional/wallet_keypool_topup.py index e7b76dfaf2..30a0c9a760 100755 --- a/test/functional/wallet_keypool_topup.py +++ b/test/functional/wallet_keypool_topup.py @@ -10,6 +10,7 @@ Two nodes. Node1 is under test. Node0 is providing transactions and generating b - Generate 110 keys (enough to drain the keypool). Store key 90 (in the initial keypool) and key 110 (beyond the initial keypool). Send funds to key 90 and key 110. - Stop node1, clear the datadir, move wallet file back into the datadir and restart node1. - connect node1 to node0. Verify that they sync and node1 receives its funds.""" +import os import shutil from test_framework.test_framework import BitcoinTestFramework @@ -19,6 +20,7 @@ from test_framework.util import ( sync_blocks, ) + class KeypoolRestoreTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True @@ -26,26 +28,23 @@ class KeypoolRestoreTest(BitcoinTestFramework): self.extra_args = [[], ['-keypool=100', '-keypoolmin=20']] def run_test(self): - self.tmpdir = self.options.tmpdir + wallet_path = os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat") + wallet_backup_path = os.path.join(self.nodes[1].datadir, "wallet.bak") self.nodes[0].generate(101) self.log.info("Make backup of wallet") - self.stop_node(1) - - shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak") + shutil.copyfile(wallet_path, wallet_backup_path) self.start_node(1, self.extra_args[1]) connect_nodes_bi(self.nodes, 0, 1) self.log.info("Generate keys for wallet") - for _ in range(90): addr_oldpool = self.nodes[1].getnewaddress() for _ in range(20): addr_extpool = self.nodes[1].getnewaddress() self.log.info("Send funds to wallet") - self.nodes[0].sendtoaddress(addr_oldpool, 10) self.nodes[0].generate(1) self.nodes[0].sendtoaddress(addr_extpool, 5) @@ -53,22 +52,18 @@ class KeypoolRestoreTest(BitcoinTestFramework): sync_blocks(self.nodes) self.log.info("Restart node with wallet backup") - self.stop_node(1) - - shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallets/wallet.dat") - - self.log.info("Verify keypool is restored and balance is correct") - + shutil.copyfile(wallet_backup_path, wallet_path) self.start_node(1, self.extra_args[1]) connect_nodes_bi(self.nodes, 0, 1) self.sync_all() + self.log.info("Verify keypool is restored and balance is correct") assert_equal(self.nodes[1].getbalance(), 15) assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive") - # Check that we have marked all keys up to the used keypool key as used assert_equal(self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress())['hdkeypath'], "m/0'/0'/110'") + if __name__ == '__main__': KeypoolRestoreTest().main() 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/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 378c06ee59..0285263ef9 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -7,10 +7,15 @@ Verify that a bitcoind node can load multiple wallet files """ import os +import re import shutil from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) + class MultiWalletTest(BitcoinTestFramework): def set_test_params(self): @@ -60,29 +65,31 @@ class MultiWalletTest(BitcoinTestFramework): assert_equal(os.path.isfile(wallet_dir(wallet_name)), True) # should not initialize if wallet path can't be created - self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'Not a directory') + exp_stderr = "boost::filesystem::create_directory: (The system cannot find the path specified|Not a directory):" + self.nodes[0].assert_start_raises_init_error(['-wallet=wallet.dat/bad'], exp_stderr, partial_match=True) - self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') - self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) - self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) + self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist') + self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir()) + self.nodes[0].assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir()) # should not initialize if there are duplicate wallets - self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.') + self.nodes[0].assert_start_raises_init_error(['-wallet=w1', '-wallet=w1'], 'Error: Error loading wallet w1. Duplicate -wallet filename specified.') # should not initialize if one wallet is a copy of another shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) - self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid') + exp_stderr = "CDB: Can't open database w8_copy \(duplicates fileid \w+ from w8\)" + self.nodes[0].assert_start_raises_init_error(['-wallet=w8', '-wallet=w8_copy'], exp_stderr, partial_match=True) # should not initialize if wallet file is a symlink os.symlink('w8', wallet_dir('w8_symlink')) - self.assert_start_raises_init_error(0, ['-wallet=w8_symlink'], 'Invalid -wallet path') + self.nodes[0].assert_start_raises_init_error(['-wallet=w8_symlink'], 'Error: Invalid -wallet path \'w8_symlink\'\. .*') # should not initialize if the specified walletdir does not exist - self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') + self.nodes[0].assert_start_raises_init_error(['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist') # should not initialize if the specified walletdir is not a directory not_a_dir = wallet_dir('notadir') open(not_a_dir, 'a').close() - self.assert_start_raises_init_error(0, ['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory') + self.nodes[0].assert_start_raises_init_error(['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + re.escape(not_a_dir) + '" is not a directory') # if wallets/ doesn't exist, datadir should be the default wallet dir wallet_dir2 = data_dir('walletdir') @@ -102,8 +109,9 @@ class MultiWalletTest(BitcoinTestFramework): competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir') os.mkdir(competing_wallet_dir) - self.restart_node(0, ['-walletdir='+competing_wallet_dir]) - self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment') + self.restart_node(0, ['-walletdir=' + competing_wallet_dir]) + exp_stderr = "Error: Error initializing wallet database environment \"\S+competing_walletdir\"!" + self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr, partial_match=True) self.restart_node(0, extra_args) 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"], |