Skip to content

Commit

Permalink
Remove coincurve dependency, use python-bitcointx
Browse files Browse the repository at this point in the history
This commit changes the underlying functions used in
jmbitcoin from the private and public key primitives
in coincurve and replaces them with equivalent
primitives CKey and CPubKey from python-bitcointx,
this removes the need to install coincurve and its
own bundled libsecp256k1 dynamic library.
Note that an additional pubkey_tweak_mul function
is exposed with ctypes from python-bitcointx's
bundled libsecp256k1 library.
  • Loading branch information
AdamISZ committed Dec 30, 2021
1 parent 7d1c4eb commit 537e317
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 102 deletions.
12 changes: 8 additions & 4 deletions docs/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ The Issues list is for specific bugs or feature requests.

* PEP8 compliance.
* Details which may or may not be included in PEP8 might be, consistent variable naming conventions, and use of single/double quotes.
* ~~Porting to Python 3~~. This is done in that we are now Py2 and Py3 compatible as of 0.5.0; but we may deprecate Py2 soon.
* ~~Porting to Python 3~~. This is done in that we are now Py2 and Py3 compatible as of 0.5.0; ~~but we may deprecate Py2 soon~~ Python2 is now deprecated, as is Python3 below 3.6.

~~A note on the above - took a look at it last December, but had problems in particular with some twisted elements, specifically `txsocksx`~~ Done as of 0.4.2, now switched to txtorcon.

Expand Down Expand Up @@ -76,9 +76,11 @@ Windows binaries are now being built in an automated way via #641. The same proc

Some minor progress on this can be seen in #670 and in the repo https://github.com/JoinMarket-Org/jmcontrolcenter

Much more progress after merge of #996, we have a full RPC-API including a spec definition. There is now more than one independent web interface under development, see https://github.com/joinmarket-webui/jm-web-client and https://github.com/manasgandy/joinmarket-gui.

### Bitcoin

We use coincurve as a binding to libsecp256k1.
~~We use coincurve as a binding to libsecp256k1.~~
~~The current jmbitcoin package morphed over many iterations from the original pybitcointools base code.~~
~~We need to rework it considerably as it is very messy architecturally, particularly in regard to data types.~~
~~A full rewrite is likely the best option, including in particular the removal of data type flexibility; use binary~~
Expand All @@ -88,14 +90,16 @@ We use coincurve as a binding to libsecp256k1.
~~probable need to support taproot and Schnorr (without yet implementing it).~~

