Page MenuHomePhabricator

test: refactor: deduplicate ECDSA signing for tx inputs
ClosedPublic

Authored by PiRK on Thu, Nov 13, 16:16.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC18a0ba609a96: test: refactor: deduplicate ECDSA signing for tx inputs
Summary

There are several instances in functional tests and the framework
(MiniWallet, feature_block.py) where we create a legacy
ECDSA signature for a certain transaction's input by doing the following
steps:

  1. calculate the SignatureHashForkId 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 which hides those details and
takes only the necessary parameters (tx, input index, relevant
scriptPubKey, private key). 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.

This is a backport of core#28025 and a partial backport of core#32591
https://github.com/bitcoin/bitcoin/pull/32591/commits/1a689a2c88715c583da6d88cac3f77bdbaeaf551

Backport notes:

  • for now we always use SIGHASH_ALL | SIGHASH_FORKID as a sighash flag, so no need to pass it as a parameter
  • see D18733 for why the signing code is in blocktools rather than feature_block.py
  • I didn't put the new function in script.py to avoid a script.py <-> signature_hash.py circular import. It makes more sense to put it in txtools anyway
  • we weren't testing what we expected to test in feature_block.py wrt to InvalidOPIFConstruction and DisabledOpcode_*. The blocks including these transactions were accepted because sign_tx was overwriting the problematic scriptsig. Now they properly fail block inclusion for the expected reason (pushonly rule)
Test Plan

ninja check-functional

Diff Detail

Event Timeline

PiRK requested review of this revision.Thu, Nov 13, 16:16
PiRK retitled this revision from test: refactor: deduplicate legacy ECDSA signing for tx inputs to test: refactor: deduplicate ECDSA signing for tx inputs.Thu, Nov 13, 16:48
This revision is now accepted and ready to land.Fri, Nov 14, 10:03