Skip to content

Conversation

@theStack
Copy link
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

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 25, 2023

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 DrahtBot added the Tests label Jul 25, 2023
@fanquake fanquake requested a review from pinheadmz August 3, 2023 10:50
Copy link
Member

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code review ACK at 83d7cfd

Comment on lines +1675 to +1677
tx2.wit.vtxinwit.append(CTxInWitness())
sign_input_segwitv0(tx2, 0, script, tx.vout[0].nValue, key)
signature = tx2.wit.vtxinwit[0].scriptWitness.stack.pop()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@achow101
Copy link
Member

ACK 83d7cfd

@achow101 achow101 merged commit 8247a8d into bitcoin:master Sep 20, 2023
@theStack theStack deleted the 202307-test-deduplicate_segwitv0_input_signing branch September 21, 2023 00:59
Frank-GER pushed a commit to syscoin/syscoin that referenced this pull request Sep 25, 2023
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 26, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Sep 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants