phantom/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-24 18:23:26 -03:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to 10kvB

Browse source 
154b2b2296 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow)
a29f1df289 [policy] restrict all v3 transactions to 10kvB (glozow)
d578e2e354 [policy] explicitly require non-v3 for CPFP carve out (glozow)

Pull request description:

  Opening for discussion / conceptual review.

  We like the idea of a smaller maximum transaction size because:
  - It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction)
  - They are easier to bin-pack in block template production
  - They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space.

  History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510):
  - 2010-09-13 In 3df62878c3 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction()
  - 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well

  Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning).

  This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult.
  ~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number.

ACKs for top commit:
  sdaftuar:
    ACK 154b2b2296
  achow101:
    ACK 154b2b2296
  instagibbs:
    ACK 154b2b2296
  t-bast:
    ACK 154b2b2296
  murchandamus:
    crACK 154b2b2296

Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
This commit is contained in:
Ava Chow 2024-05-23 11:54:18 -04:00
commit 867f6af803
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
5 changed files with 76 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/policy/v3_policy.cpp
View file

@ -67,6 +67,12 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
// Now we have all ancestors, so we can start checking v3 rules.
if (ptx->nVersion == 3) {
// SingleV3Checks should have checked this already.
if (!Assume(vsize <= V3_MAX_VSIZE)) {
return strprintf("v3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_MAX_VSIZE);
}
if (mempool_ancestors.size() + in_package_parents.size() + 1 > V3_ANCESTOR_LIMIT) {
return strprintf("tx %s (wtxid=%s) would have too many ancestors",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
@ -186,6 +192,12 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTra
// The rest of the rules only apply to transactions with nVersion=3.
if (ptx->nVersion != 3) return std::nullopt;
if (vsize > V3_MAX_VSIZE) {
return std::make_pair(strprintf("v3 tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_MAX_VSIZE),
nullptr);
}
// Check that V3_ANCESTOR_LIMIT would not be violated.
if (mempool_ancestors.size() + 1 > V3_ANCESTOR_LIMIT) {
return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",

7
src/policy/v3_policy.h
View file

@ -24,11 +24,13 @@ static constexpr unsigned int V3_DESCENDANT_LIMIT{2};
/** Maximum number of transactions including a V3 tx and all its mempool ancestors. */
static constexpr unsigned int V3_ANCESTOR_LIMIT{2};
/** Maximum sigop-adjusted virtual size of all v3 transactions. */
static constexpr int64_t V3_MAX_VSIZE{10000};
/** Maximum sigop-adjusted virtual size of a tx which spends from an unconfirmed v3 transaction. */
static constexpr int64_t V3_CHILD_MAX_VSIZE{1000};
// These limits are within the default ancestor/descendant limits.
static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR <= DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000);
static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR <= DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1000);
static_assert(V3_MAX_VSIZE + V3_CHILD_MAX_VSIZE <= DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000);
static_assert(V3_MAX_VSIZE + V3_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * 1000);
/** Must be called for every transaction, even if not v3. Not strictly necessary for transactions
* accepted through AcceptMultipleTransactions.
@ -40,6 +42,7 @@ static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR
* 4. A v3's descendant set, including itself, must be within V3_DESCENDANT_LIMIT.
* 5. If a v3 tx has any unconfirmed ancestors, the tx's sigop-adjusted vsize must be within
* V3_CHILD_MAX_VSIZE.
* 6. A v3 tx must be within V3_MAX_VSIZE.
*
*
* @param[in] mempool_ancestors The in-mempool ancestors of ptx.

6
src/test/util/txmempool.cpp
View file

@ -119,9 +119,15 @@ void CheckMempoolV3Invariants(const CTxMemPool& tx_pool)
for (const auto& tx_info : tx_pool.infoAll()) {
const auto& entry = *Assert(tx_pool.GetEntry(tx_info.tx->GetHash()));
if (tx_info.tx->nVersion == 3) {
// Check that special maximum virtual size is respected
Assert(entry.GetTxSize() <= V3_MAX_VSIZE);
// Check that special v3 ancestor/descendant limits and rules are always respected
Assert(entry.GetCountWithDescendants() <= V3_DESCENDANT_LIMIT);
Assert(entry.GetCountWithAncestors() <= V3_ANCESTOR_LIMIT);
Assert(entry.GetSizeWithDescendants() <= V3_MAX_VSIZE + V3_CHILD_MAX_VSIZE);
Assert(entry.GetSizeWithAncestors() <= V3_MAX_VSIZE + V3_CHILD_MAX_VSIZE);
// If this transaction has at least 1 ancestor, it's a "child" and has restricted weight.
if (entry.GetCountWithAncestors() > 1) {
Assert(entry.GetTxSize() <= V3_CHILD_MAX_VSIZE);

5
src/validation.cpp
View file

@ -951,7 +951,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
// 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.
// descendant limit descendants. The transaction also cannot be v3,
// 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
@ -965,7 +966,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
.descendant_size_vbytes = maybe_rbf_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT,
};
const auto error_message{util::ErrorString(ancestors).original};
if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT) {
if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || ws.m_ptx->nVersion == 3) {
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", error_message);
}
if (auto ancestors_retry{m_pool.CalculateMemPoolAncestors(*entry, cpfp_carve_out_limits)}) {

57
test/functional/mempool_accept_v3.py
View file

@ -6,6 +6,7 @@ from decimal import Decimal
from test_framework.messages import (
MAX_BIP125_RBF_SEQUENCE,
WITNESS_SCALE_FACTOR,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
@ -21,6 +22,7 @@ from test_framework.wallet import (
)
MAX_REPLACEMENT_CANDIDATES = 100
V3_MAX_VSIZE = 10000
def cleanup(extra_args=None):
def decorator(func):
@ -49,6 +51,20 @@ class MempoolAcceptV3(BitcoinTestFramework):
assert_equal(len(txids), len(mempool_contents))
assert all([txid in txids for txid in mempool_contents])
@cleanup(extra_args=["-datacarriersize=20000", "-acceptnonstdtxn=1"])
def test_v3_max_vsize(self):
node = self.nodes[0]
self.log.info("Test v3-specific maximum transaction vsize")
tx_v3_heavy = self.wallet.create_self_transfer(target_weight=(V3_MAX_VSIZE + 1) * WITNESS_SCALE_FACTOR, version=3)
assert_greater_than_or_equal(tx_v3_heavy["tx"].get_vsize(), V3_MAX_VSIZE)
expected_error_heavy = f"v3-rule-violation, v3 tx {tx_v3_heavy['txid']} (wtxid={tx_v3_heavy['wtxid']}) is too big"
assert_raises_rpc_error(-26, expected_error_heavy, node.sendrawtransaction, tx_v3_heavy["hex"])
self.check_mempool([])
# Ensure we are hitting the v3-specific limit and not something else
tx_v2_heavy = self.wallet.send_self_transfer(from_node=node, target_weight=(V3_MAX_VSIZE + 1) * WITNESS_SCALE_FACTOR, version=2)
self.check_mempool([tx_v2_heavy["txid"]])
@cleanup(extra_args=["-datacarriersize=1000", "-acceptnonstdtxn=1"])
def test_v3_acceptance(self):
node = self.nodes[0]
@ -201,10 +217,24 @@ class MempoolAcceptV3(BitcoinTestFramework):
"""
node = self.nodes[0]
self.log.info("Test that a decreased limitdescendantsize also applies to v3 child")
tx_v3_parent_large1 = self.wallet.send_self_transfer(from_node=node, target_weight=99900, version=3)
tx_v3_child_large1 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent_large1["new_utxo"], version=3)
# Child is within v3 limits, but parent's descendant limit is exceeded
assert_greater_than(1000, tx_v3_child_large1["tx"].get_vsize())
parent_target_weight = 9990 * WITNESS_SCALE_FACTOR
child_target_weight = 500 * WITNESS_SCALE_FACTOR
tx_v3_parent_large1 = self.wallet.send_self_transfer(
from_node=node,
target_weight=parent_target_weight,
version=3
)
tx_v3_child_large1 = self.wallet.create_self_transfer(
utxo_to_spend=tx_v3_parent_large1["new_utxo"],
target_weight=child_target_weight,
version=3
)
# Parent and child are within v3 limits, but parent's 10kvB descendant limit is exceeded
assert_greater_than_or_equal(V3_MAX_VSIZE, tx_v3_parent_large1["tx"].get_vsize())
assert_greater_than_or_equal(1000, tx_v3_child_large1["tx"].get_vsize())
assert_greater_than(tx_v3_parent_large1["tx"].get_vsize() + tx_v3_child_large1["tx"].get_vsize(), 10000)
assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds descendant size limit for tx {tx_v3_parent_large1['txid']}", node.sendrawtransaction, tx_v3_child_large1["hex"])
self.check_mempool([tx_v3_parent_large1["txid"]])
assert_equal(node.getmempoolentry(tx_v3_parent_large1["txid"])["descendantcount"], 1)
@ -212,10 +242,22 @@ class MempoolAcceptV3(BitcoinTestFramework):
self.log.info("Test that a decreased limitancestorsize also applies to v3 parent")
self.restart_node(0, extra_args=["-limitancestorsize=10", "-datacarriersize=40000", "-acceptnonstdtxn=1"])
tx_v3_parent_large2 = self.wallet.send_self_transfer(from_node=node, target_weight=99900, version=3)
tx_v3_child_large2 = self.wallet.create_self_transfer(utxo_to_spend=tx_v3_parent_large2["new_utxo"], version=3)
# Child is within v3 limits
tx_v3_parent_large2 = self.wallet.send_self_transfer(
from_node=node,
target_weight=parent_target_weight,
version=3
)
tx_v3_child_large2 = self.wallet.create_self_transfer(
utxo_to_spend=tx_v3_parent_large2["new_utxo"],
target_weight=child_target_weight,
version=3
)
# Parent and child are within v3 limits
assert_greater_than_or_equal(V3_MAX_VSIZE, tx_v3_parent_large2["tx"].get_vsize())
assert_greater_than_or_equal(1000, tx_v3_child_large2["tx"].get_vsize())
assert_greater_than(tx_v3_parent_large2["tx"].get_vsize() + tx_v3_child_large2["tx"].get_vsize(), 10000)
assert_raises_rpc_error(-26, f"too-long-mempool-chain, exceeds ancestor size limit", node.sendrawtransaction, tx_v3_child_large2["hex"])
self.check_mempool([tx_v3_parent_large2["txid"]])
@ -585,6 +627,7 @@ class MempoolAcceptV3(BitcoinTestFramework):
node = self.nodes[0]
self.wallet = MiniWallet(node)
self.generate(self.wallet, 120)
self.test_v3_max_vsize()
self.test_v3_acceptance()
self.test_v3_replacement()
self.test_v3_bip125()