Merge 9749 via unique_spk_mempool-28+knots

This commit is contained in:
Luke Dashjr 2025-03-05 03:27:08 +00:00
commit 84eff5944d
10 changed files with 163 additions and 21 deletions

View File

@ -690,6 +690,7 @@ void SetupServerArgs(ArgsManager& argsman)
OptionsCategory::NODE_RELAY);
argsman.AddArg("-minrelaytxfee=<amt>", strprintf("Fees (in %s/kvB) smaller than this are considered zero fee for relaying, mining and transaction creation (default: %s)",
CURRENCY_UNIT, FormatMoney(DEFAULT_MIN_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-spkreuse=<policy>", strprintf("Either \"allow\" to relay/mine transactions reusing addresses or other pubkey scripts, or \"conflict\" to treat them as exclusive prior to being mined (default: %s)", DEFAULT_SPKREUSE), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-whitelistforcerelay", strprintf("Add 'forcerelay' permission to whitelisted peers with default permissions. This will relay transactions even if the transactions were already in the mempool. (default: %d)", DEFAULT_WHITELISTFORCERELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
argsman.AddArg("-whitelistrelay", strprintf("Add 'relay' permission to whitelisted peers with default permissions. This will accept relayed transactions even when not relaying transactions (default: %d)", DEFAULT_WHITELISTRELAY), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY);
@ -1087,6 +1088,16 @@ bool AppInitParameterInteraction(const ArgsManager& args)
if (!g_wallet_init_interface.ParameterInteraction()) return false;
{
std::string strSpkReuse = gArgs.GetArg("-spkreuse", DEFAULT_SPKREUSE);
// Uses string values so future versions can implement other modes
if (strSpkReuse == "allow" || gArgs.GetBoolArg("-spkreuse", false)) {
SpkReuseMode = SRM_ALLOW;
} else {
SpkReuseMode = SRM_REJECT;
}
}
// Option to startup with mocktime set (used for regression testing):
SetMockTime(args.GetIntArg("-mocktime", 0)); // SetMockTime(0) is a no-op

View File

@ -37,6 +37,15 @@ struct LockPoints {
CBlockIndex* maxInputBlock{nullptr};
};
enum MemPool_SPK_State {
MSS_UNSEEN = 0,
MSS_SPENT = 1, // .second
MSS_CREATED = 2, // .first
MSS_BOTH = 3,
};
typedef std::map<uint160, enum MemPool_SPK_State> SPKStates_t;
struct CompareIteratorByHash {
// SFINAE for T where T is either a pointer type (e.g., a txiter) or a reference_wrapper<T>
// (e.g. a wrapped CTxMemPoolEntry&)
@ -215,6 +224,8 @@ public:
mutable size_t idx_randomized; //!< Index in mempool's txns_randomized
mutable Epoch::Marker m_epoch_marker; //!< epoch when last touched, useful for graph algorithms
SPKStates_t mapSPK;
};
using CTxMemPoolEntryRef = CTxMemPoolEntry::CTxMemPoolEntryRef;

View File

@ -67,6 +67,7 @@ static constexpr unsigned int MAX_STANDARD_SCRIPTSIG_SIZE{1650};
* only increase the dust limit after prior releases were already not creating
* outputs below the new threshold */
static constexpr unsigned int DUST_RELAY_TX_FEE{3000};
static const std::string DEFAULT_SPKREUSE{"allow"};
/** Default for -minrelaytxfee, minimum relay fee for transactions */
static constexpr unsigned int DEFAULT_MIN_RELAY_TX_FEE{1000};
/** Default for -limitancestorcount, max number of in-mempool ancestors */

View File

@ -119,12 +119,17 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
}
std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
const std::set<Txid>& direct_conflicts,
const uint256& txid)
const std::map<Txid, bool>& direct_conflicts,
const uint256& txid, bool* const out_violates_policy)
{
for (CTxMemPool::txiter ancestorIt : ancestors) {
const Txid& hashAncestor = ancestorIt->GetTx().GetHash();
if (direct_conflicts.count(hashAncestor)) {
const auto& conflictit = direct_conflicts.find(hashAncestor);
if (conflictit != direct_conflicts.end()) {
if (!conflictit->second /* mere SPK conflict, NOT invalid */) {
if (out_violates_policy) *out_violates_policy = true;
continue;
}
return strprintf("%s spends conflicting transaction %s",
txid.ToString(),
hashAncestor.ToString());

View File

@ -88,11 +88,12 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTx
* @param[in] direct_conflicts Set of txids corresponding to the mempool conflicts
* (candidates to be replaced).
* @param[in] txid Transaction ID, included in the error message if violation occurs.
* @returns error message if the sets intersect, std::nullopt if they are disjoint.
* @param[out] out_violates_policy Assigned to true if there are any policy-only conflicts.
* @returns error message if the sets intersect (consensus-only conflicts), std::nullopt if they are disjoint or only intersect on policy matters.
*/
std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
const std::set<Txid>& direct_conflicts,
const uint256& txid);
const std::map<Txid, bool>& direct_conflicts,
const uint256& txid, bool* out_violates_policy);
/** Check that the feerate of the replacement transaction(s) is higher than the feerate of each
* of the transactions in iters_conflicting.

View File

@ -216,14 +216,22 @@ BOOST_FIXTURE_TEST_CASE(rbf_helper_functions, TestChain100Setup)
BOOST_CHECK(PaysMoreThanConflicts(set_34_cpfp, CFeeRate(entry4_high->GetModifiedFee(), entry4_high->GetTxSize()), unused_txid).has_value());
// Tests for EntriesAndTxidsDisjoint
BOOST_CHECK(EntriesAndTxidsDisjoint(empty_set, {tx1->GetHash()}, unused_txid) == std::nullopt);
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx3->GetHash()}, unused_txid) == std::nullopt);
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2_normal}, {tx2->GetHash()}, unused_txid).has_value());
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx1->GetHash()}, unused_txid).has_value());
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {tx2->GetHash()}, unused_txid).has_value());
bool violates_policy{false};
BOOST_CHECK(EntriesAndTxidsDisjoint(empty_set, {{tx1->GetHash(), true}}, unused_txid, &violates_policy) == std::nullopt);
BOOST_CHECK(!violates_policy);
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {{tx3->GetHash(), true}}, unused_txid, &violates_policy) == std::nullopt);
BOOST_CHECK(!violates_policy);
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2_normal}, {{tx2->GetHash(), true}}, unused_txid, &violates_policy).has_value());
BOOST_CHECK(!violates_policy);
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {{tx1->GetHash(), true}}, unused_txid, &violates_policy).has_value());
BOOST_CHECK(!violates_policy);
BOOST_CHECK(EntriesAndTxidsDisjoint(set_12_normal, {{tx2->GetHash(), true}}, unused_txid, &violates_policy).has_value());
BOOST_CHECK(!violates_policy);
// EntriesAndTxidsDisjoint does not calculate descendants of iters_conflicting; it uses whatever
// the caller passed in. As such, no error is returned even though entry2_normal is a descendant of tx1.
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2_normal}, {tx1->GetHash()}, unused_txid) == std::nullopt);
BOOST_CHECK(EntriesAndTxidsDisjoint({entry2_normal}, {{tx1->GetHash(), true}}, unused_txid, &violates_policy) == std::nullopt);
BOOST_CHECK(!violates_policy);
// TODO: Add tests for policy-only conflicts
// Tests for PaysForRBF
const CFeeRate incremental_relay_feerate{DEFAULT_INCREMENTAL_RELAY_FEE};

View File

@ -11,12 +11,14 @@
#include <consensus/consensus.h>
#include <consensus/tx_verify.h>
#include <consensus/validation.h>
#include <crypto/ripemd160.h>
#include <logging.h>
#include <policy/coin_age_priority.h>
#include <policy/policy.h>
#include <policy/settings.h>
#include <random.h>
#include <tinyformat.h>
#include <script/script.h>
#include <util/check.h>
#include <util/feefrac.h>
#include <util/moneystr.h>
@ -52,6 +54,13 @@ bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
return true;
}
uint160 ScriptHashkey(const CScript& script)
{
uint160 hash;
CRIPEMD160().Write(script.data(), script.size()).Finalize(hash.begin());
return hash;
}
void CTxMemPool::UpdateForDescendants(txiter updateIt, cacheMap& cachedDescendants,
const std::set<uint256>& setExclude, std::set<uint256>& descendants_to_remove)
{
@ -480,6 +489,17 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
txns_randomized.emplace_back(newit->GetSharedTx());
newit->idx_randomized = txns_randomized.size() - 1;
for (auto& vSPK : entry.mapSPK) {
const uint160& SPKKey = vSPK.first;
const MemPool_SPK_State& claims = vSPK.second;
if (claims & MSS_CREATED) {
mapUsedSPK[SPKKey].first = &tx;
}
if (claims & MSS_SPENT) {
mapUsedSPK[SPKKey].second = &tx;
}
}
TRACE3(mempool, added,
entry.GetTx().GetHash().data(),
entry.GetTxSize(),
@ -508,6 +528,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
std::chrono::duration_cast<std::chrono::duration<std::uint64_t>>(it->GetTime()).count()
);
const CTransaction& tx = it->GetTx();
for (const CTxIn& txin : it->GetTx().vin)
mapNextTx.erase(txin.prevout);
@ -524,6 +545,19 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
} else
txns_randomized.clear();
for (auto& vSPK : it->mapSPK) {
const uint160& SPKKey = vSPK.first;
if (mapUsedSPK[SPKKey].first == &tx) {
mapUsedSPK[SPKKey].first = NULL;
}
if (mapUsedSPK[SPKKey].second == &tx) {
mapUsedSPK[SPKKey].second = NULL;
}
if (!(mapUsedSPK[SPKKey].first || mapUsedSPK[SPKKey].second)) {
mapUsedSPK.erase(SPKKey);
}
}
totalTxSize -= it->GetTxSize();
m_total_fee -= it->GetFee();
cachedInnerUsage -= it->DynamicMemoryUsage();

View File

@ -17,6 +17,7 @@
#include <policy/feerate.h>
#include <policy/packages.h>
#include <primitives/transaction.h>
#include <script/script.h>
#include <sync.h>
#include <util/epochguard.h>
#include <util/hasher.h>
@ -205,6 +206,8 @@ public:
}
};
uint160 ScriptHashkey(const CScript& script);
// Multi_index tag names
struct descendant_score {};
struct entry_time {};
@ -403,6 +406,9 @@ public:
using Limits = kernel::MemPoolLimits;
uint64_t CalculateDescendantMaximum(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs);
std::map<uint160, std::pair<const CTransaction *, const CTransaction *>> mapUsedSPK;
private:
typedef std::map<txiter, setEntries, CompareIteratorByHash> cacheMap;

View File

@ -110,6 +110,7 @@ const std::vector<std::string> CHECKLEVEL_DOC {
GlobalMutex g_best_block_mutex;
std::condition_variable g_best_block_cv;
uint256 g_best_block;
SpkReuseModes SpkReuseMode;
const CBlockIndex* Chainstate::FindForkInGlobalIndex(const CBlockLocator& locator) const
{
@ -660,7 +661,8 @@ private:
explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {}
/** Txids of mempool transactions that this transaction directly conflicts with or may
* replace via sibling eviction. */
std::set<Txid> m_conflicts;
/** .second=true is a consensus conflict, and .second=false is a policy conflict. */
std::map<Txid, bool> m_conflicts_incl_policy;
/** Iterators to mempool entries that this transaction directly conflicts with or may
* replace via sibling eviction. */
CTxMemPool::setEntries m_iters_conflicting;
@ -870,6 +872,12 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-same-nonwitness-data-in-mempool");
}
auto spk_reuse_mode = SpkReuseMode;
if (ignore_rejects.count("txn-spk-reused")) {
spk_reuse_mode = SRM_ALLOW;
}
SPKStates_t mapSPK;
// Check for conflicts with in-memory transactions
for (const CTxIn &txin : tx.vin)
{
@ -879,7 +887,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// Transaction conflicts with a mempool tx, but we're not allowing replacements.
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "bip125-replacement-disallowed");
}
if (!ws.m_conflicts.count(ptxConflicting->GetHash()))
if (!ws.m_conflicts_incl_policy.count(ptxConflicting->GetHash()))
{
// Transactions that don't explicitly signal replaceability are
// *not* replaceable with the current logic, even if one of their
@ -905,11 +913,30 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
}
ws.m_conflicts.insert(ptxConflicting->GetHash());
ws.m_conflicts_incl_policy.emplace(ptxConflicting->GetHash(), true);
}
}
}
if (spk_reuse_mode != SRM_ALLOW) {
for (const CTxOut& txout : tx.vout) {
uint160 hashSPK = ScriptHashkey(txout.scriptPubKey);
const auto& SPKUsedIn = m_pool.mapUsedSPK.find(hashSPK);
if (SPKUsedIn != m_pool.mapUsedSPK.end()) {
if (SPKUsedIn->second.first) {
ws.m_conflicts_incl_policy.emplace(SPKUsedIn->second.first->GetHash(), false);
}
if (SPKUsedIn->second.second) {
ws.m_conflicts_incl_policy.emplace(SPKUsedIn->second.second->GetHash(), false);
}
}
if (mapSPK.find(hashSPK) != mapSPK.end()) {
MaybeReject(TxValidationResult::TX_MEMPOOL_POLICY, "txn-spk-reused-twinoutputs");
}
mapSPK[hashSPK] = MemPool_SPK_State(mapSPK[hashSPK] | MSS_CREATED);
}
}
m_view.SetBackend(m_viewmempool);
const CCoinsViewCache& coins_cache = m_active_chainstate.CoinsTip();
@ -962,6 +989,27 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
return false; // state filled in by CheckTxInputs
}
if (spk_reuse_mode != SRM_ALLOW) {
for (const CTxIn& txin : tx.vin) {
const Coin &coin = m_view.AccessCoin(txin.prevout);
uint160 hashSPK = ScriptHashkey(coin.out.scriptPubKey);
SPKStates_t::iterator mssit = mapSPK.find(hashSPK);
if (mssit != mapSPK.end()) {
if (mssit->second & MSS_CREATED) {
MaybeReject(TxValidationResult::TX_MEMPOOL_POLICY, "txn-spk-reused-change");
}
}
const auto& SPKit = m_pool.mapUsedSPK.find(hashSPK);
if (SPKit != m_pool.mapUsedSPK.end()) {
if (SPKit->second.second /* Spent */) {
ws.m_conflicts_incl_policy.emplace(SPKit->second.second->GetHash(), false);
}
}
mapSPK[hashSPK] = MemPool_SPK_State(mapSPK[hashSPK] | MSS_SPENT);
}
}
if (m_pool.m_opts.require_standard && !AreInputsStandard(tx, m_view, "bad-txns-input-", reason, ignore_rejects)) {
return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, reason);
}
@ -1001,6 +1049,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
inChainInputValue,
fSpendsCoinbase, nSigOpsCost, lock_points.value()));
ws.m_vsize = entry->GetTxSize();
entry->mapSPK = mapSPK;
// To avoid rejecting low-sigop bare-multisig transactions, the sigops
// are counted a second time more accurately.
@ -1026,14 +1075,18 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// feerate later.
if (!args.m_package_feerates && !CheckFeeRate(ws.m_vsize, ws.m_modified_fees, state, args.m_ignore_rejects)) return false;
ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts);
std::set<Txid> conflicts_as_a_set;
std::transform(ws.m_conflicts_incl_policy.begin(), ws.m_conflicts_incl_policy.end(),
std::inserter(conflicts_as_a_set, conflicts_as_a_set.end()),
[](const std::pair<Txid, bool>& pair){ return pair.first; });
ws.m_iters_conflicting = m_pool.GetIterSet(conflicts_as_a_set);
// Note that these modifications are only applicable to single transaction scenarios;
// carve-outs are disabled for multi-transaction evaluations.
CTxMemPool::Limits maybe_rbf_limits = m_pool.m_opts.limits;
// Calculate in-mempool ancestors, up to a limit.
if (ws.m_conflicts.size() == 1 && args.m_allow_carveouts) {
if (ws.m_conflicts_incl_policy.size() == 1 && args.m_allow_carveouts) {
// In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we
// would meet the chain limits after the conflicts have been removed. However, there isn't a practical
// way to do this short of calculating the ancestor and descendant sets with an overlay cache of
@ -1117,7 +1170,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// check using the full ancestor set here because it's more convenient to use what we have
// already calculated.
if (m_pool.m_opts.truc_policy == TRUCPolicy::Enforce) {
if (const auto err{SingleTRUCChecks(ws.m_ptx, "truc-", reason, ignore_rejects, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
if (const auto err{SingleTRUCChecks(ws.m_ptx, "truc-", reason, ignore_rejects, ws.m_ancestors, conflicts_as_a_set, ws.m_vsize)}) {
// Single transaction contexts only.
if (args.m_allow_sibling_eviction && err->second != nullptr) {
// We should only be considering where replacement is considered valid as well.
@ -1125,7 +1178,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// Potential sibling eviction. Add the sibling to our list of mempool conflicts to be
// included in RBF checks.
ws.m_conflicts.insert(err->second->GetHash());
ws.m_conflicts_incl_policy.emplace(err->second->GetHash(), false);
conflicts_as_a_set.insert(err->second->GetHash());
// Adding the sibling to m_iters_conflicting here means that it doesn't count towards
// RBF Carve Out above. This is correct, since removing to-be-replaced transactions from
// the descendant count is done separately in SingleTRUCChecks for TRUC transactions.
@ -1144,14 +1198,18 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// that we have the set of all ancestors we can detect this
// pathological case by making sure ws.m_conflicts and ws.m_ancestors don't
// intersect.
if (const auto err_string{EntriesAndTxidsDisjoint(ws.m_ancestors, ws.m_conflicts, hash)}) {
bool has_policy_conflict{false};
if (const auto err_string{EntriesAndTxidsDisjoint(ws.m_ancestors, ws.m_conflicts_incl_policy, hash, &has_policy_conflict)}) {
// We classify this as a consensus error because a transaction depending on something it
// conflicts with would be inconsistent.
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string);
}
if (has_policy_conflict) {
MaybeReject(TxValidationResult::TX_MEMPOOL_POLICY, "txn-spk-reused-chained");
}
// We want to detect conflicts in any tx in a package to trigger package RBF logic
m_subpackage.m_rbf |= !ws.m_conflicts.empty();
m_subpackage.m_rbf |= !ws.m_conflicts_incl_policy.empty();
return true;
}

View File

@ -92,6 +92,13 @@ extern std::condition_variable g_best_block_cv;
/** Used to notify getblocktemplate RPC of new tips. */
extern uint256 g_best_block;
enum SpkReuseModes {
SRM_ALLOW,
SRM_REJECT,
};
extern SpkReuseModes SpkReuseMode;
/** Documentation for argument 'checklevel'. */
extern const std::vector<std::string> CHECKLEVEL_DOC;