MOVEONLY: BIP125 Rule 2 to policy/rbf

This commit is contained in:
glozow 2021-08-03 13:13:43 +01:00
parent f8ad2a57c6
commit 7b60c02b7d
3 changed files with 49 additions and 32 deletions

View File

@ -75,3 +75,40 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx,
return std::nullopt;
}
std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
const CTxMemPool& m_pool,
const CTxMemPool::setEntries& setIterConflicting)
{
AssertLockHeld(m_pool.cs);
std::set<uint256> setConflictsParents;
for (const auto& mi : setIterConflicting) {
for (const CTxIn &txin : mi->GetTx().vin)
{
setConflictsParents.insert(txin.prevout.hash);
}
}
for (unsigned int j = 0; j < tx.vin.size(); j++)
{
// We don't want to accept replacements that require low
// feerate junk to be mined first. Ideally we'd keep track of
// the ancestor feerates and make the decision based on that,
// but for now requiring all new inputs to be confirmed works.
//
// Note that if you relax this to make RBF a little more useful,
// this may break the CalculateMempoolAncestors RBF relaxation,
// above. See the comment above the first CalculateMempoolAncestors
// call for more info.
if (!setConflictsParents.count(tx.vin[j].prevout.hash))
{
// Rather than check the UTXO set - potentially expensive -
// it's cheaper to just check if the new input refers to a
// tx that's in the mempool.
if (m_pool.exists(tx.vin[j].prevout.hash)) {
return strprintf("replacement %s adds unconfirmed input, idx %d",
tx.GetHash().ToString(), j);
}
}
}
return std::nullopt;
}

View File

@ -49,4 +49,11 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, CTxMem
const CTxMemPool::setEntries& setIterConflicting,
CTxMemPool::setEntries& allConflicting)
EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
/** BIP125 Rule #2: "The replacement transaction may only include an unconfirmed input if that input
* was included in one of the original transactions."
* @returns error message if Rule #2 is broken, otherwise std::nullopt. */
std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTxMemPool& m_pool,
const CTxMemPool::setEntries& setIterConflicting)
EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
#endif // BITCOIN_POLICY_RBF_H

View File

@ -821,6 +821,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
"too many potential replacements", *err_string);
}
// Enforce Rule #2.
if (const auto err_string{HasNoNewUnconfirmed(tx, m_pool, setIterConflicting)}) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
"replacement-adds-unconfirmed", *err_string);
}
// Check if it's economically rational to mine this transaction rather
// than the ones it replaces.
@ -829,38 +834,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
nConflictingSize += it->GetTxSize();
}
std::set<uint256> setConflictsParents;
for (const auto& mi : setIterConflicting) {
for (const CTxIn &txin : mi->GetTx().vin)
{
setConflictsParents.insert(txin.prevout.hash);
}
}
for (unsigned int j = 0; j < tx.vin.size(); j++)
{
// We don't want to accept replacements that require low
// feerate junk to be mined first. Ideally we'd keep track of
// the ancestor feerates and make the decision based on that,
// but for now requiring all new inputs to be confirmed works.
//
// Note that if you relax this to make RBF a little more useful,
// this may break the CalculateMempoolAncestors RBF relaxation,
// above. See the comment above the first CalculateMempoolAncestors
// call for more info.
if (!setConflictsParents.count(tx.vin[j].prevout.hash))
{
// Rather than check the UTXO set - potentially expensive -
// it's cheaper to just check if the new input refers to a
// tx that's in the mempool.
if (m_pool.exists(tx.vin[j].prevout.hash)) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "replacement-adds-unconfirmed",
strprintf("replacement %s adds unconfirmed input, idx %d",
hash.ToString(), j));
}
}
}
// The replacement must pay greater fees than the transactions it
// replaces - if we did the bandwidth used by those conflicting
// transactions would not be paid for.