From 4715b31c769ffabdb1083b0f41691b0222fb11ae Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 9 Jul 2018 20:12:37 +0100 Subject: [PATCH 1/8] Implement CKeyStore::GetSaplingPaymentAddresses() --- src/keystore.h | 14 ++++++++++++++ src/wallet/gtest/test_wallet_zkeys.cpp | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/keystore.h b/src/keystore.h index 76772dbf1..c856005c1 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -75,6 +75,7 @@ public: virtual bool GetSaplingIncomingViewingKey( const libzcash::SaplingPaymentAddress &addr, libzcash::SaplingIncomingViewingKey& ivkOut) const =0; + virtual void GetSaplingPaymentAddresses(std::set &setAddress) const =0; //! Support for viewing keys virtual bool AddViewingKey(const libzcash::SproutViewingKey &vk) =0; @@ -251,6 +252,19 @@ public: virtual bool GetSaplingIncomingViewingKey( const libzcash::SaplingPaymentAddress &addr, libzcash::SaplingIncomingViewingKey& ivkOut) const; + void GetSaplingPaymentAddresses(std::set &setAddress) const + { + setAddress.clear(); + { + LOCK(cs_SpendingKeyStore); + auto mi = mapSaplingIncomingViewingKeys.begin(); + while (mi != mapSaplingIncomingViewingKeys.end()) + { + setAddress.insert((*mi).first); + mi++; + } + } + } virtual bool AddViewingKey(const libzcash::SproutViewingKey &vk); virtual bool RemoveViewingKey(const libzcash::SproutViewingKey &vk); diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 317e4ff54..53963c482 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -16,7 +16,15 @@ TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) { CWallet wallet; + // wallet should be empty + std::set addrs; + wallet.GetSaplingPaymentAddresses(addrs); + ASSERT_EQ(0, addrs.size()); + + // wallet should have one key auto address = wallet.GenerateNewSaplingZKey(); + wallet.GetSaplingPaymentAddresses(addrs); + ASSERT_EQ(1, addrs.size()); // verify wallet has incoming viewing key for the address ASSERT_TRUE(wallet.HaveSaplingIncomingViewingKey(address)); @@ -28,6 +36,16 @@ TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) { // verify wallet did add it auto fvk = sk.full_viewing_key(); ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + + // verify spending key stored correctly + libzcash::SaplingSpendingKey keyOut; + wallet.GetSaplingSpendingKey(fvk, keyOut); + ASSERT_EQ(sk, keyOut); + + // verify there are two keys + wallet.GetSaplingPaymentAddresses(addrs); + ASSERT_EQ(2, addrs.size()); + ASSERT_EQ(1, addrs.count(address)); } /** From 281b51e567427fa77ec2162dc27f6f736ad47ffb Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 9 Jul 2018 23:41:41 +0100 Subject: [PATCH 2/8] Raise the 90-character limit on Bech32 encodings Regtest addresses are 91 characters, and ZIP 32's Bech32 encoding will be significantly longer. --- src/bech32.cpp | 2 +- src/test/bech32_tests.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bech32.cpp b/src/bech32.cpp index 2889f8f99..78c35b976 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -169,7 +169,7 @@ std::pair Decode(const std::string& str) { } if (lower && upper) return {}; size_t pos = str.rfind('1'); - if (str.size() > 90 || pos == str.npos || pos == 0 || pos + 7 > str.size()) { + if (str.size() > 1023 || pos == str.npos || pos == 0 || pos + 7 > str.size()) { return {}; } data values(str.size() - 1 - pos); diff --git a/src/test/bech32_tests.cpp b/src/test/bech32_tests.cpp index f71ca1bf2..02252bcbf 100644 --- a/src/test/bech32_tests.cpp +++ b/src/test/bech32_tests.cpp @@ -28,6 +28,7 @@ BOOST_AUTO_TEST_CASE(bip173_testvectors_valid) "A12UEL5L", "a12uel5l", "an83characterlonghumanreadablepartthatcontainsthenumber1andtheexcludedcharactersbio1tt5tgs", + "an84characterslonghumanreadablepartthatcontainsthenumber1andtheexcludedcharactersbio1569pvx", "abcdef1qpzry9x8gf2tvdw0s3jn54khce6mua7lmqqqxw", "11qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqc8247j", "split1checkupstagehandshakeupstreamerranterredcaperred2y9e3w", @@ -48,7 +49,6 @@ BOOST_AUTO_TEST_CASE(bip173_testvectors_invalid) " 1nwldj5", "\x7f""1axkwrx", "\x80""1eym55h", - "an84characterslonghumanreadablepartthatcontainsthenumber1andtheexcludedcharactersbio1569pvx", "pzry9x0s0muk", "1pzry9x0s0muk", "x1b4n0q5v", From eec85c4388691da0fa5950217c158bb87bf59cd4 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Mon, 9 Jul 2018 23:46:36 +0100 Subject: [PATCH 3/8] Add Sapling support to z_getnewaddress and z_listaddresses --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/wallet_addresses.py | 85 ++++++++++++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 61 ++++++++++++++++++----- 3 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 qa/rpc-tests/wallet_addresses.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 6a2348012..73d9ddb76 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -23,6 +23,7 @@ testScripts=( 'wallet_overwintertx.py' 'wallet_nullifiers.py' 'wallet_1941.py' + 'wallet_addresses.py' 'listtransactions.py' 'mempool_resurrect_test.py' 'txn_doublespend.py' diff --git a/qa/rpc-tests/wallet_addresses.py b/qa/rpc-tests/wallet_addresses.py new file mode 100644 index 000000000..874088719 --- /dev/null +++ b/qa/rpc-tests/wallet_addresses.py @@ -0,0 +1,85 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, start_nodes + +# Test wallet address behaviour across network upgradesa\ +class WalletAddressesTest(BitcoinTestFramework): + + def setup_nodes(self): + return start_nodes(4, self.options.tmpdir, [[ + '-nuparams=5ba81b19:202', # Overwinter + '-nuparams=76b809bb:204', # Sapling + ]] * 4) + + def run_test(self): + def addr_checks(default_type): + # Check default type + addr = self.nodes[0].z_getnewaddress() + res = self.nodes[0].z_validateaddress(addr) + assert(res['isvalid']) + assert(res['ismine']) + assert_equal(res['type'], default_type) + assert(addr in self.nodes[0].z_listaddresses()) + + # Check explicit Sprout type + addr = self.nodes[0].z_getnewaddress('sprout') + res = self.nodes[0].z_validateaddress(addr) + assert(res['isvalid']) + assert(res['ismine']) + assert_equal(res['type'], 'sprout') + assert(addr in self.nodes[0].z_listaddresses()) + + # Check explicit Sapling type + addr = self.nodes[0].z_getnewaddress('sapling') + res = self.nodes[0].z_validateaddress(addr) + assert(res['isvalid']) + assert(res['ismine']) + assert_equal(res['type'], 'sapling') + assert(addr in self.nodes[0].z_listaddresses()) + + # Sanity-check the test harness + assert_equal(self.nodes[0].getblockcount(), 200) + + # Current height = 200 -> Sprout + # Default address type is Sprout + print "Testing height 200 (Sprout)" + addr_checks('sprout') + + self.nodes[0].generate(1) + self.sync_all() + + # Current height = 201 -> Sprout + # Default address type is Sprout + print "Testing height 201 (Sprout)" + addr_checks('sprout') + + self.nodes[0].generate(1) + self.sync_all() + + # Current height = 202 -> Overwinter + # Default address type is Sprout + print "Testing height 202 (Overwinter)" + addr_checks('sprout') + + self.nodes[0].generate(1) + self.sync_all() + + # Current height = 203 -> Overwinter + # Default address type is Sprout + print "Testing height 203 (Overwinter)" + addr_checks('sprout') + + self.nodes[0].generate(1) + self.sync_all() + + # Current height = 204 -> Sapling + # Default address type is Sprout + print "Testing height 204 (Sapling)" + addr_checks('sprout') + +if __name__ == '__main__': + WalletAddressesTest().main() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 696e2b288..04f3e9ebf 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -42,6 +42,9 @@ using namespace std; using namespace libzcash; +const std::string ADDR_TYPE_SPROUT = "sprout"; +const std::string ADDR_TYPE_SAPLING = "sapling"; + extern UniValue TxJoinSplitToJSON(const CTransaction& tx); int64_t nWalletUnlockTime; @@ -3100,15 +3103,21 @@ UniValue z_getnewaddress(const UniValue& params, bool fHelp) if (!EnsureWalletIsAvailable(fHelp)) return NullUniValue; - if (fHelp || params.size() > 0) + std::string defaultType = ADDR_TYPE_SPROUT; + + if (fHelp || params.size() > 1) throw runtime_error( - "z_getnewaddress\n" - "\nReturns a new zaddr for receiving payments.\n" + "z_getnewaddress ( type )\n" + "\nReturns a new shielded address for receiving payments.\n" + "\nWith no arguments, returns a Sprout address.\n" "\nArguments:\n" + "1. \"type\" (string, optional, default=\"" + defaultType + "\") The type of address. One of [\"" + + ADDR_TYPE_SPROUT + "\", \"" + ADDR_TYPE_SAPLING + "\"].\n" "\nResult:\n" - "\"zcashaddress\" (string) The new zaddr\n" + "\"zcashaddress\" (string) The new shielded address.\n" "\nExamples:\n" + HelpExampleCli("z_getnewaddress", "") + + HelpExampleCli("z_getnewaddress", ADDR_TYPE_SAPLING) + HelpExampleRpc("z_getnewaddress", "") ); @@ -3116,8 +3125,18 @@ UniValue z_getnewaddress(const UniValue& params, bool fHelp) EnsureWalletIsUnlocked(); - auto zaddr = pwalletMain->GenerateNewZKey(); - return EncodePaymentAddress(zaddr); + auto addrType = defaultType; + if (params.size() > 0) { + addrType = params[0].get_str(); + } + + if (addrType == ADDR_TYPE_SPROUT) { + return EncodePaymentAddress(pwalletMain->GenerateNewZKey()); + } else if (addrType == ADDR_TYPE_SAPLING) { + return EncodePaymentAddress(pwalletMain->GenerateNewSaplingZKey()); + } else { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid address type"); + } } @@ -3129,7 +3148,7 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp) if (fHelp || params.size() > 1) throw runtime_error( "z_listaddresses ( includeWatchonly )\n" - "\nReturns the list of zaddr belonging to the wallet.\n" + "\nReturns the list of Sprout and Sapling shielded addresses belonging to the wallet.\n" "\nArguments:\n" "1. includeWatchonly (bool, optional, default=false) Also include watchonly addresses (see 'z_importviewingkey')\n" "\nResult:\n" @@ -3150,12 +3169,28 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp) } UniValue ret(UniValue::VARR); - // TODO: Add Sapling support - std::set addresses; - pwalletMain->GetPaymentAddresses(addresses); - for (auto addr : addresses ) { - if (fIncludeWatchonly || pwalletMain->HaveSpendingKey(addr)) { - ret.push_back(EncodePaymentAddress(addr)); + { + std::set addresses; + pwalletMain->GetPaymentAddresses(addresses); + for (auto addr : addresses ) { + if (fIncludeWatchonly || pwalletMain->HaveSpendingKey(addr)) { + ret.push_back(EncodePaymentAddress(addr)); + } + } + } + { + std::set addresses; + pwalletMain->GetSaplingPaymentAddresses(addresses); + libzcash::SaplingIncomingViewingKey ivk; + libzcash::SaplingFullViewingKey fvk; + for (auto addr : addresses ) { + if (fIncludeWatchonly || ( + pwalletMain->GetSaplingIncomingViewingKey(addr, ivk) && + pwalletMain->GetSaplingFullViewingKey(ivk, fvk) && + pwalletMain->HaveSaplingSpendingKey(fvk) + )) { + ret.push_back(EncodePaymentAddress(addr)); + } } } return ret; From fbd029d99d3d32e43d88c02673f90053a7e7057d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 31 Jul 2018 23:28:42 +0100 Subject: [PATCH 4/8] Formatting --- src/wallet/rpcwallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 04f3e9ebf..d196da4e5 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3172,7 +3172,7 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp) { std::set addresses; pwalletMain->GetPaymentAddresses(addresses); - for (auto addr : addresses ) { + for (auto addr : addresses) { if (fIncludeWatchonly || pwalletMain->HaveSpendingKey(addr)) { ret.push_back(EncodePaymentAddress(addr)); } @@ -3183,7 +3183,7 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp) pwalletMain->GetSaplingPaymentAddresses(addresses); libzcash::SaplingIncomingViewingKey ivk; libzcash::SaplingFullViewingKey fvk; - for (auto addr : addresses ) { + for (auto addr : addresses) { if (fIncludeWatchonly || ( pwalletMain->GetSaplingIncomingViewingKey(addr, ivk) && pwalletMain->GetSaplingFullViewingKey(ivk, fvk) && From d75e69da5a9e58205d53f208751d622b9e5a257b Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 1 Aug 2018 09:44:31 +0100 Subject: [PATCH 5/8] test: Deduplicate logic in wallet_addresses RPC test --- qa/rpc-tests/wallet_addresses.py | 34 ++++++++++++-------------------- 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/qa/rpc-tests/wallet_addresses.py b/qa/rpc-tests/wallet_addresses.py index 874088719..0b9669972 100644 --- a/qa/rpc-tests/wallet_addresses.py +++ b/qa/rpc-tests/wallet_addresses.py @@ -17,29 +17,21 @@ class WalletAddressesTest(BitcoinTestFramework): def run_test(self): def addr_checks(default_type): - # Check default type - addr = self.nodes[0].z_getnewaddress() - res = self.nodes[0].z_validateaddress(addr) - assert(res['isvalid']) - assert(res['ismine']) - assert_equal(res['type'], default_type) - assert(addr in self.nodes[0].z_listaddresses()) + # Check default type, as well as explicit types + types_and_addresses = [ + (default_type, self.nodes[0].z_getnewaddress()), + ('sprout', self.nodes[0].z_getnewaddress('sprout')), + ('sapling', self.nodes[0].z_getnewaddress('sapling')), + ] - # Check explicit Sprout type - addr = self.nodes[0].z_getnewaddress('sprout') - res = self.nodes[0].z_validateaddress(addr) - assert(res['isvalid']) - assert(res['ismine']) - assert_equal(res['type'], 'sprout') - assert(addr in self.nodes[0].z_listaddresses()) + all_addresses = self.nodes[0].z_listaddresses() - # Check explicit Sapling type - addr = self.nodes[0].z_getnewaddress('sapling') - res = self.nodes[0].z_validateaddress(addr) - assert(res['isvalid']) - assert(res['ismine']) - assert_equal(res['type'], 'sapling') - assert(addr in self.nodes[0].z_listaddresses()) + for addr_type, addr in types_and_addresses: + res = self.nodes[0].z_validateaddress(addr) + assert(res['isvalid']) + assert(res['ismine']) + assert_equal(res['type'], addr_type) + assert(addr in all_addresses) # Sanity-check the test harness assert_equal(self.nodes[0].getblockcount(), 200) From 4fab49e173ce408c4345eebc3c57e2ecd6374d8e Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 1 Aug 2018 09:46:32 +0100 Subject: [PATCH 6/8] test: Another assert in wallet_zkeys_tests.store_and_load_sapling_zkeys --- src/wallet/gtest/test_wallet_zkeys.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 53963c482..80c8154c8 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -46,6 +46,7 @@ TEST(wallet_zkeys_tests, store_and_load_sapling_zkeys) { wallet.GetSaplingPaymentAddresses(addrs); ASSERT_EQ(2, addrs.size()); ASSERT_EQ(1, addrs.count(address)); + ASSERT_EQ(1, addrs.count(sk.default_address())); } /** From 4aabeebc1f2d3596db931e1c825996b31e681adb Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 1 Aug 2018 19:56:01 +0100 Subject: [PATCH 7/8] test: Fix permissions of wallet_addresses --- qa/rpc-tests/wallet_addresses.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 qa/rpc-tests/wallet_addresses.py diff --git a/qa/rpc-tests/wallet_addresses.py b/qa/rpc-tests/wallet_addresses.py old mode 100644 new mode 100755 From 40dc060cb02d7986f4b724d970370c99391bb79c Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 1 Aug 2018 19:59:57 +0100 Subject: [PATCH 8/8] test: Update rpc_wallet_z_importexport to account for Sapling changes --- src/test/rpc_wallet_tests.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 5064526ba..4b478df40 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -561,6 +561,9 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importexport) std::set addrs; pwalletMain->GetPaymentAddresses(addrs); BOOST_CHECK(addrs.size()==0); + std::set saplingAddrs; + pwalletMain->GetSaplingPaymentAddresses(saplingAddrs); + BOOST_CHECK(saplingAddrs.empty()); // verify import and export key for (int i = 0; i < n1; i++) { @@ -586,7 +589,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importexport) // Verify we can list the keys imported BOOST_CHECK_NO_THROW(retValue = CallRPC("z_listaddresses")); UniValue arr = retValue.get_array(); - BOOST_CHECK(arr.size() == n1); + BOOST_CHECK(arr.size() == (2 * n1)); // Put addresses into a set std::unordered_set myaddrs; @@ -601,9 +604,10 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_importexport) // Verify number of addresses stored in wallet is n1+n2 int numAddrs = myaddrs.size(); - BOOST_CHECK(numAddrs == n1+n2); + BOOST_CHECK(numAddrs == (2 * n1) + n2); pwalletMain->GetPaymentAddresses(addrs); - BOOST_CHECK(addrs.size()==numAddrs); + pwalletMain->GetSaplingPaymentAddresses(saplingAddrs); + BOOST_CHECK(addrs.size() + saplingAddrs.size() == numAddrs); // Ask wallet to list addresses BOOST_CHECK_NO_THROW(retValue = CallRPC("z_listaddresses"));