Skip to content

Commit b1269fe

Browse files
achow101vijaydasmp
authored andcommitted
Merge bitcoin#28025: test: refactor: deduplicate legacy ECDSA signing for tx inputs
5cf4427 test: refactor: deduplicate legacy ECDSA signing for tx inputs (Sebastian Falbesoner) Pull request description: 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. ACKs for top commit: dimitaracev: ACK `5cf4427` achow101: ACK 5cf4427 pinheadmz: ACK 5cf4427 Tree-SHA512: 8f0e4fb2c3e0f84fac5dbc4dda87973276242b0f628034272a7f3e45434c1e17dd1b26a37edfb302dcaf380dbfe98b0417391ace5e0ac9720155d8fba702031e
1 parent ac6bd1d commit b1269fe

File tree

3 files changed

+22
-20
lines changed

3 files changed

+22
-20
lines changed

test/functional/feature_block.py

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@
4242
OP_INVALIDOPCODE,
4343
OP_RETURN,
4444
OP_TRUE,
45-
SIGHASH_ALL,
46-
SignatureHash,
45+
sign_input,
4746
)
4847
from test_framework.script_util import (
4948
script_to_p2sh_script,
@@ -551,12 +550,8 @@ def run_test(self):
551550
# second input is corresponding P2SH output from b39
552551
tx.vin.append(CTxIn(COutPoint(b39.vtx[i].sha256, 0), b''))
553552
# Note: must pass the redeem_script (not p2sh_script) to the signature hash function
554-
(sighash, err) = SignatureHash(redeem_script, tx, 1, SIGHASH_ALL)
555-
sig = self.coinbase_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))
556-
scriptSig = CScript([sig, redeem_script])
557-
558-
tx.vin[1].scriptSig = scriptSig
559-
tx.rehash()
553+
tx.vin[1].scriptSig = CScript([redeem_script])
554+
sign_input(tx, 1, redeem_script, self.coinbase_key)
560555
new_txs.append(tx)
561556
lastOutpoint = COutPoint(tx.sha256, 0)
562557

@@ -1354,8 +1349,7 @@ def sign_tx(self, tx, spend_tx):
13541349
if (scriptPubKey[0] == OP_TRUE): # an anyone-can-spend
13551350
tx.vin[0].scriptSig = CScript()
13561351
return
1357-
(sighash, err) = SignatureHash(spend_tx.vout[0].scriptPubKey, tx, 0, SIGHASH_ALL)
1358-
tx.vin[0].scriptSig = CScript([self.coinbase_key.sign_ecdsa(sighash) + bytes(bytearray([SIGHASH_ALL]))])
1352+
sign_input(tx, 0, spend_tx.vout[0].scriptPubKey, self.coinbase_key)
13591353

13601354
def create_and_sign_transaction(self, spend_tx, value, script=CScript([OP_TRUE])):
13611355
tx = self.create_tx(spend_tx, 0, value, script)

test/functional/test_framework/script.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,17 @@ def SignatureHash(script, txTo, inIdx, hashtype):
666666

667667
return (hash, None)
668668

669+
def sign_input(tx, input_index, input_scriptpubkey, privkey, sighash_type=SIGHASH_ALL):
670+
"""Add legacy ECDSA signature for a given transaction input. Note that the signature
671+
is prepended to the scriptSig field, i.e. additional data pushes necessary for more
672+
complex spends than P2PK (e.g. pubkey for P2PKH) can be already set before."""
673+
(sighash, err) = SignatureHash(input_scriptpubkey, tx, input_index, sighash_type)
674+
assert err is None
675+
der_sig = privkey.sign_ecdsa(sighash)
676+
tx.vin[input_index].scriptSig = bytes(CScript([der_sig + bytes([sighash_type])])) + tx.vin[input_index].scriptSig
677+
tx.rehash()
678+
679+
669680
class TestFrameworkScript(unittest.TestCase):
670681
def test_bn2vch(self):
671682
self.assertEqual(bn2vch(0), bytes([]))

test/functional/test_framework/wallet.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,9 @@
3030
)
3131
from test_framework.script import (
3232
CScript,
33-
SignatureHash,
3433
OP_TRUE,
3534
OP_NOP,
36-
SIGHASH_ALL,
35+
sign_input,
3736
)
3837
from test_framework.script_util import (
3938
key_to_p2pk_script,
@@ -134,18 +133,16 @@ def scan_tx(self, tx):
134133

135134
def sign_tx(self, tx, fixed_length=True):
136135
if self._mode == MiniWalletMode.RAW_P2PK:
137-
(sighash, err) = SignatureHash(CScript(self._scriptPubKey), tx, 0, SIGHASH_ALL)
138-
assert err is None
139136
# for exact fee calculation, create only signatures with fixed size by default (>49.89% probability):
140137
# 65 bytes: high-R val (33 bytes) + low-S val (32 bytes)
141-
# with the DER header/skeleton data of 6 bytes added, this leads to a target size of 71 bytes
142-
der_sig = b''
143-
while not len(der_sig) == 71:
144-
der_sig = self._priv_key.sign_ecdsa(sighash)
138+
# with the DER header/skeleton data of 6 bytes added, plus 2 bytes scriptSig overhead
139+
# (OP_PUSHn and SIGHASH_ALL), this leads to a scriptSig target size of 73 bytes
140+
tx.vin[0].scriptSig = b''
141+
while not len(tx.vin[0].scriptSig) == 73:
142+
tx.vin[0].scriptSig = b''
143+
sign_input(tx, 0, self._scriptPubKey, self._priv_key)
145144
if not fixed_length:
146145
break
147-
tx.vin[0].scriptSig = CScript([der_sig + bytes(bytearray([SIGHASH_ALL]))])
148-
tx.rehash()
149146
elif self._mode == MiniWalletMode.RAW_OP_TRUE:
150147
for i in range(len(tx.vin)):
151148
tx.vin[i].scriptSig = CScript([OP_NOP] * 24) # pad to identical size

0 commit comments

Comments
 (0)