Skip to content

Commit

Permalink
Pass chain locked variables where needed
Browse files Browse the repository at this point in the history 
This commit does not change behavior. All it does is pass new function
parameters.

It is easiest to review this change with:

    git log -p -n1 -U0 --word-diff-regex=.
  • Loading branch information
@ryanofsky
ryanofsky committed Nov 6, 2018
1 parent 79d579f commit 081accb
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 152 deletions.
48 changes: 27 additions & 21 deletions src/interfaces/wallet.cpp
View file
Expand Up @@ -69,7 +69,7 @@ class PendingWalletTxImpl : public PendingWalletTx
};

//! Construct wallet tx struct.
static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
WalletTx MakeWalletTx(interfaces::Chain::Lock& locked_chain, CWallet& wallet, const CWalletTx& wtx)
{
WalletTx result;
result.tx = wtx.tx;
Expand All @@ -87,7 +87,7 @@ static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LO
IsMine(wallet, result.txout_address.back()) :
ISMINE_NO);
}
result.credit = wtx.GetCredit(ISMINE_ALL);
result.credit = wtx.GetCredit(locked_chain, ISMINE_ALL);
result.debit = wtx.GetDebit(ISMINE_ALL);
result.change = wtx.GetChange();
result.time = wtx.GetTxTime();
Expand All @@ -97,32 +97,38 @@ static WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LO
}

//! Construct wallet tx status struct.
static WalletTxStatus MakeWalletTxStatus(const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
WalletTxStatus MakeWalletTxStatus(interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx)
{
LockAnnotation lock(::cs_main); // Temporary, for CheckFinalTx below. Removed in upcoming commit.

WalletTxStatus result;
auto mi = ::mapBlockIndex.find(wtx.hashBlock);
CBlockIndex* block = mi != ::mapBlockIndex.end() ? mi->second : nullptr;
result.block_height = (block ? block->nHeight : std::numeric_limits<int>::max());
result.blocks_to_maturity = wtx.GetBlocksToMaturity();
result.depth_in_main_chain = wtx.GetDepthInMainChain();
result.blocks_to_maturity = wtx.GetBlocksToMaturity(locked_chain);
result.depth_in_main_chain = wtx.GetDepthInMainChain(locked_chain);
result.time_received = wtx.nTimeReceived;
result.lock_time = wtx.tx->nLockTime;
result.is_final = CheckFinalTx(*wtx.tx);
result.is_trusted = wtx.IsTrusted();
result.is_trusted = wtx.IsTrusted(locked_chain);
result.is_abandoned = wtx.isAbandoned();
result.is_coinbase = wtx.IsCoinBase();
result.is_in_main_chain = wtx.IsInMainChain();
result.is_in_main_chain = wtx.IsInMainChain(locked_chain);
return result;
}

//! Construct wallet TxOut struct.
static WalletTxOut MakeWalletTxOut(CWallet& wallet, const CWalletTx& wtx, int n, int depth) EXCLUSIVE_LOCKS_REQUIRED(cs_main, wallet.cs_wallet)
WalletTxOut MakeWalletTxOut(interfaces::Chain::Lock& locked_chain,
CWallet& wallet,
const CWalletTx& wtx,
int n,
int depth) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
{
WalletTxOut result;
result.txout = wtx.tx->vout[n];
result.time = wtx.GetTxTime();
result.depth_in_main_chain = depth;
result.is_spent = wallet.IsSpent(wtx.GetHash(), n);
result.is_spent = wallet.IsSpent(locked_chain, wtx.GetHash(), n);
return result;
}

Expand Down Expand Up @@ -242,7 +248,7 @@ class WalletImpl : public Wallet
auto locked_chain = m_wallet.chain().lock();
LOCK(m_wallet.cs_wallet);
auto pending = MakeUnique<PendingWalletTxImpl>(m_wallet);
if (!m_wallet.CreateTransaction(recipients, pending->m_tx, pending->m_key, fee, change_pos,
if (!m_wallet.CreateTransaction(*locked_chain, recipients, pending->m_tx, pending->m_key, fee, change_pos,
fail_reason, coin_control, sign)) {
return {};
}
Expand All @@ -253,7 +259,7 @@ class WalletImpl : public Wallet
{
auto locked_chain = m_wallet.chain().lock();
LOCK(m_wallet.cs_wallet);
return m_wallet.AbandonTransaction(txid);
return m_wallet.AbandonTransaction(*locked_chain, txid);
}
bool transactionCanBeBumped(const uint256& txid) override
{
Expand Down Expand Up @@ -295,7 +301,7 @@ class WalletImpl : public Wallet
LOCK(m_wallet.cs_wallet);
auto mi = m_wallet.mapWallet.find(txid);
if (mi != m_wallet.mapWallet.end()) {
return MakeWalletTx(m_wallet, mi->second);
return MakeWalletTx(*locked_chain, m_wallet, mi->second);
}
return {};
}
Expand All @@ -306,7 +312,7 @@ class WalletImpl : public Wallet
std::vector<WalletTx> result;
result.reserve(m_wallet.mapWallet.size());
for (const auto& entry : m_wallet.mapWallet) {
result.emplace_back(MakeWalletTx(m_wallet, entry.second));
result.emplace_back(MakeWalletTx(*locked_chain, m_wallet, entry.second));
}
return result;
}
Expand All @@ -327,7 +333,7 @@ class WalletImpl : public Wallet
return false;
}
num_blocks = ::chainActive.Height();
tx_status = MakeWalletTxStatus(mi->second);
tx_status = MakeWalletTxStatus(*locked_chain, mi->second);
return true;
}
WalletTx getWalletTxDetails(const uint256& txid,
Expand All @@ -343,8 +349,8 @@ class WalletImpl : public Wallet
num_blocks = ::chainActive.Height();
in_mempool = mi->second.InMempool();
order_form = mi->second.vOrderForm;
tx_status = MakeWalletTxStatus(mi->second);
return MakeWalletTx(m_wallet, mi->second);
tx_status = MakeWalletTxStatus(*locked_chain, mi->second);
return MakeWalletTx(*locked_chain, m_wallet, mi->second);
}
return {};
}
Expand Down Expand Up @@ -408,11 +414,11 @@ class WalletImpl : public Wallet
auto locked_chain = m_wallet.chain().lock();
LOCK(m_wallet.cs_wallet);
CoinsList result;
for (const auto& entry : m_wallet.ListCoins()) {
for (const auto& entry : m_wallet.ListCoins(*locked_chain)) {
auto& group = result[entry.first];
for (const auto& coin : entry.second) {
group.emplace_back(
COutPoint(coin.tx->GetHash(), coin.i), MakeWalletTxOut(m_wallet, *coin.tx, coin.i, coin.nDepth));
group.emplace_back(COutPoint(coin.tx->GetHash(), coin.i),
MakeWalletTxOut(*locked_chain, m_wallet, *coin.tx, coin.i, coin.nDepth));
}
}
return result;
Expand All @@ -427,9 +433,9 @@ class WalletImpl : public Wallet
result.emplace_back();
auto it = m_wallet.mapWallet.find(output.hash);
if (it != m_wallet.mapWallet.end()) {
int depth = it->second.GetDepthInMainChain();
int depth = it->second.GetDepthInMainChain(*locked_chain);
if (depth >= 0) {
result.back() = MakeWalletTxOut(m_wallet, it->second, output.n, depth);
result.back() = MakeWalletTxOut(*locked_chain, m_wallet, it->second, output.n, depth);
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions src/threadsafety.h
View file
Expand Up @@ -54,4 +54,15 @@
#define ASSERT_EXCLUSIVE_LOCK(...)
#endif // __GNUC__

// Utility class for indicating to compiler thread analysis that a mutex is
// locked (when it couldn't be determined otherwise).
struct SCOPED_LOCKABLE LockAnnotation
{
template <typename Mutex>
explicit LockAnnotation(Mutex& mutex) EXCLUSIVE_LOCK_FUNCTION(mutex)
{
}
~LockAnnotation() UNLOCK_FUNCTION() {}
};

#endif // BITCOIN_THREADSAFETY_H
10 changes: 5 additions & 5 deletions src/wallet/feebumper.cpp
View file
Expand Up @@ -19,7 +19,7 @@

//! Check whether transaction has descendant in wallet or mempool, or has been
//! mined, or conflicts with a mined transaction. Return a feebumper::Result.
static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(cs_main, wallet->cs_wallet)
static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chain, const CWallet* wallet, const CWalletTx& wtx, std::vector<std::string>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet)
{
if (wallet->HasWalletSpend(wtx.GetHash())) {
errors.push_back("Transaction has descendants in the wallet");
Expand All @@ -35,7 +35,7 @@ static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWallet
}
}

if (wtx.GetDepthInMainChain() != 0) {
if (wtx.GetDepthInMainChain(locked_chain) != 0) {
errors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
return feebumper::Result::WALLET_ERROR;
}
Expand Down Expand Up @@ -71,7 +71,7 @@ bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid)
if (wtx == nullptr) return false;

std::vector<std::string> errors_dummy;
feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy);
feebumper::Result res = PreconditionChecks(*locked_chain, wallet, *wtx, errors_dummy);
return res == feebumper::Result::OK;
}

