Auto merge of zcash#2143 - str4d:1997-viewing-keys, r=str4d

Implement incoming viewing keys

Closes zcash#1997.

contrib/zcash-cli.bash-completion

@@ -82,10 +82,14 @@ _zcash_cli() {
                 COMPREPLY=( $( compgen -W "add remove" -- "$cur" ) )
                 return 0
                 ;;
-            fundrawtransaction|getblock|getblockheader|getmempoolancestors|getmempooldescendants|getrawtransaction|gettransaction|listaccounts|listreceivedbyaccount|listreceivedbyaddress|sendrawtransaction|z_importkey)
+            fundrawtransaction|getblock|getblockheader|getmempoolancestors|getmempooldescendants|getrawtransaction|gettransaction|listaccounts|listreceivedbyaccount|listreceivedbyaddress|sendrawtransaction)
                 COMPREPLY=( $( compgen -W "true false" -- "$cur" ) )
                 return 0
                 ;;
+            z_importkey|z_importviewingkey)
+                COMPREPLY=( $( compgen -W "yes no whenkeyisnew" -- "$cur" ) )
+                return 0
+                ;;
             move|setaccount)
                 _zcash_accounts
                 return 0

doc/release-notes.md

@@ -4,3 +4,31 @@ release-notes at release time)
 Notable changes
 ===============
 
