highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-01-19 14:53:43 +01:00
Code Issues Packages Projects Releases Wiki Activity

policy: Remove CPFP carveout rule

Browse Source 
The addition of a cluster size limit makes the CPFP carveout rule useless,
because carveout cannot be used to bypass the cluster size limit. Remove this
policy rule and update tests to no longer rely on the behavior.
This commit is contained in:
Suhas Daftuar
2023-10-03 10:54:30 -04:00
parent c3f1afc934
commit ff8f115dec
4 changed files with 7 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
src/validation.cpp
View File

@@ -1014,40 +1014,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
if (auto ancestors{m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle, maybe_rbf_limits)}) {
ws.m_ancestors = std::move(*ancestors);
} else {
// If CalculateMemPoolAncestors fails second time, we want the original error string.
const auto error_message{util::ErrorString(ancestors).original};
// Carve-out is not allowed in this context; fail
if (!args.m_allow_carveouts) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
}
// Contracting/payment channels CPFP carve-out:
// If the new transaction is relatively small (up to 40k weight)
// and has at most one ancestor (ie ancestor limit of 2, including
// the new transaction), allow it if its parent has exactly the
// descendant limit descendants. The transaction also cannot be TRUC,
// as its topology restrictions do not allow a second child.
//
// This allows protocols which rely on distrusting counterparties
// being able to broadcast descendants of an unconfirmed transaction
// to be secure by simply only having two immediately-spendable
// outputs - one for each counterparty. For more info on the uses for
// this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
CTxMemPool::Limits cpfp_carve_out_limits{
.ancestor_count = 2,
.ancestor_size_vbytes = maybe_rbf_limits.ancestor_size_vbytes,
.descendant_count = maybe_rbf_limits.descendant_count + 1,
.descendant_size_vbytes = maybe_rbf_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT,
};
if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || ws.m_ptx->version == TRUC_VERSION) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
}
if (auto ancestors_retry{m_subpackage.m_changeset->CalculateMemPoolAncestors(ws.m_tx_handle, cpfp_carve_out_limits)}) {
ws.m_ancestors = std::move(*ancestors_retry);
} else {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
}
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", util::ErrorString(ancestors).original);
}
// Even though just checking direct mempool parents for inheritance would be sufficient, we

80
test/functional/mempool_package_onemore.py
View File

@@ -1,80 +0,0 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2022 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test descendant package tracking carve-out allowing one final transaction in
an otherwise-full package as long as it has only one parent and is <= 10k in
size.
"""
from test_framework.messages import (
DEFAULT_ANCESTOR_LIMIT,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
)
from test_framework.wallet import MiniWallet
class MempoolPackagesTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
def chain_tx(self, utxos_to_spend, *, num_outputs=1):
return self.wallet.send_self_transfer_multi(
from_node=self.nodes[0],
utxos_to_spend=utxos_to_spend,
num_outputs=num_outputs)['new_utxos']
def run_test(self):
self.wallet = MiniWallet(self.nodes[0])
# DEFAULT_ANCESTOR_LIMIT transactions off a confirmed tx should be fine
chain = []
utxo = self.wallet.get_utxo()
for _ in range(4):
utxo, utxo2 = self.chain_tx([utxo], num_outputs=2)
chain.append(utxo2)
for _ in range(DEFAULT_ANCESTOR_LIMIT - 4):
utxo, = self.chain_tx([utxo])
chain.append(utxo)
second_chain, = self.chain_tx([self.wallet.get_utxo(confirmed_only=True)])
# Check mempool has DEFAULT_ANCESTOR_LIMIT + 1 transactions in it
assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 1)
# Adding one more transaction on to the chain should fail.
assert_raises_rpc_error(-26, "too-long-mempool-chain, too many unconfirmed ancestors [limit: 25]", self.chain_tx, [utxo])
# ... or if it chains on from some point in the middle of the chain.
assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[2]])
assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[1]])
# ...even if it chains on to two parent transactions with one in the chain.
assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[0], second_chain])
# ...especially if its > 40k weight
assert_raises_rpc_error(-26, "too-long-mempool-chain, too many descendants", self.chain_tx, [chain[0]], num_outputs=350)
# ...even if it's submitted with other transactions
replaceable_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=[chain[0]])
txns = [replaceable_tx["tx"], self.wallet.create_self_transfer_multi(utxos_to_spend=replaceable_tx["new_utxos"])["tx"]]
txns_hex = [tx.serialize().hex() for tx in txns]
assert_equal(self.nodes[0].testmempoolaccept(txns_hex)[0]["reject-reason"], "too-long-mempool-chain")
pkg_result = self.nodes[0].submitpackage(txns_hex)
assert "too-long-mempool-chain" in pkg_result["tx-results"][txns[0].wtxid_hex]["error"]
assert_equal(pkg_result["tx-results"][txns[1].wtxid_hex]["error"], "bad-txns-inputs-missingorspent")
# But not if it chains directly off the first transaction
self.nodes[0].sendrawtransaction(replaceable_tx["hex"])
# and the second chain should work just fine
self.chain_tx([second_chain])
# Ensure an individual transaction with single direct conflict can RBF the chain which used our carve-out rule
replacement_tx = replaceable_tx["tx"]
replacement_tx.vout[0].nValue -= 1000000
self.nodes[0].sendrawtransaction(replacement_tx.serialize().hex())
# Finally, check that we added two transactions
assert_equal(len(self.nodes[0].getrawmempool()), DEFAULT_ANCESTOR_LIMIT + 3)
if __name__ == '__main__':
MempoolPackagesTest(__file__).main()

9
test/functional/mempool_packages.py
View File

@@ -20,7 +20,7 @@ from test_framework.wallet import MiniWallet
# custom limits for node1
CUSTOM_ANCESTOR_LIMIT = 5
CUSTOM_DESCENDANT_LIMIT = 10
CUSTOM_DESCENDANT_LIMIT = 11
assert CUSTOM_DESCENDANT_LIMIT >= CUSTOM_ANCESTOR_LIMIT
@@ -240,12 +240,15 @@ class MempoolPackagesTest(BitcoinTestFramework):
# - parent tx for descendant test
# - txs chained off parent tx (-> custom descendant limit)
self.wait_until(lambda: len(self.nodes[1].getrawmempool()) ==
CUSTOM_ANCESTOR_LIMIT + 1 + CUSTOM_DESCENDANT_LIMIT, timeout=10)
CUSTOM_ANCESTOR_LIMIT + CUSTOM_DESCENDANT_LIMIT, timeout=10)
mempool0 = self.nodes[0].getrawmempool(False)
mempool1 = self.nodes[1].getrawmempool(False)
assert set(mempool1).issubset(set(mempool0))
assert parent_transaction in mempool1
for tx in chain[:CUSTOM_DESCENDANT_LIMIT]:
# Note: this test is brittle, because it relies on the relay sort order
# of node0 to be based on ancestor count (so that the first 10
# descendants of parent_transaction relay before the later ones).
for tx in chain[:CUSTOM_DESCENDANT_LIMIT-1]:
assert tx in mempool1
for tx in chain[CUSTOM_DESCENDANT_LIMIT:]:
assert tx not in mempool1

1
test/functional/test_runner.py
View File

@@ -254,7 +254,6 @@ BASE_SCRIPTS = [
'feature_utxo_set_hash.py',
'feature_rbf.py',
'mempool_packages.py',
'mempool_package_onemore.py',
'mempool_package_limits.py',
'mempool_package_rbf.py',
'tool_utxo_to_sqlite.py',