Skip to content

Commit df24197

Browse files
committed
Merge bitcoin/bitcoin#30554: test: Avoid CScript() as default function argument
fa46a1b test: Avoid CScript() as default function argument (MarcoFalke) fadf621 test: Make leaf_script mandatory when scriptpath is set in TaprootSignatureMsg (MarcoFalke) Pull request description: Unlike other function calls in default arguments, CScript should not cause any issues in the tests, because they are const. However, this change allows to enable the "function-call-in-default-argument (B008)" lint rule, which will help to catch severe test bugs, such as bitcoin/bitcoin#30543 (comment) . The lint rule will be enabled in a follow-up, when all violations are fixed. ACKs for top commit: instagibbs: utACK fa46a1b theStack: lgtm ACK fa46a1b ismaelsadeeq: Tested ACK fa46a1b Tree-SHA512: bc68b15121d50ead0fc70ad772360a7829908aedeaff8426efcb8a67f33117f67d26b4f5da94fa735dd8de9c9ff65fc10a29323f1b12f238b75486fa7cc32a89
2 parents 9c6c667 + fa46a1b commit df24197

File tree

5 files changed

+24
-16
lines changed

5 files changed

+24
-16
lines changed

test/functional/data/invalid_txs.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ class SpendTooMuch(BadTxTemplate):
192192

193193
def get_tx(self):
194194
return create_tx_with_script(
195-
self.spend_tx, 0, script_pub_key=basic_p2sh, amount=(self.spend_avail + 1))
195+
self.spend_tx, 0, output_script=basic_p2sh, amount=(self.spend_avail + 1))
196196

197197

198198
class CreateNegative(BadTxTemplate):
@@ -242,7 +242,7 @@ def get_tx(self):
242242
lotsa_checksigs = CScript([OP_CHECKSIG] * (MAX_BLOCK_SIGOPS))
243243
return create_tx_with_script(
244244
self.spend_tx, 0,
245-
script_pub_key=lotsa_checksigs,
245+
output_script=lotsa_checksigs,
246246
amount=1)
247247

248248
def getDisabledOpcodeTemplate(opcode):

test/functional/feature_block.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1326,8 +1326,10 @@ def add_transactions_to_block(self, block, tx_list):
13261326
block.vtx.extend(tx_list)
13271327

13281328
# this is a little handier to use than the version in blocktools.py
1329-
def create_tx(self, spend_tx, n, value, script=CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])):
1330-
return create_tx_with_script(spend_tx, n, amount=value, script_pub_key=script)
1329+
def create_tx(self, spend_tx, n, value, output_script=None):
1330+
if output_script is None:
1331+
output_script = CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])
1332+
return create_tx_with_script(spend_tx, n, amount=value, output_script=output_script)
13311333

13321334
# sign a transaction, using the key we know about
13331335
# this signs input 0 in tx, which is assumed to be spending output 0 in spend_tx
@@ -1338,13 +1340,17 @@ def sign_tx(self, tx, spend_tx):
13381340
return
13391341
sign_input_legacy(tx, 0, spend_tx.vout[0].scriptPubKey, self.coinbase_key)
13401342

1341-
def create_and_sign_transaction(self, spend_tx, value, script=CScript([OP_TRUE])):
1342-
tx = self.create_tx(spend_tx, 0, value, script)
1343+
def create_and_sign_transaction(self, spend_tx, value, output_script=None):
1344+
if output_script is None:
1345+
output_script = CScript([OP_TRUE])
1346+
tx = self.create_tx(spend_tx, 0, value, output_script=output_script)
13431347
self.sign_tx(tx, spend_tx)
13441348
tx.rehash()
13451349
return tx
13461350