This was all done in the switch to [python-bitcointx](https://github.com/Simplexum/python-bitcointx) included in 0.7.0 via #536 .
Complete removal of coincurve for all functions is still to be done.
~~Complete removal of coincurve for all functions is still to be done.~~ Now done via #1134

Taproot support needs to be added, see #1084.


### Extra features.

PayJoin is already implemented, ~~though not in GUI, that could be added.~~ Full BIP78 Payjoin now in GUI also.

Maker functionality is not in GUI, that could quite plausibly be added and is quite widely requested. - See #487, this is now largely functional but still needs work.
Maker functionality is not in GUI, that could quite plausibly be added and is quite widely requested. - See #487, this is now largely functional but still needs work. However this is probably superseded by work on the RPC-API, see above under "Alternative implementations".

~~SNICKER exists currently as a proposed code update but is not quite ready, see #403.~~ "Full" SNICKER functionality is now merged via #768, albeit it will need more testing before it can be auto-switched on for mainnet yieldgenerators.

Expand Down
2 changes: 0 additions & 2 deletions jmbitcoin/jmbitcoin/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import coincurve as secp256k1

# If user has compiled and installed libsecp256k1 via
# JM installation script install.sh, use that;
# if not, it is assumed to be present at the system level
Expand Down
7 changes: 3 additions & 4 deletions jmbitcoin/jmbitcoin/secp256k1_ecies.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import coincurve as secp256k1
import base64
import hmac
import hashlib
import pyaes
import os
import jmbitcoin as btc
from bitcointx.core.key import CPubKey

ECIES_MAGIC_BYTES = b'BIE1'

Expand Down Expand Up @@ -68,9 +68,8 @@ def ecies_decrypt(privkey, encrypted):
if magic != ECIES_MAGIC_BYTES:
raise ECIESDecryptionError()
ephemeral_pubkey = encrypted[4:37]
try:
testR = secp256k1.PublicKey(ephemeral_pubkey)
except:
testR = CPubKey(ephemeral_pubkey)
if not testR.is_fullyvalid():
raise ECIESDecryptionError()
ciphertext = encrypted[37:-32]
mac = encrypted[-32:]
Expand Down
120 changes: 72 additions & 48 deletions jmbitcoin/jmbitcoin/secp256k1_main.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import base64
import struct
import coincurve as secp256k1

from jmbase import bintohex
from bitcointx import base58
from bitcointx.core import Hash
from bitcointx.core.key import CKeyBase
from bitcointx.core.secp256k1 import _secp256k1 as secp_lib
from bitcointx.core.secp256k1 import secp256k1_context_verify
from bitcointx.core.key import CKey, CKeyBase, CPubKey
from bitcointx.signmessage import BitcoinMessage

# This extra function definition, not present in the
# underlying bitcointx library, is to allow
# multiplication of pubkeys by scalars, as is required
# for PoDLE.
import ctypes
secp_lib.secp256k1_ec_pubkey_tweak_mul.restype = ctypes.c_int
secp_lib.secp256k1_ec_pubkey_tweak_mul.argtypes = [ctypes.c_void_p, ctypes.c_char_p, ctypes.c_char_p]

#Required only for PoDLE calculation:
N = 115792089237316195423570985008687907852837564279074904382605163141518161494337

Expand All @@ -18,24 +28,26 @@
"""
def getG(compressed=True):
"""Returns the public key binary
representation of secp256k1 G
representation of secp256k1 G;
note that CPubKey is of type bytes.
"""
priv = b"\x00"*31 + b"\x01"
G = secp256k1.PrivateKey(priv).public_key.format(compressed)
k = CKey(priv, compressed=compressed)
G = k.pub
return G

podle_PublicKey_class = secp256k1.PublicKey
podle_PrivateKey_class = secp256k1.PrivateKey
podle_PublicKey_class = CPubKey
podle_PrivateKey_class = CKey

def podle_PublicKey(P):
"""Returns a PublicKey object from a binary string
"""
return secp256k1.PublicKey(P)
return CPubKey(P)

def podle_PrivateKey(priv):
"""Returns a PrivateKey object from a binary string
"""
return secp256k1.PrivateKey(priv)
return CKey(priv)

def read_privkey(priv):
if len(priv) == 33:
Expand All @@ -51,13 +63,14 @@ def read_privkey(priv):

def privkey_to_pubkey(priv):
'''Take 32/33 byte raw private key as input.
If 32 bytes, return compressed (33 byte) raw public key.
If 33 bytes, read the final byte as compression flag,
and return compressed/uncompressed public key as appropriate.'''
If 32 bytes, return as uncompressed raw public key.
If 33 bytes and the final byte is 01, return
compresse public key. Else throws Exception.'''
compressed, priv = read_privkey(priv)
#secp256k1 checks for validity of key value.
newpriv = secp256k1.PrivateKey(secret=priv)
return newpriv.public_key.format(compressed)
# CKey checks for validity of key value;
# any invalidity throws ValueError.
newpriv = CKey(priv, compressed=compressed)
return newpriv.pub

# b58check wrapper functions around bitcointx.base58 functions:
# (avoids complexity of key management structure)
Expand Down Expand Up @@ -86,9 +99,9 @@ def b58check_to_bin(s):
def get_version_byte(s):
return b58check_to_bin(s)[0]

def ecdsa_sign(msg, priv, formsg=False):
def ecdsa_sign(msg, priv):
hashed_msg = BitcoinMessage(msg).GetHash()
sig = ecdsa_raw_sign(hashed_msg, priv, rawmsg=True, formsg=formsg)
sig = ecdsa_raw_sign(hashed_msg, priv, rawmsg=True)
return base64.b64encode(sig).decode('ascii')

def ecdsa_verify(msg, sig, pub):
Expand All @@ -114,10 +127,10 @@ def is_valid_pubkey(pubkey, require_compressed=False):
valid_uncompressed):
return False
# serialization is valid, but we must ensure it corresponds
# to a valid EC point:
try:
dummy = secp256k1.PublicKey(pubkey)
except:
# to a valid EC point. The CPubKey constructor calls the pubkey_parse
# operation from the libsecp256k1 library:
dummy = CPubKey(pubkey)
if not dummy.is_fullyvalid():
return False
return True

Expand All @@ -131,19 +144,37 @@ def multiply(s, pub, return_serialized=True):
of the scalar s.
('raw' options passed in)
'''
newpub = secp256k1.PublicKey(pub)
#see note to "tweak_mul" function in podle.py
res = newpub.multiply(s)
try:
CKey(s)
except ValueError:
raise ValueError("Invalid tweak for libsecp256k1 "
"multiply: {}".format(bintohex(s)))

pub_obj = CPubKey(pub)
if not pub_obj.is_fullyvalid():
raise ValueError("Invalid pubkey for multiply: {}".format(
bintohex(pub)))

privkey_arg = ctypes.c_char_p(s)
pubkey_buf = pub_obj._to_ctypes_char_array()
ret = secp_lib.secp256k1_ec_pubkey_tweak_mul(
secp256k1_context_verify, pubkey_buf, privkey_arg)
if ret != 1:
assert ret == 0
raise ValueError('Multiplication failed')
if not return_serialized:
return res
return res.format()
return CPubKey._from_ctypes_char_array(pubkey_buf)
return bytes(CPubKey._from_ctypes_char_array(pubkey_buf))

def add_pubkeys(pubkeys):
'''Input a list of binary compressed pubkeys
and return their sum as a binary compressed pubkey.'''
pubkey_list = [secp256k1.PublicKey(x) for x in pubkeys]
r = secp256k1.PublicKey.combine_keys(pubkey_list)
return r.format()
pubkey_list = [CPubKey(x) for x in pubkeys]
if not all([x.is_compressed() for x in pubkey_list]):
raise ValueError("Only compressed pubkeys can be added.")
if not all([x.is_fullyvalid() for x in pubkey_list]):
raise ValueError("Invalid pubkey format.")
return CPubKey.combine(*pubkey_list)

def add_privkeys(priv1, priv2):
'''Add privkey 1 to privkey 2.
Expand All @@ -156,8 +187,7 @@ def add_privkeys(priv1, priv2):
else:
compressed = y[0]
newpriv1, newpriv2 = (y[1], z[1])
p1 = secp256k1.PrivateKey(newpriv1)
res = p1.add(newpriv2).secret
res = CKey.add(CKey(newpriv1), CKey(newpriv2)).secret_bytes
if compressed:
res += b'\x01'
return res
Expand All @@ -167,18 +197,17 @@ def ecdh(privkey, pubkey):
and a pubkey serialized in compressed, binary format (33 bytes),
and output the shared secret as a 32 byte hash digest output.
The exact calculation is:
shared_secret = SHA256(privkey * pubkey)
shared_secret = SHA256(compressed_serialization_of_pubkey(privkey * pubkey))
.. where * is elliptic curve scalar multiplication.
See https://github.com/bitcoin/bitcoin/blob/master/src/secp256k1/src/modules/ecdh/main_impl.h
for implementation details.
"""
secp_privkey = secp256k1.PrivateKey(privkey)
return secp_privkey.ecdh(pubkey)
_, priv = read_privkey(privkey)
return CKey(priv).ECDH(CPubKey(pubkey))

def ecdsa_raw_sign(msg,
priv,
rawmsg=False,
formsg=False):
rawmsg=False):
'''Take the binary message msg and sign it with the private key
priv.
If rawmsg is True, no sha256 hash is applied to msg before signing.
Expand All @@ -188,17 +217,12 @@ def ecdsa_raw_sign(msg,
Return value: the calculated signature.'''
if rawmsg and len(msg) != 32:
raise Exception("Invalid hash input to ECDSA raw sign.")

