policy: bump TX_MAX_STANDARD_VERSION to 3 #29496

glozow · 2024-02-27T22:19:48Z

Make nVersion=3 (which is currently nonstandard on mainnet) standard.

Note that we will treat these transactions as Topologically Restricted Until Confirmation (TRUC). Spec is in BIP 431 and implementation is in #28948, #29306, and #29873

See #27463 for overall project tracking, and #29319 for information about relevance to cluster mempool.

DrahtBot · 2024-02-27T22:19:51Z

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, murchandamus, ismaelsadeeq, sdaftuar, achow101
Concept ACK	hernanmarino

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:

#29325 (consensus: Store transaction nVersion as uint32_t by achow101)
#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.

ariard · 2024-03-04T02:31:24Z

What is your thinking on #29454 ?

I would still favor to make the 1000 vb limit dynamic and applying on the whole package, not only the child.
Doing with v3 itself would present a) a policy signaling mechanism saving and b) avoid some policy bargains like we had with mempoolfullrbf (between LN-vs-0-confs).

If you prefer to have the discussion on the v3 BIP PR let me know.

ariard · 2024-03-07T00:16:36Z

What is your thinking on #29454 ?

Same as said that for sibling evictions - I’ll go to hack a tx-relay jamming / pinning toolkit, that way we’ll have more information on the robustness of those current proposed anti-pinning mitigations. I still maintain there are very incomplete for time-sensitive transaction flows, especially in face of “loophole” and NTA pinnings. I’m incredibly worried for the security of the Lightning Network if we don’t deploy additional anti-pinning mitigations on top of those ones over the future years - We have not seen yet script kiddies attacking the network.

Under today inertia, we’ll probably converge towards a centralized network of ~50 LSPs relying on traditional social trust rather than relying on trust-minimized confirmation of pre-signed transactions. This is very problematic if they wish to accept channel traffic from low-resources spokes LN nodes, especially in geographical areas where social trust mechanism cannot be afforded by end-users.

On #29454, I still prefer to keep experimenting on “transaction-issuer selected policy limits” which might encompass more stringent tx-relay jamming on a minor fork of Bitcoin Core. Coming up with a sound technical proposal in matters of tx-relay policy likely demands few rounds of iterations and realistically we already have a severe shortage of qualified review bandwidth in those areas.

The initial reason closing of #29448 was my own realization that the introduction of differentiated tx-relay policy among the tx-relay network of nodes will probably generate with time tiers of transaction economic traffic priority (I really cannot see how it worsens memory / CPU DoS robustness per se ?). I don’t think this is a phenomena we can do a lot as a full-node implementation, as we’re seeing with the appearance of transaction accelerators by mining pools. There is transaction issuer demand for such services apparently. Yet we should be careful of doing works in parallel to maintain bip152 block propagation and as such long-term decentralization of the base-layer network.

I’m not opposing the deployment of TX_MAX_STANDARD_VERSION=3 in Bitcoin Core v28.0.

glozow · 2024-03-13T12:13:01Z

Rebased

murchandamus

Concept ACK and code changes look simple enough. Given that the proposed rules for V3 transactions entail a voluntary topology restriction by the sender on their own transactions, it seems reasonable to me to merge this into the master branch soon

hernanmarino

Concept ACK 7b18f0e

glozow · 2024-05-23T13:55:23Z

Added constant for #29873 (comment)
Also added a test that wallet does not signal v3 as it uses the default CURRENT_VERSION (this is correct as it wouldn't be safe to make transactions v3 before any of the network has adopted this change). I believe the nVersion can be changed via CCoinControl::m_version, though I'm not sure if that is accessible so I didn't write a test for it.

Rebased on top of #29873 + master (conflict with #29086)

murchandamus

Still looks good to me

crACK e41dae3

src/policy/v3_policy.h

test/functional/wallet_create_tx.py

glozow · 2024-05-24T09:32:17Z

Rebased for #30072

instagibbs · 2024-05-30T14:47:51Z

src/validation.cpp

@@ -1017,7 +1017,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
            .descendant_count = maybe_rbf_limits.descendant_count + 1,
            .descendant_size_vbytes = maybe_rbf_limits.descendant_size_vbytes + EXTRA_DESCENDANT_TX_SIZE_LIMIT,
        };
