Merge #13120: policy: Treat segwit as always active

fa7a6cf1b36284db70e941bd2915fd6edbb0f9d6 policy: Treat segwit as always active (MarcoFalke)

Pull request description:

  Now that segwit is active for a long time, there is no need to reject transactions with the reason that segwit hasn't activated.

  Strictly speaking, this is a bug fix, because with the release of 0.16, we create segwit transactions in our wallet by default without checking if they are allowed by local policy.

  More broadly, this simplifies the code as if "premature witness" was always set to true with the corresponding command line args.

Tree-SHA512: 484c26aa3a66faba6b41e8554a91a29bfc15fbf6caae3d5363a3966283143189c4bd5333a610b0669c1238f75620691264e73f6b9f1161cdacf7574d946436da

4 files changed, 100 insertions, 95 deletions

diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py
index 7db6a03b45..6577d83f5c 100755
--- a/test/functional/feature_nulldummy.py
+++ b/test/functional/feature_nulldummy.py
@@ -42,7 +42,7 @@ class NULLDUMMYTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         # This script tests NULLDUMMY activation, which is part of the 'segwit' deployment, so we go through
         # normal segwit activation here (and don't use the default always-on behaviour).
-        self.extra_args = [['-whitelist=127.0.0.1', '-walletprematurewitness', '-vbparams=segwit:0:999999999999', '-addresstype=legacy', "-deprecatedrpc=addwitnessaddress"]]
+        self.extra_args = [['-whitelist=127.0.0.1', '-vbparams=segwit:0:999999999999', '-addresstype=legacy', "-deprecatedrpc=addwitnessaddress"]]
 
     def run_test(self):
         self.address = self.nodes[0].getnewaddress()
diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py
index 3622e1834a..b10306b283 100755
--- a/test/functional/feature_segwit.py
+++ b/test/functional/feature_segwit.py
@@ -42,9 +42,9 @@ class SegWitTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.num_nodes = 3
         # This test tests SegWit both pre and post-activation, so use the normal BIP9 activation.
-        self.extra_args = [["-walletprematurewitness", "-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"],
-                           ["-blockversion=4", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"],
-                           ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-prematurewitness", "-walletprematurewitness", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]]
+        self.extra_args = [["-rpcserialversion=0", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"],
+                           ["-blockversion=4", "-promiscuousmempoolflags=517", "-rpcserialversion=1", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"],
+                           ["-blockversion=536870915", "-promiscuousmempoolflags=517", "-vbparams=segwit:0:999999999999", "-addresstype=legacy", "-deprecatedrpc=addwitnessaddress"]]
 
     def setup_network(self):
         super().setup_network()
@@ -129,21 +129,6 @@ class SegWitTest(BitcoinTestFramework):
         self.nodes[0].generate(260) #block 423
         sync_blocks(self.nodes)
 
-        self.log.info("Verify default node can't accept any witness format txs before fork")
-        # unsigned, no scriptsig
-        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", wit_ids[NODE_0][WIT_V0][0], False)
-        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", wit_ids[NODE_0][WIT_V1][0], False)
-        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V0][0], False)
-        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V1][0], False)
-        # unsigned with redeem script
-        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V0][0], False, witness_script(False, self.pubkey[0]))
-        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V1][0], False, witness_script(True, self.pubkey[0]))
-        # signed
-        self.fail_accept(self.nodes[0], "no-witness-yet", wit_ids[NODE_0][WIT_V0][0], True)
-        self.fail_accept(self.nodes[0], "no-witness-yet", wit_ids[NODE_0][WIT_V1][0], True)
-        self.fail_accept(self.nodes[0], "no-witness-yet", p2sh_ids[NODE_0][WIT_V0][0], True)
-        self.fail_accept(self.nodes[0], "no-witness-yet", p2sh_ids[NODE_0][WIT_V1][0], True)
-
         self.log.info("Verify witness txs are skipped for mining before the fork")
         self.skip_mine(self.nodes[2], wit_ids[NODE_2][WIT_V0][0], True) #block 424
         self.skip_mine(self.nodes[2], wit_ids[NODE_2][WIT_V1][0], True) #block 425
@@ -164,6 +149,16 @@ class SegWitTest(BitcoinTestFramework):
         segwit_tx_list = self.nodes[2].getblock(block[0])["tx"]
         assert_equal(len(segwit_tx_list), 5)
 
