diff --git a/qa/rpc-tests/wallet_accounts.py b/qa/rpc-tests/wallet_accounts.py index 15f097d78..34a1e7fd5 100755 --- a/qa/rpc-tests/wallet_accounts.py +++ b/qa/rpc-tests/wallet_accounts.py @@ -55,7 +55,7 @@ class WalletAccountsTest(BitcoinTestFramework): # Generate the first address for account 0. addr0 = self.nodes[0].z_getaddressforaccount(0) assert_equal(addr0['account'], 0) - assert_equal(set(addr0['pools']), set(['transparent', 'sapling'])) + assert_equal(set(addr0['pools']), set(['transparent', 'sapling', 'orchard'])) ua0 = addr0['unifiedaddress'] # We pick mnemonic phrases to ensure that we can always generate the default @@ -70,16 +70,38 @@ class WalletAccountsTest(BitcoinTestFramework): 'no address at diversifier index 0', self.nodes[0].z_getaddressforaccount, 0, [], 0) + # The second address for account 0 is different to the first address. + addr0_2 = self.nodes[0].z_getaddressforaccount(0) + assert_equal(addr0_2['account'], 0) + assert_equal(set(addr0_2['pools']), set(['transparent', 'sapling', 'orchard'])) + ua0_2 = addr0_2['unifiedaddress'] + assert(ua0 != ua0_2) + + # We can generate a fully-shielded address. + addr0_3 = self.nodes[0].z_getaddressforaccount(0, ['sapling', 'orchard']) + assert_equal(addr0_3['account'], 0) + assert_equal(set(addr0_3['pools']), set(['sapling', 'orchard'])) + ua0_3 = addr0_3['unifiedaddress'] + + # We can generate an address without a Sapling receiver. + addr0_4 = self.nodes[0].z_getaddressforaccount(0, ['transparent', 'orchard']) + assert_equal(addr0_4['account'], 0) + assert_equal(set(addr0_4['pools']), set(['transparent', 'orchard'])) + ua0_4 = addr0_4['unifiedaddress'] + # The first address for account 1 is different to account 0. addr1 = self.nodes[0].z_getaddressforaccount(1) assert_equal(addr1['account'], 1) - assert_equal(set(addr1['pools']), set(['transparent', 'sapling'])) + assert_equal(set(addr1['pools']), set(['transparent', 'sapling', 'orchard'])) ua1 = addr1['unifiedaddress'] assert(ua0 != ua1) # The UA contains the expected receiver kinds. - self.check_receiver_types(ua0, ['transparent', 'sapling']) - self.check_receiver_types(ua1, ['transparent', 'sapling']) + self.check_receiver_types(ua0, ['transparent', 'sapling', 'orchard']) + self.check_receiver_types(ua0_2, ['transparent', 'sapling', 'orchard']) + self.check_receiver_types(ua0_3, [ 'sapling', 'orchard']) + self.check_receiver_types(ua0_4, ['transparent', 'orchard']) + self.check_receiver_types(ua1, ['transparent', 'sapling', 'orchard']) # The balances of the accounts are all zero. self.check_balance(0, 0, ua0, {}) diff --git a/qa/rpc-tests/wallet_listreceived.py b/qa/rpc-tests/wallet_listreceived.py index fd67ebb76..ec809eec3 100755 --- a/qa/rpc-tests/wallet_listreceived.py +++ b/qa/rpc-tests/wallet_listreceived.py @@ -381,9 +381,10 @@ class ListReceivedTest (BitcoinTestFramework): r = node.z_getaddressforaccount(account) unified_addr = r['unifiedaddress'] receivers = node.z_listunifiedreceivers(unified_addr) - assert_equal(len(receivers), 2) + assert_equal(len(receivers), 3) assert 'transparent' in receivers assert 'sapling' in receivers + assert 'orchard' in receivers # Wallet contains no notes r = node.z_listreceivedbyaddress(unified_addr, 0) assert_equal(len(r), 0, "unified_addr should have received zero notes") diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index c99b504d6..5e9863cc1 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -578,6 +578,35 @@ TEST(KeystoreTests, StoreAndRetrieveUFVK) { EXPECT_FALSE(ufvkmeta.value().second.has_value()); } +TEST(KeystoreTests, StoreAndRetrieveUFVKByOrchard) { + SelectParams(CBaseChainParams::TESTNET); + CBasicKeyStore keyStore; + + auto seed = MnemonicSeed::Random(SLIP44_TESTNET_TYPE); + auto usk = ZcashdUnifiedSpendingKey::ForAccount(seed, SLIP44_TESTNET_TYPE, 0); + EXPECT_TRUE(usk.has_value()); + + auto ufvk = usk.value().ToFullViewingKey(); + auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), ufvk); + auto ufvkid = zufvk.GetKeyID(); + EXPECT_FALSE(keyStore.GetUnifiedFullViewingKey(ufvkid).has_value()); + + EXPECT_TRUE(keyStore.AddUnifiedFullViewingKey(zufvk)); + EXPECT_EQ(keyStore.GetUnifiedFullViewingKey(ufvkid).value(), zufvk); + + auto addrPair = std::get>(zufvk.FindAddress(diversifier_index_t(0), {ReceiverType::Orchard})); + EXPECT_TRUE(addrPair.first.GetOrchardReceiver().has_value()); + auto orchardReceiver = addrPair.first.GetOrchardReceiver().value(); + + // We don't store Orchard addresses in CBasicKeyStore (the addr -> ivk + // mapping is stored in the Rust wallet), but we still detect this because + // we trial-decrypt diversifiers (which also means we learn the index). + auto ufvkmetaUnadded = keyStore.GetUFVKMetadataForReceiver(orchardReceiver); + EXPECT_TRUE(ufvkmetaUnadded.has_value()); + EXPECT_EQ(ufvkmetaUnadded.value().first, ufvkid); + EXPECT_EQ(ufvkmetaUnadded.value().second.value(), addrPair.second); +} + TEST(KeystoreTests, AddTransparentReceiverForUnifiedAddress) { SelectParams(CBaseChainParams::TESTNET); CBasicKeyStore keyStore; diff --git a/src/rust/src/wallet.rs b/src/rust/src/wallet.rs index 7ebeb4a54..2ff15f336 100644 --- a/src/rust/src/wallet.rs +++ b/src/rust/src/wallet.rs @@ -73,8 +73,12 @@ impl KeyStore { } pub fn add_full_viewing_key(&mut self, fvk: FullViewingKey) { - let ivk = IncomingViewingKey::from(&fvk); - self.viewing_keys.insert(ivk, fvk); + // When we add a full viewing key, we need to add both the internal and external + // incoming viewing keys. + let external_ivk = IncomingViewingKey::from(&fvk); + let internal_ivk = IncomingViewingKey::from(&fvk.derive_internal()); + self.viewing_keys.insert(external_ivk, fvk.clone()); + self.viewing_keys.insert(internal_ivk, fvk); } pub fn add_spending_key(&mut self, sk: SpendingKey) { diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index dc0123cac..a745c5cef 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -44,6 +44,10 @@ class TestWallet : public CWallet { public: TestWallet(const CChainParams& params) : CWallet(params) { } + OrchardWallet& GetOrchardWallet() { + return orchardWallet; + } + bool EncryptKeys(CKeyingMaterial& vMasterKeyIn) { return CCryptoKeyStore::EncryptKeys(vMasterKeyIn); } @@ -2245,3 +2249,43 @@ TEST(WalletTests, GenerateUnifiedAddress) { // Revert to default RegtestDeactivateSapling(); } + +TEST(WalletTests, GenerateUnifiedSpendingKeyAddsOrchardAddresses) { + (void) RegtestActivateSapling(); + TestWallet wallet(Params()); + wallet.GenerateNewSeed(); + + // Create an account. + auto ufvkpair = wallet.GenerateNewUnifiedSpendingKey(); + auto ufvk = ufvkpair.first; + auto account = ufvkpair.second; + auto fvk = ufvk.GetOrchardKey(); + EXPECT_TRUE(fvk.has_value()); + + // At this point the Orchard wallet should contain the change address, but + // no other addresses. We detect this by trying to look up their IVKs. + auto changeAddr = fvk->ToInternalIncomingViewingKey().Address(libzcash::diversifier_index_t{0}); + auto internalIvk = wallet.GetOrchardWallet().GetIncomingViewingKeyForAddress(changeAddr); + EXPECT_TRUE(internalIvk.has_value()); + EXPECT_EQ(internalIvk.value(), fvk->ToInternalIncomingViewingKey()); + auto externalAddr = fvk->ToIncomingViewingKey().Address(libzcash::diversifier_index_t{0}); + auto externalIvk = wallet.GetOrchardWallet().GetIncomingViewingKeyForAddress(externalAddr); + EXPECT_FALSE(externalIvk.has_value()); + + // Generate an address. + auto uaResult = wallet.GenerateUnifiedAddress(account, {ReceiverType::Orchard}); + auto ua = std::get_if>(&uaResult); + EXPECT_NE(ua, nullptr); + + auto uaOrchardReceiver = ua->first.GetOrchardReceiver(); + EXPECT_TRUE(uaOrchardReceiver.has_value()); + EXPECT_EQ(uaOrchardReceiver.value(), externalAddr); + + // Now we can look up the external IVK. + externalIvk = wallet.GetOrchardWallet().GetIncomingViewingKeyForAddress(externalAddr); + EXPECT_TRUE(externalIvk.has_value()); + EXPECT_EQ(externalIvk.value(), fvk->ToIncomingViewingKey()); + + // Revert to default + RegtestDeactivateSapling(); +} diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index aa99bb0cb..fd8cc05df 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3119,14 +3119,16 @@ UniValue z_getaddressforaccount(const UniValue& params, bool fHelp) receivers.insert(ReceiverType::P2PKH); } else if (p == "sapling") { receivers.insert(ReceiverType::Sapling); + } else if (p == "orchard") { + receivers.insert(ReceiverType::Orchard); } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, "pool arguments must be \"transparent\", or \"sapling\""); + throw JSONRPCError(RPC_INVALID_PARAMETER, "pool arguments must be \"transparent\", \"sapling\", or \"orchard\""); } } } if (receivers.empty()) { // Default is the best and second-best shielded pools, and the transparent pool. - receivers = {ReceiverType::P2PKH, ReceiverType::Sapling}; + receivers = {ReceiverType::P2PKH, ReceiverType::Sapling, ReceiverType::Orchard}; } std::optional j = std::nullopt; @@ -3220,6 +3222,9 @@ UniValue z_getaddressforaccount(const UniValue& params, bool fHelp) case ReceiverType::Sapling: pools.push_back("sapling"); break; + case ReceiverType::Orchard: + pools.push_back("orchard"); + break; default: // Unreachable assert(false); @@ -3331,6 +3336,12 @@ UniValue z_listunifiedreceivers(const UniValue& params, bool fHelp) UniValue result(UniValue::VOBJ); for (const auto& receiver : ua) { std::visit(match { + [&](const libzcash::OrchardRawAddress& addr) { + // Create a single-receiver UA that just contains this Orchard receiver. + UnifiedAddress singleReceiver; + singleReceiver.AddReceiver(addr); + result.pushKV("orchard", keyIO.EncodePaymentAddress(singleReceiver)); + }, [&](const libzcash::SaplingPaymentAddress& addr) { result.pushKV("sapling", keyIO.EncodePaymentAddress(addr)); }, @@ -3762,6 +3773,7 @@ UniValue z_getbalanceforviewingkey(const UniValue& params, bool fHelp) CAmount transparentBalance = 0; CAmount sproutBalance = 0; CAmount saplingBalance = 0; + CAmount orchardBalance = 0; for (const auto& t : spendableInputs.utxos) { transparentBalance += t.Value(); } @@ -3771,6 +3783,9 @@ UniValue z_getbalanceforviewingkey(const UniValue& params, bool fHelp) for (const auto& t : spendableInputs.saplingNoteEntries) { saplingBalance += t.note.value(); } + for (const auto& t : spendableInputs.orchardNoteMetadata) { + orchardBalance += t.GetNoteValue(); + } UniValue pools(UniValue::VOBJ); auto renderBalance = [&](std::string poolName, CAmount balance) { @@ -3783,6 +3798,7 @@ UniValue z_getbalanceforviewingkey(const UniValue& params, bool fHelp) renderBalance("transparent", transparentBalance); renderBalance("sprout", sproutBalance); renderBalance("sapling", saplingBalance); + renderBalance("orchard", orchardBalance); UniValue result(UniValue::VOBJ); result.pushKV("pools", pools); @@ -3863,12 +3879,16 @@ UniValue z_getbalanceforaccount(const UniValue& params, bool fHelp) CAmount transparentBalance = 0; CAmount saplingBalance = 0; + CAmount orchardBalance = 0; for (const auto& t : spendableInputs.utxos) { transparentBalance += t.Value(); } for (const auto& t : spendableInputs.saplingNoteEntries) { saplingBalance += t.note.value(); } + for (const auto& t : spendableInputs.orchardNoteMetadata) { + orchardBalance += t.GetNoteValue(); + } UniValue pools(UniValue::VOBJ); auto renderBalance = [&](std::string poolName, CAmount balance) { @@ -3880,6 +3900,7 @@ UniValue z_getbalanceforaccount(const UniValue& params, bool fHelp) }; renderBalance("transparent", transparentBalance); renderBalance("sapling", saplingBalance); + renderBalance("orchard", orchardBalance); UniValue result(UniValue::VOBJ); result.pushKV("pools", pools); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5aca42d7e..e03e57514 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -268,6 +268,25 @@ bool CWallet::AddOrchardFullViewingKey(const libzcash::OrchardFullViewingKey &fv return true; // TODO ORCHARD: persist fvk } +// Add Orchard payment address -> incoming viewing key map entry +bool CWallet::AddOrchardRawAddress( + const libzcash::OrchardIncomingViewingKey &ivk, + const libzcash::OrchardRawAddress &addr) +{ + AssertLockHeld(cs_wallet); // orchardWallet + if (!orchardWallet.AddRawAddress(addr, ivk)) { + // We should never add an Orchard raw address for which we don't know + // the corresponding FVK. + return false; + }; + + if (!fFileBacked) { + return true; + } + + return true; // TODO ORCHARD: ensure mapping will be recreated on wallet load +} + // Loads a payment address -> incoming viewing key map entry // to the in-memory wallet's keystore. bool CWallet::LoadOrchardRawAddress( @@ -577,7 +596,17 @@ std::optional }; // Add Orchard spending key to the wallet - orchardWallet.AddSpendingKey(usk.value().GetOrchardKey()); + auto orchardSk = usk.value().GetOrchardKey(); + orchardWallet.AddSpendingKey(orchardSk); + + // Associate the Orchard default change address with its IVK. We do this + // here because there is only ever a single Orchard change receiver, and + // it is never exposed to the user. External Orchard receivers are added + // when the user calls z_getaddressforaccount. + auto orchardInternalFvk = orchardSk.ToFullViewingKey().ToInternalIncomingViewingKey(); + if (!AddOrchardRawAddress(orchardInternalFvk, orchardInternalFvk.Address(0))) { + throw std::runtime_error("CWallet::GenerateUnifiedSpendingKeyForAccount(): Failed to add Orchard change address to the wallet."); + }; auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), ufvk); if (!CCryptoKeyStore::AddUnifiedFullViewingKey(zufvk)) { @@ -767,6 +796,17 @@ WalletUAGenerationResult CWallet::GenerateUnifiedAddress( AddSaplingPaymentAddress(dfvk.value().ToIncomingViewingKey(), saplingAddress.value()); } + // If the address has an Orchard component, add an association between + // that address and the Orchard IVK corresponding to the ufvk + auto hasOrchard = receiverTypes.find(ReceiverType::Orchard) != receiverTypes.end(); + if (hasOrchard) { + auto fvk = ufvk.value().GetOrchardKey(); + auto orchardReceiver = address.first.GetOrchardReceiver(); + assert (fvk.has_value() && orchardReceiver.has_value()); + + AddOrchardRawAddress(fvk.value().ToIncomingViewingKey(), orchardReceiver.value()); + } + // Save the metadata for the generated address so that we can re-derive // it in the future. ZcashdUnifiedAddressMetadata addrmeta(ufvkid, address.second, receiverTypes); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bbe1279e2..7a25a2947 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -794,6 +794,7 @@ public: std::vector utxos; std::vector sproutNoteEntries; std::vector saplingNoteEntries; + std::vector orchardNoteMetadata; /** * Selectively discard notes that are not required to obtain the desired @@ -816,6 +817,9 @@ public: for (const auto& t : saplingNoteEntries) { result += t.note.value(); } + for (const auto& t : orchardNoteMetadata) { + result += t.GetNoteValue(); + } return result; } @@ -1463,7 +1467,15 @@ public: bool AddOrchardZKey(const libzcash::OrchardSpendingKey &sk); bool AddOrchardFullViewingKey(const libzcash::OrchardFullViewingKey &fvk); /** - * Adds an address/ivk mapping to the in-memory wallet. Returns `true` + * Adds an address/ivk mapping to the in-memory wallet. Returns `false` if + * the mapping could not be persisted, or the IVK does not correspond to an + * FVK known by the wallet. + */ + bool AddOrchardRawAddress( + const libzcash::OrchardIncomingViewingKey &ivk, + const libzcash::OrchardRawAddress &addr); + /** + * Loads an address/ivk mapping to the in-memory wallet. Returns `true` * if the provided IVK corresponds to an FVK known by the wallet. */ bool LoadOrchardRawAddress( diff --git a/src/zcash/address/orchard.cpp b/src/zcash/address/orchard.cpp index bbcf39143..a1271a4b0 100644 --- a/src/zcash/address/orchard.cpp +++ b/src/zcash/address/orchard.cpp @@ -24,7 +24,7 @@ OrchardIncomingViewingKey OrchardFullViewingKey::ToIncomingViewingKey() const { } OrchardIncomingViewingKey OrchardFullViewingKey::ToInternalIncomingViewingKey() const { - return OrchardIncomingViewingKey(orchard_full_viewing_key_to_incoming_viewing_key(inner.get())); + return OrchardIncomingViewingKey(orchard_full_viewing_key_to_internal_incoming_viewing_key(inner.get())); } OrchardSpendingKey OrchardSpendingKey::ForAccount( diff --git a/src/zcash/address/unified.cpp b/src/zcash/address/unified.cpp index 93dd1b34f..9fe8c131a 100644 --- a/src/zcash/address/unified.cpp +++ b/src/zcash/address/unified.cpp @@ -150,7 +150,7 @@ UnifiedAddressGenerationResult ZcashdUnifiedFullViewingKey::FindAddress( UnifiedAddressGenerationResult ZcashdUnifiedFullViewingKey::FindAddress( const diversifier_index_t& j) const { - return FindAddress(j, {ReceiverType::P2PKH, ReceiverType::Sapling}); + return FindAddress(j, {ReceiverType::P2PKH, ReceiverType::Sapling, ReceiverType::Orchard}); } std::optional ZcashdUnifiedFullViewingKey::GetChangeAddress(const ChangeRequest& req) const {