diff options
author | Ava Chow <github@achow101.com> | 2024-02-09 23:29:39 -0500 |
---|---|---|
committer | Ava Chow <github@achow101.com> | 2024-02-09 23:37:57 -0500 |
commit | 7143d4388407ab3d12005e55a02d5e8f334e4dc9 (patch) | |
tree | a377c5653b546f1eee5d4c02f074760e8a076bab /src/validation.cpp | |
parent | 1d334d830f9423caef0a230dc62e4458a56d3bbe (diff) | |
parent | 29029df5c700e6940c712028303761d91ae15847 (diff) | |
download | bitcoin-7143d4388407ab3d12005e55a02d5e8f334e4dc9.tar.xz |
Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinning
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow)
e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow)
1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow)
27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke)
9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow)
eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow)
9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow)
158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow)
Pull request description:
See #27463 for overall package relay tracking.
Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340
Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418
Rationale:
- There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2]
- Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution.
V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2.
Immediate benefits:
- You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later.
- Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction.
This also enables some other cool things (again see #27463 for overall roadmap):
- Ephemeral Anchors
- Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees.
- We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use.
- We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6].
[1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html
[2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward.
[3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html
[4]: Original PR #25038 also contains a lot of the discussion
[5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7
[6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12
ACKs for top commit:
sdaftuar:
ACK 29029df5c700e6940c712028303761d91ae15847
achow101:
ACK 29029df5c700e6940c712028303761d91ae15847
instagibbs:
ACK 29029df5c700e6940c712028303761d91ae15847 modulo that
Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
Diffstat (limited to 'src/validation.cpp')
-rw-r--r-- | src/validation.cpp | 31 |
1 files changed, 25 insertions, 6 deletions
diff --git a/src/validation.cpp b/src/validation.cpp index caa4ff3115..0552bde665 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -29,6 +29,7 @@ #include <logging/timer.h> #include <node/blockstorage.h> #include <node/utxo_snapshot.h> +#include <policy/v3_policy.h> #include <policy/policy.h> #include <policy/rbf.h> #include <policy/settings.h> @@ -333,7 +334,9 @@ void Chainstate::MaybeUpdateMempoolForReorg( // Also updates valid entries' cached LockPoints if needed. // If false, the tx is still valid and its lockpoints are updated. // If true, the tx would be invalid in the next block; remove this entry and all of its descendants. - const auto filter_final_and_mature = [this](CTxMemPool::txiter it) + // Note that v3 rules are not applied here, so reorgs may cause violations of v3 inheritance or + // topology restrictions. + const auto filter_final_and_mature = [&](CTxMemPool::txiter it) EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs, ::cs_main) { AssertLockHeld(m_mempool->cs); AssertLockHeld(::cs_main); @@ -583,7 +586,7 @@ private: struct Workspace { explicit Workspace(const CTransactionRef& ptx) : m_ptx(ptx), m_hash(ptx->GetHash()) {} /** Txids of mempool transactions that this transaction directly conflicts with. */ - std::set<uint256> m_conflicts; + std::set<Txid> m_conflicts; /** Iterators to mempool entries that this transaction directly conflicts with. */ CTxMemPool::setEntries m_iters_conflicting; /** Iterators to all mempool entries that would be replaced by this transaction, including @@ -761,9 +764,12 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // check all unconfirmed ancestors; otherwise an opt-in ancestor // might be replaced, causing removal of this descendant. // - // If replaceability signaling is ignored due to node setting, - // replacement is always allowed. - if (!m_pool.m_full_rbf && !SignalsOptInRBF(*ptxConflicting)) { + // All V3 transactions are considered replaceable. + // + // Replaceability signaling of the original transactions may be + // ignored due to node setting. + const bool allow_rbf{m_pool.m_full_rbf || SignalsOptInRBF(*ptxConflicting) || ptxConflicting->nVersion == 3}; + if (!allow_rbf) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict"); } @@ -865,7 +871,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // while a tx could be package CPFP'd when entering the mempool, we do not have a DoS-resistant // method of ensuring the tx remains bumped. For example, the fee-bumping child could disappear // due to a replacement. - if (!bypass_limits && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) { + // The only exception is v3 transactions. + if (!bypass_limits && ws.m_ptx->nVersion != 3 && ws.m_modified_fees < m_pool.m_min_relay_feerate.GetFee(ws.m_vsize)) { // Even though this is a fee-related failure, this result is TX_MEMPOOL_POLICY, not // TX_RECONSIDERABLE, because it cannot be bypassed using package validation. return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "min relay fee not met", @@ -947,6 +954,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } ws.m_ancestors = *ancestors; + if (const auto err_string{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) { + return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", *err_string); + } // A transaction that spends outputs that would be replaced by it is invalid. Now // that we have the set of all ancestors we can detect this @@ -1307,6 +1317,15 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: m_viewmempool.PackageAddTransaction(ws.m_ptx); } + // At this point we have all in-mempool ancestors, and we know every transaction's vsize. + // Run the v3 checks on the package. + for (Workspace& ws : workspaces) { + if (auto err{PackageV3Checks(ws.m_ptx, ws.m_vsize, txns, ws.m_ancestors)}) { + package_state.Invalid(PackageValidationResult::PCKG_POLICY, "v3-violation", err.value()); + return PackageMempoolAcceptResult(package_state, {}); + } + } + // Transactions must meet two minimum feerates: the mempool minimum fee and min relay fee. // For transactions consisting of exactly one child and its parents, it suffices to use the // package feerate (total modified fees / total virtual size) to check this requirement. |