diff options
| -rw-r--r-- | src/wallet/test/wallet_tests.cpp | 16 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 12 | ||||
| -rw-r--r-- | src/wallet/wallet.h | 2 |
3 files changed, 16 insertions, 14 deletions
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index b436efc276..3a8e6f751a 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -320,7 +320,11 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey // address. - auto list = wallet->ListCoins(); + std::map<CTxDestination, std::vector<COutput>> list; + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U); @@ -333,7 +337,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) // coinbaseKey pubkey, even though the change address has a different // pubkey. AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); - list = wallet->ListCoins(); + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); @@ -359,7 +366,10 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) } // Confirm ListCoins still returns same result as before, despite coins // being locked. - list = wallet->ListCoins(); + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d3ccd2dfaa..6cbfbc1f90 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2252,20 +2252,12 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const std::map<CTxDestination, std::vector<COutput>> 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. + AssertLockHeld(cs_main); + AssertLockHeld(cs_wallet); std::map<CTxDestination, std::vector<COutput>> result; std::vector<COutput> availableCoins; - LOCK2(cs_main, cs_wallet); AvailableCoins(availableCoins); for (auto& coin : availableCoins) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 05ae9a1bbf..da326517c0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -764,7 +764,7 @@ public: /** * Return list of available coins and locked coins grouped by non-change output address. */ - std::map<CTxDestination, std::vector<COutput>> ListCoins() const; + std::map<CTxDestination, std::vector<COutput>> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); /** * Find non-change parent output. |