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

pull glozow wants to merge 6 commits into bitcoin:master from glozow:2024-06-v3-followups changing 14 files +314 −302
  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, ismaelsadeeq, achow101
    Concept ACK 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)
    • #28690 (build: Introduce internal kernel library by TheCharlatan)
    • #28676 ([WIP] Cluster mempool implementation by sdaftuar)
    • #28132 (policy: Enable full-rbf by default by petertodd)

    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. glozow force-pushed on Jun 18, 2024
  12. glozow marked this as ready for review on Jun 18, 2024
  13. DrahtBot removed the label Needs rebase on Jun 18, 2024
  14. DrahtBot added the label CI failed on Jun 18, 2024
  15. theStack commented at 3:21 pm on June 18, 2024: contributor
    Concept ACK 🚜
  16. in doc/release-notes-29496.md:6 in a6821a0a4e outdated
    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:

    glozow commented at 10:50 am on July 2, 2024:
    added
  17. instagibbs approved
  18. instagibbs commented at 4:36 pm on June 18, 2024: member

    ACK a6821a0a4e1cf2aba679962289f4beeefd3d539a

    fairly mechanical changes

  19. DrahtBot requested review from theStack on Jun 18, 2024
  20. DrahtBot requested review from ismaelsadeeq on Jun 18, 2024
  21. DrahtBot removed the label CI failed on Jun 18, 2024
  22. glozow commented at 8:58 am on June 19, 2024: member
    cc @murchandamus @ismaelsadeeq since you suggested the doc changes
  23. 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",
    

    glozow commented at 11:06 am on July 2, 2024:
    updated all
  24. 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 
    

    glozow commented at 10:51 am on July 2, 2024:
    fixed
  25. 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.


    glozow commented at 10:51 am on July 2, 2024:
    did unit test docs as well
  26. 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?

    glozow commented at 10:51 am on July 2, 2024:
    added
  27. 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

  28. DrahtBot requested review from ismaelsadeeq on Jun 19, 2024
  29. rename mempool_accept_v3.py to mempool_truc.py 089b5757df
  30. [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.
    Also keep variable names in tests, as it is less verbose to keep v3 and v2.
    a573dd2617
  31. 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')
    sed -i 's/CheckMempoolV3Invariants/CheckMempoolTRUCInvariants/g' $(git grep -l 'CheckMempoolV3Invariants')
    -END VERIFY SCRIPT-
    881fac8e60
  32. glozow force-pushed on Jul 2, 2024
  33. glozow commented at 11:07 am on July 2, 2024: member
    thanks @ismaelsadeeq! Took your suggestions and fixed docs in a few more places
  34. use version=3 instead of v3 in debug strings
    Make it more clear to the user what we mean by v3.
    19a9b90617
  35. [doc] add release note for TRUC 926b8e39dc
  36. glozow force-pushed on Jul 2, 2024
  37. instagibbs commented at 3:32 pm on July 2, 2024: member

    reACK https://github.com/bitcoin/bitcoin/pull/30272/commits/926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746

    reviewed via “git range-diff master a6821a0 926b8e3”

  38. ismaelsadeeq approved
  39. ismaelsadeeq commented at 5:44 pm on July 2, 2024: member
    Code review ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746
  40. DrahtBot requested review from achow101 on Jul 2, 2024
  41. achow101 commented at 9:41 pm on July 2, 2024: member
    ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746
  42. achow101 merged this on Jul 2, 2024
  43. achow101 closed this on Jul 2, 2024

  44. hebasto added the label Needs CMake port on Jul 3, 2024
  45. glozow deleted the branch on Jul 3, 2024
  46. hebasto commented at 6:57 am on July 14, 2024: member
    Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/264.
  47. hebasto removed the label Needs CMake port on Jul 14, 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-12-26 21:12 UTC

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