diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -52,7 +52,7 @@ addCoin(1000 * COIN, wallet, vCoins); addCoin(3 * COIN, wallet, vCoins); - std::set> setCoinsRet; + std::set setCoinsRet; Amount nValueRet; bool success = wallet.SelectCoinsMinConf(1003 * COIN, 1, 6, 0, vCoins, setCoinsRet, nValueRet); 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 @@ -33,7 +33,7 @@ std::vector> wtxn; -typedef std::set> CoinSet; +typedef std::set CoinSet; BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -464,6 +464,37 @@ std::set GetConflicts() const; }; +class CInputCoin { +public: + CInputCoin(const CWalletTx *walletTx, unsigned int i) : wtx(walletTx) { + 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->GetId(), i); + txout = walletTx->tx->vout[i]; + } + + COutPoint outpoint; + CTxOut txout; + const CWalletTx *wtx; + + 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: const CWalletTx *tx; @@ -612,10 +643,10 @@ * all coins from coinControl are selected; Never select unconfirmed coins * if they are not ours. */ - bool SelectCoins( - const std::vector &vAvailableCoins, const Amount nTargetValue, - std::set> &setCoinsRet, - Amount &nValueRet, const CCoinControl *coinControl = nullptr) const; + bool SelectCoins(const std::vector &vAvailableCoins, + const Amount nTargetValue, + std::set &setCoinsRet, Amount &nValueRet, + const CCoinControl *coinControl = nullptr) const; CWalletDB *pwalletdbEncryption; @@ -807,11 +838,11 @@ * completion the coin set and corresponding actual target value is * assembled. */ - bool SelectCoinsMinConf( - const Amount nTargetValue, int nConfMine, int nConfTheirs, - uint64_t nMaxAncestors, std::vector vCoins, - std::set> &setCoinsRet, - Amount &nValueRet) const; + bool SelectCoinsMinConf(const Amount nTargetValue, int nConfMine, + int nConfTheirs, uint64_t nMaxAncestors, + std::vector vCoins, + std::set &setCoinsRet, + Amount &nValueRet) const; bool IsSpent(const TxId &txid, uint32_t n) const; @@ -1215,8 +1246,7 @@ // Fill in dummy signatures for fee calculation. int nIn = 0; for (const auto &coin : coins) { - const CScript &scriptPubKey = - coin.first->tx->vout[coin.second].scriptPubKey; + const CScript &scriptPubKey = coin.txout.scriptPubKey; SignatureData sigdata; if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -64,11 +64,8 @@ */ struct CompareValueOnly { - bool operator()( - const std::pair> &t1, - const std::pair> &t2) - const { - return t1.first < t2.first; + bool operator()(const CInputCoin &t1, const CInputCoin &t2) const { + return t1.txout.nValue < t2.txout.nValue; } }; @@ -2382,11 +2379,11 @@ } } -static void ApproximateBestSubset( - std::vector>> - vValue, - const Amount nTotalLower, const Amount nTargetValue, - std::vector &vfBest, Amount &nBest, int iterations = 1000) { +static void ApproximateBestSubset(const std::vector &vValue, + const Amount &nTotalLower, + const Amount &nTargetValue, + std::vector &vfBest, Amount &nBest, + int iterations = 1000) { std::vector vfIncluded; vfBest.assign(vValue.size(), true); @@ -2406,7 +2403,7 @@ // 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].first; + nTotal += vValue[i].txout.nValue; vfIncluded[i] = true; if (nTotal >= nTargetValue) { fReachedTarget = true; @@ -2415,7 +2412,7 @@ vfBest = vfIncluded; } - nTotal -= vValue[i].first; + nTotal -= vValue[i].txout.nValue; vfIncluded[i] = false; } } @@ -2424,21 +2421,18 @@ } } -bool CWallet::SelectCoinsMinConf( - const Amount nTargetValue, const int nConfMine, const int nConfTheirs, - const uint64_t nMaxAncestors, std::vector vCoins, - std::set> &setCoinsRet, - Amount &nValueRet) const { +bool CWallet::SelectCoinsMinConf(const Amount nTargetValue, const int nConfMine, + const int nConfTheirs, + const uint64_t nMaxAncestors, + std::vector vCoins, + std::set &setCoinsRet, + Amount &nValueRet) const { setCoinsRet.clear(); nValueRet = Amount::zero(); // List of values less than target - std::pair> - coinLowestLarger; - coinLowestLarger.first = MAX_MONEY; - coinLowestLarger.second.first = nullptr; - std::vector>> - vValue; + boost::optional coinLowestLarger; + std::vector vValue; Amount nTotalLower = Amount::zero(); random_shuffle(vCoins.begin(), vCoins.end(), GetRandInt); @@ -2461,41 +2455,37 @@ } int i = output.i; - Amount n = pcoin->tx->vout[i].nValue; + CInputCoin coin = CInputCoin(pcoin, i); - std::pair> coin = - std::make_pair(n, std::make_pair(pcoin, i)); - - if (n == nTargetValue) { - setCoinsRet.insert(coin.second); - nValueRet += coin.first; + if (coin.txout.nValue == nTargetValue) { + setCoinsRet.insert(coin); + nValueRet += coin.txout.nValue; return true; - } - - if (n < nTargetValue + MIN_CHANGE) { + } else if (coin.txout.nValue < nTargetValue + MIN_CHANGE) { vValue.push_back(coin); - nTotalLower += n; - } else if (n < coinLowestLarger.first) { + nTotalLower += coin.txout.nValue; + } else if (!coinLowestLarger || + coin.txout.nValue < coinLowestLarger->txout.nValue) { coinLowestLarger = coin; } } if (nTotalLower == nTargetValue) { for (unsigned int i = 0; i < vValue.size(); ++i) { - setCoinsRet.insert(vValue[i].second); - nValueRet += vValue[i].first; + setCoinsRet.insert(vValue[i]); + nValueRet += vValue[i].txout.nValue; } return true; } if (nTotalLower < nTargetValue) { - if (coinLowestLarger.second.first == nullptr) { + if (!coinLowestLarger) { return false; } - setCoinsRet.insert(coinLowestLarger.second); - nValueRet += coinLowestLarger.first; + setCoinsRet.insert(coinLowestLarger.get()); + nValueRet += coinLowestLarger->txout.nValue; return true; } @@ -2514,16 +2504,16 @@ // 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.second.first && + if (coinLowestLarger && ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || - coinLowestLarger.first <= nBest)) { - setCoinsRet.insert(coinLowestLarger.second); - nValueRet += coinLowestLarger.first; + 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].second); - nValueRet += vValue[i].first; + setCoinsRet.insert(vValue[i]); + nValueRet += vValue[i].txout.nValue; } } @@ -2532,7 +2522,7 @@ for (size_t i = 0; i < vValue.size(); i++) { if (vfBest[i]) { LogPrint(BCLog::SELECTCOINS, "%s ", - FormatMoney(vValue[i].first)); + FormatMoney(vValue[i].txout.nValue)); } } LogPrint(BCLog::SELECTCOINS, "total %s\n", FormatMoney(nBest)); @@ -2542,10 +2532,10 @@ return true; } -bool CWallet::SelectCoins( - const std::vector &vAvailableCoins, const Amount nTargetValue, - std::set> &setCoinsRet, - Amount &nValueRet, const CCoinControl *coinControl) const { +bool CWallet::SelectCoins(const std::vector &vAvailableCoins, + const Amount nTargetValue, + std::set &setCoinsRet, Amount &nValueRet, + const CCoinControl *coinControl) const { std::vector vCoins(vAvailableCoins); // coin control -> return all selected outputs (we want all selected to go @@ -2558,14 +2548,14 @@ } nValueRet += out.tx->tx->vout[out.i].nValue; - setCoinsRet.insert(std::make_pair(out.tx, out.i)); + setCoinsRet.insert(CInputCoin(out.tx, out.i)); } return (nValueRet >= nTargetValue); } // Calculate value from preset inputs and store them. - std::set> setPresetCoins; + std::set setPresetCoins; Amount nValueFromPresetInputs = Amount::zero(); std::vector vPresetInputs; @@ -2588,13 +2578,13 @@ } nValueFromPresetInputs += pcoin->tx->vout[outpoint.GetN()].nValue; - setPresetCoins.insert(std::make_pair(pcoin, outpoint.GetN())); + setPresetCoins.insert(CInputCoin(pcoin, outpoint.GetN())); } // Remove preset inputs from vCoins. for (std::vector::iterator it = vCoins.begin(); it != vCoins.end() && coinControl && coinControl->HasSelected();) { - if (setPresetCoins.count(std::make_pair(it->tx, it->i))) { + if (setPresetCoins.count(CInputCoin(it->tx, it->i))) { it = vCoins.erase(it); } else { ++it; @@ -2774,7 +2764,7 @@ assert(txNew.nLockTime < LOCKTIME_THRESHOLD); { - std::set> setCoins; + std::set setCoins; LOCK2(cs_main, cs_wallet); std::vector vAvailableCoins; @@ -2842,13 +2832,13 @@ } for (const auto &pcoin : setCoins) { - Amount nCredit = pcoin.first->tx->vout[pcoin.second].nValue; + Amount nCredit = pcoin.txout.nValue; // The coin age after the next block (depth+1) is used instead // of the current, reflecting an assumption the user would // accept a bit more delay for a chance at a free transaction. // But mempool inputs might still be in the mempool, so their // age stays 0. - int age = pcoin.first->GetDepthInMainChain(); + int age = pcoin.wtx->GetDepthInMainChain(); assert(age >= 0); if (age != 0) { age += 1; @@ -2953,7 +2943,7 @@ // nLockTime set above actually works. for (const auto &coin : setCoins) { txNew.vin.push_back( - CTxIn(coin.first->GetId(), coin.second, CScript(), + CTxIn(coin.outpoint, CScript(), std::numeric_limits::max() - 1)); } @@ -3050,16 +3040,13 @@ CTransaction txNewConst(txNew); int nIn = 0; for (const auto &coin : setCoins) { - const CScript &scriptPubKey = - coin.first->tx->vout[coin.second].scriptPubKey; + const CScript &scriptPubKey = coin.txout.scriptPubKey; SignatureData sigdata; - if (!ProduceSignature( - TransactionSignatureCreator( - this, &txNewConst, nIn, - coin.first->tx->vout[coin.second].nValue, - sigHashType), - scriptPubKey, sigdata)) { + if (!ProduceSignature(TransactionSignatureCreator( + this, &txNewConst, nIn, + coin.txout.nValue, sigHashType), + scriptPubKey, sigdata)) { strFailReason = _("Signing transaction failed"); return false; }