highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-05-03 16:30:42 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge #16426: Reverse cs_main, cs_wallet lock order and reduce cs_main locking

Browse Source 
6a72f26968cf931c985d8d4797b6264274cabd06 [wallet] Remove locked_chain from CWallet, its RPCs and tests (Antoine Riard)
841178820d31e1c24a00cb2c8fc0b1fd2f126f56 [wallet] Move methods from Chain::Lock interface to simple Chain (Antoine Riard)
0a76287387950bc9c5b634e95c5cd5fb1029f42d [wallet] Move getBlockHash from Chain::Lock interface to simple Chain (Antoine Riard)
de13363a472ea30dff2f8f55c6ae572281115380 [wallet] Move getBlockHeight from Chain::Lock interface to simple Chain (Antoine Riard)
b855592d835bf4b3fb1263b88d4f96669a1722b1 [wallet] Move getHeight from Chain::Lock interface to simple Chain (Antoine Riard)

Pull request description:

  This change is intended to make the bitcoin node and its rpc, network and gui interfaces more responsive while the wallet is in use. Currently, because the node's `cs_main` mutex is always locked before the wallet's `cs_wallet` mutex (to prevent deadlocks), `cs_main` currently stays locked while the wallet does relatively slow things like creating and listing transactions.

  Switching the lock order so `cs_main` is acquired after `cs_wallet` allows `cs_main` to be only locked intermittently while the wallet is doing slow operations, so the node is not blocked waiting for the wallet.

  To review the present PR, most of getting right the move is ensuring any `LockAssertion` in `Chain::Lock` method is amended as a `LOCK(cs_main)`. And in final commit, check that any wallet code which was previously locking the chain is now calling a  method, enforcing the lock taking job. So far the only exception I found is `handleNotifications`, which should be corrected.

ACKs for top commit:
  MarcoFalke:
    re-ACK 6a72f26968 🔏
  fjahr:
    re-ACK 6a72f26968cf931c985d8d4797b6264274cabd06
  ryanofsky:
    Code review ACK 6a72f26968cf931c985d8d4797b6264274cabd06. Only difference compared to the rebase I posted is reverting unneeded SetLastBlockProcessed change in wallet_disableprivkeys test

Tree-SHA512: 9168b3bf3432d4f8bc4d9fa9246ac057050848e673efc264c8f44345f243ba9697b05c22c809a79d1b51bf0de1c4ed317960e496480f8d71e584468d4dd1b0ad
This commit is contained in:
MarcoFalke 2020-05-01 06:58:53 -04:00
commit 608359b071
No known key found for this signature in database
GPG Key ID: CE2B75697E69A548
11 changed files with 251 additions and 373 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

144
src/interfaces/chain.cpp
View File

