diff options
author | merge-script <fanquake@gmail.com> | 2024-08-02 14:02:04 +0100 |
---|---|---|
committer | merge-script <fanquake@gmail.com> | 2024-08-02 14:02:04 +0100 |
commit | df241970a3a3e7e24c001c399435d1baf3b8a2ae (patch) | |
tree | c0596c587777e4a2746e5a15f98c3d31ff9e99e3 | |
parent | 9c6c667bc2bcd34270241db52411369c46c9f2f0 (diff) | |
parent | fa46a1b74bd35371036af17b2df2036dbc993ce1 (diff) |
Merge bitcoin/bitcoin#30554: test: Avoid CScript() as default function argument
fa46a1b74bd35371036af17b2df2036dbc993ce1 test: Avoid CScript() as default function argument (MarcoFalke)
fadf621825fdbf5fd131da14419bb19bb81e5801 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 https://github.com/bitcoin/bitcoin/issues/30543#issuecomment-2259260024 .
The lint rule will be enabled in a follow-up, when all violations are fixed.
ACKs for top commit:
instagibbs:
utACK fa46a1b74bd35371036af17b2df2036dbc993ce1
theStack:
lgtm ACK fa46a1b74bd35371036af17b2df2036dbc993ce1
ismaelsadeeq:
Tested ACK fa46a1b74bd35371036af17b2df2036dbc993ce1
Tree-SHA512: bc68b15121d50ead0fc70ad772360a7829908aedeaff8426efcb8a67f33117f67d26b4f5da94fa735dd8de9c9ff65fc10a29323f1b12f238b75486fa7cc32a89
-rw-r--r-- | test/functional/data/invalid_txs.py | 4 | ||||
-rwxr-xr-x | test/functional/feature_block.py | 18 | ||||
-rwxr-xr-x | test/functional/feature_taproot.py | 2 | ||||
-rw-r--r-- | test/functional/test_framework/blocktools.py | 6 | ||||
-rw-r--r-- | test/functional/test_framework/script.py | 10 |
5 files changed, 24 insertions, 16 deletions
diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index 33054fd517..2e4ca83bf0 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -192,7 +192,7 @@ class SpendTooMuch(BadTxTemplate): def get_tx(self): return create_tx_with_script( - self.spend_tx, 0, script_pub_key=basic_p2sh, amount=(self.spend_avail + 1)) + self.spend_tx, 0, output_script=basic_p2sh, amount=(self.spend_avail + 1)) class CreateNegative(BadTxTemplate): @@ -242,7 +242,7 @@ class TooManySigops(BadTxTemplate): lotsa_checksigs = CScript([OP_CHECKSIG] * (MAX_BLOCK_SIGOPS)) return create_tx_with_script( self.spend_tx, 0, - script_pub_key=lotsa_checksigs, + output_script=lotsa_checksigs, amount=1) def getDisabledOpcodeTemplate(opcode): diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 172af27848..384ca311c7 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -1326,8 +1326,10 @@ class FullBlockTest(BitcoinTestFramework): block.vtx.extend(tx_list) # this is a little handier to use than the version in blocktools.py - def create_tx(self, spend_tx, n, value, script=CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])): - return create_tx_with_script(spend_tx, n, amount=value, script_pub_key=script) + def create_tx(self, spend_tx, n, value, output_script=None): + if output_script is None: + output_script = CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE]) + return create_tx_with_script(spend_tx, n, amount=value, output_script=output_script) # sign a transaction, using the key we know about # this signs input 0 in tx, which is assumed to be spending output 0 in spend_tx @@ -1338,13 +1340,17 @@ class FullBlockTest(BitcoinTestFramework): return sign_input_legacy(tx, 0, spend_tx.vout[0].scriptPubKey, self.coinbase_key) - def create_and_sign_transaction(self, spend_tx, value, script=CScript([OP_TRUE])): - tx = self.create_tx(spend_tx, 0, value, script) + def create_and_sign_transaction(self, spend_tx, value, output_script=None): + if output_script is None: + output_script = CScript([OP_TRUE]) + tx = self.create_tx(spend_tx, 0, value, output_script=output_script) self.sign_tx(tx, spend_tx) tx.rehash() return tx - def next_block(self, number, spend=None, additional_coinbase_value=0, script=CScript([OP_TRUE]), *, version=4): + def next_block(self, number, spend=None, additional_coinbase_value=0, *, script=None, version=4): + if script is None: + script = CScript([OP_TRUE]) if self.tip is None: base_block_hash = self.genesis_hash block_time = int(time.time()) + 1 @@ -1361,7 +1367,7 @@ class FullBlockTest(BitcoinTestFramework): else: coinbase.vout[0].nValue += spend.vout[0].nValue - 1 # all but one satoshi to fees coinbase.rehash() - tx = self.create_tx(spend, 0, 1, script) # spend 1 satoshi + tx = self.create_tx(spend, 0, 1, output_script=script) # spend 1 satoshi self.sign_tx(tx, spend) tx.rehash() block = create_block(base_block_hash, coinbase, block_time, version=version, txlist=[tx]) diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index b2e030adc7..443c1c33da 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -231,7 +231,7 @@ def default_sigmsg(ctx): codeseppos = get(ctx, "codeseppos") leaf_ver = get(ctx, "leafversion") script = get(ctx, "script_taproot") - return TaprootSignatureMsg(tx, utxos, hashtype, idx, scriptpath=True, script=script, leaf_ver=leaf_ver, codeseparator_pos=codeseppos, annex=annex) + return TaprootSignatureMsg(tx, utxos, hashtype, idx, scriptpath=True, leaf_script=script, leaf_ver=leaf_ver, codeseparator_pos=codeseppos, annex=annex) else: return TaprootSignatureMsg(tx, utxos, hashtype, idx, scriptpath=False, annex=annex) elif mode == "witv0": diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 338b7fa3b0..5c2fa28a31 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -154,16 +154,18 @@ def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_scr coinbase.calc_sha256() return coinbase -def create_tx_with_script(prevtx, n, script_sig=b"", *, amount, script_pub_key=CScript()): +def create_tx_with_script(prevtx, n, script_sig=b"", *, amount, output_script=None): """Return one-input, one-output transaction object spending the prevtx's n-th output with the given amount. Can optionally pass scriptPubKey and scriptSig, default is anyone-can-spend output. """ + if output_script is None: + output_script = CScript() tx = CTransaction() assert n < len(prevtx.vout) tx.vin.append(CTxIn(COutPoint(prevtx.sha256, n), script_sig, SEQUENCE_FINAL)) - tx.vout.append(CTxOut(amount, script_pub_key)) + tx.vout.append(CTxOut(amount, output_script)) tx.calc_sha256() return tx diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 97d62f957b..d510cf9b1c 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -810,7 +810,7 @@ def BIP341_sha_sequences(txTo): def BIP341_sha_outputs(txTo): return sha256(b"".join(o.serialize() for o in txTo.vout)) -def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index = 0, scriptpath = False, script = CScript(), codeseparator_pos = -1, annex = None, leaf_ver = LEAF_VERSION_TAPSCRIPT): +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): assert (len(txTo.vin) == len(spent_utxos)) assert (input_index < len(txTo.vin)) 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 spend_type = 0 if annex is not None: spend_type |= 1 - if (scriptpath): + if scriptpath: spend_type |= 2 ss += bytes([spend_type]) if in_type == SIGHASH_ANYONECANPAY: @@ -846,11 +846,11 @@ def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index = 0, scriptpat ss += sha256(txTo.vout[input_index].serialize()) else: ss += bytes(0 for _ in range(32)) - if (scriptpath): - ss += TaggedHash("TapLeaf", bytes([leaf_ver]) + ser_string(script)) + if scriptpath: + ss += TaggedHash("TapLeaf", bytes([leaf_ver]) + ser_string(leaf_script)) ss += bytes([0]) ss += codeseparator_pos.to_bytes(4, "little", signed=True) - 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 + 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 return ss def TaprootSignatureHash(*args, **kwargs): |