diff --git a/doc/release-notes.md b/doc/release-notes.md
index ffeab2d3c00..509ca40fac5 100644
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -1,4 +1,4 @@
-Bitcoin Core version 28.3rc1 is now available from:
+Bitcoin Core version 28.3rc2 is now available from:
@@ -37,7 +37,7 @@ unsupported systems.
Notable changes
===============
-### Policy
+### Mempool & Policy
The minimum block feerate (`-blockmintxfee`) has been changed to 1 satoshi per kvB. It can still be changed using the
configuration option.
@@ -52,6 +52,7 @@ recommended to change both together if you decide to do so.
before attempting to create transactions with lower feerates using the wallet.
- #33106 policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee
+- #33504 mempool: Do not enforce TRUC checks on reorg
### P2P
diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp
index 64861311dbd..2acddd2f78f 100644
--- a/src/test/fuzz/tx_pool.cpp
+++ b/src/test/fuzz/tx_pool.cpp
@@ -292,7 +292,6 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
std::set added;
auto txr = std::make_shared(removed, added);
node.validation_signals->RegisterSharedValidationInterface(txr);
- const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
// Make sure ProcessNewPackage on one transaction works.
// The result is not guaranteed to be the same as what is returned by ATMP.
@@ -307,7 +306,7 @@ FUZZ_TARGET(tx_pool_standard, .init = initialize_tx_pool)
it->second.m_result_type == MempoolAcceptResult::ResultType::INVALID);
}
- const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
+ const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), /*bypass_limits=*/false, /*test_accept=*/false));
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
node.validation_signals->SyncWithValidationInterfaceQueue();
node.validation_signals->UnregisterSharedValidationInterface(txr);
@@ -389,6 +388,9 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
chainstate.SetMempool(&tx_pool);
+ // If we ever bypass limits, do not do TRUC invariants checks
+ bool ever_bypassed_limits{false};
+
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 300)
{
const auto mut_tx = ConsumeTransaction(fuzzed_data_provider, txids);
@@ -407,13 +409,17 @@ FUZZ_TARGET(tx_pool, .init = initialize_tx_pool)
tx_pool.PrioritiseTransaction(txid.ToUint256(), delta);
}
+ const bool bypass_limits{fuzzed_data_provider.ConsumeBool()};
+ ever_bypassed_limits |= bypass_limits;
+
const auto tx = MakeTransactionRef(mut_tx);
- const bool bypass_limits = fuzzed_data_provider.ConsumeBool();
const auto res = WITH_LOCK(::cs_main, return AcceptToMemoryPool(chainstate, tx, GetTime(), bypass_limits, /*test_accept=*/false));
const bool accepted = res.m_result_type == MempoolAcceptResult::ResultType::VALID;
if (accepted) {
txids.push_back(tx->GetHash());
- CheckMempoolTRUCInvariants(tx_pool);
+ if (!ever_bypassed_limits) {
+ CheckMempoolTRUCInvariants(tx_pool);
+ }
}
}
Finish(fuzzed_data_provider, tx_pool, chainstate);
diff --git a/src/validation.cpp b/src/validation.cpp
index 59beb5cbdaf..9daad0b40b2 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1039,26 +1039,28 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// Even though just checking direct mempool parents for inheritance would be sufficient, we
// check using the full ancestor set here because it's more convenient to use what we have
// already calculated.
- if (const auto err{SingleTRUCChecks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, 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.
- Assume(args.m_allow_replacement);
+ if (!args.m_bypass_limits) {
+ if (const auto err{SingleTRUCChecks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, 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.
+ Assume(args.m_allow_replacement);
- // 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());
- // 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.
- ws.m_iters_conflicting.insert(m_pool.GetIter(err->second->GetHash()).value());
- ws.m_sibling_eviction = true;
- // The sibling will be treated as part of the to-be-replaced set in ReplacementChecks.
- // Note that we are not checking whether it opts in to replaceability via BIP125 or TRUC
- // (which is normally done in PreChecks). However, the only way a TRUC transaction can
- // have a non-TRUC and non-BIP125 descendant is due to a reorg.
- } else {
- return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "TRUC-violation", err->first);
+ // 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());
+ // 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.
+ ws.m_iters_conflicting.insert(m_pool.GetIter(err->second->GetHash()).value());
+ ws.m_sibling_eviction = true;
+ // The sibling will be treated as part of the to-be-replaced set in ReplacementChecks.
+ // Note that we are not checking whether it opts in to replaceability via BIP125 or TRUC
+ // (which is normally done in PreChecks). However, the only way a TRUC transaction can
+ // have a non-TRUC and non-BIP125 descendant is due to a reorg.
+ } else {
+ return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "TRUC-violation", err->first);
+ }
}
}
diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py
index 774c9e7c1f2..3b96493cca9 100755
--- a/test/functional/mempool_truc.py
+++ b/test/functional/mempool_truc.py
@@ -191,23 +191,36 @@ class MempoolTRUC(BitcoinTestFramework):
def test_truc_reorg(self):
node = self.nodes[0]
self.log.info("Test that, during a reorg, TRUC rules are not enforced")
- tx_v2_block = self.wallet.send_self_transfer(from_node=node, version=2)
- tx_v3_block = self.wallet.send_self_transfer(from_node=node, version=3)
- tx_v3_block2 = self.wallet.send_self_transfer(from_node=node, version=3)
- self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"]])
+ self.check_mempool([])
+
+ # Testing 2<-3 versions allowed
+ tx_v2_block = self.wallet.create_self_transfer(version=2)
+
+ # Testing 3<-2 versions allowed
+ tx_v3_block = self.wallet.create_self_transfer(version=3)
+
+ # Testing overly-large child size
+ tx_v3_block2 = self.wallet.create_self_transfer(version=3)
+
+ # Also create a linear chain of 3 TRUC transactions that will be directly mined, followed by one v2 in-mempool after block is made
+ tx_chain_1 = self.wallet.create_self_transfer(version=3)
+ tx_chain_2 = self.wallet.create_self_transfer(utxo_to_spend=tx_chain_1["new_utxo"], version=3)
+ tx_chain_3 = self.wallet.create_self_transfer(utxo_to_spend=tx_chain_2["new_utxo"], version=3)
+
+ tx_to_mine = [tx_v3_block["hex"], tx_v2_block["hex"], tx_v3_block2["hex"], tx_chain_1["hex"], tx_chain_2["hex"], tx_chain_3["hex"]]
+ block = self.generateblock(node, output="raw(42)", transactions=tx_to_mine)
- block = self.generate(node, 1)
self.check_mempool([])
tx_v2_from_v3 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block["new_utxo"], version=2)
tx_v3_from_v2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v2_block["new_utxo"], version=3)
tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_weight=5000, version=3)
assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000)
- self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]])
- node.invalidateblock(block[0])
- self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]])
- # This is needed because generate() will create the exact same block again.
- node.reconsiderblock(block[0])
+ tx_chain_4 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_chain_3["new_utxo"], version=2)
+ self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"], tx_chain_4["txid"]])
+ # Reorg should have all block transactions re-accepted, ignoring TRUC enforcement
+ node.invalidateblock(block["hash"])
+ self.check_mempool([tx_v3_block["txid"], tx_v2_block["txid"], tx_v3_block2["txid"], tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"], tx_chain_1["txid"], tx_chain_2["txid"], tx_chain_3["txid"], tx_chain_4["txid"]])
@cleanup(extra_args=["-limitdescendantsize=10", "-datacarriersize=40000"])
def test_nondefault_package_limits(self):