@ -53,88 +53,6 @@ bool FillBlock(const CBlockIndex* index, const FoundBlock& block, UniqueLock<Rec
return true; return true;
} }
class LockImpl : public Chain::Lock, public UniqueLock<RecursiveMutex>
{
Optional<int> getHeight() override
{
LockAssertion lock(::cs_main);
int height = ::ChainActive().Height();
if (height >= 0) {
return height;
}
return nullopt;
}
Optional<int> getBlockHeight(const uint256& hash) override
{
LockAssertion lock(::cs_main);
CBlockIndex* block = LookupBlockIndex(hash);
if (block && ::ChainActive().Contains(block)) {
return block->nHeight;
}
return nullopt;
}
uint256 getBlockHash(int height) override
{
LockAssertion lock(::cs_main);
CBlockIndex* block = ::ChainActive()[height];
assert(block != nullptr);
return block->GetBlockHash();
}
bool haveBlockOnDisk(int height) override
{
LockAssertion lock(::cs_main);
CBlockIndex* block = ::ChainActive()[height];
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
}
Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) override
{
LockAssertion lock(::cs_main);
CBlockIndex* block = ::ChainActive().FindEarliestAtLeast(time, height);
if (block) {
if (hash) *hash = block->GetBlockHash();
return block->nHeight;
}
return nullopt;
}
Optional<int> findFork(const uint256& hash, Optional<int>* height) override
{
LockAssertion lock(::cs_main);
const CBlockIndex* block = LookupBlockIndex(hash);
const CBlockIndex* fork = block ? ::ChainActive().FindFork(block) : nullptr;
if (height) {
if (block) {
*height = block->nHeight;
} else {
height->reset();
}
}
if (fork) {
return fork->nHeight;
}
return nullopt;
}
CBlockLocator getTipLocator() override
{
LockAssertion lock(::cs_main);
return ::ChainActive().GetLocator();
}
Optional<int> findLocatorFork(const CBlockLocator& locator) override
{
LockAssertion lock(::cs_main);
if (CBlockIndex* fork = FindForkInGlobalIndex(::ChainActive(), locator)) {
return fork->nHeight;
}
return nullopt;
}
bool checkFinalTx(const CTransaction& tx) override
{
LockAssertion lock(::cs_main);
return CheckFinalTx(tx);
}
using UniqueLock::UniqueLock;
};
class NotificationsProxy : public CValidationInterface class NotificationsProxy : public CValidationInterface
{ {
public: public:
@ -227,12 +145,64 @@ class ChainImpl : public Chain
{ {
public: public:
explicit ChainImpl(NodeContext& node) : m_node(node) {} explicit ChainImpl(NodeContext& node) : m_node(node) {}
std::unique_ptr<Chain::Lock> lock(bool try_lock) override Optional<int> getHeight() override
{ {
auto lock = MakeUnique<LockImpl>(::cs_main, "cs_main", __FILE__, __LINE__, try_lock); LOCK(::cs_main);
if (try_lock && lock && !*lock) return {}; int height = ::ChainActive().Height();
std::unique_ptr<Chain::Lock> result = std::move(lock); // Temporary to avoid CWG 1579 if (height >= 0) {
return result; return height;
}
return nullopt;
}
Optional<int> getBlockHeight(const uint256& hash) override
{
LOCK(::cs_main);
CBlockIndex* block = LookupBlockIndex(hash);
if (block && ::ChainActive().Contains(block)) {
return block->nHeight;
}
return nullopt;
}
uint256 getBlockHash(int height) override
{
LOCK(::cs_main);
CBlockIndex* block = ::ChainActive()[height];
assert(block);
return block->GetBlockHash();
}
bool haveBlockOnDisk(int height) override
{
LOCK(cs_main);
CBlockIndex* block = ::ChainActive()[height];
return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0;
}
Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) override
{
LOCK(cs_main);
CBlockIndex* block = ::ChainActive().FindEarliestAtLeast(time, height);
if (block) {
if (hash) *hash = block->GetBlockHash();
return block->nHeight;
}
return nullopt;
}
CBlockLocator getTipLocator() override
{
LOCK(cs_main);
return ::ChainActive().GetLocator();
}
bool checkFinalTx(const CTransaction& tx) override
{
LOCK(cs_main);
return CheckFinalTx(tx);
}
Optional<int> findLocatorFork(const CBlockLocator& locator) override
{
LOCK(cs_main);
if (CBlockIndex* fork = FindForkInGlobalIndex(::ChainActive(), locator)) {
return fork->nHeight;
}
return nullopt;
} }
bool findBlock(const uint256& hash, const FoundBlock& block) override bool findBlock(const uint256& hash, const FoundBlock& block) override
{ {

40
src/interfaces/chain.h
View File

@ -59,12 +59,7 @@ public:
//! internal workings of the bitcoin node, and not being very convenient to use. //! internal workings of the bitcoin node, and not being very convenient to use.
//! Chain methods should be cleaned up and simplified over time. Examples: //! Chain methods should be cleaned up and simplified over time. Examples:
//! //!
//! * The Chain::lock() method, which lets clients delay chain tip updates //! * The initMessages() and showProgress() methods which the wallet uses to send
//! should be removed when clients are able to respond to updates
//! asynchronously
//! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269).
//!
//! * The initMessage() and showProgress() methods which the wallet uses to send
//! notifications to the GUI should go away when GUI and wallet can directly //! notifications to the GUI should go away when GUI and wallet can directly
//! communicate with each other without going through the node //! communicate with each other without going through the node
//! (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096). //! (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096).
@ -72,24 +67,22 @@ public:
//! * The handleRpc, registerRpcs, rpcEnableDeprecated methods and other RPC //! * The handleRpc, registerRpcs, rpcEnableDeprecated methods and other RPC
//! methods can go away if wallets listen for HTTP requests on their own //! methods can go away if wallets listen for HTTP requests on their own
//! ports instead of registering to handle requests on the node HTTP port. //! ports instead of registering to handle requests on the node HTTP port.
//!
//! * Move fee estimation queries to an asynchronous interface and let the
//! wallet cache it, fee estimation being driven by node mempool, wallet
//! should be the consumer.
//!
//! * The `guessVerificationProgress`, `getBlockHeight`, `getBlockHash`, etc
//! methods can go away if rescan logic is moved on the node side, and wallet
//! only register rescan request.
class Chain class Chain
{ {
public: public:
virtual ~Chain() {} virtual ~Chain() {}
//! Interface for querying locked chain state, used by legacy code that
//! assumes state won't change between calls. New code should avoid using
//! the Lock interface and instead call higher-level Chain methods
//! that return more information so the chain doesn't need to stay locked
//! between calls.
class Lock
{
public:
virtual ~Lock() {}
//! Get current chain height, not including genesis block (returns 0 if //! Get current chain height, not including genesis block (returns 0 if
//! chain only contains genesis block, nullopt if chain does not contain //! chain only contains genesis block, nullopt if chain does not contain
//! any blocks). //! any blocks)
virtual Optional<int> getHeight() = 0; virtual Optional<int> getHeight() = 0;
//! Get block height above genesis block. Returns 0 for genesis block, //! Get block height above genesis block. Returns 0 for genesis block,
@ -111,14 +104,6 @@ public:
//! (to avoid the cost of a second lookup in case this information is needed.) //! (to avoid the cost of a second lookup in case this information is needed.)
virtual Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0; virtual Optional<int> findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0;
//! Return height of the specified block if it is on the chain, otherwise
//! return the height of the highest block on chain that's an ancestor
//! of the specified block, or nullopt if there is no common ancestor.
//! Also return the height of the specified block as an optional output
//! parameter (to avoid the cost of a second hash lookup in case this
//! information is desired).
virtual Optional<int> findFork(const uint256& hash, Optional<int>* height) = 0;
//! Get locator for the current chain tip. //! Get locator for the current chain tip.
virtual CBlockLocator getTipLocator() = 0; virtual CBlockLocator getTipLocator() = 0;
@ -129,11 +114,6 @@ public:
//! Check if transaction will be final given chain height current time. //! Check if transaction will be final given chain height current time.
virtual bool checkFinalTx(const CTransaction& tx) = 0; virtual bool checkFinalTx(const CTransaction& tx) = 0;
};
//! Return Lock interface. Chain is locked when this is called, and
//! unlocked when the returned interface is freed.
virtual std::unique_ptr<Lock> lock(bool try_lock = false) = 0;
//! Return whether node has the block and optionally return block metadata //! Return whether node has the block and optionally return block metadata
//! or contents. //! or contents.

40
src/interfaces/wallet.cpp
View File

@ -60,7 +60,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
} }
//! Construct wallet tx status struct. //! Construct wallet tx status struct.
WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx) WalletTxStatus MakeWalletTxStatus(CWallet& wallet, const CWalletTx& wtx)
{ {
WalletTxStatus result; WalletTxStatus result;
result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits<int>::max(); result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits<int>::max();
@ -68,8 +68,8 @@ WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const C
result.depth_in_main_chain = wtx.GetDepthInMainChain(); result.depth_in_main_chain = wtx.GetDepthInMainChain();
result.time_received = wtx.nTimeReceived; result.time_received = wtx.nTimeReceived;
result.lock_time = wtx.tx->nLockTime; result.lock_time = wtx.tx->nLockTime;
result.is_final = locked_chain.checkFinalTx(*wtx.tx); result.is_final = wallet.chain().checkFinalTx(*wtx.tx);
result.is_trusted = wtx.IsTrusted(locked_chain); result.is_trusted = wtx.IsTrusted();
result.is_abandoned = wtx.isAbandoned(); result.is_abandoned = wtx.isAbandoned();
result.is_coinbase = wtx.IsCoinBase(); result.is_coinbase = wtx.IsCoinBase();
result.is_in_main_chain = wtx.IsInMainChain(); result.is_in_main_chain = wtx.IsInMainChain();
@ -196,25 +196,21 @@ public:
} }
void lockCoin(const COutPoint& output) override void lockCoin(const COutPoint& output) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->LockCoin(output); return m_wallet->LockCoin(output);
} }
void unlockCoin(const COutPoint& output) override void unlockCoin(const COutPoint& output) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->UnlockCoin(output); return m_wallet->UnlockCoin(output);
} }
bool isLockedCoin(const COutPoint& output) override bool isLockedCoin(const COutPoint& output) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->IsLockedCoin(output.hash, output.n); return m_wallet->IsLockedCoin(output.hash, output.n);
} }
void listLockedCoins(std::vector<COutPoint>& outputs) override void listLockedCoins(std::vector<COutPoint>& outputs) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->ListLockedCoins(outputs); return m_wallet->ListLockedCoins(outputs);
} }
@ -225,10 +221,9 @@ public:
CAmount& fee, CAmount& fee,
std::string& fail_reason) override std::string& fail_reason) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
CTransactionRef tx; CTransactionRef tx;
if (!m_wallet->CreateTransaction(*locked_chain, recipients, tx, fee, change_pos, if (!m_wallet->CreateTransaction(recipients, tx, fee, change_pos,
fail_reason, coin_control, sign)) { fail_reason, coin_control, sign)) {
return {}; return {};
} }
@ -238,14 +233,12 @@ public:
WalletValueMap value_map, WalletValueMap value_map,
WalletOrderForm order_form) override WalletOrderForm order_form) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form)); m_wallet->CommitTransaction(std::move(tx), std::move(value_map), std::move(order_form));
} }
bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); } bool transactionCanBeAbandoned(const uint256& txid) override { return m_wallet->TransactionCanBeAbandoned(txid); }
bool abandonTransaction(const uint256& txid) override bool abandonTransaction(const uint256& txid) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->AbandonTransaction(txid); return m_wallet->AbandonTransaction(txid);
} }
@ -273,7 +266,6 @@ public:
} }
CTransactionRef getTx(const uint256& txid) override CTransactionRef getTx(const uint256& txid) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
auto mi = m_wallet->mapWallet.find(txid); auto mi = m_wallet->mapWallet.find(txid);
if (mi != m_wallet->mapWallet.end()) { if (mi != m_wallet->mapWallet.end()) {
@ -283,7 +275,6 @@ public:
} }
WalletTx getWalletTx(const uint256& txid) override WalletTx getWalletTx(const uint256& txid) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
auto mi = m_wallet->mapWallet.find(txid); auto mi = m_wallet->mapWallet.find(txid);
if (mi != m_wallet->mapWallet.end()) { if (mi != m_wallet->mapWallet.end()) {
@ -293,7 +284,6 @@ public:
} }
std::vector<WalletTx> getWalletTxs() override std::vector<WalletTx> getWalletTxs() override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
std::vector<WalletTx> result; std::vector<WalletTx> result;
result.reserve(m_wallet->mapWallet.size()); result.reserve(m_wallet->mapWallet.size());
@ -307,10 +297,6 @@ public:
int& num_blocks, int& num_blocks,
int64_t& block_time) override int64_t& block_time) override
{ {
auto locked_chain = m_wallet->chain().lock(true /* try_lock */);
if (!locked_chain) {
return false;
}
TRY_LOCK(m_wallet->cs_wallet, locked_wallet); TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
if (!locked_wallet) { if (!locked_wallet) {
return false; return false;
@ -322,7 +308,7 @@ public:
num_blocks = m_wallet->GetLastBlockHeight(); num_blocks = m_wallet->GetLastBlockHeight();
block_time = -1; block_time = -1;
CHECK_NONFATAL(m_wallet->chain().findBlock(m_wallet->GetLastBlockHash(), FoundBlock().time(block_time))); CHECK_NONFATAL(m_wallet->chain().findBlock(m_wallet->GetLastBlockHash(), FoundBlock().time(block_time)));
tx_status = MakeWalletTxStatus(*locked_chain, mi->second); tx_status = MakeWalletTxStatus(*m_wallet, mi->second);
return true; return true;
} }
WalletTx getWalletTxDetails(const uint256& txid, WalletTx getWalletTxDetails(const uint256& txid,
@ -331,14 +317,13 @@ public:
bool& in_mempool, bool& in_mempool,
int& num_blocks) override int& num_blocks) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
auto mi = m_wallet->mapWallet.find(txid); auto mi = m_wallet->mapWallet.find(txid);
if (mi != m_wallet->mapWallet.end()) { if (mi != m_wallet->mapWallet.end()) {
num_blocks = locked_chain->getHeight().get_value_or(-1); num_blocks = m_wallet->GetLastBlockHeight();
in_mempool = mi->second.InMempool(); in_mempool = mi->second.InMempool();
order_form = mi->second.vOrderForm; order_form = mi->second.vOrderForm;
tx_status = MakeWalletTxStatus(*locked_chain, mi->second); tx_status = MakeWalletTxStatus(*m_wallet, mi->second);
return MakeWalletTx(*m_wallet, mi->second); return MakeWalletTx(*m_wallet, mi->second);
} }
return {}; return {};
@ -368,8 +353,6 @@ public:
} }
bool tryGetBalances(WalletBalances& balances, int& num_blocks, bool force, int cached_num_blocks) override bool tryGetBalances(WalletBalances& balances, int& num_blocks, bool force, int cached_num_blocks) override
{ {
auto locked_chain = m_wallet->chain().lock(true /* try_lock */);
if (!locked_chain) return false;
TRY_LOCK(m_wallet->cs_wallet, locked_wallet); TRY_LOCK(m_wallet->cs_wallet, locked_wallet);
if (!locked_wallet) { if (!locked_wallet) {
return false; return false;
@ -386,34 +369,29 @@ public:
} }
isminetype txinIsMine(const CTxIn& txin) override isminetype txinIsMine(const CTxIn& txin) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->IsMine(txin); return m_wallet->IsMine(txin);
} }
isminetype txoutIsMine(const CTxOut& txout) override isminetype txoutIsMine(const CTxOut& txout) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->IsMine(txout); return m_wallet->IsMine(txout);
} }
CAmount getDebit(const CTxIn& txin, isminefilter filter) override CAmount getDebit(const CTxIn& txin, isminefilter filter) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->GetDebit(txin, filter); return m_wallet->GetDebit(txin, filter);
} }
CAmount getCredit(const CTxOut& txout, isminefilter filter) override CAmount getCredit(const CTxOut& txout, isminefilter filter) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
return m_wallet->GetCredit(txout, filter); return m_wallet->GetCredit(txout, filter);
} }
CoinsList listCoins() override CoinsList listCoins() override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
CoinsList result; CoinsList result;
for (const auto& entry : m_wallet->ListCoins(*locked_chain)) { for (const auto& entry : m_wallet->ListCoins()) {
auto& group = result[entry.first]; auto& group = result[entry.first];
for (const auto& coin : entry.second) { for (const auto& coin : entry.second) {
group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i), group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i),
@ -424,7 +402,6 @@ public:
} }
std::vector<WalletTxOut> getCoins(const std::vector<COutPoint>& outputs) override std::vector<WalletTxOut> getCoins(const std::vector<COutPoint>& outputs) override
{ {
auto locked_chain = m_wallet->chain().lock();
LOCK(m_wallet->cs_wallet); LOCK(m_wallet->cs_wallet);
std::vector<WalletTxOut> result; std::vector<WalletTxOut> result;
result.reserve(outputs.size()); result.reserve(outputs.size());
@ -496,6 +473,7 @@ public:
{ {
return MakeHandler(m_wallet->NotifyCanGetAddressesChanged.connect(fn)); return MakeHandler(m_wallet->NotifyCanGetAddressesChanged.connect(fn));
} }
CWallet* wallet() override { return m_wallet.get(); }
std::shared_ptr<CWallet> m_wallet; std::shared_ptr<CWallet> m_wallet;
}; };

3
src/interfaces/wallet.h
View File

@ -300,6 +300,9 @@ public:
//! Register handler for keypool changed messages. //! Register handler for keypool changed messages.
using CanGetAddressesChangedFn = std::function<void()>; using CanGetAddressesChangedFn = std::function<void()>;
virtual std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) = 0; virtual std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) = 0;
//! Return pointer to internal wallet class, useful for testing.
virtual CWallet* wallet() { return nullptr; }
}; };
//! Information about one wallet address. //! Information about one wallet address.