-        if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || ws.m_ptx->nVersion == 3) {


few remaining spots that I can see:

src/test/fuzz/package_eval.cpp:176: tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION; src/test/fuzz/tx_pool.cpp:229: tx_mut.nVersion = fuzzed_data_provider.ConsumeBool() ? 3 : CTransaction::CURRENT_VERSION; src/test/txvalidation_tests.cpp:289: mtx_many_sigops.nVersion = 3; src/test/util/txmempool.cpp:121: if (tx_info.tx->nVersion == 3) { src/test/util/txmempool.cpp:136: Assert(parents.begin()->get().GetSharedTx()->nVersion == 3); src/test/util/txmempool.cpp:141: Assert(parent.get().GetSharedTx()->nVersion != 3);

can also use this constant

thanks! fixed these now

Note that, as CURRENT_VERSION = 2, the wallet will not make transactions with nVersion=3 yet.

instagibbs

utACK 30a0113

ismaelsadeeq

Concept ACK

ismaelsadeeq · 2024-06-03T13:45:30Z

src/policy/v3_policy.h

@@ -15,8 +15,9 @@
 #include <set>
 #include <string>

-// This module enforces rules for transactions with nVersion=3 ("v3 transactions") which help make
+// This module enforces rules for BIP 431 TRUC transactions (with nVersion=3) which help make


nit, I think the mention of TRUC transactions suffices, from the BIP specification we know they have nVersion=3?

Suggested change

// This module enforces rules for BIP 431 TRUC transactions (with nVersion=3) which help make

// This module enforces rules for BIP 431 TRUC transactions which help make

The file name could also be renamed to refer to TRUC transactions instead of v3_policy.

done in #30272

ismaelsadeeq · 2024-06-03T13:52:03Z

src/policy/v3_policy.cpp

@@ -146,14 +146,14 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
    } else {
        // Non-v3 transactions cannot have v3 parents.


Should we replace all v3 mentions with TRUC for clarity and uniformity.
The BIP specification refers to this policy rules as TRUC Transactions.

Sure, can be in another PR

done in #30272

murchandamus

ACK 30a0113

murchandamus · 2024-06-06T15:24:17Z

src/policy/v3_policy.h

@@ -15,8 +15,9 @@
 #include <set>
 #include <string>

-// This module enforces rules for transactions with nVersion=3 ("v3 transactions") which help make
+// This module enforces rules for BIP 431 TRUC transactions (with nVersion=3) which help make


The file name could also be renamed to refer to TRUC transactions instead of v3_policy.

ismaelsadeeq

ACK 30a0113 🛰️

sdaftuar · 2024-06-07T14:04:44Z

utACK 30a0113

achow101 · 2024-06-07T16:19:39Z

ACK 30a0113

926b8e3 [doc] add release note for TRUC (glozow) 19a9b90 use version=3 instead of v3 in debug strings (glozow) 881fac8 scripted-diff: change names from V3 to TRUC (glozow) a573dd2 [doc] replace mentions of v3 with TRUC (glozow) 089b575 rename mempool_accept_v3.py to mempool_truc.py (glozow) f543852 rename policy/v3_policy.* to policy/truc_policy.* (glozow) Pull request description: 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 - #29496 (comment) - #29496 (comment) 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. ACKs for top commit: instagibbs: reACK 926b8e3 achow101: ACK 926b8e3 ismaelsadeeq: Code review ACK 926b8e3 Tree-SHA512: 16c88add0a29dc6d1236c4d45f34a17b850f6727b231953cbd52eb9f7268d1d802563eadfc8b7928c94ed3d7a615275dd103e57e81439ebf3ba2b12efa1e42af

glozow added the TX fees and policy label Feb 27, 2024

glozow changed the title ~~policy: make maximum standard transaction version 3~~ policy: bump TX_MAX_STANDARD_VERSION to 3 Feb 27, 2024

sdaftuar mentioned this pull request Feb 27, 2024

Cluster mempool, CPFP carveout, and V3 transaction policy #29319

Open

DrahtBot mentioned this pull request Feb 28, 2024

consensus: Store transaction nVersion as uint32_t #29325

Merged

glozow mentioned this pull request Feb 28, 2024

Package Relay Project Tracking #27463

Open

42 tasks

DrahtBot mentioned this pull request Feb 29, 2024

policy: enable sibling eviction for v3 transactions #29306

Merged

This was referenced Mar 8, 2024

Cluster mempool implementation #28676

Open

policy: Enable full-rbf by default #28132

Closed

DrahtBot added the Needs rebase label Mar 12, 2024

glozow force-pushed the 2024-02-v3-live branch from c839efe to 7b18f0e Compare March 13, 2024 12:12

DrahtBot removed the Needs rebase label Mar 13, 2024

murchandamus reviewed Mar 13, 2024

View reviewed changes

theStack mentioned this pull request Apr 9, 2024

test: p2p: add test for rejected tx request logic (m_recent_rejects filter) #29827

Closed

hernanmarino reviewed Apr 12, 2024

View reviewed changes

DrahtBot mentioned this pull request Apr 15, 2024

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

Merged

glozow mentioned this pull request May 23, 2024

BIP431: Opt In Topologically Restricted Until Confirmation Transactions For More Robust Fee-bumping bitcoin/bips#1541

Merged

glozow force-pushed the 2024-02-v3-live branch 2 times, most recently from 85def66 to e41dae3 Compare May 23, 2024 13:52

murchandamus reviewed May 23, 2024

View reviewed changes

src/policy/v3_policy.h Outdated Show resolved Hide resolved

test/functional/wallet_create_tx.py Show resolved Hide resolved

DrahtBot requested a review from hernanmarino May 23, 2024 15:19

glozow force-pushed the 2024-02-v3-live branch from e41dae3 to 72c0eb7 Compare May 23, 2024 16:41

DrahtBot mentioned this pull request May 23, 2024

refactor prep for package rbf #30072

Merged

glozow force-pushed the 2024-02-v3-live branch from 72c0eb7 to 6d67b9a Compare May 24, 2024 09:32

instagibbs reviewed May 30, 2024

View reviewed changes

glozow added 3 commits May 31, 2024 08:46

[refactor] use TRUC_VERSION in place of 3

052ede7

[policy] make v3 transactions standard

539404f

Note that, as CURRENT_VERSION = 2, the wallet will not make transactions with nVersion=3 yet.

[test] wallet uses CURRENT_VERSION which is 2

9dbe6a0

glozow force-pushed the 2024-02-v3-live branch from 6d67b9a to 9dbe6a0 Compare June 2, 2024 07:00

[doc] update bips.md for 431

30a0113

instagibbs approved these changes Jun 3, 2024

View reviewed changes

DrahtBot requested a review from murchandamus June 3, 2024 13:35

ismaelsadeeq reviewed Jun 3, 2024

View reviewed changes

murchandamus reviewed Jun 6, 2024

View reviewed changes

DrahtBot requested a review from ismaelsadeeq June 6, 2024 15:26

instagibbs mentioned this pull request Jun 6, 2024

Ephemeral Dust #30239

Merged

ismaelsadeeq approved these changes Jun 6, 2024

View reviewed changes

achow101 merged commit 6e4d18f into bitcoin:master Jun 7, 2024

glozow deleted the 2024-02-v3-live branch June 10, 2024 09:10

This was referenced Jun 10, 2024

Cluster size 2 package rbf #28984

Merged

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

Merged

delbonis mentioned this pull request Jun 14, 2024

Socratic Seminar 24 Reading List (June 18, 2024) arminsabouri/bostonbitdevs#31

Closed

5twelve approved these changes Jun 29, 2024

View reviewed changes

525411 mentioned this pull request Jul 27, 2024

Still looks good to me sigstore/protobuf-specs#374

Closed

orangesurf mentioned this pull request Aug 29, 2024

Don't label TRUC as non-standard mempool/mempool#5488

Closed

mononaut mentioned this pull request Aug 29, 2024

Make v3 transactions standard mempool/mempool#5489

Merged

1 task

xyephy mentioned this pull request Feb 26, 2025

Upgrade to bitcoin v28 whiveio/whive#12

Open

bitcoin locked and limited conversation to collaborators Jun 29, 2025

	// This module enforces rules for BIP 431 TRUC transactions (with nVersion=3) which help make
	// This module enforces rules for BIP 431 TRUC transactions which help make

		@@ -146,14 +146,14 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
		} else {
		// Non-v3 transactions cannot have v3 parents.

policy: bump TX_MAX_STANDARD_VERSION to 3 #29496

policy: bump TX_MAX_STANDARD_VERSION to 3 #29496

Uh oh!

Conversation

glozow commented Feb 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Feb 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

ariard commented Mar 4, 2024

Uh oh!

ariard commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

glozow commented Mar 13, 2024

Uh oh!

murchandamus left a comment

Choose a reason for hiding this comment

Uh oh!

hernanmarino left a comment

Choose a reason for hiding this comment

Uh oh!

glozow commented May 23, 2024

Uh oh!

murchandamus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

glozow commented May 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

instagibbs left a comment

Choose a reason for hiding this comment

Uh oh!

ismaelsadeeq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

murchandamus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ismaelsadeeq left a comment

Choose a reason for hiding this comment

Uh oh!

sdaftuar commented Jun 7, 2024

Uh oh!

achow101 commented Jun 7, 2024

Uh oh!

Uh oh!

glozow commented Feb 27, 2024 •

edited

Loading

DrahtBot commented Feb 27, 2024 •

edited

Loading

ariard commented Mar 7, 2024 •

edited

Loading