From b305ad289255dbb737318910d89917a9041c0a3b Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 16 Dec 2021 16:28:43 -0700 Subject: [PATCH 01/13] Remove the `InvalidEncoding` type from key & address variants. The presence of this variant results in a situation where more of the code than necessary needs to be aware of and handle decoding failures. This change moves all handling of decoding failures to the point of decoding. --- src/consensus/params.cpp | 7 +- src/gtest/test_keys.cpp | 30 +-- src/init.cpp | 7 +- src/key_io.cpp | 18 +- src/key_io.h | 6 +- src/miner.cpp | 15 +- src/miner.h | 24 +- src/rpc/misc.cpp | 6 +- src/test/key_tests.cpp | 24 +- src/util/match.h | 24 ++ .../asyncrpcoperation_mergetoaddress.cpp | 4 +- .../asyncrpcoperation_saplingmigration.cpp | 7 +- src/wallet/asyncrpcoperation_sendmany.cpp | 42 ++-- .../asyncrpcoperation_shieldcoinbase.cpp | 9 +- src/wallet/asyncrpcoperation_shieldcoinbase.h | 1 - src/wallet/gtest/test_wallet.cpp | 26 +-- src/wallet/rpcdump.cpp | 24 +- src/wallet/rpcwallet.cpp | 212 ++++++++++-------- src/wallet/test/rpc_wallet_tests.cpp | 20 +- src/wallet/wallet.cpp | 50 +---- src/wallet/wallet.h | 11 +- src/zcash/Address.cpp | 32 +-- src/zcash/Address.hpp | 30 +-- 23 files changed, 286 insertions(+), 343 deletions(-) create mode 100644 src/util/match.h diff --git a/src/consensus/params.cpp b/src/consensus/params.cpp index 2ced730e8..58528b7b4 100644 --- a/src/consensus/params.cpp +++ b/src/consensus/params.cpp @@ -169,10 +169,11 @@ namespace Consensus { addresses.push_back(GetScriptForDestination(taddr)); } else { auto zaddr = keyIO.DecodePaymentAddress(addr); - // If the string is not a valid transparent or Sapling address, we will - // throw here. + if (!(zaddr.has_value() && std::holds_alternative(zaddr.value()))) { + throw std::runtime_error("Funding stream address was not a valid transparent or Sapling address."); + } - addresses.push_back(std::get(zaddr)); + addresses.push_back(std::get(zaddr.value())); } } diff --git a/src/gtest/test_keys.cpp b/src/gtest/test_keys.cpp index b7aef27bb..a632f3dec 100644 --- a/src/gtest/test_keys.cpp +++ b/src/gtest/test_keys.cpp @@ -27,11 +27,11 @@ TEST(Keys, EncodeAndDecodeSapling) Params().Bech32HRP(CChainParams::SAPLING_EXTENDED_SPEND_KEY)); auto spendingkey2 = keyIO.DecodeSpendingKey(sk_string); - EXPECT_TRUE(IsValidSpendingKey(spendingkey2)); + EXPECT_TRUE(spendingkey2.has_value()); - ASSERT_TRUE(std::get_if(&spendingkey2) != nullptr); - auto sk2 = std::get(spendingkey2); - EXPECT_EQ(sk, sk2); + auto sk2 = std::get_if(&spendingkey2.value()); + EXPECT_NE(sk2, nullptr); + EXPECT_EQ(sk, *sk2); } { auto extfvk = sk.ToXFVK(); @@ -41,11 +41,11 @@ TEST(Keys, EncodeAndDecodeSapling) Params().Bech32HRP(CChainParams::SAPLING_EXTENDED_FVK)); auto viewingkey2 = keyIO.DecodeViewingKey(vk_string); - EXPECT_TRUE(IsValidViewingKey(viewingkey2)); + EXPECT_TRUE(viewingkey2.has_value()); - ASSERT_TRUE(std::get_if(&viewingkey2) != nullptr); - auto extfvk2 = std::get(viewingkey2); - EXPECT_EQ(extfvk, extfvk2); + auto extfvk2 = std::get_if(&viewingkey2.value()); + EXPECT_NE(extfvk2, nullptr); + EXPECT_EQ(extfvk, *extfvk2); } { auto addr = sk.DefaultAddress(); @@ -56,11 +56,11 @@ TEST(Keys, EncodeAndDecodeSapling) Params().Bech32HRP(CChainParams::SAPLING_PAYMENT_ADDRESS)); auto paymentaddr2 = keyIO.DecodePaymentAddress(addr_string); - EXPECT_TRUE(IsValidPaymentAddress(paymentaddr2)); + EXPECT_TRUE(paymentaddr2.has_value()); - ASSERT_TRUE(std::get_if(&paymentaddr2) != nullptr); - auto addr2 = std::get(paymentaddr2); - EXPECT_EQ(addr, addr2); + auto addr2 = std::get_if(&paymentaddr2.value()); + EXPECT_NE(addr2, nullptr); + EXPECT_EQ(addr, *addr2); } } } @@ -152,8 +152,10 @@ TEST(Keys, EncodeAndDecodeUnified) std::string expected(expectedBytes.begin(), expectedBytes.end()); auto decoded = keyIO.DecodePaymentAddress(expected); - ASSERT_TRUE(std::holds_alternative(decoded)); - EXPECT_EQ(std::get(decoded), ua); + EXPECT_TRUE(decoded.has_value()); + auto ua_ptr = std::get_if(&decoded.value()); + EXPECT_NE(ua_ptr, nullptr); + EXPECT_EQ(*ua_ptr, ua); auto encoded = keyIO.EncodePaymentAddress(ua); EXPECT_EQ(encoded, expected); diff --git a/src/init.cpp b/src/init.cpp index b632d7e2b..040ff0707 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1102,7 +1102,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) if (!IsValidDestination(addr)) { // Try a payment address auto zaddr = keyIO.DecodePaymentAddress(mapArgs["-mineraddress"]); - if (!std::visit(IsValidMinerAddress(), std::visit(ExtractMinerAddress(), zaddr))) + if (!zaddr.has_value() || std::visit(ExtractMinerAddress(), zaddr.value()).has_value()) { return InitError(strprintf( _("Invalid address for -mineraddress=: '%s' (must be a Sapling or transparent address)"), @@ -1684,8 +1684,11 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) minerAddressInLocalWallet = pwalletMain->HaveKey(keyID); } else { auto zaddr = keyIO.DecodePaymentAddress(mapArgs["-mineraddress"]); + if (!zaddr.has_value()) { + return InitError(_("-mineraddress is not a valid zcash address.")); + } minerAddressInLocalWallet = std::visit( - HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); + HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr.value()); } } if (GetBoolArg("-minetolocalwallet", true) && !minerAddressInLocalWallet) { diff --git a/src/key_io.cpp b/src/key_io.cpp index db83bea04..671cc8d34 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -151,8 +151,6 @@ public: zcash_address_string_free(encoded); return res; } - - std::string operator()(const libzcash::InvalidEncoding& no) const { return {}; } }; class ViewingKeyEncoder @@ -189,8 +187,6 @@ public: memory_cleanse(data.data(), data.size()); return ret; } - - std::string operator()(const libzcash::InvalidEncoding& no) const { return {}; } }; class SpendingKeyEncoder @@ -227,8 +223,6 @@ public: memory_cleanse(data.data(), data.size()); return ret; } - - std::string operator()(const libzcash::InvalidEncoding& no) const { return {}; } }; // Sizes of SaplingPaymentAddress, SaplingExtendedFullViewingKey, and @@ -356,7 +350,7 @@ std::string KeyIO::EncodePaymentAddress(const libzcash::PaymentAddress& zaddr) } template -T1 DecodeAny( +std::optional DecodeAny( const KeyConstants& keyConstants, const std::string& str, std::pair sprout, @@ -393,7 +387,7 @@ T1 DecodeAny( } memory_cleanse(data.data(), data.size()); - return libzcash::InvalidEncoding(); + return std::nullopt; } /** @@ -447,7 +441,7 @@ static bool AddUnknownReceiver(void* ua, uint32_t typecode, const unsigned char* return reinterpret_cast(ua)->AddReceiver(receiver); } -libzcash::PaymentAddress KeyIO::DecodePaymentAddress(const std::string& str) +std::optional KeyIO::DecodePaymentAddress(const std::string& str) { // Try parsing as a Unified Address. libzcash::UnifiedAddress ua; @@ -475,7 +469,7 @@ libzcash::PaymentAddress KeyIO::DecodePaymentAddress(const std::string& str) } bool KeyIO::IsValidPaymentAddressString(const std::string& str) { - return IsValidPaymentAddress(DecodePaymentAddress(str)); + return DecodePaymentAddress(str).has_value(); } std::string KeyIO::EncodeViewingKey(const libzcash::ViewingKey& vk) @@ -483,7 +477,7 @@ std::string KeyIO::EncodeViewingKey(const libzcash::ViewingKey& vk) return std::visit(ViewingKeyEncoder(keyConstants), vk); } -libzcash::ViewingKey KeyIO::DecodeViewingKey(const std::string& str) +std::optional KeyIO::DecodeViewingKey(const std::string& str) { return DecodeAny KeyIO::DecodeSpendingKey(const std::string& str) { return DecodeAny DecodePaymentAddress(const std::string& str); bool IsValidPaymentAddressString(const std::string& str); std::string EncodeViewingKey(const libzcash::ViewingKey& vk); - libzcash::ViewingKey DecodeViewingKey(const std::string& str); + std::optional DecodeViewingKey(const std::string& str); std::string EncodeSpendingKey(const libzcash::SpendingKey& zkey); - libzcash::SpendingKey DecodeSpendingKey(const std::string& str); + std::optional DecodeSpendingKey(const std::string& str); }; #endif // BITCOIN_KEY_IO_H diff --git a/src/miner.cpp b/src/miner.cpp index 0ed57273e..d2a21a987 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -117,9 +117,7 @@ void UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, } bool IsShieldedMinerAddress(const MinerAddress& minerAddr) { - return !( - std::holds_alternative(minerAddr) || - std::holds_alternative>(minerAddr)); + return !std::holds_alternative>(minerAddr); } class AddFundingStreamValueToTx @@ -241,8 +239,6 @@ public: } } - void operator()(const InvalidMinerAddress &invalid) const {} - // Create shielded output void operator()(const libzcash::SaplingPaymentAddress &pa) const { auto ctx = librustzcash_sapling_proving_ctx_init(); @@ -721,9 +717,12 @@ void GetMinerAddress(MinerAddress &minerAddress) minerAddress = mAddr; } else { // Try a payment address - auto zaddr = std::visit(ExtractMinerAddress(), keyIO.DecodePaymentAddress(mAddrArg)); - if (std::visit(IsValidMinerAddress(), zaddr)) { - minerAddress = zaddr; + auto zaddr0 = keyIO.DecodePaymentAddress(mAddrArg); + if (zaddr0.has_value()) { + auto zaddr = std::visit(ExtractMinerAddress(), zaddr0.value()); + if (zaddr.has_value()) { + minerAddress = zaddr.value(); + } } } } diff --git a/src/miner.h b/src/miner.h index 52ac1a29d..61dcc938b 100644 --- a/src/miner.h +++ b/src/miner.h @@ -23,14 +23,7 @@ static const int DEFAULT_GENERATE_THREADS = 1; static const bool DEFAULT_PRINTPRIORITY = false; -class InvalidMinerAddress { -public: - friend bool operator==(const InvalidMinerAddress &a, const InvalidMinerAddress &b) { return true; } - friend bool operator<(const InvalidMinerAddress &a, const InvalidMinerAddress &b) { return true; } -}; - typedef std::variant< - InvalidMinerAddress, libzcash::SaplingPaymentAddress, boost::shared_ptr> MinerAddress; @@ -39,16 +32,13 @@ class ExtractMinerAddress public: ExtractMinerAddress() {} - MinerAddress operator()(const libzcash::InvalidEncoding &invalid) const { - return InvalidMinerAddress(); + std::optional operator()(const libzcash::SproutPaymentAddress &addr) const { + return std::nullopt; } - MinerAddress operator()(const libzcash::SproutPaymentAddress &addr) const { - return InvalidMinerAddress(); - } - MinerAddress operator()(const libzcash::SaplingPaymentAddress &addr) const { + std::optional operator()(const libzcash::SaplingPaymentAddress &addr) const { return addr; } - MinerAddress operator()(const libzcash::UnifiedAddress &addr) const { + std::optional operator()(const libzcash::UnifiedAddress &addr) const { auto recipient = RecipientForPaymentAddress()(addr); if (recipient) { // This looks like a recursive call, but we are actually calling @@ -66,7 +56,7 @@ public: // Either the UA only contains unknown shielded receivers (unlikely that we // wouldn't know about them), or it only contains transparent receivers // (which are invalid). - return InvalidMinerAddress(); + return std::nullopt; } } }; @@ -76,7 +66,6 @@ class KeepMinerAddress public: KeepMinerAddress() {} - void operator()(const InvalidMinerAddress &invalid) const {} void operator()(const libzcash::SaplingPaymentAddress &pa) const {} void operator()(const boost::shared_ptr &coinbaseScript) const { coinbaseScript->KeepScript(); @@ -90,9 +79,6 @@ class IsValidMinerAddress public: IsValidMinerAddress() {} - bool operator()(const InvalidMinerAddress &invalid) const { - return false; - } bool operator()(const libzcash::SaplingPaymentAddress &pa) const { return true; } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index dffee0b8b..c09b694bf 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -217,8 +217,6 @@ UniValue validateaddress(const UniValue& params, bool fHelp) class DescribePaymentAddressVisitor { public: - UniValue operator()(const libzcash::InvalidEncoding &zaddr) const { return UniValue(UniValue::VOBJ); } - UniValue operator()(const libzcash::SproutPaymentAddress &zaddr) const { UniValue obj(UniValue::VOBJ); obj.pushKV("type", "sprout"); @@ -288,14 +286,14 @@ UniValue z_validateaddress(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); string strAddress = params[0].get_str(); auto address = keyIO.DecodePaymentAddress(strAddress); - bool isValid = IsValidPaymentAddress(address); + bool isValid = address.has_value(); UniValue ret(UniValue::VOBJ); ret.pushKV("isvalid", isValid); if (isValid) { ret.pushKV("address", strAddress); - UniValue detail = std::visit(DescribePaymentAddressVisitor(), address); + UniValue detail = std::visit(DescribePaymentAddressVisitor(), address.value()); ret.pushKVs(detail); } return ret; diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index 959bb25da..2de4e28f4 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -202,9 +202,9 @@ BOOST_AUTO_TEST_CASE(zc_address_test) BOOST_CHECK(sk_string[1] == 'K'); auto spendingkey2 = keyIO.DecodeSpendingKey(sk_string); - BOOST_CHECK(IsValidSpendingKey(spendingkey2)); - BOOST_ASSERT(std::get_if(&spendingkey2) != nullptr); - auto sk2 = std::get(spendingkey2); + BOOST_CHECK(spendingkey2.has_value()); + BOOST_ASSERT(std::get_if(&spendingkey2.value()) != nullptr); + auto sk2 = std::get(spendingkey2.value()); BOOST_CHECK(sk.inner() == sk2.inner()); } { @@ -216,10 +216,10 @@ BOOST_AUTO_TEST_CASE(zc_address_test) BOOST_CHECK(addr_string[1] == 'c'); auto paymentaddr2 = keyIO.DecodePaymentAddress(addr_string); - BOOST_ASSERT(IsValidPaymentAddress(paymentaddr2)); + BOOST_ASSERT(paymentaddr2.has_value()); - BOOST_ASSERT(std::get_if(&paymentaddr2) != nullptr); - auto addr2 = std::get(paymentaddr2); + BOOST_ASSERT(std::get_if(&paymentaddr2.value()) != nullptr); + auto addr2 = std::get(paymentaddr2.value()); BOOST_CHECK(addr.a_pk == addr2.a_pk); BOOST_CHECK(addr.pk_enc == addr2.pk_enc); } @@ -240,10 +240,10 @@ BOOST_AUTO_TEST_CASE(zs_address_test) BOOST_CHECK(sk_string.compare(0, 27, Params().Bech32HRP(CChainParams::SAPLING_EXTENDED_SPEND_KEY)) == 0); auto spendingkey2 = keyIO.DecodeSpendingKey(sk_string); - BOOST_CHECK(IsValidSpendingKey(spendingkey2)); + BOOST_CHECK(spendingkey2.has_value()); - BOOST_ASSERT(std::get_if(&spendingkey2) != nullptr); - auto sk2 = std::get(spendingkey2); + BOOST_ASSERT(std::get_if(&spendingkey2.value()) != nullptr); + auto sk2 = std::get(spendingkey2.value()); BOOST_CHECK(sk == sk2); } { @@ -253,10 +253,10 @@ BOOST_AUTO_TEST_CASE(zs_address_test) BOOST_CHECK(addr_string.compare(0, 15, Params().Bech32HRP(CChainParams::SAPLING_PAYMENT_ADDRESS)) == 0); auto paymentaddr2 = keyIO.DecodePaymentAddress(addr_string); - BOOST_CHECK(IsValidPaymentAddress(paymentaddr2)); + BOOST_CHECK(paymentaddr2.has_value()); - BOOST_ASSERT(std::get_if(&paymentaddr2) != nullptr); - auto addr2 = std::get(paymentaddr2); + BOOST_ASSERT(std::get_if(&paymentaddr2.value()) != nullptr); + auto addr2 = std::get(paymentaddr2.value()); BOOST_CHECK(addr == addr2); } } diff --git a/src/util/match.h b/src/util/match.h new file mode 100644 index 000000000..d966ae4da --- /dev/null +++ b/src/util/match.h @@ -0,0 +1,24 @@ +// Copyright (c) 2021 The Zcash developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php . + +#ifndef ZCASH_UTIL_MATCH_H +#define ZCASH_UTIL_MATCH_H + +// Helper for using `std::visit` with Rust-style match syntax. +// +// This can be used in place of defining an explicit visitor. `std::visit` requires an +// exhaustive match; to emulate Rust's catch-all binding, use an `(auto arg)` template +// operator(). +// +// Care must be taken that implicit conversions are handled correctly. For instance, a +// `(double arg)` operator() *will also* bind to `int` and `long`, if there isn't an +// earlier satisfying match. +// +// This corresponds to visitor example #4 in the `std::visit` documentation: +// https://en.cppreference.com/w/cpp/utility/variant/visit +template struct match : Ts... { using Ts::operator()...; }; +// explicit deduction guide (not needed as of C++20) +template match(Ts...) -> match; + +#endif // ZCASH_UTIL_MATCH_H diff --git a/src/wallet/asyncrpcoperation_mergetoaddress.cpp b/src/wallet/asyncrpcoperation_mergetoaddress.cpp index f6faaa473..b3ea22bbe 100644 --- a/src/wallet/asyncrpcoperation_mergetoaddress.cpp +++ b/src/wallet/asyncrpcoperation_mergetoaddress.cpp @@ -102,9 +102,9 @@ AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress( if (!isToTaddr_) { auto address = keyIO.DecodePaymentAddress(std::get<0>(recipient)); - if (IsValidPaymentAddress(address)) { + if (address.has_value()) { isToZaddr_ = true; - toPaymentAddress_ = address; + toPaymentAddress_ = address.value(); } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid recipient address"); } diff --git a/src/wallet/asyncrpcoperation_saplingmigration.cpp b/src/wallet/asyncrpcoperation_saplingmigration.cpp index 6671298c7..d7f48fc60 100644 --- a/src/wallet/asyncrpcoperation_saplingmigration.cpp +++ b/src/wallet/asyncrpcoperation_saplingmigration.cpp @@ -85,7 +85,7 @@ bool AsyncRPCOperation_saplingmigration::main_impl() { // We set minDepth to 11 to avoid unconfirmed notes and in anticipation of specifying // an anchor at height N-10 for each Sprout JoinSplit description // Consider, should notes be sorted? - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, "", 11); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 11); } CAmount availableFunds = 0; for (const SproutNoteEntry& sproutEntry : sproutEntries) { @@ -197,8 +197,9 @@ libzcash::SaplingPaymentAddress AsyncRPCOperation_saplingmigration::getMigration if (mapArgs.count("-migrationdestaddress")) { std::string migrationDestAddress = mapArgs["-migrationdestaddress"]; auto address = keyIO.DecodePaymentAddress(migrationDestAddress); - auto saplingAddress = std::get_if(&address); - assert(saplingAddress != nullptr); // This is checked in init.cpp + assert(address.has_value()); // This is checked in init.cpp + auto saplingAddress = std::get_if(&address.value()); + assert(saplingAddress != nullptr); // This is also checked in init.cpp return *saplingAddress; } // Derive the address for Sapling account 0 diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 155afd700..30b899207 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -98,15 +98,15 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( if (!isfromtaddr_) { auto address = keyIO.DecodePaymentAddress(fromAddress); - if (IsValidPaymentAddress(address)) { + if (address.has_value()) { // We don't need to lock on the wallet as spending key related methods are thread-safe - if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), address)) { + if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), address.value())) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, no spending key found for zaddr"); } isfromzaddr_ = true; - frompaymentaddress_ = address; - spendingkey_ = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address).value(); + frompaymentaddress_ = address.value(); + spendingkey_ = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address.value()).value(); } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address"); } @@ -375,8 +375,8 @@ bool AsyncRPCOperation_sendmany::main_impl() { auto hexMemo = r.memo; auto addr = keyIO.DecodePaymentAddress(address); - assert(std::get_if(&addr) != nullptr); - auto to = std::get(addr); + assert(addr.has_value() && std::get_if(&addr.value()) != nullptr); + auto to = std::get(addr.value()); auto memo = get_memo_from_hex_string(hexMemo); @@ -533,12 +533,14 @@ bool AsyncRPCOperation_sendmany::main_impl() { std::string hexMemo = smr.memo; zOutputsDeque.pop_front(); - PaymentAddress pa = keyIO.DecodePaymentAddress(address); - JSOutput jso = JSOutput(std::get(pa), value); - if (hexMemo.size() > 0) { - jso.memo = get_memo_from_hex_string(hexMemo); + std::optional pa = keyIO.DecodePaymentAddress(address); + if (pa.has_value()) { + JSOutput jso = JSOutput(std::get(pa.value()), value); + if (hexMemo.size() > 0) { + jso.memo = get_memo_from_hex_string(hexMemo); + } + info.vjsout.push_back(jso); } - info.vjsout.push_back(jso); // Funds are removed from the value pool and enter the private pool info.vpub_old += value; @@ -800,13 +802,15 @@ bool AsyncRPCOperation_sendmany::main_impl() { assert(value==0); info.vjsout.push_back(JSOutput()); // dummy output while we accumulate funds into a change note for vpub_new } else { - PaymentAddress pa = keyIO.DecodePaymentAddress(address); - // If we are here, we know we have no Sapling outputs. - JSOutput jso = JSOutput(std::get(pa), value); - if (hexMemo.size() > 0) { - jso.memo = get_memo_from_hex_string(hexMemo); + std::optional pa = keyIO.DecodePaymentAddress(address); + if (pa.has_value()) { + // If we are here, we know we have no Sapling outputs. + JSOutput jso = JSOutput(std::get(pa.value()), value); + if (hexMemo.size() > 0) { + jso.memo = get_memo_from_hex_string(hexMemo); + } + info.vjsout.push_back(jso); } - info.vjsout.push_back(jso); } // create output for any change @@ -927,7 +931,9 @@ bool AsyncRPCOperation_sendmany::load_inputs(TxValues& txValues) { bool AsyncRPCOperation_sendmany::find_unspent_notes() { std::vector sproutEntries; std::vector saplingEntries; - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, fromaddress_, mindepth_); + // TODO: move this to the caller + auto zaddr = KeyIO(Params()).DecodePaymentAddress(fromaddress_); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddr, mindepth_); // If using the TransactionBuilder, we only want Sapling notes. // If not using it, we only want Sprout notes. diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp index 31098c404..0b1960b76 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp @@ -81,8 +81,8 @@ AsyncRPCOperation_shieldcoinbase::AsyncRPCOperation_shieldcoinbase( // Check the destination address is valid for this network i.e. not testnet being used on mainnet KeyIO keyIO(Params()); auto address = keyIO.DecodePaymentAddress(toAddress); - if (IsValidPaymentAddress(address)) { - tozaddr_ = address; + if (address.has_value()) { + tozaddr_ = address.value(); } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid to address"); } @@ -264,11 +264,6 @@ bool ShieldToAddress::operator()(const libzcash::UnifiedAddress &uaddr) const { return false; } -bool ShieldToAddress::operator()(const libzcash::InvalidEncoding& no) const { - return false; -} - - UniValue AsyncRPCOperation_shieldcoinbase::perform_joinsplit(ShieldCoinbaseJSInfo & info) { uint32_t consensusBranchId; uint256 anchor; diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.h b/src/wallet/asyncrpcoperation_shieldcoinbase.h index badc7fa5a..c1048c00a 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.h +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.h @@ -106,7 +106,6 @@ public: bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; bool operator()(const libzcash::UnifiedAddress &uaddr) const; - bool operator()(const libzcash::InvalidEncoding& no) const; }; diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 33361a22b..142d83357 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -206,11 +206,11 @@ TEST(WalletTests, FindUnspentSproutNotes) { // We currently have an unspent and unconfirmed note in the wallet (depth of -1) std::vector sproutEntries; std::vector saplingEntries; - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 0); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 0); EXPECT_EQ(0, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", -1); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, -1); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); @@ -233,15 +233,15 @@ TEST(WalletTests, FindUnspentSproutNotes) { // We now have an unspent and confirmed note in the wallet (depth of 1) - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 0); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 0); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 1); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 1); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 2); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 2); EXPECT_EQ(0, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); @@ -271,22 +271,22 @@ TEST(WalletTests, FindUnspentSproutNotes) { EXPECT_TRUE(wallet.IsSproutSpent(nullifier)); // The note has been spent. By default, GetFilteredNotes() ignores spent notes. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 0); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 0); EXPECT_EQ(0, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); // Let's include spent notes to retrieve it. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 0, false); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 0, INT_MAX, false); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); // The spent note has two confirmations. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 2, false); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 2, INT_MAX, false); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); // It does not have 3 confirmations. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 3, false); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 3, INT_MAX, false); EXPECT_EQ(0, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); @@ -329,22 +329,22 @@ TEST(WalletTests, FindUnspentSproutNotes) { wallet.AddToWallet(wtx3, true, NULL); // We now have an unspent note which has one confirmation, in addition to our spent note. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 1); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 1); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); // Let's return the spent note too. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 1, false); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 1, INT_MAX, false); EXPECT_EQ(2, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); // Increasing number of confirmations will exclude our new unspent note. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 2, false); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 2, INT_MAX, false); EXPECT_EQ(1, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); // If we also ignore spent notes at this depth, we won't find any notes. - wallet.GetFilteredNotes(sproutEntries, saplingEntries, "", 2, true); + wallet.GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 2, INT_MAX, true); EXPECT_EQ(0, sproutEntries.size()); sproutEntries.clear(); saplingEntries.clear(); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index d6c3130b0..0ad8e24b1 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -400,9 +400,9 @@ UniValue importwallet_impl(const UniValue& params, bool fImportZKeys) // Only include hdKeypath and seedFpStr if we have both std::optional hdKeypath = (vstr.size() > 3) ? std::optional(vstr[2]) : std::nullopt; std::optional seedFpStr = (vstr.size() > 3) ? std::optional(vstr[3]) : std::nullopt; - if (IsValidSpendingKey(spendingkey)) { + if (spendingkey.has_value()) { auto addResult = std::visit( - AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, hdKeypath, seedFpStr, true), spendingkey); + AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus(), nTime, hdKeypath, seedFpStr, true), spendingkey.value()); if (addResult == KeyAlreadyExists){ LogPrint("zrpc", "Skipping import of zaddr (key already present)\n"); } else if (addResult == KeyNotAdded) { @@ -751,17 +751,17 @@ UniValue z_importkey(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); string strSecret = params[0].get_str(); auto spendingkey = keyIO.DecodeSpendingKey(strSecret); - if (!IsValidSpendingKey(spendingkey)) { + if (!spendingkey.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); } - auto addrInfo = std::visit(libzcash::AddressInfoFromSpendingKey{}, spendingkey); + auto addrInfo = std::visit(libzcash::AddressInfoFromSpendingKey{}, spendingkey.value()); UniValue result(UniValue::VOBJ); result.pushKV("type", addrInfo.first); result.pushKV("address", keyIO.EncodePaymentAddress(addrInfo.second)); // Sapling support - auto addResult = std::visit(AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus()), spendingkey); + auto addResult = std::visit(AddSpendingKeyToWallet(pwalletMain, Params().GetConsensus()), spendingkey.value()); if (addResult == KeyAlreadyExists && fIgnoreExistingKey) { return result; } @@ -846,17 +846,17 @@ UniValue z_importviewingkey(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); string strVKey = params[0].get_str(); auto viewingkey = keyIO.DecodeViewingKey(strVKey); - if (!IsValidViewingKey(viewingkey)) { + if (!viewingkey.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid viewing key"); } - auto addrInfo = std::visit(libzcash::AddressInfoFromViewingKey{}, viewingkey); + auto addrInfo = std::visit(libzcash::AddressInfoFromViewingKey{}, viewingkey.value()); UniValue result(UniValue::VOBJ); const string strAddress = keyIO.EncodePaymentAddress(addrInfo.second); result.pushKV("type", addrInfo.first); result.pushKV("address", strAddress); - auto addResult = std::visit(AddViewingKeyToWallet(pwalletMain), viewingkey); + auto addResult = std::visit(AddViewingKeyToWallet(pwalletMain), viewingkey.value()); if (addResult == SpendingKeyExists) { throw JSONRPCError( RPC_WALLET_ERROR, @@ -905,12 +905,12 @@ UniValue z_exportkey(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); auto address = keyIO.DecodePaymentAddress(strAddress); - if (!IsValidPaymentAddress(address)) { + if (!address.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr"); } // Sapling support - auto sk = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address); + auto sk = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address.value()); if (!sk) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold private zkey for this zaddr"); } @@ -944,11 +944,11 @@ UniValue z_exportviewingkey(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); auto address = keyIO.DecodePaymentAddress(strAddress); - if (!IsValidPaymentAddress(address)) { + if (!address.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr"); } - auto vk = std::visit(GetViewingKeyForPaymentAddress(pwalletMain), address); + auto vk = std::visit(GetViewingKeyForPaymentAddress(pwalletMain), address.value()); if (vk) { return keyIO.EncodeViewingKey(vk.value()); } else { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a23336794..83c63e554 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -18,6 +18,7 @@ #include "timedata.h" #include "transaction_builder.h" #include "util.h" +#include "util/match.h" #include "utilmoneystr.h" #include "wallet.h" #include "walletdb.h" @@ -2266,16 +2267,16 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) } string address = o.get_str(); auto zaddr = keyIO.DecodePaymentAddress(address); - if (!IsValidPaymentAddress(zaddr)) { + if (!zaddr.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address); } - auto hasSpendingKey = std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); + auto hasSpendingKey = std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr.value()); if (!fIncludeWatchonly && !hasSpendingKey) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); } // We want to return unspent notes corresponding to any receiver within a // Unified Address. - for (const auto ra : std::visit(GetRawAddresses(), zaddr)) { + for (const auto ra : std::visit(GetRawAddresses(), zaddr.value())) { zaddrs.insert(ra); } @@ -2597,13 +2598,13 @@ UniValue zc_raw_receive(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); auto spendingkey = keyIO.DecodeSpendingKey(params[0].get_str()); - if (!IsValidSpendingKey(spendingkey)) { + if (!spendingkey.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); } - if (std::get_if(&spendingkey) == nullptr) { + if (std::get_if(&spendingkey.value()) == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Only works with Sprout spending keys"); } - SproutSpendingKey k = std::get(spendingkey); + SproutSpendingKey k = std::get(spendingkey.value()); uint256 epk; unsigned char nonce; @@ -2723,13 +2724,13 @@ UniValue zc_raw_joinsplit(const UniValue& params, bool fHelp) KeyIO keyIO(Params()); for (const string& name_ : inputs.getKeys()) { auto spendingkey = keyIO.DecodeSpendingKey(inputs[name_].get_str()); - if (!IsValidSpendingKey(spendingkey)) { + if (!spendingkey.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); } - if (std::get_if(&spendingkey) == nullptr) { + if (std::get_if(&spendingkey.value()) == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Only works with Sprout spending keys"); } - SproutSpendingKey k = std::get(spendingkey); + SproutSpendingKey k = std::get(spendingkey.value()); keys.push_back(k); @@ -2770,11 +2771,13 @@ UniValue zc_raw_joinsplit(const UniValue& params, bool fHelp) } for (const string& name_ : outputs.getKeys()) { - auto addrTo = keyIO.DecodePaymentAddress(name_); - if (!IsValidPaymentAddress(addrTo)) { + auto addrToDecoded = keyIO.DecodePaymentAddress(name_); + if (!addrToDecoded.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid recipient address."); } - if (std::get_if(&addrTo) == nullptr) { + + libzcash::PaymentAddress addrTo(addrToDecoded.value()); + if (!std::holds_alternative(addrTo)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Only works with Sprout payment addresses"); } CAmount nAmount = AmountFromValue(outputs[name_]); @@ -3123,70 +3126,77 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) auto fromaddress = params[0].get_str(); KeyIO keyIO(Params()); - auto zaddr = keyIO.DecodePaymentAddress(fromaddress); - if (!IsValidPaymentAddress(zaddr)) { + auto decoded = keyIO.DecodePaymentAddress(fromaddress); + if (!decoded.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr."); } // Visitor to support Sprout and Sapling addrs - if (!std::visit(PaymentAddressBelongsToWallet(pwalletMain), zaddr)) { + if (!std::visit(PaymentAddressBelongsToWallet(pwalletMain), decoded.value())) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key or viewing key not found."); } UniValue result(UniValue::VARR); std::vector sproutEntries; std::vector saplingEntries; - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, fromaddress, nMinDepth, false, false); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, decoded, nMinDepth, false, false); std::set> nullifierSet; - auto hasSpendingKey = std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr); + auto hasSpendingKey = std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), decoded.value()); if (hasSpendingKey) { - nullifierSet = pwalletMain->GetNullifiersForAddresses(std::visit(GetRawAddresses(), zaddr)); + nullifierSet = pwalletMain->GetNullifiersForAddresses(std::visit(GetRawAddresses(), decoded.value())); } - if (std::get_if(&zaddr) != nullptr) { - for (SproutNoteEntry & entry : sproutEntries) { - UniValue obj(UniValue::VOBJ); - obj.pushKV("txid", entry.jsop.hash.ToString()); - obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); - obj.pushKV("amountZat", CAmount(entry.note.value())); - std::string data(entry.memo.begin(), entry.memo.end()); - obj.pushKV("memo", HexStr(data)); - obj.pushKV("jsindex", entry.jsop.js); - obj.pushKV("jsoutindex", entry.jsop.n); - obj.pushKV("confirmations", entry.confirmations); + std::visit(match { + [&](libzcash::SproutPaymentAddress addr) { + for (SproutNoteEntry & entry : sproutEntries) { + UniValue obj(UniValue::VOBJ); + obj.pushKV("txid", entry.jsop.hash.ToString()); + obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); + obj.pushKV("amountZat", CAmount(entry.note.value())); + std::string data(entry.memo.begin(), entry.memo.end()); + obj.pushKV("memo", HexStr(data)); + obj.pushKV("jsindex", entry.jsop.js); + obj.pushKV("jsoutindex", entry.jsop.n); + obj.pushKV("confirmations", entry.confirmations); - txblock BlockData(entry.jsop.hash); - obj.pushKV("blockheight", BlockData.height); - obj.pushKV("blockindex", BlockData.index); - obj.pushKV("blocktime", BlockData.time); + txblock BlockData(entry.jsop.hash); + obj.pushKV("blockheight", BlockData.height); + obj.pushKV("blockindex", BlockData.index); + obj.pushKV("blocktime", BlockData.time); - if (hasSpendingKey) { - obj.pushKV("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop)); + if (hasSpendingKey) { + obj.pushKV("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop)); + } + result.push_back(obj); } - result.push_back(obj); - } - } else if (std::get_if(&zaddr) != nullptr) { - for (SaplingNoteEntry & entry : saplingEntries) { - UniValue obj(UniValue::VOBJ); - obj.pushKV("txid", entry.op.hash.ToString()); - obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); - obj.pushKV("amountZat", CAmount(entry.note.value())); - obj.pushKV("memo", HexStr(entry.memo)); - obj.pushKV("outindex", (int)entry.op.n); - obj.pushKV("confirmations", entry.confirmations); + }, + [&](libzcash::SaplingPaymentAddress addr) { + for (SaplingNoteEntry & entry : saplingEntries) { + UniValue obj(UniValue::VOBJ); + obj.pushKV("txid", entry.op.hash.ToString()); + obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); + obj.pushKV("amountZat", CAmount(entry.note.value())); + obj.pushKV("memo", HexStr(entry.memo)); + obj.pushKV("outindex", (int)entry.op.n); + obj.pushKV("confirmations", entry.confirmations); - txblock BlockData(entry.op.hash); - obj.pushKV("blockheight", BlockData.height); - obj.pushKV("blockindex", BlockData.index); - obj.pushKV("blocktime", BlockData.time); + txblock BlockData(entry.op.hash); + obj.pushKV("blockheight", BlockData.height); + obj.pushKV("blockindex", BlockData.index); + obj.pushKV("blocktime", BlockData.time); - if (hasSpendingKey) { - obj.pushKV("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)); + if (hasSpendingKey) { + obj.pushKV("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)); + } + result.push_back(obj); } - result.push_back(obj); + }, + [&](libzcash::UnifiedAddress) { + // TODO UNIFIED } - } + }, decoded.value()); + return result; } @@ -3234,10 +3244,10 @@ UniValue z_getbalance(const UniValue& params, bool fHelp) auto pa = keyIO.DecodePaymentAddress(fromaddress); fromTaddr = IsValidDestination(taddr); if (!fromTaddr) { - if (!IsValidPaymentAddress(pa)) { + if (!pa.has_value()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr."); } - if (!std::visit(PaymentAddressBelongsToWallet(pwalletMain), pa)) { + if (!std::visit(PaymentAddressBelongsToWallet(pwalletMain), pa.value())) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, spending key or viewing key not found."); } } @@ -3247,7 +3257,7 @@ UniValue z_getbalance(const UniValue& params, bool fHelp) nBalance = getBalanceTaddr(fromaddress, nMinDepth, false); } else { // TODO: Return an error if a UA is provided (once we support UAs). - auto zaddr = std::visit(RecipientForPaymentAddress(), pa).value(); + auto zaddr = std::visit(RecipientForPaymentAddress(), pa.value()).value(); nBalance = getBalanceZaddr(zaddr, nMinDepth, INT_MAX, false); } @@ -3718,19 +3728,20 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) CTxDestination taddr = keyIO.DecodeDestination(fromaddress); fromTaddr = IsValidDestination(taddr); if (!fromTaddr) { - auto res = keyIO.DecodePaymentAddress(fromaddress); - if (!IsValidPaymentAddress(res)) { + auto decoded = keyIO.DecodePaymentAddress(fromaddress); + if (!decoded.has_value()) { // invalid throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr."); } // Check that we have the spending key - if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), res)) { + libzcash::PaymentAddress addr(decoded.value()); + if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), addr)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key not found."); } // Remember whether this is a Sprout or Sapling address - fromSapling = std::get_if(&res) != nullptr; + fromSapling = std::holds_alternative(addr); } } // This logic will need to be updated if we add a new shielded pool @@ -3770,13 +3781,14 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) bool isZaddr = false; CTxDestination taddr = keyIO.DecodeDestination(address); if (!IsValidDestination(taddr)) { - auto res = keyIO.DecodePaymentAddress(address); - if (IsValidPaymentAddress(res)) { + auto decoded = keyIO.DecodePaymentAddress(address); + if (decoded.has_value()) { + libzcash::PaymentAddress addr(decoded.value()); isZaddr = true; - bool toSapling = std::get_if(&res) != nullptr; - bool toSprout = !toSapling; - noSproutAddrs = noSproutAddrs && toSapling; + bool toSapling = std::holds_alternative(addr); + bool toSprout = std::holds_alternative(addr); + noSproutAddrs = !toSprout && noSproutAddrs && toSapling; containsSproutOutput |= toSprout; containsSaplingOutput |= toSapling; @@ -3872,16 +3884,23 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) size_t txsize = 0; for (int i = 0; i < zaddrRecipients.size(); i++) { auto address = zaddrRecipients[i].address; - auto res = keyIO.DecodePaymentAddress(address); - bool toSapling = std::get_if(&res) != nullptr; - if (toSapling) { - mtx.vShieldedOutput.push_back(OutputDescription()); - } else { - JSDescription jsdesc; - if (mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)) { - jsdesc.proof = GrothProof(); - } - mtx.vJoinSplit.push_back(jsdesc); + auto decoded = keyIO.DecodePaymentAddress(address); + if (decoded.has_value()) { + std::visit(match { + [&](libzcash::SaplingPaymentAddress addr) { + mtx.vShieldedOutput.push_back(OutputDescription()); + }, + [&](libzcash::SproutPaymentAddress addr) { + JSDescription jsdesc; + if (mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)) { + jsdesc.proof = GrothProof(); + } + mtx.vJoinSplit.push_back(jsdesc); + }, + [&](libzcash::UnifiedAddress) { + // TODO UNIFIED + } + }, decoded.value()); } } CTransaction tx(mtx); @@ -4174,10 +4193,11 @@ UniValue z_shieldcoinbase(const UniValue& params, bool fHelp) if (canopyActive) { auto decodeAddr = keyIO.DecodePaymentAddress(destaddress); - bool isToSproutZaddr = (std::get_if(&decodeAddr) != nullptr); - - if (isToSproutZaddr) { - throw JSONRPCError(RPC_VERIFY_REJECTED, "Sprout shielding is not supported after Canopy activation"); + if (decodeAddr.has_value()) { + libzcash::PaymentAddress addr(decodeAddr.value()); + if (std::holds_alternative(addr)) { + throw JSONRPCError(RPC_VERIFY_REJECTED, "Sprout shielding is not supported after Canopy activation"); + } } } @@ -4427,12 +4447,12 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) isFromNonSprout = true; } else { auto zaddr = keyIO.DecodePaymentAddress(address); - if (IsValidPaymentAddress(zaddr)) { + if (zaddr.has_value()) { // We want to merge notes corresponding to any receiver within a // Unified Address. - for (const auto ra : std::visit(GetRawAddresses(), zaddr)) { + for (const libzcash::RawAddress& ra : std::visit(GetRawAddresses(), zaddr.value())) { zaddrs.insert(ra); - if (std::get_if(&ra) != nullptr) { + if (std::holds_alternative(ra)) { isFromNonSprout = true; } } @@ -4465,17 +4485,23 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) bool isToSaplingZaddr = false; CTxDestination taddr = keyIO.DecodeDestination(destaddress); if (!IsValidDestination(taddr)) { - auto decodeAddr = keyIO.DecodePaymentAddress(destaddress); - if (IsValidPaymentAddress(decodeAddr)) { - if (std::get_if(&decodeAddr) != nullptr) { - isToSaplingZaddr = true; - // If Sapling is not active, do not allow sending to a sapling addresses. - if (!saplingActive) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); + auto zaddr = keyIO.DecodePaymentAddress(destaddress); + if (zaddr.has_value()) { + std::visit(match { + [&](libzcash::SaplingPaymentAddress addr) { + isToSaplingZaddr = true; + // If Sapling is not active, do not allow sending to a sapling addresses. + if (!saplingActive) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); + } + }, + [&](libzcash::SproutPaymentAddress addr) { + isToSproutZaddr = true; + }, + [&](libzcash::UnifiedAddress) { + // TODO UNIFIED } - } else { - isToSproutZaddr = true; - } + }, zaddr.value()); } else { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress ); } diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 2d411842f..5947c444a 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -602,15 +602,16 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importwallet) BOOST_CHECK(addrs.size()==1); // check that we have the spending key for the address - auto address = keyIO.DecodePaymentAddress(testAddr); - BOOST_CHECK(IsValidPaymentAddress(address)); - BOOST_ASSERT(std::get_if(&address) != nullptr); - auto addr = std::get(address); - BOOST_CHECK(pwalletMain->HaveSproutSpendingKey(addr)); + auto decoded = keyIO.DecodePaymentAddress(testAddr); + BOOST_CHECK(decoded.has_value()); + libzcash::PaymentAddress address(decoded.value()); + BOOST_ASSERT(std::holds_alternative(address)); + auto sprout_addr = std::get(address); + BOOST_CHECK(pwalletMain->HaveSproutSpendingKey(sprout_addr)); // Verify the spending key is the same as the test data libzcash::SproutSpendingKey k; - BOOST_CHECK(pwalletMain->GetSproutSpendingKey(addr, k)); + BOOST_CHECK(pwalletMain->GetSproutSpendingKey(sprout_addr, k)); BOOST_CHECK_EQUAL(testKey, keyIO.EncodeSpendingKey(k)); } @@ -776,10 +777,9 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importexport) // Check if address is of given type and spendable from our wallet. template -void CheckHaveAddr(const libzcash::PaymentAddress& addr) { - - BOOST_CHECK(IsValidPaymentAddress(addr)); - auto addr_of_type = std::get_if(&addr); +void CheckHaveAddr(const std::optional& addr) { + BOOST_CHECK(addr.has_value()); + auto addr_of_type = std::get_if(&(addr.value())); BOOST_ASSERT(addr_of_type != nullptr); HaveSpendingKeyForPaymentAddress test(pwalletMain); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index adc171603..c6fd173fd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4927,8 +4927,8 @@ bool CWallet::ParameterInteraction(const CChainParams& params) // Check Sapling migration address if set and is a valid Sapling address if (mapArgs.count("-migrationdestaddress")) { std::string migrationDestAddress = mapArgs["-migrationdestaddress"]; - libzcash::PaymentAddress address = keyIO.DecodePaymentAddress(migrationDestAddress); - if (std::get_if(&address) == nullptr) { + std::optional address = keyIO.DecodePaymentAddress(migrationDestAddress); + if (!address.has_value() || std::get_if(&address.value()) == nullptr) { return UIError(_("-migrationdestaddress must be a valid Sapling address.")); } } @@ -5032,17 +5032,15 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee) void CWallet::GetFilteredNotes( std::vector& sproutEntries, std::vector& saplingEntries, - std::string address, + std::optional address, int minDepth, bool ignoreSpent, bool requireSpendingKey) { std::set filterAddresses; - KeyIO keyIO(Params()); - if (address.length() > 0) { - auto addr = keyIO.DecodePaymentAddress(address); - for (const auto ra : std::visit(GetRawAddresses(), addr)) { + if (address.has_value()) { + for (const auto ra : std::visit(GetRawAddresses(), address.value())) { filterAddresses.insert(ra); } } @@ -5210,11 +5208,6 @@ bool PaymentAddressBelongsToWallet::operator()(const libzcash::UnifiedAddress &u return false; } -bool PaymentAddressBelongsToWallet::operator()(const libzcash::InvalidEncoding& no) const -{ - return false; -} - /// PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const @@ -5253,12 +5246,6 @@ PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::Unif return AddressNotFound; } -PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::InvalidEncoding& no) const -{ - return AddressNotFound; -} - - /// std::optional GetViewingKeyForPaymentAddress::operator()( @@ -5297,13 +5284,6 @@ std::optional GetViewingKeyForPaymentAddress::operator()( return libzcash::ViewingKey(); } -std::optional GetViewingKeyForPaymentAddress::operator()( - const libzcash::InvalidEncoding& no) const -{ - // Defaults to InvalidEncoding - return libzcash::ViewingKey(); -} - bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const { return m_wallet->HaveSproutSpendingKey(zaddr); @@ -5325,11 +5305,6 @@ bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::UnifiedAddress return false; } -bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::InvalidEncoding& no) const -{ - return false; -} - std::optional GetSpendingKeyForPaymentAddress::operator()( const libzcash::SproutPaymentAddress &zaddr) const { @@ -5359,13 +5334,6 @@ std::optional GetSpendingKeyForPaymentAddress::operator() return libzcash::SpendingKey(); } -std::optional GetSpendingKeyForPaymentAddress::operator()( - const libzcash::InvalidEncoding& no) const -{ - // Defaults to InvalidEncoding - return libzcash::SpendingKey(); -} - KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SproutViewingKey &vkey) const { auto addr = vkey.address(); @@ -5392,10 +5360,6 @@ KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SaplingExtendedFu } } -KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::InvalidEncoding& no) const { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid viewing key"); -} - KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { auto addr = sk.address(); KeyIO keyIO(Params()); @@ -5447,7 +5411,3 @@ KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedS } } } - -KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::InvalidEncoding& no) const { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid spending key"); -} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7045a1ea2..57fda6366 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1304,10 +1304,10 @@ public: /* Set the current encrypted HD seed, without saving it to disk (used by LoadWallet) */ bool LoadCryptedHDSeed(const uint256& seedFp, const std::vector& seed); - /* Find notes filtered by payment address, min depth, ability to spend */ + /* Find notes filtered by (optional) payment address, min depth, ability to spend */ void GetFilteredNotes(std::vector& sproutEntries, std::vector& saplingEntries, - std::string address, + std::optional address, int minDepth=1, bool ignoreSpent=true, bool requireSpendingKey=true); @@ -1372,7 +1372,6 @@ public: bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; bool operator()(const libzcash::UnifiedAddress &uaddr) const; - bool operator()(const libzcash::InvalidEncoding& no) const; }; class GetViewingKeyForPaymentAddress @@ -1385,7 +1384,6 @@ public: std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; - std::optional operator()(const libzcash::InvalidEncoding& no) const; }; class HaveSpendingKeyForPaymentAddress @@ -1398,7 +1396,6 @@ public: bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; bool operator()(const libzcash::UnifiedAddress &uaddr) const; - bool operator()(const libzcash::InvalidEncoding& no) const; }; class GetSpendingKeyForPaymentAddress @@ -1411,7 +1408,6 @@ public: std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; - std::optional operator()(const libzcash::InvalidEncoding& no) const; }; enum PaymentAddressSource { @@ -1433,7 +1429,6 @@ public: PaymentAddressSource operator()(const libzcash::SproutPaymentAddress &zaddr) const; PaymentAddressSource operator()(const libzcash::SaplingPaymentAddress &zaddr) const; PaymentAddressSource operator()(const libzcash::UnifiedAddress &uaddr) const; - PaymentAddressSource operator()(const libzcash::InvalidEncoding& no) const; }; enum KeyAddResult { @@ -1452,7 +1447,6 @@ public: KeyAddResult operator()(const libzcash::SproutViewingKey &sk) const; KeyAddResult operator()(const libzcash::SaplingExtendedFullViewingKey &sk) const; - KeyAddResult operator()(const libzcash::InvalidEncoding& no) const; }; class AddSpendingKeyToWallet @@ -1479,7 +1473,6 @@ public: KeyAddResult operator()(const libzcash::SproutSpendingKey &sk) const; KeyAddResult operator()(const libzcash::SaplingExtendedSpendingKey &sk) const; - KeyAddResult operator()(const libzcash::InvalidEncoding& no) const; }; diff --git a/src/zcash/Address.cpp b/src/zcash/Address.cpp index d827a395a..dbd8e7c20 100644 --- a/src/zcash/Address.cpp +++ b/src/zcash/Address.cpp @@ -41,9 +41,6 @@ std::pair AddressInfoFromSpendingKey::operator()(co std::pair AddressInfoFromSpendingKey::operator()(const SaplingExtendedSpendingKey &sk) const { return std::make_pair("sapling", sk.DefaultAddress()); } -std::pair AddressInfoFromSpendingKey::operator()(const InvalidEncoding&) const { - throw std::invalid_argument("Cannot derive default address from invalid spending key"); -} std::pair AddressInfoFromViewingKey::operator()(const SproutViewingKey &sk) const { return std::make_pair("sprout", sk.address()); @@ -51,23 +48,8 @@ std::pair AddressInfoFromViewingKey::operator()(con std::pair AddressInfoFromViewingKey::operator()(const SaplingExtendedFullViewingKey &sk) const { return std::make_pair("sapling", sk.DefaultAddress()); } -std::pair AddressInfoFromViewingKey::operator()(const InvalidEncoding&) const { - throw std::invalid_argument("Cannot derive default address from invalid viewing key"); -} -} - -bool IsValidPaymentAddress(const libzcash::PaymentAddress& zaddr) { - return !std::holds_alternative(zaddr); -} - -bool IsValidViewingKey(const libzcash::ViewingKey& vk) { - return !std::holds_alternative(vk); -} - -bool IsValidSpendingKey(const libzcash::SpendingKey& zkey) { - return !std::holds_alternative(zkey); -} +} // namespace libzcash uint32_t TypecodeForReceiver::operator()( const libzcash::SaplingPaymentAddress &zaddr) const @@ -117,12 +99,6 @@ std::optional ReceiverToRawAddress::operator()( return std::nullopt; } -std::optional RecipientForPaymentAddress::operator()( - const libzcash::InvalidEncoding& no) const -{ - return std::nullopt; -} - std::optional RecipientForPaymentAddress::operator()( const libzcash::SproutPaymentAddress &zaddr) const { @@ -146,12 +122,6 @@ std::optional RecipientForPaymentAddress::operator()( return std::nullopt; } -std::set GetRawAddresses::operator()( - const libzcash::InvalidEncoding& no) const -{ - return {}; -} - std::set GetRawAddresses::operator()( const libzcash::SproutPaymentAddress &zaddr) const { diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 38d416ee2..8eec84ee5 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -24,12 +24,6 @@ public: /** Protocol addresses that can receive funds in a transaction. */ typedef std::variant RawAddress; -class InvalidEncoding { -public: - friend bool operator==(const InvalidEncoding &a, const InvalidEncoding &b) { return true; } - friend bool operator<(const InvalidEncoding &a, const InvalidEncoding &b) { return true; } -}; - class UnknownReceiver { public: uint32_t typecode; @@ -137,38 +131,32 @@ public: /** Addresses that can appear in a string encoding. */ typedef std::variant< - InvalidEncoding, SproutPaymentAddress, SaplingPaymentAddress, UnifiedAddress> PaymentAddress; -typedef std::variant ViewingKey; -typedef std::variant SpendingKey; +/** Viewing keys that can have a string encoding. */ +typedef std::variant< + SproutViewingKey, + SaplingExtendedFullViewingKey> ViewingKey; +/** Spending keys that can have a string encoding. */ +typedef std::variant< + SproutSpendingKey, + SaplingExtendedSpendingKey> SpendingKey; class AddressInfoFromSpendingKey { public: std::pair operator()(const SproutSpendingKey&) const; std::pair operator()(const struct SaplingExtendedSpendingKey&) const; - std::pair operator()(const InvalidEncoding&) const; }; class AddressInfoFromViewingKey { public: std::pair operator()(const SproutViewingKey&) const; std::pair operator()(const struct SaplingExtendedFullViewingKey&) const; - std::pair operator()(const InvalidEncoding&) const; }; } -/** Check whether a PaymentAddress is not an InvalidEncoding. */ -bool IsValidPaymentAddress(const libzcash::PaymentAddress& zaddr); - -/** Check whether a ViewingKey is not an InvalidEncoding. */ -bool IsValidViewingKey(const libzcash::ViewingKey& vk); - -/** Check whether a SpendingKey is not an InvalidEncoding. */ -bool IsValidSpendingKey(const libzcash::SpendingKey& zkey); - /** * Gets the typecode for the given UA receiver. */ @@ -202,7 +190,6 @@ class RecipientForPaymentAddress { public: RecipientForPaymentAddress() {} - std::optional operator()(const libzcash::InvalidEncoding& no) const; std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; @@ -215,7 +202,6 @@ class GetRawAddresses { public: GetRawAddresses() {} - std::set operator()(const libzcash::InvalidEncoding& no) const; std::set operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::set operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::set operator()(const libzcash::UnifiedAddress &uaddr) const; From 62b86d6afc468c42ba54f58ccefcc01c6d442124 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 19 Nov 2021 16:21:36 -0700 Subject: [PATCH 02/13] Use CKeyID and CScriptID instead of new P2PKH/P2SHAddress classes. --- src/gtest/test_keys.cpp | 8 ++++---- src/key_io.cpp | 12 ++++++------ src/pubkey.h | 1 + src/script/script.h | 10 ++++++++++ src/script/standard.h | 9 --------- src/zcash/Address.cpp | 12 ++++++------ src/zcash/Address.hpp | 25 ++++++++----------------- 7 files changed, 35 insertions(+), 42 deletions(-) diff --git a/src/gtest/test_keys.cpp b/src/gtest/test_keys.cpp index a632f3dec..2ad3ecfc2 100644 --- a/src/gtest/test_keys.cpp +++ b/src/gtest/test_keys.cpp @@ -78,11 +78,11 @@ namespace libzcash { return tfm::format("Sapling(%s)", HexStr(ss.begin(), ss.end())); } - std::string operator()(const P2SHAddress &p2sh) const { + std::string operator()(const CScriptID &p2sh) const { return tfm::format("P2SH(%s)", p2sh.GetHex()); } - std::string operator()(const P2PKHAddress &p2pkh) const { + std::string operator()(const CKeyID &p2pkh) const { return tfm::format("P2PKH(%s)", p2pkh.GetHex()); } @@ -140,11 +140,11 @@ TEST(Keys, EncodeAndDecodeUnified) ua.AddReceiver(r); } if (!test[1].isNull()) { - libzcash::P2SHAddress r(ParseHex(test[1].get_str())); + CScriptID r(ParseHex(test[1].get_str())); ua.AddReceiver(r); } if (!test[0].isNull()) { - libzcash::P2PKHAddress r(ParseHex(test[0].get_str())); + CKeyID r(ParseHex(test[0].get_str())); ua.AddReceiver(r); } diff --git a/src/key_io.cpp b/src/key_io.cpp index 671cc8d34..c8f180296 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -56,8 +56,8 @@ public: DataLenForReceiver() {} size_t operator()(const libzcash::SaplingPaymentAddress &zaddr) const { return 43; } - size_t operator()(const libzcash::P2SHAddress &p2sh) const { return 20; } - size_t operator()(const libzcash::P2PKHAddress &p2pkh) const { return 20; } + size_t operator()(const CScriptID &p2sh) const { return 20; } + size_t operator()(const CKeyID &p2pkh) const { return 20; } size_t operator()(const libzcash::UnknownReceiver &unknown) const { return unknown.data.size(); } }; @@ -82,11 +82,11 @@ public: memcpy(data, ss.data(), ss.size()); } - void operator()(const libzcash::P2SHAddress &p2sh) const { + void operator()(const CScriptID &p2sh) const { memcpy(data, p2sh.begin(), p2sh.size()); } - void operator()(const libzcash::P2PKHAddress &p2pkh) const { + void operator()(const CKeyID &p2pkh) const { memcpy(data, p2pkh.begin(), p2pkh.size()); } @@ -415,7 +415,7 @@ static bool AddP2SHReceiver(void* ua, const unsigned char* raw) reinterpret_cast(raw + 20), SER_NETWORK, PROTOCOL_VERSION); - libzcash::P2SHAddress receiver; + CScriptID receiver; ss >> receiver; return reinterpret_cast(ua)->AddReceiver(receiver); } @@ -430,7 +430,7 @@ static bool AddP2PKHReceiver(void* ua, const unsigned char* raw) reinterpret_cast(raw + 20), SER_NETWORK, PROTOCOL_VERSION); - libzcash::P2PKHAddress receiver; + CKeyID receiver; ss >> receiver; return reinterpret_cast(ua)->AddReceiver(receiver); } diff --git a/src/pubkey.h b/src/pubkey.h index 4cdfbef38..94ff547c4 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -22,6 +22,7 @@ class CKeyID : public uint160 public: CKeyID() : uint160() {} CKeyID(const uint160& in) : uint160(in) {} + explicit CKeyID(const std::vector& vch) : uint160(vch) {} }; typedef uint256 ChainCode; diff --git a/src/script/script.h b/src/script/script.h index e67119197..b20227caf 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -625,6 +625,16 @@ public: } }; +/** A reference to a CScript: the Hash160 of its serialization */ +class CScriptID : public uint160 +{ +public: + CScriptID() : uint160() {} + explicit CScriptID(const CScript& in); + CScriptID(const uint160& in) : uint160(in) {} + explicit CScriptID(const std::vector& vch) : uint160(vch) {} +}; + class CReserveScript { public: diff --git a/src/script/standard.h b/src/script/standard.h index 09a23368f..920ac165d 100644 --- a/src/script/standard.h +++ b/src/script/standard.h @@ -18,15 +18,6 @@ static const bool DEFAULT_ACCEPT_DATACARRIER = true; class CKeyID; class CScript; -/** A reference to a CScript: the Hash160 of its serialization (see script.h) */ -class CScriptID : public uint160 -{ -public: - CScriptID() : uint160() {} - explicit CScriptID(const CScript& in); - CScriptID(const uint160& in) : uint160(in) {} -}; - /** * Default setting for nMaxDatacarrierBytes. 80 bytes of data, +1 for OP_RETURN, * +2 for the pushdata opcodes. diff --git a/src/zcash/Address.cpp b/src/zcash/Address.cpp index dbd8e7c20..fe048c1a9 100644 --- a/src/zcash/Address.cpp +++ b/src/zcash/Address.cpp @@ -24,8 +24,8 @@ bool UnifiedAddress::AddReceiver(Receiver receiver) { auto t = std::visit(TypecodeForReceiver(), r); if ( (t == typecode) || - (std::holds_alternative(r) && std::holds_alternative(receiver)) || - (std::holds_alternative(r) && std::holds_alternative(receiver)) + (std::holds_alternative(r) && std::holds_alternative(receiver)) || + (std::holds_alternative(r) && std::holds_alternative(receiver)) ) { return false; } @@ -58,13 +58,13 @@ uint32_t TypecodeForReceiver::operator()( } uint32_t TypecodeForReceiver::operator()( - const libzcash::P2SHAddress &p2sh) const + const CScriptID &p2sh) const { return ZCASH_UA_TYPECODE_P2SH; } uint32_t TypecodeForReceiver::operator()( - const libzcash::P2PKHAddress &p2sh) const + const CKeyID &p2sh) const { return ZCASH_UA_TYPECODE_P2PKH; } @@ -82,13 +82,13 @@ std::optional ReceiverToRawAddress::operator()( } std::optional ReceiverToRawAddress::operator()( - const libzcash::P2SHAddress &p2sh) const + const CScriptID &p2sh) const { return std::nullopt; } std::optional ReceiverToRawAddress::operator()( - const libzcash::P2PKHAddress &p2sh) const + const CKeyID &p2sh) const { return std::nullopt; } diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 8eec84ee5..fd95418d4 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -2,6 +2,8 @@ #define ZC_ADDRESS_H_ #include "uint256.h" +#include "pubkey.h" +#include "script/script.h" #include "zcash/address/sapling.hpp" #include "zcash/address/sprout.hpp" #include "zcash/address/zip32.h" @@ -9,17 +11,6 @@ #include namespace libzcash { -// We use new classes here instead of CKeyID and CScriptID to prevent a cyclic dependency. -class P2PKHAddress: public uint160 { -public: - P2PKHAddress() {} - explicit P2PKHAddress(const std::vector& vch) : uint160(vch) {} -}; -class P2SHAddress: public uint160 { -public: - P2SHAddress() {} - explicit P2SHAddress(const std::vector& vch) : uint160(vch) {} -}; /** Protocol addresses that can receive funds in a transaction. */ typedef std::variant RawAddress; @@ -53,8 +44,8 @@ public: */ typedef std::variant< SaplingPaymentAddress, - P2SHAddress, - P2PKHAddress, + CScriptID, + CKeyID, UnknownReceiver> Receiver; struct ReceiverIterator { @@ -165,8 +156,8 @@ public: TypecodeForReceiver() {} uint32_t operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - uint32_t operator()(const libzcash::P2SHAddress &p2sh) const; - uint32_t operator()(const libzcash::P2PKHAddress &p2pkh) const; + uint32_t operator()(const CScriptID &p2sh) const; + uint32_t operator()(const CKeyID &p2pkh) const; uint32_t operator()(const libzcash::UnknownReceiver &p2pkh) const; }; @@ -178,8 +169,8 @@ public: ReceiverToRawAddress() {} std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - std::optional operator()(const libzcash::P2SHAddress &p2sh) const; - std::optional operator()(const libzcash::P2PKHAddress &p2pkh) const; + std::optional operator()(const CScriptID &p2sh) const; + std::optional operator()(const CKeyID &p2pkh) const; std::optional operator()(const libzcash::UnknownReceiver &p2pkh) const; }; From c35e2b4438c449186c1dc4ee08e0600226445706 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 17 Dec 2021 12:59:30 -0700 Subject: [PATCH 03/13] Remove spurious uses of HaveSpendingKeyForPaymentAddress There's no need to dispatch through this visitor in cases where the payment address type is already statically known. --- src/keystore.cpp | 19 ++++++++-- src/keystore.h | 3 ++ src/rpc/misc.cpp | 4 +- src/wallet/rpcwallet.cpp | 80 ++++++++++++++++++++++------------------ src/wallet/wallet.cpp | 5 ++- src/zcash/Address.hpp | 2 +- 6 files changed, 68 insertions(+), 45 deletions(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index e39a0d6db..645d6f456 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -151,7 +151,7 @@ bool CBasicKeyStore::AddSproutSpendingKey(const libzcash::SproutSpendingKey &sk) return true; } -//! Sapling +//! Sapling bool CBasicKeyStore::AddSaplingSpendingKey( const libzcash::SaplingExtendedSpendingKey &sk) { @@ -187,7 +187,7 @@ bool CBasicKeyStore::AddSaplingFullViewingKey( return CBasicKeyStore::AddSaplingIncomingViewingKey(ivk, extfvk.DefaultAddress()); } -// This function updates the wallet's internal address->ivk map. +// This function updates the wallet's internal address->ivk map. // If we add an address that is already in the map, the map will // remain unchanged as each address only has one ivk. bool CBasicKeyStore::AddSaplingIncomingViewingKey( @@ -265,8 +265,9 @@ bool CBasicKeyStore::GetSaplingIncomingViewingKey(const libzcash::SaplingPayment return false; } -bool CBasicKeyStore::GetSaplingExtendedSpendingKey(const libzcash::SaplingPaymentAddress &addr, - libzcash::SaplingExtendedSpendingKey &extskOut) const { +bool CBasicKeyStore::GetSaplingExtendedSpendingKey( + const libzcash::SaplingPaymentAddress &addr, + libzcash::SaplingExtendedSpendingKey &extskOut) const { libzcash::SaplingIncomingViewingKey ivk; libzcash::SaplingExtendedFullViewingKey extfvk; @@ -275,3 +276,13 @@ bool CBasicKeyStore::GetSaplingExtendedSpendingKey(const libzcash::SaplingPaymen GetSaplingFullViewingKey(ivk, extfvk) && GetSaplingSpendingKey(extfvk, extskOut); } + +bool CBasicKeyStore::HaveSaplingSpendingKeyForAddress( + const libzcash::SaplingPaymentAddress &addr) const { + libzcash::SaplingIncomingViewingKey ivk; + libzcash::SaplingExtendedFullViewingKey extfvk; + + return GetSaplingIncomingViewingKey(addr, ivk) && + GetSaplingFullViewingKey(ivk, extfvk) && + HaveSaplingSpendingKey(extfvk); +} diff --git a/src/keystore.h b/src/keystore.h index 8246461aa..7b021acf2 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -66,6 +66,8 @@ public: //! Check whether a Sapling spending key corresponding to a given Sapling viewing key is present in the store. virtual bool HaveSaplingSpendingKey( const libzcash::SaplingExtendedFullViewingKey &extfvk) const =0; + virtual bool HaveSaplingSpendingKeyForAddress( + const libzcash::SaplingPaymentAddress &addr) const =0; virtual bool GetSaplingSpendingKey( const libzcash::SaplingExtendedFullViewingKey &extfvk, libzcash::SaplingExtendedSpendingKey& skOut) const =0; @@ -246,6 +248,7 @@ public: } return result; } + bool HaveSaplingSpendingKeyForAddress(const libzcash::SaplingPaymentAddress &addr) const; bool GetSaplingSpendingKey( const libzcash::SaplingExtendedFullViewingKey &extfvk, libzcash::SaplingExtendedSpendingKey &skOut) const diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index c09b694bf..0f21145b7 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -224,7 +224,7 @@ public: obj.pushKV("transmissionkey", zaddr.pk_enc.GetHex()); #ifdef ENABLE_WALLET if (pwalletMain) { - obj.pushKV("ismine", HaveSpendingKeyForPaymentAddress(pwalletMain)(zaddr)); + obj.pushKV("ismine", pwalletMain->HaveSproutSpendingKey(zaddr)); } #endif return obj; @@ -237,7 +237,7 @@ public: obj.pushKV("diversifiedtransmissionkey", zaddr.pk_d.GetHex()); #ifdef ENABLE_WALLET if (pwalletMain) { - obj.pushKV("ismine", HaveSpendingKeyForPaymentAddress(pwalletMain)(zaddr)); + obj.pushKV("ismine", pwalletMain->HaveSaplingSpendingKeyForAddress(zaddr)); } #endif return obj; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 83c63e554..984d02969 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -432,7 +432,7 @@ UniValue listaddresses(const UniValue& params, bool fHelp) if (!sproutAddresses.empty()) { UniValue random_sprout_addrs(UniValue::VARR); for (const SproutPaymentAddress& addr : sproutAddresses) { - if (HaveSpendingKeyForPaymentAddress(pwalletMain)(addr)) { + if (pwalletMain->HaveSproutSpendingKey(addr)) { random_sprout_addrs.push_back(keyIO.EncodePaymentAddress(addr)); } } @@ -2270,13 +2270,25 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) if (!zaddr.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address); } - auto hasSpendingKey = std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr.value()); - if (!fIncludeWatchonly && !hasSpendingKey) { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); - } + // We want to return unspent notes corresponding to any receiver within a // Unified Address. for (const auto ra : std::visit(GetRawAddresses(), zaddr.value())) { + bool hasSpendingKey = std::visit(match { + [&](const SaplingPaymentAddress& addr) { + return pwalletMain->HaveSaplingSpendingKeyForAddress(addr); + }, + [&](const SproutPaymentAddress& addr) { + return pwalletMain->HaveSproutSpendingKey(addr); + } + }, ra); + + // If we don't include watchonly addresses, we must reject any address + // for which we do not have the spending key. + if (!fIncludeWatchonly && !hasSpendingKey) { + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); + } + zaddrs.insert(ra); } @@ -2313,7 +2325,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) obj.pushKV("jsindex", (int)entry.jsop.js ); obj.pushKV("jsoutindex", (int)entry.jsop.n); obj.pushKV("confirmations", entry.confirmations); - bool hasSproutSpendingKey = HaveSpendingKeyForPaymentAddress(pwalletMain)(entry.address); + bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(entry.address); obj.pushKV("spendable", hasSproutSpendingKey); obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address)); obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); @@ -2330,7 +2342,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) obj.pushKV("txid", entry.op.hash.ToString()); obj.pushKV("outindex", (int)entry.op.n); obj.pushKV("confirmations", entry.confirmations); - bool hasSaplingSpendingKey = HaveSpendingKeyForPaymentAddress(pwalletMain)(entry.address); + bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(entry.address); obj.pushKV("spendable", hasSaplingSpendingKey); // TODO: If we found this entry via a UA, show that instead. obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address)); @@ -2980,7 +2992,7 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp) std::set addresses; pwalletMain->GetSproutPaymentAddresses(addresses); for (auto addr : addresses) { - if (fIncludeWatchonly || HaveSpendingKeyForPaymentAddress(pwalletMain)(addr)) { + if (fIncludeWatchonly || pwalletMain->HaveSproutSpendingKey(addr)) { ret.push_back(keyIO.EncodePaymentAddress(addr)); } } @@ -2989,7 +3001,7 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp) std::set addresses; pwalletMain->GetSaplingPaymentAddresses(addresses); for (auto addr : addresses) { - if (fIncludeWatchonly || HaveSpendingKeyForPaymentAddress(pwalletMain)(addr)) { + if (fIncludeWatchonly || pwalletMain->HaveSaplingSpendingKeyForAddress(addr)) { ret.push_back(keyIO.EncodePaymentAddress(addr)); } } @@ -3141,14 +3153,10 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) std::vector saplingEntries; pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, decoded, nMinDepth, false, false); - std::set> nullifierSet; - auto hasSpendingKey = std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), decoded.value()); - if (hasSpendingKey) { - nullifierSet = pwalletMain->GetNullifiersForAddresses(std::visit(GetRawAddresses(), decoded.value())); - } - std::visit(match { - [&](libzcash::SproutPaymentAddress addr) { + [&](const libzcash::SproutPaymentAddress& addr) { + bool hasSpendingKey = pwalletMain->HaveSproutSpendingKey(addr); + auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr}); for (SproutNoteEntry & entry : sproutEntries) { UniValue obj(UniValue::VOBJ); obj.pushKV("txid", entry.jsop.hash.ToString()); @@ -3171,7 +3179,9 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) result.push_back(obj); } }, - [&](libzcash::SaplingPaymentAddress addr) { + [&](const libzcash::SaplingPaymentAddress& addr) { + bool hasSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(addr); + auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr}); for (SaplingNoteEntry & entry : saplingEntries) { UniValue obj(UniValue::VOBJ); obj.pushKV("txid", entry.op.hash.ToString()); @@ -3187,12 +3197,12 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) obj.pushKV("blocktime", BlockData.time); if (hasSpendingKey) { - obj.pushKV("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)); + obj.pushKV("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)); } result.push_back(obj); } }, - [&](libzcash::UnifiedAddress) { + [&](const libzcash::UnifiedAddress& addr) { // TODO UNIFIED } }, decoded.value()); @@ -3721,6 +3731,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) auto fromaddress = params[0].get_str(); bool fromTaddr = false; bool fromSapling = false; + bool fromSprout = false; KeyIO keyIO(Params()); if (fromaddress == "ANY_TADDR") { fromTaddr = true; @@ -3728,27 +3739,24 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) CTxDestination taddr = keyIO.DecodeDestination(fromaddress); fromTaddr = IsValidDestination(taddr); if (!fromTaddr) { - auto decoded = keyIO.DecodePaymentAddress(fromaddress); - if (!decoded.has_value()) { + auto addr = keyIO.DecodePaymentAddress(fromaddress); + if (!addr.has_value()) { // invalid throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr."); } - // Check that we have the spending key - libzcash::PaymentAddress addr(decoded.value()); - if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), addr)) { + // This is a sanity check; the actual checks will come later when the spend is attempted. + if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), addr.value())) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key not found."); } // Remember whether this is a Sprout or Sapling address - fromSapling = std::holds_alternative(addr); + fromSapling = std::holds_alternative(addr.value()); + fromSprout = std::holds_alternative(addr.value()); } } - // This logic will need to be updated if we add a new shielded pool - bool fromSprout = !(fromTaddr || fromSapling); UniValue outputs = params[1].get_array(); - if (outputs.size()==0) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, amounts array is empty."); @@ -3781,13 +3789,12 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) bool isZaddr = false; CTxDestination taddr = keyIO.DecodeDestination(address); if (!IsValidDestination(taddr)) { - auto decoded = keyIO.DecodePaymentAddress(address); - if (decoded.has_value()) { - libzcash::PaymentAddress addr(decoded.value()); + auto addr = keyIO.DecodePaymentAddress(address); + if (addr.has_value()) { isZaddr = true; - bool toSapling = std::holds_alternative(addr); - bool toSprout = std::holds_alternative(addr); + bool toSapling = std::holds_alternative(addr.value()); + bool toSprout = std::holds_alternative(addr.value()); noSproutAddrs = !toSprout && noSproutAddrs && toSapling; containsSproutOutput |= toSprout; @@ -3885,19 +3892,20 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) for (int i = 0; i < zaddrRecipients.size(); i++) { auto address = zaddrRecipients[i].address; auto decoded = keyIO.DecodePaymentAddress(address); + if (decoded.has_value()) { std::visit(match { - [&](libzcash::SaplingPaymentAddress addr) { + [&](const libzcash::SaplingPaymentAddress& addr) { mtx.vShieldedOutput.push_back(OutputDescription()); }, - [&](libzcash::SproutPaymentAddress addr) { + [&](const libzcash::SproutPaymentAddress& addr) { JSDescription jsdesc; if (mtx.fOverwintered && (mtx.nVersion >= SAPLING_TX_VERSION)) { jsdesc.proof = GrothProof(); } mtx.vJoinSplit.push_back(jsdesc); }, - [&](libzcash::UnifiedAddress) { + [&](const libzcash::UnifiedAddress& ua) { // TODO UNIFIED } }, decoded.value()); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c6fd173fd..4504d5d30 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -25,6 +25,7 @@ #include "script/sign.h" #include "timedata.h" #include "utilmoneystr.h" +#include "util/match.h" #include "zcash/JoinSplit.hpp" #include "zcash/Note.hpp" #include "crypter.h" @@ -5166,7 +5167,7 @@ void CWallet::GetFilteredNotes( } // skip notes which cannot be spent - if (requireSpendingKey && !HaveSpendingKeyForPaymentAddress(this)(pa)) { + if (requireSpendingKey && !HaveSaplingSpendingKeyForAddress(pa)) { continue; } @@ -5227,7 +5228,7 @@ PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::Sapl if (m_wallet->mapSaplingZKeyMetadata.count(ivk) > 0 && m_wallet->mapSaplingZKeyMetadata[ivk].hdKeypath != "") { return LegacyHDSeed; - } else if (HaveSpendingKeyForPaymentAddress(m_wallet)(zaddr)) { + } else if (m_wallet->HaveSaplingSpendingKeyForAddress(zaddr)) { return Imported; } else { return ImportedWatchOnly; diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index fd95418d4..83792eddf 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -125,7 +125,7 @@ typedef std::variant< SproutPaymentAddress, SaplingPaymentAddress, UnifiedAddress> PaymentAddress; -/** Viewing keys that can have a string encoding. */ +/** Viewing keys that can be decoded from a string representation. */ typedef std::variant< SproutViewingKey, SaplingExtendedFullViewingKey> ViewingKey; From 890e1d841d9046a1e0da1fa158fc7507e0a8d235 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 23 Dec 2021 15:08:11 -0700 Subject: [PATCH 04/13] Add raw transparent address types to PaymentAddress The addition of `UnifiedAddress` to the `PaymentAddress` type introduced the need for methods that interact with payment addresses to support transparent receivers as shielded ones, which is somewhat inconsistent with previous uses of the `PaymentAddress` type. This commit adds both `CKeyID` and `CScriptID` as new variants of the `PaymentAddress` type. Following commits will shift encoding and decoding to use the `PaymentAddress` type exclusively wherever possible, rather than the current mix of `CDestination` and `PaymentAddress` that complicates the wallet codebase. --- src/init.cpp | 16 +-- src/key_io.cpp | 105 ++++++++++++++---- src/miner.h | 10 +- src/rpc/misc.cpp | 22 ++++ .../asyncrpcoperation_shieldcoinbase.cpp | 8 ++ src/wallet/asyncrpcoperation_shieldcoinbase.h | 2 + src/wallet/rpcwallet.cpp | 68 +++++++----- src/wallet/wallet.cpp | 83 +++++++++++--- src/wallet/wallet.h | 11 ++ src/zcash/Address.cpp | 60 ++++++---- src/zcash/Address.hpp | 10 +- 11 files changed, 295 insertions(+), 100 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 040ff0707..851c1f9bb 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1678,18 +1678,12 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) #ifdef ENABLE_WALLET bool minerAddressInLocalWallet = false; if (pwalletMain) { - CTxDestination addr = keyIO.DecodeDestination(mapArgs["-mineraddress"]); - if (IsValidDestination(addr)) { - CKeyID keyID = std::get(addr); - minerAddressInLocalWallet = pwalletMain->HaveKey(keyID); - } else { - auto zaddr = keyIO.DecodePaymentAddress(mapArgs["-mineraddress"]); - if (!zaddr.has_value()) { - return InitError(_("-mineraddress is not a valid zcash address.")); - } - minerAddressInLocalWallet = std::visit( - HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr.value()); + auto zaddr = keyIO.DecodePaymentAddress(mapArgs["-mineraddress"]); + if (!zaddr.has_value()) { + return InitError(_("-mineraddress is not a valid zcash address.")); } + minerAddressInLocalWallet = std::visit( + HaveSpendingKeyForPaymentAddress(pwalletMain), zaddr.value()); } if (GetBoolArg("-minetolocalwallet", true) && !minerAddressInLocalWallet) { return InitError(_("-mineraddress is not in the local wallet. Either use a local address, or set -minetolocalwallet=0")); diff --git a/src/key_io.cpp b/src/key_io.cpp index c8f180296..6c1a40344 100644 --- a/src/key_io.cpp +++ b/src/key_io.cpp @@ -16,6 +16,7 @@ #include #include #include +#include "util/match.h" namespace { @@ -113,6 +114,18 @@ private: public: PaymentAddressEncoder(const KeyConstants& keyConstants) : keyConstants(keyConstants) {} + std::string operator()(const CKeyID& id) const + { + std::vector data = keyConstants.Base58Prefix(KeyConstants::PUBKEY_ADDRESS); + data.insert(data.end(), id.begin(), id.end()); + return EncodeBase58Check(data); + } + std::string operator()(const CScriptID& id) const + { + std::vector data = keyConstants.Base58Prefix(KeyConstants::SCRIPT_ADDRESS); + data.insert(data.end(), id.begin(), id.end()); + return EncodeBase58Check(data); + } std::string operator()(const libzcash::SproutPaymentAddress& zaddr) const { CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); @@ -349,17 +362,16 @@ std::string KeyIO::EncodePaymentAddress(const libzcash::PaymentAddress& zaddr) return std::visit(PaymentAddressEncoder(keyConstants), zaddr); } -template -std::optional DecodeAny( - const KeyConstants& keyConstants, - const std::string& str, - std::pair sprout, - std::pair sapling) +template +std::optional DecodeSprout( + const KeyConstants& keyConstants, + const std::string& str, + const std::pair& keyMeta) { std::vector data; if (DecodeBase58Check(str, data)) { - const std::vector& prefix = keyConstants.Base58Prefix(sprout.first); - if ((data.size() == sprout.second + prefix.size()) && + const std::vector& prefix = keyConstants.Base58Prefix(keyMeta.first); + if ((data.size() == keyMeta.second + prefix.size()) && std::equal(prefix.begin(), prefix.end(), data.begin())) { CSerializeData serialized(data.begin() + prefix.size(), data.end()); CDataStream ss(serialized, SER_NETWORK, PROTOCOL_VERSION); @@ -371,15 +383,26 @@ std::optional DecodeAny( } } - data.clear(); + memory_cleanse(data.data(), data.size()); + return std::nullopt; +} + +template +std::optional DecodeSapling( + const KeyConstants& keyConstants, + const std::string& str, + const std::pair& keyMeta) +{ + std::vector data; + auto bech = bech32::Decode(str); - if (bech.first == keyConstants.Bech32HRP(sapling.first) && - bech.second.size() == sapling.second) { + if (bech.first == keyConstants.Bech32HRP(keyMeta.first) && + bech.second.size() == keyMeta.second) { // Bech32 decoding data.reserve((bech.second.size() * 5) / 8); if (ConvertBits<5, 8, false>([&](unsigned char c) { data.push_back(c); }, bech.second.begin(), bech.second.end())) { CDataStream ss(data, SER_NETWORK, PROTOCOL_VERSION); - T3 ret; + T2 ret; ss >> ret; memory_cleanse(data.data(), data.size()); return ret; @@ -390,6 +413,26 @@ std::optional DecodeAny( return std::nullopt; } +template +std::optional DecodeAny( + const KeyConstants& keyConstants, + const std::string& str, + const std::pair& sproutKeyMeta, + const std::pair& saplingKeyMeta) +{ + auto sprout = DecodeSprout(keyConstants, str, sproutKeyMeta); + if (sprout.has_value()) { + return sprout.value(); + } + + auto sapling = DecodeSapling(keyConstants, str, saplingKeyMeta); + if (sapling.has_value()) { + return sapling.value(); + } + + return std::nullopt; +} + /** * `raw` MUST be 43 bytes. */ @@ -457,15 +500,39 @@ std::optional KeyIO::DecodePaymentAddress(const std::s return ua; } - // Fall back on trying Sprout or Sapling. - return DecodeAny( + // Try parsing as a Sapling address + auto sapling = DecodeSapling( keyConstants, str, - std::make_pair(KeyConstants::ZCPAYMENT_ADDRESS, libzcash::SerializedSproutPaymentAddressSize), - std::make_pair(KeyConstants::SAPLING_PAYMENT_ADDRESS, ConvertedSaplingPaymentAddressSize) - ); + std::make_pair(KeyConstants::SAPLING_PAYMENT_ADDRESS, ConvertedSaplingPaymentAddressSize)); + if (sapling.has_value()) { + return sapling.value(); + } + + // Try parsing as a Sprout address + auto sprout = DecodeSprout( + keyConstants, + str, + std::make_pair(KeyConstants::ZCPAYMENT_ADDRESS, libzcash::SerializedSproutPaymentAddressSize)); + if (sprout.has_value()) { + return sprout.value(); + } + + // Finally, try parsing as transparent + return std::visit(match { + [](const CKeyID& keyIdIn) { + std::optional keyId = keyIdIn; + return keyId; + }, + [](const CScriptID& scriptIdIn) { + std::optional scriptId = scriptIdIn; + return scriptId; + }, + [](const CNoDestination& d) { + std::optional result = std::nullopt; + return result; + } + }, DecodeDestination(str)); } bool KeyIO::IsValidPaymentAddressString(const std::string& str) { diff --git a/src/miner.h b/src/miner.h index 61dcc938b..6f9073dc5 100644 --- a/src/miner.h +++ b/src/miner.h @@ -32,6 +32,12 @@ class ExtractMinerAddress public: ExtractMinerAddress() {} + std::optional operator()(const CKeyID &addr) const { + return std::nullopt; + } + std::optional operator()(const CScriptID &addr) const { + return std::nullopt; + } std::optional operator()(const libzcash::SproutPaymentAddress &addr) const { return std::nullopt; } @@ -40,7 +46,7 @@ public: } std::optional operator()(const libzcash::UnifiedAddress &addr) const { auto recipient = RecipientForPaymentAddress()(addr); - if (recipient) { + if (recipient.has_value()) { // This looks like a recursive call, but we are actually calling // ExtractMinerAddress with a different type: // - libzcash::PaymentAddress has a libzcash::UnifiedAddress @@ -51,7 +57,7 @@ public: // This works because std::visit does not require the visitor to // solely match the std::variant, only that it can handle all of // the variant's alternatives. - return std::visit(ExtractMinerAddress(), *recipient); + return std::visit(ExtractMinerAddress(), recipient.value()); } else { // Either the UA only contains unknown shielded receivers (unlikely that we // wouldn't know about them), or it only contains transparent receivers diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 0f21145b7..14f28d8f6 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -217,6 +217,28 @@ UniValue validateaddress(const UniValue& params, bool fHelp) class DescribePaymentAddressVisitor { public: + UniValue operator()(const CKeyID &addr) const { + UniValue obj(UniValue::VOBJ); + obj.pushKV("type", "p2pkh"); +#ifdef ENABLE_WALLET + if (pwalletMain) { + obj.pushKV("ismine", pwalletMain->HaveKey(addr)); + } +#endif + return obj; + } + + UniValue operator()(const CScriptID &addr) const { + UniValue obj(UniValue::VOBJ); + obj.pushKV("type", "p2sh"); +#ifdef ENABLE_WALLET + if (pwalletMain) { + obj.pushKV("ismine", pwalletMain->HaveCScript(addr)); + } +#endif + return obj; + } + UniValue operator()(const libzcash::SproutPaymentAddress &zaddr) const { UniValue obj(UniValue::VOBJ); obj.pushKV("type", "sprout"); diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp index 0b1960b76..9c564110a 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.cpp +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.cpp @@ -202,6 +202,14 @@ bool AsyncRPCOperation_shieldcoinbase::main_impl() { return std::visit(ShieldToAddress(this, sendAmount), tozaddr_); } +bool ShieldToAddress::operator()(const CKeyID &addr) const { + return false; +} + +bool ShieldToAddress::operator()(const CScriptID &addr) const { + return false; +} + bool ShieldToAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const { // update the transaction with these inputs CMutableTransaction rawTx(m_op->tx_); diff --git a/src/wallet/asyncrpcoperation_shieldcoinbase.h b/src/wallet/asyncrpcoperation_shieldcoinbase.h index c1048c00a..403e9f40c 100644 --- a/src/wallet/asyncrpcoperation_shieldcoinbase.h +++ b/src/wallet/asyncrpcoperation_shieldcoinbase.h @@ -103,6 +103,8 @@ public: ShieldToAddress(AsyncRPCOperation_shieldcoinbase *op, CAmount sendAmount) : m_op(op), sendAmount(sendAmount) {} + bool operator()(const CKeyID &zaddr) const; + bool operator()(const CScriptID &zaddr) const; bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; bool operator()(const libzcash::UnifiedAddress &uaddr) const; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 984d02969..60151e5bb 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2273,7 +2273,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) // We want to return unspent notes corresponding to any receiver within a // Unified Address. - for (const auto ra : std::visit(GetRawAddresses(), zaddr.value())) { + for (const auto ra : std::visit(GetRawShieldedAddresses(), zaddr.value())) { bool hasSpendingKey = std::visit(match { [&](const SaplingPaymentAddress& addr) { return pwalletMain->HaveSaplingSpendingKeyForAddress(addr); @@ -3154,10 +3154,16 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, decoded, nMinDepth, false, false); std::visit(match { + [&](const CKeyID& addr) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transparent addresses are not supported by this endpoint."); + }, + [&](const CScriptID& addr) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transparent addresses are not supported by this endpoint."); + }, [&](const libzcash::SproutPaymentAddress& addr) { bool hasSpendingKey = pwalletMain->HaveSproutSpendingKey(addr); auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr}); - for (SproutNoteEntry & entry : sproutEntries) { + for (const SproutNoteEntry& entry : sproutEntries) { UniValue obj(UniValue::VOBJ); obj.pushKV("txid", entry.jsop.hash.ToString()); obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); @@ -3182,7 +3188,7 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) [&](const libzcash::SaplingPaymentAddress& addr) { bool hasSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(addr); auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr}); - for (SaplingNoteEntry & entry : saplingEntries) { + for (const SaplingNoteEntry& entry : saplingEntries) { UniValue obj(UniValue::VOBJ); obj.pushKV("txid", entry.op.hash.ToString()); obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); @@ -3895,6 +3901,12 @@ UniValue z_sendmany(const UniValue& params, bool fHelp) if (decoded.has_value()) { std::visit(match { + [&](const CKeyID&) { + // Handled elsewhere + }, + [&](const CScriptID&) { + // Handled elsewhere + }, [&](const libzcash::SaplingPaymentAddress& addr) { mtx.vShieldedOutput.push_back(OutputDescription()); }, @@ -4458,7 +4470,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) if (zaddr.has_value()) { // We want to merge notes corresponding to any receiver within a // Unified Address. - for (const libzcash::RawAddress& ra : std::visit(GetRawAddresses(), zaddr.value())) { + for (const libzcash::RawAddress& ra : std::visit(GetRawShieldedAddresses(), zaddr.value())) { zaddrs.insert(ra); if (std::holds_alternative(ra)) { isFromNonSprout = true; @@ -4489,30 +4501,34 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) // Validate the destination address auto destaddress = params[1].get_str(); + bool isToTaddr = false; bool isToSproutZaddr = false; bool isToSaplingZaddr = false; - CTxDestination taddr = keyIO.DecodeDestination(destaddress); - if (!IsValidDestination(taddr)) { - auto zaddr = keyIO.DecodePaymentAddress(destaddress); - if (zaddr.has_value()) { - std::visit(match { - [&](libzcash::SaplingPaymentAddress addr) { - isToSaplingZaddr = true; - // If Sapling is not active, do not allow sending to a sapling addresses. - if (!saplingActive) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); - } - }, - [&](libzcash::SproutPaymentAddress addr) { - isToSproutZaddr = true; - }, - [&](libzcash::UnifiedAddress) { - // TODO UNIFIED + auto paymentAddress = keyIO.DecodePaymentAddress(destaddress); + if (paymentAddress.has_value()) { + std::visit(match { + [&](CKeyID addr) { + isToTaddr = true; + }, + [&](CScriptID addr) { + isToTaddr = true; + }, + [&](libzcash::SaplingPaymentAddress addr) { + isToSaplingZaddr = true; + // If Sapling is not active, do not allow sending to a sapling addresses. + if (!saplingActive) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, Sapling has not activated"); } - }, zaddr.value()); - } else { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress ); - } + }, + [&](libzcash::SproutPaymentAddress addr) { + isToSproutZaddr = true; + }, + [&](libzcash::UnifiedAddress) { + // TODO UNIFIED + } + }, paymentAddress.value()); + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ") + destaddress ); } if (canopyActive && isFromNonSprout && isToSproutZaddr) { @@ -4750,7 +4766,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) contextInfo.pushKV("toaddress", params[1]); contextInfo.pushKV("fee", ValueFromAmount(nFee)); - if (!sproutNoteInputs.empty() || !saplingNoteInputs.empty() || !IsValidDestination(taddr)) { + if (!sproutNoteInputs.empty() || !saplingNoteInputs.empty() || !isToTaddr) { // We have shielded inputs or the recipient is a shielded address, and // therefore we cannot create transactions before Sapling activates. if (!saplingActive) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4504d5d30..ae0673c4a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5041,7 +5041,7 @@ void CWallet::GetFilteredNotes( std::set filterAddresses; if (address.has_value()) { - for (const auto ra : std::visit(GetRawAddresses(), address.value())) { + for (const auto ra : std::visit(GetRawShieldedAddresses(), address.value())) { filterAddresses.insert(ra); } } @@ -5185,37 +5185,57 @@ void CWallet::GetFilteredNotes( // -// Shielded key and address generalizations +// Payment address operations // +// PaymentAddressBelongsToWallet + +bool PaymentAddressBelongsToWallet::operator()(const CKeyID &addr) const +{ + CScript script = GetScriptForDestination(addr); + return m_wallet->HaveKey(addr) || m_wallet->HaveWatchOnly(script); +} +bool PaymentAddressBelongsToWallet::operator()(const CScriptID &addr) const +{ + CScript script = GetScriptForDestination(addr); + return m_wallet->HaveCScript(addr) || m_wallet->HaveWatchOnly(script); +} bool PaymentAddressBelongsToWallet::operator()(const libzcash::SproutPaymentAddress &zaddr) const { return m_wallet->HaveSproutSpendingKey(zaddr) || m_wallet->HaveSproutViewingKey(zaddr); } - bool PaymentAddressBelongsToWallet::operator()(const libzcash::SaplingPaymentAddress &zaddr) const { libzcash::SaplingIncomingViewingKey ivk; // If we have a SaplingExtendedSpendingKey in the wallet, then we will // also have the corresponding SaplingExtendedFullViewingKey. - return m_wallet->GetSaplingIncomingViewingKey(zaddr, ivk) && + return + m_wallet->GetSaplingIncomingViewingKey(zaddr, ivk) && m_wallet->HaveSaplingFullViewingKey(ivk); } - bool PaymentAddressBelongsToWallet::operator()(const libzcash::UnifiedAddress &uaddr) const { // TODO return false; } -/// +// GetSourceForPaymentAddress +PaymentAddressSource GetSourceForPaymentAddress::operator()(const CKeyID &zaddr) const +{ + // TODO + return AddressNotFound; +} +PaymentAddressSource GetSourceForPaymentAddress::operator()(const CScriptID &zaddr) const +{ + // TODO + return AddressNotFound; +} PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const { return Random; } - PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::SaplingPaymentAddress &zaddr) const { libzcash::SaplingIncomingViewingKey ivk; @@ -5240,15 +5260,24 @@ PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::Sapl return AddressNotFound; } } - PaymentAddressSource GetSourceForPaymentAddress::operator()(const libzcash::UnifiedAddress &uaddr) const { // TODO return AddressNotFound; } -/// +// GetViewingKeyForPaymentAddress +std::optional GetViewingKeyForPaymentAddress::operator()( + const CKeyID &zaddr) const +{ + return std::nullopt; +} +std::optional GetViewingKeyForPaymentAddress::operator()( + const CScriptID &zaddr) const +{ + return std::nullopt; +} std::optional GetViewingKeyForPaymentAddress::operator()( const libzcash::SproutPaymentAddress &zaddr) const { @@ -5262,7 +5291,6 @@ std::optional GetViewingKeyForPaymentAddress::operator()( } return libzcash::ViewingKey(vk); } - std::optional GetViewingKeyForPaymentAddress::operator()( const libzcash::SaplingPaymentAddress &zaddr) const { @@ -5277,19 +5305,27 @@ std::optional GetViewingKeyForPaymentAddress::operator()( return std::nullopt; } } - std::optional GetViewingKeyForPaymentAddress::operator()( const libzcash::UnifiedAddress &uaddr) const { // TODO - return libzcash::ViewingKey(); + return std::nullopt; } +// HaveSpendingKeyForPaymentAddress + +bool HaveSpendingKeyForPaymentAddress::operator()(const CKeyID &addr) const +{ + return m_wallet->HaveKey(addr); +} +bool HaveSpendingKeyForPaymentAddress::operator()(const CScriptID &addr) const +{ + return m_wallet->HaveCScript(addr); +} bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::SproutPaymentAddress &zaddr) const { return m_wallet->HaveSproutSpendingKey(zaddr); } - bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::SaplingPaymentAddress &zaddr) const { libzcash::SaplingIncomingViewingKey ivk; @@ -5299,13 +5335,24 @@ bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::SaplingPayment m_wallet->GetSaplingFullViewingKey(ivk, extfvk) && m_wallet->HaveSaplingSpendingKey(extfvk); } - bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::UnifiedAddress &uaddr) const { // TODO return false; } +// GetSpendingKeyForPaymentAddress + +std::optional GetSpendingKeyForPaymentAddress::operator()( + const CKeyID &zaddr) const +{ + return std::nullopt; +} +std::optional GetSpendingKeyForPaymentAddress::operator()( + const CScriptID &zaddr) const +{ + return std::nullopt; +} std::optional GetSpendingKeyForPaymentAddress::operator()( const libzcash::SproutPaymentAddress &zaddr) const { @@ -5316,7 +5363,6 @@ std::optional GetSpendingKeyForPaymentAddress::operator() return std::nullopt; } } - std::optional GetSpendingKeyForPaymentAddress::operator()( const libzcash::SaplingPaymentAddress &zaddr) const { @@ -5327,7 +5373,6 @@ std::optional GetSpendingKeyForPaymentAddress::operator() return std::nullopt; } } - std::optional GetSpendingKeyForPaymentAddress::operator()( const libzcash::UnifiedAddress &uaddr) const { @@ -5335,6 +5380,8 @@ std::optional GetSpendingKeyForPaymentAddress::operator() return libzcash::SpendingKey(); } +// AddViewingKeyToWallet + KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SproutViewingKey &vkey) const { auto addr = vkey.address(); @@ -5348,7 +5395,6 @@ KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SproutViewingKey return KeyNotAdded; } } - KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SaplingExtendedFullViewingKey &extfvk) const { if (m_wallet->HaveSaplingSpendingKey(extfvk)) { return SpendingKeyExists; @@ -5361,6 +5407,8 @@ KeyAddResult AddViewingKeyToWallet::operator()(const libzcash::SaplingExtendedFu } } +// AddSpendingKeyToWallet + KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKey &sk) const { auto addr = sk.address(); KeyIO keyIO(Params()); @@ -5376,7 +5424,6 @@ KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SproutSpendingKe return KeyNotAdded; } } - KeyAddResult AddSpendingKeyToWallet::operator()(const libzcash::SaplingExtendedSpendingKey &sk) const { auto extfvk = sk.ToXFVK(); auto ivk = extfvk.fvk.in_viewing_key(); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 57fda6366..e1790dad5 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1369,6 +1369,8 @@ private: public: PaymentAddressBelongsToWallet(CWallet *wallet) : m_wallet(wallet) {} + bool operator()(const CKeyID &zaddr) const; + bool operator()(const CScriptID &zaddr) const; bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; bool operator()(const libzcash::UnifiedAddress &uaddr) const; @@ -1381,6 +1383,8 @@ private: public: GetViewingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} + std::optional operator()(const CKeyID &zaddr) const; + std::optional operator()(const CScriptID &zaddr) const; std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; @@ -1393,6 +1397,8 @@ private: public: HaveSpendingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} + bool operator()(const CKeyID &addr) const; + bool operator()(const CScriptID &addr) const; bool operator()(const libzcash::SproutPaymentAddress &zaddr) const; bool operator()(const libzcash::SaplingPaymentAddress &zaddr) const; bool operator()(const libzcash::UnifiedAddress &uaddr) const; @@ -1405,8 +1411,11 @@ private: public: GetSpendingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} + std::optional operator()(const CKeyID &zaddr) const; + std::optional operator()(const CScriptID &zaddr) const; std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; + // FIXME: this doesn't make sense std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; }; @@ -1426,6 +1435,8 @@ private: public: GetSourceForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} + PaymentAddressSource operator()(const CKeyID &zaddr) const; + PaymentAddressSource operator()(const CScriptID &zaddr) const; PaymentAddressSource operator()(const libzcash::SproutPaymentAddress &zaddr) const; PaymentAddressSource operator()(const libzcash::SaplingPaymentAddress &zaddr) const; PaymentAddressSource operator()(const libzcash::UnifiedAddress &uaddr) const; diff --git a/src/zcash/Address.cpp b/src/zcash/Address.cpp index fe048c1a9..121f90611 100644 --- a/src/zcash/Address.cpp +++ b/src/zcash/Address.cpp @@ -56,61 +56,67 @@ uint32_t TypecodeForReceiver::operator()( { return ZCASH_UA_TYPECODE_SAPLING; } - uint32_t TypecodeForReceiver::operator()( const CScriptID &p2sh) const { return ZCASH_UA_TYPECODE_P2SH; } - uint32_t TypecodeForReceiver::operator()( const CKeyID &p2sh) const { return ZCASH_UA_TYPECODE_P2PKH; } - uint32_t TypecodeForReceiver::operator()( const libzcash::UnknownReceiver &unknown) const { return unknown.typecode; } -std::optional ReceiverToRawAddress::operator()( - const libzcash::SaplingPaymentAddress &zaddr) const -{ - return zaddr; -} - -std::optional ReceiverToRawAddress::operator()( - const CScriptID &p2sh) const -{ - return std::nullopt; -} +// ReceiverToRawAddress std::optional ReceiverToRawAddress::operator()( const CKeyID &p2sh) const { return std::nullopt; } - +std::optional ReceiverToRawAddress::operator()( + const CScriptID &p2sh) const +{ + return std::nullopt; +} +std::optional ReceiverToRawAddress::operator()( + const libzcash::SaplingPaymentAddress &zaddr) const +{ + return zaddr; +} std::optional ReceiverToRawAddress::operator()( const libzcash::UnknownReceiver &p2sh) const { return std::nullopt; } +// RecipientForPaymentAddress + +std::optional RecipientForPaymentAddress::operator()( + const CKeyID &p2sh) const +{ + return std::nullopt; +} +std::optional RecipientForPaymentAddress::operator()( + const CScriptID &p2sh) const +{ + return std::nullopt; +} std::optional RecipientForPaymentAddress::operator()( const libzcash::SproutPaymentAddress &zaddr) const { return zaddr; } - std::optional RecipientForPaymentAddress::operator()( const libzcash::SaplingPaymentAddress &zaddr) const { return zaddr; } - std::optional RecipientForPaymentAddress::operator()( const libzcash::UnifiedAddress &uaddr) const { @@ -122,19 +128,29 @@ std::optional RecipientForPaymentAddress::operator()( return std::nullopt; } -std::set GetRawAddresses::operator()( +// GetRawShieldedAddresses + +std::set GetRawShieldedAddresses::operator()( + const CKeyID &addr) const +{ + return {}; +} +std::set GetRawShieldedAddresses::operator()( + const CScriptID &addr) const +{ + return {}; +} +std::set GetRawShieldedAddresses::operator()( const libzcash::SproutPaymentAddress &zaddr) const { return {zaddr}; } - -std::set GetRawAddresses::operator()( +std::set GetRawShieldedAddresses::operator()( const libzcash::SaplingPaymentAddress &zaddr) const { return {zaddr}; } - -std::set GetRawAddresses::operator()( +std::set GetRawShieldedAddresses::operator()( const libzcash::UnifiedAddress &uaddr) const { std::set ret; diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 83792eddf..5cd709c6c 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -122,6 +122,8 @@ public: /** Addresses that can appear in a string encoding. */ typedef std::variant< + CKeyID, + CScriptID, SproutPaymentAddress, SaplingPaymentAddress, UnifiedAddress> PaymentAddress; @@ -181,6 +183,8 @@ class RecipientForPaymentAddress { public: RecipientForPaymentAddress() {} + std::optional operator()(const CKeyID &addr) const; + std::optional operator()(const CScriptID &addr) const; std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; @@ -189,10 +193,12 @@ public: /** * Returns all protocol addresses contained within the given payment address. */ -class GetRawAddresses { +class GetRawShieldedAddresses { public: - GetRawAddresses() {} + GetRawShieldedAddresses() {} + std::set operator()(const CKeyID &addr) const; + std::set operator()(const CScriptID &addr) const; std::set operator()(const libzcash::SproutPaymentAddress &zaddr) const; std::set operator()(const libzcash::SaplingPaymentAddress &zaddr) const; std::set operator()(const libzcash::UnifiedAddress &uaddr) const; From 4f2ff2a9f8579b81968082e14e64c75a14f2a140 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Mon, 20 Dec 2021 12:25:44 -0700 Subject: [PATCH 05/13] Remove spurious variant from asyncrpcoperation_sendmany The `spendingkey_` field previously used by asyncrpcoperation_sendmany caused a situation where call sites would assume the type of the spending key based upon boolean flags that were derived elsewhere. In attempting to support unified addresses, it is desirable to remove these boolean deductions and instead work primarily based on the direct evidence of the data at hand. This commit therefore removes the `spendingkey_` field that could potentially produce results that would disagree with these boolean values, in favor of such direct evidence, making these call sites a bit less error prone. --- src/wallet/asyncrpcoperation_sendmany.cpp | 15 +++--- src/wallet/asyncrpcoperation_sendmany.h | 1 - src/wallet/rpcdump.cpp | 66 ++++++++++++++++++----- src/wallet/wallet.cpp | 56 +++++++++++++++---- src/wallet/wallet.h | 29 +++++++--- 5 files changed, 128 insertions(+), 39 deletions(-) diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 30b899207..e2493e161 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -106,7 +106,6 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany( isfromzaddr_ = true; frompaymentaddress_ = address.value(); - spendingkey_ = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address.value()).value(); } else { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address"); } @@ -306,9 +305,9 @@ bool AsyncRPCOperation_sendmany::main_impl() { // Get various necessary keys SaplingExpandedSpendingKey expsk; uint256 ovk; - if (isfromzaddr_) { - auto sk = std::get(spendingkey_); - expsk = sk.expsk; + auto saplingKey = std::visit(GetSaplingKeyForPaymentAddress(pwalletMain), frompaymentaddress_); + if (saplingKey.has_value()) { + expsk = saplingKey.value().expsk; ovk = expsk.full_viewing_key().ovk; } else { // Sending from a t-address, which we don't have an ovk for. Instead, @@ -523,6 +522,7 @@ bool AsyncRPCOperation_sendmany::main_impl() { UniValue obj(UniValue::VOBJ); while (zOutputsDeque.size() > 0) { AsyncJoinSplitInfo info; + // FIXME: make sure this .value() call is safe info.vpub_old = 0; info.vpub_new = 0; int n = 0; @@ -641,7 +641,9 @@ bool AsyncRPCOperation_sendmany::main_impl() { intermediates.insert(std::make_pair(tree.root(), tree)); // chained js are interstitial (found in between block boundaries) // Decrypt the change note's ciphertext to retrieve some data we need - ZCNoteDecryption decryptor(std::get(spendingkey_).receiving_key()); + // FIXME: make sure this .value() call is safe + auto sk = std::visit(GetSproutKeyForPaymentAddress(pwalletMain), frompaymentaddress_).value(); + ZCNoteDecryption decryptor(sk.receiving_key()); auto hSig = ZCJoinSplit::h_sig( prevJoinSplit.randomSeed, prevJoinSplit.nullifiers, @@ -1023,7 +1025,8 @@ UniValue AsyncRPCOperation_sendmany::perform_joinsplit( if (!witnesses[i]) { throw runtime_error("joinsplit input could not be found in tree"); } - info.vjsin.push_back(JSInput(*witnesses[i], info.notes[i], std::get(spendingkey_))); + auto sk = std::visit(GetSproutKeyForPaymentAddress(pwalletMain), frompaymentaddress_).value(); + info.vjsin.push_back(JSInput(*witnesses[i], info.notes[i], sk)); } // Make sure there are two inputs and two outputs diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h index 7bd92dfea..cb3c4b9cb 100644 --- a/src/wallet/asyncrpcoperation_sendmany.h +++ b/src/wallet/asyncrpcoperation_sendmany.h @@ -104,7 +104,6 @@ private: bool isfromzaddr_; CTxDestination fromtaddr_; PaymentAddress frompaymentaddress_; - SpendingKey spendingkey_; Ed25519VerificationKey joinSplitPubKey_; Ed25519SigningKey joinSplitPrivKey_; diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 0ad8e24b1..faaea7cf0 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -3,6 +3,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php . #include "chain.h" +#include "core_io.h" #include "key_io.h" #include "rpc/server.h" #include "init.h" @@ -11,6 +12,7 @@ #include "script/standard.h" #include "sync.h" #include "util.h" +#include "util/match.h" #include "utiltime.h" #include "wallet.h" @@ -67,7 +69,7 @@ std::string DecodeDumpString(const std::string &str) { for (unsigned int pos = 0; pos < str.length(); pos++) { unsigned char c = str[pos]; if (c == '%' && pos+2 < str.length()) { - c = (((str[pos+1]>>6)*9+((str[pos+1]-'0')&15)) << 4) | + c = (((str[pos+1]>>6)*9+((str[pos+1]-'0')&15)) << 4) | ((str[pos+2]>>6)*9+((str[pos+2]-'0')&15)); pos += 2; } @@ -80,7 +82,7 @@ UniValue importprivkey(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - + if (fHelp || params.size() < 1 || params.size() > 3) throw runtime_error( "importprivkey \"zcashprivkey\" ( \"label\" rescan )\n" @@ -189,7 +191,7 @@ UniValue importaddress(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - + if (fHelp || params.size() < 1 || params.size() > 4) throw runtime_error( "importaddress \"address\" ( \"label\" rescan p2sh )\n" @@ -338,7 +340,7 @@ UniValue importwallet(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - + if (fHelp || params.size() != 1) throw runtime_error( "importwallet \"filename\"\n" @@ -476,7 +478,7 @@ UniValue dumpprivkey(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - + if (fHelp || params.size() != 1) throw runtime_error( "dumpprivkey \"t-addr\"\n" @@ -520,7 +522,7 @@ UniValue z_exportwallet(const UniValue& params, bool fHelp) { if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - + if (fHelp || params.size() != 1) throw runtime_error( "z_exportwallet \"filename\"\n" @@ -769,10 +771,10 @@ UniValue z_importkey(const UniValue& params, bool fHelp) if (addResult == KeyNotAdded) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding spending key to wallet"); } - + // whenever a key is imported, we need to scan the whole chain pwalletMain->nTimeFirstKey = 1; // 0 would be considered 'no value' - + // We want to scan for transactions and notes if (fRescan) { pwalletMain->ScanForWalletTransactions(chainActive[nRescanHeight], true); @@ -909,12 +911,48 @@ UniValue z_exportkey(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr"); } - // Sapling support - auto sk = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address.value()); - if (!sk) { - throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold private zkey for this zaddr"); - } - return keyIO.EncodeSpendingKey(sk.value()); + std::string result = std::visit(match { + [&](const CKeyID& addr) { + CKey key; + if (pwalletMain->GetKey(addr, key)) { + return keyIO.EncodeSecret(key); + } else { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private key for this address."); + } + }, + [&](const CScriptID& addr) { + CScript redeemScript; + if (pwalletMain->GetCScript(addr, redeemScript)) { + return FormatScript(redeemScript); + } else { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private key for this address."); + } + }, + [&](const libzcash::SproutPaymentAddress& addr) { + libzcash::SproutSpendingKey key; + if (pwalletMain->GetSproutSpendingKey(addr, key)) { + return keyIO.EncodeSpendingKey(key); + } else { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private zkey for this zaddr"); + } + }, + [&](const libzcash::SaplingPaymentAddress& addr) { + libzcash::SaplingExtendedSpendingKey extsk; + if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) { + return keyIO.EncodeSpendingKey(extsk); + } else { + throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private zkey for this zaddr"); + } + }, + [&](const libzcash::UnifiedAddress& ua) { + throw JSONRPCError( + RPC_WALLET_ERROR, + "No serialized form is defined for unified spending keys. " + "Use the emergency recovery phrase for this wallet for backup purposes instead."); + return std::string(); //unreachable, here to make the compiler happy + } + }, address.value()); + return result; } UniValue z_exportviewingkey(const UniValue& params, bool fHelp) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ae0673c4a..efba55107 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5341,43 +5341,79 @@ bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::UnifiedAddress return false; } -// GetSpendingKeyForPaymentAddress +// GetSproutKeyForPaymentAddress -std::optional GetSpendingKeyForPaymentAddress::operator()( +std::optional GetSproutKeyForPaymentAddress::operator()( const CKeyID &zaddr) const { return std::nullopt; } -std::optional GetSpendingKeyForPaymentAddress::operator()( +std::optional GetSproutKeyForPaymentAddress::operator()( const CScriptID &zaddr) const { return std::nullopt; } -std::optional GetSpendingKeyForPaymentAddress::operator()( +std::optional GetSproutKeyForPaymentAddress::operator()( const libzcash::SproutPaymentAddress &zaddr) const { libzcash::SproutSpendingKey k; if (m_wallet->GetSproutSpendingKey(zaddr, k)) { - return libzcash::SpendingKey(k); + return k; } else { return std::nullopt; } } -std::optional GetSpendingKeyForPaymentAddress::operator()( +std::optional GetSproutKeyForPaymentAddress::operator()( + const libzcash::SaplingPaymentAddress &zaddr) const +{ + return std::nullopt; +} +std::optional GetSproutKeyForPaymentAddress::operator()( + const libzcash::UnifiedAddress &uaddr) const +{ + return std::nullopt; +} + +// GetSaplingKeyForPaymentAddress + +std::optional GetSaplingKeyForPaymentAddress::operator()( + const CKeyID &zaddr) const +{ + return std::nullopt; +} +std::optional GetSaplingKeyForPaymentAddress::operator()( + const CScriptID &zaddr) const +{ + return std::nullopt; +} +std::optional GetSaplingKeyForPaymentAddress::operator()( + const libzcash::SproutPaymentAddress &zaddr) const +{ + return std::nullopt; +} +std::optional GetSaplingKeyForPaymentAddress::operator()( const libzcash::SaplingPaymentAddress &zaddr) const { libzcash::SaplingExtendedSpendingKey extsk; if (m_wallet->GetSaplingExtendedSpendingKey(zaddr, extsk)) { - return libzcash::SpendingKey(extsk); + return extsk; } else { return std::nullopt; } } -std::optional GetSpendingKeyForPaymentAddress::operator()( +std::optional GetSaplingKeyForPaymentAddress::operator()( const libzcash::UnifiedAddress &uaddr) const { - // TODO - return libzcash::SpendingKey(); + for (const libzcash::Receiver& receiver: uaddr) { + auto saplingAddr = std::get_if(&receiver); + if (saplingAddr != nullptr) { + libzcash::SaplingExtendedSpendingKey extsk; + if (m_wallet->GetSaplingExtendedSpendingKey(*saplingAddr, extsk)) { + return extsk; + } + } + } + return std::nullopt; } // AddViewingKeyToWallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e1790dad5..bbc6ecfc2 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1404,19 +1404,32 @@ public: bool operator()(const libzcash::UnifiedAddress &uaddr) const; }; -class GetSpendingKeyForPaymentAddress +class GetSproutKeyForPaymentAddress { private: CWallet *m_wallet; public: - GetSpendingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} + GetSproutKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} - std::optional operator()(const CKeyID &zaddr) const; - std::optional operator()(const CScriptID &zaddr) const; - std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; - std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - // FIXME: this doesn't make sense - std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; + std::optional operator()(const CKeyID &zaddr) const; + std::optional operator()(const CScriptID &zaddr) const; + std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; + std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; + std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; +}; + +class GetSaplingKeyForPaymentAddress +{ +private: + CWallet *m_wallet; +public: + GetSaplingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {} + + std::optional operator()(const CKeyID &zaddr) const; + std::optional operator()(const CScriptID &zaddr) const; + std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; + std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; + std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; }; enum PaymentAddressSource { From f5d4f6fef1c3553717ca589f4bb881afd2c63898 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 23 Dec 2021 23:06:03 -0700 Subject: [PATCH 06/13] Remove `RawAddress` This adds a new `AddrSet` type for use in note retrieval as a filter, in place of a heterogeneous list of `RawAddress` values. `RawAddress` will complicate the handling of addresses within the wallet after the addition of unified addresses, because it does not contain transparent receiver types, and if we retain this polymorphism it means a lot of invalid states are represented in places we don't want them to be. It's better to figure out what types of addresses we're working with as soon as possible after parsing, and use monomorphic calls from there on in. --- qa/rpc-tests/wallet_shieldingcoinbase.py | 2 +- src/miner.h | 23 +-- src/rust/src/zip339_ffi.rs | 2 +- src/wallet/asyncrpcoperation_sendmany.cpp | 6 +- src/wallet/rpcwallet.cpp | 183 ++++++++++------------ src/wallet/wallet.cpp | 151 +++++++++++------- src/wallet/wallet.h | 51 ++++-- src/zcash/Address.cpp | 91 ----------- src/zcash/Address.hpp | 44 ------ 9 files changed, 228 insertions(+), 325 deletions(-) diff --git a/qa/rpc-tests/wallet_shieldingcoinbase.py b/qa/rpc-tests/wallet_shieldingcoinbase.py index c6be8fdce..01f7ee8e6 100755 --- a/qa/rpc-tests/wallet_shieldingcoinbase.py +++ b/qa/rpc-tests/wallet_shieldingcoinbase.py @@ -151,7 +151,7 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): results = self.nodes[1].z_listunspent(1, 999, False, [myzaddr]) except JSONRPCException as e: errorString = e.error['message'] - assert_equal("Invalid parameter, spending key for address does not belong to wallet" in errorString, True) + assert_equal("Invalid parameter, spending key for an address does not belong to the wallet.", errorString) # Verify that debug=zrpcunsafe logs params, and that full txid is associated with opid initialized_line = check_node_log(self, 0, myopid + ": z_sendmany initialized", False) diff --git a/src/miner.h b/src/miner.h index 6f9073dc5..3b2081f58 100644 --- a/src/miner.h +++ b/src/miner.h @@ -45,25 +45,12 @@ public: return addr; } std::optional operator()(const libzcash::UnifiedAddress &addr) const { - auto recipient = RecipientForPaymentAddress()(addr); - if (recipient.has_value()) { - // This looks like a recursive call, but we are actually calling - // ExtractMinerAddress with a different type: - // - libzcash::PaymentAddress has a libzcash::UnifiedAddress - // alternative, which invokes this method. - // - RecipientForPaymentAddress() returns libzcash::RawAddress, - // which does not have a libzcash::UnifiedAddress alternative. - // - // This works because std::visit does not require the visitor to - // solely match the std::variant, only that it can handle all of - // the variant's alternatives. - return std::visit(ExtractMinerAddress(), recipient.value()); - } else { - // Either the UA only contains unknown shielded receivers (unlikely that we - // wouldn't know about them), or it only contains transparent receivers - // (which are invalid). - return std::nullopt; + for (const auto& receiver: addr) { + if (std::holds_alternative(receiver)) { + return std::get(receiver); + } } + return std::nullopt; } }; diff --git a/src/rust/src/zip339_ffi.rs b/src/rust/src/zip339_ffi.rs index eeafa5bcd..852c10921 100644 --- a/src/rust/src/zip339_ffi.rs +++ b/src/rust/src/zip339_ffi.rs @@ -63,7 +63,7 @@ pub extern "C" fn zip339_free_phrase(phrase: *const c_char) { if !phrase.is_null() { unsafe { // It is correct to cast away const here; the memory is not actually immutable. - CString::from_raw(phrase as *mut c_char); + drop(CString::from_raw(phrase as *mut c_char)); } } } diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index e2493e161..ac2880e86 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -935,7 +935,11 @@ bool AsyncRPCOperation_sendmany::find_unspent_notes() { std::vector saplingEntries; // TODO: move this to the caller auto zaddr = KeyIO(Params()).DecodePaymentAddress(fromaddress_); - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddr, mindepth_); + std::optional noteFilter = std::nullopt; + if (zaddr.has_value()) { + noteFilter = AddrSet::ForPaymentAddresses(std::vector({zaddr.value()})); + } + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, mindepth_); // If using the TransactionBuilder, we only want Sapling notes. // If not using it, we only want Sprout notes. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 60151e5bb..445175d73 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1987,7 +1987,7 @@ UniValue settxfee(const UniValue& params, bool fHelp) return true; } -CAmount getBalanceZaddr(std::optional address, int minDepth = 1, int maxDepth = INT_MAX, bool ignoreUnspendable=true); +CAmount getBalanceZaddr(std::optional address, int minDepth = 1, int maxDepth = INT_MAX, bool ignoreUnspendable=true); UniValue getwalletinfo(const UniValue& params, bool fHelp) { @@ -2241,8 +2241,6 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Maximum number of confirmations must be greater or equal to the minimum number of confirmations"); } - std::set zaddrs = {}; - bool fIncludeWatchonly = false; if (params.size() > 2) { fIncludeWatchonly = params[2].get_bool(); @@ -2250,109 +2248,93 @@ UniValue z_listunspent(const UniValue& params, bool fHelp) LOCK2(cs_main, pwalletMain->cs_wallet); + std::optional noteFilter = std::nullopt; + std::set> sproutNullifiers; + std::set> saplingNullifiers; + KeyIO keyIO(Params()); // User has supplied zaddrs to filter on if (params.size() > 3) { UniValue addresses = params[3].get_array(); - if (addresses.size()==0) + if (addresses.size() == 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, addresses array is empty."); - - // Keep track of addresses to spot duplicates - set setAddress; + } // Sources + std::vector sourceAddrs; for (const UniValue& o : addresses.getValues()) { if (!o.isStr()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected string"); } - string address = o.get_str(); - auto zaddr = keyIO.DecodePaymentAddress(address); + + auto zaddr = keyIO.DecodePaymentAddress(o.get_str()); if (!zaddr.has_value()) { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, address is not a valid zaddr: ") + address); + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, not a valid Zcash address: ") + o.get_str()); } - // We want to return unspent notes corresponding to any receiver within a - // Unified Address. - for (const auto ra : std::visit(GetRawShieldedAddresses(), zaddr.value())) { - bool hasSpendingKey = std::visit(match { - [&](const SaplingPaymentAddress& addr) { - return pwalletMain->HaveSaplingSpendingKeyForAddress(addr); - }, - [&](const SproutPaymentAddress& addr) { - return pwalletMain->HaveSproutSpendingKey(addr); - } - }, ra); - - // If we don't include watchonly addresses, we must reject any address - // for which we do not have the spending key. - if (!fIncludeWatchonly && !hasSpendingKey) { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for address does not belong to wallet: ") + address); - } - - zaddrs.insert(ra); - } - - if (setAddress.count(address)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, duplicated address: ") + address); - } - setAddress.insert(address); + sourceAddrs.push_back(zaddr.value()); } - } - else { + + noteFilter = AddrSet::ForPaymentAddresses(sourceAddrs); + sproutNullifiers = pwalletMain->GetSproutNullifiers(noteFilter.value().GetSproutAddresses()); + saplingNullifiers = pwalletMain->GetSaplingNullifiers(noteFilter.value().GetSaplingAddresses()); + + // If we don't include watchonly addresses, we must reject any address + // for which we do not have the spending key. + if (!fIncludeWatchonly && !pwalletMain->HasSpendingKeys(noteFilter.value())) { + throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, spending key for an address does not belong to the wallet.")); + } + } else { // User did not provide zaddrs, so use default i.e. all addresses std::set sproutzaddrs = {}; pwalletMain->GetSproutPaymentAddresses(sproutzaddrs); + sproutNullifiers = pwalletMain->GetSproutNullifiers(sproutzaddrs); // Sapling support std::set saplingzaddrs = {}; pwalletMain->GetSaplingPaymentAddresses(saplingzaddrs); - - zaddrs.insert(sproutzaddrs.begin(), sproutzaddrs.end()); - zaddrs.insert(saplingzaddrs.begin(), saplingzaddrs.end()); + saplingNullifiers = pwalletMain->GetSaplingNullifiers(saplingzaddrs); } UniValue results(UniValue::VARR); - if (zaddrs.size() > 0) { - std::vector sproutEntries; - std::vector saplingEntries; - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddrs, nMinDepth, nMaxDepth, true, !fIncludeWatchonly, false); - auto nullifierSet = pwalletMain->GetNullifiersForAddresses(zaddrs); + std::vector sproutEntries; + std::vector saplingEntries; + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, nMinDepth, nMaxDepth, true, !fIncludeWatchonly, false); - for (auto & entry : sproutEntries) { - UniValue obj(UniValue::VOBJ); - obj.pushKV("txid", entry.jsop.hash.ToString()); - obj.pushKV("jsindex", (int)entry.jsop.js ); - obj.pushKV("jsoutindex", (int)entry.jsop.n); - obj.pushKV("confirmations", entry.confirmations); - bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(entry.address); - obj.pushKV("spendable", hasSproutSpendingKey); - obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address)); - obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); - std::string data(entry.memo.begin(), entry.memo.end()); - obj.pushKV("memo", HexStr(data)); - if (hasSproutSpendingKey) { - obj.pushKV("change", pwalletMain->IsNoteSproutChange(nullifierSet, entry.address, entry.jsop)); - } - results.push_back(obj); + for (auto & entry : sproutEntries) { + UniValue obj(UniValue::VOBJ); + obj.pushKV("txid", entry.jsop.hash.ToString()); + obj.pushKV("jsindex", (int)entry.jsop.js ); + obj.pushKV("jsoutindex", (int)entry.jsop.n); + obj.pushKV("confirmations", entry.confirmations); + bool hasSproutSpendingKey = pwalletMain->HaveSproutSpendingKey(entry.address); + obj.pushKV("spendable", hasSproutSpendingKey); + obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address)); + obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); + std::string data(entry.memo.begin(), entry.memo.end()); + obj.pushKV("memo", HexStr(data)); + if (hasSproutSpendingKey) { + obj.pushKV("change", pwalletMain->IsNoteSproutChange(sproutNullifiers, entry.address, entry.jsop)); } + results.push_back(obj); + } - for (auto & entry : saplingEntries) { - UniValue obj(UniValue::VOBJ); - obj.pushKV("txid", entry.op.hash.ToString()); - obj.pushKV("outindex", (int)entry.op.n); - obj.pushKV("confirmations", entry.confirmations); - bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(entry.address); - obj.pushKV("spendable", hasSaplingSpendingKey); - // TODO: If we found this entry via a UA, show that instead. - obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address)); - obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); // note.value() is equivalent to plaintext.value() - obj.pushKV("memo", HexStr(entry.memo)); - if (hasSaplingSpendingKey) { - obj.pushKV("change", pwalletMain->IsNoteSaplingChange(nullifierSet, entry.address, entry.op)); - } - results.push_back(obj); + for (auto & entry : saplingEntries) { + UniValue obj(UniValue::VOBJ); + obj.pushKV("txid", entry.op.hash.ToString()); + obj.pushKV("outindex", (int)entry.op.n); + obj.pushKV("confirmations", entry.confirmations); + bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(entry.address); + obj.pushKV("spendable", hasSaplingSpendingKey); + // TODO: If we found this entry via a UA, show that instead. + obj.pushKV("address", keyIO.EncodePaymentAddress(entry.address)); + obj.pushKV("amount", ValueFromAmount(CAmount(entry.note.value()))); // note.value() is equivalent to plaintext.value() + obj.pushKV("memo", HexStr(entry.memo)); + if (hasSaplingSpendingKey) { + obj.pushKV("change", pwalletMain->IsNoteSaplingChange(saplingNullifiers, entry.address, entry.op)); } + results.push_back(obj); } return results; @@ -3053,18 +3035,18 @@ CAmount getBalanceTaddr(std::string transparentAddress, int minDepth=1, bool ign return balance; } -CAmount getBalanceZaddr(std::optional address, int minDepth, int maxDepth, bool ignoreUnspendable) { +CAmount getBalanceZaddr(std::optional address, int minDepth, int maxDepth, bool ignoreUnspendable) { CAmount balance = 0; std::vector sproutEntries; std::vector saplingEntries; LOCK2(cs_main, pwalletMain->cs_wallet); - std::set filterAddresses; - if (address) { - filterAddresses.insert(address.value()); + std::optional noteFilter = std::nullopt; + if (address.has_value()) { + noteFilter = AddrSet::ForPaymentAddresses(std::vector({address.value()})); } - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, filterAddresses, minDepth, maxDepth, true, ignoreUnspendable); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, minDepth, maxDepth, true, ignoreUnspendable); for (auto & entry : sproutEntries) { balance += CAmount(entry.note.value()); } @@ -3151,7 +3133,8 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) UniValue result(UniValue::VARR); std::vector sproutEntries; std::vector saplingEntries; - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, decoded, nMinDepth, false, false); + auto noteFilter = AddrSet::ForPaymentAddresses(std::vector({decoded.value()})); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter, nMinDepth, INT_MAX, false, false); std::visit(match { [&](const CKeyID& addr) { @@ -3162,7 +3145,10 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) }, [&](const libzcash::SproutPaymentAddress& addr) { bool hasSpendingKey = pwalletMain->HaveSproutSpendingKey(addr); - auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr}); + std::set> nullifierSet; + if (hasSpendingKey) { + nullifierSet = pwalletMain->GetSproutNullifiers({addr}); + } for (const SproutNoteEntry& entry : sproutEntries) { UniValue obj(UniValue::VOBJ); obj.pushKV("txid", entry.jsop.hash.ToString()); @@ -3187,7 +3173,10 @@ UniValue z_listreceivedbyaddress(const UniValue& params, bool fHelp) }, [&](const libzcash::SaplingPaymentAddress& addr) { bool hasSpendingKey = pwalletMain->HaveSaplingSpendingKeyForAddress(addr); - auto nullifierSet = pwalletMain->GetNullifiersForAddresses({addr}); + std::set> nullifierSet; + if (hasSpendingKey) { + nullifierSet = pwalletMain->GetSaplingNullifiers({addr}); + } for (const SaplingNoteEntry& entry : saplingEntries) { UniValue obj(UniValue::VOBJ); obj.pushKV("txid", entry.op.hash.ToString()); @@ -3273,8 +3262,7 @@ UniValue z_getbalance(const UniValue& params, bool fHelp) nBalance = getBalanceTaddr(fromaddress, nMinDepth, false); } else { // TODO: Return an error if a UA is provided (once we support UAs). - auto zaddr = std::visit(RecipientForPaymentAddress(), pa.value()).value(); - nBalance = getBalanceZaddr(zaddr, nMinDepth, INT_MAX, false); + nBalance = getBalanceZaddr(pa, nMinDepth, INT_MAX, false); } // inZat @@ -4069,10 +4057,9 @@ UniValue z_getmigrationstatus(const UniValue& params, bool fHelp) { { std::vector sproutEntries; std::vector saplingEntries; - std::set noFilter; // Here we are looking for any and all Sprout notes for which we have the spending key, including those // which are locked and/or only exist in the mempool, as they should be included in the unmigrated amount. - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noFilter, 0, INT_MAX, true, true, false); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, std::nullopt, 0, INT_MAX, true, true, false); CAmount unmigratedAmount = 0; for (const auto& sproutEntry : sproutEntries) { unmigratedAmount += sproutEntry.note.value(); @@ -4432,8 +4419,8 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) bool useAnyUTXO = false; bool useAnySprout = false; bool useAnySapling = false; - std::set taddrs = {}; - std::set zaddrs = {}; + std::set taddrs; + std::vector zaddrs; UniValue addresses = params[0].get_array(); if (addresses.size()==0) @@ -4468,13 +4455,9 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) } else { auto zaddr = keyIO.DecodePaymentAddress(address); if (zaddr.has_value()) { - // We want to merge notes corresponding to any receiver within a - // Unified Address. - for (const libzcash::RawAddress& ra : std::visit(GetRawShieldedAddresses(), zaddr.value())) { - zaddrs.insert(ra); - if (std::holds_alternative(ra)) { - isFromNonSprout = true; - } + zaddrs.push_back(zaddr.value()); + if (std::holds_alternative(zaddr.value())) { + isFromNonSprout = true; } } else { throw JSONRPCError(RPC_INVALID_PARAMETER, string("Unknown address format: ") + address); @@ -4651,7 +4634,11 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp) // Get available notes std::vector sproutEntries; std::vector saplingEntries; - pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, zaddrs); + std::optional noteFilter = + useAnySprout || useAnySapling ? + std::nullopt : + std::optional(AddrSet::ForPaymentAddresses(zaddrs)); + pwalletMain->GetFilteredNotes(sproutEntries, saplingEntries, noteFilter); // If Sapling is not active, do not allow sending from a sapling addresses. if (!saplingActive && saplingEntries.size() > 0) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index efba55107..0d37b8e34 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -684,49 +684,58 @@ void CWallet::SetBestChain(const CBlockLocator& loc) SetBestChainINTERNAL(walletdb, loc); } -std::set> CWallet::GetNullifiersForAddresses( - const std::set & addresses) -{ - std::set> nullifierSet; - // Sapling ivk -> list of addrs map - // (There may be more than one diversified address for a given ivk.) - std::map> ivkMap; - for (const auto & addr : addresses) { - auto saplingAddr = std::get_if(&addr); - if (saplingAddr != nullptr) { - libzcash::SaplingIncomingViewingKey ivk; - this->GetSaplingIncomingViewingKey(*saplingAddr, ivk); - ivkMap[ivk].push_back(*saplingAddr); - } - } - for (const auto & txPair : mapWallet) { - // Sprout - for (const auto & noteDataPair : txPair.second.mapSproutNoteData) { - auto & noteData = noteDataPair.second; - auto & nullifier = noteData.nullifier; - auto & address = noteData.address; - if (nullifier && addresses.count(address)) { - nullifierSet.insert(std::make_pair(address, nullifier.value())); - } - } - // Sapling - for (const auto & noteDataPair : txPair.second.mapSaplingNoteData) { - auto & noteData = noteDataPair.second; - auto & nullifier = noteData.nullifier; - auto & ivk = noteData.ivk; - if (nullifier && ivkMap.count(ivk)) { - for (const auto & addr : ivkMap[ivk]) { - nullifierSet.insert(std::make_pair(addr, nullifier.value())); +std::set> CWallet::GetSproutNullifiers( + const std::set& addresses) { + std::set> nullifierSet; + if (!addresses.empty()) { + for (const auto& txPair : mapWallet) { + for (const auto & noteDataPair : txPair.second.mapSproutNoteData) { + auto & noteData = noteDataPair.second; + auto & nullifier = noteData.nullifier; + auto & address = noteData.address; + if (nullifier && addresses.count(address) > 0) { + nullifierSet.insert(std::make_pair(address, nullifier.value())); } } } } + + return nullifierSet; +} + +std::set> CWallet::GetSaplingNullifiers( + const std::set& addresses) { + std::set> nullifierSet; + if (!addresses.empty()) { + // Sapling ivk -> list of addrs map + // (There may be more than one diversified address for a given ivk.) + std::map> ivkMap; + for (const auto & addr : addresses) { + libzcash::SaplingIncomingViewingKey ivk; + this->GetSaplingIncomingViewingKey(addr, ivk); + ivkMap[ivk].push_back(addr); + } + + for (const auto& txPair : mapWallet) { + for (const auto& noteDataPair : txPair.second.mapSaplingNoteData) { + auto & noteData = noteDataPair.second; + auto & nullifier = noteData.nullifier; + auto & ivk = noteData.ivk; + if (nullifier && ivkMap.count(ivk) > 0) { + for (const auto & addr : ivkMap[ivk]) { + nullifierSet.insert(std::make_pair(addr, nullifier.value())); + } + } + } + } + } + return nullifierSet; } bool CWallet::IsNoteSproutChange( - const std::set> & nullifierSet, - const libzcash::RawAddress & address, + const std::set> & nullifierSet, + const libzcash::SproutPaymentAddress& address, const JSOutPoint & jsop) { // A Note is marked as "change" if the address that received it @@ -748,8 +757,9 @@ bool CWallet::IsNoteSproutChange( return false; } -bool CWallet::IsNoteSaplingChange(const std::set> & nullifierSet, - const libzcash::RawAddress & address, +bool CWallet::IsNoteSaplingChange( + const std::set> & nullifierSet, + const libzcash::SaplingPaymentAddress& address, const SaplingOutPoint & op) { // A Note is marked as "change" if the address that received it @@ -760,6 +770,8 @@ bool CWallet::IsNoteSaplingChange(const std::set& sproutEntries, - std::vector& saplingEntries, - std::optional address, - int minDepth, - bool ignoreSpent, - bool requireSpendingKey) -{ - std::set filterAddresses; +AddrSet AddrSet::ForPaymentAddresses(const std::vector& paymentAddrs) { + AddrSet addrs; + for (const auto& addr: paymentAddrs) { + std::visit(match { + [&](const CKeyID& keyId) { }, + [&](const CScriptID& scriptId) { }, + [&](const libzcash::SproutPaymentAddress& addr) { + addrs.sproutAddresses.insert(addr); + }, + [&](const libzcash::SaplingPaymentAddress& addr) { + addrs.saplingAddresses.insert(addr); + }, + [&](const libzcash::UnifiedAddress& uaddr) { + for (auto& receiver : uaddr) { + std::visit(match { + [&](const libzcash::SaplingPaymentAddress& addr) { + addrs.saplingAddresses.insert(addr); + }, + [&](const auto& other) { } + }, receiver); + } + }, + }, addr); + } + return addrs; +} - if (address.has_value()) { - for (const auto ra : std::visit(GetRawShieldedAddresses(), address.value())) { - filterAddresses.insert(ra); +bool CWallet::HasSpendingKeys(const AddrSet& addrSet) const { + for (const auto& zaddr : addrSet.GetSproutAddresses()) { + if (!HaveSproutSpendingKey(zaddr)) { + return false; } } - - GetFilteredNotes(sproutEntries, saplingEntries, filterAddresses, minDepth, INT_MAX, ignoreSpent, requireSpendingKey); + for (const auto& zaddr : addrSet.GetSaplingAddresses()) { + if (!HaveSaplingSpendingKeyForAddress(zaddr)) { + return false; + } + } + return true; } + /** * Find notes in the wallet filtered by payment addresses, min depth, max depth, * if the note is spent, if a spending key is required, and if the notes are locked. @@ -5057,7 +5088,7 @@ void CWallet::GetFilteredNotes( void CWallet::GetFilteredNotes( std::vector& sproutEntries, std::vector& saplingEntries, - std::set& filterAddresses, + const std::optional& noteFilter, int minDepth, int maxDepth, bool ignoreSpent, @@ -5087,8 +5118,8 @@ void CWallet::GetFilteredNotes( SproutNoteData nd = pair.second; SproutPaymentAddress pa = nd.address; - // skip notes which belong to a different payment address in the wallet - if (!(filterAddresses.empty() || filterAddresses.count(pa))) { + // skip notes which do not conform to the filter, if supplied + if (noteFilter.has_value() && !noteFilter.value().HasSproutAddress(pa)) { continue; } @@ -5157,8 +5188,8 @@ void CWallet::GetFilteredNotes( assert(static_cast(maybe_pa)); auto pa = maybe_pa.value(); - // skip notes which belong to a different payment address in the wallet - if (!(filterAddresses.empty() || filterAddresses.count(pa))) { + // skip notes which do not conform to the filter, if supplied + if (noteFilter.has_value() && !noteFilter.value().HasSaplingAddress(pa)) { continue; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bbc6ecfc2..f32c4873b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -623,7 +623,31 @@ public: }; +class AddrSet { +private: + std::set sproutAddresses; + std::set saplingAddresses; + AddrSet() {} +public: + static AddrSet ForPaymentAddresses(const std::vector& addrs); + + const std::set& GetSproutAddresses() const { + return sproutAddresses; + } + + const std::set& GetSaplingAddresses() const { + return saplingAddresses; + } + + bool HasSproutAddress(libzcash::SproutPaymentAddress addr) const { + return sproutAddresses.count(addr) > 0; + } + + bool HasSaplingAddress(libzcash::SaplingPaymentAddress addr) const { + return saplingAddresses.count(addr) > 0; + } +}; class COutput { @@ -1194,9 +1218,20 @@ public: void AddPendingSaplingMigrationTx(const CTransaction& tx); /** Saves witness caches and best block locator to disk. */ void SetBestChain(const CBlockLocator& loc); - std::set> GetNullifiersForAddresses(const std::set & addresses); - bool IsNoteSproutChange(const std::set> & nullifierSet, const libzcash::RawAddress & address, const JSOutPoint & entry); - bool IsNoteSaplingChange(const std::set> & nullifierSet, const libzcash::RawAddress & address, const SaplingOutPoint & entry); + + std::set> GetSproutNullifiers( + const std::set& addresses); + bool IsNoteSproutChange( + const std::set> & nullifierSet, + const libzcash::SproutPaymentAddress& address, + const JSOutPoint & entry); + + std::set> GetSaplingNullifiers( + const std::set& addresses); + bool IsNoteSaplingChange( + const std::set> & nullifierSet, + const libzcash::SaplingPaymentAddress& address, + const SaplingOutPoint & entry); DBErrors LoadWallet(bool& fFirstRunRet); DBErrors ZapWalletTx(std::vector& vWtx); @@ -1304,19 +1339,13 @@ public: /* Set the current encrypted HD seed, without saving it to disk (used by LoadWallet) */ bool LoadCryptedHDSeed(const uint256& seedFp, const std::vector& seed); - /* Find notes filtered by (optional) payment address, min depth, ability to spend */ - void GetFilteredNotes(std::vector& sproutEntries, - std::vector& saplingEntries, - std::optional address, - int minDepth=1, - bool ignoreSpent=true, - bool requireSpendingKey=true); + bool HasSpendingKeys(const AddrSet& noteFilter) const; /* Find notes filtered by payment addresses, min depth, max depth, if they are spent, if a spending key is required, and if they are locked */ void GetFilteredNotes(std::vector& sproutEntries, std::vector& saplingEntries, - std::set& filterAddresses, + const std::optional& noteFilter, int minDepth=1, int maxDepth=INT_MAX, bool ignoreSpent=true, diff --git a/src/zcash/Address.cpp b/src/zcash/Address.cpp index 121f90611..65e0084e7 100644 --- a/src/zcash/Address.cpp +++ b/src/zcash/Address.cpp @@ -71,94 +71,3 @@ uint32_t TypecodeForReceiver::operator()( { return unknown.typecode; } - -// ReceiverToRawAddress - -std::optional ReceiverToRawAddress::operator()( - const CKeyID &p2sh) const -{ - return std::nullopt; -} -std::optional ReceiverToRawAddress::operator()( - const CScriptID &p2sh) const -{ - return std::nullopt; -} -std::optional ReceiverToRawAddress::operator()( - const libzcash::SaplingPaymentAddress &zaddr) const -{ - return zaddr; -} -std::optional ReceiverToRawAddress::operator()( - const libzcash::UnknownReceiver &p2sh) const -{ - return std::nullopt; -} - -// RecipientForPaymentAddress - -std::optional RecipientForPaymentAddress::operator()( - const CKeyID &p2sh) const -{ - return std::nullopt; -} -std::optional RecipientForPaymentAddress::operator()( - const CScriptID &p2sh) const -{ - return std::nullopt; -} -std::optional RecipientForPaymentAddress::operator()( - const libzcash::SproutPaymentAddress &zaddr) const -{ - return zaddr; -} -std::optional RecipientForPaymentAddress::operator()( - const libzcash::SaplingPaymentAddress &zaddr) const -{ - return zaddr; -} -std::optional RecipientForPaymentAddress::operator()( - const libzcash::UnifiedAddress &uaddr) const -{ - for (auto& receiver : uaddr) { - // Return the first one. - return std::visit(ReceiverToRawAddress(), receiver); - } - - return std::nullopt; -} - -// GetRawShieldedAddresses - -std::set GetRawShieldedAddresses::operator()( - const CKeyID &addr) const -{ - return {}; -} -std::set GetRawShieldedAddresses::operator()( - const CScriptID &addr) const -{ - return {}; -} -std::set GetRawShieldedAddresses::operator()( - const libzcash::SproutPaymentAddress &zaddr) const -{ - return {zaddr}; -} -std::set GetRawShieldedAddresses::operator()( - const libzcash::SaplingPaymentAddress &zaddr) const -{ - return {zaddr}; -} -std::set GetRawShieldedAddresses::operator()( - const libzcash::UnifiedAddress &uaddr) const -{ - std::set ret; - for (auto& receiver : uaddr) { - auto ra = std::visit(ReceiverToRawAddress(), receiver); - if (ra) { - ret.insert(*ra); - } - } - return ret; -} diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 5cd709c6c..11e4f5bfe 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -12,9 +12,6 @@ namespace libzcash { -/** Protocol addresses that can receive funds in a transaction. */ -typedef std::variant RawAddress; - class UnknownReceiver { public: uint32_t typecode; @@ -163,45 +160,4 @@ public: uint32_t operator()(const libzcash::UnknownReceiver &p2pkh) const; }; -/** - * Converts the given UA receiver to a protocol address, if it is a shielded receiver. - */ -class ReceiverToRawAddress { -public: - ReceiverToRawAddress() {} - - std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - std::optional operator()(const CScriptID &p2sh) const; - std::optional operator()(const CKeyID &p2pkh) const; - std::optional operator()(const libzcash::UnknownReceiver &p2pkh) const; -}; - -/** - * Returns the protocol address that should be used in transaction outputs. - */ -class RecipientForPaymentAddress { -public: - RecipientForPaymentAddress() {} - - std::optional operator()(const CKeyID &addr) const; - std::optional operator()(const CScriptID &addr) const; - std::optional operator()(const libzcash::SproutPaymentAddress &zaddr) const; - std::optional operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - std::optional operator()(const libzcash::UnifiedAddress &uaddr) const; -}; - -/** - * Returns all protocol addresses contained within the given payment address. - */ -class GetRawShieldedAddresses { -public: - GetRawShieldedAddresses() {} - - std::set operator()(const CKeyID &addr) const; - std::set operator()(const CScriptID &addr) const; - std::set operator()(const libzcash::SproutPaymentAddress &zaddr) const; - std::set operator()(const libzcash::SaplingPaymentAddress &zaddr) const; - std::set operator()(const libzcash::UnifiedAddress &uaddr) const; -}; - #endif // ZC_ADDRESS_H_ From d376e2838285376de2b07e468bda2b4319030cf5 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 29 Dec 2021 11:55:46 -0700 Subject: [PATCH 07/13] Replace `DecodeDestination` in `GetMinerAddress` with `DecodePaymentAddress` --- src/miner.cpp | 45 +++++++++++++++++++++++++++++---------------- src/miner.h | 25 +++++-------------------- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index d2a21a987..7ab770c2f 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -702,27 +702,40 @@ class MinerAddressScript : public CReserveScript void KeepScript() {} }; +std::optional ExtractMinerAddress::operator()(const CKeyID &keyID) const { + boost::shared_ptr mAddr(new MinerAddressScript()); + mAddr->reserveScript = CScript() << OP_DUP << OP_HASH160 << ToByteVector(keyID) << OP_EQUALVERIFY << OP_CHECKSIG; + return mAddr; +} +std::optional ExtractMinerAddress::operator()(const CScriptID &addr) const { + return std::nullopt; +} +std::optional ExtractMinerAddress::operator()(const libzcash::SproutPaymentAddress &addr) const { + return std::nullopt; +} +std::optional ExtractMinerAddress::operator()(const libzcash::SaplingPaymentAddress &addr) const { + return addr; +} +std::optional ExtractMinerAddress::operator()(const libzcash::UnifiedAddress &addr) const { + for (const auto& receiver: addr) { + if (std::holds_alternative(receiver)) { + return std::get(receiver); + } + } + return std::nullopt; +} + + void GetMinerAddress(MinerAddress &minerAddress) { KeyIO keyIO(Params()); - // Try a transparent address first auto mAddrArg = GetArg("-mineraddress", ""); - CTxDestination addr = keyIO.DecodeDestination(mAddrArg); - if (IsValidDestination(addr)) { - boost::shared_ptr mAddr(new MinerAddressScript()); - CKeyID keyID = std::get(addr); - - mAddr->reserveScript = CScript() << OP_DUP << OP_HASH160 << ToByteVector(keyID) << OP_EQUALVERIFY << OP_CHECKSIG; - minerAddress = mAddr; - } else { - // Try a payment address - auto zaddr0 = keyIO.DecodePaymentAddress(mAddrArg); - if (zaddr0.has_value()) { - auto zaddr = std::visit(ExtractMinerAddress(), zaddr0.value()); - if (zaddr.has_value()) { - minerAddress = zaddr.value(); - } + auto zaddr0 = keyIO.DecodePaymentAddress(mAddrArg); + if (zaddr0.has_value()) { + auto zaddr = std::visit(ExtractMinerAddress(), zaddr0.value()); + if (zaddr.has_value()) { + minerAddress = zaddr.value(); } } } diff --git a/src/miner.h b/src/miner.h index 3b2081f58..b6851f3f3 100644 --- a/src/miner.h +++ b/src/miner.h @@ -32,26 +32,11 @@ class ExtractMinerAddress public: ExtractMinerAddress() {} - std::optional operator()(const CKeyID &addr) const { - return std::nullopt; - } - std::optional operator()(const CScriptID &addr) const { - return std::nullopt; - } - std::optional operator()(const libzcash::SproutPaymentAddress &addr) const { - return std::nullopt; - } - std::optional operator()(const libzcash::SaplingPaymentAddress &addr) const { - return addr; - } - std::optional operator()(const libzcash::UnifiedAddress &addr) const { - for (const auto& receiver: addr) { - if (std::holds_alternative(receiver)) { - return std::get(receiver); - } - } - return std::nullopt; - } + std::optional operator()(const CKeyID &keyID) const; + std::optional operator()(const CScriptID &addr) const; + std::optional operator()(const libzcash::SproutPaymentAddress &addr) const; + std::optional operator()(const libzcash::SaplingPaymentAddress &addr) const; + std::optional operator()(const libzcash::UnifiedAddress &addr) const; }; class KeepMinerAddress From 20266ac91112cf753a6c74b8b13d2d2c49540937 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Wed, 29 Dec 2021 13:44:55 -0700 Subject: [PATCH 08/13] Remove uses of KeyIO::DecodeDestination --- qa/rpc-tests/wallet_shieldingcoinbase.py | 8 +- src/chainparams.cpp | 8 +- src/consensus/params.cpp | 29 ++- src/init.cpp | 15 +- .../asyncrpcoperation_mergetoaddress.cpp | 39 +++- src/wallet/asyncrpcoperation_sendmany.cpp | 38 +++- src/wallet/asyncrpcoperation_sendmany.h | 10 +- src/wallet/rpcwallet.cpp | 190 ++++++++++-------- src/wallet/test/rpc_wallet_tests.cpp | 17 +- 9 files changed, 216 insertions(+), 138 deletions(-) diff --git a/qa/rpc-tests/wallet_shieldingcoinbase.py b/qa/rpc-tests/wallet_shieldingcoinbase.py index 01f7ee8e6..129af7011 100755 --- a/qa/rpc-tests/wallet_shieldingcoinbase.py +++ b/qa/rpc-tests/wallet_shieldingcoinbase.py @@ -82,9 +82,11 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework): # Node 3 will test that watch only address utxos are not selected self.nodes[3].importaddress(mytaddr) recipients= [{"address":myzaddr, "amount": Decimal('1')}] - myopid = self.nodes[3].z_sendmany(mytaddr, recipients) - - wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "Insufficient transparent funds, no UTXOs found for taddr from address.", 10) + try: + myopid = self.nodes[3].z_sendmany(mytaddr, recipients) + except JSONRPCException as e: + errorString = e.error['message'] + assert_equal("Invalid from address: does not belong to this node, spending key not found.", errorString); # This send will fail because our wallet does not allow any change when shielding a coinbase utxo, # as it's currently not possible to specify a change address in z_sendmany. diff --git a/src/chainparams.cpp b/src/chainparams.cpp index d9a7cb182..d117db8ee 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -822,10 +822,10 @@ CScript CChainParams::GetFoundersRewardScriptAtHeight(int nHeight) const { assert(nHeight > 0 && nHeight <= consensus.GetLastFoundersRewardBlockHeight(nHeight)); KeyIO keyIO(*this); - CTxDestination address = keyIO.DecodeDestination(GetFoundersRewardAddressAtHeight(nHeight).c_str()); - assert(IsValidDestination(address)); - assert(IsScriptDestination(address)); - CScriptID scriptID = std::get(address); // address is a variant + auto address = keyIO.DecodePaymentAddress(GetFoundersRewardAddressAtHeight(nHeight).c_str()); + assert(address.has_value()); + assert(std::holds_alternative(address.value())); + CScriptID scriptID = std::get(address.value()); CScript script = CScript() << OP_HASH160 << ToByteVector(scriptID) << OP_EQUAL; return script; } diff --git a/src/consensus/params.cpp b/src/consensus/params.cpp index 58528b7b4..f1641e7aa 100644 --- a/src/consensus/params.cpp +++ b/src/consensus/params.cpp @@ -9,6 +9,7 @@ #include