+        self.log.info("Verify default node can't accept txs with missing witness")
+        # unsigned, no scriptsig
+        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", wit_ids[NODE_0][WIT_V0][0], False)
+        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", wit_ids[NODE_0][WIT_V1][0], False)
+        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V0][0], False)
+        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V1][0], False)
+        # unsigned with redeem script
+        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V0][0], False, witness_script(False, self.pubkey[0]))
+        self.fail_accept(self.nodes[0], "mandatory-script-verify-flag", p2sh_ids[NODE_0][WIT_V1][0], False, witness_script(True, self.pubkey[0]))
+
         self.log.info("Verify block and transaction serialization rpcs return differing serializations depending on rpc serialization flag")
         assert(self.nodes[2].getblock(block[0], False) !=  self.nodes[0].getblock(block[0], False))
         assert(self.nodes[1].getblock(block[0], False) ==  self.nodes[2].getblock(block[0], False))
diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index e80a764477..940d085e89 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -230,55 +230,9 @@ class SegWitTest(BitcoinTestFramework):
 
         sync_blocks(self.nodes)
 
-        # Create a p2sh output -- this is so we can pass the standardness
-        # rules (an anyone-can-spend OP_TRUE would be rejected, if not wrapped
-        # in P2SH).
-        p2sh_program = CScript([OP_TRUE])
-        p2sh_pubkey = hash160(p2sh_program)
-        scriptPubKey = CScript([OP_HASH160, p2sh_pubkey, OP_EQUAL])
-
-        # Now check that unnecessary witnesses can't be used to blind a node
-        # to a transaction, eg by violating standardness checks.
-        tx2 = CTransaction()
-        tx2.vin.append(CTxIn(COutPoint(tx.sha256, 0), b""))
-        tx2.vout.append(CTxOut(tx.vout[0].nValue-1000, scriptPubKey))
-        tx2.rehash()
-        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx2, False, True)
-        self.nodes[0].generate(1)
-        sync_blocks(self.nodes)
-
-        # We'll add an unnecessary witness to this transaction that would cause
-        # it to be non-standard, to test that violating policy with a witness before
-        # segwit activation doesn't blind a node to a transaction.  Transactions
-        # rejected for having a witness before segwit activation shouldn't be added
-        # to the rejection cache.
-        tx3 = CTransaction()
-        tx3.vin.append(CTxIn(COutPoint(tx2.sha256, 0), CScript([p2sh_program])))
-        tx3.vout.append(CTxOut(tx2.vout[0].nValue-1000, scriptPubKey))
-        tx3.wit.vtxinwit.append(CTxInWitness())
-        tx3.wit.vtxinwit[0].scriptWitness.stack = [b'a'*400000]
-        tx3.rehash()
-        # Note that this should be rejected for the premature witness reason,
-        # rather than a policy check, since segwit hasn't activated yet.
-        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx3, True, False, b'no-witness-yet')
-
-        # If we send without witness, it should be accepted.
-        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx3, False, True)
-
-        # Now create a new anyone-can-spend utxo for the next test.
-        tx4 = CTransaction()
-        tx4.vin.append(CTxIn(COutPoint(tx3.sha256, 0), CScript([p2sh_program])))
-        tx4.vout.append(CTxOut(tx3.vout[0].nValue - 1000, CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])))
-        tx4.rehash()
-        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx3, False, True)
-        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx4, False, True)
-
-        self.nodes[0].generate(1)
-        sync_blocks(self.nodes)
-
         # Update our utxo list; we spent the first entry.
         self.utxo.pop(0)
-        self.utxo.append(UTXO(tx4.sha256, 0, tx4.vout[0].nValue))
+        self.utxo.append(UTXO(tx.sha256, 0, tx.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
@@ -1119,7 +1073,7 @@ class SegWitTest(BitcoinTestFramework):
             self.old_node.announce_tx_and_wait_for_getdata(block4.vtx[0])
             assert(block4.sha256 not in self.old_node.getdataset)
 
-    # V0 segwit outputs should be standard after activation, but not before.
+    # V0 segwit outputs and inputs are always standard. V0 segwit inputs may only be mined after activation, but not before.
     def test_standardness_v0(self, segwit_activated):
         self.log.info("Testing standardness of v0 outputs (%s activation)" % ("after" if segwit_activated else "before"))
         assert(len(self.utxo))
@@ -1148,45 +1102,46 @@ class SegWitTest(BitcoinTestFramework):
         tx.vin = [CTxIn(COutPoint(p2sh_tx.sha256, 0), CScript([witness_program]))]
         tx.vout = [CTxOut(p2sh_tx.vout[0].nValue-10000, scriptPubKey)]
         tx.vout.append(CTxOut(8000, scriptPubKey)) # Might burn this later
+        tx.vin[0].nSequence = BIP125_SEQUENCE_NUMBER  # Just to have the option to bump this tx from the mempool
         tx.rehash()
 
-        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx, with_witness=True, accepted=segwit_activated)
+        # This is always accepted, since the mempool policy is to consider segwit as always active
+        # and thus allow segwit outputs
+        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx, with_witness=True, accepted=True)
 
         # Now create something that looks like a P2PKH output. This won't be spendable.
         scriptPubKey = CScript([OP_0, hash160(witness_hash)])
         tx2 = CTransaction()