Expand All @@ -88,7 +88,7 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
}
const CWalletTx& wtx = it->second;

Result result = PreconditionChecks(wallet, wtx, errors);
Result result = PreconditionChecks(*locked_chain, wallet, wtx, errors);
if (result != Result::OK) {
return result;
}
Expand Down Expand Up @@ -235,7 +235,7 @@ Result CommitTransaction(CWallet* wallet, const uint256& txid, CMutableTransacti
CWalletTx& oldWtx = it->second;

// make sure the transaction still has no descendants and hasn't been mined in the meantime
Result result = PreconditionChecks(wallet, oldWtx, errors);
Result result = PreconditionChecks(*locked_chain, wallet, oldWtx, errors);
if (result != Result::OK) {
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/rpcdump.cpp
View file
Expand Up @@ -732,7 +732,7 @@ UniValue dumpwallet(const JSONRPCRequest& request)

std::map<CTxDestination, int64_t> mapKeyBirth;
const std::map<CKeyID, int64_t>& mapKeyPool = pwallet->GetAllReserveKeys();
pwallet->GetKeyBirthTimes(mapKeyBirth);
pwallet->GetKeyBirthTimes(*locked_chain, mapKeyBirth);

std::set<CScriptID> scripts = pwallet->GetCScripts();
// TODO: include scripts in GetKeyBirthTimes() output instead of separate
Expand Down

0 comments on commit 081accb

Please sign in to comment.