Merge #11739: Enforce SCRIPT_VERIFY_P2SH and SCRIPT_VERIFY_WITNESS from genesis

8b56fc0b91 [qa] Test that v0 segwit outputs can't be spent pre-activation (Suhas Daftuar)
ccb8ca42a4 Always enforce SCRIPT_VERIFY_WITNESS with P2SH (Suhas Daftuar)
5c31b20a35 [qa] Remove some pre-activation segwit tests (Suhas Daftuar)
95749a5836 Separate NULLDUMMY enforcement from SEGWIT enforcement (Suhas Daftuar)
ce650182f4 Use P2SH consensus rules for all blocks (Suhas Daftuar)

Pull request description:

  As discussed at the IRC meeting back in October (https://botbot.me/freenode/bitcoin-core-dev/2017-10-12/?msg=92231929&page=2), I had looked into the feasibility of enforcing P2SH and SCRIPT_VERIFY_WITNESS back to the genesis block.

  The P2SH change is pretty straightforward -- there was only one historical block on mainnet that violated the rule, so I carved out an exception to it, similar to the way we have exceptions for the BIP30 violators.

  The segwit change is not entirely as clear.  The code changes themselves are relatively straightforward: we can just always turn on SCRIPT_VERIFY_WITNESS whenever P2SH is active.  However conceptually, this amounts to splitting up BIP141 into two parts, the part that implements new script rules, and the part that handles witness commitments in blocks.

  Arguably though the script rules are really defined in BIP 143 anyway, and so this really amounts to backdating BIP 143 -- script rules for v0 segwit outputs -- back to genesis.  So maybe conceptually this isn't so bad...

  I don't feel strongly about this change in either direction; I started working on it because I was searching for a way to simplify the way we understand and implement the consensus rules around segwit, but I'm not yet sure whether I think this achieves anything toward that goal.

  ping @TheBlueMatt

Tree-SHA512: 73551d4a983eb9792c7ac67f56005822528ac4d1fd52c27cee6d305ebee953f69687ef4ddee8bdc0fec77f77e6b5a9d669750793efee54c076533a095e233042

2 files changed, 85 insertions, 14 deletions

diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py
index fa1732c4c5..e835b9d777 100755
--- a/test/functional/feature_segwit.py
+++ b/test/functional/feature_segwit.py
@@ -150,19 +150,11 @@ class SegWitTest(BitcoinTestFramework):
         self.skip_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V0][0], True) #block 426
         self.skip_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V1][0], True) #block 427
 
-        # TODO: An old node would see these txs without witnesses and be able to mine them
-
-        self.log.info("Verify unsigned bare witness txs in versionbits-setting blocks are valid before the fork")
-        self.success_mine(self.nodes[2], wit_ids[NODE_2][WIT_V0][1], False) #block 428
-        self.success_mine(self.nodes[2], wit_ids[NODE_2][WIT_V1][1], False) #block 429
-
         self.log.info("Verify unsigned p2sh witness txs without a redeem script are invalid")
         self.fail_accept(self.nodes[2], "mandatory-script-verify-flag", p2sh_ids[NODE_2][WIT_V0][1], False)
         self.fail_accept(self.nodes[2], "mandatory-script-verify-flag", p2sh_ids[NODE_2][WIT_V1][1], False)
 
-        self.log.info("Verify unsigned p2sh witness txs with a redeem script in versionbits-settings blocks are valid before the fork")
-        self.success_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V0][1], False, witness_script(False, self.pubkey[2])) #block 430
-        self.success_mine(self.nodes[2], p2sh_ids[NODE_2][WIT_V1][1], False, witness_script(True, self.pubkey[2])) #block 431
+        self.nodes[2].generate(4) # blocks 428-431
 
         self.log.info("Verify previous witness txs skipped for mining can now be mined")
         assert_equal(len(self.nodes[2].getrawmempool()), 4)
diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index f8afe22eaf..4fecd4ffee 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -48,7 +48,7 @@ def test_transaction_acceptance(rpc, p2p, tx, with_witness, accepted, reason=Non
         with mininode_lock:
             assert_equal(p2p.last_message["reject"].reason, reason)
 
-def test_witness_block(rpc, p2p, block, accepted, with_witness=True):
+def test_witness_block(rpc, p2p, block, accepted, with_witness=True, reason=None):
     """Send a block to the node and check that it's accepted
 
     - Submit the block over the p2p interface
@@ -59,6 +59,10 @@ def test_witness_block(rpc, p2p, block, accepted, with_witness=True):
         p2p.send_message(msg_block(block))
     p2p.sync_with_ping()
     assert_equal(rpc.getbestblockhash() == block.hash, accepted)
+    if (reason != None and not accepted):
+        # Check the rejection reason as well.
+        with mininode_lock:
+            assert_equal(p2p.last_message["reject"].reason, reason)
 
 class TestP2PConn(P2PInterface):
     def __init__(self):
@@ -272,6 +276,80 @@ class SegWitTest(BitcoinTestFramework):
         self.utxo.pop(0)
         self.utxo.append(UTXO(tx4.sha256, 0, tx4.vout[0].nValue))
 
+    # ~6 months after segwit activation, the SCRIPT_VERIFY_WITNESS flag was
+    # backdated so that it applies to all blocks, going back to the genesis
+    # block.
+    #
+    # Consequently, version 0 witness outputs are never spendable without
+    # witness, and so can't be spent before segwit activation (the point at which
+    # blocks are permitted to contain witnesses).
+    def test_v0_outputs_arent_spendable(self):
+        self.log.info("Testing that v0 witness program outputs aren't spendable before activation")
+
+        assert len(self.utxo), "self.utxo is empty"
+
+        # Create two outputs, a p2wsh and p2sh-p2wsh
+        witness_program = CScript([OP_TRUE])
+        witness_hash = sha256(witness_program)
+        scriptPubKey = CScript([OP_0, witness_hash])
+
+        p2sh_pubkey = hash160(scriptPubKey)
+        p2sh_scriptPubKey = CScript([OP_HASH160, p2sh_pubkey, OP_EQUAL])
+
+        value = self.utxo[0].nValue // 3
+
+        tx = CTransaction()
+        tx.vin = [CTxIn(COutPoint(self.utxo[0].sha256, self.utxo[0].n), b'')]
+        tx.vout = [CTxOut(value, scriptPubKey), CTxOut(value, p2sh_scriptPubKey)]
+        tx.vout.append(CTxOut(value, CScript([OP_TRUE])))
+        tx.rehash()
+        txid = tx.sha256
+
+        # Add it to a block
+        block = self.build_next_block()
+        self.update_witness_block_with_transactions(block, [tx])
+        # Verify that segwit isn't activated. A block serialized with witness
+        # should be rejected prior to activation.
+        test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=True, reason = b'unexpected-witness')
+        # Now send the block without witness. It should be accepted
+        test_witness_block(self.nodes[0], self.test_node, block, accepted=True, with_witness=False)
+
+        # Now try to spend the outputs. This should fail since SCRIPT_VERIFY_WITNESS is always enabled.
+        p2wsh_tx = CTransaction()
+        p2wsh_tx.vin = [CTxIn(COutPoint(txid, 0), b'')]
+        p2wsh_tx.vout = [CTxOut(value, CScript([OP_TRUE]))]
+        p2wsh_tx.wit.vtxinwit.append(CTxInWitness())
+        p2wsh_tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])]
+        p2wsh_tx.rehash()
+
+        p2sh_p2wsh_tx = CTransaction()
+        p2sh_p2wsh_tx.vin = [CTxIn(COutPoint(txid, 1), CScript([scriptPubKey]))]
+        p2sh_p2wsh_tx.vout = [CTxOut(value, CScript([OP_TRUE]))]
+        p2sh_p2wsh_tx.wit.vtxinwit.append(CTxInWitness())
+        p2sh_p2wsh_tx.wit.vtxinwit[0].scriptWitness.stack = [CScript([OP_TRUE])]
+        p2sh_p2wsh_tx.rehash()
+
+        for tx in [p2wsh_tx, p2sh_p2wsh_tx]:
+
+            block = self.build_next_block()
+            self.update_witness_block_with_transactions(block, [tx])
+
+            # When the block is serialized with a witness, the block will be rejected because witness
+            # data isn't allowed in blocks that don't commit to witness data.
+            test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=True, reason=b'unexpected-witness')
+
+            # When the block is serialized without witness, validation fails because the transaction is
+            # invalid (transactions are always validated with SCRIPT_VERIFY_WITNESS so a segwit v0 transaction
+            # without a witness is invalid).
+            # Note: The reject reason for this failure could be
+            # 'block-validation-failed' (if script check threads > 1) or
+            # 'non-mandatory-script-verify-flag (Witness program was passed an
+            # empty witness)' (otherwise).
+            # TODO: support multiple acceptable reject reasons.
+            test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=False)
+
+        self.utxo.pop(0)
+        self.utxo.append(UTXO(txid, 2, value))
 
     # Mine enough blocks for segwit's vb state to be 'started'.
     def advance_to_segwit_started(self):
@@ -1479,9 +1557,10 @@ class SegWitTest(BitcoinTestFramework):
         block = self.build_next_block()
         self.update_witness_block_with_transactions(block, [spend_tx])
 
-        # If we're before activation, then sending this without witnesses
-        # should be valid.  If we're after activation, then sending this with
-        # witnesses should be valid.
+        # If we're after activation, then sending this with witnesses should be valid.
+        # This no longer works before activation, because SCRIPT_VERIFY_WITNESS
+        # is always set.
+        # TODO: rewrite this test to make clear that it only works after activation.
         if segwit_activated:
             test_witness_block(self.nodes[0].rpc, self.test_node, block, accepted=True)
         else:
@@ -1900,6 +1979,7 @@ class SegWitTest(BitcoinTestFramework):
         self.test_witness_services() # Verifies NODE_WITNESS
         self.test_non_witness_transaction() # non-witness tx's are accepted
         self.test_unnecessary_witness_before_segwit_activation()
+        self.test_v0_outputs_arent_spendable()
         self.test_block_relay(segwit_activated=False)
 
         # Advance to segwit being 'started'
@@ -1917,7 +1997,6 @@ class SegWitTest(BitcoinTestFramework):
         self.test_unnecessary_witness_before_segwit_activation()
         self.test_witness_tx_relay_before_segwit_activation()
         self.test_block_relay(segwit_activated=False)
-        self.test_p2sh_witness(segwit_activated=False)
         self.test_standardness_v0(segwit_activated=False)
 
         sync_blocks(self.nodes)