+Incoming viewing keys
+---------------------
+
+Support for incoming viewing keys, as described in
+[the Zcash protocol spec](https://github.com/zcash/zips/blob/master/protocol/protocol.pdf),
+has been added to the wallet.
+
+Use the `z_exportviewingkey` RPC method to obtain the incoming viewing key for a
+z-address in a node's wallet. For Sprout z-addresses, these always begin with
+"ZiVK" (or "ZiVt" for testnet z-addresses). Use `z_importviewingkey` to import
+these into another node.
+
+A node that possesses an incoming viewing key for a z-address can view all past
+transactions received by that address, as well as all future transactions sent
+to it, by using `z_listreceivedbyaddress`. They cannot spend any funds from the
+address. This is similar to the behaviour of "watch-only" t-addresses.
+
+`z_gettotalbalance` now has an additional boolean parameter for including the
+balance of "watch-only" addresses (both transparent and shielded), which is set
+to `false` by default. `z_getbalance` has also been updated to work with
+watch-only addresses.
+
+- **Caution:** for z-addresses, these balances will **not** be accurate if any
+  funds have been sent from the address. This is because incoming viewing keys
+  cannot detect spends, and so the "balance" is just the sum of all received
+  notes, including ones that have been spent. Some future use-cases for incoming
+  viewing keys will include synchronization data to keep their balances accurate
+  (e.g. [#2542](https://github.com/zcash/zcash/issues/2542)).

qa/rpc-tests/wallet_nullifiers.py

@@ -170,5 +170,50 @@ def run_test (self):
         assert_equal(self.nodes[1].z_getbalance(myzaddr), zaddrremaining2)
         assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining2)
 
+        # Test viewing keys
+
+        node3mined = Decimal('250.0')
+        assert_equal({k: Decimal(v) for k, v in self.nodes[3].z_gettotalbalance().items()}, {
+            'transparent': node3mined,
+            'private': zsendmany2notevalue,
+            'total': node3mined + zsendmany2notevalue,
+        })
+
+        # add node 1 address and node 2 viewing key to node 3
+        myzvkey = self.nodes[2].z_exportviewingkey(myzaddr)
+        self.nodes[3].importaddress(mytaddr1)
+        self.nodes[3].z_importviewingkey(myzvkey)
+
+        # Check the address has been imported
+        assert_equal(myzaddr in self.nodes[3].z_listaddresses(), False)
+        assert_equal(myzaddr in self.nodes[3].z_listaddresses(True), True)
+
+        # Node 3 should see the same received notes as node 2
+        assert_equal(
+            self.nodes[2].z_listreceivedbyaddress(myzaddr),
+            self.nodes[3].z_listreceivedbyaddress(myzaddr))
+
+        # Node 3's balances should be unchanged without explicitly requesting
+        # to include watch-only balances
+        assert_equal({k: Decimal(v) for k, v in self.nodes[3].z_gettotalbalance().items()}, {
+            'transparent': node3mined,
+            'private': zsendmany2notevalue,
+            'total': node3mined + zsendmany2notevalue,
+        })
+
+        # Wallet can't cache nullifiers for notes received by addresses it only has a
+        # viewing key for, and therefore can't detect spends. So it sees a balance
+        # corresponding to the sum of all notes the address received.
+        # TODO: Fix this during the Sapling upgrade (via #2277)
+        assert_equal({k: Decimal(v) for k, v in self.nodes[3].z_gettotalbalance(1, True).items()}, {
+            'transparent': node3mined + Decimal('1.0'),
+            'private': zsendmany2notevalue + zsendmanynotevalue + zaddrremaining + zaddrremaining2,
+            'total': node3mined + Decimal('1.0') + zsendmany2notevalue + zsendmanynotevalue + zaddrremaining + zaddrremaining2,
+        })
+
+        # Check individual balances reflect the above
+        assert_equal(self.nodes[3].z_getbalance(mytaddr1), Decimal('1.0'))
+        assert_equal(self.nodes[3].z_getbalance(myzaddr), zsendmanynotevalue + zaddrremaining + zaddrremaining2)
+
 if __name__ == '__main__':
     WalletNullifiersTest().main ()

src/base58.cpp

@@ -323,67 +323,60 @@ bool CBitcoinSecret::SetString(const std::string& strSecret)
     return SetString(strSecret.c_str());
 }
 
-bool CZCPaymentAddress::Set(const libzcash::PaymentAddress& addr)
+template<class DATA_TYPE, CChainParams::Base58Type PREFIX, size_t SER_SIZE>
+bool CZCEncoding<DATA_TYPE, PREFIX, SER_SIZE>::Set(const DATA_TYPE& addr)
 {
     CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
     ss << addr;
     std::vector<unsigned char> addrSerialized(ss.begin(), ss.end());
-    assert(addrSerialized.size() == libzcash::SerializedPaymentAddressSize);
-    SetData(Params().Base58Prefix(CChainParams::ZCPAYMENT_ADDRRESS), &addrSerialized[0], libzcash::SerializedPaymentAddressSize);
+    assert(addrSerialized.size() == SER_SIZE);
+    SetData(Params().Base58Prefix(PREFIX), &addrSerialized[0], SER_SIZE);
     return true;
 }
 
-libzcash::PaymentAddress CZCPaymentAddress::Get() const
+template<class DATA_TYPE, CChainParams::Base58Type PREFIX, size_t SER_SIZE>
+DATA_TYPE CZCEncoding<DATA_TYPE, PREFIX, SER_SIZE>::Get() const
 {
-    if (vchData.size() != libzcash::SerializedPaymentAddressSize) {
+    if (vchData.size() != SER_SIZE) {
         throw std::runtime_error(
-            "payment address is invalid"
+            PrependName(" is invalid")
         );
     }
 
-    if (vchVersion != Params().Base58Prefix(CChainParams::ZCPAYMENT_ADDRRESS)) {
+    if (vchVersion != Params().Base58Prefix(PREFIX)) {
         throw std::runtime_error(
-            "payment address is for wrong network type"
+            PrependName(" is for wrong network type")
         );
     }
 
     std::vector<unsigned char> serialized(vchData.begin(), vchData.end());
 
     CDataStream ss(serialized, SER_NETWORK, PROTOCOL_VERSION);
-    libzcash::PaymentAddress ret;
-    ss >> ret;
-    return ret;
-}
-
-bool CZCSpendingKey::Set(const libzcash::SpendingKey& addr)
-{
-    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);
-    ss << addr;
-    std::vector<unsigned char> addrSerialized(ss.begin(), ss.end());
-    assert(addrSerialized.size() == libzcash::SerializedSpendingKeySize);
-    SetData(Params().Base58Prefix(CChainParams::ZCSPENDING_KEY), &addrSerialized[0], libzcash::SerializedSpendingKeySize);
-    return true;
-}
-
-libzcash::SpendingKey CZCSpendingKey::Get() const
-{
-    if (vchData.size() != libzcash::SerializedSpendingKeySize) {
-        throw std::runtime_error(
-            "spending key is invalid"
-        );
-    }
-
-    if (vchVersion != Params().Base58Prefix(CChainParams::ZCSPENDING_KEY)) {
-        throw std::runtime_error(
-            "spending key is for wrong network type"
-        );
-    }
-
-    std::vector<unsigned char> serialized(vchData.begin(), vchData.end());
-
-    CDataStream ss(serialized, SER_NETWORK, PROTOCOL_VERSION);
-    libzcash::SpendingKey ret;
+    DATA_TYPE ret;
     ss >> ret;
     return ret;
 }
 
+// Explicit instantiations for libzcash::PaymentAddress
+template bool CZCEncoding<libzcash::PaymentAddress,
+                          CChainParams::ZCPAYMENT_ADDRRESS,
+                          libzcash::SerializedPaymentAddressSize>::Set(const libzcash::PaymentAddress& addr);
+template libzcash::PaymentAddress CZCEncoding<libzcash::PaymentAddress,
+                                              CChainParams::ZCPAYMENT_ADDRRESS,
+                                              libzcash::SerializedPaymentAddressSize>::Get() const;
+
+// Explicit instantiations for libzcash::ViewingKey
+template bool CZCEncoding<libzcash::ViewingKey,
+                          CChainParams::ZCVIEWING_KEY,
+                          libzcash::SerializedViewingKeySize>::Set(const libzcash::ViewingKey& vk);
+template libzcash::ViewingKey CZCEncoding<libzcash::ViewingKey,
+                                          CChainParams::ZCVIEWING_KEY,
+                                          libzcash::SerializedViewingKeySize>::Get() const;
+
+// Explicit instantiations for libzcash::SpendingKey
+template bool CZCEncoding<libzcash::SpendingKey,
+                          CChainParams::ZCSPENDING_KEY,
+                          libzcash::SerializedSpendingKeySize>::Set(const libzcash::SpendingKey& sk);
+template libzcash::SpendingKey CZCEncoding<libzcash::SpendingKey,
+                                           CChainParams::ZCSPENDING_KEY,
+                                           libzcash::SerializedSpendingKeySize>::Get() const;

src/base58.h

@@ -96,26 +96,48 @@ class CBase58Data
     bool operator> (const CBase58Data& b58) const { return CompareTo(b58) >  0; }
 };
 
-class CZCPaymentAddress : public CBase58Data {
+template<class DATA_TYPE, CChainParams::Base58Type PREFIX, size_t SER_SIZE>
+class CZCEncoding : public CBase58Data {
+protected:
+    virtual std::string PrependName(const std::string& s) const = 0;
+
+public:
+    bool Set(const DATA_TYPE& addr);
+
+    DATA_TYPE Get() const;
+};
+
+class CZCPaymentAddress : public CZCEncoding<libzcash::PaymentAddress, CChainParams::ZCPAYMENT_ADDRRESS, libzcash::SerializedPaymentAddressSize> {
+protected:
+    std::string PrependName(const std::string& s) const { return "payment address" + s; }
+
 public:
-    bool Set(const libzcash::PaymentAddress& addr);
     CZCPaymentAddress() {}
 
     CZCPaymentAddress(const std::string& strAddress) { SetString(strAddress.c_str(), 2); }
     CZCPaymentAddress(const libzcash::PaymentAddress& addr) { Set(addr); }
+};
+
+class CZCViewingKey : public CZCEncoding<libzcash::ViewingKey, CChainParams::ZCVIEWING_KEY, libzcash::SerializedViewingKeySize> {
+protected:
+    std::string PrependName(const std::string& s) const { return "viewing key" + s; }
+
+public:
+    CZCViewingKey() {}
 
-    libzcash::PaymentAddress Get() const;
+    CZCViewingKey(const std::string& strViewingKey) { SetString(strViewingKey.c_str(), 3); }
+    CZCViewingKey(const libzcash::ViewingKey& vk) { Set(vk); }
 };
 
-class CZCSpendingKey : public CBase58Data {
+class CZCSpendingKey : public CZCEncoding<libzcash::SpendingKey, CChainParams::ZCSPENDING_KEY, libzcash::SerializedSpendingKeySize> {
+protected:
+    std::string PrependName(const std::string& s) const { return "spending key" + s; }
+
 public:
-    bool Set(const libzcash::SpendingKey& addr);
     CZCSpendingKey() {}
 
     CZCSpendingKey(const std::string& strAddress) { SetString(strAddress.c_str(), 2); }
     CZCSpendingKey(const libzcash::SpendingKey& addr) { Set(addr); }
-
-    libzcash::SpendingKey Get() const;
 };
 
 /** base58-encoded Bitcoin addresses.

src/chainparams.cpp

@@ -110,6 +110,8 @@ class CMainParams : public CChainParams {
         base58Prefixes[EXT_SECRET_KEY]     = {0x04,0x88,0xAD,0xE4};
         // guarantees the first 2 characters, when base58 encoded, are "zc"
         base58Prefixes[ZCPAYMENT_ADDRRESS] = {0x16,0x9A};
+        // guarantees the first 4 characters, when base58 encoded, are "ZiVK"
+        base58Prefixes[ZCVIEWING_KEY]      = {0xA8,0xAB,0xD3};
         // guarantees the first 2 characters, when base58 encoded, are "SK"
         base58Prefixes[ZCSPENDING_KEY]     = {0xAB,0x36};
 
@@ -241,6 +243,8 @@ class CTestNetParams : public CMainParams {
         base58Prefixes[EXT_SECRET_KEY]     = {0x04,0x35,0x83,0x94};
         // guarantees the first 2 characters, when base58 encoded, are "zt"
         base58Prefixes[ZCPAYMENT_ADDRRESS] = {0x16,0xB6};
+        // guarantees the first 4 characters, when base58 encoded, are "ZiVt"
+        base58Prefixes[ZCVIEWING_KEY]      = {0xA8,0xAC,0x0C};
         // guarantees the first 2 characters, when base58 encoded, are "ST"
         base58Prefixes[ZCSPENDING_KEY]     = {0xAC,0x08};
 

src/chainparams.h

@@ -44,6 +44,7 @@ class CChainParams
 
         ZCPAYMENT_ADDRRESS,
         ZCSPENDING_KEY,
+        ZCVIEWING_KEY,
 
         MAX_BASE58_TYPES
     };

src/gtest/test_joinsplit.cpp

@@ -89,7 +89,7 @@ void test_full_api(ZCJoinSplit* js)
     // Recipient should decrypt
     // Now the recipient should spend the money again
     auto h_sig = js->h_sig(randomSeed, nullifiers, pubKeyHash);
-    ZCNoteDecryption decryptor(recipient_key.viewing_key());
+    ZCNoteDecryption decryptor(recipient_key.receiving_key());
 
     auto note_pt = NotePlaintext::decrypt(
         decryptor,

src/gtest/test_keystore.cpp

@@ -43,7 +43,64 @@ TEST(keystore_tests, store_and_retrieve_note_decryptor) {
 
     keyStore.AddSpendingKey(sk);
     EXPECT_TRUE(keyStore.GetNoteDecryptor(addr, decOut));
-    EXPECT_EQ(ZCNoteDecryption(sk.viewing_key()), decOut);
+    EXPECT_EQ(ZCNoteDecryption(sk.receiving_key()), decOut);
+}
+
+TEST(keystore_tests, StoreAndRetrieveViewingKey) {
+    CBasicKeyStore keyStore;
+    libzcash::ViewingKey vkOut;
+    libzcash::SpendingKey skOut;
+    ZCNoteDecryption decOut;
+
+    auto sk = libzcash::SpendingKey::random();
+    auto vk = sk.viewing_key();
+    auto addr = sk.address();
+
+    // Sanity-check: we can't get a viewing key we haven't added
+    EXPECT_FALSE(keyStore.HaveViewingKey(addr));
+    EXPECT_FALSE(keyStore.GetViewingKey(addr, vkOut));
+
+    // and we shouldn't have a spending key or decryptor either
+    EXPECT_FALSE(keyStore.HaveSpendingKey(addr));
+    EXPECT_FALSE(keyStore.GetSpendingKey(addr, skOut));
+    EXPECT_FALSE(keyStore.GetNoteDecryptor(addr, decOut));
+
+    // and we can't find it in our list of addresses
+    std::set<libzcash::PaymentAddress> addresses;
+    keyStore.GetPaymentAddresses(addresses);
+    EXPECT_FALSE(addresses.count(addr));
+
+    keyStore.AddViewingKey(vk);
+    EXPECT_TRUE(keyStore.HaveViewingKey(addr));
+    EXPECT_TRUE(keyStore.GetViewingKey(addr, vkOut));
+    EXPECT_EQ(vk, vkOut);
+
+    // We should still not have the spending key...
+    EXPECT_FALSE(keyStore.HaveSpendingKey(addr));
+    EXPECT_FALSE(keyStore.GetSpendingKey(addr, skOut));
+
+    // ... but we should have a decryptor
+    EXPECT_TRUE(keyStore.GetNoteDecryptor(addr, decOut));
+    EXPECT_EQ(ZCNoteDecryption(sk.receiving_key()), decOut);
+
+    // ... and we should find it in our list of addresses
+    addresses.clear();
+    keyStore.GetPaymentAddresses(addresses);
+    EXPECT_TRUE(addresses.count(addr));
+
+    keyStore.RemoveViewingKey(vk);
+    EXPECT_FALSE(keyStore.HaveViewingKey(addr));
+    EXPECT_FALSE(keyStore.GetViewingKey(addr, vkOut));
+    EXPECT_FALSE(keyStore.HaveSpendingKey(addr));
+    EXPECT_FALSE(keyStore.GetSpendingKey(addr, skOut));
+    addresses.clear();
+    keyStore.GetPaymentAddresses(addresses);
+    EXPECT_FALSE(addresses.count(addr));
+
+    // We still have a decryptor because those are cached in memory
+    // (and also we only remove viewing keys when adding a spending key)
+    EXPECT_TRUE(keyStore.GetNoteDecryptor(addr, decOut));
+    EXPECT_EQ(ZCNoteDecryption(sk.receiving_key()), decOut);
 }
 
 #ifdef ENABLE_WALLET
@@ -72,13 +129,13 @@ TEST(keystore_tests, store_and_retrieve_spending_key_in_encrypted_store) {
     ASSERT_TRUE(keyStore.GetSpendingKey(addr, keyOut));
     ASSERT_EQ(sk, keyOut);
     EXPECT_TRUE(keyStore.GetNoteDecryptor(addr, decOut));
-    EXPECT_EQ(ZCNoteDecryption(sk.viewing_key()), decOut);
+    EXPECT_EQ(ZCNoteDecryption(sk.receiving_key()), decOut);
 
     ASSERT_TRUE(keyStore.EncryptKeys(vMasterKey));
     ASSERT_TRUE(keyStore.HaveSpendingKey(addr));
     ASSERT_FALSE(keyStore.GetSpendingKey(addr, keyOut));
     EXPECT_TRUE(keyStore.GetNoteDecryptor(addr, decOut));
-    EXPECT_EQ(ZCNoteDecryption(sk.viewing_key()), decOut);
+    EXPECT_EQ(ZCNoteDecryption(sk.receiving_key()), decOut);
 
     // Unlocking with a random key should fail
     uint256 r2 {GetRandHash()};
@@ -109,19 +166,19 @@ TEST(keystore_tests, store_and_retrieve_spending_key_in_encrypted_store) {
     ASSERT_TRUE(keyStore.GetSpendingKey(addr2, keyOut));
     ASSERT_EQ(sk2, keyOut);
     EXPECT_TRUE(keyStore.GetNoteDecryptor(addr2, decOut));
-    EXPECT_EQ(ZCNoteDecryption(sk2.viewing_key()), decOut);
+    EXPECT_EQ(ZCNoteDecryption(sk2.receiving_key()), decOut);
 
     ASSERT_TRUE(keyStore.Lock());
     ASSERT_TRUE(keyStore.HaveSpendingKey(addr2));
     ASSERT_FALSE(keyStore.GetSpendingKey(addr2, keyOut));
     EXPECT_TRUE(keyStore.GetNoteDecryptor(addr2, decOut));
-    EXPECT_EQ(ZCNoteDecryption(sk2.viewing_key()), decOut);
+    EXPECT_EQ(ZCNoteDecryption(sk2.receiving_key()), decOut);
 
     ASSERT_TRUE(keyStore.Unlock(vMasterKey));
     ASSERT_TRUE(keyStore.GetSpendingKey(addr2, keyOut));
     ASSERT_EQ(sk2, keyOut);
     EXPECT_TRUE(keyStore.GetNoteDecryptor(addr2, decOut));
-    EXPECT_EQ(ZCNoteDecryption(sk2.viewing_key()), decOut);
+    EXPECT_EQ(ZCNoteDecryption(sk2.receiving_key()), decOut);
 
     keyStore.GetPaymentAddresses(addrs);
     ASSERT_EQ(2, addrs.size());