diff --git a/src/Makefile.qttest.include b/src/Makefile.qttest.include --- a/src/Makefile.qttest.include +++ b/src/Makefile.qttest.include @@ -50,7 +50,8 @@ if ENABLE_WALLET qt_test_test_bitcoin_qt_SOURCES += \ qt/test/paymentservertests.cpp \ - qt/test/wallettests.cpp + qt/test/wallettests.cpp \ + wallet/test/wallet_test_fixture.cpp endif nodist_qt_test_test_bitcoin_qt_SOURCES = $(TEST_QT_MOC_CPP) diff --git a/src/qt/test/CMakeLists.txt b/src/qt/test/CMakeLists.txt --- a/src/qt/test/CMakeLists.txt +++ b/src/qt/test/CMakeLists.txt @@ -27,6 +27,7 @@ PRIVATE paymentservertests.cpp wallettests.cpp + ../../wallet/test/wallet_test_fixture.cpp ) target_link_libraries(test_bitcoin-qt wallet) diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -69,7 +69,6 @@ } return {}; } -} //! Simple qt wallet tests. // @@ -84,7 +83,7 @@ // src/qt/test/test_bitcoin-qt -platform xcb # Linux // src/qt/test/test_bitcoin-qt -platform windows # Windows // src/qt/test/test_bitcoin-qt -platform cocoa # macOS -void WalletTests::walletTests() { +void TestSendCoins() { #ifdef Q_OS_MAC if (QApplication::platformName() == "minimal") { // Disable for mac on "minimal" platform to avoid crashes inside the Qt @@ -144,3 +143,9 @@ bitdb.Flush(true); bitdb.Reset(); } + +} + +void WalletTests::walletTests() { + TestSendCoins(); +} diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -58,16 +58,7 @@ Amount WalletModel::getBalance(const CCoinControl *coinControl) const { if (coinControl) { - Amount nBalance = Amount::zero(); - std::vector vCoins; - wallet->AvailableCoins(vCoins, true, coinControl); - for (const COutput &out : vCoins) { - if (out.fSpendable) { - nBalance += out.tx->tx->vout[out.i].nValue; - } - } - - return nBalance; + return wallet->GetAvailableBalance(coinControl); } return wallet->GetBalance(); @@ -586,59 +577,13 @@ // group with wallet address) void WalletModel::listCoins( std::map> &mapCoins) const { - std::vector vCoins; - wallet->AvailableCoins(vCoins); - - // ListLockedCoins, mapWallet - LOCK2(cs_main, wallet->cs_wallet); - std::vector vLockedCoins; - wallet->ListLockedCoins(vLockedCoins); - - // add locked coins - for (const COutPoint &outpoint : vLockedCoins) { - if (!wallet->mapWallet.count(outpoint.GetTxId())) { - continue; - } - int nDepth = - wallet->mapWallet[outpoint.GetTxId()].GetDepthInMainChain(); - if (nDepth < 0) { - continue; - } - COutput out(&wallet->mapWallet[outpoint.GetTxId()], outpoint.GetN(), - nDepth, true /* spendable */, true /* solvable */, - true /* safe */); - if (outpoint.GetN() < out.tx->tx->vout.size() && - wallet->IsMine(out.tx->tx->vout[outpoint.GetN()]) == - ISMINE_SPENDABLE) { - vCoins.push_back(out); + for (auto &group : wallet->ListCoins()) { + auto &resultGroup = + mapCoins[QString::fromStdString(EncodeDestination(group.first))]; + for (auto &coin : group.second) { + resultGroup.emplace_back(std::move(coin)); } } - - for (const COutput &out : vCoins) { - COutput cout = out; - - while (wallet->IsChange(cout.tx->tx->vout[cout.i]) && - cout.tx->tx->vin.size() > 0 && - wallet->IsMine(cout.tx->tx->vin[0])) { - if (!wallet->mapWallet.count( - cout.tx->tx->vin[0].prevout.GetTxId())) { - break; - } - cout = COutput( - &wallet->mapWallet[cout.tx->tx->vin[0].prevout.GetTxId()], - cout.tx->tx->vin[0].prevout.GetN(), 0 /* depth */, - true /* spendable */, true /* solvable */, true /* safe */); - } - - CTxDestination address; - if (!out.fSpendable || - !ExtractDestination(cout.tx->tx->vout[cout.i].scriptPubKey, - address)) { - continue; - } - mapCoins[QString::fromStdString(EncodeDestination(address))].push_back( - out); - } } bool WalletModel::isLockedCoin(const TxId &txid, uint32_t n) const { @@ -663,17 +608,8 @@ void WalletModel::loadReceiveRequests( std::vector &vReceiveRequests) { - LOCK(wallet->cs_wallet); - for (const std::pair &item : - wallet->mapAddressBook) { - for (const std::pair &item2 : - item.second.destdata) { - if (item2.first.size() > 2 && item2.first.substr(0, 2) == "rr") { - // receive request - vReceiveRequests.push_back(item2.second); - } - } - } + // receive request + vReceiveRequests = wallet->GetDestValues("rr"); } bool WalletModel::saveReceiveRequest(const std::string &sAddress, @@ -692,14 +628,7 @@ } bool WalletModel::transactionCanBeAbandoned(const TxId &txid) const { - LOCK2(cs_main, wallet->cs_wallet); - const CWalletTx *wtx = wallet->GetWalletTx(txid); - if (!wtx || wtx->isAbandoned() || wtx->GetDepthInMainChain() > 0 || - wtx->InMempool()) { - return false; - } - - return true; + return wallet->TransactionCanBeAbandoned(txid); } bool WalletModel::abandonTransaction(const TxId &txid) const { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -6,6 +6,8 @@ #include "chainparams.h" #include "config.h" + +#include "consensus/validation.h" #include "rpc/server.h" #include "test/test_bitcoin.h" #include "validation.h" @@ -681,4 +683,107 @@ SetMockTime(0); } +BOOST_AUTO_TEST_CASE(LoadReceiveRequests) { + CTxDestination dest = CKeyID(); + pwalletMain->AddDestData(dest, "misc", "val_misc"); + pwalletMain->AddDestData(dest, "rr0", "val_rr0"); + pwalletMain->AddDestData(dest, "rr1", "val_rr1"); + + auto values = pwalletMain->GetDestValues("rr"); + BOOST_CHECK_EQUAL(values.size(), 2); + BOOST_CHECK_EQUAL(values[0], "val_rr0"); + BOOST_CHECK_EQUAL(values[1], "val_rr1"); +} + +class ListCoinsTestingSetup : public TestChain100Setup { +public: + ListCoinsTestingSetup() { + CreateAndProcessBlock({}, + GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + ::bitdb.MakeMock(); + wallet.reset(new CWallet( + Params(), std::unique_ptr( + new CWalletDBWrapper(&bitdb, "wallet_test.dat")))); + bool firstRun; + wallet->LoadWallet(firstRun); + LOCK(wallet->cs_wallet); + wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); + wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr); + } + + ~ListCoinsTestingSetup() { + wallet.reset(); + ::bitdb.Flush(true); + ::bitdb.Reset(); + } + + CWalletTx &AddTx(CRecipient recipient) { + CWalletTx wtx; + CReserveKey reservekey(wallet.get()); + Amount fee; + int changePos = -1; + std::string error; + BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, + changePos, error)); + CValidationState state; + BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + auto it = wallet->mapWallet.find(wtx.GetId()); + BOOST_CHECK(it != wallet->mapWallet.end()); + CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, + GetScriptForRawPubKey(coinbaseKey.GetPubKey())); + it->second.SetMerkleBranch(chainActive.Tip(), 1); + return it->second; + } + + std::unique_ptr wallet; +}; + +BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) { + std::string coinbaseAddress = coinbaseKey.GetPubKey().GetID().ToString(); + LOCK(wallet->cs_wallet); + + // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey + // address. + auto list = wallet->ListCoins(); + BOOST_CHECK_EQUAL(list.size(), 1); + BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), + coinbaseAddress); + BOOST_CHECK_EQUAL(list.begin()->second.size(), 1); + + // Check initial balance from one mature coinbase transaction. + BOOST_CHECK_EQUAL(50 * COIN, wallet->GetAvailableBalance()); + + // Add a transaction creating a change address, and confirm ListCoins still + // returns the coin associated with the change address underneath the + // coinbaseKey pubkey, even though the change address has a different + // pubkey. + AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, + false /* subtract fee */}); + list = wallet->ListCoins(); + BOOST_CHECK_EQUAL(list.size(), 1); + BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), + coinbaseAddress); + BOOST_CHECK_EQUAL(list.begin()->second.size(), 2); + + // Lock both coins. Confirm number of available coins drops to 0. + std::vector available; + wallet->AvailableCoins(available); + BOOST_CHECK_EQUAL(available.size(), 2); + for (const auto &group : list) { + for (const auto &coin : group.second) { + wallet->LockCoin(COutPoint(coin.tx->GetId(), coin.i)); + } + } + wallet->AvailableCoins(available); + BOOST_CHECK_EQUAL(available.size(), 0); + + // Confirm ListCoins still returns same result as before, despite coins + // being locked. + list = wallet->ListCoins(); + BOOST_CHECK_EQUAL(list.size(), 1); + BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), + coinbaseAddress); + BOOST_CHECK_EQUAL(list.begin()->second.size(), 2); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -832,6 +832,18 @@ const int &nMinDepth = 0, const int &nMaxDepth = 9999999) const; + /** + * Return list of available coins and locked coins grouped by non-change + * output address. + */ + std::map> ListCoins() const; + + /** + * Find non-change parent output. + */ + const CTxOut &FindNonChangeParentOutput(const CTransaction &tx, + int output) const; + /** * Shuffle and select coins until nTargetValue is reached while avoiding * small change; This method is stochastic for some inputs and upon @@ -850,7 +862,7 @@ void LockCoin(const COutPoint &output); void UnlockCoin(const COutPoint &output); void UnlockAllCoins(); - void ListLockedCoins(std::vector &vOutpts); + void ListLockedCoins(std::vector &vOutpts) const; /* * Rescan abort properties @@ -907,6 +919,8 @@ //! false otherwise bool GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const; + //! Get all destination values matching a prefix. + std::vector GetDestValues(const std::string &prefix) const; //! Adds a watch-only address to the store, and saves it to disk. bool AddWatchOnly(const CScript &dest, int64_t nCreateTime); @@ -972,6 +986,7 @@ Amount GetImmatureWatchOnlyBalance() const; Amount GetLegacyBalance(const isminefilter &filter, int minDepth, const std::string *account) const; + Amount GetAvailableBalance(const CCoinControl *coinControl = nullptr) const; /** * Insert additional inputs into the transaction by calling @@ -1140,6 +1155,9 @@ fBroadcastTransactions = broadcast; } + /** Return whether transaction can be abandoned */ + bool TransactionCanBeAbandoned(const TxId &txid) const; + /** * Mark a transaction (and it in-wallet descendants) as abandoned so its * inputs may be respent. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1116,6 +1116,13 @@ return false; } +bool CWallet::TransactionCanBeAbandoned(const TxId &txid) const { + LOCK2(cs_main, cs_wallet); + const CWalletTx *wtx = GetWalletTx(txid); + return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() <= 0 && + !wtx->InMempool(); +} + bool CWallet::AbandonTransaction(const TxId &txid) { LOCK2(cs_main, cs_wallet); @@ -2260,6 +2267,20 @@ return balance; } +Amount CWallet::GetAvailableBalance(const CCoinControl *coinControl) const { + LOCK2(cs_main, cs_wallet); + + Amount balance = Amount::zero(); + std::vector vCoins; + AvailableCoins(vCoins, true, coinControl); + for (const COutput &out : vCoins) { + if (out.fSpendable) { + balance += out.tx->tx->vout[out.i].nValue; + } + } + return balance; +} + void CWallet::AvailableCoins(std::vector &vCoins, bool fOnlySafe, const CCoinControl *coinControl, const Amount nMinimumAmount, @@ -2379,6 +2400,76 @@ } } +std::map> CWallet::ListCoins() const { + // TODO: Add AssertLockHeld(cs_wallet) here. + // + // Because the return value from this function contains pointers to + // CWalletTx objects, callers to this function really should acquire the + // cs_wallet lock before calling it. However, the current caller doesn't + // acquire this lock yet. There was an attempt to add the missing lock in + // https://github.com/bitcoin/bitcoin/pull/10340, but that change has been + // postponed until after https://github.com/bitcoin/bitcoin/pull/10244 to + // avoid adding some extra complexity to the Qt code. + + std::map> result; + + std::vector availableCoins; + AvailableCoins(availableCoins); + + LOCK2(cs_main, cs_wallet); + for (auto &coin : availableCoins) { + CTxDestination address; + if (coin.fSpendable && + ExtractDestination( + FindNonChangeParentOutput(*coin.tx->tx, coin.i).scriptPubKey, + address)) { + result[address].emplace_back(std::move(coin)); + } + } + + std::vector lockedCoins; + ListLockedCoins(lockedCoins); + for (const auto &output : lockedCoins) { + auto it = mapWallet.find(output.GetTxId()); + if (it != mapWallet.end()) { + int depth = it->second.GetDepthInMainChain(); + if (depth >= 0 && output.GetN() < it->second.tx->vout.size() && + IsMine(it->second.tx->vout[output.GetN()]) == + ISMINE_SPENDABLE) { + CTxDestination address; + if (ExtractDestination( + FindNonChangeParentOutput(*it->second.tx, output.GetN()) + .scriptPubKey, + address)) { + result[address].emplace_back( + &it->second, output.GetN(), depth, true /* spendable */, + true /* solvable */, false /* safe */); + } + } + } + } + + return result; +} + +const CTxOut &CWallet::FindNonChangeParentOutput(const CTransaction &tx, + int output) const { + const CTransaction *ptx = &tx; + int n = output; + while (IsChange(ptx->vout[n]) && ptx->vin.size() > 0) { + const COutPoint &prevout = ptx->vin[0].prevout; + auto it = mapWallet.find(prevout.GetTxId()); + if (it == mapWallet.end() || + it->second.tx->vout.size() <= prevout.GetN() || + !IsMine(it->second.tx->vout[prevout.GetN()])) { + break; + } + ptx = it->second.tx.get(); + n = prevout.GetN(); + } + return ptx->vout[n]; +} + static void ApproximateBestSubset(const std::vector &vValue, const Amount &nTotalLower, const Amount &nTargetValue, @@ -3814,7 +3905,7 @@ return setLockedCoins.count(outpt) > 0; } -void CWallet::ListLockedCoins(std::vector &vOutpts) { +void CWallet::ListLockedCoins(std::vector &vOutpts) const { // setLockedCoins AssertLockHeld(cs_wallet); for (COutPoint outpoint : setLockedCoins) { @@ -3994,10 +4085,23 @@ return true; } } - return false; } +std::vector +CWallet::GetDestValues(const std::string &prefix) const { + LOCK(cs_wallet); + std::vector values; + for (const auto &address : mapAddressBook) { + for (const auto &data : address.second.destdata) { + if (!data.first.compare(0, prefix.size(), prefix)) { + values.emplace_back(data.second); + } + } + } + return values; +} + CWallet *CWallet::CreateWalletFromFile(const CChainParams &chainParams, const std::string walletFile) { // Needed to restore wallet transaction meta data after -zapwallettxes