test: refactor: deduplicate segwitv0 ECDSA signing for tx inputs #28154

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202307-test-deduplicate_segwitv0_input_signing changing 2 files +18 −11

theStack commented at 8:31 PM on July 25, 2023: contributor
This PR is a simple follow-up for #28025. It introduces a signing_input_segwitv0 helper in order to deduplicate the following steps needed to create a segwitv0 ECDSA signature:
1. calculate the SegwitV0SignatureHash 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 (plus sighash byte) at the bottom of the witness stack
test: refactor: deduplicate segwitv0 ECDSA signing for tx inputs
Follow-up for #28025.
83d7cfd542
DrahtBot commented at 8:32 PM on July 25, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK pinheadmz, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Jul 25, 2023
fanquake requested review from pinheadmz on Aug 3, 2023

Type	Reviewers
ACK	pinheadmz, achow101

in test/functional/p2p_segwit.py:1677 in 83d7cfd542

1671 | @@ -1676,11 +1672,13 @@ def test_signature_version_1(self):
1672 |          tx2.vout.append(CTxOut(tx.vout[0].nValue, CScript([OP_TRUE])))
1673 |  
1674 |          script = keyhash_to_p2pkh_script(pubkeyhash)
1675 | -        sig_hash = SegwitV0SignatureHash(script, tx2, 0, SIGHASH_ALL, tx.vout[0].nValue)
1676 | -        signature = key.sign_ecdsa(sig_hash) + b'\x01'  # 0x1 is SIGHASH_ALL
1677 | +        tx2.wit.vtxinwit.append(CTxInWitness())
1678 | +        sign_input_segwitv0(tx2, 0, script, tx.vout[0].nValue, key)
1679 | +        signature = tx2.wit.vtxinwit[0].scriptWitness.stack.pop()

pinheadmz commented at 3:35 PM on August 3, 2023:

I kinda feel like you don't need to use the helper for this one. It's actually +2 lines as opposed to -2 in the other locations.

pinheadmz commented at 3:36 PM on August 3, 2023: member

code review ACK at 83d7cfd5429b0be7755bc48145032956f5f56dae
achow101 commented at 5:43 PM on September 20, 2023: member

ACK 83d7cfd5429b0be7755bc48145032956f5f56dae
achow101 merged this on Sep 20, 2023
achow101 closed this on Sep 20, 2023
theStack deleted the branch on Sep 21, 2023
Frank-GER referenced this in commit bf7fe797ea on Sep 25, 2023
sidhujag referenced this in commit ff8bde8b5c on Sep 26, 2023
bitcoin locked this on Sep 20, 2024

Contributors

Labels

Linked (view graph)

#28025 test: refactor: deduplicate legacy ECDSA signing for tx inputs

test: refactor: deduplicate segwitv0 ECDSA signing for tx inputs #28154

Reviews