1
src/qt/test/wallettests.cpp
View File

@ -145,7 +145,6 @@ void TestGUI(interfaces::Node& node)
wallet->LoadWallet(firstRun); wallet->LoadWallet(firstRun);
{ {
auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan();
auto locked_chain = wallet->chain().lock();
LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore);
wallet->SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type), "", "receive"); wallet->SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type), "", "receive");
spk_man->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey()); spk_man->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey());

6
src/wallet/feebumper.cpp
View File

@ -140,7 +140,6 @@ namespace feebumper {
bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
{ {
auto locked_chain = wallet.chain().lock();
LOCK(wallet.cs_wallet); LOCK(wallet.cs_wallet);
const CWalletTx* wtx = wallet.GetWalletTx(txid); const CWalletTx* wtx = wallet.GetWalletTx(txid);
if (wtx == nullptr) return false; if (wtx == nullptr) return false;
@ -156,7 +155,6 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
// We are going to modify coin control later, copy to re-use // We are going to modify coin control later, copy to re-use
CCoinControl new_coin_control(coin_control); CCoinControl new_coin_control(coin_control);
auto locked_chain = wallet.chain().lock();
LOCK(wallet.cs_wallet); LOCK(wallet.cs_wallet);
errors.clear(); errors.clear();
auto it = wallet.mapWallet.find(txid); auto it = wallet.mapWallet.find(txid);
@ -219,7 +217,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
CAmount fee_ret; CAmount fee_ret;
int change_pos_in_out = -1; // No requested location for change int change_pos_in_out = -1; // No requested location for change
std::string fail_reason; std::string fail_reason;
if (!wallet.CreateTransaction(*locked_chain, recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) { if (!wallet.CreateTransaction(recipients, tx_new, fee_ret, change_pos_in_out, fail_reason, new_coin_control, false)) {
errors.push_back("Unable to create transaction: " + fail_reason); errors.push_back("Unable to create transaction: " + fail_reason);
return Result::WALLET_ERROR; return Result::WALLET_ERROR;
} }
@ -240,14 +238,12 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
} }
bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) { bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {
auto locked_chain = wallet.chain().lock();
LOCK(wallet.cs_wallet); LOCK(wallet.cs_wallet);
return wallet.SignTransaction(mtx); return wallet.SignTransaction(mtx);
} }
Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<std::string>& errors, uint256& bumped_txid) Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector<std::string>& errors, uint256& bumped_txid)
{ {
auto locked_chain = wallet.chain().lock();
LOCK(wallet.cs_wallet); LOCK(wallet.cs_wallet);
if (!errors.empty()) { if (!errors.empty()) {
return Result::MISC_ERROR; return Result::MISC_ERROR;

16
src/wallet/rpcdump.cpp
View File

@ -133,7 +133,6 @@ UniValue importprivkey(const JSONRPCRequest& request)
WalletRescanReserver reserver(*pwallet); WalletRescanReserver reserver(*pwallet);
bool fRescan = true; bool fRescan = true;
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -285,7 +284,6 @@ UniValue importaddress(const JSONRPCRequest& request)
fP2SH = request.params[3].get_bool(); fP2SH = request.params[3].get_bool();
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
CTxDestination dest = DecodeDestination(request.params[0].get_str()); CTxDestination dest = DecodeDestination(request.params[0].get_str());
@ -317,7 +315,6 @@ UniValue importaddress(const JSONRPCRequest& request)
{ {
RescanWallet(*pwallet, reserver); RescanWallet(*pwallet, reserver);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
pwallet->ReacceptWalletTransactions(); pwallet->ReacceptWalletTransactions();
} }
@ -361,7 +358,6 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock"); throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Something wrong with merkleblock");
} }
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
int height; int height;
if (!pwallet->chain().findAncestorByHash(pwallet->GetLastBlockHash(), merkleBlock.header.GetHash(), FoundBlock().height(height))) { if (!pwallet->chain().findAncestorByHash(pwallet->GetLastBlockHash(), merkleBlock.header.GetHash(), FoundBlock().height(height))) {
@ -407,7 +403,6 @@ UniValue removeprunedfunds(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
uint256 hash(ParseHashV(request.params[0], "txid")); uint256 hash(ParseHashV(request.params[0], "txid"));
@ -487,7 +482,6 @@ UniValue importpubkey(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key");
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
std::set<CScript> script_pub_keys; std::set<CScript> script_pub_keys;
@ -505,7 +499,6 @@ UniValue importpubkey(const JSONRPCRequest& request)
{ {
RescanWallet(*pwallet, reserver); RescanWallet(*pwallet, reserver);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
pwallet->ReacceptWalletTransactions(); pwallet->ReacceptWalletTransactions();
} }
@ -557,7 +550,6 @@ UniValue importwallet(const JSONRPCRequest& request)
int64_t nTimeBegin = 0; int64_t nTimeBegin = 0;
bool fGood = true; bool fGood = true;
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -700,7 +692,6 @@ UniValue dumpprivkey(const JSONRPCRequest& request)
LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet); LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*wallet);
auto locked_chain = pwallet->chain().lock();
LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -756,7 +747,6 @@ UniValue dumpwallet(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
wallet.BlockUntilSyncedToCurrentChain(); wallet.BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore);
EnsureWalletIsUnlocked(&wallet); EnsureWalletIsUnlocked(&wallet);
@ -780,7 +770,7 @@ UniValue dumpwallet(const JSONRPCRequest& request)
std::map<CKeyID, int64_t> mapKeyBirth; std::map<CKeyID, int64_t> mapKeyBirth;
const std::map<CKeyID, int64_t>& mapKeyPool = spk_man.GetAllReserveKeys(); const std::map<CKeyID, int64_t>& mapKeyPool = spk_man.GetAllReserveKeys();
pwallet->GetKeyBirthTimes(*locked_chain, mapKeyBirth); pwallet->GetKeyBirthTimes(mapKeyBirth);
std::set<CScriptID> scripts = spk_man.GetCScripts(); std::set<CScriptID> scripts = spk_man.GetCScripts();
@ -1379,7 +1369,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
int64_t nLowestTimestamp = 0; int64_t nLowestTimestamp = 0;
UniValue response(UniValue::VARR); UniValue response(UniValue::VARR);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -1414,7 +1403,6 @@ UniValue importmulti(const JSONRPCRequest& mainRequest)
if (fRescan && fRunScan && requests.size()) { if (fRescan && fRunScan && requests.size()) {
int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */); int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
pwallet->ReacceptWalletTransactions(); pwallet->ReacceptWalletTransactions();
} }
@ -1676,7 +1664,6 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) {
bool rescan = false; bool rescan = false;
UniValue response(UniValue::VARR); UniValue response(UniValue::VARR);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -1705,7 +1692,6 @@ UniValue importdescriptors(const JSONRPCRequest& main_request) {
if (rescan) { if (rescan) {
int64_t scanned_time = pwallet->RescanFromTime(lowest_timestamp, reserver, true /* update */); int64_t scanned_time = pwallet->RescanFromTime(lowest_timestamp, reserver, true /* update */);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
pwallet->ReacceptWalletTransactions(); pwallet->ReacceptWalletTransactions();
} }

82
src/wallet/rpcwallet.cpp
View File

@ -133,7 +133,7 @@ LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_cr
return *spk_man; return *spk_man;
} }
static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx, UniValue& entry) static void WalletTxToJSON(interfaces::Chain& chain, const CWalletTx& wtx, UniValue& entry)
{ {
int confirms = wtx.GetDepthInMainChain(); int confirms = wtx.GetDepthInMainChain();
entry.pushKV("confirmations", confirms); entry.pushKV("confirmations", confirms);
@ -148,7 +148,7 @@ static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& lo
CHECK_NONFATAL(chain.findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(block_time))); CHECK_NONFATAL(chain.findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(block_time)));
entry.pushKV("blocktime", block_time); entry.pushKV("blocktime", block_time);
} else { } else {
entry.pushKV("trusted", wtx.IsTrusted(locked_chain)); entry.pushKV("trusted", wtx.IsTrusted());
} }
uint256 hash = wtx.GetHash(); uint256 hash = wtx.GetHash();
entry.pushKV("txid", hash.GetHex()); entry.pushKV("txid", hash.GetHex());
@ -322,7 +322,7 @@ static UniValue setlabel(const JSONRPCRequest& request)
} }
static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet * const pwallet, const CTxDestination &address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue) static CTransactionRef SendMoney(CWallet* const pwallet, const CTxDestination& address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue)
{ {
CAmount curBalance = pwallet->GetBalance(0, coin_control.m_avoid_address_reuse).m_mine_trusted; CAmount curBalance = pwallet->GetBalance(0, coin_control.m_avoid_address_reuse).m_mine_trusted;
@ -344,7 +344,7 @@ static CTransactionRef SendMoney(interfaces::Chain::Lock& locked_chain, CWallet
CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount};
vecSend.push_back(recipient); vecSend.push_back(recipient);
CTransactionRef tx; CTransactionRef tx;
if (!pwallet->CreateTransaction(locked_chain, vecSend, tx, nFeeRequired, nChangePosRet, strError, coin_control)) { if (!pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, strError, coin_control)) {
if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance)
strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired)); strError = strprintf("Error: This transaction requires a transaction fee of at least %s", FormatMoney(nFeeRequired));
throw JSONRPCError(RPC_WALLET_ERROR, strError); throw JSONRPCError(RPC_WALLET_ERROR, strError);
@ -399,7 +399,6 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
CTxDestination dest = DecodeDestination(request.params[0].get_str()); CTxDestination dest = DecodeDestination(request.params[0].get_str());
@ -445,7 +444,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
CTransactionRef tx = SendMoney(*locked_chain, pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue)); CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue));
return tx->GetHash().GetHex(); return tx->GetHash().GetHex();
} }
@ -487,11 +486,10 @@ static UniValue listaddressgroupings(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
UniValue jsonGroupings(UniValue::VARR); UniValue jsonGroupings(UniValue::VARR);
std::map<CTxDestination, CAmount> balances = pwallet->GetAddressBalances(*locked_chain); std::map<CTxDestination, CAmount> balances = pwallet->GetAddressBalances();
for (const std::set<CTxDestination>& grouping : pwallet->GetAddressGroupings()) { for (const std::set<CTxDestination>& grouping : pwallet->GetAddressGroupings()) {
UniValue jsonGrouping(UniValue::VARR); UniValue jsonGrouping(UniValue::VARR);
for (const CTxDestination& address : grouping) for (const CTxDestination& address : grouping)
@ -543,7 +541,6 @@ static UniValue signmessage(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -572,7 +569,7 @@ static UniValue signmessage(const JSONRPCRequest& request)
return signature; return signature;
} }
static CAmount GetReceived(interfaces::Chain::Lock& locked_chain, const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) static CAmount GetReceived(const CWallet& wallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{ {
std::set<CTxDestination> address_set; std::set<CTxDestination> address_set;
@ -602,7 +599,7 @@ static CAmount GetReceived(interfaces::Chain::Lock& locked_chain, const CWallet&
CAmount amount = 0; CAmount amount = 0;
for (const std::pair<const uint256, CWalletTx>& wtx_pair : wallet.mapWallet) { for (const std::pair<const uint256, CWalletTx>& wtx_pair : wallet.mapWallet) {
const CWalletTx& wtx = wtx_pair.second; const CWalletTx& wtx = wtx_pair.second;
if (wtx.IsCoinBase() || !locked_chain.checkFinalTx(*wtx.tx) || wtx.GetDepthInMainChain() < min_depth) { if (wtx.IsCoinBase() || !wallet.chain().checkFinalTx(*wtx.tx) || wtx.GetDepthInMainChain() < min_depth) {
continue; continue;
} }
@ -652,10 +649,9 @@ static UniValue getreceivedbyaddress(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
return ValueFromAmount(GetReceived(*locked_chain, *pwallet, request.params, /* by_label */ false)); return ValueFromAmount(GetReceived(*pwallet, request.params, /* by_label */ false));
} }
@ -693,10 +689,9 @@ static UniValue getreceivedbylabel(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
return ValueFromAmount(GetReceived(*locked_chain, *pwallet, request.params, /* by_label */ true)); return ValueFromAmount(GetReceived(*pwallet, request.params, /* by_label */ true));
} }
@ -736,7 +731,6 @@ static UniValue getbalance(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
const UniValue& dummy_value = request.params[0]; const UniValue& dummy_value = request.params[0];
@ -778,7 +772,6 @@ static UniValue getunconfirmedbalance(const JSONRPCRequest &request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
return ValueFromAmount(pwallet->GetBalance().m_mine_untrusted_pending); return ValueFromAmount(pwallet->GetBalance().m_mine_untrusted_pending);
@ -841,7 +834,6 @@ static UniValue sendmany(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
if (!request.params[0].isNull() && !request.params[0].get_str().empty()) { if (!request.params[0].isNull() && !request.params[0].get_str().empty()) {
@ -913,7 +905,7 @@ static UniValue sendmany(const JSONRPCRequest& request)
int nChangePosRet = -1; int nChangePosRet = -1;
std::string strFailReason; std::string strFailReason;
CTransactionRef tx; CTransactionRef tx;
bool fCreated = pwallet->CreateTransaction(*locked_chain, vecSend, tx, nFeeRequired, nChangePosRet, strFailReason, coin_control); bool fCreated = pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, strFailReason, coin_control);
if (!fCreated) if (!fCreated)
throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason); throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, strFailReason);
pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */); pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */);
@ -963,7 +955,6 @@ static UniValue addmultisigaddress(const JSONRPCRequest& request)
LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet); LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet);
auto locked_chain = pwallet->chain().lock();
LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore);
std::string label; std::string label;
@ -1016,7 +1007,7 @@ struct tallyitem
} }
}; };
static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWallet* const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) static UniValue ListReceived(const CWallet* const pwallet, const UniValue& params, bool by_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
{ {
// Minimum confirmations // Minimum confirmations
int nMinDepth = 1; int nMinDepth = 1;
@ -1049,7 +1040,7 @@ static UniValue ListReceived(interfaces::Chain::Lock& locked_chain, const CWalle
for (const std::pair<const uint256, CWalletTx>& pairWtx : pwallet->mapWallet) { for (const std::pair<const uint256, CWalletTx>& pairWtx : pwallet->mapWallet) {
const CWalletTx& wtx = pairWtx.second; const CWalletTx& wtx = pairWtx.second;
if (wtx.IsCoinBase() || !locked_chain.checkFinalTx(*wtx.tx)) { if (wtx.IsCoinBase() || !pwallet->chain().checkFinalTx(*wtx.tx)) {
continue; continue;
} }
@ -1209,10 +1200,9 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
return ListReceived(*locked_chain, pwallet, request.params, false); return ListReceived(pwallet, request.params, false);
} }
static UniValue listreceivedbylabel(const JSONRPCRequest& request) static UniValue listreceivedbylabel(const JSONRPCRequest& request)
@ -1254,10 +1244,9 @@ static UniValue listreceivedbylabel(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
return ListReceived(*locked_chain, pwallet, request.params, true); return ListReceived(pwallet, request.params, true);
} }
static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
@ -1278,7 +1267,7 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest)
* @param filter_ismine The "is mine" filter flags. * @param filter_ismine The "is mine" filter flags.
* @param filter_label Optional label string to filter incoming transactions. * @param filter_label Optional label string to filter incoming transactions.
*/ */
static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) static void ListTransactions(const CWallet* const pwallet, const CWalletTx& wtx, int nMinDepth, bool fLong, UniValue& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)
{ {
CAmount nFee; CAmount nFee;
std::list<COutputEntry> listReceived; std::list<COutputEntry> listReceived;
@ -1307,7 +1296,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle
entry.pushKV("vout", s.vout); entry.pushKV("vout", s.vout);
entry.pushKV("fee", ValueFromAmount(-nFee)); entry.pushKV("fee", ValueFromAmount(-nFee));
if (fLong) if (fLong)
WalletTxToJSON(pwallet->chain(), locked_chain, wtx, entry); WalletTxToJSON(pwallet->chain(), wtx, entry);
entry.pushKV("abandoned", wtx.isAbandoned()); entry.pushKV("abandoned", wtx.isAbandoned());
ret.push_back(entry); ret.push_back(entry);
} }
@ -1349,7 +1338,7 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, const CWalle
} }
entry.pushKV("vout", r.vout); entry.pushKV("vout", r.vout);
if (fLong) if (fLong)
WalletTxToJSON(pwallet->chain(), locked_chain, wtx, entry); WalletTxToJSON(pwallet->chain(), wtx, entry);
ret.push_back(entry); ret.push_back(entry);
} }
} }
@ -1464,7 +1453,6 @@ UniValue listtransactions(const JSONRPCRequest& request)
UniValue ret(UniValue::VARR); UniValue ret(UniValue::VARR);
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
const CWallet::TxItems & txOrdered = pwallet->wtxOrdered; const CWallet::TxItems & txOrdered = pwallet->wtxOrdered;
@ -1473,7 +1461,7 @@ UniValue listtransactions(const JSONRPCRequest& request)
for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it) for (CWallet::TxItems::const_reverse_iterator it = txOrdered.rbegin(); it != txOrdered.rend(); ++it)
{ {
CWalletTx *const pwtx = (*it).second; CWalletTx *const pwtx = (*it).second;
ListTransactions(*locked_chain, pwallet, *pwtx, 0, true, ret, filter, filter_label); ListTransactions(pwallet, *pwtx, 0, true, ret, filter, filter_label);
if ((int)ret.size() >= (nCount+nFrom)) break; if ((int)ret.size() >= (nCount+nFrom)) break;
} }
} }
@ -1557,7 +1545,6 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
// The way the 'height' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0. // The way the 'height' is initialized is just a workaround for the gcc bug #47679 since version 4.6.0.
@ -1598,7 +1585,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
CWalletTx tx = pairWtx.second; CWalletTx tx = pairWtx.second;
if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) { if (depth == -1 || abs(tx.GetDepthInMainChain()) < depth) {
ListTransactions(*locked_chain, pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); ListTransactions(pwallet, tx, 0, true, transactions, filter, nullptr /* filter_label */);
} }
} }
@ -1615,7 +1602,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
if (it != pwallet->mapWallet.end()) { if (it != pwallet->mapWallet.end()) {
// We want all transactions regardless of confirmation count to appear here, // We want all transactions regardless of confirmation count to appear here,
// even negative confirmation ones, hence the big negative. // even negative confirmation ones, hence the big negative.
ListTransactions(*locked_chain, pwallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */); ListTransactions(pwallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */);
} }
} }
blockId = block.hashPrevBlock; blockId = block.hashPrevBlock;
@ -1700,7 +1687,6 @@ static UniValue gettransaction(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
uint256 hash(ParseHashV(request.params[0], "txid")); uint256 hash(ParseHashV(request.params[0], "txid"));
@ -1729,10 +1715,10 @@ static UniValue gettransaction(const JSONRPCRequest& request)
if (wtx.IsFromMe(filter)) if (wtx.IsFromMe(filter))
entry.pushKV("fee", ValueFromAmount(nFee)); entry.pushKV("fee", ValueFromAmount(nFee));
WalletTxToJSON(pwallet->chain(), *locked_chain, wtx, entry); WalletTxToJSON(pwallet->chain(), wtx, entry);
UniValue details(UniValue::VARR); UniValue details(UniValue::VARR);
ListTransactions(*locked_chain, pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */); ListTransactions(pwallet, wtx, 0, false, details, filter, nullptr /* filter_label */);
entry.pushKV("details", details); entry.pushKV("details", details);
std::string strHex = EncodeHexTx(*wtx.tx, pwallet->chain().rpcSerializationFlags()); std::string strHex = EncodeHexTx(*wtx.tx, pwallet->chain().rpcSerializationFlags());
@ -1776,7 +1762,6 @@ static UniValue abandontransaction(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
uint256 hash(ParseHashV(request.params[0], "txid")); uint256 hash(ParseHashV(request.params[0], "txid"));
@ -1817,7 +1802,6 @@ static UniValue backupwallet(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
std::string strDest = request.params[0].get_str(); std::string strDest = request.params[0].get_str();
@ -1855,7 +1839,6 @@ static UniValue keypoolrefill(const JSONRPCRequest& request)
throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet"); throw JSONRPCError(RPC_WALLET_ERROR, "Error: Private keys are disabled for this wallet");
} }
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
// 0 is interpreted by TopUpKeyPool() as the default keypool size given by -keypool // 0 is interpreted by TopUpKeyPool() as the default keypool size given by -keypool
@ -1909,7 +1892,6 @@ static UniValue walletpassphrase(const JSONRPCRequest& request)
int64_t nSleepTime; int64_t nSleepTime;
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
if (!pwallet->IsCrypted()) { if (!pwallet->IsCrypted()) {
@ -1991,7 +1973,6 @@ static UniValue walletpassphrasechange(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
if (!pwallet->IsCrypted()) { if (!pwallet->IsCrypted()) {
@ -2047,7 +2028,6 @@ static UniValue walletlock(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
if (!pwallet->IsCrypted()) { if (!pwallet->IsCrypted()) {
@ -2094,7 +2074,6 @@ static UniValue encryptwallet(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
@ -2173,7 +2152,6 @@ static UniValue lockunspent(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
RPCTypeCheckArgument(request.params[0], UniValue::VBOOL); RPCTypeCheckArgument(request.params[0], UniValue::VBOOL);
@ -2286,7 +2264,6 @@ static UniValue listlockunspent(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
std::vector<COutPoint> vOutpts; std::vector<COutPoint> vOutpts;
@ -2329,7 +2306,6 @@ static UniValue settxfee(const JSONRPCRequest& request)
}, },
}.Check(request); }.Check(request);
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
CAmount nAmount = AmountFromValue(request.params[0]); CAmount nAmount = AmountFromValue(request.params[0]);
@ -2388,7 +2364,6 @@ static UniValue getbalances(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
wallet.BlockUntilSyncedToCurrentChain(); wallet.BlockUntilSyncedToCurrentChain();
auto locked_chain = wallet.chain().lock();
LOCK(wallet.cs_wallet); LOCK(wallet.cs_wallet);
const auto bal = wallet.GetBalance(); const auto bal = wallet.GetBalance();
@ -2465,7 +2440,6 @@ static UniValue getwalletinfo(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
UniValue obj(UniValue::VOBJ); UniValue obj(UniValue::VOBJ);
@ -2940,9 +2914,8 @@ static UniValue listunspent(const JSONRPCRequest& request)
cctl.m_avoid_address_reuse = false; cctl.m_avoid_address_reuse = false;
cctl.m_min_depth = nMinDepth; cctl.m_min_depth = nMinDepth;
cctl.m_max_depth = nMaxDepth; cctl.m_max_depth = nMaxDepth;
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
pwallet->AvailableCoins(*locked_chain, vecOutputs, !include_unsafe, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount); pwallet->AvailableCoins(vecOutputs, !include_unsafe, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount);
} }
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
@ -3312,7 +3285,6 @@ UniValue signrawtransactionwithwallet(const JSONRPCRequest& request)
} }
// Sign the transaction // Sign the transaction
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -3441,7 +3413,6 @@ static UniValue bumpfee(const JSONRPCRequest& request)
// the user could have gotten from another RPC command prior to now // the user could have gotten from another RPC command prior to now
pwallet->BlockUntilSyncedToCurrentChain(); pwallet->BlockUntilSyncedToCurrentChain();
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(pwallet); EnsureWalletIsUnlocked(pwallet);
@ -3548,7 +3519,6 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
Optional<int> stop_height; Optional<int> stop_height;
uint256 start_block; uint256 start_block;
{ {
auto locked_chain = pwallet->chain().lock();
LOCK(pwallet->cs_wallet); LOCK(pwallet->cs_wallet);
int tip_height = pwallet->GetLastBlockHeight(); int tip_height = pwallet->GetLastBlockHeight();
@ -3563,8 +3533,7 @@ UniValue rescanblockchain(const JSONRPCRequest& request)
stop_height = request.params[1].get_int(); stop_height = request.params[1].get_int();
if (*stop_height < 0 || *stop_height > tip_height) { if (*stop_height < 0 || *stop_height > tip_height) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height"); throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid stop_height");
} } else if (*stop_height < start_height) {
else if (*stop_height < start_height) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater than start_height"); throw JSONRPCError(RPC_INVALID_PARAMETER, "stop_height must be greater than start_height");
} }
} }
@ -4011,7 +3980,6 @@ UniValue sethdseed(const JSONRPCRequest& request)
throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed to a wallet with private keys disabled"); throw JSONRPCError(RPC_WALLET_ERROR, "Cannot set a HD seed to a wallet with private keys disabled");
} }
auto locked_chain = pwallet->chain().lock();
LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore);
// Do not do anything to non-HD wallets // Do not do anything to non-HD wallets