compressed, p = read_privkey(priv)
newpriv = secp256k1.PrivateKey(p)
if formsg:
sig = newpriv.sign_recoverable(msg)
return sig
newpriv = CKey(p, compressed=compressed)
if rawmsg:
sig = newpriv.sign(msg, _ecdsa_sig_grind_low_r=False)
else:
if rawmsg:
sig = newpriv.sign(msg, hasher=None)
else:
sig = newpriv.sign(msg)
sig = newpriv.sign(Hash(msg), _ecdsa_sig_grind_low_r=False)
return sig

def ecdsa_raw_verify(msg, pub, sig, rawmsg=False):
Expand All @@ -216,12 +240,12 @@ def ecdsa_raw_verify(msg, pub, sig, rawmsg=False):
try:
if rawmsg:
assert len(msg) == 32
newpub = secp256k1.PublicKey(pub)
newpub = CPubKey(pub)
if rawmsg:
retval = newpub.verify(sig, msg, hasher=None)
retval = newpub.verify(msg, sig)
else:
retval = newpub.verify(sig, msg)
except Exception as e:
retval = newpub.verify(Hash(msg), sig)
except Exception:
return False
return retval

Expand Down
19 changes: 13 additions & 6 deletions jmbitcoin/jmbitcoin/snicker.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from jmbitcoin.secp256k1_transaction import *
from collections import Counter

from bitcointx.core.key import CKey, CPubKey

