diff options
43 files changed, 1533 insertions, 813 deletions
diff --git a/.travis.yml b/.travis.yml index e5254fe610..6c3a13d73d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,7 @@ cache: - depends/sdk-sources - $HOME/.ccache git: - depth: false # full clone for git subtree check, this works around issue #12388 + depth: 1 env: global: - MAKEJOBS=-j3 @@ -48,6 +48,7 @@ install: - if [ -n "$PACKAGES" ]; then travis_retry sudo apt-get install --no-install-recommends --no-upgrade -qq $PACKAGES; fi - if [ "$CHECK_DOC" = 1 -a "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then travis_retry pip3 install flake8 --user; fi before_script: + - if [ "$CHECK_DOC" = 1 ]; then git fetch --unshallow; fi - if [ "$CHECK_DOC" = 1 -a "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/commit-script-check.sh $TRAVIS_COMMIT_RANGE; fi - if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/git-subtree-check.sh src/crypto/ctaes; fi - if [ "$CHECK_DOC" = 1 ]; then contrib/devtools/git-subtree-check.sh src/secp256k1; fi diff --git a/configure.ac b/configure.ac index 48f104bdca..18f707f0ba 100644 --- a/configure.ac +++ b/configure.ac @@ -1055,7 +1055,7 @@ if test x$system_univalue != xno ; then m4_ifdef( [PKG_CHECK_MODULES], [ - PKG_CHECK_MODULES([UNIVALUE],[libunivalue],[found_univalue=yes],[true]) + PKG_CHECK_MODULES([UNIVALUE],[libunivalue >= 1.0.4],[found_univalue=yes],[true]) ] ) else diff --git a/contrib/init/bitcoind.service b/contrib/init/bitcoind.service index ee113d7615..877abafd19 100644 --- a/contrib/init/bitcoind.service +++ b/contrib/init/bitcoind.service @@ -19,7 +19,26 @@ User=bitcoin Type=forking PIDFile=/run/bitcoind/bitcoind.pid Restart=on-failure + +# Hardening measures +#################### + +# Provide a private /tmp and /var/tmp. PrivateTmp=true +# Mount /usr, /boot/ and /etc read-only for the process. +ProtectSystem=full + +# Disallow the process and all of its children to gain +# new privileges through execve(). +NoNewPrivileges=true + +# Use a new /dev namespace only populated with API pseudo devices +# such as /dev/null, /dev/zero and /dev/random. +PrivateDevices=true + +# Deny the creation of writable and executable memory mappings. +MemoryDenyWriteExecute=true + [Install] WantedBy=multi-user.target diff --git a/depends/packages/native_biplist.mk b/depends/packages/native_biplist.mk index 3c6e8900f6..5f247e9bf3 100644 --- a/depends/packages/native_biplist.mk +++ b/depends/packages/native_biplist.mk @@ -1,14 +1,9 @@ package=native_biplist -$(package)_version=0.9 -$(package)_download_path=https://pypi.python.org/packages/source/b/biplist +$(package)_version=1.0.3 +$(package)_download_path=https://bitbucket.org/wooster/biplist/downloads $(package)_file_name=biplist-$($(package)_version).tar.gz -$(package)_sha256_hash=b57cadfd26e4754efdf89e9e37de87885f9b5c847b2615688ca04adfaf6ca604 +$(package)_sha256_hash=4c0549764c5fe50b28042ec21aa2e14fe1a2224e239a1dae77d9e7f3932aa4c6 $(package)_install_libdir=$(build_prefix)/lib/python/dist-packages -$(package)_patches=sorted_list.patch - -define $(package)_preprocess_cmds - patch -p1 < $($(package)_patch_dir)/sorted_list.patch -endef define $(package)_build_cmds python setup.py build diff --git a/depends/patches/native_biplist/sorted_list.patch b/depends/patches/native_biplist/sorted_list.patch deleted file mode 100644 index 89abdb1b71..0000000000 --- a/depends/patches/native_biplist/sorted_list.patch +++ /dev/null @@ -1,29 +0,0 @@ ---- a/biplist/__init__.py 2014-10-26 19:03:11.000000000 +0000 -+++ b/biplist/__init__.py 2016-07-19 19:30:17.663521999 +0000 -@@ -541,7 +541,7 @@ - return HashableWrapper(n) - elif isinstance(root, dict): - n = {} -- for key, value in iteritems(root): -+ for key, value in sorted(iteritems(root)): - n[self.wrapRoot(key)] = self.wrapRoot(value) - return HashableWrapper(n) - elif isinstance(root, list): -@@ -616,7 +616,7 @@ - elif isinstance(obj, dict): - size = proc_size(len(obj)) - self.incrementByteCount('dictBytes', incr=1+size) -- for key, value in iteritems(obj): -+ for key, value in sorted(iteritems(obj)): - check_key(key) - self.computeOffsets(key, asReference=True) - self.computeOffsets(value, asReference=True) -@@ -714,7 +714,7 @@ - keys = [] - values = [] - objectsToWrite = [] -- for key, value in iteritems(obj): -+ for key, value in sorted(iteritems(obj)): - keys.append(key) - values.append(value) - for key in keys: diff --git a/doc/build-osx.md b/doc/build-osx.md index 2b84c7cc2c..f19e4f0b8d 100644 --- a/doc/build-osx.md +++ b/doc/build-osx.md @@ -16,7 +16,7 @@ Then install [Homebrew](https://brew.sh). Dependencies ---------------------- - brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf python3 qt libevent + brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf python qt libevent See [dependencies.md](dependencies.md) for a complete overview. diff --git a/doc/build-windows.md b/doc/build-windows.md index cff4ea9362..0a4136173b 100644 --- a/doc/build-windows.md +++ b/doc/build-windows.md @@ -62,6 +62,8 @@ installing the toolchain will be different. First, install the general dependencies: + sudo apt update + sudo apt upgrade sudo apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils curl git A host toolchain (`build-essential`) is necessary because some dependency diff --git a/src/Makefile.am b/src/Makefile.am index 0a9370c85c..7c2fe56d9d 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -172,6 +172,7 @@ BITCOIN_CORE_H = \ wallet/wallet.h \ wallet/walletdb.h \ wallet/walletutil.h \ + wallet/coinselection.h \ warnings.h \ zmq/zmqabstractnotifier.h \ zmq/zmqconfig.h\ @@ -253,6 +254,7 @@ libbitcoin_wallet_a_SOURCES = \ wallet/wallet.cpp \ wallet/walletdb.cpp \ wallet/walletutil.cpp \ + wallet/coinselection.cpp \ $(BITCOIN_CORE_H) # crypto primitives library diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 4ee9102519..4d0819ab79 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -94,7 +94,8 @@ BITCOIN_TESTS += \ wallet/test/wallet_test_fixture.h \ wallet/test/accounting_tests.cpp \ wallet/test/wallet_tests.cpp \ - wallet/test/crypto_tests.cpp + wallet/test/crypto_tests.cpp \ + wallet/test/coinselector_tests.cpp endif test_test_bitcoin_SOURCES = $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES) diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 98965840c7..4b2a0e72fe 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -4,6 +4,7 @@ #include <bench/bench.h> #include <wallet/wallet.h> +#include <wallet/coinselection.h> #include <set> @@ -44,7 +45,11 @@ static void CoinSelection(benchmark::State& state) std::set<CInputCoin> setCoinsRet; CAmount nValueRet; - bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet); + bool bnb_used; + CoinEligibilityFilter filter_standard(1, 6, 0); + CoinSelectionParams coin_selection_params(false, 34, 148, CFeeRate(0), 0); + bool success = wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) + || wallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used); assert(success); assert(nValueRet == 1003 * COIN); assert(setCoinsRet.size() == 2); @@ -57,4 +62,47 @@ static void CoinSelection(benchmark::State& state) } } +typedef std::set<CInputCoin> CoinSet; + +// Copied from src/wallet/test/coinselector_tests.cpp +static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + set.emplace_back(MakeTransactionRef(tx), nInput); +} +// Copied from src/wallet/test/coinselector_tests.cpp +static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) +{ + utxo_pool.clear(); + CAmount target = 0; + for (int i = 0; i < utxos; ++i) { + target += (CAmount)1 << (utxos+i); + add_coin((CAmount)1 << (utxos+i), 2*i, utxo_pool); + add_coin(((CAmount)1 << (utxos+i)) + ((CAmount)1 << (utxos-1-i)), 2*i + 1, utxo_pool); + } + return target; +} + +static void BnBExhaustion(benchmark::State& state) +{ + // Setup + std::vector<CInputCoin> utxo_pool; + CoinSet selection; + CAmount value_ret = 0; + CAmount not_input_fees = 0; + + while (state.KeepRunning()) { + // Benchmark + CAmount target = make_hard_case(17, utxo_pool); + SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees); // Should exhaust + + // Cleanup + utxo_pool.clear(); + selection.clear(); + } +} + BENCHMARK(CoinSelection, 650); +BENCHMARK(BnBExhaustion, 650); diff --git a/src/consensus/validation.h b/src/consensus/validation.h index c2a343c155..757df518ae 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -101,5 +101,10 @@ 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) +{ + // 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); +} #endif // BITCOIN_CONSENSUS_VALIDATION_H diff --git a/src/miner.cpp b/src/miner.cpp index fcb376c6cb..4b86446774 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -282,7 +282,7 @@ bool BlockAssembler::SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_tran return mapModifiedTx.count(it) || inBlock.count(it) || failedTx.count(it); } -void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, CTxMemPool::txiter entry, std::vector<CTxMemPool::txiter>& sortedEntries) +void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries) { // Sort package by ancestor count // If a transaction A depends on transaction B, then A's ancestor count @@ -418,7 +418,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda // Package can be added. Sort the entries in a valid order. std::vector<CTxMemPool::txiter> sortedEntries; - SortForBlock(ancestors, iter, sortedEntries); + SortForBlock(ancestors, sortedEntries); for (size_t i=0; i<sortedEntries.size(); ++i) { AddToBlock(sortedEntries[i]); diff --git a/src/miner.h b/src/miner.h index 9c086332d4..33a22ba75f 100644 --- a/src/miner.h +++ b/src/miner.h @@ -185,7 +185,7 @@ private: * or if the transaction's cached data in mapTx is incorrect. */ bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx); /** Sort the package in an order that is valid to appear in a block */ - void SortForBlock(const CTxMemPool::setEntries& package, CTxMemPool::txiter entry, std::vector<CTxMemPool::txiter>& sortedEntries); + void SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries); /** Add descendants of given transactions to mapModifiedTx with ancestor * state updated assuming given transactions are inBlock. Returns number * of updated descendants. */ @@ -469,6 +469,13 @@ public: virtual bool SendMessages(CNode* pnode, std::atomic<bool>& interrupt) = 0; virtual void InitializeNode(CNode* pnode) = 0; virtual void FinalizeNode(NodeId id, bool& update_connection_time) = 0; + +protected: + /** + * Protected destructor so that instances can only be deleted by derived classes. + * If that restriction is no longer desired, this should be made public and virtual. + */ + ~NetEventsInterface() = default; }; enum diff --git a/src/net_processing.h b/src/net_processing.h index ff1ebc59da..11543129cf 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -35,7 +35,7 @@ static constexpr int64_t EXTRA_PEER_CHECK_INTERVAL = 45; /** Minimum time an outbound-peer-eviction candidate must be connected for, in order to evict, in seconds */ static constexpr int64_t MINIMUM_CONNECT_TIME = 30; -class PeerLogicValidation : public CValidationInterface, public NetEventsInterface { +class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { private: CConnman* const connman; diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index bff58932b4..41f967c985 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -258,3 +258,8 @@ int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost) { return GetVirtualTransactionSize(GetTransactionWeight(tx), nSigOpCost); } + +int64_t GetVirtualTransactionInputSize(const CTxIn& txin, int64_t nSigOpCost) +{ + return GetVirtualTransactionSize(GetTransationInputWeight(txin), nSigOpCost); +} diff --git a/src/policy/policy.h b/src/policy/policy.h index 3d96406bbc..e4eda4b635 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -102,5 +102,6 @@ extern unsigned int nBytesPerSigOp; /** Compute the virtual transaction size (weight reinterpreted as bytes). */ int64_t GetVirtualTransactionSize(int64_t nWeight, int64_t nSigOpCost); int64_t GetVirtualTransactionSize(const CTransaction& tx, int64_t nSigOpCost = 0); +int64_t GetVirtualTransactionInputSize(const CTxIn& tx, int64_t nSigOpCost = 0); #endif // BITCOIN_POLICY_POLICY_H diff --git a/src/qt/addressbookpage.cpp b/src/qt/addressbookpage.cpp index 517aa49e2b..78dc9f81dd 100644 --- a/src/qt/addressbookpage.cpp +++ b/src/qt/addressbookpage.cpp @@ -21,6 +21,41 @@ #include <QMessageBox> #include <QSortFilterProxyModel> +class AddressBookSortFilterProxyModel final : public QSortFilterProxyModel +{ + const QString m_type; + +public: + AddressBookSortFilterProxyModel(const QString& type, QObject* parent) + : QSortFilterProxyModel(parent) + , m_type(type) + { + setDynamicSortFilter(true); + setFilterCaseSensitivity(Qt::CaseInsensitive); + setSortCaseSensitivity(Qt::CaseInsensitive); + } + +protected: + bool filterAcceptsRow(int row, const QModelIndex& parent) const + { + auto model = sourceModel(); + auto label = model->index(row, AddressTableModel::Label, parent); + + if (model->data(label, AddressTableModel::TypeRole).toString() != m_type) { + return false; + } + + auto address = model->index(row, AddressTableModel::Address, parent); + + if (filterRegExp().indexIn(model->data(address).toString()) < 0 && + filterRegExp().indexIn(model->data(label).toString()) < 0) { + return false; + } + + return true; + } +}; + AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode, Tabs _tab, QWidget *parent) : QDialog(parent), ui(new Ui::AddressBookPage), @@ -113,24 +148,12 @@ void AddressBookPage::setModel(AddressTableModel *_model) if(!_model) return; - proxyModel = new QSortFilterProxyModel(this); + auto type = tab == ReceivingTab ? AddressTableModel::Receive : AddressTableModel::Send; + proxyModel = new AddressBookSortFilterProxyModel(type, this); proxyModel->setSourceModel(_model); - proxyModel->setDynamicSortFilter(true); - proxyModel->setSortCaseSensitivity(Qt::CaseInsensitive); - proxyModel->setFilterCaseSensitivity(Qt::CaseInsensitive); - switch(tab) - { - case ReceivingTab: - // Receive filter - proxyModel->setFilterRole(AddressTableModel::TypeRole); - proxyModel->setFilterFixedString(AddressTableModel::Receive); - break; - case SendingTab: - // Send filter - proxyModel->setFilterRole(AddressTableModel::TypeRole); - proxyModel->setFilterFixedString(AddressTableModel::Send); - break; - } + + connect(ui->searchLineEdit, SIGNAL(textChanged(QString)), proxyModel, SLOT(setFilterWildcard(QString))); + ui->tableView->setModel(proxyModel); ui->tableView->sortByColumn(0, Qt::AscendingOrder); diff --git a/src/qt/addressbookpage.h b/src/qt/addressbookpage.h index 54a43478d1..8877d07330 100644 --- a/src/qt/addressbookpage.h +++ b/src/qt/addressbookpage.h @@ -7,6 +7,7 @@ #include <QDialog> +class AddressBookSortFilterProxyModel; class AddressTableModel; class PlatformStyle; @@ -18,7 +19,6 @@ QT_BEGIN_NAMESPACE class QItemSelection; class QMenu; class QModelIndex; -class QSortFilterProxyModel; QT_END_NAMESPACE /** Widget that shows a list of sending or receiving addresses. @@ -53,7 +53,7 @@ private: Mode mode; Tabs tab; QString returnValue; - QSortFilterProxyModel *proxyModel; + AddressBookSortFilterProxyModel *proxyModel; QMenu *contextMenu; QAction *deleteAction; // to be able to explicitly disable it QString newAddressToSelect; diff --git a/src/qt/forms/addressbookpage.ui b/src/qt/forms/addressbookpage.ui index 264edeb720..7ac216286c 100644 --- a/src/qt/forms/addressbookpage.ui +++ b/src/qt/forms/addressbookpage.ui @@ -22,6 +22,13 @@ </widget> </item> <item> + <widget class="QLineEdit" name="searchLineEdit"> + <property name="placeholderText"> + <string>Enter address or label to search</string> + </property> + </widget> + </item> + <item> <widget class="QTableView" name="tableView"> <property name="contextMenuPolicy"> <enum>Qt::CustomContextMenu</enum> diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index a702461f7a..6301af7553 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -31,31 +31,35 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & { QModelIndex index = sourceModel()->index(sourceRow, 0, sourceParent); - int type = index.data(TransactionTableModel::TypeRole).toInt(); - QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime(); - bool involvesWatchAddress = index.data(TransactionTableModel::WatchonlyRole).toBool(); - QString address = index.data(TransactionTableModel::AddressRole).toString(); - QString label = index.data(TransactionTableModel::LabelRole).toString(); - QString txid = index.data(TransactionTableModel::TxHashRole).toString(); - qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); int status = index.data(TransactionTableModel::StatusRole).toInt(); - - if(!showInactive && status == TransactionStatus::Conflicted) + if (!showInactive && status == TransactionStatus::Conflicted) return false; - if(!(TYPE(type) & typeFilter)) + + int type = index.data(TransactionTableModel::TypeRole).toInt(); + if (!(TYPE(type) & typeFilter)) return false; + + bool involvesWatchAddress = index.data(TransactionTableModel::WatchonlyRole).toBool(); if (involvesWatchAddress && watchOnlyFilter == WatchOnlyFilter_No) return false; if (!involvesWatchAddress && watchOnlyFilter == WatchOnlyFilter_Yes) return false; - if(datetime < dateFrom || datetime > dateTo) + + QDateTime datetime = index.data(TransactionTableModel::DateRole).toDateTime(); + if (datetime < dateFrom || datetime > dateTo) return false; + + QString address = index.data(TransactionTableModel::AddressRole).toString(); + QString label = index.data(TransactionTableModel::LabelRole).toString(); + QString txid = index.data(TransactionTableModel::TxHashRole).toString(); if (!address.contains(m_search_string, Qt::CaseInsensitive) && ! label.contains(m_search_string, Qt::CaseInsensitive) && ! txid.contains(m_search_string, Qt::CaseInsensitive)) { return false; } - if(amount < minAmount) + + qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); + if (amount < minAmount) return false; return true; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index e7d9d276d7..39ef20c835 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -275,9 +275,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact int nChangePosRet = -1; std::string strFailReason; - CWalletTx *newTx = transaction.getTransaction(); + CTransactionRef& newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); - bool fCreated = wallet->CreateTransaction(vecSend, *newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); + bool fCreated = wallet->CreateTransaction(vecSend, newTx, *keyChange, nFeeRequired, nChangePosRet, strFailReason, coinControl); transaction.setTransactionFee(nFeeRequired); if (fSubtractFeeFromAmount && fCreated) transaction.reassignAmounts(nChangePosRet); @@ -309,8 +309,8 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran { LOCK2(cs_main, wallet->cs_wallet); - CWalletTx *newTx = transaction.getTransaction(); + std::vector<std::pair<std::string, std::string>> vOrderForm; for (const SendCoinsRecipient &rcp : transaction.getRecipients()) { if (rcp.paymentRequest.IsInitialized()) @@ -321,22 +321,22 @@ WalletModel::SendCoinsReturn WalletModel::sendCoins(WalletModelTransaction &tran } // Store PaymentRequests in wtx.vOrderForm in wallet. - std::string key("PaymentRequest"); std::string value; rcp.paymentRequest.SerializeToString(&value); - newTx->vOrderForm.push_back(make_pair(key, value)); + vOrderForm.emplace_back("PaymentRequest", std::move(value)); } else if (!rcp.message.isEmpty()) // Message from normal bitcoin:URI (bitcoin:123...?message=example) - newTx->vOrderForm.push_back(make_pair("Message", rcp.message.toStdString())); + vOrderForm.emplace_back("Message", rcp.message.toStdString()); } + CTransactionRef& newTx = transaction.getTransaction(); CReserveKey *keyChange = transaction.getPossibleKeyChange(); CValidationState state; - if(!wallet->CommitTransaction(*newTx, *keyChange, g_connman.get(), state)) + if (!wallet->CommitTransaction(newTx, {} /* mapValue */, std::move(vOrderForm), {} /* fromAccount */, *keyChange, g_connman.get(), state)) return SendCoinsReturn(TransactionCommitFailed, QString::fromStdString(state.GetRejectReason())); CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION); - ssTx << *newTx->tx; + ssTx << newTx; transaction_array.append(&(ssTx[0]), ssTx.size()); } diff --git a/src/qt/walletmodeltransaction.cpp b/src/qt/walletmodeltransaction.cpp index 4b2bef2690..4df8a5687e 100644 --- a/src/qt/walletmodeltransaction.cpp +++ b/src/qt/walletmodeltransaction.cpp @@ -12,12 +12,6 @@ WalletModelTransaction::WalletModelTransaction(const QList<SendCoinsRecipient> & walletTransaction(0), fee(0) { - walletTransaction = new CWalletTx(); -} - -WalletModelTransaction::~WalletModelTransaction() -{ - delete walletTransaction; } QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const @@ -25,14 +19,14 @@ QList<SendCoinsRecipient> WalletModelTransaction::getRecipients() const return recipients; } -CWalletTx *WalletModelTransaction::getTransaction() const +CTransactionRef& WalletModelTransaction::getTransaction() { return walletTransaction; } unsigned int WalletModelTransaction::getTransactionSize() { - return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction->tx)); + return (!walletTransaction ? 0 : ::GetVirtualTransactionSize(*walletTransaction)); } CAmount WalletModelTransaction::getTransactionFee() const @@ -62,7 +56,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) if (out.amount() <= 0) continue; if (i == nChangePosRet) i++; - subtotal += walletTransaction->tx->vout[i].nValue; + subtotal += walletTransaction->vout[i].nValue; i++; } rcp.amount = subtotal; @@ -71,7 +65,7 @@ void WalletModelTransaction::reassignAmounts(int nChangePosRet) { if (i == nChangePosRet) i++; - rcp.amount = walletTransaction->tx->vout[i].nValue; + rcp.amount = walletTransaction->vout[i].nValue; i++; } } diff --git a/src/qt/walletmodeltransaction.h b/src/qt/walletmodeltransaction.h index cd531dba4b..931e960d18 100644 --- a/src/qt/walletmodeltransaction.h +++ b/src/qt/walletmodeltransaction.h @@ -20,11 +20,10 @@ class WalletModelTransaction { public: explicit WalletModelTransaction(const QList<SendCoinsRecipient> &recipients); - ~WalletModelTransaction(); QList<SendCoinsRecipient> getRecipients() const; - CWalletTx *getTransaction() const; + CTransactionRef& getTransaction(); unsigned int getTransactionSize(); void setTransactionFee(const CAmount& newFee); @@ -39,7 +38,7 @@ public: private: QList<SendCoinsRecipient> recipients; - CWalletTx *walletTransaction; + CTransactionRef walletTransaction; std::unique_ptr<CReserveKey> keyChange; CAmount fee; }; diff --git a/src/script/sign.cpp b/src/script/sign.cpp index aaba5e5926..baa712dc2d 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -194,11 +194,16 @@ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nI return data; } +void UpdateInput(CTxIn& input, const SignatureData& data) +{ + input.scriptSig = data.scriptSig; + input.scriptWitness = data.scriptWitness; +} + void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data) { assert(tx.vin.size() > nIn); - tx.vin[nIn].scriptSig = data.scriptSig; - tx.vin[nIn].scriptWitness = data.scriptWitness; + UpdateInput(tx.vin[nIn], data); } bool SignSignature(const CKeyStore &keystore, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType) diff --git a/src/script/sign.h b/src/script/sign.h index 97c0014cd0..2c749521cd 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -80,6 +80,7 @@ SignatureData CombineSignatures(const CScript& scriptPubKey, const BaseSignature /** Extract signature data from a transaction, and insert it. */ SignatureData DataFromTransaction(const CMutableTransaction& tx, unsigned int nIn); void UpdateTransaction(CMutableTransaction& tx, unsigned int nIn, const SignatureData& data); +void UpdateInput(CTxIn& input, const SignatureData& data); /* Check whether we know how to sign for an output like this, assuming we * have all private keys. While this function does not need private keys, the passed diff --git a/src/validation.cpp b/src/validation.cpp index f49dc5a155..bee890437e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2780,7 +2780,11 @@ bool CChainState::InvalidateBlock(CValidationState& state, const CChainParams& c } InvalidChainFound(pindex); - uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev); + + // Only notify about a new block tip if the active chain was modified. + if (pindex_was_in_chain) { + uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev); + } return true; } bool InvalidateBlock(CValidationState& state, const CChainParams& chainparams, CBlockIndex *pindex) { diff --git a/src/validationinterface.h b/src/validationinterface.h index 56ea698a2e..63097166af 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -56,6 +56,11 @@ void SyncWithValidationInterfaceQueue(); class CValidationInterface { protected: /** + * Protected destructor so that instances can only be deleted by derived classes. + * If that restriction is no longer desired, this should be made public and virtual. + */ + ~CValidationInterface() = default; + /** * Notifies listeners of updated block chain tip * * Called on a background thread. diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp new file mode 100644 index 0000000000..8596ad2adc --- /dev/null +++ b/src/wallet/coinselection.cpp @@ -0,0 +1,300 @@ +// Copyright (c) 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. + +#include <wallet/coinselection.h> +#include <util.h> +#include <utilmoneystr.h> + +// Descending order comparator +struct { + bool operator()(const CInputCoin& a, const CInputCoin& b) const + { + return a.effective_value > b.effective_value; + } +} descending; + +/* + * This is the Branch and Bound Coin Selection algorithm designed by Murch. It searches for an input + * set that can pay for the spending target and does not exceed the spending target by more than the + * cost of creating and spending a change output. The algorithm uses a depth-first search on a binary + * tree. In the binary tree, each node corresponds to the inclusion or the omission of a UTXO. UTXOs + * are sorted by their effective values and the trees is explored deterministically per the inclusion + * branch first. At each node, the algorithm checks whether the selection is within the target range. + * While the selection has not reached the target range, more UTXOs are included. When a selection's + * value exceeds the target range, the complete subtree deriving from this selection can be omitted. + * At that point, the last included UTXO is deselected and the corresponding omission branch explored + * instead. The search ends after the complete tree has been searched or after a limited number of tries. + * + * The search continues to search for better solutions after one solution has been found. The best + * solution is chosen by minimizing the waste metric. The waste metric is defined as the cost to + * spend the current inputs at the given fee rate minus the long term expected cost to spend the + * inputs, plus the amount the selection exceeds the spending target: + * + * waste = selectionTotal - target + inputs × (currentFeeRate - longTermFeeRate) + * + * The algorithm uses two additional optimizations. A lookahead keeps track of the total value of + * the unexplored UTXOs. A subtree is not explored if the lookahead indicates that the target range + * cannot be reached. Further, it is unnecessary to test equivalent combinations. This allows us + * to skip testing the inclusion of UTXOs that match the effective value and waste of an omitted + * predecessor. + * + * The Branch and Bound algorithm is described in detail in Murch's Master Thesis: + * https://murch.one/wp-content/uploads/2016/11/erhardt2016coinselection.pdf + * + * @param const std::vector<CInputCoin>& utxo_pool The set of UTXOs that we are choosing from. + * These UTXOs will be sorted in descending order by effective value and the CInputCoins' + * values are their effective values. + * @param const CAmount& target_value This is the value that we want to select. It is the lower + * bound of the range. + * @param const CAmount& cost_of_change This is the cost of creating and spending a change output. + * This plus target_value is the upper bound of the range. + * @param std::set<CInputCoin>& out_set -> This is an output parameter for the set of CInputCoins + * that have been selected. + * @param CAmount& value_ret -> This is an output parameter for the total value of the CInputCoins + * that were selected. + * @param CAmount not_input_fees -> The fees that need to be paid for the outputs and fixed size + * overhead (version, locktime, marker and flag) + */ + +static const size_t TOTAL_TRIES = 100000; + +bool SelectCoinsBnB(std::vector<CInputCoin>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees) +{ + out_set.clear(); + CAmount curr_value = 0; + + std::vector<bool> curr_selection; // select the utxo at this index + curr_selection.reserve(utxo_pool.size()); + CAmount actual_target = not_input_fees + target_value; + + // Calculate curr_available_value + CAmount curr_available_value = 0; + for (const CInputCoin& utxo : utxo_pool) { + // Assert that this utxo is not negative. It should never be negative, effective value calculation should have removed it + assert(utxo.effective_value > 0); + curr_available_value += utxo.effective_value; + } + if (curr_available_value < actual_target) { + return false; + } + + // Sort the utxo_pool + std::sort(utxo_pool.begin(), utxo_pool.end(), descending); + + CAmount curr_waste = 0; + std::vector<bool> best_selection; + CAmount best_waste = MAX_MONEY; + + // Depth First search loop for choosing the UTXOs + for (size_t i = 0; i < TOTAL_TRIES; ++i) { + // Conditions for starting a backtrack + bool backtrack = false; + if (curr_value + curr_available_value < actual_target || // Cannot possibly reach target with the amount remaining in the curr_available_value. + curr_value > actual_target + cost_of_change || // Selected value is out of range, go back and try other branch + (curr_waste > best_waste && (utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0)) { // Don't select things which we know will be more wasteful if the waste is increasing + backtrack = true; + } else if (curr_value >= actual_target) { // Selected value is within range + curr_waste += (curr_value - actual_target); // This is the excess value which is added to the waste for the below comparison + // Adding another UTXO after this check could bring the waste down if the long term fee is higher than the current fee. + // However we are not going to explore that because this optimization for the waste is only done when we have hit our target + // value. Adding any more UTXOs will be just burning the UTXO; it will go entirely to fees. Thus we aren't going to + // explore any more UTXOs to avoid burning money like that. + if (curr_waste <= best_waste) { + best_selection = curr_selection; + best_selection.resize(utxo_pool.size()); + best_waste = curr_waste; + } + curr_waste -= (curr_value - actual_target); // Remove the excess value as we will be selecting different coins now + backtrack = true; + } + + // Backtracking, moving backwards + if (backtrack) { + // Walk backwards to find the last included UTXO that still needs to have its omission branch traversed. + while (!curr_selection.empty() && !curr_selection.back()) { + curr_selection.pop_back(); + curr_available_value += utxo_pool.at(curr_selection.size()).effective_value; + }; + + if (curr_selection.empty()) { // We have walked back to the first utxo and no branch is untraversed. All solutions searched + break; + } + + // Output was included on previous iterations, try excluding now. + curr_selection.back() = false; + CInputCoin& utxo = utxo_pool.at(curr_selection.size() - 1); + curr_value -= utxo.effective_value; + curr_waste -= utxo.fee - utxo.long_term_fee; + } else { // Moving forwards, continuing down this branch + CInputCoin& utxo = utxo_pool.at(curr_selection.size()); + + // Remove this utxo from the curr_available_value utxo amount + curr_available_value -= utxo.effective_value; + + // Avoid searching a branch if the previous UTXO has the same value and same waste and was excluded. Since the ratio of fee to + // long term fee is the same, we only need to check if one of those values match in order to know that the waste is the same. + if (!curr_selection.empty() && !curr_selection.back() && + utxo.effective_value == utxo_pool.at(curr_selection.size() - 1).effective_value && + utxo.fee == utxo_pool.at(curr_selection.size() - 1).fee) { + curr_selection.push_back(false); + } else { + // Inclusion branch first (Largest First Exploration) + curr_selection.push_back(true); + curr_value += utxo.effective_value; + curr_waste += utxo.fee - utxo.long_term_fee; + } + } + } + + // Check for solution + if (best_selection.empty()) { + return false; + } + + // Set output set + value_ret = 0; + for (size_t i = 0; i < best_selection.size(); ++i) { + if (best_selection.at(i)) { + out_set.insert(utxo_pool.at(i)); + value_ret += utxo_pool.at(i).txout.nValue; + } + } + + return true; +} + +static void ApproximateBestSubset(const std::vector<CInputCoin>& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, + std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) +{ + std::vector<char> vfIncluded; + + vfBest.assign(vValue.size(), true); + nBest = nTotalLower; + + FastRandomContext insecure_rand; + + for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++) + { + vfIncluded.assign(vValue.size(), false); + CAmount nTotal = 0; + bool fReachedTarget = false; + for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) + { + for (unsigned int i = 0; i < vValue.size(); i++) + { + //The solver here uses a randomized algorithm, + //the randomness serves no real security purpose but is just + //needed to prevent degenerate behavior and it is important + //that the rng is fast. We do not use a constant random sequence, + //because there may be some privacy improvement by making + //the selection random. + if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) + { + nTotal += vValue[i].txout.nValue; + vfIncluded[i] = true; + if (nTotal >= nTargetValue) + { + fReachedTarget = true; + if (nTotal < nBest) + { + nBest = nTotal; + vfBest = vfIncluded; + } + nTotal -= vValue[i].txout.nValue; + vfIncluded[i] = false; + } + } + } + } + } +} + +bool KnapsackSolver(const CAmount& nTargetValue, std::vector<CInputCoin>& vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) +{ + setCoinsRet.clear(); + nValueRet = 0; + + // List of values less than target + boost::optional<CInputCoin> coinLowestLarger; + std::vector<CInputCoin> vValue; + CAmount nTotalLower = 0; + + random_shuffle(vCoins.begin(), vCoins.end(), GetRandInt); + + for (const CInputCoin &coin : vCoins) + { + if (coin.txout.nValue == nTargetValue) + { + setCoinsRet.insert(coin); + nValueRet += coin.txout.nValue; + return true; + } + else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) + { + vValue.push_back(coin); + nTotalLower += coin.txout.nValue; + } + else if (!coinLowestLarger || coin.txout.nValue < coinLowestLarger->txout.nValue) + { + coinLowestLarger = coin; + } + } + + if (nTotalLower == nTargetValue) + { + for (const auto& input : vValue) + { + setCoinsRet.insert(input); + nValueRet += input.txout.nValue; + } + return true; + } + + if (nTotalLower < nTargetValue) + { + if (!coinLowestLarger) + return false; + setCoinsRet.insert(coinLowestLarger.get()); + nValueRet += coinLowestLarger->txout.nValue; + return true; + } + + // Solve subset sum by stochastic approximation + std::sort(vValue.begin(), vValue.end(), descending); + std::vector<char> vfBest; + CAmount nBest; + + ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); + if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) + ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); + + // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, + // or the next bigger coin is closer), return the bigger coin + if (coinLowestLarger && + ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger->txout.nValue <= nBest)) + { + setCoinsRet.insert(coinLowestLarger.get()); + nValueRet += coinLowestLarger->txout.nValue; + } + else { + for (unsigned int i = 0; i < vValue.size(); i++) + if (vfBest[i]) + { + setCoinsRet.insert(vValue[i]); + nValueRet += vValue[i].txout.nValue; + } + + if (LogAcceptCategory(BCLog::SELECTCOINS)) { + LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); + for (unsigned int i = 0; i < vValue.size(); i++) { + if (vfBest[i]) { + LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(vValue[i].txout.nValue)); + } + } + LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); + } + } + + return true; +} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h new file mode 100644 index 0000000000..4d1a43bc17 --- /dev/null +++ b/src/wallet/coinselection.h @@ -0,0 +1,54 @@ +// Copyright (c) 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. + +#ifndef BITCOIN_COINSELECTION_H +#define BITCOIN_COINSELECTION_H + +#include <amount.h> +#include <primitives/transaction.h> +#include <random.h> + +//! target minimum change amount +static const CAmount MIN_CHANGE = CENT; +//! final minimum change amount after paying for fees +static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; + +class CInputCoin { +public: + CInputCoin(const CTransactionRef& tx, unsigned int i) + { + if (!tx) + throw std::invalid_argument("tx should not be null"); + if (i >= tx->vout.size()) + throw std::out_of_range("The output index is out of range"); + + outpoint = COutPoint(tx->GetHash(), i); + txout = tx->vout[i]; + effective_value = txout.nValue; + } + + COutPoint outpoint; + CTxOut txout; + CAmount effective_value; + CAmount fee = 0; + CAmount long_term_fee = 0; + + bool operator<(const CInputCoin& rhs) const { + return outpoint < rhs.outpoint; + } + + bool operator!=(const CInputCoin& rhs) const { + return outpoint != rhs.outpoint; + } + + bool operator==(const CInputCoin& rhs) const { + return outpoint == rhs.outpoint; + } +}; + +bool SelectCoinsBnB(std::vector<CInputCoin>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees); + +// Original coin selection algorithm as a fallback +bool KnapsackSolver(const CAmount& nTargetValue, std::vector<CInputCoin>& vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet); +#endif // BITCOIN_COINSELECTION_H diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 9cae660c60..82a5017de0 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -16,33 +16,6 @@ #include <util.h> #include <net.h> -// Calculate the size of the transaction assuming all signatures are max size -// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. -// TODO: re-use this in CWallet::CreateTransaction (right now -// CreateTransaction uses the constructed dummy-signed tx to do a priority -// calculation, but we should be able to refactor after priority is removed). -// NOTE: this requires that all inputs must be in mapWallet (eg the tx should -// be IsAllFromMe). -static int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet) -{ - CMutableTransaction txNew(tx); - std::vector<CInputCoin> vCoins; - // Look up the inputs. We should have already checked that this transaction - // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our - // wallet, with a valid index into the vout array. - for (auto& input : tx.vin) { - const auto mi = wallet->mapWallet.find(input.prevout.hash); - assert(mi != wallet->mapWallet.end() && input.prevout.n < mi->second.tx->vout.size()); - vCoins.emplace_back(CInputCoin(&(mi->second), input.prevout.n)); - } - if (!wallet->DummySignTx(txNew, vCoins)) { - // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) - // implies that we can sign for every input. - return -1; - } - return GetVirtualTransactionSize(txNew); -} - //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) @@ -262,23 +235,20 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti return result; } - CWalletTx wtxBumped(wallet, MakeTransactionRef(std::move(mtx))); // commit/broadcast the tx + CTransactionRef tx = MakeTransactionRef(std::move(mtx)); + mapValue_t mapValue = oldWtx.mapValue; + mapValue["replaces_txid"] = oldWtx.GetHash().ToString(); + CReserveKey reservekey(wallet); - wtxBumped.mapValue = oldWtx.mapValue; - wtxBumped.mapValue["replaces_txid"] = oldWtx.GetHash().ToString(); - wtxBumped.vOrderForm = oldWtx.vOrderForm; - wtxBumped.strFromAccount = oldWtx.strFromAccount; - wtxBumped.fTimeReceivedIsTxTime = true; - wtxBumped.fFromMe = true; CValidationState state; - if (!wallet->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { + if (!wallet->CommitTransaction(tx, std::move(mapValue), oldWtx.vOrderForm, oldWtx.strFromAccount, reservekey, g_connman.get(), state)) { // NOTE: CommitTransaction never returns false, so this should never happen. errors.push_back(strprintf("The transaction was rejected: %s", FormatStateMessage(state))); return Result::WALLET_ERROR; } - bumped_txid = wtxBumped.GetHash(); + bumped_txid = tx->GetHash(); if (state.IsInvalid()) { // This can happen if the mempool rejected the transaction. Report // what happened in the "errors" response. @@ -286,7 +256,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti } // mark the original tx as bumped - if (!wallet->MarkReplaced(oldWtx.GetHash(), wtxBumped.GetHash())) { + if (!wallet->MarkReplaced(oldWtx.GetHash(), bumped_txid)) { // TODO: see if JSON-RPC has a standard way of returning a response // along with an exception. It would be good to return information about // wtxBumped to the caller even if marking the original transaction diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 385fdc963a..03c32d3b97 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -21,6 +21,22 @@ CAmount GetRequiredFee(unsigned int nTxBytes) CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) { + CAmount fee_needed = GetMinimumFeeRate(coin_control, pool, estimator, feeCalc).GetFee(nTxBytes); + // Always obey the maximum + if (fee_needed > maxTxFee) { + fee_needed = maxTxFee; + if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; + } + return fee_needed; +} + +CFeeRate GetRequiredFeeRate() +{ + return std::max(CWallet::minTxFee, ::minRelayTxFee); +} + +CFeeRate GetMinimumFeeRate(const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) +{ /* User control of how to calculate fee uses the following parameter precedence: 1. coin_control.m_feerate 2. coin_control.m_confirm_target @@ -28,15 +44,15 @@ CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, c 4. nTxConfirmTarget (user-set global variable) The first parameter that is set is used. */ - CAmount fee_needed; + CFeeRate feerate_needed ; if (coin_control.m_feerate) { // 1. - fee_needed = coin_control.m_feerate->GetFee(nTxBytes); + feerate_needed = *(coin_control.m_feerate); if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; // Allow to override automatic min/max check over coin control instance - if (coin_control.fOverrideFeeRate) return fee_needed; + if (coin_control.fOverrideFeeRate) return feerate_needed; } else if (!coin_control.m_confirm_target && ::payTxFee != CFeeRate(0)) { // 3. TODO: remove magic value of 0 for global payTxFee - fee_needed = ::payTxFee.GetFee(nTxBytes); + feerate_needed = ::payTxFee; if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; } else { // 2. or 4. @@ -48,38 +64,32 @@ CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, c if (coin_control.m_fee_mode == FeeEstimateMode::CONSERVATIVE) conservative_estimate = true; else if (coin_control.m_fee_mode == FeeEstimateMode::ECONOMICAL) conservative_estimate = false; - fee_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate).GetFee(nTxBytes); - if (fee_needed == 0) { + feerate_needed = estimator.estimateSmartFee(target, feeCalc, conservative_estimate); + if (feerate_needed == CFeeRate(0)) { // if we don't have enough data for estimateSmartFee, then use fallbackFee - fee_needed = CWallet::fallbackFee.GetFee(nTxBytes); + feerate_needed = CWallet::fallbackFee; if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; // directly return if fallback fee is disabled (feerate 0 == disabled) - if (CWallet::fallbackFee.GetFee(1000) == 0) return fee_needed; + if (CWallet::fallbackFee == CFeeRate(0)) return feerate_needed; } // Obey mempool min fee when using smart fee estimation - CAmount min_mempool_fee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nTxBytes); - if (fee_needed < min_mempool_fee) { - fee_needed = min_mempool_fee; + CFeeRate min_mempool_feerate = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); + if (feerate_needed < min_mempool_feerate) { + feerate_needed = min_mempool_feerate; if (feeCalc) feeCalc->reason = FeeReason::MEMPOOL_MIN; } } // prevent user from paying a fee below minRelayTxFee or minTxFee - CAmount required_fee = GetRequiredFee(nTxBytes); - if (required_fee > fee_needed) { - fee_needed = required_fee; + CFeeRate required_feerate = GetRequiredFeeRate(); + if (required_feerate > feerate_needed) { + feerate_needed = required_feerate; if (feeCalc) feeCalc->reason = FeeReason::REQUIRED; } - // But always obey the maximum - if (fee_needed > maxTxFee) { - fee_needed = maxTxFee; - if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; - } - return fee_needed; + return feerate_needed; } - CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator) { unsigned int highest_target = estimator.HighestTargetTracked(FeeEstimateHorizon::LONG_HALFLIFE); diff --git a/src/wallet/fees.h b/src/wallet/fees.h index 225aff08ad..a627af70b0 100644 --- a/src/wallet/fees.h +++ b/src/wallet/fees.h @@ -27,6 +27,18 @@ CAmount GetRequiredFee(unsigned int nTxBytes); CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); /** + * Return the minimum required feerate taking into account the + * floating relay feerate and user set minimum transaction feerate + */ +CFeeRate GetRequiredFeeRate(); + +/** + * Estimate the minimum fee rate considering user set parameters + * and the required fee + */ +CFeeRate GetMinimumFeeRate(const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); + +/** * Return the maximum feerate for discarding change. */ CFeeRate GetDiscardRate(const CBlockPolicyEstimator& estimator); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c02c1bd18d..7ad9efff70 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -404,7 +404,7 @@ UniValue getaddressesbyaccount(const JSONRPCRequest& request) return ret; } -static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, CWalletTx& wtxNew, const CCoinControl& coin_control) +static CTransactionRef SendMoney(CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue, std::string fromAccount) { CAmount curBalance = pwallet->GetBalance(); @@ -430,16 +430,18 @@ static void SendMoney(CWallet * const pwallet, const CTxDestination &address, CA int nChangePosRet = -1; CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; vecSend.push_back(recipient); - if (!pwallet->CreateTransaction(vecSend, wtxNew, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { + CTransactionRef tx; + if (!pwallet->CreateTransaction(vecSend, tx, reservekey, nFeeRequired, nChangePosRet, strError, coin_control)) { if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } CValidationState state; - if (!pwallet->CommitTransaction(wtxNew, reservekey, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, std::move(fromAccount), reservekey, g_connman.get(), state)) { strError = strprintf("Error: The transaction was rejected! Reason given: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strError); } + return tx; } UniValue sendtoaddress(const JSONRPCRequest& request) @@ -498,11 +500,11 @@ UniValue sendtoaddress(const JSONRPCRequest& request) throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); // Wallet comments - CWalletTx wtx; + mapValue_t mapValue; if (!request.params[2].isNull() && !request.params[2].get_str().empty()) - wtx.mapValue["comment"] = request.params[2].get_str(); + mapValue["comment"] = request.params[2].get_str(); if (!request.params[3].isNull() && !request.params[3].get_str().empty()) - wtx.mapValue["to"] = request.params[3].get_str(); + mapValue["to"] = request.params[3].get_str(); bool fSubtractFeeFromAmount = false; if (!request.params[4].isNull()) { @@ -527,9 +529,8 @@ UniValue sendtoaddress(const JSONRPCRequest& request) EnsureWalletIsUnlocked(pwallet); - SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, wtx, coin_control); - - return wtx.GetHash().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue), {} /* fromAccount */); + return tx->GetHash().GetHex(); } UniValue listaddressgroupings(const JSONRPCRequest& request) @@ -995,12 +996,11 @@ UniValue sendfrom(const JSONRPCRequest& request) if (!request.params[3].isNull()) nMinDepth = request.params[3].get_int(); - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (!request.params[4].isNull() && !request.params[4].get_str().empty()) - wtx.mapValue["comment"] = request.params[4].get_str(); + mapValue["comment"] = request.params[4].get_str(); if (!request.params[5].isNull() && !request.params[5].get_str().empty()) - wtx.mapValue["to"] = request.params[5].get_str(); + mapValue["to"] = request.params[5].get_str(); EnsureWalletIsUnlocked(pwallet); @@ -1010,9 +1010,8 @@ UniValue sendfrom(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Account has insufficient funds"); CCoinControl no_coin_control; // This is a deprecated API - SendMoney(pwallet, dest, nAmount, false, wtx, no_coin_control); - - return wtx.GetHash().GetHex(); + CTransactionRef tx = SendMoney(pwallet, dest, nAmount, false, no_coin_control, std::move(mapValue), std::move(strAccount)); + return tx->GetHash().GetHex(); } @@ -1083,10 +1082,9 @@ UniValue sendmany(const JSONRPCRequest& request) if (!request.params[2].isNull()) nMinDepth = request.params[2].get_int(); - CWalletTx wtx; - wtx.strFromAccount = strAccount; + mapValue_t mapValue; if (!request.params[3].isNull() && !request.params[3].get_str().empty()) - wtx.mapValue["comment"] = request.params[3].get_str(); + mapValue["comment"] = request.params[3].get_str(); UniValue subtractFeeFromAmount(UniValue::VARR); if (!request.params[4].isNull()) @@ -1152,16 +1150,17 @@ UniValue sendmany(const JSONRPCRequest& request) CAmount nFeeRequired = 0; int nChangePosRet = -1; std::string strFailReason; - bool fCreated = pwallet->CreateTransaction(vecSend, wtx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); + CTransactionRef tx; + bool fCreated = pwallet->CreateTransaction(vecSend, tx, keyChange, nFeeRequired, nChangePosRet, strFailReason, coin_control); if (!fCreated) throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); CValidationState state; - if (!pwallet->CommitTransaction(wtx, keyChange, g_connman.get(), state)) { + if (!pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */, std::move(strAccount), keyChange, g_connman.get(), state)) { strFailReason = strprintf("Transaction commit failed:: %s", FormatStateMessage(state)); throw JSONRPCError(RPC_WALLET_ERROR, strFailReason); } - return wtx.GetHash().GetHex(); + return tx->GetHash().GetHex(); } UniValue addmultisigaddress(const JSONRPCRequest& request) diff --git a/src/wallet/test/accounting_tests.cpp b/src/wallet/test/accounting_tests.cpp index 7b20bd7b02..aae328d81f 100644 --- a/src/wallet/test/accounting_tests.cpp +++ b/src/wallet/test/accounting_tests.cpp @@ -29,7 +29,7 @@ GetResults(CWallet& wallet, std::map<CAmount, CAccountingEntry>& results) BOOST_AUTO_TEST_CASE(acc_orderupgrade) { std::vector<CWalletTx*> vpwtx; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); CAccountingEntry ae; std::map<CAmount, CAccountingEntry> results; @@ -44,7 +44,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "z"; m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.SetTx(MakeTransactionRef(std::move(tx))); } m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; @@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.SetTx(MakeTransactionRef(std::move(tx))); } m_wallet.AddToWallet(wtx); - vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]); + vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash())); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp new file mode 100644 index 0000000000..f05c81cd4c --- /dev/null +++ b/src/wallet/test/coinselector_tests.cpp @@ -0,0 +1,561 @@ +// Copyright (c) 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. + +#include "wallet/wallet.h" +#include "wallet/coinselection.h" +#include "amount.h" +#include "primitives/transaction.h" +#include "random.h" +#include "test/test_bitcoin.h" +#include "wallet/test/wallet_test_fixture.h" + +#include <boost/test/unit_test.hpp> +#include <random> + +BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup) + +// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles +#define RUN_TESTS 100 + +// some tests fail 1% of the time due to bad luck. +// we repeat those tests this many times and only complain if all iterations of the test fail +#define RANDOM_REPEATS 5 + +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 CAmount balance = 0; + +CoinEligibilityFilter filter_standard(1, 6, 0); +CoinEligibilityFilter filter_confirmed(1, 1, 0); +CoinEligibilityFilter filter_standard_extra(6, 6, 0); +CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0); + +static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + set.emplace_back(MakeTransactionRef(tx), nInput); +} + +static void add_coin(const CAmount& nValue, int nInput, CoinSet& set) +{ + CMutableTransaction tx; + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + set.emplace(MakeTransactionRef(tx), nInput); +} + +static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) +{ + balance += nValue; + static int nextLockTime = 0; + CMutableTransaction tx; + tx.nLockTime = nextLockTime++; // so all transactions get different hashes + tx.vout.resize(nInput + 1); + tx.vout[nInput].nValue = nValue; + if (fIsFromMe) { + // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), + // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() + tx.vin.resize(1); + } + std::unique_ptr<CWalletTx> wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); + if (fIsFromMe) + { + wtx->fDebitCached = true; + wtx->nDebitCached = 1; + } + COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); + vCoins.push_back(output); + wtxn.emplace_back(std::move(wtx)); +} + +static void empty_wallet(void) +{ + vCoins.clear(); + wtxn.clear(); + balance = 0; +} + +static bool equal_sets(CoinSet a, CoinSet b) +{ + std::pair<CoinSet::iterator, CoinSet::iterator> ret = mismatch(a.begin(), a.end(), b.begin()); + return ret.first == a.end() && ret.second == b.end(); +} + +static CAmount make_hard_case(int utxos, std::vector<CInputCoin>& utxo_pool) +{ + utxo_pool.clear(); + CAmount target = 0; + for (int i = 0; i < utxos; ++i) { + target += (CAmount)1 << (utxos+i); + add_coin((CAmount)1 << (utxos+i), 2*i, utxo_pool); + add_coin(((CAmount)1 << (utxos+i)) + ((CAmount)1 << (utxos-1-i)), 2*i + 1, utxo_pool); + } + return target; +} + +// Branch and bound coin selection tests +BOOST_AUTO_TEST_CASE(bnb_search_test) +{ + + LOCK(testWallet.cs_wallet); + + // Setup + std::vector<CInputCoin> utxo_pool; + CoinSet selection; + CoinSet actual_selection; + CAmount value_ret = 0; + CAmount not_input_fees = 0; + + ///////////////////////// + // Known Outcome tests // + ///////////////////////// + BOOST_TEST_MESSAGE("Testing known outcomes"); + + // Empty utxo pool + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 1 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + selection.clear(); + + // Add utxos + add_coin(1 * CENT, 1, utxo_pool); + add_coin(2 * CENT, 2, utxo_pool); + add_coin(3 * CENT, 3, utxo_pool); + add_coin(4 * CENT, 4, utxo_pool); + + // Select 1 Cent + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 1 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 2 Cent + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 2 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 5 Cent + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 5 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Select 11 Cent, not possible + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 11 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + actual_selection.clear(); + selection.clear(); + + // Select 10 Cent + add_coin(5 * CENT, 5, utxo_pool); + add_coin(4 * CENT, 4, actual_selection); + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + add_coin(1 * CENT, 1, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 10 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + BOOST_CHECK(equal_sets(selection, actual_selection)); + actual_selection.clear(); + selection.clear(); + + // Negative effective value + // Select 10 Cent but have 1 Cent not be possible because too small + add_coin(5 * CENT, 5, actual_selection); + add_coin(3 * CENT, 3, actual_selection); + add_coin(2 * CENT, 2, actual_selection); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 10 * CENT, 5000, selection, value_ret, not_input_fees)); + + // Select 0.25 Cent, not possible + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 0.25 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); + actual_selection.clear(); + selection.clear(); + + // Iteration exhaustion test + CAmount target = make_hard_case(17, utxo_pool); + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees)); // Should exhaust + target = make_hard_case(14, utxo_pool); + BOOST_CHECK(SelectCoinsBnB(utxo_pool, target, 0, selection, value_ret, not_input_fees)); // Should not exhaust + + // Test same value early bailout optimization + add_coin(7 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, actual_selection); + add_coin(2 * CENT, 7, actual_selection); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(7 * CENT, 7, utxo_pool); + add_coin(2 * CENT, 7, utxo_pool); + for (int i = 0; i < 50000; ++i) { + add_coin(5 * CENT, 7, utxo_pool); + } + BOOST_CHECK(SelectCoinsBnB(utxo_pool, 30 * CENT, 5000, selection, value_ret, not_input_fees)); + + //////////////////// + // Behavior tests // + //////////////////// + // Select 1 Cent with pool of only greater than 5 Cent + utxo_pool.clear(); + for (int i = 5; i <= 20; ++i) { + add_coin(i * CENT, i, utxo_pool); + } + // Run 100 times, to make sure it is never finding a solution + for (int i = 0; i < 100; ++i) { + BOOST_CHECK(!SelectCoinsBnB(utxo_pool, 1 * CENT, 2 * CENT, selection, value_ret, not_input_fees)); + } + + // Make sure that effective value is working in SelectCoinsMinConf when BnB is used + CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0); + CoinSet setCoinsRet; + CAmount nValueRet; + bool bnb_used; + empty_wallet(); + 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)); +} + +BOOST_AUTO_TEST_CASE(knapsack_solver_test) +{ + CoinSet setCoinsRet, setCoinsRet2; + CAmount nValueRet; + bool bnb_used; + + LOCK(testWallet.cs_wallet); + + // test multiple times to allow for differences in the shuffle order + for (int i = 0; i < RUN_TESTS; i++) + { + empty_wallet(); + + // with an empty wallet we can't even pay one cent + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + add_coin(1*CENT, 4); // add a new 1 cent coin + + // with a new 1 cent coin, we still can't find a mature 1 cent + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + // but we can find a new 1 cent + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); + + add_coin(2*CENT); // add a mature 2 cent coin + + // we can't make 3 cents of mature coins + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + // we can make 3 cents of new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); + + add_coin(5*CENT); // add a mature 5 cent coin, + add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses + add_coin(20*CENT); // and a mature 20 cent coin + + // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 + + // we can't make 38 cents only if we disallow new coins: + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + // we can't even make 37 cents if we don't allow new coins even if they're from us + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + // but we can make 37 cents if we accept new coins from ourself + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); + // and we can make 38 cents if we accept all new coins + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); + + // try making 34 cents from 1,2,5,10,20 - we can't do it exactly + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) + + // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(nValueRet == 8 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + + // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin + empty_wallet(); + + add_coin( 6*CENT); + add_coin( 7*CENT); + add_coin( 8*CENT); + add_coin(20*CENT); + add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total + + // check that we have 71 and not 72 + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + + // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total + + // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + + add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 + + // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins + + // now try making 11 cents. we should get 5+6 + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // check that the smallest bigger coin is used + add_coin( 1*COIN); + add_coin( 2*COIN); + add_coin( 3*COIN); + add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // empty the wallet and start again, now with fractions of a cent, to test small change avoidance + + empty_wallet(); + add_coin(MIN_CHANGE * 1 / 10); + add_coin(MIN_CHANGE * 2 / 10); + add_coin(MIN_CHANGE * 3 / 10); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 5 / 10); + + // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE + // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); + + // but if we add a bigger coin, small change is avoided + add_coin(1111*MIN_CHANGE); + + // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // if we add more small coins: + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + + // and try again to make 1.0 * MIN_CHANGE + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount + + // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) + // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change + empty_wallet(); + for (int j = 0; j < 20; j++) + add_coin(50000 * COIN); + + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount + BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins + + // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), + // we need to try finding an exact subset anyway + + // sometimes it will fail, and so we use the next biggest coin: + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 7 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + + // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) + empty_wallet(); + add_coin(MIN_CHANGE * 4 / 10); + add_coin(MIN_CHANGE * 6 / 10); + add_coin(MIN_CHANGE * 8 / 10); + add_coin(1111 * MIN_CHANGE); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 + + // test avoiding small change + empty_wallet(); + add_coin(MIN_CHANGE * 5 / 100); + add_coin(MIN_CHANGE * 1); + add_coin(MIN_CHANGE * 100); + + // trying to make 100.01 from these three coins + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins + BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); + + // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + // test with many inputs + for (CAmount amt=1500; amt < COIN; amt*=10) { + empty_wallet(); + // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) + for (uint16_t j = 0; j < 676; j++) + add_coin(amt); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + if (amt - 2000 < MIN_CHANGE) { + // needs more than one input: + uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); + CAmount returnValue = amt * returnSize; + BOOST_CHECK_EQUAL(nValueRet, returnValue); + BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); + } else { + // one input is sufficient: + BOOST_CHECK_EQUAL(nValueRet, amt); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); + } + } + + // test randomness + { + empty_wallet(); + for (int i2 = 0; i2 < 100; i2++) + add_coin(COIN); + + // picking 50 from 100 coins doesn't depend on the shuffle, + // but does depend on randomness in the stochastic approximation code + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); + + int fails = 0; + for (int j = 0; j < RANDOM_REPEATS; j++) + { + // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time + // run the test RANDOM_REPEATS times and only complain if all of them fail + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + + // add 75 cents in small change. not enough to make 90 cents, + // then try making 90 cents. there are multiple competing "smallest bigger" coins, + // one of which should be picked at random + add_coin(5 * CENT); + add_coin(10 * CENT); + add_coin(15 * CENT); + add_coin(20 * CENT); + add_coin(25 * CENT); + + fails = 0; + for (int j = 0; j < RANDOM_REPEATS; j++) + { + // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time + // run the test RANDOM_REPEATS times and only complain if all of them fail + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + if (equal_sets(setCoinsRet, setCoinsRet2)) + fails++; + } + BOOST_CHECK_NE(fails, RANDOM_REPEATS); + } + } + empty_wallet(); +} + +BOOST_AUTO_TEST_CASE(ApproximateBestSubset) +{ + CoinSet setCoinsRet; + CAmount nValueRet; + bool bnb_used; + + LOCK(testWallet.cs_wallet); + + empty_wallet(); + + // Test vValue sort order + for (int i = 0; i < 1000; i++) + add_coin(1000 * COIN); + add_coin(3 * COIN); + + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); + BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); + + empty_wallet(); +} + +// Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value +BOOST_AUTO_TEST_CASE(SelectCoins_test) +{ + // Random generator stuff + std::default_random_engine generator; + std::exponential_distribution<double> distribution (100); + FastRandomContext rand; + + // Output stuff + CAmount out_value = 0; + CoinSet out_set; + CAmount target = 0; + bool bnb_used; + + // Run this test 100 times + for (int i = 0; i < 100; ++i) + { + // Reset + out_value = 0; + target = 0; + out_set.clear(); + empty_wallet(); + + // Make a wallet with 1000 exponentially distributed random inputs + for (int j = 0; j < 1000; ++j) + { + add_coin((CAmount)(distribution(generator)*10000000)); + } + + // Generate a random fee rate in the range of 100 - 400 + CFeeRate rate(rand.randrange(300) + 100); + + // Generate a random target value between 1000 and wallet balance + target = rand.randrange(balance - 1000) + 1000; + + // Perform selection + CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0); + CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK_GE(out_value, target); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 41348b50a4..d6b0daf4fe 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -23,345 +23,8 @@ extern UniValue importmulti(const JSONRPCRequest& request); extern UniValue dumpwallet(const JSONRPCRequest& request); extern UniValue importwallet(const JSONRPCRequest& request); -// how many times to run all the tests to have a chance to catch errors that only show up with particular random shuffles -#define RUN_TESTS 100 - -// some tests fail 1% of the time due to bad luck. -// we repeat those tests this many times and only complain if all iterations of the test fail -#define RANDOM_REPEATS 5 - -std::vector<std::unique_ptr<CWalletTx>> wtxn; - -typedef std::set<CInputCoin> CoinSet; - BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const CWallet testWallet("dummy", CWalletDBWrapper::CreateDummy()); -static std::vector<COutput> vCoins; - -static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) -{ - static int nextLockTime = 0; - CMutableTransaction tx; - tx.nLockTime = nextLockTime++; // so all transactions get different hashes - tx.vout.resize(nInput+1); - tx.vout[nInput].nValue = nValue; - if (fIsFromMe) { - // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), - // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() - tx.vin.resize(1); - } - std::unique_ptr<CWalletTx> wtx(new CWalletTx(&testWallet, MakeTransactionRef(std::move(tx)))); - if (fIsFromMe) - { - wtx->fDebitCached = true; - wtx->nDebitCached = 1; - } - COutput output(wtx.get(), nInput, nAge, true /* spendable */, true /* solvable */, true /* safe */); - vCoins.push_back(output); - wtxn.emplace_back(std::move(wtx)); -} - -static void empty_wallet(void) -{ - vCoins.clear(); - wtxn.clear(); -} - -static bool equal_sets(CoinSet a, CoinSet b) -{ - std::pair<CoinSet::iterator, CoinSet::iterator> ret = mismatch(a.begin(), a.end(), b.begin()); - return ret.first == a.end() && ret.second == b.end(); -} - -BOOST_AUTO_TEST_CASE(coin_selection_tests) -{ - CoinSet setCoinsRet, setCoinsRet2; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - // test multiple times to allow for differences in the shuffle order - for (int i = 0; i < RUN_TESTS; i++) - { - empty_wallet(); - - // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - add_coin(1*CENT, 4); // add a new 1 cent coin - - // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); - - add_coin(2*CENT); // add a mature 2 cent coin - - // we can't make 3 cents of mature coins - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - - // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); - - add_coin(5*CENT); // add a mature 5 cent coin, - add_coin(10*CENT, 3, true); // a new 10 cent coin sent from one of our own addresses - add_coin(20*CENT); // and a mature 20 cent coin - - // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 - - // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, 6, 6, 0, vCoins, setCoinsRet, nValueRet)); - // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); - // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); - - // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) - - // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(nValueRet == 8 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - - // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - empty_wallet(); - - add_coin( 6*CENT); - add_coin( 7*CENT); - add_coin( 8*CENT); - add_coin(20*CENT); - add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total - - // check that we have 71 and not 72 - BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - - // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - add_coin( 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total - - // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - - add_coin( 18*CENT); // now we have 5+6+7+8+18+20+30 - - // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins - - // now try making 11 cents. we should get 5+6 - BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // check that the smallest bigger coin is used - add_coin( 1*COIN); - add_coin( 2*COIN); - add_coin( 3*COIN); - add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - - empty_wallet(); - add_coin(MIN_CHANGE * 1 / 10); - add_coin(MIN_CHANGE * 2 / 10); - add_coin(MIN_CHANGE * 3 / 10); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 5 / 10); - - // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE - // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); - - // but if we add a bigger coin, small change is avoided - add_coin(1111*MIN_CHANGE); - - // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // if we add more small coins: - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - - // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount - - // run the 'mtgox' test (see http://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) - // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - empty_wallet(); - for (int j = 0; j < 20; j++) - add_coin(50000 * COIN); - - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins - - // if there's not enough in the smaller coins to make at least 1 * MIN_CHANGE change (0.5+0.6+0.7 < 1.0+1.0), - // we need to try finding an exact subset anyway - - // sometimes it will fail, and so we use the next biggest coin: - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 7 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - - // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - empty_wallet(); - add_coin(MIN_CHANGE * 4 / 10); - add_coin(MIN_CHANGE * 6 / 10); - add_coin(MIN_CHANGE * 8 / 10); - add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 - - // test avoiding small change - empty_wallet(); - add_coin(MIN_CHANGE * 5 / 100); - add_coin(MIN_CHANGE * 1); - add_coin(MIN_CHANGE * 100); - - // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins - BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); - - // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - // test with many inputs - for (CAmount amt=1500; amt < COIN; amt*=10) { - empty_wallet(); - // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) - for (uint16_t j = 0; j < 676; j++) - add_coin(amt); - BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, 1, 1, 0, vCoins, setCoinsRet, nValueRet)); - if (amt - 2000 < MIN_CHANGE) { - // needs more than one input: - uint16_t returnSize = std::ceil((2000.0 + MIN_CHANGE)/amt); - CAmount returnValue = amt * returnSize; - BOOST_CHECK_EQUAL(nValueRet, returnValue); - BOOST_CHECK_EQUAL(setCoinsRet.size(), returnSize); - } else { - // one input is sufficient: - BOOST_CHECK_EQUAL(nValueRet, amt); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - } - } - - // test randomness - { - empty_wallet(); - for (int i2 = 0; i2 < 100; i2++) - add_coin(COIN); - - // picking 50 from 100 coins doesn't depend on the shuffle, - // but does depend on randomness in the stochastic approximation code - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); - - int fails = 0; - for (int j = 0; j < RANDOM_REPEATS; j++) - { - // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time - // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - - // add 75 cents in small change. not enough to make 90 cents, - // then try making 90 cents. there are multiple competing "smallest bigger" coins, - // one of which should be picked at random - add_coin(5 * CENT); - add_coin(10 * CENT); - add_coin(15 * CENT); - add_coin(20 * CENT); - add_coin(25 * CENT); - - fails = 0; - for (int j = 0; j < RANDOM_REPEATS; j++) - { - // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time - // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet , nValueRet)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, 1, 6, 0, vCoins, setCoinsRet2, nValueRet)); - if (equal_sets(setCoinsRet, setCoinsRet2)) - fails++; - } - BOOST_CHECK_NE(fails, RANDOM_REPEATS); - } - } - empty_wallet(); -} - -BOOST_AUTO_TEST_CASE(ApproximateBestSubset) -{ - CoinSet setCoinsRet; - CAmount nValueRet; - - LOCK(testWallet.cs_wallet); - - empty_wallet(); - - // Test vValue sort order - for (int i = 0; i < 1000; i++) - add_coin(1000 * COIN); - add_coin(3 * COIN); - - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet)); - BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); - BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); - - empty_wallet(); -} - static void AddKey(CWallet& wallet, const CKey& key) { LOCK(wallet.cs_wallet); @@ -553,7 +216,10 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 if (block) { wtx.SetMerkleBranch(block, 0); } - wallet.AddToWallet(wtx); + { + LOCK(cs_main); + wallet.AddToWallet(wtx); + } LOCK(wallet.cs_wallet); return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart; } @@ -624,23 +290,23 @@ public: CWalletTx& AddTx(CRecipient recipient) { - CWalletTx wtx; + CTransactionRef tx; CReserveKey reservekey(wallet.get()); CAmount fee; int changePos = -1; std::string error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, reservekey, fee, changePos, error, dummy)); CValidationState state; - BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + BOOST_CHECK(wallet->CommitTransaction(tx, {}, {}, {}, reservekey, nullptr, state)); CMutableTransaction blocktx; { LOCK(wallet->cs_wallet); - blocktx = CMutableTransaction(*wallet->mapWallet.at(wtx.GetHash()).tx); + blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx); } CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); - auto it = wallet->mapWallet.find(wtx.GetHash()); + auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e1f940e114..bd5094085e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -8,6 +8,7 @@ #include <checkpoints.h> #include <chain.h> #include <wallet/coincontrol.h> +#include <wallet/coinselection.h> #include <consensus/consensus.h> #include <consensus/validation.h> #include <fs.h> @@ -68,15 +69,6 @@ const uint256 CMerkleTx::ABANDON_HASH(uint256S("00000000000000000000000000000000 * @{ */ -struct CompareValueOnly -{ - bool operator()(const CInputCoin& t1, - const CInputCoin& t2) const - { - return t1.txout.nValue < t2.txout.nValue; - } -}; - std::string COutput::ToString() const { return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); @@ -531,7 +523,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran int nMinOrderPos = std::numeric_limits<int>::max(); const CWalletTx* copyFrom = nullptr; for (TxSpends::iterator it = range.first; it != range.second; ++it) { - const CWalletTx* wtx = &mapWallet[it->second]; + const CWalletTx* wtx = &mapWallet.at(it->second); if (wtx->nOrderPos < nMinOrderPos) { nMinOrderPos = wtx->nOrderPos;; copyFrom = wtx; @@ -544,7 +536,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran for (TxSpends::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; - CWalletTx* copyTo = &mapWallet[hash]; + CWalletTx* copyTo = &mapWallet.at(hash); if (copyFrom == copyTo) continue; assert(copyFrom && "Oldest wallet transaction in range assumed to have been found."); if (!copyFrom->IsEquivalentTo(*copyTo)) continue; @@ -1541,6 +1533,79 @@ int CWalletTx::GetRequestCount() const return nRequests; } +// Helper for producing a max-sized low-S signature (eg 72 bytes) +bool CWallet::DummySignInput(CTxIn &tx_in, const CTxOut &txout) const +{ + // Fill in dummy signatures for fee calculation. + const CScript& scriptPubKey = txout.scriptPubKey; + SignatureData sigdata; + + if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) + { + return false; + } else { + UpdateInput(tx_in, sigdata); + } + return true; +} + +// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) +bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts) const +{ + // Fill in dummy signatures for fee calculation. + int nIn = 0; + for (const auto& txout : txouts) + { + if (!DummySignInput(txNew.vin[nIn], txout)) { + return false; + } + + nIn++; + } + return true; +} + +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet) +{ + std::vector<CTxOut> txouts; + // Look up the inputs. We should have already checked that this transaction + // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our + // wallet, with a valid index into the vout array, and the ability to sign. + for (auto& input : tx.vin) { + const auto mi = wallet->mapWallet.find(input.prevout.hash); + if (mi == wallet->mapWallet.end()) { + return -1; + } + assert(input.prevout.n < mi->second.tx->vout.size()); + txouts.emplace_back(mi->second.tx->vout[input.prevout.n]); + } + return CalculateMaximumSignedTxSize(tx, wallet, txouts); +} + +// txouts needs to be in the order of tx.vin +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts) +{ + CMutableTransaction txNew(tx); + if (!wallet->DummySignTx(txNew, txouts)) { + // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) + // implies that we can sign for every input. + return -1; + } + return GetVirtualTransactionSize(txNew); +} + +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet) +{ + CMutableTransaction txn; + txn.vin.push_back(CTxIn(COutPoint())); + if (!wallet->DummySignInput(txn.vin[0], txout)) { + // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) + // implies that we can sign for every input. + return -1; + } + return GetVirtualTransactionInputSize(txn.vin[0]); +} + void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived, std::list<COutputEntry>& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const { @@ -2362,171 +2427,88 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -static void ApproximateBestSubset(const std::vector<CInputCoin>& vValue, const CAmount& nTotalLower, const CAmount& nTargetValue, - std::vector<char>& vfBest, CAmount& nBest, int iterations = 1000) +bool CWallet::OutputEligibleForSpending(const COutput& output, const CoinEligibilityFilter& eligibilty_filter) const { - std::vector<char> vfIncluded; + if (!output.fSpendable) + return false; - vfBest.assign(vValue.size(), true); - nBest = nTotalLower; + if (output.nDepth < (output.tx->IsFromMe(ISMINE_ALL) ? eligibilty_filter.conf_mine : eligibilty_filter.conf_theirs)) + return false; - FastRandomContext insecure_rand; + if (!mempool.TransactionWithinChainLimit(output.tx->GetHash(), eligibilty_filter.max_ancestors)) + return false; - for (int nRep = 0; nRep < iterations && nBest != nTargetValue; nRep++) - { - vfIncluded.assign(vValue.size(), false); - CAmount nTotal = 0; - bool fReachedTarget = false; - for (int nPass = 0; nPass < 2 && !fReachedTarget; nPass++) - { - for (unsigned int i = 0; i < vValue.size(); i++) - { - //The solver here uses a randomized algorithm, - //the randomness serves no real security purpose but is just - //needed to prevent degenerate behavior and it is important - //that the rng is fast. We do not use a constant random sequence, - //because there may be some privacy improvement by making - //the selection random. - if (nPass == 0 ? insecure_rand.randbool() : !vfIncluded[i]) - { - nTotal += vValue[i].txout.nValue; - vfIncluded[i] = true; - if (nTotal >= nTargetValue) - { - fReachedTarget = true; - if (nTotal < nBest) - { - nBest = nTotal; - vfBest = vfIncluded; - } - nTotal -= vValue[i].txout.nValue; - vfIncluded[i] = false; - } - } - } - } - } + return true; } -bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMine, const int nConfTheirs, const uint64_t nMaxAncestors, std::vector<COutput> vCoins, - std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) const +bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_filter, std::vector<COutput> vCoins, + std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { setCoinsRet.clear(); nValueRet = 0; - // List of values less than target - boost::optional<CInputCoin> coinLowestLarger; - std::vector<CInputCoin> vValue; - CAmount nTotalLower = 0; - - random_shuffle(vCoins.begin(), vCoins.end(), GetRandInt); - - for (const COutput &output : vCoins) - { - if (!output.fSpendable) - continue; - - const CWalletTx *pcoin = output.tx; - - if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs)) - continue; - - if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors)) - continue; + std::vector<CInputCoin> utxo_pool; + if (coin_selection_params.use_bnb) { - int i = output.i; + // Get long term estimate + FeeCalculation feeCalc; + CCoinControl temp; + temp.m_confirm_target = 1008; + CFeeRate long_term_feerate = GetMinimumFeeRate(temp, ::mempool, ::feeEstimator, &feeCalc); - CInputCoin coin = CInputCoin(pcoin, i); + // Calculate cost of change + CAmount cost_of_change = GetDiscardRate(::feeEstimator).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); - if (coin.txout.nValue == nTargetValue) - { - setCoinsRet.insert(coin); - nValueRet += coin.txout.nValue; - return true; - } - else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) - { - vValue.push_back(coin); - nTotalLower += coin.txout.nValue; - } - else if (!coinLowestLarger || coin.txout.nValue < coinLowestLarger->txout.nValue) - { - coinLowestLarger = coin; - } - } - - if (nTotalLower == nTargetValue) - { - for (const auto& input : vValue) + // Filter by the min conf specs and add to utxo_pool and calculate effective value + for (const COutput &output : vCoins) { - setCoinsRet.insert(input); - nValueRet += input.txout.nValue; - } - return true; - } - - if (nTotalLower < nTargetValue) - { - if (!coinLowestLarger) - return false; - setCoinsRet.insert(coinLowestLarger.get()); - nValueRet += coinLowestLarger->txout.nValue; - return true; - } - - // Solve subset sum by stochastic approximation - std::sort(vValue.begin(), vValue.end(), CompareValueOnly()); - std::reverse(vValue.begin(), vValue.end()); - std::vector<char> vfBest; - CAmount nBest; - - ApproximateBestSubset(vValue, nTotalLower, nTargetValue, vfBest, nBest); - if (nBest != nTargetValue && nTotalLower >= nTargetValue + MIN_CHANGE) - ApproximateBestSubset(vValue, nTotalLower, nTargetValue + MIN_CHANGE, vfBest, nBest); + if (!OutputEligibleForSpending(output, eligibilty_filter)) + continue; - // If we have a bigger coin and (either the stochastic approximation didn't find a good solution, - // or the next bigger coin is closer), return the bigger coin - if (coinLowestLarger && - ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || coinLowestLarger->txout.nValue <= nBest)) - { - setCoinsRet.insert(coinLowestLarger.get()); - nValueRet += coinLowestLarger->txout.nValue; - } - else { - for (unsigned int i = 0; i < vValue.size(); i++) - if (vfBest[i]) - { - setCoinsRet.insert(vValue[i]); - nValueRet += vValue[i].txout.nValue; + CInputCoin coin(output.tx->tx, output.i); + coin.effective_value = coin.txout.nValue - (output.nInputBytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(output.nInputBytes)); + // Only include outputs that are positive effective value (i.e. not dust) + if (coin.effective_value > 0) { + coin.fee = output.nInputBytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(output.nInputBytes); + coin.long_term_fee = output.nInputBytes < 0 ? 0 : long_term_feerate.GetFee(output.nInputBytes); + utxo_pool.push_back(coin); } + } + // Calculate the fees for things that aren't inputs + CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); + bnb_used = true; + return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); + } else { + // Filter by the min conf specs and add to utxo_pool + for (const COutput &output : vCoins) + { + if (!OutputEligibleForSpending(output, eligibilty_filter)) + continue; - if (LogAcceptCategory(BCLog::SELECTCOINS)) { - LogPrint(BCLog::SELECTCOINS, "SelectCoins() best subset: "); - for (unsigned int i = 0; i < vValue.size(); i++) { - if (vfBest[i]) { - LogPrint(BCLog::SELECTCOINS, "%s ", FormatMoney(vValue[i].txout.nValue)); - } - } - LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); + CInputCoin coin = CInputCoin(output.tx->tx, output.i); + utxo_pool.push_back(coin); } + bnb_used = false; + return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet); } - - return true; } -bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAmount& nTargetValue, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CCoinControl* coinControl) const +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 { std::vector<COutput> vCoins(vAvailableCoins); // coin control -> return all selected outputs (we want all selected to go into the transaction for sure) - if (coinControl && coinControl->HasSelected() && !coinControl->fAllowOtherInputs) + if (coin_control.HasSelected() && !coin_control.fAllowOtherInputs) { + // We didn't use BnB here, so set it to false. + bnb_used = false; + for (const COutput& out : vCoins) { if (!out.fSpendable) continue; nValueRet += out.tx->tx->vout[out.i].nValue; - setCoinsRet.insert(CInputCoin(out.tx, out.i)); + setCoinsRet.insert(CInputCoin(out.tx->tx, out.i)); } return (nValueRet >= nTargetValue); } @@ -2536,10 +2518,12 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm CAmount nValueFromPresetInputs = 0; std::vector<COutPoint> vPresetInputs; - if (coinControl) - coinControl->ListSelected(vPresetInputs); + coin_control.ListSelected(vPresetInputs); for (const COutPoint& outpoint : vPresetInputs) { + // For now, don't use BnB if preset inputs are selected. TODO: Enable this later + bnb_used = false; + std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(outpoint.hash); if (it != mapWallet.end()) { @@ -2547,16 +2531,17 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm // Clearly invalid input, fail if (pcoin->tx->vout.size() <= outpoint.n) return false; + // Just to calculate the marginal byte size nValueFromPresetInputs += pcoin->tx->vout[outpoint.n].nValue; - setPresetCoins.insert(CInputCoin(pcoin, outpoint.n)); + setPresetCoins.insert(CInputCoin(pcoin->tx, outpoint.n)); } else return false; // TODO: Allow non-wallet inputs } // remove preset inputs from vCoins - for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coinControl && coinControl->HasSelected();) + for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();) { - if (setPresetCoins.count(CInputCoin(it->tx, it->i))) + if (setPresetCoins.count(CInputCoin(it->tx->tx, it->i))) it = vCoins.erase(it); else ++it; @@ -2566,13 +2551,13 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); bool res = nTargetValue <= nValueFromPresetInputs || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 6, 0, vCoins, setCoinsRet, nValueRet) || - SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 1, 1, 0, vCoins, setCoinsRet, nValueRet) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, 2, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::min((size_t)4, nMaxChainLength/3), vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength/2, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, nMaxChainLength, vCoins, setCoinsRet, nValueRet)) || - (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, 0, 1, std::numeric_limits<uint64_t>::max(), vCoins, setCoinsRet, nValueRet)); + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::min((size_t)4, nMaxChainLength/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, nMaxChainLength), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (bSpendZeroConfChange && !fRejectLongChains && SelectCoinsMinConf(nTargetValue - nValueFromPresetInputs, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max()), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset setCoinsRet.insert(setPresetCoins.begin(), setPresetCoins.end()); @@ -2629,13 +2614,13 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC LOCK2(cs_main, cs_wallet); CReserveKey reservekey(this); - CWalletTx wtx; - if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { + CTransactionRef tx_new; + if (!CreateTransaction(vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { return false; } if (nChangePosInOut != -1) { - tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]); + tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]); // We don't have the normal Create/Commit cycle, and don't want to risk // reusing change, so just remove the key from the keypool here. reservekey.KeepKey(); @@ -2644,11 +2629,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC // Copy output sizes from new transaction; they may have had the fee // subtracted from them. for (unsigned int idx = 0; idx < tx.vout.size(); idx++) { - tx.vout[idx].nValue = wtx.tx->vout[idx].nValue; + tx.vout[idx].nValue = tx_new->vout[idx].nValue; } // Add new txins while keeping original txin scriptSig/order. - for (const CTxIn& txin : wtx.tx->vin) { + for (const CTxIn& txin : tx_new->vin) { if (!coinControl.IsSelected(txin.prevout)) { tx.vin.push_back(txin); @@ -2689,7 +2674,7 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec return g_address_type; } -bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, +bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign) { CAmount nValue = 0; @@ -2713,8 +2698,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT return false; } - wtxNew.fTimeReceivedIsTxTime = true; - wtxNew.BindWallet(this); CMutableTransaction txNew; // Discourage fee sniping. @@ -2750,13 +2733,14 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT assert(txNew.nLockTime < LOCKTIME_THRESHOLD); FeeCalculation feeCalc; CAmount nFeeNeeded; - unsigned int nBytes; + int nBytes; { std::set<CInputCoin> setCoins; LOCK2(cs_main, cs_wallet); { std::vector<COutput> vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coin_control); + CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy // Create change script that will be used if we need change // TODO: pass in scriptChange instead of reservekey so @@ -2790,25 +2774,34 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT scriptChange = GetScriptForDestination(GetDestinationForKey(vchPubKey, change_type)); } CTxOut change_prototype_txout(0, scriptChange); - size_t change_prototype_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); + coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout, SER_DISK, 0); CFeeRate discard_rate = GetDiscardRate(::feeEstimator); + + // Get the fee rate to use effective values in coin selection + CFeeRate nFeeRateNeeded = GetMinimumFeeRate(coin_control, ::mempool, ::feeEstimator, &feeCalc); + nFeeRet = 0; bool pick_new_inputs = true; CAmount nValueIn = 0; + + // BnB selector is the only selector used when this is true. + // That should only happen on the first pass through the loop. + coin_selection_params.use_bnb = nSubtractFeeFromAmount == 0; // If we are doing subtract fee from recipient, then don't use BnB // Start with no fee and loop until there is enough fee while (true) { nChangePosInOut = nChangePosRequest; txNew.vin.clear(); txNew.vout.clear(); - wtxNew.fFromMe = true; bool fFirst = true; CAmount nValueToSelect = nValue; if (nSubtractFeeFromAmount == 0) nValueToSelect += nFeeRet; + // vouts to the payees + coin_selection_params.tx_noinputs_size = 11; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 output count, 1 witness overhead (dummy, flag, stack size) for (const auto& recipient : vecSend) { CTxOut txout(recipient.nAmount, recipient.scriptPubKey); @@ -2824,6 +2817,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT txout.nValue -= nFeeRet % nSubtractFeeFromAmount; } } + // Include the fee cost for outputs. Note this is only used for BnB right now + coin_selection_params.tx_noinputs_size += ::GetSerializeSize(txout, SER_NETWORK, PROTOCOL_VERSION); if (IsDust(txout, ::dustRelayFee)) { @@ -2842,18 +2837,27 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT } // Choose coins to use + bool bnb_used; if (pick_new_inputs) { nValueIn = 0; setCoins.clear(); - if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, &coin_control)) + coin_selection_params.change_spend_size = CalculateMaximumSignedInputSize(change_prototype_txout, this); + coin_selection_params.effective_fee = nFeeRateNeeded; + if (!SelectCoins(vAvailableCoins, nValueToSelect, setCoins, nValueIn, coin_control, coin_selection_params, bnb_used)) { - strFailReason = _("Insufficient funds"); - return false; + // If BnB was used, it was the first pass. No longer the first pass and continue loop with knapsack. + if (bnb_used) { + coin_selection_params.use_bnb = false; + continue; + } + else { + strFailReason = _("Insufficient funds"); + return false; + } } } const CAmount nChange = nValueIn - nValueToSelect; - if (nChange > 0) { // Fill a vout to ourself @@ -2861,7 +2865,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // Never create dust outputs; if we would, just // add the dust to the fee. - if (IsDust(newTxOut, discard_rate)) + // The nChange when BnB is used is always going to go to fees. + if (IsDust(newTxOut, discard_rate) || bnb_used) { nChangePosInOut = -1; nFeeRet += nChange; @@ -2901,20 +2906,12 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT txNew.vin.push_back(CTxIn(coin.outpoint,CScript(), nSequence)); - // Fill in dummy signatures for fee calculation. - if (!DummySignTx(txNew, setCoins)) { + nBytes = CalculateMaximumSignedTxSize(txNew, this); + if (nBytes < 0) { strFailReason = _("Signing transaction failed"); return false; } - nBytes = GetVirtualTransactionSize(txNew); - - // Remove scriptSigs to eliminate the fee calculation dummy signatures - for (auto& vin : txNew.vin) { - vin.scriptSig = CScript(); - vin.scriptWitness.SetNull(); - } - nFeeNeeded = GetMinimumFee(nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc); if (feeCalc.reason == FeeReason::FALLBACK && !g_wallet_allow_fallback_fee) { // eventually allow a fallback fee @@ -2942,7 +2939,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // (because of reduced tx size) and so we should add a // change output. Only try this once. if (nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { - unsigned int tx_size_with_change = nBytes + change_prototype_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size + unsigned int tx_size_with_change = nBytes + coin_selection_params.change_output_size + 2; // Add 2 as a buffer in case increasing # of outputs changes compact size CAmount fee_needed_with_change = GetMinimumFee(tx_size_with_change, coin_control, ::mempool, ::feeEstimator, nullptr); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, discard_rate); if (nFeeRet >= fee_needed_with_change + minimum_value_for_change) { @@ -2990,6 +2987,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT // Include more fee and try again. nFeeRet = nFeeNeeded; + coin_selection_params.use_bnb = false; continue; } } @@ -3017,11 +3015,11 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT } } - // Embed the constructed transaction data in wtxNew. - wtxNew.SetTx(MakeTransactionRef(std::move(txNew))); + // Return the constructed transaction data. + tx = MakeTransactionRef(std::move(txNew)); // Limit size - if (GetTransactionWeight(*wtxNew.tx) >= MAX_STANDARD_TX_WEIGHT) + if (GetTransactionWeight(*tx) >= MAX_STANDARD_TX_WEIGHT) { strFailReason = _("Transaction too large"); return false; @@ -3031,7 +3029,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits LockPoints lp; - CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); CTxMemPool::setEntries setAncestors; size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT); size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000; @@ -3058,10 +3056,18 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT /** * Call after CreateTransaction unless you want to abort */ -bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state) +bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state) { { LOCK2(cs_main, cs_wallet); + + CWalletTx wtxNew(this, std::move(tx)); + wtxNew.mapValue = std::move(mapValue); + wtxNew.vOrderForm = std::move(orderForm); + wtxNew.strFromAccount = std::move(fromAccount); + wtxNew.fTimeReceivedIsTxTime = true; + wtxNew.fFromMe = true; + LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString()); { // Take key pair from key pool so it won't be used again @@ -3074,7 +3080,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon // Notify that old coins are spent for (const CTxIn& txin : wtxNew.tx->vin) { - CWalletTx &coin = mapWallet[txin.prevout.hash]; + CWalletTx &coin = mapWallet.at(txin.prevout.hash); coin.BindWallet(this); NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED); } @@ -3085,7 +3091,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon // Get the inserted-CWalletTx from mapWallet so that the // fInMempool flag is cached properly - CWalletTx& wtx = mapWallet[wtxNew.GetHash()]; + CWalletTx& wtx = mapWallet.at(wtxNew.GetHash()); if (fBroadcastTransactions) { @@ -3541,7 +3547,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings() CTxDestination address; if(!IsMine(txin)) /* If this input isn't mine, ignore it */ continue; - if(!ExtractDestination(mapWallet[txin.prevout.hash].tx->vout[txin.prevout.n].scriptPubKey, address)) + if(!ExtractDestination(mapWallet.at(txin.prevout.hash).tx->vout[txin.prevout.n].scriptPubKey, address)) continue; grouping.insert(address); any_mine = true; @@ -3813,12 +3819,7 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const { unsigned int nTimeSmart = wtx.nTimeReceived; if (!wtx.hashUnset()) { - const CBlockIndex* pindex = nullptr; - { - LOCK(cs_main); - pindex = LookupBlockIndex(wtx.hashBlock); - } - if (pindex) { + if (const CBlockIndex* pindex = LookupBlockIndex(wtx.hashBlock)) { int64_t latestNow = wtx.nTimeReceived; int64_t latestEntry = 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0f29bff1ff..dc0e8d07d7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -17,6 +17,7 @@ #include <script/sign.h> #include <util.h> #include <wallet/crypter.h> +#include <wallet/coinselection.h> #include <wallet/walletdb.h> #include <wallet/rpcwallet.h> @@ -53,10 +54,6 @@ static const CAmount DEFAULT_DISCARD_FEE = 10000; static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; //! minimum recommended increment for BIP 125 replacement txs static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; -//! target minimum change amount -static const CAmount MIN_CHANGE = CENT; -//! final minimum change amount after paying for fees -static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; //! Default for -walletrejectlongchains @@ -269,6 +266,9 @@ public: bool IsCoinBase() const { return tx->IsCoinBase(); } }; +//Get the marginal bytes of spending the specified output +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet); + /** * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. @@ -348,11 +348,6 @@ public: mutable CAmount nAvailableWatchCreditCached; mutable CAmount nChangeCached; - CWalletTx() - { - Init(nullptr); - } - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg)) { Init(pwalletIn); @@ -456,6 +451,12 @@ public: CAmount GetAvailableWatchOnlyCredit(const bool fUseCache=true) const; CAmount GetChange() const; + // Get the marginal bytes if spending the specified output from this transaction + int GetSpendSize(unsigned int out) const + { + return CalculateMaximumSignedInputSize(tx->vout[out], pwallet); + } + void GetAmounts(std::list<COutputEntry>& listReceived, std::list<COutputEntry>& listSent, CAmount& nFee, std::string& strSentAccount, const isminefilter& filter) const; @@ -482,36 +483,6 @@ public: std::set<uint256> GetConflicts() const; }; - -class CInputCoin { -public: - CInputCoin(const CWalletTx* walletTx, unsigned int i) - { - if (!walletTx) - throw std::invalid_argument("walletTx should not be null"); - if (i >= walletTx->tx->vout.size()) - throw std::out_of_range("The output index is out of range"); - - outpoint = COutPoint(walletTx->GetHash(), i); - txout = walletTx->tx->vout[i]; - } - - COutPoint outpoint; - CTxOut txout; - - bool operator<(const CInputCoin& rhs) const { - return outpoint < rhs.outpoint; - } - - bool operator!=(const CInputCoin& rhs) const { - return outpoint != rhs.outpoint; - } - - bool operator==(const CInputCoin& rhs) const { - return outpoint == rhs.outpoint; - } -}; - class COutput { public: @@ -519,6 +490,9 @@ public: int i; int nDepth; + /** Pre-computed estimated size of this output as a fully-signed input in a transaction. Can be -1 if it could not be calculated */ + int nInputBytes; + /** Whether we have the private keys to spend this output */ bool fSpendable; @@ -534,7 +508,12 @@ public: COutput(const CWalletTx *txIn, int iIn, int nDepthIn, bool fSpendableIn, bool fSolvableIn, bool fSafeIn) { - tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; + tx = txIn; i = iIn; nDepth = nDepthIn; fSpendable = fSpendableIn; fSolvable = fSolvableIn; fSafe = fSafeIn; nInputBytes = -1; + // If known and signable by the given wallet, compute nInputBytes + // Failure will keep this value -1 + if (fSpendable && tx) { + nInputBytes = tx->GetSpendSize(i); + } } std::string ToString() const; @@ -653,6 +632,26 @@ private: std::vector<char> _ssExtra; }; +struct CoinSelectionParams +{ + bool use_bnb = true; + size_t change_output_size = 0; + size_t change_spend_size = 0; + CFeeRate effective_fee = CFeeRate(0); + size_t tx_noinputs_size = 0; + + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), tx_noinputs_size(tx_noinputs_size) {} + CoinSelectionParams() {} +}; + +struct CoinEligibilityFilter +{ + const int conf_mine; + const int conf_theirs; + const uint64_t max_ancestors; + + CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors) {} +}; class WalletRescanReserver; //forward declarations for ScanForWalletTransactions/RescanFromTime /** @@ -674,7 +673,8 @@ private: * 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 *coinControl = nullptr) const; + 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; @@ -855,7 +855,8 @@ public: * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, uint64_t nMaxAncestors, std::vector<COutput> vCoins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet) const; + bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibilty_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; @@ -969,15 +970,21 @@ public: * selected by SelectCoins(); Also create the change output, when needed * @note passing nChangePosInOut as -1 will result in setting a random position */ - bool CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, + bool CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true); - bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state); + bool CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state); void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries); bool AddAccountingEntry(const CAccountingEntry&); bool AddAccountingEntry(const CAccountingEntry&, CWalletDB *pwalletdb); - template <typename ContainerType> - bool DummySignTx(CMutableTransaction &txNew, const ContainerType &coins) const; + bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts) const + { + std::vector<CTxOut> v_txouts(txouts.size()); + std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); + return DummySignTx(txNew, v_txouts); + } + bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts) const; + bool DummySignInput(CTxIn &tx_in, const CTxOut &txout) const; static CFeeRate minTxFee; static CFeeRate fallbackFee; @@ -1158,6 +1165,9 @@ public: * This function will automatically add the necessary scripts to the wallet. */ 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; }; /** A key allocated from the key pool. */ @@ -1222,31 +1232,6 @@ public: } }; -// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) -// ContainerType is meant to hold pair<CWalletTx *, int>, and be iterable -// so that each entry corresponds to each vIn, in order. -template <typename ContainerType> -bool CWallet::DummySignTx(CMutableTransaction &txNew, const ContainerType &coins) const -{ - // Fill in dummy signatures for fee calculation. - int nIn = 0; - for (const auto& coin : coins) - { - const CScript& scriptPubKey = coin.txout.scriptPubKey; - SignatureData sigdata; - - if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) - { - return false; - } else { - UpdateTransaction(txNew, nIn, sigdata); - } - - nIn++; - } - return true; -} - OutputType ParseOutputType(const std::string& str, OutputType default_type = OUTPUT_TYPE_DEFAULT); const std::string& FormatOutputType(OutputType type); @@ -1294,4 +1279,10 @@ public: } }; +// Calculate the size of the transaction assuming all signatures are max size +// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. +// NOTE: this requires that all inputs must be in mapWallet (eg the tx should +// be IsAllFromMe). +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet); +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts); #endif // BITCOIN_WALLET_WALLET_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 0b0880a2ba..7f5f3b84b2 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -265,7 +265,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, { uint256 hash; ssKey >> hash; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); ssValue >> wtx; CValidationState state; if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid())) @@ -603,7 +603,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) pwallet->UpdateTimeFirstKey(1); for (uint256 hash : wss.vWalletUpgrade) - WriteTx(pwallet->mapWallet[hash]); + WriteTx(pwallet->mapWallet.at(hash)); // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc: if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000)) @@ -664,7 +664,7 @@ DBErrors CWalletDB::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CWal uint256 hash; ssKey >> hash; - CWalletTx wtx; + CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef()); ssValue >> wtx; vTxHash.push_back(hash); diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index a9e14d3e3c..17e24453e5 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -32,6 +32,18 @@ from test_framework.util import ( assert_is_hex_string, assert_is_hash_string, ) +from test_framework.blocktools import ( + create_block, + create_coinbase, +) +from test_framework.messages import ( + msg_block, +) +from test_framework.mininode import ( + P2PInterface, + network_thread_start, +) + class BlockchainTest(BitcoinTestFramework): def set_test_params(self): @@ -46,6 +58,7 @@ class BlockchainTest(BitcoinTestFramework): self._test_getdifficulty() self._test_getnetworkhashps() self._test_stopatheight() + self._test_waitforblockheight() assert self.nodes[0].verifychain(4, 0) def _test_getblockchaininfo(self): @@ -241,6 +254,50 @@ class BlockchainTest(BitcoinTestFramework): self.start_node(0) assert_equal(self.nodes[0].getblockcount(), 207) + def _test_waitforblockheight(self): + self.log.info("Test waitforblockheight") + + node = self.nodes[0] + + # Start a P2P connection since we'll need to create some blocks. + node.add_p2p_connection(P2PInterface()) + network_thread_start() + node.p2p.wait_for_verack() + + current_height = node.getblock(node.getbestblockhash())['height'] + + # Create a fork somewhere below our current height, invalidate the tip + # of that fork, and then ensure that waitforblockheight still + # works as expected. + # + # (Previously this was broken based on setting + # `rpc/blockchain.cpp:latestblock` incorrectly.) + # + b20hash = node.getblockhash(20) + b20 = node.getblock(b20hash) + + def solve_and_send_block(prevhash, height, time): + b = create_block(prevhash, create_coinbase(height), time) + b.solve() + node.p2p.send_message(msg_block(b)) + node.p2p.sync_with_ping() + return b + + b21f = solve_and_send_block(int(b20hash, 16), 21, b20['time'] + 1) + b22f = solve_and_send_block(b21f.sha256, 22, b21f.nTime + 1) + + node.invalidateblock(b22f.hash) + + def assert_waitforheight(height, timeout=2): + assert_equal( + node.waitforblockheight(height, timeout)['height'], + current_height) + + assert_waitforheight(0) + assert_waitforheight(current_height - 1) + assert_waitforheight(current_height) + assert_waitforheight(current_height + 1) + if __name__ == '__main__': BlockchainTest().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index f4e77d32dc..8efac9c475 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -24,8 +24,8 @@ from .util import ( check_json_precision, connect_nodes_bi, disconnect_nodes, + get_datadir_path, initialize_datadir, - log_filename, p2p_port, set_node_times, sync_blocks, @@ -382,7 +382,7 @@ class BitcoinTestFramework(): assert self.num_nodes <= MAX_NODES create_cache = False for i in range(MAX_NODES): - if not os.path.isdir(os.path.join(self.options.cachedir, 'node' + str(i))): + if not os.path.isdir(get_datadir_path(self.options.cachedir, i)): create_cache = True break @@ -391,8 +391,8 @@ class BitcoinTestFramework(): # find and delete old cache directories if any exist for i in range(MAX_NODES): - if os.path.isdir(os.path.join(self.options.cachedir, "node" + str(i))): - shutil.rmtree(os.path.join(self.options.cachedir, "node" + str(i))) + if os.path.isdir(get_datadir_path(self.options.cachedir, i)): + shutil.rmtree(get_datadir_path(self.options.cachedir, i)) # Create cache directories, run bitcoinds: for i in range(MAX_NODES): @@ -430,15 +430,18 @@ class BitcoinTestFramework(): self.stop_nodes() self.nodes = [] self.disable_mocktime() + + def cache_path(n, *paths): + return os.path.join(get_datadir_path(self.options.cachedir, n), "regtest", *paths) + for i in range(MAX_NODES): - os.remove(log_filename(self.options.cachedir, i, "debug.log")) - os.remove(log_filename(self.options.cachedir, i, "wallets/db.log")) - os.remove(log_filename(self.options.cachedir, i, "peers.dat")) - os.remove(log_filename(self.options.cachedir, i, "fee_estimates.dat")) + for entry in os.listdir(cache_path(i)): + if entry not in ['wallets', 'chainstate', 'blocks']: + os.remove(cache_path(i, entry)) for i in range(self.num_nodes): - from_dir = os.path.join(self.options.cachedir, "node" + str(i)) - to_dir = os.path.join(self.options.tmpdir, "node" + str(i)) + from_dir = get_datadir_path(self.options.cachedir, i) + to_dir = get_datadir_path(self.options.tmpdir, i) shutil.copytree(from_dir, to_dir) initialize_datadir(self.options.tmpdir, i) # Overwrite port/rpcport in bitcoin.conf diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 0c7be67a57..7be695550b 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -328,9 +328,6 @@ def get_auth_cookie(datadir): raise ValueError("No RPC credentials") return user, password -def log_filename(dirname, n_node, logname): - return os.path.join(dirname, "node" + str(n_node), "regtest", logname) - def get_bip9_status(node, key): info = node.getblockchaininfo() return info['bip9_softforks'][key] |