111
src/wallet/test/wallet_tests.cpp
View File

@ -75,8 +75,6 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
NodeContext node; NodeContext node;
auto chain = interfaces::MakeChain(node); auto chain = interfaces::MakeChain(node);
auto locked_chain = chain->lock();
LockAssertion lock(::cs_main);
// Verify ScanForWalletTransactions fails to read an unknown start block. // Verify ScanForWalletTransactions fails to read an unknown start block.
{ {
@ -116,7 +114,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
} }
// Prune the older block file. // Prune the older block file.
{
LOCK(cs_main);
PruneOneBlockFile(oldTip->GetBlockPos().nFile); PruneOneBlockFile(oldTip->GetBlockPos().nFile);
}
UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); UnlinkPrunedFiles({oldTip->GetBlockPos().nFile});
// Verify ScanForWalletTransactions only picks transactions in the new block // Verify ScanForWalletTransactions only picks transactions in the new block
@ -139,7 +140,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup)
} }
// Prune the remaining block file. // Prune the remaining block file.
{
LOCK(cs_main);
PruneOneBlockFile(newTip->GetBlockPos().nFile); PruneOneBlockFile(newTip->GetBlockPos().nFile);
}
UnlinkPrunedFiles({newTip->GetBlockPos().nFile}); UnlinkPrunedFiles({newTip->GetBlockPos().nFile});
// Verify ScanForWalletTransactions scans no blocks. // Verify ScanForWalletTransactions scans no blocks.
@ -171,11 +175,12 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
NodeContext node; NodeContext node;
auto chain = interfaces::MakeChain(node); auto chain = interfaces::MakeChain(node);
auto locked_chain = chain->lock();
LockAssertion lock(::cs_main);
// Prune the older block file. // Prune the older block file.
{
LOCK(cs_main);
PruneOneBlockFile(oldTip->GetBlockPos().nFile); PruneOneBlockFile(oldTip->GetBlockPos().nFile);
}
UnlinkPrunedFiles({oldTip->GetBlockPos().nFile}); UnlinkPrunedFiles({oldTip->GetBlockPos().nFile});
// Verify importmulti RPC returns failure for a key whose creation time is // Verify importmulti RPC returns failure for a key whose creation time is
@ -241,8 +246,6 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
NodeContext node; NodeContext node;
auto chain = interfaces::MakeChain(node); auto chain = interfaces::MakeChain(node);
auto locked_chain = chain->lock();
LockAssertion lock(::cs_main);
std::string backup_file = (GetDataDir() / "wallet.backup").string(); std::string backup_file = (GetDataDir() / "wallet.backup").string();
@ -308,8 +311,6 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan();
CWalletTx wtx(&wallet, m_coinbase_txns.back()); CWalletTx wtx(&wallet, m_coinbase_txns.back());
auto locked_chain = chain->lock();
LockAssertion lock(::cs_main);
LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore); LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore);
wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
@ -334,8 +335,6 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
SetMockTime(mockTime); SetMockTime(mockTime);
CBlockIndex* block = nullptr; CBlockIndex* block = nullptr;
if (blockTime > 0) { if (blockTime > 0) {
auto locked_chain = wallet.chain().lock();
LockAssertion lock(::cs_main);
auto inserted = ::BlockIndex().emplace(GetRandHash(), new CBlockIndex); auto inserted = ::BlockIndex().emplace(GetRandHash(), new CBlockIndex);
assert(inserted.second); assert(inserted.second);
const uint256& hash = inserted.first->first; const uint256& hash = inserted.first->first;
@ -345,7 +344,6 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
} }
CWalletTx wtx(&wallet, MakeTransactionRef(tx)); CWalletTx wtx(&wallet, MakeTransactionRef(tx));
LOCK(cs_main);
LOCK(wallet.cs_wallet); LOCK(wallet.cs_wallet);
// If transaction is already in map, to avoid inconsistencies, unconfirmation // If transaction is already in map, to avoid inconsistencies, unconfirmation
// is needed before confirm again with different block. // is needed before confirm again with different block.
@ -492,7 +490,7 @@ public:
CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock()); wallet = MakeUnique<CWallet>(m_chain.get(), WalletLocation(), WalletDatabase::CreateMock());
{ {
LOCK2(::cs_main, wallet->cs_wallet); LOCK2(wallet->cs_wallet, ::cs_main);
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
} }
bool firstRun; bool firstRun;
@ -520,8 +518,7 @@ public:
std::string error; std::string error;
CCoinControl dummy; CCoinControl dummy;
{ {
auto locked_chain = m_chain->lock(); BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy));
BOOST_CHECK(wallet->CreateTransaction(*locked_chain, {recipient}, tx, fee, changePos, error, dummy));
} }
wallet->CommitTransaction(tx, {}, {}); wallet->CommitTransaction(tx, {}, {});
CMutableTransaction blocktx; CMutableTransaction blocktx;
@ -531,7 +528,6 @@ public:
} }
CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
LOCK(cs_main);
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash()); wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash());
auto it = wallet->mapWallet.find(tx->GetHash()); auto it = wallet->mapWallet.find(tx->GetHash());
@ -554,9 +550,8 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
// address. // address.
std::map<CTxDestination, std::vector<COutput>> list; std::map<CTxDestination, std::vector<COutput>> list;
{ {
auto locked_chain = m_chain->lock();
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
list = wallet->ListCoins(*locked_chain); list = wallet->ListCoins();
} }
BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(list.size(), 1U);
BOOST_CHECK_EQUAL(boost::get<PKHash>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(boost::get<PKHash>(list.begin()->first).ToString(), coinbaseAddress);
@ -571,9 +566,8 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
// pubkey. // pubkey.
AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */}); AddTx(CRecipient{GetScriptForRawPubKey({}), 1 * COIN, false /* subtract fee */});
{ {
auto locked_chain = m_chain->lock();
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
list = wallet->ListCoins(*locked_chain); list = wallet->ListCoins();
} }
BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(list.size(), 1U);
BOOST_CHECK_EQUAL(boost::get<PKHash>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(boost::get<PKHash>(list.begin()->first).ToString(), coinbaseAddress);
@ -581,10 +575,9 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
// Lock both coins. Confirm number of available coins drops to 0. // Lock both coins. Confirm number of available coins drops to 0.
{ {
auto locked_chain = m_chain->lock();
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
std::vector<COutput> available; std::vector<COutput> available;
wallet->AvailableCoins(*locked_chain, available); wallet->AvailableCoins(available);
BOOST_CHECK_EQUAL(available.size(), 2U); BOOST_CHECK_EQUAL(available.size(), 2U);
} }
for (const auto& group : list) { for (const auto& group : list) {
@ -594,18 +587,16 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
} }
} }
{ {
auto locked_chain = m_chain->lock();
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
std::vector<COutput> available; std::vector<COutput> available;
wallet->AvailableCoins(*locked_chain, available); wallet->AvailableCoins(available);
BOOST_CHECK_EQUAL(available.size(), 0U); BOOST_CHECK_EQUAL(available.size(), 0U);
} }
// Confirm ListCoins still returns same result as before, despite coins // Confirm ListCoins still returns same result as before, despite coins
// being locked. // being locked.
{ {
auto locked_chain = m_chain->lock();
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
list = wallet->ListCoins(*locked_chain); list = wallet->ListCoins();
} }
BOOST_CHECK_EQUAL(list.size(), 1U); BOOST_CHECK_EQUAL(list.size(), 1U);
BOOST_CHECK_EQUAL(boost::get<PKHash>(list.begin()->first).ToString(), coinbaseAddress); BOOST_CHECK_EQUAL(boost::get<PKHash>(list.begin()->first).ToString(), coinbaseAddress);
@ -694,11 +685,20 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
//! conditions if it's called the same time an incoming transaction shows up in //! conditions if it's called the same time an incoming transaction shows up in
//! the mempool or a new block. //! the mempool or a new block.
//! //!
//! It isn't possible for a unit test to totally verify there aren't race //! It isn't possible to verify there aren't race condition in every case, so
//! conditions without hooking into the implementation more, so this test just //! this test just checks two specific cases and ensures that timing of
//! verifies that new transactions are detected during loading without any //! notifications in these cases doesn't prevent the wallet from detecting
//! notifications at all, to infer that timing of notifications shouldn't //! transactions.
//! matter. The test could be extended to cover other scenarios in the future. //!
//! In the first case, block and mempool transactions are created before the
//! wallet is loaded, but notifications about these transactions are delayed
//! until after it is loaded. The notifications are superfluous in this case, so
//! the test verifies the transactions are detected before they arrive.
//!
//! In the second case, block and mempool transactions are created after the
//! wallet rescan and notifications are immediately synced, to verify the wallet
//! must already have a handler in place for them, and there's no gap after
//! rescanning where new transactions in new blocks could be lost.
BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
{ {
// Create new wallet with known key and unload it. // Create new wallet with known key and unload it.
@ -709,6 +709,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
AddKey(*wallet, key); AddKey(*wallet, key);
TestUnloadWallet(std::move(wallet)); TestUnloadWallet(std::move(wallet));
// Add log hook to detect AddToWallet events from rescans, blockConnected, // Add log hook to detect AddToWallet events from rescans, blockConnected,
// and transactionAddedToMempool notifications // and transactionAddedToMempool notifications
int addtx_count = 0; int addtx_count = 0;
@ -717,21 +718,14 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
return false; return false;
}); });
bool rescan_completed = false; bool rescan_completed = false;
DebugLogHelper rescan_check("[default wallet] Rescan completed", [&](const std::string* s) { DebugLogHelper rescan_check("[default wallet] Rescan completed", [&](const std::string* s) {
if (s) { if (s) rescan_completed = true;
// For now, just assert that cs_main is being held during the
// rescan, ensuring that a new block couldn't be connected
// that the wallet would miss. After
// https://github.com/bitcoin/bitcoin/pull/16426 when cs_main is no
// longer held, the test can be extended to append a new block here
// and check it's handled correctly.
AssertLockHeld(::cs_main);
rescan_completed = true;
}
return false; return false;
}); });
// Block the queue to prevent the wallet receiving blockConnected and // Block the queue to prevent the wallet receiving blockConnected and
// transactionAddedToMempool notifications, and create block and mempool // transactionAddedToMempool notifications, and create block and mempool
// transactions paying to the wallet // transactions paying to the wallet
@ -746,29 +740,56 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup)
auto mempool_tx = TestSimpleSpend(*m_coinbase_txns[1], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey())); auto mempool_tx = TestSimpleSpend(*m_coinbase_txns[1], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error)); BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
// Reload wallet and make sure new transactions are detected despite events // Reload wallet and make sure new transactions are detected despite events
// being blocked // being blocked
wallet = TestLoadWallet(*chain); wallet = TestLoadWallet(*chain);
BOOST_CHECK(rescan_completed); BOOST_CHECK(rescan_completed);
BOOST_CHECK_EQUAL(addtx_count, 2); BOOST_CHECK_EQUAL(addtx_count, 2);
unsigned int block_tx_time, mempool_tx_time;
{ {
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
block_tx_time = wallet->mapWallet.at(block_tx.GetHash()).nTimeReceived; BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1);
mempool_tx_time = wallet->mapWallet.at(mempool_tx.GetHash()).nTimeReceived; BOOST_CHECK_EQUAL(wallet->mapWallet.count(mempool_tx.GetHash()), 1);
} }
// Unblock notification queue and make sure stale blockConnected and // Unblock notification queue and make sure stale blockConnected and
// transactionAddedToMempool events are processed // transactionAddedToMempool events are processed
promise.set_value(); promise.set_value();
SyncWithValidationInterfaceQueue(); SyncWithValidationInterfaceQueue();
BOOST_CHECK_EQUAL(addtx_count, 4); BOOST_CHECK_EQUAL(addtx_count, 4);
TestUnloadWallet(std::move(wallet));
// Load wallet again, this time creating new block and mempool transactions
// paying to the wallet as the wallet finishes loading and syncing the
// queue so the events have to be handled immediately. Releasing the wallet
// lock during the sync is a little artificial but is needed to avoid a
// deadlock during the sync and simulates a new block notification happening
// as soon as possible.
addtx_count = 0;
auto handler = HandleLoadWallet([&](std::unique_ptr<interfaces::Wallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->wallet()->cs_wallet) {
BOOST_CHECK(rescan_completed);
m_coinbase_txns.push_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
block_tx = TestSimpleSpend(*m_coinbase_txns[2], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
m_coinbase_txns.push_back(CreateAndProcessBlock({block_tx}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]);
mempool_tx = TestSimpleSpend(*m_coinbase_txns[3], 0, coinbaseKey, GetScriptForRawPubKey(key.GetPubKey()));
BOOST_CHECK(chain->broadcastTransaction(MakeTransactionRef(mempool_tx), DEFAULT_TRANSACTION_MAXFEE, false, error));
LEAVE_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
SyncWithValidationInterfaceQueue();
ENTER_CRITICAL_SECTION(wallet->wallet()->cs_wallet);
});
wallet = TestLoadWallet(*chain);
BOOST_CHECK_EQUAL(addtx_count, 4);
{ {
LOCK(wallet->cs_wallet); LOCK(wallet->cs_wallet);
BOOST_CHECK_EQUAL(block_tx_time, wallet->mapWallet.at(block_tx.GetHash()).nTimeReceived); BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1);
BOOST_CHECK_EQUAL(mempool_tx_time, wallet->mapWallet.at(mempool_tx.GetHash()).nTimeReceived); BOOST_CHECK_EQUAL(wallet->mapWallet.count(mempool_tx.GetHash()), 1);
} }
TestUnloadWallet(std::move(wallet)); TestUnloadWallet(std::move(wallet));
} }

