Merge #16363: test: Add test for BIP30 duplicate tx

fa8489a155 test: Add test for BIP30 duplicate tx (MarcoFalke)
77770d95e2 test: Properly serialize BIP34 coinbase height (MarcoFalke)

Pull request description:

  This adds a test for BIP30 to check that duplicate txs can exist in the blockchain given the first one was completely spent when the second one is added. (Requested by ajtowns in https://github.com/bitcoin/bitcoin/pull/16333#issuecomment-508604071)

  We can not add a test that a later duplicate tx overwrites a previous one, because BIP30 is always enforced on regtest. If someone feels strongly about such a test, some Bitcoin Core code would have to be modified, which can be done in a follow up pull request.

  Also, add a commit to fix the BIP34 test failures reported in https://github.com/bitcoin/bitcoin/pull/14633#issue-227712540

ACKs for top commit:
  laanwj:
    Code review ACK fa8489a155

Tree-SHA512: c707d0bdc93937263876b603425b53322a2a9f9ec3f50716ae2fa9de8ddc644beb22b26c1bfde7f4aab102633e096b354ef380db919176bd2cb44a2828f884aa

test/functional/test_framework/blocktools.py

@@ -22,13 +22,14 @@ from .messages import (
     ToHex,
     hash256,
     hex_str_to_bytes,
-    ser_string,
     ser_uint256,
     sha256,
     uint256_from_str,
 )
 from .script import (
     CScript,
+    CScriptNum,
+    CScriptOp,
     OP_0,
     OP_1,
     OP_CHECKMULTISIG,
@@ -89,20 +90,14 @@ def add_witness_commitment(block, nonce=0):
     block.hashMerkleRoot = block.calc_merkle_root()
     block.rehash()
 
-def serialize_script_num(value):
-    r = bytearray(0)
-    if value == 0:
-        return r
-    neg = value < 0
-    absvalue = -value if neg else value
-    while (absvalue):
-        r.append(int(absvalue & 0xff))
-        absvalue >>= 8
-    if r[-1] & 0x80:
-        r.append(0x80 if neg else 0)
-    elif neg:
-        r[-1] |= 0x80
-    return r
+
+def script_BIP34_coinbase_height(height):
+    if height <= 16:
+        res = CScriptOp.encode_op_n(height)
+        # Append dummy to increase scriptSig size above 2 (see bad-cb-length consensus rule)
+        return CScript([res, OP_1])
+    return CScript([CScriptNum(height)])
+
 
 def create_coinbase(height, pubkey=None):
     """Create a coinbase transaction, assuming no miner fees.
@@ -110,8 +105,7 @@ def create_coinbase(height, pubkey=None):
     If pubkey is passed in, the coinbase output will be a P2PK output;
     otherwise an anyone-can-spend output."""
     coinbase = CTransaction()
-    coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff),
-                        ser_string(serialize_script_num(height)), 0xffffffff))
+    coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), script_BIP34_coinbase_height(height), 0xffffffff))
     coinbaseoutput = CTxOut()
     coinbaseoutput.nValue = 50 * COIN
     halvings = int(height / 150)  # regtest