Make GetEntriesForConflicts return std::optional

Avoids reusing err_string.
This commit is contained in:
glozow 2021-09-02 15:27:37 +01:00
parent b3a2b8c29f
commit f8ad2a57c6
3 changed files with 12 additions and 15 deletions

View File

@ -47,11 +47,10 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx)
return SignalsOptInRBF(tx) ? RBFTransactionState::REPLACEABLE_BIP125 : RBFTransactionState::UNKNOWN; return SignalsOptInRBF(tx) ? RBFTransactionState::REPLACEABLE_BIP125 : RBFTransactionState::UNKNOWN;
} }
bool GetEntriesForConflicts(const CTransaction& tx, std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx,
CTxMemPool& m_pool, CTxMemPool& m_pool,
const CTxMemPool::setEntries& setIterConflicting, const CTxMemPool::setEntries& setIterConflicting,
CTxMemPool::setEntries& allConflicting, CTxMemPool::setEntries& allConflicting)
std::string& err_string)
{ {
AssertLockHeld(m_pool.cs); AssertLockHeld(m_pool.cs);
const uint256 hash = tx.GetHash(); const uint256 hash = tx.GetHash();
@ -62,11 +61,10 @@ bool GetEntriesForConflicts(const CTransaction& tx,
// but we just want to be conservative to avoid doing too much // but we just want to be conservative to avoid doing too much
// work. // work.
if (nConflictingCount > MAX_BIP125_REPLACEMENT_CANDIDATES) { if (nConflictingCount > MAX_BIP125_REPLACEMENT_CANDIDATES) {
err_string = strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n", return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
hash.ToString(), hash.ToString(),
nConflictingCount, nConflictingCount,
MAX_BIP125_REPLACEMENT_CANDIDATES); MAX_BIP125_REPLACEMENT_CANDIDATES);
return false;
} }
} }
// If not too many to replace, then calculate the set of // If not too many to replace, then calculate the set of
@ -74,6 +72,6 @@ bool GetEntriesForConflicts(const CTransaction& tx,
for (CTxMemPool::txiter it : setIterConflicting) { for (CTxMemPool::txiter it : setIterConflicting) {
m_pool.CalculateDescendants(it, allConflicting); m_pool.CalculateDescendants(it, allConflicting);
} }
return true; return std::nullopt;
} }

View File

@ -40,14 +40,13 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx);
* mempool must not exceed a total of 100 transactions." Quit as early as possible. There cannot be * mempool must not exceed a total of 100 transactions." Quit as early as possible. There cannot be
* more than MAX_BIP125_REPLACEMENT_CANDIDATES potential entries. * more than MAX_BIP125_REPLACEMENT_CANDIDATES potential entries.
* @param[in] setIterConflicting The set of iterators to mempool entries. * @param[in] setIterConflicting The set of iterators to mempool entries.
* @param[out] err_string Used to return errors, if any.
* @param[out] allConflicting Populated with all the mempool entries that would be replaced, * @param[out] allConflicting Populated with all the mempool entries that would be replaced,
* which includes descendants of setIterConflicting. Not cleared at * which includes descendants of setIterConflicting. Not cleared at
* the start; any existing mempool entries will remain in the set. * the start; any existing mempool entries will remain in the set.
* @returns false if Rule 5 is broken. * @returns an error message if Rule #5 is broken, otherwise a std::nullopt.
*/ */
bool GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& m_pool, std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& m_pool,
const CTxMemPool::setEntries& setIterConflicting, const CTxMemPool::setEntries& setIterConflicting,
CTxMemPool::setEntries& allConflicting, CTxMemPool::setEntries& allConflicting)
std::string& err_string) EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs); EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
#endif // BITCOIN_POLICY_RBF_H #endif // BITCOIN_POLICY_RBF_H

View File

@ -789,7 +789,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
fReplacementTransaction = setConflicts.size(); fReplacementTransaction = setConflicts.size();
if (fReplacementTransaction) if (fReplacementTransaction)
{ {
std::string err_string;
CFeeRate newFeeRate(nModifiedFees, nSize); CFeeRate newFeeRate(nModifiedFees, nSize);
for (const auto& mi : setIterConflicting) { for (const auto& mi : setIterConflicting) {
// Don't allow the replacement to reduce the feerate of the // Don't allow the replacement to reduce the feerate of the
@ -818,8 +817,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
} }
// Calculate all conflicting entries and enforce Rule #5. // Calculate all conflicting entries and enforce Rule #5.
if (!GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting, err_string)) { if (const auto err_string{GetEntriesForConflicts(tx, m_pool, setIterConflicting, allConflicting)}) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too many potential replacements", err_string); return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
"too many potential replacements", *err_string);
} }
// Check if it's economically rational to mine this transaction rather // Check if it's economically rational to mine this transaction rather