Merge #15893: Add test for superfluous witness record in deserialization

cc556e4a30 Add test for superfluous witness record in deserialization (Gregory Sanders)
25b0786581 Fix missing input template by making minimal tx (Gregory Sanders)

Pull request description:

  Adds coverage for changed behavior in https://github.com/bitcoin/bitcoin/pull/14039

ACKs for commit cc556e:
  MarcoFalke:
    utACK cc556e4a30

Tree-SHA512: 3404c8f75e87503983fac5ae27d877309eb3b902f2ec993762911c71610ca449bef0ed98bd17e029414828025b2713e1bd012e63b2a06497e34f1056acaa6321

test/functional/data/invalid_txs.py

@@ -71,9 +71,13 @@ class InputMissing(BadTxTemplate):
     reject_reason = "bad-txns-vin-empty"
     expect_disconnect = False
 
+    # We use a blank transaction here to make sure
+    # it is interpreted as a non-witness transaction.
+    # Otherwise the transaction will fail the
+    # "surpufluous witness" check during deserialization
+    # rather than the input count check.
     def get_tx(self):
         tx = CTransaction()
-        tx.vout.append(CTxOut(0, sc.CScript([sc.OP_TRUE] * 100)))
         tx.calc_sha256()
         return tx
 

test/functional/feature_block.py

@@ -146,20 +146,6 @@ class FullBlockTest(BitcoinTestFramework):
             badtx = template.get_tx()
             if TxTemplate != invalid_txs.InputMissing:
                 self.sign_tx(badtx, attempt_spend_tx)
-            else:
-                # Segwit is active in regtest at this point, so to deserialize a
-                # transaction without any inputs correctly, we set the outputs
-                # to an empty list. This is a hack, as the serialization of an
-                # empty list of outputs is deserialized as flags==0 and thus
-                # deserialization of the outputs is skipped.
-                # A policy check requires "loose" txs to be of a minimum size,
-                # so vtx is not set to be empty in the TxTemplate class and we
-                # only apply the workaround where txs are not "loose", i.e. in
-                # blocks.
-                #
-                # The workaround has the purpose that both sides calculate
-                # the same tx hash in the merkle tree
-                badtx.vout = []
             badtx.rehash()
             badblock = self.update_block(blockname, [badtx])
             self.send_blocks(

test/functional/p2p_segwit.py

@@ -36,6 +36,7 @@ from test_framework.messages import (
     ser_vector,
     sha256,
     uint256_from_str,
+    FromHex,
 )
 from test_framework.mininode import (
     P2PInterface,
@@ -77,6 +78,7 @@ from test_framework.util import (
     disconnect_nodes,
     get_bip9_status,
     hex_str_to_bytes,
+    assert_raises_rpc_error,
 )
 
 # The versionbit bit used to signal activation of SegWit
@@ -269,6 +271,7 @@ class SegWitTest(BitcoinTestFramework):
         self.test_non_standard_witness()
         self.test_upgrade_after_activation()
         self.test_witness_sigops()
+        self.test_superfluous_witness()
 
     # Individual tests
 
@@ -2034,5 +2037,31 @@ class SegWitTest(BitcoinTestFramework):
 
         # TODO: test p2sh sigop counting
 
+    def test_superfluous_witness(self):
+        # Serialization of tx that puts witness flag to 1 always
+        def serialize_with_bogus_witness(tx):
+            flags = 1
+            r = b""
+            r += struct.pack("<i", tx.nVersion)
+            if flags:
+                dummy = []
+                r += ser_vector(dummy)
+                r += struct.pack("<B", flags)
+            r += ser_vector(tx.vin)
+            r += ser_vector(tx.vout)
+            if flags & 1:
+                if (len(tx.wit.vtxinwit) != len(tx.vin)):
+                    # vtxinwit must have the same length as vin
+                    tx.wit.vtxinwit = tx.wit.vtxinwit[:len(tx.vin)]
+                    for i in range(len(tx.wit.vtxinwit), len(tx.vin)):
+                        tx.wit.vtxinwit.append(CTxInWitness())
+                r += tx.wit.serialize()
+            r += struct.pack("<I", tx.nLockTime)
+            return r
+
+        raw = self.nodes[0].createrawtransaction([{"txid":"00"*32, "vout":0}], {self.nodes[0].getnewaddress():1})
+        tx = FromHex(CTransaction(), raw)
+        assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].decoderawtransaction, serialize_with_bogus_witness(tx).hex())
+
 if __name__ == '__main__':
     SegWitTest().main()