Merge #14305: Tests: enforce critical class instance attributes in fu…

…nctional tests, fix segwit test specificity e460232 Document fixed attribute behavior in critical test framework classes. (Justin Turner Arthur) 17b42f4 Check for specific tx acceptance failures based on script signature (Justin Turner Arthur) 3a4449e Strictly enforce instance attrs in critical functional test classes. (Justin Turner Arthur) 1d0ce94 Fix for incorrect version attr set on functional test segwit block. (Justin Turner Arthur) ba923e3 test: Fix broken segwit test (practicalswift) Pull request description: No extra attributes will be able to be added to instances of the C++ class ports or of other critical classes without causing an exception. Helps prevent adding or depending on attributes that aren't in the intended object structure. It may prevent issues such as the one fixed in #14300. This request fixes the erroneous version attribute used in the p2p_segwit.py functional tests. This pull includes the commit from #14300. Tree-SHA512: 1b8c58e7aa0f71075ed5ff3e5be0a5182599108d8cd9bce682feac3b1842508124288e9335432b16a43f40f159c9710899e6d84af1b5868f48c947bc6f3e07ec
bitcoin · Sep 27, 2018 · edcf29c · edcf29c
2 parents ae1cc01 + e460232
commit edcf29c
Show file tree

Hide file tree

Showing 4 changed files with 153 additions and 50 deletions.
diff --git a/test/functional/README.md b/test/functional/README.md
@@ -60,6 +60,11 @@ don't have test cases for.
 - When calling RPCs with lots of arguments, consider using named keyword
   arguments instead of positional arguments to make the intent of the call
   clear to readers.
+- Many of the core test framework classes such as `CBlock` and `CTransaction`
+  don't allow new attributes to be added to their objects at runtime like
+  typical Python objects allow. This helps prevent unpredictable side effects
+  from typographical errors or usage of the objects outside of their intended
+  purpose.
 
 #### RPC and P2P definitions
 
@@ -72,7 +77,7 @@ P2P messages. These can be found in the following source files:
 
 #### Using the P2P interface
 
-- `mininode.py` contains all the definitions for objects that pass
+- `messages.py` contains all the definitions for objects that pass
 over the network (`CBlock`, `CTransaction`, etc, along with the network-level
 wrappers for them, `msg_block`, `msg_tx`, etc).
 

diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
@@ -205,7 +205,7 @@ def build_next_block(self, version=4):
         height = self.nodes[0].getblockcount() + 1
         block_time = self.nodes[0].getblockheader(tip)["mediantime"] + 1
         block = create_block(int(tip, 16), create_coinbase(height), block_time)
-        block.version = version
+        block.nVersion = version
         block.rehash()
         return block
 
@@ -769,12 +769,16 @@ def test_p2sh_witness(self):
         # will require a witness to spend a witness program regardless of
         # segwit activation.  Note that older bitcoind's that are not
         # segwit-aware would also reject this for failing CLEANSTACK.
-        test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
+        with self.nodes[0].assert_debug_log(
+                expected_msgs=(spend_tx.hash, 'was not accepted: non-mandatory-script-verify-flag (Witness program was passed an empty witness)')):
+            test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
 
-        # Try to put the witness script in the script_sig, should also fail.
-        spend_tx.vin[0].script_sig = CScript([p2wsh_pubkey, b'a'])
+        # Try to put the witness script in the scriptSig, should also fail.
+        spend_tx.vin[0].scriptSig = CScript([p2wsh_pubkey, b'a'])
         spend_tx.rehash()
-        test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
+        with self.nodes[0].assert_debug_log(
+                expected_msgs=('Not relaying invalid transaction {}'.format(spend_tx.hash), 'was not accepted: mandatory-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)')):
+            test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
 
         # Now put the witness script in the witness, should succeed after
         # segwit activates.