From 7c929cf5bc75aeb7c39af83324821bcd97ccfba0 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 9 Aug 2016 13:34:58 +1200 Subject: [PATCH 1/6] Add support for spending keys to the basic key store --- src/Makefile.gtest.include | 1 + src/gtest/test_keystore.cpp | 26 ++++++++++++++++++++ src/keystore.cpp | 11 +++++++++ src/keystore.h | 49 +++++++++++++++++++++++++++++++++++++ src/uint252.h | 2 ++ src/zcash/Address.cpp | 6 ++--- src/zcash/Address.hpp | 6 +++-- 7 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 src/gtest/test_keystore.cpp diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index 5b7fa31e2..99fdf1e75 100644 --- a/src/Makefile.gtest.include +++ b/src/Makefile.gtest.include @@ -8,6 +8,7 @@ zcash_gtest_SOURCES = \ gtest/test_checktransaction.cpp \ gtest/test_equihash.cpp \ gtest/test_joinsplit.cpp \ + gtest/test_keystore.cpp \ gtest/test_noteencryption.cpp \ gtest/test_merkletree.cpp \ gtest/test_circuit.cpp \ diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp new file mode 100644 index 000000000..987bdd13a --- /dev/null +++ b/src/gtest/test_keystore.cpp @@ -0,0 +1,26 @@ +#include + +#include "keystore.h" +#include "zcash/Address.hpp" + +TEST(keystore_tests, store_and_retrieve_spending_key) { + CBasicKeyStore keyStore; + + std::set addrs; + keyStore.GetPaymentAddresses(addrs); + ASSERT_EQ(0, addrs.size()); + + auto sk = libzcash::SpendingKey::random(); + keyStore.AddSpendingKey(sk); + + auto addr = sk.address(); + ASSERT_TRUE(keyStore.HaveSpendingKey(addr)); + + libzcash::SpendingKey keyOut; + keyStore.GetSpendingKey(addr, keyOut); + ASSERT_EQ(sk, keyOut); + + keyStore.GetPaymentAddresses(addrs); + ASSERT_EQ(1, addrs.size()); + ASSERT_EQ(1, addrs.count(addr)); +} diff --git a/src/keystore.cpp b/src/keystore.cpp index 3bae24b7b..376654e7f 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -23,6 +23,10 @@ bool CKeyStore::AddKey(const CKey &key) { return AddKeyPubKey(key, key.GetPubKey()); } +bool CKeyStore::AddSpendingKey(const libzcash::SpendingKey &key) { + return AddSpendingKeyPaymentAddress(key, key.address()); +} + bool CBasicKeyStore::AddKeyPubKey(const CKey& key, const CPubKey &pubkey) { LOCK(cs_KeyStore); @@ -83,3 +87,10 @@ bool CBasicKeyStore::HaveWatchOnly() const LOCK(cs_KeyStore); return (!setWatchOnly.empty()); } + +bool CBasicKeyStore::AddSpendingKeyPaymentAddress(const libzcash::SpendingKey& key, const libzcash::PaymentAddress &address) +{ + LOCK(cs_KeyStore); + mapSpendingKeys[address] = key; + return true; +} diff --git a/src/keystore.h b/src/keystore.h index 4a4b6d20a..bbd04f235 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -11,6 +11,7 @@ #include "script/script.h" #include "script/standard.h" #include "sync.h" +#include "zcash/Address.hpp" #include #include @@ -44,11 +45,21 @@ public: virtual bool RemoveWatchOnly(const CScript &dest) =0; virtual bool HaveWatchOnly(const CScript &dest) const =0; virtual bool HaveWatchOnly() const =0; + + //! Add a spending key to the store. + virtual bool AddSpendingKeyPaymentAddress(const libzcash::SpendingKey &key, const libzcash::PaymentAddress &address) =0; + virtual bool AddSpendingKey(const libzcash::SpendingKey &key); + + //! Check whether a spending key corresponding to a given payment address is present in the store. + virtual bool HaveSpendingKey(const libzcash::PaymentAddress &address) const =0; + virtual bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey& keyOut) const =0; + virtual void GetPaymentAddresses(std::set &setAddress) const =0; }; typedef std::map KeyMap; typedef std::map ScriptMap; typedef std::set WatchOnlySet; +typedef std::map SpendingKeyMap; /** Basic key store, that keeps keys in an address->secret map */ class CBasicKeyStore : public CKeyStore @@ -57,6 +68,7 @@ protected: KeyMap mapKeys; ScriptMap mapScripts; WatchOnlySet setWatchOnly; + SpendingKeyMap mapSpendingKeys; public: bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey); @@ -103,6 +115,43 @@ public: virtual bool RemoveWatchOnly(const CScript &dest); virtual bool HaveWatchOnly(const CScript &dest) const; virtual bool HaveWatchOnly() const; + + bool AddSpendingKeyPaymentAddress(const libzcash::SpendingKey &key, const libzcash::PaymentAddress &address); + bool HaveSpendingKey(const libzcash::PaymentAddress &address) const + { + bool result; + { + LOCK(cs_KeyStore); + result = (mapSpendingKeys.count(address) > 0); + } + return result; + } + bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey &keyOut) const + { + { + LOCK(cs_KeyStore); + SpendingKeyMap::const_iterator mi = mapSpendingKeys.find(address); + if (mi != mapSpendingKeys.end()) + { + keyOut = mi->second; + return true; + } + } + return false; + } + void GetPaymentAddresses(std::set &setAddress) const + { + setAddress.clear(); + { + LOCK(cs_KeyStore); + SpendingKeyMap::const_iterator mi = mapSpendingKeys.begin(); + while (mi != mapSpendingKeys.end()) + { + setAddress.insert((*mi).first); + mi++; + } + } + } }; typedef std::vector > CKeyingMaterial; diff --git a/src/uint252.h b/src/uint252.h index c5ab1a380..6281e8533 100644 --- a/src/uint252.h +++ b/src/uint252.h @@ -43,6 +43,8 @@ public: uint256 inner() const { return contents; } + + friend inline bool operator==(const uint252& a, const uint252& b) { return a.contents == b.contents; } }; #endif diff --git a/src/zcash/Address.cpp b/src/zcash/Address.cpp index 446a63db1..9bb32fb6c 100644 --- a/src/zcash/Address.cpp +++ b/src/zcash/Address.cpp @@ -8,7 +8,7 @@ uint256 ViewingKey::pk_enc() { return ZCNoteEncryption::generate_pubkey(*this); } -ViewingKey SpendingKey::viewing_key() { +ViewingKey SpendingKey::viewing_key() const { return ViewingKey(ZCNoteEncryption::generate_privkey(*this)); } @@ -16,8 +16,8 @@ SpendingKey SpendingKey::random() { return SpendingKey(random_uint252()); } -PaymentAddress SpendingKey::address() { +PaymentAddress SpendingKey::address() const { return PaymentAddress(PRF_addr_a_pk(*this), viewing_key().pk_enc()); } -} \ No newline at end of file +} diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 86e351cf7..36b9402a3 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -22,6 +22,8 @@ public: READWRITE(a_pk); READWRITE(pk_enc); } + + friend inline bool operator<(const PaymentAddress& a, const PaymentAddress& b) { return a.a_pk < b.a_pk; } }; class ViewingKey : public uint256 { @@ -38,8 +40,8 @@ public: static SpendingKey random(); - ViewingKey viewing_key(); - PaymentAddress address(); + ViewingKey viewing_key() const; + PaymentAddress address() const; }; } From 0bfdb9628e2329886943e5cb28c46f47419fbf8a Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 12 Aug 2016 14:24:29 +1200 Subject: [PATCH 2/6] Merge AddSpendingKeyPaymentAddress into AddSpendingKey to simplify API --- src/keystore.cpp | 8 ++------ src/keystore.h | 5 ++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index 376654e7f..ef1878a9b 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -23,10 +23,6 @@ bool CKeyStore::AddKey(const CKey &key) { return AddKeyPubKey(key, key.GetPubKey()); } -bool CKeyStore::AddSpendingKey(const libzcash::SpendingKey &key) { - return AddSpendingKeyPaymentAddress(key, key.address()); -} - bool CBasicKeyStore::AddKeyPubKey(const CKey& key, const CPubKey &pubkey) { LOCK(cs_KeyStore); @@ -88,9 +84,9 @@ bool CBasicKeyStore::HaveWatchOnly() const return (!setWatchOnly.empty()); } -bool CBasicKeyStore::AddSpendingKeyPaymentAddress(const libzcash::SpendingKey& key, const libzcash::PaymentAddress &address) +bool CBasicKeyStore::AddSpendingKey(const libzcash::SpendingKey &sk) { LOCK(cs_KeyStore); - mapSpendingKeys[address] = key; + mapSpendingKeys[sk.address()] = sk; return true; } diff --git a/src/keystore.h b/src/keystore.h index bbd04f235..4beba84fe 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -47,8 +47,7 @@ public: virtual bool HaveWatchOnly() const =0; //! Add a spending key to the store. - virtual bool AddSpendingKeyPaymentAddress(const libzcash::SpendingKey &key, const libzcash::PaymentAddress &address) =0; - virtual bool AddSpendingKey(const libzcash::SpendingKey &key); + virtual bool AddSpendingKey(const libzcash::SpendingKey &sk) =0; //! Check whether a spending key corresponding to a given payment address is present in the store. virtual bool HaveSpendingKey(const libzcash::PaymentAddress &address) const =0; @@ -116,7 +115,7 @@ public: virtual bool HaveWatchOnly(const CScript &dest) const; virtual bool HaveWatchOnly() const; - bool AddSpendingKeyPaymentAddress(const libzcash::SpendingKey &key, const libzcash::PaymentAddress &address); + bool AddSpendingKey(const libzcash::SpendingKey &sk); bool HaveSpendingKey(const libzcash::PaymentAddress &address) const { bool result; From b5c06c83b0ca4392469e2253d46902c99a92519c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 12 Aug 2016 14:37:17 +1200 Subject: [PATCH 3/6] Consistent parameter naming --- src/gtest/test_keystore.cpp | 6 +++--- src/keystore.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 987bdd13a..8baf02cab 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -16,9 +16,9 @@ TEST(keystore_tests, store_and_retrieve_spending_key) { auto addr = sk.address(); ASSERT_TRUE(keyStore.HaveSpendingKey(addr)); - libzcash::SpendingKey keyOut; - keyStore.GetSpendingKey(addr, keyOut); - ASSERT_EQ(sk, keyOut); + libzcash::SpendingKey skOut; + keyStore.GetSpendingKey(addr, skOut); + ASSERT_EQ(sk, skOut); keyStore.GetPaymentAddresses(addrs); ASSERT_EQ(1, addrs.size()); diff --git a/src/keystore.h b/src/keystore.h index 4beba84fe..927ebe221 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -51,7 +51,7 @@ public: //! Check whether a spending key corresponding to a given payment address is present in the store. virtual bool HaveSpendingKey(const libzcash::PaymentAddress &address) const =0; - virtual bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey& keyOut) const =0; + virtual bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey& skOut) const =0; virtual void GetPaymentAddresses(std::set &setAddress) const =0; }; @@ -125,14 +125,14 @@ public: } return result; } - bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey &keyOut) const + bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey &skOut) const { { LOCK(cs_KeyStore); SpendingKeyMap::const_iterator mi = mapSpendingKeys.find(address); if (mi != mapSpendingKeys.end()) { - keyOut = mi->second; + skOut = mi->second; return true; } } From 0d7386916de92bd04025c936a0aa9823c97ae1b4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 16 Aug 2016 22:17:33 +1200 Subject: [PATCH 4/6] Add separate lock for SpendingKey key store operations --- src/keystore.cpp | 2 +- src/keystore.h | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index ef1878a9b..7240cd747 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -86,7 +86,7 @@ bool CBasicKeyStore::HaveWatchOnly() const bool CBasicKeyStore::AddSpendingKey(const libzcash::SpendingKey &sk) { - LOCK(cs_KeyStore); + LOCK(cs_SpendingKeyStore); mapSpendingKeys[sk.address()] = sk; return true; } diff --git a/src/keystore.h b/src/keystore.h index 927ebe221..987f32070 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -21,6 +21,7 @@ class CKeyStore { protected: mutable CCriticalSection cs_KeyStore; + mutable CCriticalSection cs_SpendingKeyStore; public: virtual ~CKeyStore() {} @@ -120,7 +121,7 @@ public: { bool result; { - LOCK(cs_KeyStore); + LOCK(cs_SpendingKeyStore); result = (mapSpendingKeys.count(address) > 0); } return result; @@ -128,7 +129,7 @@ public: bool GetSpendingKey(const libzcash::PaymentAddress &address, libzcash::SpendingKey &skOut) const { { - LOCK(cs_KeyStore); + LOCK(cs_SpendingKeyStore); SpendingKeyMap::const_iterator mi = mapSpendingKeys.find(address); if (mi != mapSpendingKeys.end()) { @@ -142,7 +143,7 @@ public: { setAddress.clear(); { - LOCK(cs_KeyStore); + LOCK(cs_SpendingKeyStore); SpendingKeyMap::const_iterator mi = mapSpendingKeys.begin(); while (mi != mapSpendingKeys.end()) { From 04dfc3c569d017adef011eb370c98d5902b0c0cd Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 18 Aug 2016 12:22:30 +1200 Subject: [PATCH 5/6] Rework test to check for failure to return a spending key --- src/gtest/test_keystore.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 8baf02cab..3d095ee56 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -5,19 +5,22 @@ TEST(keystore_tests, store_and_retrieve_spending_key) { CBasicKeyStore keyStore; + libzcash::SpendingKey skOut; std::set addrs; keyStore.GetPaymentAddresses(addrs); ASSERT_EQ(0, addrs.size()); auto sk = libzcash::SpendingKey::random(); - keyStore.AddSpendingKey(sk); - auto addr = sk.address(); - ASSERT_TRUE(keyStore.HaveSpendingKey(addr)); - libzcash::SpendingKey skOut; - keyStore.GetSpendingKey(addr, skOut); + // Sanity-check: we can't get a key we haven't added + ASSERT_FALSE(keyStore.HaveSpendingKey(addr)); + ASSERT_FALSE(keyStore.GetSpendingKey(addr, skOut)); + + keyStore.AddSpendingKey(sk); + ASSERT_TRUE(keyStore.HaveSpendingKey(addr)); + ASSERT_TRUE(keyStore.GetSpendingKey(addr, skOut)); ASSERT_EQ(sk, skOut); keyStore.GetPaymentAddresses(addrs); From a4f4fa8fe95789d7b39318c8aaa78d18f4011133 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 18 Aug 2016 12:25:01 +1200 Subject: [PATCH 6/6] ASSERT -> EXPECT in test to get more info per test run about future regressions --- src/gtest/test_keystore.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 3d095ee56..8587ba49b 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -9,21 +9,21 @@ TEST(keystore_tests, store_and_retrieve_spending_key) { std::set addrs; keyStore.GetPaymentAddresses(addrs); - ASSERT_EQ(0, addrs.size()); + EXPECT_EQ(0, addrs.size()); auto sk = libzcash::SpendingKey::random(); auto addr = sk.address(); // Sanity-check: we can't get a key we haven't added - ASSERT_FALSE(keyStore.HaveSpendingKey(addr)); - ASSERT_FALSE(keyStore.GetSpendingKey(addr, skOut)); + EXPECT_FALSE(keyStore.HaveSpendingKey(addr)); + EXPECT_FALSE(keyStore.GetSpendingKey(addr, skOut)); keyStore.AddSpendingKey(sk); - ASSERT_TRUE(keyStore.HaveSpendingKey(addr)); - ASSERT_TRUE(keyStore.GetSpendingKey(addr, skOut)); - ASSERT_EQ(sk, skOut); + EXPECT_TRUE(keyStore.HaveSpendingKey(addr)); + EXPECT_TRUE(keyStore.GetSpendingKey(addr, skOut)); + EXPECT_EQ(sk, skOut); keyStore.GetPaymentAddresses(addrs); - ASSERT_EQ(1, addrs.size()); - ASSERT_EQ(1, addrs.count(addr)); + EXPECT_EQ(1, addrs.size()); + EXPECT_EQ(1, addrs.count(addr)); }