Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#27653: test: add unit test coverage for Python …
Browse files Browse the repository at this point in the history
…ECDSA implementation

96b3f2d test: add unit test coverage for Python ECDSA implementation (Sebastian Falbesoner)

Pull request description:

  This PR adds missing unit test coverage for the Python ECDSA implementation, which should be useful for detecting potential problems early whenever changes in the test framework's Python implementation of secp256k1 are made (e.g. #26222). Note that right now we don't call `ECPubKey.verify_ecdsa` anywhere in our tests, so we wouldn't notice if it is broken at some point.

  To keep it simple, the already existing unit test for Schnorr signatures is extended to also check ECDSA signatures. For that purpose, the dictionary storing private-key/public-key entries use their legacy types `ECKey/ECPubKey` instead of bare byte-arrays, and for Schnorr signing/verification the necessary conversions (ECKey -> bare private key, ECPubKey -> x-only pubkey) is done later when needed. To avoid code duplication, a helper function `random_bitflip` for damaging signatures is introduced.

  The unit test can be run by either calling it for this single module:
  `$ python3 -m unittest ./test/functional/test_framework/key.py`
  or simply running `$ ./test/functional/test_runner.py` which calls all test framework module's unit tests at the start (see TEST_FRAMEWORK_MODULES list).

ACKs for top commit:
  achow101:
    ACK 96b3f2d
  sipa:
    utACK 96b3f2d
  stratospher:
    tested ACK 96b3f2d.

Tree-SHA512: b993f25b843fa047376addda4ce4b0f15750ffba926528b5cca4c5f99b9af456206f4e8af885d25a017dddddf382ddebf38765819b3d16a3f28810d03b010808
  • Loading branch information
achow101 committed Sep 29, 2023
2 parents f562856 + 96b3f2d commit 5bbf735
Showing 1 changed file with 21 additions and 12 deletions.
33 changes: 21 additions & 12 deletions test/functional/test_framework/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,24 +290,33 @@ def sign_schnorr(key, msg, aux=None, flip_p=False, flip_r=False):


class TestFrameworkKey(unittest.TestCase):
def test_schnorr(self):
"""Test the Python Schnorr implementation."""
def test_ecdsa_and_schnorr(self):
"""Test the Python ECDSA and Schnorr implementations."""
def random_bitflip(sig):
sig = list(sig)
sig[random.randrange(len(sig))] ^= (1 << (random.randrange(8)))
return bytes(sig)

byte_arrays = [generate_privkey() for _ in range(3)] + [v.to_bytes(32, 'big') for v in [0, ORDER - 1, ORDER, 2**256 - 1]]
keys = {}
for privkey in byte_arrays: # build array of key/pubkey pairs
pubkey, _ = compute_xonly_pubkey(privkey)
if pubkey is not None:
keys[privkey] = pubkey
for privkey_bytes in byte_arrays: # build array of key/pubkey pairs
privkey = ECKey()
privkey.set(privkey_bytes, compressed=True)
if privkey.is_valid:
keys[privkey] = privkey.get_pubkey()
for msg in byte_arrays: # test every combination of message, signing key, verification key
for sign_privkey, _ in keys.items():
sig = sign_schnorr(sign_privkey, msg)
sig_ecdsa = sign_privkey.sign_ecdsa(msg)
sig_schnorr = sign_schnorr(sign_privkey.get_bytes(), msg)
for verify_privkey, verify_pubkey in keys.items():
verify_xonly_pubkey = verify_pubkey.get_bytes()[1:]
if verify_privkey == sign_privkey:
self.assertTrue(verify_schnorr(verify_pubkey, sig, msg))
sig = list(sig)
sig[random.randrange(64)] ^= (1 << (random.randrange(8))) # damaging signature should break things
sig = bytes(sig)
self.assertFalse(verify_schnorr(verify_pubkey, sig, msg))
self.assertTrue(verify_pubkey.verify_ecdsa(sig_ecdsa, msg))
self.assertTrue(verify_schnorr(verify_xonly_pubkey, sig_schnorr, msg))
sig_ecdsa = random_bitflip(sig_ecdsa) # damaging signature should break things
sig_schnorr = random_bitflip(sig_schnorr)
self.assertFalse(verify_pubkey.verify_ecdsa(sig_ecdsa, msg))
self.assertFalse(verify_schnorr(verify_xonly_pubkey, sig_schnorr, msg))

def test_schnorr_testvectors(self):
"""Implement the BIP340 test vectors (read from bip340_test_vectors.csv)."""
Expand Down

0 comments on commit 5bbf735

Please sign in to comment.