From bd3c860cb478ceeead2d716caecb9196b2ea0167 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Tue, 18 Sep 2018 23:22:50 +0100 Subject: [PATCH 1/2] Use ovk directly in the TransactionBuilder API instead of fvk --- src/gtest/test_transaction_builder.cpp | 12 ++++++------ src/transaction_builder.cpp | 10 +++++----- src/transaction_builder.h | 6 +++--- src/wallet/asyncrpcoperation_sendmany.cpp | 6 +++--- src/wallet/gtest/test_wallet.cpp | 24 +++++++++++------------ 5 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index b8fe54466..05a8cb601 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -38,7 +38,7 @@ TEST(TransactionBuilder, Invoke) // 0.0005 t-ZEC in, 0.0004 z-ZEC out, 0.0001 t-ZEC fee auto builder1 = TransactionBuilder(consensusParams, 1, &keystore); builder1.AddTransparentInput(COutPoint(), scriptPubKey, 50000); - builder1.AddSaplingOutput(fvk_from, pk, 40000, {}); + builder1.AddSaplingOutput(fvk_from.ovk, pk, 40000, {}); auto maybe_tx1 = builder1.Build(); ASSERT_EQ(static_cast(maybe_tx1), true); auto tx1 = maybe_tx1.get(); @@ -73,7 +73,7 @@ TEST(TransactionBuilder, Invoke) // Check that trying to add a different anchor fails ASSERT_FALSE(builder2.AddSaplingSpend(expsk, note, uint256(), witness)); - builder2.AddSaplingOutput(fvk, pk, 25000, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); @@ -153,7 +153,7 @@ TEST(TransactionBuilder, FailsWithNegativeChange) // Fail if there is only a Sapling output // 0.0005 z-ZEC out, 0.0001 t-ZEC fee auto builder = TransactionBuilder(consensusParams, 1); - builder.AddSaplingOutput(fvk, pk, 50000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); EXPECT_FALSE(static_cast(builder.Build())); // Fail if there is only a transparent output @@ -237,7 +237,7 @@ TEST(TransactionBuilder, ChangeOutput) { auto builder = TransactionBuilder(consensusParams, 1, &keystore); builder.AddTransparentInput(COutPoint(), scriptPubKey, 25000); - builder.SendChangeTo(zChangeAddr, fvkOut); + builder.SendChangeTo(zChangeAddr, fvkOut.ovk); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -298,7 +298,7 @@ TEST(TransactionBuilder, SetFee) { auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -315,7 +315,7 @@ TEST(TransactionBuilder, SetFee) { auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); builder.SetFee(20000); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index ed8d8c25b..e137cbdb8 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -47,13 +47,13 @@ bool TransactionBuilder::AddSaplingSpend( } void TransactionBuilder::AddSaplingOutput( - libzcash::SaplingFullViewingKey from, + uint256 ovk, libzcash::SaplingPaymentAddress to, CAmount value, std::array memo) { auto note = libzcash::SaplingNote(to, value); - outputs.emplace_back(from.ovk, note, memo); + outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } @@ -84,9 +84,9 @@ void TransactionBuilder::SetFee(CAmount fee) this->fee = fee; } -void TransactionBuilder::SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, libzcash::SaplingFullViewingKey fvkOut) +void TransactionBuilder::SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, uint256 ovk) { - zChangeAddr = std::make_pair(fvkOut, changeAddr); + zChangeAddr = std::make_pair(ovk, changeAddr); tChangeAddr = boost::none; } @@ -136,7 +136,7 @@ boost::optional TransactionBuilder::Build() auto fvk = spends[0].expsk.full_viewing_key(); auto note = spends[0].note; libzcash::SaplingPaymentAddress changeAddr(note.d, note.pk_d); - AddSaplingOutput(fvk, changeAddr, change, {}); + AddSaplingOutput(fvk.ovk, changeAddr, change, {}); } else { return boost::none; } diff --git a/src/transaction_builder.h b/src/transaction_builder.h index 90f2eff2d..560898381 100644 --- a/src/transaction_builder.h +++ b/src/transaction_builder.h @@ -65,7 +65,7 @@ private: std::vector outputs; std::vector tIns; - boost::optional> zChangeAddr; + boost::optional> zChangeAddr; boost::optional tChangeAddr; public: @@ -83,7 +83,7 @@ public: SaplingWitness witness); void AddSaplingOutput( - libzcash::SaplingFullViewingKey from, + uint256 ovk, libzcash::SaplingPaymentAddress to, CAmount value, std::array memo); @@ -93,7 +93,7 @@ public: bool AddTransparentOutput(CTxDestination& to, CAmount value); - void SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, libzcash::SaplingFullViewingKey fvkOut); + void SendChangeTo(libzcash::SaplingPaymentAddress changeAddr, uint256 ovk); bool SendChangeTo(CTxDestination& changeAddr); diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index ba65b2d75..0984b5e21 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -380,11 +380,11 @@ bool AsyncRPCOperation_sendmany::main_impl() { // Get various necessary keys SaplingExpandedSpendingKey expsk; - SaplingFullViewingKey from; + uint256 ovk; if (isfromzaddr_) { auto sk = boost::get(spendingkey_); expsk = sk.expsk; - from = expsk.full_viewing_key(); + ovk = expsk.full_viewing_key().ovk; } else { // TODO: Set "from" to something! } @@ -450,7 +450,7 @@ bool AsyncRPCOperation_sendmany::main_impl() { auto memo = get_memo_from_hex_string(hexMemo); - builder_.AddSaplingOutput(from, to, value, memo); + builder_.AddSaplingOutput(ovk, to, value, memo); } // Add transparent outputs diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index abc582faa..a67c3b491 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -384,7 +384,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 50000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); builder.SetFee(0); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); @@ -504,7 +504,7 @@ TEST(WalletTests, FindMySaplingNotes) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -644,7 +644,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Generate tx to create output note B auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 35000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 35000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -700,7 +700,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Create transaction to spend note B auto builder2 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness)); - builder2.AddSaplingOutput(fvk, pk, 20000, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 20000, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); @@ -708,7 +708,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Create conflicting transaction which also spends note B auto builder3 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness)); - builder3.AddSaplingOutput(fvk, pk, 19999, {}); + builder3.AddSaplingOutput(fvk.ovk, pk, 19999, {}); auto maybe_tx3 = builder3.Build(); ASSERT_EQ(static_cast(maybe_tx3), true); auto tx3 = maybe_tx3.get(); @@ -809,7 +809,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -906,7 +906,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -1042,7 +1042,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Generate transaction, which sends funds to note B auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -1114,7 +1114,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Create transaction to spend note B auto builder2 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness)); - builder2.AddSaplingOutput(fvk, pk, 12500, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 12500, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); @@ -1744,7 +1744,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { // Generate transaction auto builder = TransactionBuilder(consensusParams, 1); ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); - builder.AddSaplingOutput(fvk, pk2, 25000, {}); + builder.AddSaplingOutput(fvk.ovk, pk2, 25000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); @@ -1894,7 +1894,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { // 0.0005 t-ZEC in, 0.0004 z-ZEC out, 0.0001 t-ZEC fee auto builder = TransactionBuilder(consensusParams, 1, &keystore); builder.AddTransparentInput(COutPoint(), scriptPubKey, 50000); - builder.AddSaplingOutput(fvk, pk, 40000, {}); + builder.AddSaplingOutput(fvk.ovk, pk, 40000, {}); auto maybe_tx = builder.Build(); ASSERT_EQ(static_cast(maybe_tx), true); auto tx1 = maybe_tx.get(); @@ -1951,7 +1951,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { // 0.0004 z-ZEC in, 0.00025 z-ZEC out, 0.0001 t-ZEC fee, 0.00005 z-ZEC change auto builder2 = TransactionBuilder(consensusParams, 2); ASSERT_TRUE(builder2.AddSaplingSpend(expsk, note, anchor, witness)); - builder2.AddSaplingOutput(fvk, pk, 25000, {}); + builder2.AddSaplingOutput(fvk.ovk, pk, 25000, {}); auto maybe_tx2 = builder2.Build(); ASSERT_EQ(static_cast(maybe_tx2), true); auto tx2 = maybe_tx2.get(); From bb4b6982e3847d66597b956c52253ba3a6f6fc90 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 19 Sep 2018 00:51:30 +0100 Subject: [PATCH 2/2] Generate an ovk to encrypt outCiphertext for t-addr senders Closes #3506. --- src/test/rpc_wallet_tests.cpp | 102 ++++++++++++++++++++++ src/wallet/asyncrpcoperation_sendmany.cpp | 12 ++- src/zcash/zip32.cpp | 27 ++++++ src/zcash/zip32.h | 3 + 4 files changed, 143 insertions(+), 1 deletion(-) diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 3bc729f6f..6b132d0a1 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -1254,6 +1254,108 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) } +BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling) +{ + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + + LOCK(pwalletMain->cs_wallet); + + if (!pwalletMain->HaveHDSeed()) { + pwalletMain->GenerateNewSeed(); + } + + UniValue retValue; + + // add keys manually + auto taddr = pwalletMain->GenerateNewKey().GetID(); + std::string taddr1 = EncodeDestination(taddr); + auto pa = pwalletMain->GenerateNewSaplingZKey(); + std::string zaddr1 = EncodePaymentAddress(pa); + + auto consensusParams = Params().GetConsensus(); + retValue = CallRPC("getblockcount"); + int nextBlockHeight = retValue.get_int() + 1; + + // Add a fake transaction to the wallet + CMutableTransaction mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight); + CScript scriptPubKey = CScript() << OP_DUP << OP_HASH160 << ToByteVector(taddr) << OP_EQUALVERIFY << OP_CHECKSIG; + mtx.vout.push_back(CTxOut(5 * COIN, scriptPubKey)); + CWalletTx wtx(pwalletMain, mtx); + pwalletMain->AddToWallet(wtx, true, NULL); + + // Fake-mine the transaction + BOOST_CHECK_EQUAL(0, chainActive.Height()); + CBlock block; + block.hashPrevBlock = chainActive.Tip()->GetBlockHash(); + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + fakeIndex.nHeight = 1; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + BOOST_CHECK(chainActive.Contains(&fakeIndex)); + BOOST_CHECK_EQUAL(1, chainActive.Height()); + wtx.SetMerkleBranch(block); + pwalletMain->AddToWallet(wtx, true, NULL); + + // Context that z_sendmany requires + auto builder = TransactionBuilder(consensusParams, nextBlockHeight, pwalletMain); + mtx = CreateNewContextualCMutableTransaction(consensusParams, nextBlockHeight); + + std::vector recipients = { SendManyRecipient(zaddr1, 1 * COIN, "ABCD") }; + std::shared_ptr operation( new AsyncRPCOperation_sendmany(builder, mtx, taddr1, {}, recipients, 0) ); + std::shared_ptr ptr = std::dynamic_pointer_cast (operation); + + // Enable test mode so tx is not sent + static_cast(operation.get())->testmode = true; + + // Generate the Sapling shielding transaction + operation->main(); + BOOST_CHECK(operation->isSuccess()); + + // Get the transaction + auto result = operation->getResult(); + BOOST_ASSERT(result.isObject()); + auto hexTx = result["hex"].getValStr(); + CDataStream ss(ParseHex(hexTx), SER_NETWORK, PROTOCOL_VERSION); + CTransaction tx; + ss >> tx; + BOOST_ASSERT(!tx.vShieldedOutput.empty()); + + // We shouldn't be able to decrypt with the empty ovk + BOOST_CHECK(!AttemptSaplingOutDecryption( + tx.vShieldedOutput[0].outCiphertext, + uint256(), + tx.vShieldedOutput[0].cv, + tx.vShieldedOutput[0].cm, + tx.vShieldedOutput[0].ephemeralKey)); + + // We should be able to decrypt the outCiphertext with the ovk + // generated for transparent addresses + HDSeed seed; + BOOST_ASSERT(pwalletMain->GetHDSeed(seed)); + BOOST_CHECK(AttemptSaplingOutDecryption( + tx.vShieldedOutput[0].outCiphertext, + ovkForShieldingFromTaddr(seed), + tx.vShieldedOutput[0].cv, + tx.vShieldedOutput[0].cm, + tx.vShieldedOutput[0].ephemeralKey)); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); + mapArgs.erase("-developersapling"); + mapArgs.erase("-experimentalfeatures"); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); +} + + /* * This test covers storing encrypted zkeys in the wallet. */ diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index 0984b5e21..64b1a5f26 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -386,7 +386,17 @@ bool AsyncRPCOperation_sendmany::main_impl() { expsk = sk.expsk; ovk = expsk.full_viewing_key().ovk; } else { - // TODO: Set "from" to something! + // Sending from a t-address, which we don't have an ovk for. Instead, + // generate a common one from the HD seed. This ensures the data is + // recoverable, while keeping it logically separate from the ZIP 32 + // Sapling key hierarchy, which the user might not be using. + HDSeed seed; + if (!pwalletMain->GetHDSeed(seed)) { + throw JSONRPCError( + RPC_WALLET_ERROR, + "CWallet::GenerateNewSaplingZKey(): HD seed not found"); + } + ovk = ovkForShieldingFromTaddr(seed); } // Set change address if we are using transparent funds diff --git a/src/zcash/zip32.cpp b/src/zcash/zip32.cpp index cdad98aa9..15478843e 100644 --- a/src/zcash/zip32.cpp +++ b/src/zcash/zip32.cpp @@ -8,6 +8,7 @@ #include "random.h" #include "streams.h" #include "version.h" +#include "zcash/prf.h" #include #include @@ -15,6 +16,9 @@ const unsigned char ZCASH_HD_SEED_FP_PERSONAL[crypto_generichash_blake2b_PERSONALBYTES] = {'Z', 'c', 'a', 's', 'h', '_', 'H', 'D', '_', 'S', 'e', 'e', 'd', '_', 'F', 'P'}; +const unsigned char ZCASH_TADDR_OVK_PERSONAL[crypto_generichash_blake2b_PERSONALBYTES] = + {'Z', 'c', 'T', 'a', 'd', 'd', 'r', 'T', 'o', 'S', 'a', 'p', 'l', 'i', 'n', 'g'}; + HDSeed HDSeed::Random(size_t len) { assert(len >= 32); @@ -30,6 +34,29 @@ uint256 HDSeed::Fingerprint() const return h.GetHash(); } +uint256 ovkForShieldingFromTaddr(HDSeed& seed) { + auto rawSeed = seed.RawSeed(); + + // I = BLAKE2b-512("ZcTaddrToSapling", seed) + crypto_generichash_blake2b_state state; + assert(crypto_generichash_blake2b_init_salt_personal( + &state, + NULL, 0, // No key. + 64, + NULL, // No salt. + ZCASH_TADDR_OVK_PERSONAL) == 0); + crypto_generichash_blake2b_update(&state, rawSeed.data(), rawSeed.size()); + auto intermediate = std::array(); + crypto_generichash_blake2b_final(&state, intermediate.data(), 64); + + // I_L = I[0..32] + uint256 intermediate_L; + memcpy(intermediate_L.begin(), intermediate.data(), 32); + + // ovk = truncate_32(PRF^expand(I_L, [0x02])) + return PRF_ovk(intermediate_L); +} + namespace libzcash { boost::optional SaplingExtendedFullViewingKey::Derive(uint32_t i) const diff --git a/src/zcash/zip32.h b/src/zcash/zip32.h index fcd16b4e1..0fc5785bd 100644 --- a/src/zcash/zip32.h +++ b/src/zcash/zip32.h @@ -42,6 +42,9 @@ public: } }; +// This is not part of ZIP 32, but is here because it's linked to the HD seed. +uint256 ovkForShieldingFromTaddr(HDSeed& seed); + namespace libzcash { typedef blob88 diversifier_index_t;