test: refactor: deduplicate legacy ECDSA signing for tx inputs

There are several instances in functional tests and the framework
(MiniWallet, feature_block.py, p2p_segwit.py) where we create a legacy
ECDSA signature for a certain transaction's input by doing the following
steps:
    1) calculate the `LegacySignatureHash` with the desired sighash type
    2) create the actual digital signature by calling `ECKey.sign_ecdsa`
       on the signature message hash calculated above
    3) put the DER-encoded result as CScript data push into
       tx input's scriptSig

Create a new helper `sign_input_legacy` which hides those details and
takes only the necessary parameters (tx, input index, relevant
scriptPubKey, private key, sighash type [SIGHASH_ALL by default]). For
further convenience, the signature is prepended to already existing
data-pushes in scriptSig, in order to avoid rehashing the transaction
after calling the new signing function.

4 files changed, 24 insertions, 25 deletions

diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index 765db97445..58ef1e761d 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -43,8 +43,7 @@ from test_framework.script import (
     OP_INVALIDOPCODE,
     OP_RETURN,
     OP_TRUE,
-    SIGHASH_ALL,
-    LegacySignatureHash,
+    sign_input_legacy,
 )
 from test_framework.script_util import (
     script_to_p2sh_script,
@@ -539,12 +538,8 @@ class FullBlockTest(BitcoinTestFramework):
             # second input is corresponding P2SH output from b39
             tx.vin.append(CTxIn(COutPoint(b39.vtx[i].sha256, 0), b''))
             # Note: must pass the redeem_script (not p2sh_script) to the signature hash function
-            (sighash, err) = LegacySignatureHash(redeem_script, tx, 1, SIGHASH_ALL)
-            sig = self.coinbase_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))
-            scriptSig = CScript([sig, redeem_script])
-
-            tx.vin[1].scriptSig = scriptSig
-            tx.rehash()
+            tx.vin[1].scriptSig = CScript([redeem_script])
+            sign_input_legacy(tx, 1, redeem_script, self.coinbase_key)
             new_txs.append(tx)
             lastOutpoint = COutPoint(tx.sha256, 0)
 
@@ -1338,8 +1333,7 @@ class FullBlockTest(BitcoinTestFramework):
         if (scriptPubKey[0] == OP_TRUE):  # an anyone-can-spend
             tx.vin[0].scriptSig = CScript()
             return
-        (sighash, err) = LegacySignatureHash(spend_tx.vout[0].scriptPubKey, tx, 0, SIGHASH_ALL)
-        tx.vin[0].scriptSig = CScript([self.coinbase_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))])
+        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)
diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index bfae190c66..1e7bc95a63 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -71,8 +71,8 @@ from test_framework.script import (
     SIGHASH_NONE,
     SIGHASH_SINGLE,
     SegwitV0SignatureHash,
-    LegacySignatureHash,
     hash160,
+    sign_input_legacy,
 )
 from test_framework.script_util import (
     key_to_p2pk_script,
@@ -1529,10 +1529,8 @@ class SegWitTest(BitcoinTestFramework):
         tx5 = CTransaction()
         tx5.vin.append(CTxIn(COutPoint(tx4.sha256, 0), b""))
         tx5.vout.append(CTxOut(tx4.vout[0].nValue - 1000, CScript([OP_TRUE])))
-        (sig_hash, err) = LegacySignatureHash(script_pubkey, tx5, 0, SIGHASH_ALL)
-        signature = key.sign_ecdsa(sig_hash) + b'\x01'  # 0x1 is SIGHASH_ALL
-        tx5.vin[0].scriptSig = CScript([signature, pubkey])
-        tx5.rehash()
+        tx5.vin[0].scriptSig = CScript([pubkey])
+        sign_input_legacy(tx5, 0, script_pubkey, key)
         # Should pass policy and consensus.
         test_transaction_acceptance(self.nodes[0], self.test_node, tx5, True, True)
         block = self.build_next_block()
diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py
index 443cae86a1..78f58cf11f 100644
--- a/test/functional/test_framework/script.py
+++ b/test/functional/test_framework/script.py
@@ -689,6 +689,16 @@ def LegacySignatureHash(*args, **kwargs):
     else:
         return (hash256(msg), err)
 
+def sign_input_legacy(tx, input_index, input_scriptpubkey, privkey, sighash_type=SIGHASH_ALL):
+    """Add legacy ECDSA signature for a given transaction input. Note that the signature
+       is prepended to the scriptSig field, i.e. additional data pushes necessary for more
+       complex spends than P2PK (e.g. pubkey for P2PKH) can be already set before."""
+    (sighash, err) = LegacySignatureHash(input_scriptpubkey, tx, input_index, sighash_type)
+    assert err is None
+    der_sig = privkey.sign_ecdsa(sighash)
+    tx.vin[input_index].scriptSig = bytes(CScript([der_sig + bytes([sighash_type])])) + tx.vin[input_index].scriptSig
+    tx.rehash()
+
 # TODO: Allow cached hashPrevouts/hashSequence/hashOutputs to be provided.
 # Performance optimization probably not necessary for python tests, however.
 # Note that this corresponds to sigversion == 1 in EvalScript, which is used
diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py
index 271095ea21..4d75194353 100644
--- a/test/functional/test_framework/wallet.py
+++ b/test/functional/test_framework/wallet.py
@@ -36,12 +36,11 @@ from test_framework.messages import (
 )
 from test_framework.script import (
     CScript,
-    LegacySignatureHash,
     LEAF_VERSION_TAPSCRIPT,
     OP_NOP,
     OP_RETURN,
     OP_TRUE,
-    SIGHASH_ALL,
+    sign_input_legacy,
     taproot_construct,
 )
 from test_framework.script_util import (
@@ -166,18 +165,16 @@ class MiniWallet:
 
     def sign_tx(self, tx, fixed_length=True):
         if self._mode == MiniWalletMode.RAW_P2PK:
-            (sighash, err) = LegacySignatureHash(CScript(self._scriptPubKey), tx, 0, SIGHASH_ALL)
-            assert err is None
             # for exact fee calculation, create only signatures with fixed size by default (>49.89% probability):
             # 65 bytes: high-R val (33 bytes) + low-S val (32 bytes)
-            # with the DER header/skeleton data of 6 bytes added, this leads to a target size of 71 bytes
-            der_sig = b''
-            while not len(der_sig) == 71:
-                der_sig = self._priv_key.sign_ecdsa(sighash)
+            # with the DER header/skeleton data of 6 bytes added, plus 2 bytes scriptSig overhead
+            # (OP_PUSHn and SIGHASH_ALL), this leads to a scriptSig target size of 73 bytes
+            tx.vin[0].scriptSig = b''
+            while not len(tx.vin[0].scriptSig) == 73:
+                tx.vin[0].scriptSig = b''
+                sign_input_legacy(tx, 0, self._scriptPubKey, self._priv_key)
                 if not fixed_length:
                     break
-            tx.vin[0].scriptSig = CScript([der_sig + bytes(bytearray([SIGHASH_ALL]))])
-            tx.rehash()
         elif self._mode == MiniWalletMode.RAW_OP_TRUE:
             for i in tx.vin:
                 i.scriptSig = CScript([OP_NOP] * 43)  # pad to identical size