-        if segwit_activated:
-            # if tx was accepted, then we spend the second output.
-            tx2.vin = [CTxIn(COutPoint(tx.sha256, 1), b"")]
-            tx2.vout = [CTxOut(7000, scriptPubKey)]
-            tx2.wit.vtxinwit.append(CTxInWitness())
-            tx2.wit.vtxinwit[0].scriptWitness.stack = [witness_program]
-        else:
-            # if tx wasn't accepted, we just re-spend the p2sh output we started with.
-            tx2.vin = [CTxIn(COutPoint(p2sh_tx.sha256, 0), CScript([witness_program]))]
-            tx2.vout = [CTxOut(p2sh_tx.vout[0].nValue-1000, scriptPubKey)]
+        # tx was accepted, so we spend the second output.
+        tx2.vin = [CTxIn(COutPoint(tx.sha256, 1), b"")]
+        tx2.vout = [CTxOut(7000, scriptPubKey)]
+        tx2.wit.vtxinwit.append(CTxInWitness())
+        tx2.wit.vtxinwit[0].scriptWitness.stack = [witness_program]
         tx2.rehash()
 
-        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx2, with_witness=True, accepted=segwit_activated)
+        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx2, with_witness=True, accepted=True)
 
         # Now update self.utxo for later tests.
         tx3 = CTransaction()
-        if segwit_activated:
-            # tx and tx2 were both accepted.  Don't bother trying to reclaim the
-            # P2PKH output; just send tx's first output back to an anyone-can-spend.
-            sync_mempools([self.nodes[0], self.nodes[1]])
-            tx3.vin = [CTxIn(COutPoint(tx.sha256, 0), b"")]
-            tx3.vout = [CTxOut(tx.vout[0].nValue - 1000, CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]))]
-            tx3.wit.vtxinwit.append(CTxInWitness())
-            tx3.wit.vtxinwit[0].scriptWitness.stack = [witness_program]
-            tx3.rehash()
-            test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx3, with_witness=True, accepted=True)
-        else:
-            # tx and tx2 didn't go anywhere; just clean up the p2sh_tx output.
-            tx3.vin = [CTxIn(COutPoint(p2sh_tx.sha256, 0), CScript([witness_program]))]
-            tx3.vout = [CTxOut(p2sh_tx.vout[0].nValue - 1000, CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]))]
+        # tx and tx2 were both accepted.  Don't bother trying to reclaim the
+        # P2PKH output; just send tx's first output back to an anyone-can-spend.
+        sync_mempools([self.nodes[0], self.nodes[1]])
+        tx3.vin = [CTxIn(COutPoint(tx.sha256, 0), b"")]
+        tx3.vout = [CTxOut(tx.vout[0].nValue - 1000, CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]))]
+        tx3.wit.vtxinwit.append(CTxInWitness())
+        tx3.wit.vtxinwit[0].scriptWitness.stack = [witness_program]
+        tx3.rehash()
+        if not segwit_activated:
+            # Just check mempool acceptance, but don't add the transaction to the mempool, since witness is disallowed
+            # in blocks and the tx is impossible to mine right now.
+            assert_equal(self.nodes[0].testmempoolaccept([bytes_to_hex_str(tx3.serialize_with_witness())]), [{'txid': tx3.hash, 'allowed': True}])
+            # Create the same output as tx3, but by replacing tx
+            tx3_out = tx3.vout[0]
+            tx3 = tx
+            tx3.vout = [tx3_out]
             tx3.rehash()
-            test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx3, with_witness=True, accepted=True)
+            assert_equal(self.nodes[0].testmempoolaccept([bytes_to_hex_str(tx3.serialize_with_witness())]), [{'txid': tx3.hash, 'allowed': True}])
+        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx3, with_witness=True, accepted=True)
 
         self.nodes[0].generate(1)
         sync_blocks(self.nodes)
