Skip to content

Commit edcf29c

Browse files
author
MarcoFalke
committed
Merge #14305: Tests: enforce critical class instance attributes in functional 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 bitcoin/bitcoin#14300. This request fixes the erroneous version attribute used in the p2p_segwit.py functional tests. This pull includes the commit from bitcoin/bitcoin#14300. Tree-SHA512: 1b8c58e7aa0f71075ed5ff3e5be0a5182599108d8cd9bce682feac3b1842508124288e9335432b16a43f40f159c9710899e6d84af1b5868f48c947bc6f3e07ec
2 parents ae1cc01 + e460232 commit edcf29c

File tree

4 files changed

+153
-50
lines changed

4 files changed

+153
-50
lines changed

test/functional/README.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ don't have test cases for.
6060
- When calling RPCs with lots of arguments, consider using named keyword
6161
arguments instead of positional arguments to make the intent of the call
6262
clear to readers.
63+
- Many of the core test framework classes such as `CBlock` and `CTransaction`
64+
don't allow new attributes to be added to their objects at runtime like
65+
typical Python objects allow. This helps prevent unpredictable side effects
66+
from typographical errors or usage of the objects outside of their intended
67+
purpose.
6368

6469
#### RPC and P2P definitions
6570

@@ -72,7 +77,7 @@ P2P messages. These can be found in the following source files:
7277

7378
#### Using the P2P interface
7479

75-
- `mininode.py` contains all the definitions for objects that pass
80+
- `messages.py` contains all the definitions for objects that pass
7681
over the network (`CBlock`, `CTransaction`, etc, along with the network-level
7782
wrappers for them, `msg_block`, `msg_tx`, etc).
7883

test/functional/p2p_segwit.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ def build_next_block(self, version=4):
205205
height = self.nodes[0].getblockcount() + 1
206206
block_time = self.nodes[0].getblockheader(tip)["mediantime"] + 1
207207
block = create_block(int(tip, 16), create_coinbase(height), block_time)
208-
block.version = version
208+
block.nVersion = version
209209
block.rehash()
210210
return block
211211

@@ -769,12 +769,16 @@ def test_p2sh_witness(self):
769769
# will require a witness to spend a witness program regardless of
770770
# segwit activation. Note that older bitcoind's that are not
771771
# segwit-aware would also reject this for failing CLEANSTACK.
772-
test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
772+
with self.nodes[0].assert_debug_log(
773+
expected_msgs=(spend_tx.hash, 'was not accepted: non-mandatory-script-verify-flag (Witness program was passed an empty witness)')):
774+
test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
773775

774-
# Try to put the witness script in the script_sig, should also fail.
775-
spend_tx.vin[0].script_sig = CScript([p2wsh_pubkey, b'a'])
776+
# Try to put the witness script in the scriptSig, should also fail.
777+
spend_tx.vin[0].scriptSig = CScript([p2wsh_pubkey, b'a'])
776778
spend_tx.rehash()
777-
test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
779+
with self.nodes[0].assert_debug_log(
780+
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)')):
781+
test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False)
778782

779783
# Now put the witness script in the witness, should succeed after
780784
# segwit activates.

0 commit comments

Comments
 (0)