doc: use TRUC instead of v3 and add release note #30272

pull glozow wants to merge 5 commits into bitcoin:master from glozow:2024-06-v3-followups changing 13 files +253 −242
  1. glozow commented at 2:00 pm on June 12, 2024: member

    Adds a release note for TRUC policy which will be live in v28.0.

    For clarity, replaces mentions of “v3” with “TRUC” in most places. Suggested in

    I changed error strings from “v3-violation” to “TRUC-violation” but left v3 in the debug strings because I think it might be clearer for somebody who is debugging. Similarly, I left some variables unchanged because I think they’re more descriptive this way, e.g. tx_v3_from_v2_and_v3. I’m happy to debate places that should or shouldn’t be documented differently in this PR, whatever is clearest to everyone.

  2. glozow added the label Docs on Jun 12, 2024
  3. glozow added this to the milestone 28.0 on Jun 12, 2024
  4. DrahtBot commented at 2:00 pm on June 12, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK instagibbs
    Concept ACK ismaelsadeeq, theStack

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #30239 (Ephemeral Anchors, take 2 by instagibbs)
    • #30079 (Fee Estimation: Ignore all transactions that are CPFP’d by ismaelsadeeq)
    • #29790 ([DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system – WIP by hebasto)
    • #28710 (Remove the legacy wallet and BDB dependency by achow101)
    • #28676 ([WIP] Cluster mempool implementation by sdaftuar)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  5. glozow force-pushed on Jun 12, 2024
  6. DrahtBot added the label CI failed on Jun 12, 2024
  7. DrahtBot removed the label CI failed on Jun 12, 2024
  8. ismaelsadeeq commented at 3:16 pm on June 13, 2024: member
    Concept ACK
  9. DrahtBot added the label Needs rebase on Jun 17, 2024
  10. rename policy/v3_policy.* to policy/truc_policy.* f543852a89
  11. [doc] replace mentions of v3 with TRUC
    Keep mentions of "v3" in debug strings to help people who might not know
    that TRUC is applied when version=3.
    f5b26e5b9b
  12. scripted-diff: change names from V3 to TRUC
    -BEGIN VERIFY SCRIPT-
    sed -i 's/SingleV3Checks/SingleTRUCChecks/g' $(git grep -l 'SingleV3Checks')
    sed -i 's/PackageV3Checks/PackageTRUCChecks/g' $(git grep -l 'PackageV3Checks')
    sed -i 's/PV3C/PTRUCC/g' src/policy/truc_policy.h
    sed -i 's/V3_MAX_VSIZE/TRUC_MAX_VSIZE/g' $(git grep -l 'V3_MAX_VSIZE')
    sed -i 's/V3_CHILD_MAX_VSIZE/TRUC_CHILD_MAX_VSIZE/g' $(git grep -l 'V3_CHILD_MAX_VSIZE')
    sed -i 's/V3_DESCENDANT_LIMIT/TRUC_DESCENDANT_LIMIT/g' $(git grep -l 'V3_DESCENDANT_LIMIT')
    sed -i 's/V3_ANCESTOR_LIMIT/TRUC_ANCESTOR_LIMIT/g' $(git grep -l 'V3_ANCESTOR_LIMIT')
    -END VERIFY SCRIPT-
    8994c2b4e3
  13. rename mempool_accept_v3.py to mempool_truc.py d96fa7bedb
  14. [doc] add release note for TRUC a6821a0a4e
  15. glozow force-pushed on Jun 18, 2024
  16. glozow marked this as ready for review on Jun 18, 2024
  17. DrahtBot removed the label Needs rebase on Jun 18, 2024
  18. DrahtBot added the label CI failed on Jun 18, 2024
  19. theStack commented at 3:21 pm on June 18, 2024: contributor
    Concept ACK 🚜
  20. in doc/release-notes-29496.md:6 in a6821a0a4e
    0@@ -0,0 +1,10 @@
    1+Mempool Policy Changes
    2+----------------------
    3+
    4+- Transactions with version number set to 3 are now treated as standard on all networks (#29496),
    5+  subject to Opt-in Topologically Restricted Until Confirmation (TRUC) Transactions policy as
    6+  described in BIP 431.  The policy includes limits on spending unconfirmed outputs (#28948), eviction
    


    instagibbs commented at 4:33 pm on June 18, 2024:
  21. instagibbs approved
  22. instagibbs commented at 4:36 pm on June 18, 2024: member

    ACK a6821a0a4e1cf2aba679962289f4beeefd3d539a

    fairly mechanical changes

  23. DrahtBot requested review from theStack on Jun 18, 2024
  24. DrahtBot requested review from ismaelsadeeq on Jun 18, 2024
  25. DrahtBot removed the label CI failed on Jun 18, 2024
  26. glozow commented at 8:58 am on June 19, 2024: member
    cc @murchandamus @ismaelsadeeq since you suggested the doc changes
  27. in src/policy/truc_policy.cpp:208 in f5b26e5b9b outdated
    202@@ -203,7 +203,7 @@ std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTra
    203 
    204     // Remaining checks only pertain to transactions with unconfirmed ancestors.
    205     if (mempool_ancestors.size() > 0) {
    206-        // If this transaction spends V3 parents, it cannot be too large.
    207+        // If this transaction spends TRUC parents, it cannot be too large.
    208         if (vsize > V3_CHILD_MAX_VSIZE) {
    209             return std::make_pair(strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
    


    ismaelsadeeq commented at 9:07 pm on June 19, 2024:

    nit: the v3 mentions in debug strings should be explicitly stated as version=3 for clarity. here and other places

    0            return std::make_pair(strprintf("version=3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
    
  28. in src/policy/truc_policy.h:15 in f5b26e5b9b outdated
    14@@ -15,11 +15,12 @@
    15 #include <set>
    


    ismaelsadeeq commented at 9:16 pm on June 19, 2024:

    In f5b26e5b9be41cdab6e75aa0a3fbbb765a96ed06 Here are a couple of places that were not replaced

      0diff --git a/src/policy/truc_policy.h b/src/policy/truc_policy.h
      1index d1e65806cc0..37a445bcc14 100644
      2--- a/src/policy/truc_policy.h
      3+++ b/src/policy/truc_policy.h
      4@@ -23,18 +23,18 @@ static constexpr decltype(CTransaction::version) TRUC_VERSION{3};
      5 // of 2 and ancestor set size of 2.
      6 /** Maximum number of transactions including an unconfirmed tx and its descendants. */
      7 static constexpr unsigned int TRUC_DESCENDANT_LIMIT{2};
      8-/** Maximum number of transactions including a V3 tx and all its mempool ancestors. */
      9+/** Maximum number of transactions including a TRUC tx and all its mempool ancestors. */
     10 static constexpr unsigned int TRUC_ANCESTOR_LIMIT{2};
     11 
     12-/** Maximum sigop-adjusted virtual size of all v3 transactions. */
     13+/** Maximum sigop-adjusted virtual size of all TRUC transactions. */
     14 static constexpr int64_t TRUC_MAX_VSIZE{10000};
     15-/** Maximum sigop-adjusted virtual size of a tx which spends from an unconfirmed v3 
     16transaction. */
     17+/** Maximum sigop-adjusted virtual size of a tx which spends from an unconfirmed TRU
     18C transaction. */
     19 static constexpr int64_t TRUC_CHILD_MAX_VSIZE{1000};
     20 // These limits are within the default ancestor/descendant limits.
     21 static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_ANCESTOR_SIZE_LIMIT_K
     22VB * 1000);
     23 static_assert(TRUC_MAX_VSIZE + TRUC_CHILD_MAX_VSIZE <= DEFAULT_DESCENDANT_SIZE_LIMIT
     24_KVB * 1000);
     25 
     26-/** Must be called for every transaction, even if not v3. Not strictly necessary for
     27 transactions
     28+/** Must be called for every transaction, even if not TRUC. Not strictly necessary f
     29or transactions
     30  * accepted through AcceptMultipleTransactions.
     31  *
     32  * Checks the following rules:
     33diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp
     34index ecacfed58ed..71f72847dd4 100644
     35--- a/src/test/fuzz/package_eval.cpp
     36+++ b/src/test/fuzz/package_eval.cpp
     37@@ -225,7 +225,7 @@ FUZZ_TARGET(tx_package_eval, .init = initialize_tx_pool)
     38                     tx_mut.vin.emplace_back();
     39                 }
     40 
     41-                // Make a p2pk output to make sigops adjusted vsize to violate v3, p
     42otentially, which is never spent
     43+                // Make a p2pk output to make sigops adjusted vsize to violate TRUC,
     44 potentially, which is never spent
     45                 if (last_tx && amount_in > 1000 && fuzzed_data_provider.ConsumeBool(
     46)) {
     47                     tx_mut.vout.emplace_back(1000, CScript() << std::vector<unsigned
     48 char>(33, 0x02) << OP_CHECKSIG);
     49                     // Don't add any other outputs.
     50diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp
     51index 1153df4ade7..37cd0de5d2a 100644
     52--- a/src/test/util/txmempool.cpp
     53+++ b/src/test/util/txmempool.cpp
     54@@ -150,7 +150,7 @@ void CheckMempoolV3Invariants(const CTxMemPool& tx_pool)
     55             // Check that special maximum virtual size is respected
     56             Assert(entry.GetTxSize() <= TRUC_MAX_VSIZE);
     57 
     58-            // Check that special v3 ancestor/descendant limits and rules are always
     59 respected
     60+            // Check that special TRUC ancestor/descendant limits and rules are alwa
     61ys respected
     62             Assert(entry.GetCountWithDescendants() <= TRUC_DESCENDANT_LIMIT);
     63             Assert(entry.GetCountWithAncestors() <= TRUC_ANCESTOR_LIMIT);
     64             Assert(entry.GetSizeWithDescendants() <= TRUC_MAX_VSIZE + TRUC_CHILD_MAX
     65_VSIZE);
     66@@ -159,12 +159,12 @@ void CheckMempoolV3Invariants(const CTxMemPool& tx_pool)
     67             // If this transaction has at least 1 ancestor, it's a "child" and has r
     68estricted weight.
     69             if (entry.GetCountWithAncestors() > 1) {
     70                 Assert(entry.GetTxSize() <= TRUC_CHILD_MAX_VSIZE);
     71-                // All v3 transactions must only have v3 unconfirmed parents.
     72+                // All TRUC transactions must only have TRUC unconfirmed parents.
     73                 const auto& parents = entry.GetMemPoolParentsConst();
     74                 Assert(parents.begin()->get().GetSharedTx()->version == TRUC_VERSION
     75);
     76             }
     77         } else if (entry.GetCountWithAncestors() > 1) {
     78-            // All non-v3 transactions must only have non-v3 unconfirmed parents.
     79+            // All non-TRUC transactions must only have non-TRUC unconfirmed parents
     80.
     81             for (const auto& parent : entry.GetMemPoolParentsConst()) {
     82                 Assert(parent.get().GetSharedTx()->version != TRUC_VERSION);
     83             }
     84diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h
     85index 9986cabf7e9..57027cc0e62 100644
     86--- a/src/test/util/txmempool.h
     87+++ b/src/test/util/txmempool.h
     88@@ -47,12 +47,12 @@ std::optional<std::string>  CheckPackageMempoolAcceptResult(const
     89 Package& txns,
     90                                                             bool expect_valid,
     91                                                             const CTxMemPool* mempoo
     92l);
     93 
     94-/** For every transaction in tx_pool, check v3 invariants:
     95- * - a v3 tx's ancestor count must be within V3_ANCESTOR_LIMIT
     96- * - a v3 tx's descendant count must be within V3_DESCENDANT_LIMIT
     97- * - if a v3 tx has ancestors, its sigop-adjusted vsize must be within V3_CHILD_MA
     98X_VSIZE
     99- * - any non-v3 tx must only have non-v3 parents
    100- * - any v3 tx must only have v3 parents
    101+/** For every transaction in tx_pool, check TRUC invariants:
    102+ * - a TRUC tx's ancestor count must be within V3_ANCESTOR_LIMIT
    103+ * - a TRUC tx's descendant count must be within V3_DESCENDANT_LIMIT
    104+ * - if a TRUC tx has ancestors, its sigop-adjusted vsize must be within V3_CHILD_
    105MAX_VSIZE
    106+ * - any non-TRUC tx must only have non-TRUC parents
    107+ * - any TRUC tx must only have TRUC parents
    108  *   */
    109 void CheckMempoolV3Invariants(const CTxMemPool& tx_pool);
    110 
    
  29. in src/policy/truc_policy.h:17 in f5b26e5b9b outdated
    14@@ -15,11 +15,12 @@
    15 #include <set>
    16 #include <string>
    17 
    


    ismaelsadeeq commented at 9:24 pm on June 19, 2024:

    In f5b26e5b9be41cdab6e75aa0a3fbbb765a96ed06

    This left out v3 mentions in the unit tests, maybe just leave out just the variable names as you stated from PR description.

  30. in src/policy/truc_policy.cpp:16 in 8994c2b4e3 outdated
    13@@ -14,7 +14,7 @@
    14 #include <numeric>
    15 #include <vector>
    16 
    


    ismaelsadeeq commented at 9:26 pm on June 19, 2024:
    In 8994c2b4e307cf63ed717b68cf5b8adb47f53cd1 Add CheckMempoolV3Invariants also?
  31. in test/functional/mempool_accept_v3.py:645 in f5b26e5b9b outdated
    643@@ -644,4 +644,4 @@ def run_test(self):
    644 
    645 
    


    ismaelsadeeq commented at 9:37 pm on June 19, 2024:

    Maybe update test/functional/wallet_create_tx.py test.

    see https://github.com/bitcoin/bitcoin/commit/d3fa531e67c0af77f52e754ad47a8396f2cab4c4, if it makes sense feel free to cherry-pick

  32. DrahtBot requested review from ismaelsadeeq on Jun 19, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-01 10:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me