@@ -1855,6 +1810,60 @@ class SegWitTest(BitcoinTestFramework):
         test_witness_block(self.nodes[0].rpc, self.test_node, block, accepted=True)
         self.utxo.append(UTXO(tx5.sha256, 0, tx5.vout[0].nValue))
 
+    def test_non_standard_witness_blinding(self):
+        self.log.info("Testing behavior of unnecessary witnesses in transactions does not blind the node for the transaction")
+        assert (len(self.utxo) > 0)
+
+        # Create a p2sh output -- this is so we can pass the standardness
+        # rules (an anyone-can-spend OP_TRUE would be rejected, if not wrapped
+        # in P2SH).
+        p2sh_program = CScript([OP_TRUE])
+        p2sh_pubkey = hash160(p2sh_program)
+        scriptPubKey = CScript([OP_HASH160, p2sh_pubkey, OP_EQUAL])
+
+        # Now check that unnecessary witnesses can't be used to blind a node
+        # to a transaction, eg by violating standardness checks.
+        tx = CTransaction()
+        tx.vin.append(CTxIn(COutPoint(self.utxo[0].sha256, self.utxo[0].n), b""))
+        tx.vout.append(CTxOut(self.utxo[0].nValue - 1000, scriptPubKey))
+        tx.rehash()
+        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx, False, True)
+        self.nodes[0].generate(1)
+        sync_blocks(self.nodes)
+
+        # We'll add an unnecessary witness to this transaction that would cause
+        # it to be non-standard, to test that violating policy with a witness
+        # doesn't blind a node to a transaction.  Transactions
+        # rejected for having a witness shouldn't be added
+        # to the rejection cache.
+        tx2 = CTransaction()
+        tx2.vin.append(CTxIn(COutPoint(tx.sha256, 0), CScript([p2sh_program])))
+        tx2.vout.append(CTxOut(tx.vout[0].nValue - 1000, scriptPubKey))
+        tx2.wit.vtxinwit.append(CTxInWitness())
+        tx2.wit.vtxinwit[0].scriptWitness.stack = [b'a' * 400]
+        tx2.rehash()
+        # This will be rejected due to a policy check:
+        # No witness is allowed, since it is not a witness program but a p2sh program
+        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx2, True, False, b'bad-witness-nonstandard')
+
+        # If we send without witness, it should be accepted.
+        test_transaction_acceptance(self.nodes[1].rpc, self.std_node, tx2, False, True)
+
+        # Now create a new anyone-can-spend utxo for the next test.
+        tx3 = CTransaction()
+        tx3.vin.append(CTxIn(COutPoint(tx2.sha256, 0), CScript([p2sh_program])))
+        tx3.vout.append(CTxOut(tx2.vout[0].nValue - 1000, CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])))
+        tx3.rehash()
+        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx2, False, True)
+        test_transaction_acceptance(self.nodes[0].rpc, self.test_node, tx3, False, True)
+
+        self.nodes[0].generate(1)
+        sync_blocks(self.nodes)
+
+        # Update our utxo list; we spent the first entry.
+        self.utxo.pop(0)
+        self.utxo.append(UTXO(tx3.sha256, 0, tx3.vout[0].nValue))
+
     def test_non_standard_witness(self):
         self.log.info("Testing detection of non-standard P2WSH witness")
         pad = chr(1).encode('latin-1')
@@ -2023,6 +2032,7 @@ class SegWitTest(BitcoinTestFramework):
         self.test_premature_coinbase_witness_spend()
         self.test_uncompressed_pubkey()
         self.test_signature_version_1()
+        self.test_non_standard_witness_blinding()
         self.test_non_standard_witness()
         sync_blocks(self.nodes)
         self.test_upgrade_after_activation(node_id=2)
diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py
index fcc11abce0..f07041706a 100755
--- a/test/functional/wallet_bumpfee.py
+++ b/test/functional/wallet_bumpfee.py
@@ -31,7 +31,7 @@ class BumpFeeTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.setup_clean_chain = True
-        self.extra_args = [["-prematurewitness", "-walletprematurewitness", "-deprecatedrpc=addwitnessaddress", "-walletrbf={}".format(i)]
+        self.extra_args = [["-deprecatedrpc=addwitnessaddress", "-walletrbf={}".format(i)]
                            for i in range(self.num_nodes)]
 
     def run_test(self):