SNICKER_MAGIC_BYTES = b'SNICKER'

# Flags may be added in future versions
Expand All @@ -20,8 +22,10 @@ def snicker_pubkey_tweak(pub, tweak):
Return value is also a 33 byte string serialization
of the resulting pubkey (compressed).
"""
base_pub = secp256k1.PublicKey(pub)
return base_pub.add(tweak).format()
base_pub = CPubKey(pub)
# convert the tweak to a new pubkey
tweak_pub = CKey(tweak, compressed=True).pub
return add_pubkeys([base_pub, tweak_pub])

def snicker_privkey_tweak(priv, tweak):
""" use secp256k1 library to perform tweak.
Expand All @@ -30,10 +34,13 @@ def snicker_privkey_tweak(priv, tweak):
Return value isa 33 byte string serialization
of the resulting private key/secret (with compression flag).
"""
if len(priv) == 33 and priv[-1] == 1:
priv = priv[:-1]
base_priv = secp256k1.PrivateKey(priv)
return base_priv.add(tweak).secret + b'\x01'
if len(priv) == 32:
priv += b"\x01"
if len(tweak) == 32:
tweak += b"\x01"
assert priv[-1] == 1
assert tweak[-1] == 1
return add_privkeys(priv, tweak)

def verify_snicker_output(tx, pub, tweak, spk_type="p2wpkh"):
""" A convenience function to check that one output address in a transaction
Expand Down
2 changes: 1 addition & 1 deletion jmbitcoin/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@
license='GPL',
packages=['jmbitcoin'],
python_requires='>=3.6',
install_requires=['coincurve', 'python-bitcointx>=1.1.1.post0', 'pyaes', 'urldecode'],
install_requires=['python-bitcointx>=1.1.1.post0', 'pyaes', 'urldecode'],
zip_safe=False)
7 changes: 6 additions & 1 deletion jmbitcoin/test/test_ecc_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import jmbitcoin as btc
import binascii
from jmbase import bintohex
import json
import pytest
import os
Expand All @@ -14,7 +15,11 @@ def test_valid_sigs(setup_ecc):
for v in vectors['vectors']:
msg, sig, priv = (binascii.unhexlify(
v[a]) for a in ["msg", "sig", "privkey"])
assert sig == btc.ecdsa_raw_sign(msg, priv, rawmsg=True)+ b'\x01'
res = btc.ecdsa_raw_sign(msg, priv, rawmsg=True)+ b'\x01'
if not sig == res:
print("failed on sig {} from msg {} with priv {}".format(bintohex(sig), bintohex(msg), bintohex(priv)))
print("we got instead: {}".format(bintohex(res)))
assert False
# check that the signature verifies against the key(pair)
pubkey = btc.privkey_to_pubkey(priv)
assert btc.ecdsa_raw_verify(msg, pubkey, sig[:-1], rawmsg=True)
Expand Down
10 changes: 5 additions & 5 deletions jmbitcoin/test/test_ecdh.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#! /usr/bin/env python
'''Tests coincurve binding to libsecp256k1 ecdh module code'''
'''Tests python-bitcointx binding to libsecp256k1 ecdh module code'''

import hashlib
import jmbitcoin as btc
Expand All @@ -15,7 +15,7 @@ def test_ecdh():
2. Calculate the corresponding public keys.
3. Do ECDH on the cartesian product (x, Y), with x private
and Y public keys, for all combinations.
4. Compare the result from CoinCurve with the manual
4. Compare the result from secp256k1_main.ecdh with the manual
multiplication xY following by hash (sha256). Note that
sha256(xY) is the default hashing function used for ECDH
in libsecp256k1.
Expand All @@ -31,15 +31,15 @@ def test_ecdh():
key, hex_key, prop_dict = a
if prop_dict["isPrivkey"]:
c, k = btc.read_privkey(hextobin(hex_key))
extracted_privkeys.append(k)
extracted_privkeys.append(k + b"\x01")
extracted_pubkeys = [btc.privkey_to_pubkey(x) for x in extracted_privkeys]
for p in extracted_privkeys:
for P in extracted_pubkeys:
c, k = btc.read_privkey(p)
shared_secret = btc.ecdh(k, P)
shared_secret = btc.ecdh(k + b"\x01", P)
assert len(shared_secret) == 32
# try recreating the shared secret manually:
pre_secret = btc.multiply(p, P)
pre_secret = btc.multiply(k, P)
derived_secret = hashlib.sha256(pre_secret).digest()
assert derived_secret == shared_secret

Expand Down
Loading

0 comments on commit 537e317

Please sign in to comment.