Make 64 byte txs consensus invalid

The 64-byte transaction check is executed in a new ContextualCheckBlock,
so requires all CheckBlock errors to be treated as BLOCK_MUTATED.

Co-Authored-By: Matt Corallo <git@bluematt.me>

src/consensus/params.h

@@ -35,6 +35,7 @@ enum DeploymentPos : uint16_t {
     DEPLOYMENT_CHECKTEMPLATEVERIFY, // Deployment of CTV (BIP 119)
     DEPLOYMENT_ANYPREVOUT,
     DEPLOYMENT_OP_CAT,
+    DEPLOYMENT_64BYTETX,
     // NOTE: Also add new deployments to VersionBitsDeploymentInfo in deploymentinfo.cpp
     MAX_VERSION_BITS_DEPLOYMENTS
 };

src/deploymentinfo.cpp

@@ -27,6 +27,10 @@ const struct VBDeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_B
         /*.name =*/ "opcat",
         /*.gbt_force =*/ true,
     },
+    {
+        /*.name =*/ "64bytetx",
+        /*.gbt_force =*/ true,
+    },
 };
 
 std::string DeploymentName(Consensus::BuriedDeployment dep)

src/kernel/chainparams.cpp

@@ -584,6 +584,8 @@ class CRegTestParams : public CChainParams
         consensus.vDeployments[Consensus::DEPLOYMENT_CHECKTEMPLATEVERIFY] = SetupDeployment{.activate = 0x60007700, .abandon = 0x40007700, .always = true};
         consensus.vDeployments[Consensus::DEPLOYMENT_ANYPREVOUT] = SetupDeployment{.activate = 0x60007600, .abandon = 0x40007600, .always = true};
         consensus.vDeployments[Consensus::DEPLOYMENT_OP_CAT] = SetupDeployment{.activate = 0x62000100, .abandon = 0x42000100, .always = true};
+        consensus.vDeployments[Consensus::DEPLOYMENT_64BYTETX] = SetupDeployment{.activate = 0x60000001, .abandon = 0x40000001, .always = true}; // needs BIN assigned
+
         consensus.nMinimumChainWork = uint256{};
         consensus.defaultAssumeValid = uint256{};
 

src/rpc/blockchain.cpp

@@ -1396,6 +1396,7 @@ UniValue DeploymentInfo(const CBlockIndex* blockindex, const ChainstateManager&
     SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_CHECKTEMPLATEVERIFY);
     SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_ANYPREVOUT);
     SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_OP_CAT);
+    SoftForkDescPushBack(blockindex, softforks, chainman, Consensus::DEPLOYMENT_64BYTETX);
     return softforks;
 }
 } // anon namespace

src/validation.cpp

@@ -2401,7 +2401,6 @@ unsigned int GetBlockScriptFlags(const CBlockIndex& block_index, const Chainstat
     return flags;
 }
 
-
 /** Apply the effects of this block (with given index) on the UTXO set represented by coins.
  *  Validity checks that depend on the UTXO set are also done; ConnectBlock()
  *  can fail if those validity checks fail (among other reasons). */
@@ -4196,6 +4195,7 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio
     return true;
 }
 
+
 /** NOTE: This function is not currently invoked by ConnectBlock(), so we
  *  should consider upgrade issues if we change which consensus rules are
  *  enforced in this function (eg by adding a new consensus rule). See comment
@@ -4204,6 +4204,21 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio
  */
 static bool ContextualCheckBlock(const CBlock& block, BlockValidationState& state, const ChainstateManager& chainman, const CBlockIndex* pindexPrev)
 {
+
+    /**
+     * Failure of the 64-byte transaction check could indicate a mutated block,
+     * so all failed checks prior to this (see CheckBlock()) must also
+     * be treated as if they had returned BLOCK_MUTATED.
+     */
+    if (DeploymentActiveAfter(pindexPrev, chainman,
+    Consensus::DEPLOYMENT_64BYTETX)) {
+        for (const auto& tx : block.vtx) {
+            if (::GetSerializeSize(TX_NO_WITNESS(tx)) == 64) {
+                return state.Invalid(BlockValidationResult::BLOCK_MUTATED, "64-byte-transaction", strprintf("size of tx %s without witness is 64 bytes", tx->GetHash().ToString()));
+            }
+        }
+    }
+
     const int nHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1;
 
     // Enforce BIP113 (Median Time Past).

test/functional/data/invalid_txs.py

@@ -76,6 +76,9 @@ class BadTxTemplate:
     # the mempool (i.e. does it violate policy but not consensus)?
     valid_in_block = False
 
+    # Do we need a signature for this tx
+    wants_signature = True
+
     def __init__(self, *, spend_tx=None, spend_block=None):
         self.spend_tx = spend_block.vtx[0] if spend_block else spend_tx
         self.spend_avail = sum(o.nValue for o in self.spend_tx.vout)
@@ -101,6 +104,7 @@ def get_tx(self):
 class InputMissing(BadTxTemplate):
     reject_reason = "bad-txns-vin-empty"
     expect_disconnect = True
+    wants_signature = False
 
     # We use a blank transaction here to make sure
     # it is interpreted as a non-witness transaction.
@@ -118,7 +122,9 @@ def get_tx(self):
 class SizeExactly64(BadTxTemplate):
     reject_reason = "tx-size-small"
     expect_disconnect = False
-    valid_in_block = True
+    valid_in_block = False
+    wants_signature = False
+    block_reject_reason = "64-byte-transaction"
 
     def get_tx(self):
         tx = CTransaction()
@@ -133,6 +139,7 @@ class SizeSub64(BadTxTemplate):
     reject_reason = "tx-size-small"
     expect_disconnect = False
     valid_in_block = True
+    wants_signature = False
 
     def get_tx(self):
         tx = CTransaction()

test/functional/feature_block.py

@@ -159,7 +159,7 @@ def run_test(self):
             blockname = f"for_invalid.{TxTemplate.__name__}"
             self.next_block(blockname)
             badtx = template.get_tx()
-            if TxTemplate != invalid_txs.InputMissing:
+            if template.wants_signature:
                 self.sign_tx(badtx, attempt_spend_tx)
             badtx.rehash()
             badblock = self.update_block(blockname, [badtx])

test/functional/rpc_blockchain.py

@@ -261,6 +261,20 @@ def check_signalling_deploymentinfo_result(self, gdi_result, height, blockhash):
                 'height': 0,
                 'active': True,
             },
+            '64bytetx': {
+                'type': 'heretical',
+                'heretical': {
+                    'binana-id': 'BIN-2016-0000-001',
+                    'start_time': -1,
+                    'timeout': 0x7fffffffffffffff,  # testdummy does not have a timeout so is set to the max int64 value
+                    'period': 144,
+                    'status': 'active',
+                    'status_next': 'active',
+                    'since': 0,
+                },
+                'active': True,
+                'height': 0,
+            },
           }
         })