Cache bug

README.rst

@@ -36,6 +36,69 @@ will be used to reward developers for bugfixes.
 | |Donate BTC|                         | |Donate DOGE|                        |
 +--------------------------------------+--------------------------------------+
 
+NOTICE
+======
+
+** **BUG NOTICE** **
+
+Versions of ``bitmerchant`` prior to ``0.1.8`` contained a caching bug that may
+have resulted in calls to ``bip32.Wallet.get_child`` to return incorrect results.
+The steps to reproduce the bug are unlikely and do not match the typical
+usage patterns of ``bitmerchant``.
+
+**At this time, no users are known to have been affected by this bug.**
+
+If you have been affected by this bug and need help recovering any lost or
+misplaced coins, please contact me directly at
+steven.buss+bitmerchant@gmail.com.
+
+The affected versions of ``bitmerchant`` have been removed from pypi. They
+have not been untagged in git.
+
+The two possible failure scenarios are: misplaced coins and stolen coins
+
+Misplaced Coins
+---------------
+
+This is still unlikely, but slightly more likely than having your coins stolen.
+
+In order to have misplaced coins as a result of the bug, all of the below
+points must be true:
+
+#. Your master private key must be available for your code to load, rather than in a secure offline backup
+#. You call ``get_child`` directly, rather than ``create_new_address_for_user``
+#. You call ``get_child(n, is_prime=False)`` and ``get_child(n, is_prime=True)``
+    #. in the same python process
+    #. on the same wallet object
+    #. you display the public address of the second ``get_child`` call (in whichever order)
+
+In this case, the bug would have resulted in the first ``get_child``'s address
+being shown. You can easily recover these misplaced coins by updating to
+``bitmerchant>=0.1.8``, regenerating the address you accidentally sent coins
+to, and moving them to a corrected destination. The "deterministic" part of
+"hierarchical deterministic wallets" really works to your advantage here.
+
+Stolen Coins
+------------
+
+First, it is extremely unlikely that your code met all of the requirements
+to be affected by this bug. If you can answer "yes" to every one of the points
+below, then you should upgrade to ``bitmerchant>=0.1.8``, generate a new master
+private key, and move all coins to the new wallet as soon as possible.
+
+In order to have coins stolen as a result of the bug, all of the below points
+must be true:
+
+#. You expose your master public key to the public
+#. Your master private key must be available for your code to load, rather than in a secure offline backup
+#. You call ``get_child`` directly, rather than ``create_new_address_for_user``
+#. You call ``get_child(n, is_prime=False)`` and ``get_child(n, is_prime=True)``
+    #. in that order
+    #. in the same python process
+    #. on the same wallet object
+    #. with the intention of only giving the prime child to the user
+#. You give the public and private keys of child wallets to users
+
 Installation
 ============
 

bitmerchant/wallet/bip32.py

@@ -6,6 +6,7 @@
 
 import base58
 from os import urandom
+from cachetools.func import lru_cache
 from ecdsa import SECP256k1
 from ecdsa.ecdsa import Public_key as _ECDSA_Public_key
 from ecdsa.ellipticcurve import INFINITY
@@ -24,7 +25,6 @@
 from .utils import is_hex_string
 from .utils import long_or_int
 from .utils import long_to_hex
-from .utils import memoize
 
 
 class Wallet(object):
@@ -243,7 +243,7 @@ def get_child_for_path(self, path):
             return child.public_copy()
         return child
 
-    @memoize
+    @lru_cache(maxsize=1024)
     def get_child(self, child_number, is_prime=None, as_private=True):
         """Derive a child key.
 
@@ -494,7 +494,6 @@ def to_address(self):
         return ensure_str(base58.b58encode_check(network_hash160_bytes))
 
     @classmethod
-    @memoize
     def deserialize(cls, key, network=BitcoinMainNet):
         """Load the ExtendedBip32Key from a hex key.
 

bitmerchant/wallet/keys.py

@@ -20,7 +20,6 @@
 from .utils import is_hex_string
 from .utils import long_or_int
 from .utils import long_to_hex
-from .utils import memoize
 
 
 PublicPair = namedtuple("PublicPair", ["x", "y"])
@@ -60,7 +59,6 @@ def get_key(self):
         """Get the key - a hex formatted private exponent for the curve."""
         return ensure_bytes(hexlify(self._private_key.to_string()))
 
-    @memoize
     def get_public_key(self):
         """Get the PublicKey for this PrivateKey."""
         return PublicKey.from_verifying_key(
@@ -299,7 +297,6 @@ def from_hex_key(cls, key, network=BitcoinMainNet):
         return cls.from_public_pair(public_pair, network=network,
                                     compressed=compressed)
 
-    @memoize
     def create_point(self, x, y):
         """Create an ECDSA point on the SECP256k1 curve with the given coords.
 

bitmerchant/wallet/utils.py

@@ -1,4 +1,3 @@
-from functools import wraps
 import hashlib
 from hashlib import sha256
 import re
@@ -55,15 +54,3 @@ def long_to_hex(l, size):
 
 def long_or_int(val, *args):
     return long(val, *args)
-
-
-def memoize(f):
-    """Memoization decorator for a function taking one or more arguments."""
-    def _c(*args, **kwargs):
-        if not hasattr(f, 'cache'):
-            f.cache = dict()
-        key = (args, tuple(kwargs))
-        if key not in f.cache:
-            f.cache[key] = f(*args, **kwargs)
-        return f.cache[key]
-    return wraps(f)(_c)

requirements.txt

@@ -1,3 +1,4 @@
 base58>=0.2.1
 ecdsa>=0.10
 six>=1.5.2
+cachetools>=1.1.1

setup.py

@@ -81,5 +81,6 @@ def load_version():
         'base58>=0.2.1',
         'ecdsa>=0.10',
         'six>=1.5.2',
+        'cachetools>=1.1.1',
     ]
 )

tests/test_bip32.py

@@ -107,6 +107,17 @@ def test_public_export_mismatch(self):
         self.assertEqual(child.private_key, None)
         self.assertRaises(ValueError, child.serialize)
 
+    def test_prime_not_prime(self):
+        prime_child = self.master_key.get_child(0, is_prime=True)
+        not_prime_child = self.master_key.get_child(0, is_prime=False)
+        self.assertNotEqual(prime_child, not_prime_child)
+
+        pub = self.master_key.public_copy()
+        not_prime_pub = pub.get_child(0, is_prime=False)
+        self.assertNotEqual(not_prime_child, not_prime_pub)
+        self.assertEqual(not_prime_child.public_key, not_prime_pub.public_key)
+        self.assertRaises(ValueError, pub.get_child, 0, is_prime=True)
+
     def test_random_wallet(self):
         w = Wallet.new_random_wallet()
         self.assertTrue(Wallet.deserialize(w.serialize()), w)