Remove coincurve dependency, use python-bitcointx

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.

docs/TODO.md

@@ -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.
 
@@ -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~~
@@ -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.
 

jmbitcoin/jmbitcoin/__init__.py

@@ -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

jmbitcoin/jmbitcoin/secp256k1_ecies.py

@@ -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'
 
@@ -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:]

jmbitcoin/jmbitcoin/secp256k1_main.py

@@ -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
 
@@ -18,24 +28,26 @@ BTC_P2SH_VBYTE = {"mainnet": b'\x05', "testnet": b'\xc4', "signet": b'\xc4'}
 """
 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:
@@ -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)
@@ -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):
@@ -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
 
@@ -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.
@@ -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
@@ -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.
@@ -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):
@@ -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
 

jmbitcoin/jmbitcoin/snicker.py

@@ -8,6 +8,8 @@ from jmbitcoin.secp256k1_main import *
 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
@@ -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.
@@ -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

jmbitcoin/setup.py

@@ -10,5 +10,5 @@ setup(name='joinmarketbitcoin',
       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)

jmbitcoin/test/test_ecc_signing.py

@@ -4,6 +4,7 @@ signature conversion.'''
 
 import jmbitcoin as btc
 import binascii
+from jmbase import bintohex
 import json
 import pytest
 import os
@@ -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)

jmbitcoin/test/test_ecdh.py

@@ -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
@@ -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.
@@ -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
 

jmbitcoin/test/test_ecies.py

@@ -10,18 +10,9 @@ import json
 testdir = os.path.dirname(os.path.realpath(__file__))
 
 def test_ecies():
-    """Using private key test vectors from Bitcoin Core.
-    1. Import a set of private keys from the json file.
-    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
-    multiplication xY following by hash (sha256). Note that
-    sha256(xY) is the default hashing function used for ECDH
-    in libsecp256k1.
-
-    Since there are about 20 private keys in the json file, this
-    creates around 400 test cases (note xX is still valid).
+    """Tests encryption and decryption of random messages using
+    the ECIES module.
+    TODO these tests are very minimal.
     """
     with open(os.path.join(testdir,"base58_keys_valid.json"), "r") as f:
         json_data = f.read()

jmclient/jmclient/podle.py

@@ -58,7 +58,7 @@ class PoDLE(object):
             if len(priv) == 33 and priv[-1:] == b"\x01":
                 priv = priv[:-1]
             self.priv = podle_PrivateKey(priv)
-            self.P = self.priv.public_key
+            self.P = self.priv.pub
         if P2:
             self.P2 = podle_PublicKey(P2)
         else:
@@ -81,7 +81,7 @@ class PoDLE(object):
             raise PoDLEError("Cannot construct commitment, no P2 available")
         if not isinstance(self.P2, podle_PublicKey_class):
             raise PoDLEError("Cannot construct commitment, P2 is not a pubkey")
-        self.commitment = hashlib.sha256(self.P2.format()).digest()
+        self.commitment = hashlib.sha256(self.P2).digest()
         return self.commitment
 
     def generate_podle(self, index=0, k=None):
@@ -118,14 +118,13 @@ class PoDLE(object):
         if not k:
             k = os.urandom(32)
         J = getNUMS(self.i)
-        KG = podle_PrivateKey(k).public_key
-        KJ = multiply(k, J.format(), return_serialized=False)
+        KG = podle_PrivateKey(k).pub
+        KJ = multiply(k, J, return_serialized=False)
         self.P2 = getP2(self.priv, J)
         self.get_commitment()
-        self.e = hashlib.sha256(b''.join([x.format(
-        ) for x in [KG, KJ, self.P, self.P2]])).digest()
+        self.e = hashlib.sha256(b''.join([KG, KJ, self.P, self.P2])).digest()
         k_int, priv_int, e_int = (int.from_bytes(x,
-            byteorder="big") for x in [k, self.priv.secret, self.e])
+            byteorder="big") for x in [k, self.priv.secret_bytes, self.e])
         sig_int = (k_int + priv_int * e_int) % N
         self.s = (sig_int).to_bytes(32, byteorder="big")
         return self.reveal()
@@ -140,8 +139,8 @@ class PoDLE(object):
             self.get_commitment()
         return {'used': self.used,
                 'utxo': self.u,
-                'P': self.P.format(),
-                'P2': self.P2.format(),
+                'P': self.P,
+                'P2': self.P2,
                 'commit': self.commitment,
                 'sig': self.s,
                 'e': self.e}
@@ -184,17 +183,17 @@ class PoDLE(object):
 
         for J in [getNUMS(i) for i in index_range]:
             sig_priv = podle_PrivateKey(self.s)
-            sG = sig_priv.public_key
-            sJ = multiply(self.s, J.format())
+            sG = sig_priv.pub
+            sJ = multiply(self.s, J)
             e_int = int.from_bytes(self.e, byteorder="big")
             minus_e = (-e_int % N).to_bytes(32, byteorder="big")
-            minus_e_P = multiply(minus_e, self.P.format())
-            minus_e_P2 = multiply(minus_e, self.P2.format())
-            KGser = add_pubkeys([sG.format(), minus_e_P])
+            minus_e_P = multiply(minus_e, self.P)
+            minus_e_P2 = multiply(minus_e, self.P2)
+            KGser = add_pubkeys([sG, minus_e_P])
             KJser = add_pubkeys([sJ, minus_e_P2])
             #check 2: e =?= H(K_G || K_J || P || P2)
-            e_check = hashlib.sha256(KGser + KJser + self.P.format() +
-                                     self.P2.format()).digest()
+            e_check = hashlib.sha256(KGser + KJser + self.P +
+                                     self.P2).digest()
             if e_check == self.e:
                 return True
         #commitment fails for any NUMS in the provided range
@@ -245,6 +244,9 @@ def getNUMS(index=0):
             claimed_point = b"\x02" + hashed_seed
             try:
                 nums_point = podle_PublicKey(claimed_point)
+                # CPubKey does not throw ValueError or otherwise
+                # on invalid initialization data; it must be inspected:
+                assert nums_point.is_fullyvalid()
                 return nums_point
             except:
                 continue
@@ -260,7 +262,7 @@ def verify_all_NUMS(write=False):
     """
     nums_points = {}
     for i in range(256):
-        nums_points[i] = bintohex(getNUMS(i).format())
+        nums_points[i] = bintohex(getNUMS(i))
     if write:
         with open("nums_basepoints.txt", "wb") as f:
             from pprint import pformat
@@ -276,9 +278,9 @@ def getP2(priv, nums_pt):
     just the most easy way to manipulate it in the
     library), calculate priv*nums_pt
     """
-    priv_raw = priv.secret
+    priv_raw = priv.secret_bytes
     return multiply(priv_raw,
-                    nums_pt.format(),
+                    nums_pt,
                     return_serialized=False)
 
 # functions which interact with the external persistence of podle data: