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 @@ -338,7 +338,11 @@ // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey // address. - auto list = wallet->ListCoins(); + std::map> list; + { + LOCK2(cs_main, wallet->cs_wallet); + list = wallet->ListCoins(); + } BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(boost::get(list.begin()->first).ToString(), coinbaseAddress); @@ -353,7 +357,10 @@ // 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(list.begin()->first).ToString(), coinbaseAddress); @@ -380,7 +387,10 @@ } // 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(list.begin()->first).ToString(), coinbaseAddress); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -857,7 +857,8 @@ * Return list of available coins and locked coins grouped by non-change * output address. */ - std::map> ListCoins() const; + std::map> ListCoins() const + EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet); /** * Find non-change parent output. diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2520,20 +2520,12 @@ } 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. + AssertLockHeld(cs_main); + AssertLockHeld(cs_wallet); std::map> result; std::vector availableCoins; - LOCK2(cs_main, cs_wallet); AvailableCoins(availableCoins); for (auto &coin : availableCoins) {