diff options
author | Greg Sanders <gsanders87@gmail.com> | 2023-09-13 13:13:57 -0400 |
---|---|---|
committer | Greg Sanders <gsanders87@gmail.com> | 2023-09-20 08:10:30 -0400 |
commit | 533660c58ad5a218671a9bc1537299b1d67bb55d (patch) | |
tree | 778fdb31bdb83477dd84c6b44ead3ba8435a6dfa /src | |
parent | 3966b0a0b6b5e6110ba8106c04af1067fc6219bc (diff) |
Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT to avoid vbyte confusion
While allowing submitted packages to be slightly larger than what
may be allowed in the mempool to allow simpler reasoning
about contextual-less checks vs chain limits.
Diffstat (limited to 'src')
-rw-r--r-- | src/policy/packages.cpp | 8 | ||||
-rw-r--r-- | src/policy/packages.h | 20 | ||||
-rw-r--r-- | src/test/txpackage_tests.cpp | 12 |
3 files changed, 22 insertions, 18 deletions
diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp index a901ef8f38..fd272a2642 100644 --- a/src/policy/packages.cpp +++ b/src/policy/packages.cpp @@ -23,10 +23,10 @@ bool CheckPackage(const Package& txns, PackageValidationState& state) return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-many-transactions"); } - const int64_t total_size = std::accumulate(txns.cbegin(), txns.cend(), 0, - [](int64_t sum, const auto& tx) { return sum + GetVirtualTransactionSize(*tx); }); - // If the package only contains 1 tx, it's better to report the policy violation on individual tx size. - if (package_count > 1 && total_size > MAX_PACKAGE_SIZE * 1000) { + const int64_t total_weight = std::accumulate(txns.cbegin(), txns.cend(), 0, + [](int64_t sum, const auto& tx) { return sum + GetTransactionWeight(*tx); }); + // If the package only contains 1 tx, it's better to report the policy violation on individual tx weight. + if (package_count > 1 && total_weight > MAX_PACKAGE_WEIGHT) { return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-too-large"); } diff --git a/src/policy/packages.h b/src/policy/packages.h index 0a0e7cf6bb..702667b8ad 100644 --- a/src/policy/packages.h +++ b/src/policy/packages.h @@ -15,18 +15,22 @@ /** Default maximum number of transactions in a package. */ static constexpr uint32_t MAX_PACKAGE_COUNT{25}; -/** Default maximum total virtual size of transactions in a package in KvB. */ -static constexpr uint32_t MAX_PACKAGE_SIZE{101}; -static_assert(MAX_PACKAGE_SIZE * WITNESS_SCALE_FACTOR * 1000 >= MAX_STANDARD_TX_WEIGHT); +/** Default maximum total weight of transactions in a package in weight + to allow for context-less checks. This must allow a superset of sigops + weighted vsize limited transactions to not disallow transactions we would + have otherwise accepted individually. */ +static constexpr uint32_t MAX_PACKAGE_WEIGHT = 404'000; +static_assert(MAX_PACKAGE_WEIGHT >= MAX_STANDARD_TX_WEIGHT); -// If a package is submitted, it must be within the mempool's ancestor/descendant limits. Since a -// submitted package must be child-with-unconfirmed-parents (all of the transactions are an ancestor +// If a package is to be evaluated, it must be at least as large as the mempool's ancestor/descendant limits, +// otherwise transactions that would be individually accepted may be rejected in a package erroneously. +// Since a submitted package must be child-with-unconfirmed-parents (all of the transactions are an ancestor // of the child), package limits are ultimately bounded by mempool package limits. Ensure that the // defaults reflect this constraint. static_assert(DEFAULT_DESCENDANT_LIMIT >= MAX_PACKAGE_COUNT); static_assert(DEFAULT_ANCESTOR_LIMIT >= MAX_PACKAGE_COUNT); -static_assert(DEFAULT_ANCESTOR_SIZE_LIMIT_KVB >= MAX_PACKAGE_SIZE); -static_assert(DEFAULT_DESCENDANT_SIZE_LIMIT_KVB >= MAX_PACKAGE_SIZE); +static_assert(MAX_PACKAGE_WEIGHT >= DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * WITNESS_SCALE_FACTOR * 1000); +static_assert(MAX_PACKAGE_WEIGHT >= DEFAULT_DESCENDANT_SIZE_LIMIT_KVB * WITNESS_SCALE_FACTOR * 1000); /** A "reason" why a package was invalid. It may be that one or more of the included * transactions is invalid or the package itself violates our rules. @@ -47,7 +51,7 @@ class PackageValidationState : public ValidationState<PackageValidationResult> { /** Context-free package policy checks: * 1. The number of transactions cannot exceed MAX_PACKAGE_COUNT. - * 2. The total virtual size cannot exceed MAX_PACKAGE_SIZE. + * 2. The total weight cannot exceed MAX_PACKAGE_WEIGHT. * 3. If any dependencies exist between transactions, parents must appear before children. * 4. Transactions cannot conflict, i.e., spend the same inputs. */ diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index 10ab656d38..571b58156f 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -51,14 +51,14 @@ BOOST_FIXTURE_TEST_CASE(package_sanitization_tests, TestChain100Setup) BOOST_CHECK_EQUAL(state_too_many.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(state_too_many.GetRejectReason(), "package-too-many-transactions"); - // Packages can't have a total size of more than 101KvB. + // Packages can't have a total weight of more than 404'000WU. CTransactionRef large_ptx = create_placeholder_tx(150, 150); Package package_too_large; - auto size_large = GetVirtualTransactionSize(*large_ptx); - size_t total_size{0}; - while (total_size <= MAX_PACKAGE_SIZE * 1000) { + auto size_large = GetTransactionWeight(*large_ptx); + size_t total_weight{0}; + while (total_weight <= MAX_PACKAGE_WEIGHT) { package_too_large.push_back(large_ptx); - total_size += size_large; + total_weight += size_large; } BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT); PackageValidationState state_too_large; @@ -122,7 +122,7 @@ BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) // A single, giant transaction submitted through ProcessNewPackage fails on single tx policy. CTransactionRef giant_ptx = create_placeholder_tx(999, 999); - BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > MAX_PACKAGE_SIZE * 1000); + BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > DEFAULT_ANCESTOR_SIZE_LIMIT_KVB * 1000); auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /*test_accept=*/true); BOOST_CHECK(result_single_large.m_state.IsInvalid()); BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX); |