104
src/wallet/wallet.cpp
View File

@ -885,10 +885,9 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose)
void CWallet::LoadToWallet(CWalletTx& wtxIn) void CWallet::LoadToWallet(CWalletTx& wtxIn)
{ {
// If wallet doesn't have a chain (e.g bitcoin-wallet), lock can't be taken. // If wallet doesn't have a chain (e.g wallet-tool), don't bother to update txn.
auto locked_chain = LockChain(); if (HaveChain()) {
if (locked_chain) { Optional<int> block_height = chain().getBlockHeight(wtxIn.m_confirm.hashBlock);
Optional<int> block_height = locked_chain->getBlockHeight(wtxIn.m_confirm.hashBlock);
if (block_height) { if (block_height) {
// Update cached block height variable since it not stored in the // Update cached block height variable since it not stored in the
// serialized transaction. // serialized transaction.
@ -975,7 +974,6 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co
bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const bool CWallet::TransactionCanBeAbandoned(const uint256& hashTx) const
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
const CWalletTx* wtx = GetWalletTx(hashTx); const CWalletTx* wtx = GetWalletTx(hashTx);
return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool(); return wtx && !wtx->isAbandoned() && wtx->GetDepthInMainChain() == 0 && !wtx->InMempool();
@ -993,7 +991,6 @@ void CWallet::MarkInputsDirty(const CTransactionRef& tx)
bool CWallet::AbandonTransaction(const uint256& hashTx) bool CWallet::AbandonTransaction(const uint256& hashTx)
{ {
auto locked_chain = chain().lock(); // Temporary. Removed in upcoming lock cleanup
LOCK(cs_wallet); LOCK(cs_wallet);
WalletBatch batch(*database, "r+"); WalletBatch batch(*database, "r+");
@ -1048,7 +1045,6 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx) void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx)
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1; int conflictconfirms = (m_last_block_processed_height - conflicting_height + 1) * -1;
@ -1111,7 +1107,6 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio
} }
void CWallet::transactionAddedToMempool(const CTransactionRef& ptx) { void CWallet::transactionAddedToMempool(const CTransactionRef& ptx) {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
SyncTransaction(ptx, confirm); SyncTransaction(ptx, confirm);
@ -1133,7 +1128,6 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx) {
void CWallet::blockConnected(const CBlock& block, int height) void CWallet::blockConnected(const CBlock& block, int height)
{ {
const uint256& block_hash = block.GetHash(); const uint256& block_hash = block.GetHash();
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
m_last_block_processed_height = height; m_last_block_processed_height = height;
@ -1147,7 +1141,6 @@ void CWallet::blockConnected(const CBlock& block, int height)
void CWallet::blockDisconnected(const CBlock& block, int height) void CWallet::blockDisconnected(const CBlock& block, int height)
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
// At block disconnection, this will change an abandoned transaction to // At block disconnection, this will change an abandoned transaction to
@ -1686,7 +1679,6 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
uint256 next_block_hash; uint256 next_block_hash;
bool reorg = false; bool reorg = false;
if (chain().findBlock(block_hash, FoundBlock().data(block)) && !block.IsNull()) { if (chain().findBlock(block_hash, FoundBlock().data(block)) && !block.IsNull()) {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
next_block = chain().findNextBlock(block_hash, block_height, FoundBlock().hash(next_block_hash), &reorg); next_block = chain().findNextBlock(block_hash, block_height, FoundBlock().hash(next_block_hash), &reorg);
if (reorg) { if (reorg) {
@ -1715,7 +1707,6 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
break; break;
} }
{ {
auto locked_chain = chain().lock();
if (!next_block || reorg) { if (!next_block || reorg) {
// break successfully when rescan has reached the tip, or // break successfully when rescan has reached the tip, or
// previous block is no longer on the chain due to a reorg // previous block is no longer on the chain due to a reorg
@ -1928,16 +1919,16 @@ bool CWalletTx::InMempool() const
return fInMempool; return fInMempool;
} }
bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain) const bool CWalletTx::IsTrusted() const
{ {
std::set<uint256> s; std::set<uint256> s;
return IsTrusted(locked_chain, s); return IsTrusted(s);
} }
bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain, std::set<uint256>& trusted_parents) const bool CWalletTx::IsTrusted(std::set<uint256>& trusted_parents) const
{ {
// Quick answer in most cases // Quick answer in most cases
if (!locked_chain.checkFinalTx(*tx)) return false; if (!pwallet->chain().checkFinalTx(*tx)) return false;
int nDepth = GetDepthInMainChain(); int nDepth = GetDepthInMainChain();
if (nDepth >= 1) return true; if (nDepth >= 1) return true;
if (nDepth < 0) return false; if (nDepth < 0) return false;
@ -1959,7 +1950,7 @@ bool CWalletTx::IsTrusted(interfaces::Chain::Lock& locked_chain, std::set<uint25
// If we've already trusted this parent, continue // If we've already trusted this parent, continue
if (trusted_parents.count(parent->GetHash())) continue; if (trusted_parents.count(parent->GetHash())) continue;
// Recurse to check that the parent is also trusted // Recurse to check that the parent is also trusted
if (!parent->IsTrusted(locked_chain, trusted_parents)) return false; if (!parent->IsTrusted(trusted_parents)) return false;
trusted_parents.insert(parent->GetHash()); trusted_parents.insert(parent->GetHash());
} }
return true; return true;
@ -2003,8 +1994,7 @@ void CWallet::ResendWalletTransactions()
int submitted_tx_count = 0; int submitted_tx_count = 0;
{ // locked_chain and cs_wallet scope { // cs_wallet scope
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
// Relay transactions // Relay transactions
@ -2017,7 +2007,7 @@ void CWallet::ResendWalletTransactions()
std::string unused_err_string; std::string unused_err_string;
if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count; if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count;
} }
} // locked_chain and cs_wallet } // cs_wallet
if (submitted_tx_count > 0) { if (submitted_tx_count > 0) {
WalletLogPrintf("%s: resubmit %u unconfirmed transactions\n", __func__, submitted_tx_count); WalletLogPrintf("%s: resubmit %u unconfirmed transactions\n", __func__, submitted_tx_count);
@ -2045,13 +2035,12 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons
Balance ret; Balance ret;
isminefilter reuse_filter = avoid_reuse ? ISMINE_NO : ISMINE_USED; isminefilter reuse_filter = avoid_reuse ? ISMINE_NO : ISMINE_USED;
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
std::set<uint256> trusted_parents; std::set<uint256> trusted_parents;
for (const auto& entry : mapWallet) for (const auto& entry : mapWallet)
{ {
const CWalletTx& wtx = entry.second; const CWalletTx& wtx = entry.second;
const bool is_trusted{wtx.IsTrusted(*locked_chain, trusted_parents)}; const bool is_trusted{wtx.IsTrusted(trusted_parents)};
const int tx_depth{wtx.GetDepthInMainChain()}; const int tx_depth{wtx.GetDepthInMainChain()};
const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)}; const CAmount tx_credit_mine{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_SPENDABLE | reuse_filter)};
const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)}; const CAmount tx_credit_watchonly{wtx.GetAvailableCredit(/* fUseCache */ true, ISMINE_WATCH_ONLY | reuse_filter)};
@ -2072,12 +2061,11 @@ CWallet::Balance CWallet::GetBalance(const int min_depth, bool avoid_reuse) cons
CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
CAmount balance = 0; CAmount balance = 0;
std::vector<COutput> vCoins; std::vector<COutput> vCoins;
AvailableCoins(*locked_chain, vCoins, true, coinControl); AvailableCoins(vCoins, true, coinControl);
for (const COutput& out : vCoins) { for (const COutput& out : vCoins) {
if (out.fSpendable) { if (out.fSpendable) {
balance += out.tx->tx->vout[out.i].nValue; balance += out.tx->tx->vout[out.i].nValue;
@ -2086,7 +2074,7 @@ CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
return balance; return balance;
} }
void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<COutput>& vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount) const void CWallet::AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe, const CCoinControl* coinControl, const CAmount& nMinimumAmount, const CAmount& nMaximumAmount, const CAmount& nMinimumSumAmount, const uint64_t nMaximumCount) const
{ {
AssertLockHeld(cs_wallet); AssertLockHeld(cs_wallet);
@ -2104,7 +2092,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
const uint256& wtxid = entry.first; const uint256& wtxid = entry.first;
const CWalletTx& wtx = entry.second; const CWalletTx& wtx = entry.second;
if (!locked_chain.checkFinalTx(*wtx.tx)) { if (!chain().checkFinalTx(*wtx.tx)) {
continue; continue;
} }
@ -2120,7 +2108,7 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
if (nDepth == 0 && !wtx.InMempool()) if (nDepth == 0 && !wtx.InMempool())
continue; continue;
bool safeTx = wtx.IsTrusted(locked_chain, trusted_parents); bool safeTx = wtx.IsTrusted(trusted_parents);
// We should not consider coins from transactions that are replacing // We should not consider coins from transactions that are replacing
// other transactions. // other transactions.
@ -2208,14 +2196,14 @@ void CWallet::AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<
} }
} }
std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins(interfaces::Chain::Lock& locked_chain) const std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const
{ {
AssertLockHeld(cs_wallet); AssertLockHeld(cs_wallet);
std::map<CTxDestination, std::vector<COutput>> result; std::map<CTxDestination, std::vector<COutput>> result;
std::vector<COutput> availableCoins; std::vector<COutput> availableCoins;
AvailableCoins(locked_chain, availableCoins); AvailableCoins(availableCoins);
for (const COutput& coin : availableCoins) { for (const COutput& coin : availableCoins) {
CTxDestination address; CTxDestination address;
@ -2551,11 +2539,10 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
// Acquire the locks to prevent races to the new locked unspents between the // Acquire the locks to prevent races to the new locked unspents between the
// CreateTransaction call and LockCoin calls (when lockUnspents is true). // CreateTransaction call and LockCoin calls (when lockUnspents is true).
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
CTransactionRef tx_new; CTransactionRef tx_new;
if (!CreateTransaction(*locked_chain, vecSend, tx_new, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { if (!CreateTransaction(vecSend, tx_new, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
return false; return false;
} }
@ -2671,8 +2658,7 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec
return m_default_address_type; return m_default_address_type;
} }
bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
{ {
CAmount nValue = 0; CAmount nValue = 0;
const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend); const OutputType change_type = TransactionChangeType(coin_control.m_change_type ? *coin_control.m_change_type : m_default_change_type, vecSend);
@ -2703,12 +2689,11 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
int nBytes; int nBytes;
{ {
std::set<CInputCoin> setCoins; std::set<CInputCoin> setCoins;
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
txNew.nLockTime = GetLocktimeForNewTransaction(chain(), GetLastBlockHash(), GetLastBlockHeight()); txNew.nLockTime = GetLocktimeForNewTransaction(chain(), GetLastBlockHash(), GetLastBlockHeight());
{ {
std::vector<COutput> vAvailableCoins; std::vector<COutput> vAvailableCoins;
AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); AvailableCoins(vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0);
CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy
// Create change script that will be used if we need change // Create change script that will be used if we need change
@ -3021,7 +3006,6 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std
void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm) void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm)
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
CWalletTx wtxNew(this, std::move(tx)); CWalletTx wtxNew(this, std::move(tx));
@ -3061,11 +3045,6 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
DBErrors CWallet::LoadWallet(bool& fFirstRunRet) DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
{ {
// Even if we don't use this lock in this function, we want to preserve
// lock order in LoadToWallet if query of chain state is needed to know
// tx status. If lock can't be taken (e.g bitcoin-wallet), tx confirmation
// status may be not reliable.
auto locked_chain = LockChain();
LOCK(cs_wallet); LOCK(cs_wallet);
fFirstRunRet = false; fFirstRunRet = false;
@ -3284,7 +3263,7 @@ void CWallet::MarkDestinationsDirty(const std::set<CTxDestination>& destinations
} }
} }
std::map<CTxDestination, CAmount> CWallet::GetAddressBalances(interfaces::Chain::Lock& locked_chain) const std::map<CTxDestination, CAmount> CWallet::GetAddressBalances() const
{ {
std::map<CTxDestination, CAmount> balances; std::map<CTxDestination, CAmount> balances;
@ -3295,7 +3274,7 @@ std::map<CTxDestination, CAmount> CWallet::GetAddressBalances(interfaces::Chain:
{ {
const CWalletTx& wtx = walletEntry.second; const CWalletTx& wtx = walletEntry.second;
if (!wtx.IsTrusted(locked_chain, trusted_parents)) if (!wtx.IsTrusted(trusted_parents))
continue; continue;
if (wtx.IsImmatureCoinBase()) if (wtx.IsImmatureCoinBase())
@ -3511,7 +3490,7 @@ void CWallet::ListLockedCoins(std::vector<COutPoint>& vOutpts) const
/** @} */ // end of Actions /** @} */ // end of Actions
void CWallet::GetKeyBirthTimes(interfaces::Chain::Lock& locked_chain, std::map<CKeyID, int64_t>& mapKeyBirth) const { void CWallet::GetKeyBirthTimes(std::map<CKeyID, int64_t>& mapKeyBirth) const {
AssertLockHeld(cs_wallet); AssertLockHeld(cs_wallet);
mapKeyBirth.clear(); mapKeyBirth.clear();
@ -3721,11 +3700,6 @@ bool CWallet::Verify(interfaces::Chain& chain, const WalletLocation& location, b
// Recover readable keypairs: // Recover readable keypairs:
CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy()); CWallet dummyWallet(&chain, WalletLocation(), WalletDatabase::CreateDummy());
std::string backup_filename; std::string backup_filename;
// Even if we don't use this lock in this function, we want to preserve
// lock order in LoadToWallet if query of chain state is needed to know
// tx status. If lock can't be taken, tx confirmation status may be not
// reliable.
auto locked_chain = dummyWallet.LockChain();
if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) { if (!WalletBatch::Recover(wallet_path, (void *)&dummyWallet, WalletBatch::RecoverKeysOnlyFilter, backup_filename)) {
return false; return false;
} }
@ -3814,8 +3788,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
} }
} }
auto locked_chain = chain.lock(); walletInstance->chainStateFlushed(chain.getTipLocator());
walletInstance->chainStateFlushed(locked_chain->getTipLocator());
} else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) { } else if (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS) {
// Make it impossible to disable private keys after creation // Make it impossible to disable private keys after creation
error = strprintf(_("Error loading %s: Private keys can only be disabled during creation").translated, walletFile); error = strprintf(_("Error loading %s: Private keys can only be disabled during creation").translated, walletFile);
@ -3928,24 +3901,33 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
// Try to top up keypool. No-op if the wallet is locked. // Try to top up keypool. No-op if the wallet is locked.
walletInstance->TopUpKeyPool(); walletInstance->TopUpKeyPool();
auto locked_chain = chain.lock();
LOCK(walletInstance->cs_wallet); LOCK(walletInstance->cs_wallet);
// Register wallet with validationinterface. It's done before rescan to avoid
// missing block connections between end of rescan and validation subscribing.
// Because of wallet lock being hold, block connection notifications are going to
// be pending on the validation-side until lock release. It's likely to have
// block processing duplicata (if rescan block range overlaps with notification one)
// but we guarantee at least than wallet state is correct after notifications delivery.
// This is temporary until rescan and notifications delivery are unified under same
// interface.
walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance);
int rescan_height = 0; int rescan_height = 0;
if (!gArgs.GetBoolArg("-rescan", false)) if (!gArgs.GetBoolArg("-rescan", false))
{ {
WalletBatch batch(*walletInstance->database); WalletBatch batch(*walletInstance->database);
CBlockLocator locator; CBlockLocator locator;
if (batch.ReadBestBlock(locator)) { if (batch.ReadBestBlock(locator)) {
if (const Optional<int> fork_height = locked_chain->findLocatorFork(locator)) { if (const Optional<int> fork_height = chain.findLocatorFork(locator)) {
rescan_height = *fork_height; rescan_height = *fork_height;
} }
} }
} }
const Optional<int> tip_height = locked_chain->getHeight(); const Optional<int> tip_height = chain.getHeight();
if (tip_height) { if (tip_height) {
walletInstance->m_last_block_processed = locked_chain->getBlockHash(*tip_height); walletInstance->m_last_block_processed = chain.getBlockHash(*tip_height);
walletInstance->m_last_block_processed_height = *tip_height; walletInstance->m_last_block_processed_height = *tip_height;
} else { } else {
walletInstance->m_last_block_processed.SetNull(); walletInstance->m_last_block_processed.SetNull();
@ -3962,7 +3944,7 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
// If a block is pruned after this check, we will load the wallet, // If a block is pruned after this check, we will load the wallet,
// but fail the rescan with a generic error. // but fail the rescan with a generic error.
int block_height = *tip_height; int block_height = *tip_height;
while (block_height > 0 && locked_chain->haveBlockOnDisk(block_height - 1) && rescan_height != block_height) { while (block_height > 0 && chain.haveBlockOnDisk(block_height - 1) && rescan_height != block_height) {
--block_height; --block_height;
} }
@ -3984,19 +3966,19 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
if (!time_first_key || time < *time_first_key) time_first_key = time; if (!time_first_key || time < *time_first_key) time_first_key = time;
} }
if (time_first_key) { if (time_first_key) {
if (Optional<int> first_block = locked_chain->findFirstBlockWithTimeAndHeight(*time_first_key - TIMESTAMP_WINDOW, rescan_height, nullptr)) { if (Optional<int> first_block = chain.findFirstBlockWithTimeAndHeight(*time_first_key - TIMESTAMP_WINDOW, rescan_height, nullptr)) {
rescan_height = *first_block; rescan_height = *first_block;
} }
} }
{ {
WalletRescanReserver reserver(*walletInstance); WalletRescanReserver reserver(*walletInstance);
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(locked_chain->getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) { if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, {} /* max height */, reserver, true /* update */).status)) {
error = _("Failed to rescan the wallet during initialization").translated; error = _("Failed to rescan the wallet during initialization").translated;
return nullptr; return nullptr;
} }
} }
walletInstance->chainStateFlushed(locked_chain->getTipLocator()); walletInstance->chainStateFlushed(chain.getTipLocator());
walletInstance->database->IncrementUpdateCounter(); walletInstance->database->IncrementUpdateCounter();
// Restore wallet transaction metadata after -zapwallettxes=1 // Restore wallet transaction metadata after -zapwallettxes=1
@ -4031,9 +4013,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
} }
} }
// Register with the validation interface. It's ok to do this after rescan since we're still holding locked_chain.
walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance);
walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST)); walletInstance->SetBroadcastTransactions(gArgs.GetBoolArg("-walletbroadcast", DEFAULT_WALLETBROADCAST));
{ {
@ -4093,7 +4072,6 @@ bool CWallet::UpgradeWallet(int version, std::string& error, std::vector<std::st
void CWallet::postInitProcess() void CWallet::postInitProcess()
{ {
auto locked_chain = chain().lock();
LOCK(cs_wallet); LOCK(cs_wallet);
// Add wallet transactions that aren't already in a block to mempool // Add wallet transactions that aren't already in a block to mempool

19
src/wallet/wallet.h
View File

@ -499,8 +499,8 @@ public:
bool IsEquivalentTo(const CWalletTx& tx) const; bool IsEquivalentTo(const CWalletTx& tx) const;
bool InMempool() const; bool InMempool() const;
bool IsTrusted(interfaces::Chain::Lock& locked_chain) const; bool IsTrusted() const;
bool IsTrusted(interfaces::Chain::Lock& locked_chain, std::set<uint256>& trusted_parents) const; bool IsTrusted(std::set<uint256>& trusted_parents) const;
int64_t GetTxTime() const; int64_t GetTxTime() const;
@ -775,8 +775,8 @@ public:
bool IsLocked() const override; bool IsLocked() const override;
bool Lock(); bool Lock();
/** Interface to assert chain access and if successful lock it */ /** Interface to assert chain access */
std::unique_ptr<interfaces::Chain::Lock> LockChain() { return m_chain ? m_chain->lock() : nullptr; } bool HaveChain() const { return m_chain ? true : false; }
std::map<uint256, CWalletTx> mapWallet GUARDED_BY(cs_wallet); std::map<uint256, CWalletTx> mapWallet GUARDED_BY(cs_wallet);
@ -805,12 +805,12 @@ public:
/** /**
* populate vCoins with vector of available COutputs. * populate vCoins with vector of available COutputs.
*/ */
void AvailableCoins(interfaces::Chain::Lock& locked_chain, std::vector<COutput>& vCoins, bool fOnlySafe = true, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe = true, const CCoinControl* coinControl = nullptr, const CAmount& nMinimumAmount = 1, const CAmount& nMaximumAmount = MAX_MONEY, const CAmount& nMinimumSumAmount = MAX_MONEY, const uint64_t nMaximumCount = 0) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/** /**
* 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(interfaces::Chain::Lock& locked_chain) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::map<CTxDestination, std::vector<COutput>> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/** /**
* Find non-change parent output. * Find non-change parent output.
@ -875,7 +875,7 @@ public:
bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase);
bool EncryptWallet(const SecureString& strWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase);
void GetKeyBirthTimes(interfaces::Chain::Lock& locked_chain, std::map<CKeyID, int64_t> &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void GetKeyBirthTimes(std::map<CKeyID, int64_t> &mapKeyBirth) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
unsigned int ComputeTimeSmart(const CWalletTx& wtx) const; unsigned int ComputeTimeSmart(const CWalletTx& wtx) const;
/** /**
@ -961,8 +961,7 @@ public:
* selected by SelectCoins(); Also create the change output, when needed * selected by SelectCoins(); Also create the change output, when needed
* @note passing nChangePosInOut as -1 will result in setting a random position * @note passing nChangePosInOut as -1 will result in setting a random position
*/ */
bool CreateTransaction(interfaces::Chain::Lock& locked_chain, const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bool CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign = true);
std::string& strFailReason, const CCoinControl& coin_control, bool sign = true);
/** /**
* Submit the transaction to the node's mempool and then relay to peers. * Submit the transaction to the node's mempool and then relay to peers.
* Should be called after CreateTransaction unless you want to abort * Should be called after CreateTransaction unless you want to abort
@ -1012,7 +1011,7 @@ public:
int64_t GetOldestKeyPoolTime() const; int64_t GetOldestKeyPoolTime() const;
std::set<std::set<CTxDestination>> GetAddressGroupings() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::set<std::set<CTxDestination>> GetAddressGroupings() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
std::map<CTxDestination, CAmount> GetAddressBalances(interfaces::Chain::Lock& locked_chain) const; std::map<CTxDestination, CAmount> GetAddressBalances() const;
std::set<CTxDestination> GetLabelAddresses(const std::string& label) const; std::set<CTxDestination> GetLabelAddresses(const std::string& label) const;