Remove duplicate checks in SubmitMemoryPoolAndRelay

IsCoinBase check is already performed early by
AcceptToMemoryPoolWorker
GetDepthInMainChain check is already perfomed by
BroadcastTransaction

To avoid deadlock we MUST keep lock order in
ResendWalletTransactions and CommitTransaction,
even if we lock cs_main again further.
in BroadcastTransaction. Lock order will need
to be clean at once in a future refactoring
This commit is contained in:
Antoine Riard 2019-04-11 16:01:58 +00:00
parent 611291c198
commit 8753f5652b
2 changed files with 5 additions and 9 deletions

View file

@ -2150,20 +2150,16 @@ void CWallet::ReacceptWalletTransactions(interfaces::Chain::Lock& locked_chain)
for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) { for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) {
CWalletTx& wtx = *(item.second); CWalletTx& wtx = *(item.second);
std::string unused_err_string; std::string unused_err_string;
wtx.SubmitMemoryPoolAndRelay(unused_err_string, false, locked_chain); wtx.SubmitMemoryPoolAndRelay(unused_err_string, false);
} }
} }
bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain) bool CWalletTx::SubmitMemoryPoolAndRelay(std::string& err_string, bool relay)
{ {
// Can't relay if wallet is not broadcasting // Can't relay if wallet is not broadcasting
if (!pwallet->GetBroadcastTransactions()) return false; if (!pwallet->GetBroadcastTransactions()) return false;
// Don't relay coinbase transactions outside blocks
if (IsCoinBase()) return false;
// Don't relay abandoned transactions // Don't relay abandoned transactions
if (isAbandoned()) return false; if (isAbandoned()) return false;
// Don't relay conflicted or already confirmed transactions
if (GetDepthInMainChain(locked_chain) != 0) return false;
// Submit transaction to mempool for relay // Submit transaction to mempool for relay
pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString()); pwallet->WalletLogPrintf("Submitting wtx %s to mempool for relay\n", GetHash().ToString());
@ -2382,7 +2378,7 @@ void CWallet::ResendWalletTransactions()
// last block was found // last block was found
if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue;
std::string unused_err_string; std::string unused_err_string;
if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true, *locked_chain)) ++submitted_tx_count; if (wtx.SubmitMemoryPoolAndRelay(unused_err_string, true)) ++submitted_tx_count;
} }
} // locked_chain and cs_wallet } // locked_chain and cs_wallet
@ -3327,7 +3323,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
if (fBroadcastTransactions) if (fBroadcastTransactions)
{ {
std::string err_string; std::string err_string;
if (!wtx.SubmitMemoryPoolAndRelay(err_string, true, *locked_chain)) { if (!wtx.SubmitMemoryPoolAndRelay(err_string, true)) {
WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string); WalletLogPrintf("CommitTransaction(): Transaction cannot be broadcast immediately, %s\n", err_string);
// TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure. // TODO: if we expect the failure to be long term or permanent, instead delete wtx from the wallet and return failure.
} }

View file

@ -580,7 +580,7 @@ public:
int64_t GetTxTime() const; int64_t GetTxTime() const;
// Pass this transaction to node for mempool insertion and relay to peers if flag set to true // Pass this transaction to node for mempool insertion and relay to peers if flag set to true
bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay, interfaces::Chain::Lock& locked_chain); bool SubmitMemoryPoolAndRelay(std::string& err_string, bool relay);
// TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct // TODO: Remove "NO_THREAD_SAFETY_ANALYSIS" and replace it with the correct
// annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation // annotation "EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet)". The annotation