diff options
40 files changed, 566 insertions, 263 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index fb8e0f7f59..ecca08d96d 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -105,6 +105,8 @@ Low-level RPC changes * `getwalletinfo` * `getmininginfo` - The wallet RPC `getreceivedbyaddress` will return an error if called with an address not in the wallet. +- The wallet RPC `addwitnessaddress` was deprecated and will be removed in version 0.17, + set the `address_type` argument of `getnewaddress`, or option `-addresstype=[bech32|p2sh-segwit]` instead. Changed command-line options ----------------------------- diff --git a/src/compat.h b/src/compat.h index 65e9683e2f..aae84b1181 100644 --- a/src/compat.h +++ b/src/compat.h @@ -33,7 +33,7 @@ #include <ws2tcpip.h> #include <stdint.h> #else -#include <sys/fcntl.h> +#include <fcntl.h> #include <sys/mman.h> #include <sys/select.h> #include <sys/socket.h> diff --git a/src/init.cpp b/src/init.cpp index 7215e87359..b48802637b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1143,23 +1143,10 @@ bool AppInitParameterInteraction() static bool LockDataDirectory(bool probeOnly) { - std::string strDataDir = GetDataDir().string(); - // Make sure only a single Bitcoin process is using the data directory. - fs::path pathLockFile = GetDataDir() / ".lock"; - FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. - if (file) fclose(file); - - try { - static boost::interprocess::file_lock lock(pathLockFile.string().c_str()); - if (!lock.try_lock()) { - return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running."), strDataDir, _(PACKAGE_NAME))); - } - if (probeOnly) { - lock.unlock(); - } - } catch(const boost::interprocess::interprocess_exception& e) { - return InitError(strprintf(_("Cannot obtain a lock on data directory %s. %s is probably already running.") + " %s.", strDataDir, _(PACKAGE_NAME), e.what())); + fs::path datadir = GetDataDir(); + 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))); } return true; } diff --git a/src/miner.cpp b/src/miner.cpp index 4e63ab4df0..dda52790c6 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -352,7 +352,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda // Try to compare the mapTx entry to the mapModifiedTx entry iter = mempool.mapTx.project<0>(mi); if (modit != mapModifiedTx.get<ancestor_score>().end() && - CompareModifiedEntry()(*modit, CTxMemPoolModifiedEntry(iter))) { + CompareTxMemPoolEntryByAncestorFee()(*modit, CTxMemPoolModifiedEntry(iter))) { // The best entry in mapModifiedTx has higher score // than the one from mapTx. // Switch which transaction (package) to consider diff --git a/src/miner.h b/src/miner.h index 698b4a4788..9c086332d4 100644 --- a/src/miner.h +++ b/src/miner.h @@ -41,6 +41,12 @@ struct CTxMemPoolModifiedEntry { nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors(); } + int64_t GetModifiedFee() const { return iter->GetModifiedFee(); } + uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; } + CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; } + size_t GetTxSize() const { return iter->GetTxSize(); } + const CTransaction& GetTx() const { return iter->GetTx(); } + CTxMemPool::txiter iter; uint64_t nSizeWithAncestors; CAmount nModFeesWithAncestors; @@ -67,21 +73,6 @@ struct modifiedentry_iter { } }; -// This matches the calculation in CompareTxMemPoolEntryByAncestorFee, -// except operating on CTxMemPoolModifiedEntry. -// TODO: refactor to avoid duplication of this logic. -struct CompareModifiedEntry { - bool operator()(const CTxMemPoolModifiedEntry &a, const CTxMemPoolModifiedEntry &b) const - { - double f1 = (double)a.nModFeesWithAncestors * b.nSizeWithAncestors; - double f2 = (double)b.nModFeesWithAncestors * a.nSizeWithAncestors; - if (f1 == f2) { - return CTxMemPool::CompareIteratorByHash()(a.iter, b.iter); - } - return f1 > f2; - } -}; - // A comparator that sorts transactions based on number of ancestors. // This is sufficient to sort an ancestor package in an order that is valid // to appear in a block. @@ -106,7 +97,7 @@ typedef boost::multi_index_container< // Reuse same tag from CTxMemPool's similar index boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolModifiedEntry>, - CompareModifiedEntry + CompareTxMemPoolEntryByAncestorFee > > > indexed_modified_transaction_set; diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index a2521a1e9e..74f0db3520 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -341,7 +341,7 @@ void AddressTableModel::updateEntry(const QString &address, priv->updateEntry(address, label, isMine, purpose, status); } -QString AddressTableModel::addRow(const QString &type, const QString &label, const QString &address) +QString AddressTableModel::addRow(const QString &type, const QString &label, const QString &address, const OutputType address_type) { std::string strLabel = label.toStdString(); std::string strAddress = address.toStdString(); @@ -384,8 +384,8 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con return QString(); } } - wallet->LearnRelatedScripts(newKey, g_address_type); - strAddress = EncodeDestination(GetDestinationForKey(newKey, g_address_type)); + wallet->LearnRelatedScripts(newKey, address_type); + strAddress = EncodeDestination(GetDestinationForKey(newKey, address_type)); } else { diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h index d04b95ebae..11439e25d5 100644 --- a/src/qt/addresstablemodel.h +++ b/src/qt/addresstablemodel.h @@ -8,6 +8,8 @@ #include <QAbstractTableModel> #include <QStringList> +enum OutputType : int; + class AddressTablePriv; class WalletModel; @@ -61,7 +63,7 @@ public: /* Add an address to the model. Returns the added address on success, and an empty string otherwise. */ - QString addRow(const QString &type, const QString &label, const QString &address); + QString addRow(const QString &type, const QString &label, const QString &address, const OutputType address_type); /* Look up label for address in address book, if not found return empty string. */ diff --git a/src/qt/editaddressdialog.cpp b/src/qt/editaddressdialog.cpp index 6b7df2f2b9..a945fc6aa0 100644 --- a/src/qt/editaddressdialog.cpp +++ b/src/qt/editaddressdialog.cpp @@ -11,6 +11,8 @@ #include <QDataWidgetMapper> #include <QMessageBox> +extern OutputType g_address_type; + EditAddressDialog::EditAddressDialog(Mode _mode, QWidget *parent) : QDialog(parent), ui(new Ui::EditAddressDialog), @@ -77,7 +79,8 @@ bool EditAddressDialog::saveCurrentRow() address = model->addRow( mode == NewSendingAddress ? AddressTableModel::Send : AddressTableModel::Receive, ui->labelEdit->text(), - ui->addressEdit->text()); + ui->addressEdit->text(), + g_address_type); break; case EditReceivingAddress: case EditSendingAddress: diff --git a/src/qt/forms/receivecoinsdialog.ui b/src/qt/forms/receivecoinsdialog.ui index 58f030ebf0..09fb435a58 100644 --- a/src/qt/forms/receivecoinsdialog.ui +++ b/src/qt/forms/receivecoinsdialog.ui @@ -7,7 +7,7 @@ <x>0</x> <y>0</y> <width>776</width> - <height>364</height> + <height>396</height> </rect> </property> <layout class="QVBoxLayout" name="verticalLayout" stretch="0,0,1"> @@ -28,6 +28,22 @@ <layout class="QVBoxLayout" name="verticalLayout_3"> <item> <layout class="QGridLayout" name="gridLayout"> + <item row="5" column="0"> + <widget class="QLabel" name="label"> + <property name="toolTip"> + <string>An optional amount to request. Leave this empty or zero to not request a specific amount.</string> + </property> + <property name="text"> + <string>&Amount:</string> + </property> + <property name="alignment"> + <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> + </property> + <property name="buddy"> + <cstring>reqAmount</cstring> + </property> + </widget> + </item> <item row="6" column="0"> <widget class="QLabel" name="label_3"> <property name="toolTip"> @@ -51,13 +67,6 @@ </property> </widget> </item> - <item row="6" column="2"> - <widget class="QLineEdit" name="reqMessage"> - <property name="toolTip"> - <string>An optional message to attach to the payment request, which will be displayed when the request is opened. Note: The message will not be sent with the payment over the Bitcoin network.</string> - </property> - </widget> - </item> <item row="2" column="2"> <widget class="QLabel" name="label_5"> <property name="text"> @@ -81,32 +90,10 @@ </property> </widget> </item> - <item row="5" column="0"> - <widget class="QLabel" name="label"> - <property name="toolTip"> - <string>An optional amount to request. Leave this empty or zero to not request a specific amount.</string> - </property> - <property name="text"> - <string>&Amount:</string> - </property> - <property name="alignment"> - <set>Qt::AlignRight|Qt::AlignTrailing|Qt::AlignVCenter</set> - </property> - <property name="buddy"> - <cstring>reqAmount</cstring> - </property> - </widget> - </item> - <item row="5" column="2"> - <widget class="BitcoinAmountField" name="reqAmount"> - <property name="minimumSize"> - <size> - <width>80</width> - <height>0</height> - </size> - </property> + <item row="6" column="2"> + <widget class="QLineEdit" name="reqMessage"> <property name="toolTip"> - <string>An optional amount to request. Leave this empty or zero to not request a specific amount.</string> + <string>An optional message to attach to the payment request, which will be displayed when the request is opened. Note: The message will not be sent with the payment over the Bitcoin network.</string> </property> </widget> </item> @@ -174,6 +161,73 @@ </property> </widget> </item> + <item row="5" column="2"> + <layout class="QHBoxLayout" name="horizontalLayout_6"> + <item> + <widget class="BitcoinAmountField" name="reqAmount"> + <property name="sizePolicy"> + <sizepolicy hsizetype="Fixed" vsizetype="Fixed"> + <horstretch>0</horstretch> + <verstretch>0</verstretch> + </sizepolicy> + </property> + <property name="minimumSize"> + <size> + <width>80</width> + <height>0</height> + </size> + </property> + <property name="maximumSize"> + <size> + <width>1000</width> + <height>100</height> + </size> + </property> + <property name="toolTip"> + <string>An optional amount to request. Leave this empty or zero to not request a specific amount.</string> + </property> + </widget> + </item> + <item> + <widget class="QCheckBox" name="useBech32"> + <property name="sizePolicy"> + <sizepolicy hsizetype="Fixed" vsizetype="Fixed"> + <horstretch>0</horstretch> + <verstretch>0</verstretch> + </sizepolicy> + </property> + <property name="maximumSize"> + <size> + <width>1000</width> + <height>16777215</height> + </size> + </property> + <property name="focusPolicy"> + <enum>Qt::StrongFocus</enum> + </property> + <property name="toolTip"> + <string>Bech32 addresses (BIP-173) are cheaper to spend from and offer better protection against typos. When unchecked a P2SH wrapped SegWit address will be created, compatible with older wallets.</string> + </property> + <property name="text"> + <string>Generate Bech32 address</string> + </property> + </widget> + </item> + <item> + <spacer name="horizontalSpacer_3"> + <property name="orientation"> + <enum>Qt::Horizontal</enum> + </property> + <property name="sizeHint" stdset="0"> + <size> + <width>0</width> + <height>0</height> + </size> + </property> + </spacer> + </item> + </layout> + </item> </layout> </item> </layout> @@ -306,6 +360,7 @@ <tabstops> <tabstop>reqLabel</tabstop> <tabstop>reqAmount</tabstop> + <tabstop>useBech32</tabstop> <tabstop>reqMessage</tabstop> <tabstop>receiveButton</tabstop> <tabstop>clearButton</tabstop> diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index 670d6108db..558d4f108c 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -995,6 +995,18 @@ QString formatBytes(uint64_t bytes) return QString(QObject::tr("%1 GB")).arg(bytes / 1024 / 1024 / 1024); } +qreal calculateIdealFontSize(int width, const QString& text, QFont font, qreal minPointSize, qreal font_size) { + while(font_size >= minPointSize) { + font.setPointSizeF(font_size); + QFontMetrics fm(font); + if (fm.width(text) < width) { + break; + } + font_size -= 0.5; + } + return font_size; +} + void ClickableLabel::mouseReleaseEvent(QMouseEvent *event) { Q_EMIT clicked(event->pos()); diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h index ad0e22ccd6..71a69483f5 100644 --- a/src/qt/guiutil.h +++ b/src/qt/guiutil.h @@ -201,6 +201,8 @@ namespace GUIUtil QString formatBytes(uint64_t bytes); + qreal calculateIdealFontSize(int width, const QString& text, QFont font, qreal minPointSize = 4, qreal startPointSize = 14); + class ClickableLabel : public QLabel { Q_OBJECT diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp index e07f2b74ee..7fd5285467 100644 --- a/src/qt/receivecoinsdialog.cpp +++ b/src/qt/receivecoinsdialog.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <wallet/wallet.h> + #include <qt/receivecoinsdialog.h> #include <qt/forms/ui_receivecoinsdialog.h> @@ -91,6 +93,15 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model) SLOT(recentRequestsView_selectionChanged(QItemSelection, QItemSelection))); // Last 2 columns are set by the columnResizingFixer, when the table geometry is ready. columnResizingFixer = new GUIUtil::TableViewLastColumnResizingFixer(tableView, AMOUNT_MINIMUM_COLUMN_WIDTH, DATE_COLUMN_WIDTH, this); + + // configure bech32 checkbox, disable if launched with legacy as default: + if (model->getDefaultAddressType() == OUTPUT_TYPE_BECH32) { + ui->useBech32->setCheckState(Qt::Checked); + } else { + ui->useBech32->setCheckState(Qt::Unchecked); + } + + ui->useBech32->setVisible(model->getDefaultAddressType() != OUTPUT_TYPE_LEGACY); } } @@ -133,7 +144,11 @@ void ReceiveCoinsDialog::on_receiveButton_clicked() QString address; QString label = ui->reqLabel->text(); /* Generate new receiving address */ - address = model->getAddressTableModel()->addRow(AddressTableModel::Receive, label, ""); + OutputType address_type = model->getDefaultAddressType(); + if (address_type != OUTPUT_TYPE_LEGACY) { + address_type = ui->useBech32->isChecked() ? OUTPUT_TYPE_BECH32 : OUTPUT_TYPE_P2SH_SEGWIT; + } + address = model->getAddressTableModel()->addRow(AddressTableModel::Receive, label, "", address_type); SendCoinsRecipient info(address, label, ui->reqAmount->value(), ui->reqMessage->text()); ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this); diff --git a/src/qt/receiverequestdialog.cpp b/src/qt/receiverequestdialog.cpp index 209397ca0c..d4cb0e5ba2 100644 --- a/src/qt/receiverequestdialog.cpp +++ b/src/qt/receiverequestdialog.cpp @@ -183,9 +183,13 @@ void ReceiveRequestDialog::update() QPainter painter(&qrAddrImage); painter.drawImage(0, 0, qrImage.scaled(QR_IMAGE_SIZE, QR_IMAGE_SIZE)); QFont font = GUIUtil::fixedPitchFont(); - font.setPixelSize(12); - painter.setFont(font); QRect paddedRect = qrAddrImage.rect(); + + // calculate ideal font size + qreal font_size = GUIUtil::calculateIdealFontSize(paddedRect.width() - 20, info.address, font); + font.setPointSizeF(font_size); + + painter.setFont(font); paddedRect.setHeight(QR_IMAGE_SIZE+12); painter.drawText(paddedRect, Qt::AlignBottom|Qt::AlignCenter, info.address); painter.end(); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 494b46905d..4d7e977fcb 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -733,6 +733,11 @@ bool WalletModel::hdEnabled() const return wallet->IsHDEnabled(); } +OutputType WalletModel::getDefaultAddressType() const +{ + return g_address_type; +} + int WalletModel::getDefaultConfirmTarget() const { return nTxConfirmTarget; diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 7d7e4f160e..9e13de79be 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -15,6 +15,8 @@ #include <QObject> +enum OutputType : int; + class AddressTableModel; class OptionsModel; class PlatformStyle; @@ -214,6 +216,8 @@ public: bool hdEnabled() const; + OutputType getDefaultAddressType() const; + int getDefaultConfirmTarget() const; private: diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 1b2c71c4a4..346672e45a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -24,6 +24,7 @@ #include <util.h> #include <utilstrencodings.h> #include <hash.h> +#include <validationinterface.h> #include <warnings.h> #include <stdint.h> @@ -323,6 +324,21 @@ UniValue waitforblockheight(const JSONRPCRequest& request) return ret; } +UniValue syncwithvalidationinterfacequeue(const JSONRPCRequest& request) +{ + if (request.fHelp || request.params.size() > 0) { + throw std::runtime_error( + "syncwithvalidationinterfacequeue\n" + "\nWaits for the validation interface queue to catch up on everything that was there when we entered this function.\n" + "\nExamples:\n" + + HelpExampleCli("syncwithvalidationinterfacequeue","") + + HelpExampleRpc("syncwithvalidationinterfacequeue","") + ); + } + SyncWithValidationInterfaceQueue(); + return NullUniValue; +} + UniValue getdifficulty(const JSONRPCRequest& request) { if (request.fHelp || request.params.size() != 0) @@ -1628,6 +1644,7 @@ static const CRPCCommand commands[] = { "hidden", "waitfornewblock", &waitfornewblock, {"timeout"} }, { "hidden", "waitforblock", &waitforblock, {"blockhash","timeout"} }, { "hidden", "waitforblockheight", &waitforblockheight, {"height","timeout"} }, + { "hidden", "syncwithvalidationinterfacequeue", &syncwithvalidationinterfacequeue, {} }, }; void RegisterBlockchainRPCCommands(CRPCTable &t) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 95a27d474b..3bcad16316 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -668,6 +668,18 @@ UniValue echo(const JSONRPCRequest& request) return request.params; } +static UniValue getinfo_deprecated(const JSONRPCRequest& request) +{ + throw JSONRPCError(RPC_METHOD_NOT_FOUND, + "getinfo\n" + "\nThis call was removed in version 0.16.0. Use the appropriate fields from:\n" + "- getblockchaininfo: blocks, difficulty, chain\n" + "- getnetworkinfo: version, protocolversion, timeoffset, connections, proxy, relayfee, warnings\n" + "- getwalletinfo: balance, keypoololdest, keypoolsize, paytxfee, unlocked_until, walletversion\n" + "\nbitcoin-cli has the option -getinfo to collect and format these in the old format." + ); +} + static const CRPCCommand commands[] = { // category name actor (function) argNames // --------------------- ------------------------ ----------------------- ---------- @@ -682,6 +694,7 @@ static const CRPCCommand commands[] = { "hidden", "setmocktime", &setmocktime, {"timestamp"}}, { "hidden", "echo", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, { "hidden", "echojson", &echo, {"arg0","arg1","arg2","arg3","arg4","arg5","arg6","arg7","arg8","arg9"}}, + { "hidden", "getinfo", &getinfo_deprecated, {}}, }; void RegisterMiscRPCCommands(CRPCTable &t) diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index f56f341498..1766c6a093 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -287,35 +287,6 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest) pool.removeRecursive(pool.mapTx.find(tx9.GetHash())->GetTx()); pool.removeRecursive(pool.mapTx.find(tx8.GetHash())->GetTx()); - /* Now check the sort on the mining score index. - * Final order should be: - * - * tx7 (2M) - * tx2 (20k) - * tx4 (15000) - * tx1/tx5 (10000) - * tx3/6 (0) - * (Ties resolved by hash) - */ - sortedOrder.clear(); - sortedOrder.push_back(tx7.GetHash().ToString()); - sortedOrder.push_back(tx2.GetHash().ToString()); - sortedOrder.push_back(tx4.GetHash().ToString()); - if (tx1.GetHash() < tx5.GetHash()) { - sortedOrder.push_back(tx5.GetHash().ToString()); - sortedOrder.push_back(tx1.GetHash().ToString()); - } else { - sortedOrder.push_back(tx1.GetHash().ToString()); - sortedOrder.push_back(tx5.GetHash().ToString()); - } - if (tx3.GetHash() < tx6.GetHash()) { - sortedOrder.push_back(tx6.GetHash().ToString()); - sortedOrder.push_back(tx3.GetHash().ToString()); - } else { - sortedOrder.push_back(tx3.GetHash().ToString()); - sortedOrder.push_back(tx6.GetHash().ToString()); - } - CheckSort<mining_score>(pool, sortedOrder); } BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) @@ -427,6 +398,23 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest) sortedOrder.erase(sortedOrder.end()-2); sortedOrder.insert(sortedOrder.begin(), tx7.GetHash().ToString()); CheckSort<ancestor_score>(pool, sortedOrder); + + // High-fee parent, low-fee child + // tx7 -> tx8 + CMutableTransaction tx8 = CMutableTransaction(); + tx8.vin.resize(1); + tx8.vin[0].prevout = COutPoint(tx7.GetHash(), 0); + tx8.vin[0].scriptSig = CScript() << OP_11; + tx8.vout.resize(1); + tx8.vout[0].scriptPubKey = CScript() << OP_11 << OP_EQUAL; + tx8.vout[0].nValue = 10*COIN; + + // Check that we sort by min(feerate, ancestor_feerate): + // set the fee so that the ancestor feerate is above tx1/5, + // but the transaction's own feerate is lower + pool.addUnchecked(tx8.GetHash(), entry.Fee(5000LL).FromTx(tx8)); + sortedOrder.insert(sortedOrder.end()-1, tx8.GetHash().ToString()); + CheckSort<ancestor_score>(pool, sortedOrder); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ffb024aef9..d1edde284f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -906,8 +906,8 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. - return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage; + // Estimate the overhead of mapTx to be 12 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 12 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage; } void CTxMemPool::RemoveStaged(setEntries &stage, bool updateDescendants, MemPoolRemovalReason reason) { diff --git a/src/txmempool.h b/src/txmempool.h index 512e70f8fa..d6f8e7094b 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -206,18 +206,14 @@ class CompareTxMemPoolEntryByDescendantScore public: bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const { - bool fUseADescendants = UseDescendantScore(a); - bool fUseBDescendants = UseDescendantScore(b); + double a_mod_fee, a_size, b_mod_fee, b_size; - double aModFee = fUseADescendants ? a.GetModFeesWithDescendants() : a.GetModifiedFee(); - double aSize = fUseADescendants ? a.GetSizeWithDescendants() : a.GetTxSize(); - - double bModFee = fUseBDescendants ? b.GetModFeesWithDescendants() : b.GetModifiedFee(); - double bSize = fUseBDescendants ? b.GetSizeWithDescendants() : b.GetTxSize(); + GetModFeeAndSize(a, a_mod_fee, a_size); + GetModFeeAndSize(b, b_mod_fee, b_size); // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). - double f1 = aModFee * bSize; - double f2 = aSize * bModFee; + double f1 = a_mod_fee * b_size; + double f2 = a_size * b_mod_fee; if (f1 == f2) { return a.GetTime() >= b.GetTime(); @@ -225,12 +221,21 @@ public: return f1 < f2; } - // Calculate which score to use for an entry (avoiding division). - bool UseDescendantScore(const CTxMemPoolEntry &a) const + // Return the fee/size we're using for sorting this entry. + void GetModFeeAndSize(const CTxMemPoolEntry &a, double &mod_fee, double &size) const { + // Compare feerate with descendants to feerate of the transaction, and + // return the fee/size for the max. double f1 = (double)a.GetModifiedFee() * a.GetSizeWithDescendants(); double f2 = (double)a.GetModFeesWithDescendants() * a.GetTxSize(); - return f2 > f1; + + if (f2 > f1) { + mod_fee = a.GetModFeesWithDescendants(); + size = a.GetSizeWithDescendants(); + } else { + mod_fee = a.GetModifiedFee(); + size = a.GetTxSize(); + } } }; @@ -261,33 +266,53 @@ public: } }; +/** \class CompareTxMemPoolEntryByAncestorScore + * + * Sort an entry by min(score/size of entry's tx, score/size with all ancestors). + */ class CompareTxMemPoolEntryByAncestorFee { public: - bool operator()(const CTxMemPoolEntry& a, const CTxMemPoolEntry& b) const + template<typename T> + bool operator()(const T& a, const T& b) const { - double aFees = a.GetModFeesWithAncestors(); - double aSize = a.GetSizeWithAncestors(); + double a_mod_fee, a_size, b_mod_fee, b_size; - double bFees = b.GetModFeesWithAncestors(); - double bSize = b.GetSizeWithAncestors(); + GetModFeeAndSize(a, a_mod_fee, a_size); + GetModFeeAndSize(b, b_mod_fee, b_size); // Avoid division by rewriting (a/b > c/d) as (a*d > c*b). - double f1 = aFees * bSize; - double f2 = aSize * bFees; + double f1 = a_mod_fee * b_size; + double f2 = a_size * b_mod_fee; if (f1 == f2) { return a.GetTx().GetHash() < b.GetTx().GetHash(); } - return f1 > f2; } + + // Return the fee/size we're using for sorting this entry. + template <typename T> + void GetModFeeAndSize(const T &a, double &mod_fee, double &size) const + { + // Compare feerate with ancestors to feerate of the transaction, and + // return the fee/size for the min. + double f1 = (double)a.GetModifiedFee() * a.GetSizeWithAncestors(); + double f2 = (double)a.GetModFeesWithAncestors() * a.GetTxSize(); + + if (f1 > f2) { + mod_fee = a.GetModFeesWithAncestors(); + size = a.GetSizeWithAncestors(); + } else { + mod_fee = a.GetModifiedFee(); + size = a.GetTxSize(); + } + } }; // Multi_index tag names struct descendant_score {}; struct entry_time {}; -struct mining_score {}; struct ancestor_score {}; class CBlockPolicyEstimator; @@ -356,7 +381,6 @@ public: * - transaction hash * - feerate [we use max(feerate of tx, feerate of tx with all descendants)] * - time in mempool - * - mining score (feerate modified by any fee deltas from PrioritiseTransaction) * * Note: the term "descendant" refers to in-mempool transactions that depend on * this one, while "ancestor" refers to in-mempool transactions that a given @@ -446,12 +470,6 @@ public: boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime >, - // sorted by score (for mining prioritization) - boost::multi_index::ordered_unique< - boost::multi_index::tag<mining_score>, - boost::multi_index::identity<CTxMemPoolEntry>, - CompareTxMemPoolEntryByScore - >, // sorted by fee rate with ancestors boost::multi_index::ordered_non_unique< boost::multi_index::tag<ancestor_score>, diff --git a/src/util.cpp b/src/util.cpp index 150bc503df..80eed24ffd 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -72,6 +72,7 @@ #include <boost/algorithm/string/case_conv.hpp> // for to_lower() #include <boost/algorithm/string/predicate.hpp> // for startswith() and endswith() +#include <boost/interprocess/sync/file_lock.hpp> #include <boost/program_options/detail/config_file.hpp> #include <boost/thread.hpp> #include <openssl/crypto.h> @@ -375,6 +376,27 @@ int LogPrintStr(const std::string &str) return ret; } +bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only) +{ + fs::path pathLockFile = directory / lockfile_name; + FILE* file = fsbridge::fopen(pathLockFile, "a"); // empty lock file; created if it doesn't exist. + if (file) fclose(file); + + try { + static std::map<std::string, boost::interprocess::file_lock> locks; + boost::interprocess::file_lock& lock = locks.emplace(pathLockFile.string(), pathLockFile.string().c_str()).first->second; + if (!lock.try_lock()) { + return false; + } + if (probe_only) { + lock.unlock(); + } + } catch (const boost::interprocess::interprocess_exception& e) { + return error("Error while attempting to lock directory %s: %s", directory.string(), e.what()); + } + 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 6a0d6a31e7..277b4c66af 100644 --- a/src/util.h +++ b/src/util.h @@ -173,6 +173,7 @@ bool TruncateFile(FILE *file, unsigned int length); 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 TryCreateDirectories(const fs::path& p); fs::path GetDefaultDataDir(); const fs::path &GetDataDir(bool fNetSpecific = true); diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 35ff0e1eec..23c6279128 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -95,7 +95,7 @@ void CDBEnv::Close() EnvShutdown(); } -bool CDBEnv::Open(const fs::path& pathIn) +bool CDBEnv::Open(const fs::path& pathIn, bool retry) { if (fDbEnvInit) return true; @@ -103,6 +103,11 @@ bool CDBEnv::Open(const fs::path& pathIn) boost::this_thread::interruption_point(); strPath = pathIn.string(); + if (!LockDirectory(pathIn, ".walletlock")) { + LogPrintf("Cannot obtain a lock on wallet directory %s. Another instance of bitcoin may be using it.\n", strPath); + return false; + } + fs::path pathLogDir = pathIn / "database"; TryCreateDirectories(pathLogDir); fs::path pathErrorFile = pathIn / "db.log"; @@ -134,7 +139,24 @@ bool CDBEnv::Open(const fs::path& pathIn) S_IRUSR | S_IWUSR); if (ret != 0) { dbenv->close(0); - return error("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + LogPrintf("CDBEnv::Open: Error %d opening database environment: %s\n", ret, DbEnv::strerror(ret)); + if (retry) { + // try moving the database env out of the way + fs::path pathDatabaseBak = pathIn / strprintf("database.%d.bak", GetTime()); + try { + fs::rename(pathLogDir, pathDatabaseBak); + LogPrintf("Moved old %s to %s. Retrying.\n", pathLogDir.string(), pathDatabaseBak.string()); + } catch (const fs::filesystem_error&) { + // failure is ok (well, not really, but it's not worse than what we started with) + } + // try opening it again one more time + if (!Open(pathIn, false)) { + // if it still fails, it probably means we can't even create the database env + return false; + } + } else { + return false; + } } fDbEnvInit = true; @@ -269,25 +291,11 @@ bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& walle return false; } - if (!bitdb.Open(walletDir)) - { - // try moving the database env out of the way - fs::path pathDatabase = walletDir / "database"; - fs::path pathDatabaseBak = walletDir / strprintf("database.%d.bak", GetTime()); - try { - fs::rename(pathDatabase, pathDatabaseBak); - LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string()); - } catch (const fs::filesystem_error&) { - // failure is ok (well, not really, but it's not worse than what we started with) - } - - // try again - if (!bitdb.Open(walletDir)) { - // if it still fails, it probably means we can't even create the database env - errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); - return false; - } + if (!bitdb.Open(walletDir, true)) { + errorStr = strprintf(_("Error initializing wallet database environment %s!"), walletDir); + return false; } + return true; } diff --git a/src/wallet/db.h b/src/wallet/db.h index c6f317927f..787135e400 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -68,7 +68,7 @@ public: typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair; bool Salvage(const std::string& strFile, bool fAggressive, std::vector<KeyValPair>& vResult); - bool Open(const fs::path& path); + bool Open(const fs::path& path, bool retry = 0); void Close(); void Flush(bool fShutdown); void CheckpointLSN(const std::string& strFile); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e307623fd5..ad0ebcce22 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -144,7 +144,7 @@ UniValue getnewaddress(const JSONRPCRequest& request) "so payments received with the address will be credited to 'account'.\n" "\nArguments:\n" "1. \"account\" (string, optional) DEPRECATED. The account name for the address to be linked to. If not provided, the default account \"\" is used. It can also be set to the empty string \"\" to represent the default account. The account does not need to exist, it will be created if there is no account by the given name.\n" - "2. \"address_type\" (string, optional) The address type to use. Options are \"legacy\", \"p2sh\", and \"bech32\". Default is set by -addresstype.\n" + "2. \"address_type\" (string, optional) The address type to use. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\". Default is set by -addresstype.\n" "\nResult:\n" "\"address\" (string) The new bitcoin address\n" "\nExamples:\n" @@ -242,7 +242,7 @@ UniValue getrawchangeaddress(const JSONRPCRequest& request) "\nReturns a new Bitcoin address, for receiving change.\n" "This is for use with raw transactions, NOT normal use.\n" "\nArguments:\n" - "1. \"address_type\" (string, optional) The address type to use. Options are \"legacy\", \"p2sh\", and \"bech32\". Default is set by -changetype.\n" + "1. \"address_type\" (string, optional) The address type to use. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\". Default is set by -changetype.\n" "\nResult:\n" "\"address\" (string) The address\n" "\nExamples:\n" @@ -1285,7 +1285,8 @@ UniValue addwitnessaddress(const JSONRPCRequest& request) if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) { std::string msg = "addwitnessaddress \"address\" ( p2sh )\n" - "\nAdd a witness address for a script (with pubkey or redeemscript known). Requires a new wallet backup.\n" + "\nDEPRECATED: set the address_type argument of getnewaddress, or option -addresstype=[bech32|p2sh-segwit] instead.\n" + "Add a witness address for a script (with pubkey or redeemscript known). Requires a new wallet backup.\n" "It returns the witness script.\n" "\nArguments:\n" @@ -1299,6 +1300,12 @@ UniValue addwitnessaddress(const JSONRPCRequest& request) throw std::runtime_error(msg); } + if (!IsDeprecatedRPCEnabled("addwitnessaddress")) { + throw JSONRPCError(RPC_METHOD_DEPRECATED, "addwitnessaddress is deprecated and will be fully removed in v0.17. " + "To use addwitnessaddress in v0.16, restart bitcoind with -deprecatedrpc=addwitnessaddress.\n" + "Projects should transition to using the address_type argument of getnewaddress, or option -addresstype=[bech32|p2sh-segwit] instead.\n"); + } + { LOCK(cs_main); if (!IsWitnessEnabled(chainActive.Tip(), Params().GetConsensus()) && !gArgs.GetBoolArg("-walletprematurewitness", false)) { @@ -2283,7 +2290,8 @@ UniValue walletpassphrase(const JSONRPCRequest& request) "This is needed prior to performing transactions related to private keys such as sending bitcoins\n" "\nArguments:\n" "1. \"passphrase\" (string, required) The wallet passphrase\n" - "2. timeout (numeric, required) The time to keep the decryption key in seconds.\n" + "2. timeout (numeric, required) The time to keep the decryption key in seconds. Limited to at most 1073741824 (2^30) seconds.\n" + " Any value greater than 1073741824 seconds will be set to 1073741824 seconds.\n" "\nNote:\n" "Issuing the walletpassphrase command while the wallet is already unlocked will set a new unlock\n" "time that overrides the old one.\n" @@ -2312,6 +2320,17 @@ UniValue walletpassphrase(const JSONRPCRequest& request) // Alternately, find a way to make request.params[0] mlock()'d to begin with. strWalletPass = request.params[0].get_str().c_str(); + // Get the timeout + int64_t nSleepTime = request.params[1].get_int64(); + // Timeout cannot be negative, otherwise it will relock immediately + if (nSleepTime < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Timeout cannot be negative."); + } + // Clamp timeout to 2^30 seconds + if (nSleepTime > (int64_t)1 << 30) { + nSleepTime = (int64_t)1 << 30; + } + if (strWalletPass.length() > 0) { if (!pwallet->Unlock(strWalletPass)) { @@ -2325,7 +2344,6 @@ UniValue walletpassphrase(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); - int64_t nSleepTime = request.params[1].get_int64(); pwallet->nRelockTime = GetTime() + nSleepTime; RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), boost::bind(LockWallet, pwallet), nSleepTime); @@ -3468,7 +3486,7 @@ static const CRPCCommand commands[] = { "wallet", "abandontransaction", &abandontransaction, {"txid"} }, { "wallet", "abortrescan", &abortrescan, {} }, { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","account"} }, - { "wallet", "addwitnessaddress", &addwitnessaddress, {"address","p2sh"} }, + { "hidden", "addwitnessaddress", &addwitnessaddress, {"address","p2sh"} }, { "wallet", "backupwallet", &backupwallet, {"destination"} }, { "wallet", "bumpfee", &bumpfee, {"txid", "options"} }, { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 53a2c6b9d5..8acbade65c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -99,7 +99,7 @@ enum WalletFeature FEATURE_LATEST = FEATURE_COMPRPUBKEY // HD is optional, use FEATURE_COMPRPUBKEY as latest version }; -enum OutputType +enum OutputType : int { OUTPUT_TYPE_NONE, OUTPUT_TYPE_LEGACY, diff --git a/test/functional/README.md b/test/functional/README.md index 4d52751b05..d6ce490ab3 100644 --- a/test/functional/README.md +++ b/test/functional/README.md @@ -27,6 +27,20 @@ don't have test cases for. `set_test_params()`, `add_options()` and `setup_xxxx()` methods at the top of the subclass, then locally-defined helper methods, then the `run_test()` method. +#### Naming guidelines + +- Name the test `<area>_test.py`, where area can be one of the following: + - `feature` for tests for full features that aren't wallet/mining/mempool, eg `feature_rbf.py` + - `interface` for tests for other interfaces (REST, ZMQ, etc), eg `interface_rest.py` + - `mempool` for tests for mempool behaviour, eg `mempool_reorg.py` + - `mining` for tests for mining features, eg `mining_prioritisetransaction.py` + - `p2p` for tests that explicitly test the p2p interface, eg `p2p_disconnect_ban.py` + - `rpc` for tests for individual RPC methods or features, eg `rpc_listtransactions.py` + - `wallet` for tests for wallet features, eg `wallet_keypool.py` +- use an underscore to separate words + - exception: for tests for specific RPCs or command line options which don't include underscores, name the test after the exact RPC or argument name, eg `rpc_decodescript.py`, not `rpc_decode_script.py` +- Don't use the redundant word `test` in the name, eg `interface_zmq.py`, not `interface_zmq_test.py` + #### General test-writing advice - Set `self.num_nodes` to the minimum number of nodes necessary for the test. @@ -73,7 +87,7 @@ start the networking thread. (Continue with the test logic in your existing thread.) - Can be used to write tests where specific P2P protocol behavior is tested. -Examples tests are `p2p-accept-block.py`, `p2p-compactblocks.py`. +Examples tests are `p2p-acceptblock.py`, `p2p-compactblocks.py`. #### Comptool diff --git a/test/functional/bumpfee.py b/test/functional/bumpfee.py index 5cbd9f5cf7..2cd4127854 100755 --- a/test/functional/bumpfee.py +++ b/test/functional/bumpfee.py @@ -14,7 +14,7 @@ added in the future, they should try to follow the same convention and not make assumptions about execution order. """ -from segwit import send_to_witness +from test_framework.blocktools import send_to_witness from test_framework.test_framework import BitcoinTestFramework from test_framework import blocktools from test_framework.mininode import CTransaction @@ -33,7 +33,7 @@ class BumpFeeTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.setup_clean_chain = True - self.extra_args = [["-prematurewitness", "-walletprematurewitness", "-walletrbf={}".format(i)] + self.extra_args = [["-prematurewitness", "-walletprematurewitness", "-deprecatedrpc=addwitnessaddress", "-walletrbf={}".format(i)] for i in range(self.num_nodes)] def run_test(self): diff --git a/test/functional/create_cache.py b/test/functional/create_cache.py index 4c79814a26..9665c50a92 100755 --- a/test/functional/create_cache.py +++ b/test/functional/create_cache.py @@ -16,6 +16,7 @@ class CreateCache(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 0 + self.supports_cli = True def setup_network(self): pass diff --git a/test/functional/multiwallet.py b/test/functional/multiwallet.py index d0c40e5446..0891829127 100755 --- a/test/functional/multiwallet.py +++ b/test/functional/multiwallet.py @@ -15,63 +15,71 @@ from test_framework.util import assert_equal, assert_raises_rpc_error class MultiWalletTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True - self.num_nodes = 1 - self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w']] + self.num_nodes = 2 + self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w'], []] + self.supports_cli = True def run_test(self): - assert_equal(set(self.nodes[0].listwallets()), {"w1", "w2", "w3", "w"}) + node = self.nodes[0] - self.stop_node(0) + data_dir = lambda *p: os.path.join(node.datadir, 'regtest', *p) + wallet_dir = lambda *p: data_dir('wallets', *p) + wallet = lambda name: node.get_wallet_rpc(name) + + assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"}) + + self.stop_nodes() # 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.') # should not initialize if wallet file is a directory - wallet_dir = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'wallets') - os.mkdir(os.path.join(wallet_dir, 'w11')) + os.mkdir(wallet_dir('w11')) self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.') # should not initialize if one wallet is a copy of another - shutil.copyfile(os.path.join(wallet_dir, 'w2'), os.path.join(wallet_dir, 'w22')) + shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid') # should not initialize if wallet file is a symlink - os.symlink(os.path.join(wallet_dir, 'w1'), os.path.join(wallet_dir, 'w12')) + os.symlink(wallet_dir('w1'), wallet_dir('w12')) self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.') # 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') # should not initialize if the specified walletdir is not a directory - not_a_dir = os.path.join(wallet_dir, 'notadir') + 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.assert_start_raises_init_error(0, ['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory') # if wallets/ doesn't exist, datadir should be the default wallet dir - wallet_dir2 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'walletdir') - os.rename(wallet_dir, wallet_dir2) + wallet_dir2 = data_dir('walletdir') + os.rename(wallet_dir(), wallet_dir2) self.start_node(0, ['-wallet=w4', '-wallet=w5']) - assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"}) - w5 = self.nodes[0].get_wallet_rpc("w5") + assert_equal(set(node.listwallets()), {"w4", "w5"}) + w5 = wallet("w5") w5.generate(1) - self.stop_node(0) # now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded - os.rename(wallet_dir2, wallet_dir) - self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + os.path.join(self.options.tmpdir, 'node0', 'regtest')]) - assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"}) - w5 = self.nodes[0].get_wallet_rpc("w5") + os.rename(wallet_dir2, wallet_dir()) + self.restart_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + data_dir()]) + assert_equal(set(node.listwallets()), {"w4", "w5"}) + w5 = wallet("w5") w5_info = w5.getwalletinfo() assert_equal(w5_info['immature_balance'], 50) - self.stop_node(0) + 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.start_node(0, self.extra_args[0]) + self.restart_node(0, self.extra_args[0]) - w1 = self.nodes[0].get_wallet_rpc("w1") - w2 = self.nodes[0].get_wallet_rpc("w2") - w3 = self.nodes[0].get_wallet_rpc("w3") - w4 = self.nodes[0].get_wallet_rpc("w") - wallet_bad = self.nodes[0].get_wallet_rpc("bad") + w1 = wallet("w1") + w2 = wallet("w2") + w3 = wallet("w3") + w4 = wallet("w") + wallet_bad = wallet("bad") w1.generate(1) @@ -79,7 +87,7 @@ class MultiWalletTest(BitcoinTestFramework): assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo) # accessing wallet RPC without using wallet endpoint fails - assert_raises_rpc_error(-19, "Wallet file not specified", self.nodes[0].getwalletinfo) + assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo) # check w1 wallet balance w1_info = w1.getwalletinfo() diff --git a/test/functional/nulldummy.py b/test/functional/nulldummy.py index 110b7a655e..cbdcfdcf5f 100755 --- a/test/functional/nulldummy.py +++ b/test/functional/nulldummy.py @@ -42,7 +42,7 @@ class NULLDUMMYTest(BitcoinTestFramework): self.setup_clean_chain = True # This script tests NULLDUMMY activation, which is part of the 'segwit' deployment, so we go through # normal segwit activation here (and don't use the default always-on behaviour). - self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness', '-vbparams=segwit:0:999999999999', '-addresstype=legacy']] + self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness', '-vbparams=segwit:0:999999999999', '-addresstype=legacy', "-deprecatedrpc=addwitnessaddress"]] def run_test(self): self.address = self.nodes[0].getnewaddress() diff --git a/test/functional/p2p-compactblocks.py b/test/functional/p2p-compactblocks.py index e98ae31a89..d9f461a049 100755 --- a/test/functional/p2p-compactblocks.py +++ b/test/functional/p2p-compactblocks.py @@ -95,7 +95,7 @@ class CompactBlocksTest(BitcoinTestFramework): self.num_nodes = 2 # This test was written assuming SegWit is activated using BIP9 at height 432 (3x confirmation window). # TODO: Rewrite this test to support SegWit being always active. - self.extra_args = [["-vbparams=segwit:0:0"], ["-vbparams=segwit:0:999999999999", "-txindex"]] + self.extra_args = [["-vbparams=segwit:0:0"], ["-vbparams=segwit:0:999999999999", "-txindex", "-deprecatedrpc=addwitnessaddress"]] self.utxos = [] def build_block_on_tip(self, node, segwit=False): diff --git a/test/functional/segwit.py b/test/functional/segwit.py index 5a4a7468e5..67cca5f1e7 100755 --- a/test/functional/segwit.py +++ b/test/functional/segwit.py @@ -4,10 +4,18 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the SegWit changeover logic.""" +from test_framework.address import ( + key_to_p2sh_p2wpkh, + key_to_p2wpkh, + program_to_witness, + script_to_p2sh_p2wsh, + script_to_p2wsh, +) +from test_framework.blocktools import witness_script, send_to_witness from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * from test_framework.mininode import sha256, CTransaction, CTxIn, COutPoint, CTxOut, COIN, ToHex, FromHex -from test_framework.address import script_to_p2sh, key_to_p2pkh, key_to_p2sh_p2wpkh, key_to_p2wpkh, script_to_p2sh_p2wsh, script_to_p2wsh, program_to_witness +from test_framework.address import script_to_p2sh, key_to_p2pkh from test_framework.script import CScript, OP_HASH160, OP_CHECKSIG, OP_0, hash160, OP_EQUAL, OP_DUP, OP_EQUALVERIFY, OP_1, OP_2, OP_CHECKMULTISIG, OP_TRUE from io import BytesIO @@ -16,52 +24,6 @@ NODE_2 = 2 WIT_V0 = 0 WIT_V1 = 1 -# Create a scriptPubKey corresponding to either a P2WPKH output for the -# given pubkey, or a P2WSH output of a 1-of-1 multisig for the given -# pubkey. Returns the hex encoding of the scriptPubKey. -def witness_script(use_p2wsh, pubkey): - if (use_p2wsh == False): - # P2WPKH instead - pubkeyhash = hash160(hex_str_to_bytes(pubkey)) - pkscript = CScript([OP_0, pubkeyhash]) - else: - # 1-of-1 multisig - witness_program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG]) - scripthash = sha256(witness_program) - pkscript = CScript([OP_0, scripthash]) - return bytes_to_hex_str(pkscript) - -# Return a transaction (in hex) that spends the given utxo to a segwit output, -# optionally wrapping the segwit output using P2SH. -def create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount): - if use_p2wsh: - program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG]) - addr = script_to_p2sh_p2wsh(program) if encode_p2sh else script_to_p2wsh(program) - else: - addr = key_to_p2sh_p2wpkh(pubkey) if encode_p2sh else key_to_p2wpkh(pubkey) - if not encode_p2sh: - assert_equal(node.validateaddress(addr)['scriptPubKey'], witness_script(use_p2wsh, pubkey)) - return node.createrawtransaction([utxo], {addr: amount}) - -# Create a transaction spending a given utxo to a segwit output corresponding -# to the given pubkey: use_p2wsh determines whether to use P2WPKH or P2WSH; -# encode_p2sh determines whether to wrap in P2SH. -# sign=True will have the given node sign the transaction. -# insert_redeem_script will be added to the scriptSig, if given. -def send_to_witness(use_p2wsh, node, utxo, pubkey, encode_p2sh, amount, sign=True, insert_redeem_script=""): - tx_to_witness = create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount) - if (sign): - signed = node.signrawtransaction(tx_to_witness) - assert("errors" not in signed or len(["errors"]) == 0) - return node.sendrawtransaction(signed["hex"]) - else: - if (insert_redeem_script): - tx = FromHex(CTransaction(), tx_to_witness) - tx.vin[0].scriptSig += CScript([hex_str_to_bytes(insert_redeem_script)]) - tx_to_witness = ToHex(tx) - - return node.sendrawtransaction(tx_to_witness) - def getutxo(txid): utxo = {} utxo["vout"] = 0 @@ -78,9 +40,9 @@ class SegWitTest(BitcoinTestFramework): self.setup_clean_chain = True self.num_nodes = 3 # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation. - self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy"], - ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy"], - ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-vbparams=segwit:0:999999999999", "-addresstype=legacy"]] + self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"], + ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"], + ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]] def setup_network(self): super().setup_network() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 93af0037e9..642ef98a27 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -4,8 +4,24 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Utilities for manipulating blocks and transactions.""" +from .address import ( + key_to_p2sh_p2wpkh, + key_to_p2wpkh, + script_to_p2sh_p2wsh, + script_to_p2wsh, +) from .mininode import * -from .script import CScript, OP_TRUE, OP_CHECKSIG, OP_RETURN +from .script import ( + CScript, + OP_0, + OP_1, + OP_CHECKMULTISIG, + OP_CHECKSIG, + OP_RETURN, + OP_TRUE, + hash160, +) +from .util import assert_equal # Create a block (with regtest difficulty) def create_block(hashprev, coinbase, nTime=None): @@ -108,3 +124,49 @@ def get_legacy_sigopcount_tx(tx, fAccurate=True): # scriptSig might be of type bytes, so convert to CScript for the moment count += CScript(j.scriptSig).GetSigOpCount(fAccurate) return count + +# Create a scriptPubKey corresponding to either a P2WPKH output for the +# given pubkey, or a P2WSH output of a 1-of-1 multisig for the given +# pubkey. Returns the hex encoding of the scriptPubKey. +def witness_script(use_p2wsh, pubkey): + if (use_p2wsh == False): + # P2WPKH instead + pubkeyhash = hash160(hex_str_to_bytes(pubkey)) + pkscript = CScript([OP_0, pubkeyhash]) + else: + # 1-of-1 multisig + witness_program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG]) + scripthash = sha256(witness_program) + pkscript = CScript([OP_0, scripthash]) + return bytes_to_hex_str(pkscript) + +# Return a transaction (in hex) that spends the given utxo to a segwit output, +# optionally wrapping the segwit output using P2SH. +def create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount): + if use_p2wsh: + program = CScript([OP_1, hex_str_to_bytes(pubkey), OP_1, OP_CHECKMULTISIG]) + addr = script_to_p2sh_p2wsh(program) if encode_p2sh else script_to_p2wsh(program) + else: + addr = key_to_p2sh_p2wpkh(pubkey) if encode_p2sh else key_to_p2wpkh(pubkey) + if not encode_p2sh: + assert_equal(node.validateaddress(addr)['scriptPubKey'], witness_script(use_p2wsh, pubkey)) + return node.createrawtransaction([utxo], {addr: amount}) + +# Create a transaction spending a given utxo to a segwit output corresponding +# to the given pubkey: use_p2wsh determines whether to use P2WPKH or P2WSH; +# encode_p2sh determines whether to wrap in P2SH. +# sign=True will have the given node sign the transaction. +# insert_redeem_script will be added to the scriptSig, if given. +def send_to_witness(use_p2wsh, node, utxo, pubkey, encode_p2sh, amount, sign=True, insert_redeem_script=""): + tx_to_witness = create_witness_tx(node, use_p2wsh, utxo, pubkey, encode_p2sh, amount) + if (sign): + signed = node.signrawtransaction(tx_to_witness) + assert("errors" not in signed or len(["errors"]) == 0) + return node.sendrawtransaction(signed["hex"]) + else: + if (insert_redeem_script): + tx = FromHex(CTransaction(), tx_to_witness) + tx.vin[0].scriptSig += CScript([hex_str_to_bytes(insert_redeem_script)]) + tx_to_witness = ToHex(tx) + + return node.sendrawtransaction(tx_to_witness) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index e42f3e60c2..5d4f8e6720 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -62,6 +62,7 @@ class BitcoinTestFramework(): self.setup_clean_chain = False self.nodes = [] self.mocktime = 0 + self.supports_cli = False self.set_test_params() assert hasattr(self, "num_nodes"), "Test must set self.num_nodes in set_test_params()" @@ -91,6 +92,8 @@ class BitcoinTestFramework(): help="Location of the test framework config file") parser.add_option("--pdbonfailure", dest="pdbonfailure", default=False, action="store_true", help="Attach a python debugger if test fails") + parser.add_option("--usecli", dest="usecli", default=False, action="store_true", + help="use bitcoin-cli instead of RPC for all commands") self.add_options(parser) (self.options, self.args) = parser.parse_args() @@ -113,6 +116,8 @@ class BitcoinTestFramework(): success = TestStatus.FAILED try: + if self.options.usecli and not self.supports_cli: + raise SkipTest("--usecli specified but test does not support using CLI") self.setup_chain() self.setup_network() self.run_test() @@ -213,7 +218,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, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir)) + self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, use_cli=self.options.usecli)) def start_node(self, i, extra_args=None, stderr=None): """Start a bitcoind""" diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index a9248c764e..589a8f3969 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -10,6 +10,7 @@ import http.client import json import logging import os +import re import subprocess import time @@ -22,6 +23,9 @@ from .util import ( p2p_port, ) +# For Python 3.4 compatibility +JSONDecodeError = getattr(json, "JSONDecodeError", ValueError) + BITCOIND_PROC_WAIT_TIMEOUT = 60 class TestNode(): @@ -38,7 +42,7 @@ 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, extra_args, rpchost, timewait, binary, stderr, mocktime, coverage_dir): + def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mocktime, coverage_dir, use_cli=False): self.index = i self.datadir = os.path.join(dirname, "node" + str(i)) self.rpchost = rpchost @@ -58,6 +62,7 @@ class TestNode(): self.args = [self.binary, "-datadir=" + self.datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(mocktime), "-uacomment=testnode%d" % i] self.cli = TestNodeCLI(os.getenv("BITCOINCLI", "bitcoin-cli"), self.datadir) + self.use_cli = use_cli self.running = False self.process = None @@ -69,9 +74,12 @@ class TestNode(): self.p2ps = [] def __getattr__(self, name): - """Dispatches any unrecognised messages to the RPC connection.""" - assert self.rpc_connected and self.rpc is not None, "Error: no RPC connection" - return getattr(self.rpc, name) + """Dispatches any unrecognised messages to the RPC connection or a CLI instance.""" + if self.use_cli: + return getattr(self.cli, name) + else: + assert self.rpc_connected and self.rpc is not None, "Error: no RPC connection" + return getattr(self.rpc, name) def start(self, extra_args=None, stderr=None): """Start the node.""" @@ -110,10 +118,13 @@ class TestNode(): raise AssertionError("Unable to connect to bitcoind") def get_wallet_rpc(self, wallet_name): - assert self.rpc_connected - assert self.rpc - wallet_path = "wallet/%s" % wallet_name - return self.rpc / wallet_path + if self.use_cli: + return self.cli("-rpcwallet={}".format(wallet_name)) + else: + assert self.rpc_connected + assert self.rpc + wallet_path = "wallet/%s" % wallet_name + return self.rpc / wallet_path def stop_node(self): """Stop the node.""" @@ -187,6 +198,16 @@ class TestNode(): p.peer_disconnect() del self.p2ps[:] +class TestNodeCLIAttr: + def __init__(self, cli, command): + self.cli = cli + self.command = command + + def __call__(self, *args, **kwargs): + return self.cli.send_cli(self.command, *args, **kwargs) + + def get_request(self, *args, **kwargs): + return lambda: self(*args, **kwargs) class TestNodeCLI(): """Interface to bitcoin-cli for an individual node""" @@ -196,17 +217,26 @@ class TestNodeCLI(): self.binary = binary self.datadir = datadir self.input = None + self.log = logging.getLogger('TestFramework.bitcoincli') def __call__(self, *args, input=None): # TestNodeCLI is callable with bitcoin-cli command-line args - self.args = [str(arg) for arg in args] - self.input = input - return self + cli = TestNodeCLI(self.binary, self.datadir) + cli.args = [str(arg) for arg in args] + cli.input = input + return cli def __getattr__(self, command): - def dispatcher(*args, **kwargs): - return self.send_cli(command, *args, **kwargs) - return dispatcher + return TestNodeCLIAttr(self, command) + + def batch(self, requests): + results = [] + for request in requests: + try: + results.append(dict(result=request())) + except JSONRPCException as e: + results.append(dict(error=e)) + return results def send_cli(self, command, *args, **kwargs): """Run bitcoin-cli command. Deserializes returned string as python object.""" @@ -218,10 +248,18 @@ class TestNodeCLI(): if named_args: p_args += ["-named"] p_args += [command] + pos_args + named_args + self.log.debug("Running bitcoin-cli command: %s" % command) process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) cli_stdout, cli_stderr = process.communicate(input=self.input) returncode = process.poll() if returncode: + match = re.match(r'error code: ([-0-9]+)\nerror message:\n(.*)', cli_stderr) + if match: + code, message = match.groups() + raise JSONRPCException(dict(code=int(code), message=message)) # Ignore cli_stdout, raise with cli_stderr raise subprocess.CalledProcessError(returncode, self.binary, output=cli_stderr) - return json.loads(cli_stdout, parse_float=decimal.Decimal) + try: + return json.loads(cli_stdout, parse_float=decimal.Decimal) + except JSONDecodeError: + return cli_stdout.rstrip("\n") diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 664dc6d0af..7fdc171332 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -390,7 +390,7 @@ def sync_chain(rpc_connections, *, wait=1, timeout=60): timeout -= wait raise AssertionError("Chain sync failed: Best block hashes don't match") -def sync_mempools(rpc_connections, *, wait=1, timeout=60): +def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True): """ Wait until everybody has the same transactions in their memory pools @@ -402,6 +402,9 @@ def sync_mempools(rpc_connections, *, wait=1, timeout=60): if set(rpc_connections[i].getrawmempool()) == pool: num_match = num_match + 1 if num_match == len(rpc_connections): + if flush_scheduler: + for r in rpc_connections: + r.syncwithvalidationinterfacequeue() return time.sleep(wait) timeout -= wait diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2d5ea84814..72ad300e7e 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -94,6 +94,7 @@ BASE_SCRIPTS= [ 'mempool_reorg.py', 'mempool_persist.py', 'multiwallet.py', + 'multiwallet.py --usecli', 'httpbasics.py', 'multi_rpc.py', 'proxy_test.py', @@ -271,6 +272,7 @@ def main(): sys.exit(0) check_script_list(config["environment"]["SRCDIR"]) + check_script_prefixes() if not args.keepcache: shutil.rmtree("%s/test/cache" % config["environment"]["BUILDDIR"], ignore_errors=True) @@ -469,6 +471,28 @@ class TestResult(): return self.status != "Failed" +def check_script_prefixes(): + """Check that no more than `EXPECTED_VIOLATION_COUNT` of the + test scripts don't start with one of the allowed name prefixes.""" + EXPECTED_VIOLATION_COUNT = 77 + + # LEEWAY is provided as a transition measure, so that pull-requests + # that introduce new tests that don't conform with the naming + # convention don't immediately cause the tests to fail. + LEEWAY = 10 + + good_prefixes_re = re.compile("(example|feature|interface|mempool|mining|p2p|rpc|wallet)_") + bad_script_names = [script for script in ALL_SCRIPTS if good_prefixes_re.match(script) is None] + + if len(bad_script_names) < EXPECTED_VIOLATION_COUNT: + print("{}HURRAY!{} Number of functional tests violating naming convention reduced!".format(BOLD[1], BOLD[0])) + print("Consider reducing EXPECTED_VIOLATION_COUNT from %d to %d" % (EXPECTED_VIOLATION_COUNT, len(bad_script_names))) + elif len(bad_script_names) > EXPECTED_VIOLATION_COUNT: + print("INFO: %d tests not meeting naming conventions (expected %d):" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT)) + print(" %s" % ("\n ".join(sorted(bad_script_names)))) + assert len(bad_script_names) <= EXPECTED_VIOLATION_COUNT + LEEWAY, "Too many tests not following naming convention! (%d found, expected: <= %d)" % (len(bad_script_names), EXPECTED_VIOLATION_COUNT) + + def check_script_list(src_dir): """Check scripts directory. diff --git a/test/functional/wallet-dump.py b/test/functional/wallet-dump.py index 8b81c81ba8..a1a4f25921 100755 --- a/test/functional/wallet-dump.py +++ b/test/functional/wallet-dump.py @@ -69,7 +69,7 @@ def read_dump(file_name, addrs, script_addrs, hd_master_addr_old): class WalletDumpTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [["-keypool=90", "-addresstype=legacy"]] + self.extra_args = [["-keypool=90", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]] def setup_network(self, split=False): # Use 1 minute timeout because the initial getnewaddress RPC can take diff --git a/test/functional/wallet-encryption.py b/test/functional/wallet-encryption.py index 452e8ec291..3c927ee484 100755 --- a/test/functional/wallet-encryption.py +++ b/test/functional/wallet-encryption.py @@ -10,6 +10,8 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, + assert_greater_than, + assert_greater_than_or_equal, ) class WalletEncryptionTest(BitcoinTestFramework): @@ -56,6 +58,23 @@ class WalletEncryptionTest(BitcoinTestFramework): assert_raises_rpc_error(-14, "wallet passphrase entered was incorrect", self.nodes[0].walletpassphrase, passphrase, 10) self.nodes[0].walletpassphrase(passphrase2, 10) assert_equal(privkey, self.nodes[0].dumpprivkey(address)) + self.nodes[0].walletlock() + + # Test timeout bounds + assert_raises_rpc_error(-8, "Timeout cannot be negative.", self.nodes[0].walletpassphrase, passphrase2, -10) + # Check the timeout + # Check a time less than the limit + expected_time = int(time.time()) + (1 << 30) - 600 + self.nodes[0].walletpassphrase(passphrase2, (1 << 30) - 600) + actual_time = self.nodes[0].getwalletinfo()['unlocked_until'] + assert_greater_than_or_equal(actual_time, expected_time) + assert_greater_than(expected_time + 5, actual_time) # 5 second buffer + # Check a time greater than the limit + expected_time = int(time.time()) + (1 << 30) - 1 + self.nodes[0].walletpassphrase(passphrase2, (1 << 33)) + actual_time = self.nodes[0].getwalletinfo()['unlocked_until'] + assert_greater_than_or_equal(actual_time, expected_time) + assert_greater_than(expected_time + 5, actual_time) # 5 second buffer if __name__ == '__main__': WalletEncryptionTest().main() |