Merge #20611: Move TX_MAX_STANDARD_VERSION to policy

laanwj · laanwj · commit dff0f6f753ea · 2020-12-15T22:46:49.000+01:00
fade619 Move TX_MAX_STANDARD_VERSION to policy (MarcoFalke) Pull request description: `primitives` should only be used for the raw datastructures (parsing and format). It is not the right place to document relay policy. ACKs for top commit: laanwj: Code review ACK fade619 lontivero: Concept ACK fade619 Tree-SHA512: f809c4aecd14d7e9feaa7b50b9c0697232991eef36190cd960bcfb0ad6e20c71a4f6aab48c7747cf8a681eb14feda60c55b09a37f128673d519567224f29cd97
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
@@ -11,6 +11,7 @@
 #include <consensus/consensus.h>
 #include <core_io.h>
 #include <key_io.h>
+#include <policy/policy.h>
 #include <policy/rbf.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
@@ -196,8 +197,9 @@ static CAmount ExtractAndValidateValue(const std::string& strValue)
 static void MutateTxVersion(CMutableTransaction& tx, const std::string& cmdVal)
 {
     int64_t newVersion;
-    if (!ParseInt64(cmdVal, &newVersion) || newVersion < 1 || newVersion > CTransaction::MAX_STANDARD_VERSION)
+    if (!ParseInt64(cmdVal, &newVersion) || newVersion < 1 || newVersion > TX_MAX_STANDARD_VERSION) {
         throw std::runtime_error("Invalid TX version requested: '" + cmdVal + "'");
+    }
 
     tx.nVersion = (int) newVersion;
 }
diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp
@@ -75,7 +75,7 @@ bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType)
 
 bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason)
 {
-    if (tx.nVersion > CTransaction::MAX_STANDARD_VERSION || tx.nVersion < 1) {
+    if (tx.nVersion > TX_MAX_STANDARD_VERSION || tx.nVersion < 1) {
         reason = "version";
         return false;
     }
diff --git a/src/policy/policy.h b/src/policy/policy.h
@@ -90,25 +90,32 @@ CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee);
 bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee);
 
 bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType);
-    /**
-     * Check for standard transaction types
-     * @return True if all outputs (scriptPubKeys) use only standard transaction forms
-     */
+
+
+// Changing the default transaction version requires a two step process: first
+// adapting relay policy by bumping TX_MAX_STANDARD_VERSION, and then later
+// allowing the new transaction version in the wallet/RPC.
+static constexpr decltype(CTransaction::nVersion) TX_MAX_STANDARD_VERSION{2};
+
+/**
+* Check for standard transaction types
+* @return True if all outputs (scriptPubKeys) use only standard transaction forms
+*/
 bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
-    /**
-     * Check for standard transaction types
-     * @param[in] mapInputs       Map of previous transactions that have outputs we're spending
-     * @param[in] taproot_active  Whether or taproot consensus rules are active (used to decide whether spends of them are permitted)
-     * @return True if all inputs (scriptSigs) use only standard transaction forms
-     */
+/**
+* Check for standard transaction types
+* @param[in] mapInputs       Map of previous transactions that have outputs we're spending
+* @param[in] taproot_active  Whether or taproot consensus rules are active (used to decide whether spends of them are permitted)
+* @return True if all inputs (scriptSigs) use only standard transaction forms
+*/
 bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, bool taproot_active);
-    /**
-     * Check if the transaction is over standard P2WSH resources limit:
-     * 3600bytes witnessScript size, 80bytes per witness stack element, 100 witness stack elements
-     * These limits are adequate for multisignatures up to n-of-100 using OP_CHECKSIG, OP_ADD, and OP_EQUAL.
-     *
-     * Also enforce a maximum stack item size limit and no annexes for tapscript spends.
-     */
+/**
+* Check if the transaction is over standard P2WSH resources limit:
+* 3600bytes witnessScript size, 80bytes per witness stack element, 100 witness stack elements
+* These limits are adequate for multisignatures up to n-of-100 using OP_CHECKSIG, OP_ADD, and OP_EQUAL.
+*
+* Also enforce a maximum stack item size limit and no annexes for tapscript spends.
+*/
 bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs);
 
 /** Compute the virtual transaction size (weight reinterpreted as bytes). */
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
@@ -262,12 +262,6 @@ class CTransaction
     // Default transaction version.
     static const int32_t CURRENT_VERSION=2;
 
-    // Changing the default transaction version requires a two step process: first
-    // adapting relay policy by bumping MAX_STANDARD_VERSION, and then later date
-    // bumping the default CURRENT_VERSION at which point both CURRENT_VERSION and
-    // MAX_STANDARD_VERSION will be equal.
-    static const int32_t MAX_STANDARD_VERSION=2;
-
     // The local variables are made const to prevent unintended modification
     // without updating the cached hash value. However, CTransaction is not
     // actually immutable; deserialization and assignment are implemented,

Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType)`
`75`	`75`
`76`	`76`	`bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason)`
`77`	`77`	`{`
`78`		`- if (tx.nVersion > CTransaction::MAX_STANDARD_VERSION \|\| tx.nVersion < 1) {`
	`78`	`+ if (tx.nVersion > TX_MAX_STANDARD_VERSION \|\| tx.nVersion < 1) {`
`79`	`79`	`reason = "version";`
`80`	`80`	`return false;`
`81`	`81`	`}`