Merge #10605: Add AssertLockHeld assertions in CWallet::ListCoins
62b6f0f21e
Add EXCLUSIVE_LOCKS_REQUIRED to CWallet::ListCoins (Russell Yanofsky)545e85eccc
Add AssertLockHeld assertions in CWallet::ListCoins (Russell Yanofsky) Pull request description: Fixes TODO from #10295 Tree-SHA512: 2dd03a8217e5e1313aa2119cb530e0c0daf3ae3a751b6fdec611df57b8090201a90b52ff05f8f696e978a1344aaf21989d67a03beb5ef6ef79b77be38d04b451
This commit is contained in:
commit
b012bbe358
3 changed files with 16 additions and 14 deletions
|
@ -320,7 +320,11 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
|
||||||
|
|
||||||
// Confirm ListCoins initially returns 1 coin grouped under coinbaseKey
|
// Confirm ListCoins initially returns 1 coin grouped under coinbaseKey
|
||||||
// address.
|
// 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(list.size(), 1U);
|
||||||
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
|
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
|
||||||
BOOST_CHECK_EQUAL(list.begin()->second.size(), 1U);
|
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
|
// coinbaseKey pubkey, even though the change address has a different
|
||||||
// pubkey.
|
// pubkey.
|
||||||
AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */});
|
AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */});
|
||||||
|
{
|
||||||
|
LOCK2(cs_main, wallet->cs_wallet);
|
||||||
list = wallet->ListCoins();
|
list = wallet->ListCoins();
|
||||||
|
}
|
||||||
BOOST_CHECK_EQUAL(list.size(), 1U);
|
BOOST_CHECK_EQUAL(list.size(), 1U);
|
||||||
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
|
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
|
||||||
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
|
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
|
// Confirm ListCoins still returns same result as before, despite coins
|
||||||
// being locked.
|
// being locked.
|
||||||
|
{
|
||||||
|
LOCK2(cs_main, wallet->cs_wallet);
|
||||||
list = wallet->ListCoins();
|
list = wallet->ListCoins();
|
||||||
|
}
|
||||||
BOOST_CHECK_EQUAL(list.size(), 1U);
|
BOOST_CHECK_EQUAL(list.size(), 1U);
|
||||||
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
|
BOOST_CHECK_EQUAL(boost::get<CKeyID>(list.begin()->first).ToString(), coinbaseAddress);
|
||||||
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
|
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
|
||||||
|
|
|
@ -2252,20 +2252,12 @@ void CWallet::AvailableCoins(std::vector<COutput> &vCoins, bool fOnlySafe, const
|
||||||
|
|
||||||
std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const
|
std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const
|
||||||
{
|
{
|
||||||
// TODO: Add AssertLockHeld(cs_wallet) here.
|
AssertLockHeld(cs_main);
|
||||||
//
|
AssertLockHeld(cs_wallet);
|
||||||
// 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<CTxDestination, std::vector<COutput>> result;
|
std::map<CTxDestination, std::vector<COutput>> result;
|
||||||
std::vector<COutput> availableCoins;
|
std::vector<COutput> availableCoins;
|
||||||
|
|
||||||
LOCK2(cs_main, cs_wallet);
|
|
||||||
AvailableCoins(availableCoins);
|
AvailableCoins(availableCoins);
|
||||||
|
|
||||||
for (auto& coin : availableCoins) {
|
for (auto& coin : availableCoins) {
|
||||||
|
|
|
@ -764,7 +764,7 @@ public:
|
||||||
/**
|
/**
|
||||||
* Return list of available coins and locked coins grouped by non-change output address.
|
* 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.
|
* Find non-change parent output.
|
||||||
|
|
Loading…
Reference in a new issue