diff options
author | Ava Chow <github@achow101.com> | 2024-05-23 11:54:18 -0400 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-05-23 11:54:18 -0400 |
commit | 867f6af803dcc9f0f754bf5cc683c853bb09051b (patch) | |
tree | da1f217a12f6170e01a4b23097d78b6d687be506 /src/policy | |
parent | 6300438a2e0b6ee21fe210072b7b8be2f4845c17 (diff) | |
parent | 154b2b2296edccb5ed24e829798dacb6195edc11 (diff) |
Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to 10kvB
154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow)
a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow)
d578e2e3540e085942001350ff3aeb047bdac973 [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 https://github.com/bitcoin/bitcoin/commit/3df62878c3cece15a8921fbbdee7859ee9368768 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 154b2b2296edccb5ed24e829798dacb6195edc11
achow101:
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
instagibbs:
ACK 154b2b2296edccb5ed24e829798dacb6195edc11
t-bast:
ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11
murchandamus:
crACK 154b2b2296edccb5ed24e829798dacb6195edc11
Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
Diffstat (limited to 'src/policy')
-rw-r--r-- | src/policy/v3_policy.cpp | 12 | ||||
-rw-r--r-- | src/policy/v3_policy.h | 7 |
2 files changed, 17 insertions, 2 deletions
diff --git a/src/policy/v3_policy.cpp b/src/policy/v3_policy.cpp index 3c3942d707..d44832fceb 100644 --- a/src/policy/v3_policy.cpp +++ b/src/policy/v3_policy.cpp @@ -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", diff --git a/src/policy/v3_policy.h b/src/policy/v3_policy.h index 2e56f8822b..25aff37a1b 100644 --- a/src/policy/v3_policy.h +++ b/src/policy/v3_policy.h @@ -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. |