1347-
def next_block(self, number, spend=None, additional_coinbase_value=0, script=CScript([OP_TRUE]), *, version=4):
1351+
def next_block(self, number, spend=None, additional_coinbase_value=0, *, script=None, version=4):
1352+
if script is None:
1353+
script = CScript([OP_TRUE])
13481354
if self.tip is None:
13491355
base_block_hash = self.genesis_hash
13501356
block_time = int(time.time()) + 1
@@ -1361,7 +1367,7 @@ def next_block(self, number, spend=None, additional_coinbase_value=0, script=CSc
13611367
else:
13621368
coinbase.vout[0].nValue += spend.vout[0].nValue - 1 # all but one satoshi to fees
13631369
coinbase.rehash()
1364-
tx = self.create_tx(spend, 0, 1, script) # spend 1 satoshi
1370+
tx = self.create_tx(spend, 0, 1, output_script=script) # spend 1 satoshi
13651371
self.sign_tx(tx, spend)
13661372
tx.rehash()
13671373
block = create_block(base_block_hash, coinbase, block_time, version=version, txlist=[tx])

test/functional/feature_taproot.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ def default_sigmsg(ctx):
231231
codeseppos = get(ctx, "codeseppos")
232232
leaf_ver = get(ctx, "leafversion")
233233
script = get(ctx, "script_taproot")
234-
return TaprootSignatureMsg(tx, utxos, hashtype, idx, scriptpath=True, script=script, leaf_ver=leaf_ver, codeseparator_pos=codeseppos, annex=annex)
234+
return TaprootSignatureMsg(tx, utxos, hashtype, idx, scriptpath=True, leaf_script=script, leaf_ver=leaf_ver, codeseparator_pos=codeseppos, annex=annex)
235235
else:
236236
return TaprootSignatureMsg(tx, utxos, hashtype, idx, scriptpath=False, annex=annex)
237237
elif mode == "witv0":

test/functional/test_framework/blocktools.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,16 +154,18 @@ def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_scr
154154
coinbase.calc_sha256()
155155
return coinbase
156156

157-
def create_tx_with_script(prevtx, n, script_sig=b"", *, amount, script_pub_key=CScript()):
157+
def create_tx_with_script(prevtx, n, script_sig=b"", *, amount, output_script=None):
158158
"""Return one-input, one-output transaction object
159159
spending the prevtx's n-th output with the given amount.
160160
161161
Can optionally pass scriptPubKey and scriptSig, default is anyone-can-spend output.
162162
"""
163+
if output_script is None:
164+
output_script = CScript()
163165
tx = CTransaction()
164166
assert n < len(prevtx.vout)
165167
tx.vin.append(CTxIn(COutPoint(prevtx.sha256, n), script_sig, SEQUENCE_FINAL))
166-
tx.vout.append(CTxOut(amount, script_pub_key))
168+
tx.vout.append(CTxOut(amount, output_script))
167169
tx.calc_sha256()
168170
return tx
169171

test/functional/test_framework/script.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ def BIP341_sha_sequences(txTo):
810810
def BIP341_sha_outputs(txTo):
811811
return sha256(b"".join(o.serialize() for o in txTo.vout))
812812

813-
def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index = 0, scriptpath = False, script = CScript(), codeseparator_pos = -1, annex = None, leaf_ver = LEAF_VERSION_TAPSCRIPT):
813+
def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index=0, *, scriptpath=False, leaf_script=None, codeseparator_pos=-1, annex=None, leaf_ver=LEAF_VERSION_TAPSCRIPT):
814814
assert (len(txTo.vin) == len(spent_utxos))
815815
assert (input_index < len(txTo.vin))
816816
out_type = SIGHASH_ALL if hash_type == 0 else hash_type & 3
@@ -829,7 +829,7 @@ def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index = 0, scriptpat
829829
spend_type = 0
830830
if annex is not None:
831831
spend_type |= 1
832-
if (scriptpath):
832+
if scriptpath:
833833
spend_type |= 2
834834
ss += bytes([spend_type])
835835
if in_type == SIGHASH_ANYONECANPAY:
@@ -846,11 +846,11 @@ def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index = 0, scriptpat
846846
ss += sha256(txTo.vout[input_index].serialize())
847847
else:
848848
ss += bytes(0 for _ in range(32))
849-
if (scriptpath):
850-
ss += TaggedHash("TapLeaf", bytes([leaf_ver]) + ser_string(script))
849+
if scriptpath:
850+
ss += TaggedHash("TapLeaf", bytes([leaf_ver]) + ser_string(leaf_script))
851851
ss += bytes([0])
852852
ss += codeseparator_pos.to_bytes(4, "little", signed=True)
853-
assert len(ss) == 175 - (in_type == SIGHASH_ANYONECANPAY) * 49 - (out_type != SIGHASH_ALL and out_type != SIGHASH_SINGLE) * 32 + (annex is not None) * 32 + scriptpath * 37
853+
assert len(ss) == 175 - (in_type == SIGHASH_ANYONECANPAY) * 49 - (out_type != SIGHASH_ALL and out_type != SIGHASH_SINGLE) * 32 + (annex is not None) * 32 + scriptpath * 37
854854
return ss
855855

856856
def TaprootSignatureHash(*args, **kwargs):

0 commit comments

Comments
 (0)