From 57faf44e613e817ea9dcfda1f01cf9ab8efac3b7 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 25 Jul 2018 17:12:03 -0700 Subject: [PATCH 01/40] Rename FindMyNotes to FindMySproutNotes. --- src/wallet/gtest/test_wallet.cpp | 14 +++++++------- src/wallet/wallet.cpp | 10 +++++----- src/wallet/wallet.h | 4 ++-- src/zcbenchmarks.cpp | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 4624a8579..bed795b16 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -381,7 +381,7 @@ TEST(wallet_tests, GetNoteNullifier) { EXPECT_EQ(nullifier, ret); } -TEST(wallet_tests, FindMyNotes) { +TEST(wallet_tests, FindMySproutNotes) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -392,12 +392,12 @@ TEST(wallet_tests, FindMyNotes) { auto note = GetNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); - auto noteMap = wallet.FindMyNotes(wtx); + auto noteMap = wallet.FindMySproutNotes(wtx); EXPECT_EQ(0, noteMap.size()); wallet.AddSproutSpendingKey(sk); - noteMap = wallet.FindMyNotes(wtx); + noteMap = wallet.FindMySproutNotes(wtx); EXPECT_EQ(2, noteMap.size()); JSOutPoint jsoutpt {wtx.GetHash(), 0, 1}; @@ -406,7 +406,7 @@ TEST(wallet_tests, FindMyNotes) { EXPECT_EQ(nd, noteMap[jsoutpt]); } -TEST(wallet_tests, FindMyNotesInEncryptedWallet) { +TEST(wallet_tests, FindMySproutNotesInEncryptedWallet) { TestWallet wallet; uint256 r {GetRandHash()}; CKeyingMaterial vMasterKey (r.begin(), r.end()); @@ -420,7 +420,7 @@ TEST(wallet_tests, FindMyNotesInEncryptedWallet) { auto note = GetNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); - auto noteMap = wallet.FindMyNotes(wtx); + auto noteMap = wallet.FindMySproutNotes(wtx); EXPECT_EQ(2, noteMap.size()); JSOutPoint jsoutpt {wtx.GetHash(), 0, 1}; @@ -430,7 +430,7 @@ TEST(wallet_tests, FindMyNotesInEncryptedWallet) { ASSERT_TRUE(wallet.Unlock(vMasterKey)); - noteMap = wallet.FindMyNotes(wtx); + noteMap = wallet.FindMySproutNotes(wtx); EXPECT_EQ(2, noteMap.size()); EXPECT_EQ(1, noteMap.count(jsoutpt)); EXPECT_EQ(nd, noteMap[jsoutpt]); @@ -1038,7 +1038,7 @@ TEST(wallet_tests, UpdateNullifierNoteMap) { auto note = GetNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); - // Pretend that we called FindMyNotes while the wallet was locked + // Pretend that we called FindMySproutNotes while the wallet was locked mapSproutNoteData_t noteData; JSOutPoint jsoutpt {wtx.GetHash(), 0, 1}; SproutNoteData nd {sk.address()}; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 675185af2..392f1a4bd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1333,7 +1333,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; - auto noteData = FindMyNotes(tx); + auto noteData = FindMySproutNotes(tx); if (fExisted || IsMine(tx) || IsFromMe(tx) || noteData.size() > 0) { CWalletTx wtx(this,tx); @@ -1432,10 +1432,10 @@ boost::optional CWallet::GetNoteNullifier(const JSDescription& jsdesc, * PaymentAddresses in this wallet. * * It should never be necessary to call this method with a CWalletTx, because - * the result of FindMyNotes (for the addresses available at the time) will + * the result of FindMySproutNotes (for the addresses available at the time) will * already have been cached in CWalletTx.mapSproutNoteData. */ -mapSproutNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const +mapSproutNoteData_t CWallet::FindMySproutNotes(const CTransaction &tx) const { LOCK(cs_SpendingKeyStore); uint256 hash = tx.GetHash(); @@ -1465,7 +1465,7 @@ mapSproutNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const // Couldn't decrypt with this decryptor } catch (const std::exception &exc) { // Unexpected failure - LogPrintf("FindMyNotes(): Unexpected error while testing decrypt:\n"); + LogPrintf("FindMySproutNotes(): Unexpected error while testing decrypt:\n"); LogPrintf("%s\n", exc.what()); } } @@ -1680,7 +1680,7 @@ void CWalletTx::SetSproutNoteData(mapSproutNoteData_t ¬eData) // Store the address and nullifier for the Note mapSproutNoteData[nd.first] = nd.second; } else { - // If FindMyNotes() was used to obtain noteData, + // If FindMySproutNotes() was used to obtain noteData, // this should never happen throw std::logic_error("CWalletTx::SetSproutNoteData(): Invalid note"); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 9c2aa863f..33c878934 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -225,7 +225,7 @@ public: /** * Block height corresponding to the most current witness. * - * When we first create a SproutNoteData in CWallet::FindMyNotes, this is set to + * When we first create a SproutNoteData in CWallet::FindMySproutNotes, this is set to * -1 as a placeholder. The next time CWallet::ChainTip is called, we can * determine what height the witness cache for this note is valid for (even * if no witnesses were cached), and so can set the correct value in @@ -1082,7 +1082,7 @@ public: const ZCNoteDecryption& dec, const uint256& hSig, uint8_t n) const; - mapSproutNoteData_t FindMyNotes(const CTransaction& tx) const; + mapSproutNoteData_t FindMySproutNotes(const CTransaction& tx) const; bool IsFromMe(const uint256& nullifier) const; void GetSproutNoteWitnesses( std::vector notes, diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index 314de11ab..bad4ce605 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -291,7 +291,7 @@ double benchmark_try_decrypt_notes(size_t nAddrs) struct timeval tv_start; timer_start(tv_start); - auto nd = wallet.FindMyNotes(tx); + auto nd = wallet.FindMySproutNotes(tx); return timer_stop(tv_start); } From 618206c7d5e94b5a763d729951f03000a9ad4894 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 25 Jul 2018 20:03:59 -0700 Subject: [PATCH 02/40] Rename GetNoteNullifier to GetSproutNoteNullifier. --- src/wallet/gtest/test_wallet.cpp | 6 +++--- src/wallet/wallet.cpp | 14 +++++++------- src/wallet/wallet.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index bed795b16..51f17232e 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -350,7 +350,7 @@ TEST(wallet_tests, set_invalid_note_addrs_in_cwallettx) { EXPECT_THROW(wtx.SetSproutNoteData(noteData), std::logic_error); } -TEST(wallet_tests, GetNoteNullifier) { +TEST(wallet_tests, GetSproutNoteNullifier) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -364,7 +364,7 @@ TEST(wallet_tests, GetNoteNullifier) { auto hSig = wtx.vjoinsplit[0].h_sig( *params, wtx.joinSplitPubKey); - auto ret = wallet.GetNoteNullifier( + auto ret = wallet.GetSproutNoteNullifier( wtx.vjoinsplit[0], address, dec, @@ -373,7 +373,7 @@ TEST(wallet_tests, GetNoteNullifier) { wallet.AddSproutSpendingKey(sk); - ret = wallet.GetNoteNullifier( + ret = wallet.GetSproutNoteNullifier( wtx.vjoinsplit[0], address, dec, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 392f1a4bd..999c5ee39 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1148,7 +1148,7 @@ bool CWallet::UpdateNullifierNoteMap() auto i = item.first.js; auto hSig = wtxItem.second.vjoinsplit[i].h_sig( *pzcashParams, wtxItem.second.joinSplitPubKey); - item.second.nullifier = GetNoteNullifier( + item.second.nullifier = GetSproutNoteNullifier( wtxItem.second.vjoinsplit[i], item.second.address, dec, @@ -1403,11 +1403,11 @@ void CWallet::EraseFromWallet(const uint256 &hash) * Returns a nullifier if the SpendingKey is available * Throws std::runtime_error if the decryptor doesn't match this note */ -boost::optional CWallet::GetNoteNullifier(const JSDescription& jsdesc, - const libzcash::SproutPaymentAddress& address, - const ZCNoteDecryption& dec, - const uint256& hSig, - uint8_t n) const +boost::optional CWallet::GetSproutNoteNullifier(const JSDescription &jsdesc, + const libzcash::SproutPaymentAddress &address, + const ZCNoteDecryption &dec, + const uint256 &hSig, + uint8_t n) const { boost::optional ret; auto note_pt = libzcash::SproutNotePlaintext::decrypt( @@ -1448,7 +1448,7 @@ mapSproutNoteData_t CWallet::FindMySproutNotes(const CTransaction &tx) const try { auto address = item.first; JSOutPoint jsoutpt {hash, i, j}; - auto nullifier = GetNoteNullifier( + auto nullifier = GetSproutNoteNullifier( tx.vjoinsplit[i], address, item.second, diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 33c878934..1d56de863 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1076,7 +1076,7 @@ public: std::set GetAccountAddresses(const std::string& strAccount) const; - boost::optional GetNoteNullifier( + boost::optional GetSproutNoteNullifier( const JSDescription& jsdesc, const libzcash::SproutPaymentAddress& address, const ZCNoteDecryption& dec, From f41bf503e1dd3775a34d5d9dce24e3a7d5301245 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 25 Jul 2018 20:08:36 -0700 Subject: [PATCH 03/40] Rename mapNullifiersToNotes to mapSproutNullifiersToNotes. --- src/wallet/gtest/test_wallet.cpp | 20 ++++++++++---------- src/wallet/wallet.cpp | 14 +++++++------- src/wallet/wallet.h | 2 +- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 51f17232e..4b368afed 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -524,13 +524,13 @@ TEST(wallet_tests, navigate_from_nullifier_to_note) { wtx.SetSproutNoteData(noteData); - EXPECT_EQ(0, wallet.mapNullifiersToNotes.count(nullifier)); + EXPECT_EQ(0, wallet.mapSproutNullifiersToNotes.count(nullifier)); wallet.AddToWallet(wtx, true, NULL); - EXPECT_EQ(1, wallet.mapNullifiersToNotes.count(nullifier)); - EXPECT_EQ(wtx.GetHash(), wallet.mapNullifiersToNotes[nullifier].hash); - EXPECT_EQ(0, wallet.mapNullifiersToNotes[nullifier].js); - EXPECT_EQ(1, wallet.mapNullifiersToNotes[nullifier].n); + EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes.count(nullifier)); + EXPECT_EQ(wtx.GetHash(), wallet.mapSproutNullifiersToNotes[nullifier].hash); + EXPECT_EQ(0, wallet.mapSproutNullifiersToNotes[nullifier].js); + EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes[nullifier].n); } TEST(wallet_tests, spent_note_is_from_me) { @@ -1046,17 +1046,17 @@ TEST(wallet_tests, UpdateNullifierNoteMap) { wtx.SetSproutNoteData(noteData); wallet.AddToWallet(wtx, true, NULL); - EXPECT_EQ(0, wallet.mapNullifiersToNotes.count(nullifier)); + EXPECT_EQ(0, wallet.mapSproutNullifiersToNotes.count(nullifier)); EXPECT_FALSE(wallet.UpdateNullifierNoteMap()); ASSERT_TRUE(wallet.Unlock(vMasterKey)); EXPECT_TRUE(wallet.UpdateNullifierNoteMap()); - EXPECT_EQ(1, wallet.mapNullifiersToNotes.count(nullifier)); - EXPECT_EQ(wtx.GetHash(), wallet.mapNullifiersToNotes[nullifier].hash); - EXPECT_EQ(0, wallet.mapNullifiersToNotes[nullifier].js); - EXPECT_EQ(1, wallet.mapNullifiersToNotes[nullifier].n); + EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes.count(nullifier)); + EXPECT_EQ(wtx.GetHash(), wallet.mapSproutNullifiersToNotes[nullifier].hash); + EXPECT_EQ(0, wallet.mapSproutNullifiersToNotes[nullifier].js); + EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes[nullifier].n); } TEST(wallet_tests, UpdatedNoteData) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 999c5ee39..1e4220140 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1164,7 +1164,7 @@ bool CWallet::UpdateNullifierNoteMap() } /** - * Update mapNullifiersToNotes with the cached nullifiers in this tx. + * Update mapSproutNullifiersToNotes with the cached nullifiers in this tx. */ void CWallet::UpdateNullifierNoteMapWithTx(const CWalletTx& wtx) { @@ -1172,7 +1172,7 @@ void CWallet::UpdateNullifierNoteMapWithTx(const CWalletTx& wtx) LOCK(cs_wallet); for (const mapSproutNoteData_t::value_type& item : wtx.mapSproutNoteData) { if (item.second.nullifier) { - mapNullifiersToNotes[*item.second.nullifier] = item.first; + mapSproutNullifiersToNotes[*item.second.nullifier] = item.first; } } } @@ -1378,9 +1378,9 @@ void CWallet::MarkAffectedTransactionsDirty(const CTransaction& tx) } for (const JSDescription& jsdesc : tx.vjoinsplit) { for (const uint256& nullifier : jsdesc.nullifiers) { - if (mapNullifiersToNotes.count(nullifier) && - mapWallet.count(mapNullifiersToNotes[nullifier].hash)) { - mapWallet[mapNullifiersToNotes[nullifier].hash].MarkDirty(); + if (mapSproutNullifiersToNotes.count(nullifier) && + mapWallet.count(mapSproutNullifiersToNotes[nullifier].hash)) { + mapWallet[mapSproutNullifiersToNotes[nullifier].hash].MarkDirty(); } } } @@ -1478,8 +1478,8 @@ bool CWallet::IsFromMe(const uint256& nullifier) const { { LOCK(cs_wallet); - if (mapNullifiersToNotes.count(nullifier) && - mapWallet.count(mapNullifiersToNotes.at(nullifier).hash)) { + if (mapSproutNullifiersToNotes.count(nullifier) && + mapWallet.count(mapSproutNullifiersToNotes.at(nullifier).hash)) { return true; } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1d56de863..dc00c4a7e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -895,7 +895,7 @@ public: * - Restarting the node with -reindex (which operates on a locked wallet * but with the now-cached nullifiers). */ - std::map mapNullifiersToNotes; + std::map mapSproutNullifiersToNotes; std::map mapWallet; From d5e490d9f2fa87ea9b08f7f8e899a1495c37873d Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 27 Jul 2018 16:56:26 -0700 Subject: [PATCH 04/40] Rename CWallet::AddToSpends methods for clarity. --- src/wallet/wallet.cpp | 13 +++++++++---- src/wallet/wallet.h | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1e4220140..3c15aa5ff 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -725,7 +725,7 @@ bool CWallet::IsSpent(const uint256& nullifier) const return false; } -void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) +void CWallet::AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid) { mapTxSpends.insert(make_pair(outpoint, wtxid)); @@ -734,7 +734,7 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) SyncMetaData(range); } -void CWallet::AddToSpends(const uint256& nullifier, const uint256& wtxid) +void CWallet::AddToSproutSpends(const uint256& nullifier, const uint256& wtxid) { mapTxNullifiers.insert(make_pair(nullifier, wtxid)); @@ -751,11 +751,11 @@ void CWallet::AddToSpends(const uint256& wtxid) return; for (const CTxIn& txin : thisTx.vin) { - AddToSpends(txin.prevout, wtxid); + AddToTransparentSpends(txin.prevout, wtxid); } for (const JSDescription& jsdesc : thisTx.vjoinsplit) { for (const uint256& nullifier : jsdesc.nullifiers) { - AddToSpends(nullifier, wtxid); + AddToSproutSpends(nullifier, wtxid); } } } @@ -1248,7 +1248,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD wtxIn.GetHash().ToString(), wtxIn.hashBlock.ToString()); } +<<<<<<< HEAD + AddToSproutSpends(hash); +======= AddToSpends(hash); + AddToSaplingSpends(hash); +>>>>>>> e9b1ce0... fix } bool fUpdated = false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index dc00c4a7e..93024cf94 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -717,8 +717,8 @@ private: typedef TxSpendMap TxNullifiers; TxNullifiers mapTxNullifiers; - void AddToSpends(const COutPoint& outpoint, const uint256& wtxid); - void AddToSpends(const uint256& nullifier, const uint256& wtxid); + void AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid); + void AddToSproutSpends(const uint256& nullifier, const uint256& wtxid); void AddToSpends(const uint256& wtxid); public: From 3438e26cc3f48d540e2c5443b609e7f23a2ef6ee Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 27 Jul 2018 16:56:57 -0700 Subject: [PATCH 05/40] Rename mapTxNullifiers to mapTxSproutNullifiers. --- src/wallet/wallet.cpp | 10 +++++----- src/wallet/wallet.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3c15aa5ff..7f18a7c6a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -583,10 +583,10 @@ set CWallet::GetConflicts(const uint256& txid) const for (const JSDescription& jsdesc : wtx.vjoinsplit) { for (const uint256& nullifier : jsdesc.nullifiers) { - if (mapTxNullifiers.count(nullifier) <= 1) { + if (mapTxSproutNullifiers.count(nullifier) <= 1) { continue; // No conflict if zero or one spends } - range_n = mapTxNullifiers.equal_range(nullifier); + range_n = mapTxSproutNullifiers.equal_range(nullifier); for (TxNullifiers::const_iterator it = range_n.first; it != range_n.second; ++it) { result.insert(it->second); } @@ -713,7 +713,7 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const bool CWallet::IsSpent(const uint256& nullifier) const { pair range; - range = mapTxNullifiers.equal_range(nullifier); + range = mapTxSproutNullifiers.equal_range(nullifier); for (TxNullifiers::const_iterator it = range.first; it != range.second; ++it) { const uint256& wtxid = it->second; @@ -736,10 +736,10 @@ void CWallet::AddToTransparentSpends(const COutPoint& outpoint, const uint256& w void CWallet::AddToSproutSpends(const uint256& nullifier, const uint256& wtxid) { - mapTxNullifiers.insert(make_pair(nullifier, wtxid)); + mapTxSproutNullifiers.insert(make_pair(nullifier, wtxid)); pair range; - range = mapTxNullifiers.equal_range(nullifier); + range = mapTxSproutNullifiers.equal_range(nullifier); SyncMetaData(range); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 93024cf94..e01a724b7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -715,7 +715,7 @@ private: * detect and report conflicts (double-spends). */ typedef TxSpendMap TxNullifiers; - TxNullifiers mapTxNullifiers; + TxNullifiers mapTxSproutNullifiers; void AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid); void AddToSproutSpends(const uint256& nullifier, const uint256& wtxid); From a132719da59e13574c96f9335dd79fbbc63f77ce Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 28 Jul 2018 00:04:09 -0700 Subject: [PATCH 06/40] Add ivk member variable and equality comparators to SaplingNoteData class. --- src/wallet/wallet.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e01a724b7..0a1be0387 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -267,13 +267,23 @@ class SaplingNoteData { public: /** - * We initialize the hight to -1 for the same reason as we do in SproutNoteData. + * We initialize the height to -1 for the same reason as we do in SproutNoteData. * See the comment in that class for a full description. */ SaplingNoteData() : witnessHeight {-1} { } + SaplingNoteData(libzcash::SaplingIncomingViewingKey ivk) : ivk {ivk}, witnessHeight {-1} { } std::list witnesses; int witnessHeight; + libzcash::SaplingIncomingViewingKey ivk; + + friend bool operator==(const SaplingNoteData& a, const SaplingNoteData& b) { + return (a.ivk == b.ivk && a.nullifier == b.nullifier && a.witnessHeight == b.witnessHeight); + } + + friend bool operator!=(const SaplingNoteData& a, const SaplingNoteData& b) { + return !(a == b); + } }; typedef std::map mapSproutNoteData_t; From dae1c4204a10d0f5f55bc9bbff0113f2886d1c2a Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 28 Jul 2018 10:02:48 -0700 Subject: [PATCH 07/40] Update CWallet::MarkAffectedTransactionsDirty() for Sapling. --- src/wallet/wallet.cpp | 10 +++++++++- src/wallet/wallet.h | 2 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7f18a7c6a..56ab1ccec 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1384,11 +1384,19 @@ void CWallet::MarkAffectedTransactionsDirty(const CTransaction& tx) for (const JSDescription& jsdesc : tx.vjoinsplit) { for (const uint256& nullifier : jsdesc.nullifiers) { if (mapSproutNullifiersToNotes.count(nullifier) && - mapWallet.count(mapSproutNullifiersToNotes[nullifier].hash)) { + mapWallet.count(mapSproutNullifiersToNotes[nullifier].hash)) { mapWallet[mapSproutNullifiersToNotes[nullifier].hash].MarkDirty(); } } } + + for (const SpendDescription &spend : tx.vShieldedSpend) { + uint256 nullifier = spend.nullifier; + if (mapSaplingNullifiersToNotes.count(nullifier) && + mapWallet.count(mapSaplingNullifiersToNotes[nullifier].hash)) { + mapWallet[mapSaplingNullifiersToNotes[nullifier].hash].MarkDirty(); + } + } } void CWallet::EraseFromWallet(const uint256 &hash) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0a1be0387..b369c0186 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -907,6 +907,8 @@ public: */ std::map mapSproutNullifiersToNotes; + std::map mapSaplingNullifiersToNotes; + std::map mapWallet; int64_t nOrderPosNext; From 3a83e7c9a45f4c77f213dcf509f72e5450de4af4 Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 28 Jul 2018 10:04:54 -0700 Subject: [PATCH 08/40] Update CWallet::UpdatedNoteData() for Sapling. --- src/wallet/wallet.cpp | 44 ++++++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 56ab1ccec..ce34c4cc9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1310,21 +1310,39 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx) { - if (wtxIn.mapSproutNoteData.empty() || wtxIn.mapSproutNoteData == wtx.mapSproutNoteData) { - return false; - } - auto tmp = wtxIn.mapSproutNoteData; - // Ensure we keep any cached witnesses we may already have - for (const std::pair nd : wtx.mapSproutNoteData) { - if (tmp.count(nd.first) && nd.second.witnesses.size() > 0) { - tmp.at(nd.first).witnesses.assign( - nd.second.witnesses.cbegin(), nd.second.witnesses.cend()); + bool unchangedSproutFlag = (wtxIn.mapSproutNoteData.empty() || wtxIn.mapSproutNoteData == wtx.mapSproutNoteData); + if (!unchangedSproutFlag) { + auto tmp = wtxIn.mapSproutNoteData; + // Ensure we keep any cached witnesses we may already have + for (const std::pair nd : wtx.mapSproutNoteData) { + if (tmp.count(nd.first) && nd.second.witnesses.size() > 0) { + tmp.at(nd.first).witnesses.assign( + nd.second.witnesses.cbegin(), nd.second.witnesses.cend()); + } + tmp.at(nd.first).witnessHeight = nd.second.witnessHeight; } - tmp.at(nd.first).witnessHeight = nd.second.witnessHeight; + // Now copy over the updated note data + wtx.mapSproutNoteData = tmp; } - // Now copy over the updated note data - wtx.mapSproutNoteData = tmp; - return true; + + bool unchangedSaplingFlag = (wtxIn.mapSaplingNoteData.empty() || wtxIn.mapSaplingNoteData == wtx.mapSaplingNoteData); + if (!unchangedSaplingFlag) { + auto tmp = wtxIn.mapSaplingNoteData; + // Ensure we keep any cached witnesses we may already have + + for (const std::pair nd : wtx.mapSaplingNoteData) { + if (tmp.count(nd.first) && nd.second.witnesses.size() > 0) { + tmp.at(nd.first).witnesses.assign( + nd.second.witnesses.cbegin(), nd.second.witnesses.cend()); + } + tmp.at(nd.first).witnessHeight = nd.second.witnessHeight; + } + + // Now copy over the updated note data + wtx.mapSaplingNoteData = tmp; + } + + return !unchangedSproutFlag || !unchangedSaplingFlag; } /** From 52332fb4176a50613e783f13147e701bbe825758 Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 28 Jul 2018 10:06:55 -0700 Subject: [PATCH 09/40] Create CWallet::AddToSaplingSpends() to track Sapling nullifiers. --- src/wallet/wallet.cpp | 17 ++++++++++++----- src/wallet/wallet.h | 2 ++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ce34c4cc9..2e09f5546 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -758,6 +758,18 @@ void CWallet::AddToSpends(const uint256& wtxid) AddToSproutSpends(nullifier, wtxid); } } + for (const SpendDescription &spend : thisTx.vShieldedSpend) { + AddToSaplingSpends(spend.nullifier, wtxid); + } +} + +void CWallet::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) +{ + mapTxSaplingNullifiers.insert(make_pair(nullifier, wtxid)); + + pair range; + range = mapTxSaplingNullifiers.equal_range(nullifier); + SyncMetaData(range); } void CWallet::ClearNoteWitnessCache() @@ -1248,12 +1260,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD wtxIn.GetHash().ToString(), wtxIn.hashBlock.ToString()); } -<<<<<<< HEAD - AddToSproutSpends(hash); -======= AddToSpends(hash); - AddToSaplingSpends(hash); ->>>>>>> e9b1ce0... fix } bool fUpdated = false; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b369c0186..4c0188138 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -726,10 +726,12 @@ private: */ typedef TxSpendMap TxNullifiers; TxNullifiers mapTxSproutNullifiers; + TxNullifiers mapTxSaplingNullifiers; void AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid); void AddToSproutSpends(const uint256& nullifier, const uint256& wtxid); void AddToSpends(const uint256& wtxid); + void AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid); public: /* From 69c4391b0f5ff225bf063df6dc5ceff4549191c1 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Mon, 30 Jul 2018 14:37:12 -0600 Subject: [PATCH 10/40] Check commitment validity within the decryption API for Sapling note plaintexts. --- src/gtest/test_noteencryption.cpp | 32 ++++++++++++++++++++--- src/zcash/Note.cpp | 43 +++++++++++++++++++++++++++++-- src/zcash/Note.hpp | 6 +++-- 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index a674daf65..0ed6999f8 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -35,6 +35,11 @@ TEST(noteencryption, NotePlaintext) } SaplingNote note(addr, 39393); + auto cmu_opt = note.cm(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); SaplingNotePlaintext pt(note, memo); auto res = pt.encrypt(addr.pk_d); @@ -48,11 +53,20 @@ TEST(noteencryption, NotePlaintext) auto encryptor = enc.second; auto epk = encryptor.get_epk(); - // Try to decrypt + // Try to decrypt with incorrect commitment + ASSERT_FALSE(SaplingNotePlaintext::decrypt( + ct, + ivk, + epk, + uint256() + )); + + // Try to decrypt with correct commitment auto foo = SaplingNotePlaintext::decrypt( ct, ivk, - epk + epk, + cmu ); if (!foo) { @@ -112,12 +126,24 @@ TEST(noteencryption, NotePlaintext) ASSERT_TRUE(decrypted_out_ct_unwrapped.pk_d == out_pt.pk_d); ASSERT_TRUE(decrypted_out_ct_unwrapped.esk == out_pt.esk); + // Test sender won't accept invalid commitments + ASSERT_FALSE( + SaplingNotePlaintext::decrypt( + ct, + epk, + decrypted_out_ct_unwrapped.esk, + decrypted_out_ct_unwrapped.pk_d, + uint256() + ) + ); + // Test sender can decrypt the note ciphertext. foo = SaplingNotePlaintext::decrypt( ct, epk, decrypted_out_ct_unwrapped.esk, - decrypted_out_ct_unwrapped.pk_d + decrypted_out_ct_unwrapped.pk_d, + cmu ); if (!foo) { diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index c6c72e297..ee8f7b641 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -187,7 +187,8 @@ boost::optional SaplingOutgoingPlaintext::decrypt( boost::optional SaplingNotePlaintext::decrypt( const SaplingEncCiphertext &ciphertext, const uint256 &ivk, - const uint256 &epk + const uint256 &epk, + const uint256 &cmu ) { auto pt = AttemptSaplingEncDecryption(ciphertext, ivk, epk); @@ -204,6 +205,27 @@ boost::optional SaplingNotePlaintext::decrypt( assert(ss.size() == 0); + uint256 pk_d; + if (!librustzcash_ivk_to_pkd(ivk.begin(), ret.d.data(), pk_d.begin())) { + return boost::none; + } + + uint256 cmu_expected; + if (!librustzcash_sapling_compute_cm( + ret.d.data(), + pk_d.begin(), + ret.value(), + ret.rcm.begin(), + cmu_expected.begin() + )) + { + return boost::none; + } + + if (cmu_expected != cmu) { + return boost::none; + } + return ret; } @@ -211,7 +233,8 @@ boost::optional SaplingNotePlaintext::decrypt( const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, - const uint256 &pk_d + const uint256 &pk_d, + const uint256 &cmu ) { auto pt = AttemptSaplingEncDecryption(ciphertext, epk, esk, pk_d); @@ -226,6 +249,22 @@ boost::optional SaplingNotePlaintext::decrypt( SaplingNotePlaintext ret; ss >> ret; + uint256 cmu_expected; + if (!librustzcash_sapling_compute_cm( + ret.d.data(), + pk_d.begin(), + ret.value(), + ret.rcm.begin(), + cmu_expected.begin() + )) + { + return boost::none; + } + + if (cmu_expected != cmu) { + return boost::none; + } + assert(ss.size() == 0); return ret; diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index f1b8e4323..7d3377306 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -130,14 +130,16 @@ public: static boost::optional decrypt( const SaplingEncCiphertext &ciphertext, const uint256 &ivk, - const uint256 &epk + const uint256 &epk, + const uint256 &cmu ); static boost::optional decrypt( const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, - const uint256 &pk_d + const uint256 &pk_d, + const uint256 &cmu ); boost::optional note(const SaplingIncomingViewingKey& ivk) const; From d4d0ec7e9513b021f0edb487a0e93bedce29b5a2 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 1 Aug 2018 14:02:32 -0700 Subject: [PATCH 11/40] Update test to pass in required cm to SaplingNotePlaintext::decrypt(). --- src/gtest/test_transaction_builder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index e3f3655d3..b8fe54466 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -56,7 +56,7 @@ TEST(TransactionBuilder, Invoke) // Prepare to spend the note that was just created auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( - tx1.vShieldedOutput[0].encCiphertext, ivk, tx1.vShieldedOutput[0].ephemeralKey); + tx1.vShieldedOutput[0].encCiphertext, ivk, tx1.vShieldedOutput[0].ephemeralKey, tx1.vShieldedOutput[0].cm); ASSERT_EQ(static_cast(maybe_pt), true); auto maybe_note = maybe_pt.get().note(ivk); ASSERT_EQ(static_cast(maybe_note), true); From 78584ef794f8d6a9af93348c4186d0ad63cacd7b Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 28 Jul 2018 10:09:42 -0700 Subject: [PATCH 12/40] Create CWallet::FindMySaplingNotes() --- src/wallet/wallet.cpp | 52 ++++++++++++++++++++++++++++++++++++++----- src/wallet/wallet.h | 1 + 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2e09f5546..d5cbd0885 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1363,15 +1363,19 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; - auto noteData = FindMySproutNotes(tx); - if (fExisted || IsMine(tx) || IsFromMe(tx) || noteData.size() > 0) + auto sproutNoteData = FindMySproutNotes(tx); + auto saplingNoteData = FindMySaplingNotes(tx); + if (fExisted || IsMine(tx) || IsFromMe(tx) || sproutNoteData.size() > 0 || saplingNoteData.size() > 0) { CWalletTx wtx(this,tx); - if (noteData.size() > 0) { - wtx.SetSproutNoteData(noteData); + if (sproutNoteData.size() > 0) { + wtx.SetSproutNoteData(sproutNoteData); + } + + if (saplingNoteData.size() > 0) { + wtx.SetSaplingNoteData(saplingNoteData); } - // TODO: Sapling note data // Get merkle branch if transaction was found in a block if (pblock) @@ -1512,6 +1516,44 @@ mapSproutNoteData_t CWallet::FindMySproutNotes(const CTransaction &tx) const return noteData; } + +/** + * Finds all output notes in the given transaction that have been sent to + * SaplingPaymentAddresses in this wallet. + * + * It should never be necessary to call this method with a CWalletTx, because + * the result of FindMySaplingNotes (for the addresses available at the time) will + * already have been cached in CWalletTx.mapSaplingNoteData. + */ +mapSaplingNoteData_t CWallet::FindMySaplingNotes(const CTransaction &tx) const +{ + LOCK(cs_SpendingKeyStore); + uint256 hash = tx.GetHash(); + + mapSaplingNoteData_t noteData; + + // Protocol Spec: 4.19 Block Chain Scanning (Sapling) + for (uint32_t i = 0; i < tx.vShieldedOutput.size(); ++i) { + const OutputDescription output = tx.vShieldedOutput[i]; + for (auto it = mapSaplingIncomingViewingKeys.begin(); it != mapSaplingIncomingViewingKeys.end(); ++it) { + SaplingIncomingViewingKey ivk = it->second; + auto result = SaplingNotePlaintext::decrypt(output.encCiphertext, ivk, output.ephemeralKey, output.cm); + if (!result) { + continue; + } + // We don't cache the nullifier here as computing it requires knowledge of the note position + // in the commitment tree, which can only be determined when the transaction has been mined. + SaplingOutPoint op {hash, i}; + SaplingNoteData nd; + nd.ivk = ivk; + noteData.insert(std::make_pair(op, nd)); + break; + } + } + + return noteData; +} + bool CWallet::IsFromMe(const uint256& nullifier) const { { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4c0188138..675a4428f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1097,6 +1097,7 @@ public: const uint256& hSig, uint8_t n) const; mapSproutNoteData_t FindMySproutNotes(const CTransaction& tx) const; + mapSaplingNoteData_t FindMySaplingNotes(const CTransaction& tx) const; bool IsFromMe(const uint256& nullifier) const; void GetSproutNoteWitnesses( std::vector notes, From 037cacf2ceb3a39472342b8e0c318ce708ad86ce Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 30 Jul 2018 15:35:34 -0700 Subject: [PATCH 13/40] Rename IsFromMe(nullifier) to IsSproutNullifierFromMe(nullifier). --- src/wallet/wallet.cpp | 8 ++++---- src/wallet/wallet.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d5cbd0885..a6b55f106 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1554,7 +1554,7 @@ mapSaplingNoteData_t CWallet::FindMySaplingNotes(const CTransaction &tx) const return noteData; } -bool CWallet::IsFromMe(const uint256& nullifier) const +bool CWallet::IsSproutNullifierFromMe(const uint256& nullifier) const { { LOCK(cs_wallet); @@ -1707,7 +1707,7 @@ bool CWallet::IsFromMe(const CTransaction& tx) const } for (const JSDescription& jsdesc : tx.vjoinsplit) { for (const uint256& nullifier : jsdesc.nullifiers) { - if (IsFromMe(nullifier)) { + if (IsSproutNullifierFromMe(nullifier)) { return true; } } @@ -1841,7 +1841,7 @@ void CWalletTx::GetAmounts(list& listReceived, bool isFromMyZaddr = false; for (const JSDescription& js : vjoinsplit) { for (const uint256& nullifier : js.nullifiers) { - if (pwallet->IsFromMe(nullifier)) { + if (pwallet->IsSproutNullifierFromMe(nullifier)) { isFromMyZaddr = true; break; } @@ -1868,7 +1868,7 @@ void CWalletTx::GetAmounts(list& listReceived, // Check input side for (const uint256& nullifier : js.nullifiers) { - if (pwallet->IsFromMe(nullifier)) { + if (pwallet->IsSproutNullifierFromMe(nullifier)) { fMyJSDesc = true; break; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 675a4428f..aec720073 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1098,7 +1098,7 @@ public: uint8_t n) const; mapSproutNoteData_t FindMySproutNotes(const CTransaction& tx) const; mapSaplingNoteData_t FindMySaplingNotes(const CTransaction& tx) const; - bool IsFromMe(const uint256& nullifier) const; + bool IsSproutNullifierFromMe(const uint256& nullifier) const; void GetSproutNoteWitnesses( std::vector notes, std::vector>& witnesses, From d7cf640bbf1faa709d788c3b237f5ce679de6ba3 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 30 Jul 2018 17:09:13 -0700 Subject: [PATCH 14/40] Create CWallet::IsSaplingNullifierFromMe() --- src/wallet/wallet.cpp | 17 +++++++++++++++++ src/wallet/wallet.h | 2 ++ 2 files changed, 19 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a6b55f106..36595a907 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1566,6 +1566,18 @@ bool CWallet::IsSproutNullifierFromMe(const uint256& nullifier) const return false; } +bool CWallet::IsSaplingNullifierFromMe(const uint256& nullifier) const +{ + { + LOCK(cs_wallet); + if (mapSaplingNullifiersToNotes.count(nullifier) && + mapWallet.count(mapSaplingNullifiersToNotes.at(nullifier).hash)) { + return true; + } + } + return false; +} + void CWallet::GetSproutNoteWitnesses(std::vector notes, std::vector>& witnesses, uint256 &final_anchor) @@ -1712,6 +1724,11 @@ bool CWallet::IsFromMe(const CTransaction& tx) const } } } + for (const SpendDescription &spend : tx.vShieldedSpend) { + if (IsSaplingNullifierFromMe(spend.nullifier)) { + return true; + } + } return false; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index aec720073..d020357a2 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1099,6 +1099,8 @@ public: mapSproutNoteData_t FindMySproutNotes(const CTransaction& tx) const; mapSaplingNoteData_t FindMySaplingNotes(const CTransaction& tx) const; bool IsSproutNullifierFromMe(const uint256& nullifier) const; + bool IsSaplingNullifierFromMe(const uint256& nullifier) const; + void GetSproutNoteWitnesses( std::vector notes, std::vector>& witnesses, From c47c1e936366541f2a40907ea57f3b42986f2f68 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 31 Jul 2018 14:42:20 -0700 Subject: [PATCH 15/40] Remove dead code in CWalletTx::GetAmounts() as filed in issue #3434. --- src/wallet/wallet.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 36595a907..255213d64 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1854,20 +1854,6 @@ void CWalletTx::GetAmounts(list& listReceived, CAmount nDebit = GetDebit(filter); bool isFromMyTaddr = nDebit > 0; // debit>0 means we signed/sent this transaction - // Does this tx spend my notes? - bool isFromMyZaddr = false; - for (const JSDescription& js : vjoinsplit) { - for (const uint256& nullifier : js.nullifiers) { - if (pwallet->IsSproutNullifierFromMe(nullifier)) { - isFromMyZaddr = true; - break; - } - } - if (isFromMyZaddr) { - break; - } - } - // Compute fee if we sent this transaction. if (isFromMyTaddr) { CAmount nValueOut = GetValueOut(); // transparent outputs plus all vpub_old From f686388991251b7ac7dde4bb311738432cbfa6b9 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 31 Jul 2018 20:11:39 -0700 Subject: [PATCH 16/40] Cleanup CWalletTx::GetAmounts() for clarity. No-op. --- src/wallet/wallet.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 255213d64..e93525ff5 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1856,9 +1856,8 @@ void CWalletTx::GetAmounts(list& listReceived, // Compute fee if we sent this transaction. if (isFromMyTaddr) { - CAmount nValueOut = GetValueOut(); // transparent outputs plus all vpub_old - CAmount nValueIn = 0; - nValueIn += GetShieldedValueIn(); + CAmount nValueOut = GetValueOut(); // transparent outputs plus all Sprout vpub_old and negative Sapling valueBalance + CAmount nValueIn = GetShieldedValueIn(); nFee = nDebit - nValueOut + nValueIn; } From f9816408b2120bb9524934fbba7f4b0bfbba0535 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 31 Jul 2018 20:15:00 -0700 Subject: [PATCH 17/40] Update CWalletTx::GetAmounts() to return COutputEntry for Sapling valueBalance. --- src/wallet/wallet.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e93525ff5..2aa589dae 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1906,6 +1906,18 @@ void CWalletTx::GetAmounts(list& listReceived, } } + // If we sent utxos from this transaction, create output for value taken from (negative valueBalance) + // or added (positive valueBalance) to the transparent value pool by Sapling shielding and unshielding. + if (isFromMyTaddr) { + if (valueBalance < 0) { + COutputEntry output = {CNoDestination(), -valueBalance, (int) vout.size()}; + listSent.push_back(output); + } else if (valueBalance > 0) { + COutputEntry output = {CNoDestination(), valueBalance, (int) vout.size()}; + listReceived.push_back(output); + } + } + // Sent/received. for (unsigned int i = 0; i < vout.size(); ++i) { From ad1e90dd34981e93abe526173e1f2b97d4ed5fd7 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 1 Aug 2018 15:28:27 -0700 Subject: [PATCH 18/40] Add caching and updating of Sapling note nullifier. --- src/wallet/wallet.cpp | 74 ++++++++++++++++++++++++++++++++++++++++++- src/wallet/wallet.h | 8 +++-- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2aa589dae..cb6d6f0d2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -474,8 +474,10 @@ void CWallet::ChainTip(const CBlockIndex *pindex, { if (added) { IncrementNoteWitnesses(pindex, pblock, sproutTree, saplingTree); + UpdateSaplingNullifierNoteMapForBlock(pblock); } else { DecrementNoteWitnesses(pindex); + UpdateSaplingNullifierNoteMapForBlock(pblock); } } @@ -1169,6 +1171,10 @@ bool CWallet::UpdateNullifierNoteMap() } } } + + // TODO: Sapling. This method is only called from RPC walletpassphrase, which is currently unsupported + // as RPC encryptwallet is hidden behind two flags: -developerencryptwallet -experimentalfeatures + UpdateNullifierNoteMapWithTx(wtxItem.second); } } @@ -1176,7 +1182,8 @@ bool CWallet::UpdateNullifierNoteMap() } /** - * Update mapSproutNullifiersToNotes with the cached nullifiers in this tx. + * Update mapSproutNullifiersToNotes and mapSaplingNullifiersToNotes + * with the cached nullifiers in this tx. */ void CWallet::UpdateNullifierNoteMapWithTx(const CWalletTx& wtx) { @@ -1187,6 +1194,71 @@ void CWallet::UpdateNullifierNoteMapWithTx(const CWalletTx& wtx) mapSproutNullifiersToNotes[*item.second.nullifier] = item.first; } } + + for (const mapSaplingNoteData_t::value_type& item : wtx.mapSaplingNoteData) { + if (item.second.nullifier) { + mapSaplingNullifiersToNotes[*item.second.nullifier] = item.first; + } + } + } +} + +/** + * Update mapSaplingNullifiersToNotes, computing the nullifier from a cached witness if necessary. + */ +void CWallet::UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx) { + LOCK(cs_wallet); + + for (mapSaplingNoteData_t::value_type &item : wtx.mapSaplingNoteData) { + SaplingOutPoint op = item.first; + SaplingNoteData nd = item.second; + + if (nd.witnesses.size() == 0) { + // If there are no witnesses, erase the nullifier and associated mapping. + if (item.second.nullifier) { + mapSaplingNullifiersToNotes.erase(item.second.nullifier.get()); + } + item.second.nullifier = boost::none; + } + else if (nd.witnesses.size() > 0) { + uint64_t position = nd.witnesses.front().position(); + SaplingFullViewingKey fvk = mapSaplingFullViewingKeys.at(nd.ivk); + OutputDescription output = wtx.vShieldedOutput[op.n]; + auto optPlaintext = SaplingNotePlaintext::decrypt(output.encCiphertext, nd.ivk, output.ephemeralKey, output.cm); + if (!optPlaintext) { + // An item in mapSaplingNoteData must have already been successfully decrypted, + // otherwise the item would not exist in the first place. + assert(false); + } + auto optNote = optPlaintext.get().note(nd.ivk); + if (!optNote) { + assert(false); + } + auto optNullifier = optNote.get().nullifier(fvk, position); + if (!optNullifier) { + // This should not happen. If it does, maybe the position has been corrupted or miscalculated? + assert(false); + } + uint256 nullifier = optNullifier.get(); + mapSaplingNullifiersToNotes[nullifier] = op; + item.second.nullifier = nullifier; + } + } +} + +/** + * Iterate over transactions in a block and update the cached Sapling nullifiers + * for transactions which belong to the wallet. + */ +void CWallet::UpdateSaplingNullifierNoteMapForBlock(const CBlock *pblock) { + LOCK(cs_wallet); + + for (const CTransaction& tx : pblock->vtx) { + auto hash = tx.GetHash(); + bool txIsOurs = mapWallet.count(hash); + if (txIsOurs) { + UpdateSaplingNullifierNoteMapWithTx(mapWallet[hash]); + } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d020357a2..00e0af7d9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -270,12 +270,14 @@ public: * We initialize the height to -1 for the same reason as we do in SproutNoteData. * See the comment in that class for a full description. */ - SaplingNoteData() : witnessHeight {-1} { } - SaplingNoteData(libzcash::SaplingIncomingViewingKey ivk) : ivk {ivk}, witnessHeight {-1} { } + SaplingNoteData() : witnessHeight {-1}, nullifier() { } + SaplingNoteData(libzcash::SaplingIncomingViewingKey ivk) : ivk {ivk}, witnessHeight {-1}, nullifier() { } + SaplingNoteData(libzcash::SaplingIncomingViewingKey ivk, uint256 n) : ivk {ivk}, witnessHeight {-1}, nullifier(n) { } std::list witnesses; int witnessHeight; libzcash::SaplingIncomingViewingKey ivk; + boost::optional nullifier; friend bool operator==(const SaplingNoteData& a, const SaplingNoteData& b) { return (a.ivk == b.ivk && a.nullifier == b.nullifier && a.witnessHeight == b.witnessHeight); @@ -1050,6 +1052,8 @@ public: void MarkDirty(); bool UpdateNullifierNoteMap(); void UpdateNullifierNoteMapWithTx(const CWalletTx& wtx); + void UpdateSaplingNullifierNoteMapWithTx(CWalletTx& tx); + void UpdateSaplingNullifierNoteMapForBlock(const CBlock* pblock); bool AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletDB* pwalletdb); void SyncTransaction(const CTransaction& tx, const CBlock* pblock); bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate); From edfc6a787f512788bacec73743e25f34102c0137 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 18:21:28 -0700 Subject: [PATCH 19/40] Update CWallet::IsSpent() to check Sapling nullifiers. --- src/wallet/wallet.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cb6d6f0d2..aeeda4d6b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -717,6 +717,16 @@ bool CWallet::IsSpent(const uint256& nullifier) const pair range; range = mapTxSproutNullifiers.equal_range(nullifier); + for (TxNullifiers::const_iterator it = range.first; it != range.second; ++it) { + const uint256& wtxid = it->second; + std::map::const_iterator mit = mapWallet.find(wtxid); + if (mit != mapWallet.end() && mit->second.GetDepthInMainChain() >= 0) { + return true; // Spent + } + } + + range = mapTxSaplingNullifiers.equal_range(nullifier); + for (TxNullifiers::const_iterator it = range.first; it != range.second; ++it) { const uint256& wtxid = it->second; std::map::const_iterator mit = mapWallet.find(wtxid); From f12daeb44a13783faba668037fd89bf97c15df23 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 19:00:03 -0700 Subject: [PATCH 20/40] Clean up names of unit tests in gtest/test_wallet.cpp. --- src/wallet/gtest/test_wallet.cpp | 44 ++++++++++++++++---------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 4b368afed..9ab62c0d4 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -131,14 +131,14 @@ std::pair GetWitnessesAndAnchors(TestWallet& wallet, return std::make_pair(sproutAnchor, saplingAnchor); } -TEST(wallet_tests, setup_datadir_location_run_as_first_test) { +TEST(WalletTests, SetupDatadirLocationRunAsFirstTest) { // Get temporary and unique path for file. boost::filesystem::path pathTemp = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path(); boost::filesystem::create_directories(pathTemp); mapArgs["-datadir"] = pathTemp.string(); } -TEST(wallet_tests, note_data_serialisation) { +TEST(WalletTests, NoteDataSerialisation) { auto sk = libzcash::SproutSpendingKey::random(); auto wtx = GetValidReceive(sk, 10, true); auto note = GetNote(sk, wtx, 0, 1); @@ -162,7 +162,7 @@ TEST(wallet_tests, note_data_serialisation) { } -TEST(wallet_tests, find_unspent_notes) { +TEST(WalletTests, FindUnspentNotes) { SelectParams(CBaseChainParams::TESTNET); CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -321,7 +321,7 @@ TEST(wallet_tests, find_unspent_notes) { } -TEST(wallet_tests, set_note_addrs_in_cwallettx) { +TEST(WalletTests, SetNoteAddrsInCWalletTx) { auto sk = libzcash::SproutSpendingKey::random(); auto wtx = GetValidReceive(sk, 10, true); auto note = GetNote(sk, wtx, 0, 1); @@ -337,7 +337,7 @@ TEST(wallet_tests, set_note_addrs_in_cwallettx) { EXPECT_EQ(noteData, wtx.mapSproutNoteData); } -TEST(wallet_tests, set_invalid_note_addrs_in_cwallettx) { +TEST(WalletTests, SetInvalidNoteAddrsInCWalletTx) { CWalletTx wtx; EXPECT_EQ(0, wtx.mapSproutNoteData.size()); @@ -350,7 +350,7 @@ TEST(wallet_tests, set_invalid_note_addrs_in_cwallettx) { EXPECT_THROW(wtx.SetSproutNoteData(noteData), std::logic_error); } -TEST(wallet_tests, GetSproutNoteNullifier) { +TEST(WalletTests, GetSproutNoteNullifier) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -381,7 +381,7 @@ TEST(wallet_tests, GetSproutNoteNullifier) { EXPECT_EQ(nullifier, ret); } -TEST(wallet_tests, FindMySproutNotes) { +TEST(WalletTests, FindMySproutNotes) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -406,7 +406,7 @@ TEST(wallet_tests, FindMySproutNotes) { EXPECT_EQ(nd, noteMap[jsoutpt]); } -TEST(wallet_tests, FindMySproutNotesInEncryptedWallet) { +TEST(WalletTests, FindMySproutNotesInEncryptedWallet) { TestWallet wallet; uint256 r {GetRandHash()}; CKeyingMaterial vMasterKey (r.begin(), r.end()); @@ -436,7 +436,7 @@ TEST(wallet_tests, FindMySproutNotesInEncryptedWallet) { EXPECT_EQ(nd, noteMap[jsoutpt]); } -TEST(wallet_tests, get_conflicted_notes) { +TEST(WalletTests, GetConflictedNotes) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -467,7 +467,7 @@ TEST(wallet_tests, get_conflicted_notes) { EXPECT_EQ(std::set({hash2, hash3}), c3); } -TEST(wallet_tests, nullifier_is_spent) { +TEST(WalletTests, NullifierIsSpent) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -507,7 +507,7 @@ TEST(wallet_tests, nullifier_is_spent) { mapBlockIndex.erase(blockHash); } -TEST(wallet_tests, navigate_from_nullifier_to_note) { +TEST(WalletTests, NavigateFromNullifierToNote) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -533,7 +533,7 @@ TEST(wallet_tests, navigate_from_nullifier_to_note) { EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes[nullifier].n); } -TEST(wallet_tests, spent_note_is_from_me) { +TEST(WalletTests, SpentNoteIsFromMe) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -561,7 +561,7 @@ TEST(wallet_tests, spent_note_is_from_me) { EXPECT_TRUE(wallet.IsFromMe(wtx2)); } -TEST(wallet_tests, cached_witnesses_empty_chain) { +TEST(WalletTests, CachedWitnessesEmptyChain) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -620,7 +620,7 @@ TEST(wallet_tests, cached_witnesses_empty_chain) { ".*nWitnessCacheSize > 0.*"); } -TEST(wallet_tests, cached_witnesses_chain_tip) { +TEST(WalletTests, CachedWitnessesChainTip) { TestWallet wallet; std::pair anchors1; CBlock block1; @@ -722,7 +722,7 @@ TEST(wallet_tests, cached_witnesses_chain_tip) { } } -TEST(wallet_tests, CachedWitnessesDecrementFirst) { +TEST(WalletTests, CachedWitnessesDecrementFirst) { TestWallet wallet; SproutMerkleTree sproutTree; SaplingMerkleTree saplingTree; @@ -802,7 +802,7 @@ TEST(wallet_tests, CachedWitnessesDecrementFirst) { } } -TEST(wallet_tests, CachedWitnessesCleanIndex) { +TEST(WalletTests, CachedWitnessesCleanIndex) { TestWallet wallet; std::vector blocks; std::vector indices; @@ -889,7 +889,7 @@ TEST(wallet_tests, CachedWitnessesCleanIndex) { } } -TEST(wallet_tests, ClearNoteWitnessCache) { +TEST(WalletTests, ClearNoteWitnessCache) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -947,7 +947,7 @@ TEST(wallet_tests, ClearNoteWitnessCache) { EXPECT_EQ(0, wallet.nWitnessCacheSize); } -TEST(wallet_tests, WriteWitnessCache) { +TEST(WalletTests, WriteWitnessCache) { TestWallet wallet; MockWalletDB walletdb; CBlockLocator loc; @@ -1024,7 +1024,7 @@ TEST(wallet_tests, WriteWitnessCache) { wallet.SetBestChain(walletdb, loc); } -TEST(wallet_tests, UpdateNullifierNoteMap) { +TEST(WalletTests, UpdateNullifierNoteMap) { TestWallet wallet; uint256 r {GetRandHash()}; CKeyingMaterial vMasterKey (r.begin(), r.end()); @@ -1059,7 +1059,7 @@ TEST(wallet_tests, UpdateNullifierNoteMap) { EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes[nullifier].n); } -TEST(wallet_tests, UpdatedNoteData) { +TEST(WalletTests, UpdatedNoteData) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -1106,7 +1106,7 @@ TEST(wallet_tests, UpdatedNoteData) { // TODO: The new note should get witnessed (but maybe not here) (#1350) } -TEST(wallet_tests, MarkAffectedTransactionsDirty) { +TEST(WalletTests, MarkAffectedTransactionsDirty) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -1137,7 +1137,7 @@ TEST(wallet_tests, MarkAffectedTransactionsDirty) { EXPECT_FALSE(wallet.mapWallet[hash].fDebitCached); } -TEST(wallet_tests, NoteLocking) { +TEST(WalletTests, NoteLocking) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); From 28d3dc853235e484835756c7a1b27f28cf887ea8 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 19:13:24 -0700 Subject: [PATCH 21/40] Add test for CWalletTx::SetSaplingNoteData() --- src/wallet/gtest/test_wallet.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 9ab62c0d4..63dd4d891 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -350,6 +350,19 @@ TEST(WalletTests, SetInvalidNoteAddrsInCWalletTx) { EXPECT_THROW(wtx.SetSproutNoteData(noteData), std::logic_error); } +// Cannot add note data for an index which does not exist in tx.vShieldedOutput +TEST(WalletTests, SetInvalidSaplingNoteDataInCWalletTx) { + CWalletTx wtx; + EXPECT_EQ(0, wtx.mapSaplingNoteData.size()); + + mapSaplingNoteData_t noteData; + SaplingOutPoint op {uint256(), 1}; + SaplingNoteData nd; + noteData.insert(std::make_pair(op, nd)); + + EXPECT_THROW(wtx.SetSaplingNoteData(noteData), std::logic_error); +} + TEST(WalletTests, GetSproutNoteNullifier) { CWallet wallet; From ec064abbdf0ad3837bf321268051ec305e3196fa Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 22:30:30 -0700 Subject: [PATCH 22/40] Iterate over mapSaplingFullViewingKeys with ivk->fvk mapping (1:1). When diversified addresses are supported, iterating over mapSaplingIncomingViewingKeys will be inefficient as the mapping will be addresses->ivk (n:1). --- src/wallet/wallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index aeeda4d6b..6f682cc05 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1617,8 +1617,8 @@ mapSaplingNoteData_t CWallet::FindMySaplingNotes(const CTransaction &tx) const // Protocol Spec: 4.19 Block Chain Scanning (Sapling) for (uint32_t i = 0; i < tx.vShieldedOutput.size(); ++i) { const OutputDescription output = tx.vShieldedOutput[i]; - for (auto it = mapSaplingIncomingViewingKeys.begin(); it != mapSaplingIncomingViewingKeys.end(); ++it) { - SaplingIncomingViewingKey ivk = it->second; + for (auto it = mapSaplingFullViewingKeys.begin(); it != mapSaplingFullViewingKeys.end(); ++it) { + SaplingIncomingViewingKey ivk = it->first; auto result = SaplingNotePlaintext::decrypt(output.encCiphertext, ivk, output.ephemeralKey, output.cm); if (!result) { continue; From 3b6dd486b438841b784bd604bd546729fda2ed99 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 23:12:25 -0700 Subject: [PATCH 23/40] Refactor IsSpent(nullifier) for Sprout and Sapling domain separation. We need separate functions for checking Sprout and Sapling nullifiers, because they are in separate domains and aren't guaranteed to be collision-resistant (otherwise there is a possibility of a nullifier collision, however remote, between Sprout and Sapling causing the spend of one to prevent the spend of the other). --- src/wallet/gtest/test_wallet.cpp | 18 +++++++++--------- src/wallet/wallet.cpp | 11 +++++++---- src/wallet/wallet.h | 3 ++- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 63dd4d891..18bda4a65 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -179,7 +179,7 @@ TEST(WalletTests, FindUnspentNotes) { wtx.SetSproutNoteData(noteData); wallet.AddToWallet(wtx, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); // We currently have an unspent and unconfirmed note in the wallet (depth of -1) std::vector entries; @@ -204,7 +204,7 @@ TEST(WalletTests, FindUnspentNotes) { wtx.SetMerkleBranch(block); wallet.AddToWallet(wtx, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); // We now have an unspent and confirmed note in the wallet (depth of 1) @@ -222,7 +222,7 @@ TEST(WalletTests, FindUnspentNotes) { // Let's spend the note. auto wtx2 = GetValidSpend(sk, note, 5); wallet.AddToWallet(wtx2, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); // Fake-mine a spend transaction EXPECT_EQ(0, chainActive.Height()); @@ -240,7 +240,7 @@ TEST(WalletTests, FindUnspentNotes) { wtx2.SetMerkleBranch(block2); wallet.AddToWallet(wtx2, true, NULL); - EXPECT_TRUE(wallet.IsSpent(nullifier)); + EXPECT_TRUE(wallet.IsSproutSpent(nullifier)); // The note has been spent. By default, GetFilteredNotes() ignores spent notes. wallet.GetFilteredNotes(entries, "", 0); @@ -274,7 +274,7 @@ TEST(WalletTests, FindUnspentNotes) { wtx.SetSproutNoteData(noteData); wallet.AddToWallet(wtx, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); wtx3 = wtx; } @@ -490,14 +490,14 @@ TEST(WalletTests, NullifierIsSpent) { auto note = GetNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); wallet.AddToWallet(wtx, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); auto wtx2 = GetValidSpend(sk, note, 5); wallet.AddToWallet(wtx2, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier)); + EXPECT_FALSE(wallet.IsSproutSpent(nullifier)); // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); @@ -513,7 +513,7 @@ TEST(WalletTests, NullifierIsSpent) { wtx2.SetMerkleBranch(block); wallet.AddToWallet(wtx2, true, NULL); - EXPECT_TRUE(wallet.IsSpent(nullifier)); + EXPECT_TRUE(wallet.IsSproutSpent(nullifier)); // Tear down chainActive.SetTip(NULL); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6f682cc05..385e40fe8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -712,8 +712,7 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const * Note is spent if any non-conflicted transaction * spends it: */ -bool CWallet::IsSpent(const uint256& nullifier) const -{ +bool CWallet::IsSproutSpent(const uint256& nullifier) const { pair range; range = mapTxSproutNullifiers.equal_range(nullifier); @@ -724,7 +723,11 @@ bool CWallet::IsSpent(const uint256& nullifier) const return true; // Spent } } + return false; +} +bool CWallet::IsSaplingSpent(const uint256& nullifier) const { + pair range; range = mapTxSaplingNullifiers.equal_range(nullifier); for (TxNullifiers::const_iterator it = range.first; it != range.second; ++it) { @@ -4123,7 +4126,7 @@ void CWallet::GetFilteredNotes( } // skip note which has been spent - if (ignoreSpent && nd.nullifier && IsSpent(*nd.nullifier)) { + if (ignoreSpent && nd.nullifier && IsSproutSpent(*nd.nullifier)) { continue; } @@ -4204,7 +4207,7 @@ void CWallet::GetUnspentFilteredNotes( } // skip note which has been spent - if (nd.nullifier && IsSpent(*nd.nullifier)) { + if (nd.nullifier && IsSproutSpent(*nd.nullifier)) { continue; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 00e0af7d9..a43a53b8e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -936,7 +936,8 @@ public: bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, std::vector vCoins, std::set >& setCoinsRet, CAmount& nValueRet) const; bool IsSpent(const uint256& hash, unsigned int n) const; - bool IsSpent(const uint256& nullifier) const; + bool IsSproutSpent(const uint256& nullifier) const; + bool IsSaplingSpent(const uint256& nullifier) const; bool IsLockedCoin(uint256 hash, unsigned int n) const; void LockCoin(COutPoint& output); From c343e2db9ad18aaaf4725613a88047c78e3f335d Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 23:21:55 -0700 Subject: [PATCH 24/40] Fix code review nits. --- src/wallet/wallet.cpp | 25 ++++++++++++------------- src/wallet/wallet.h | 2 +- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 385e40fe8..0d127ccbd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -474,11 +474,10 @@ void CWallet::ChainTip(const CBlockIndex *pindex, { if (added) { IncrementNoteWitnesses(pindex, pblock, sproutTree, saplingTree); - UpdateSaplingNullifierNoteMapForBlock(pblock); } else { DecrementNoteWitnesses(pindex); - UpdateSaplingNullifierNoteMapForBlock(pblock); } + UpdateSaplingNullifierNoteMapForBlock(pblock); } void CWallet::SetBestChain(const CBlockLocator& loc) @@ -758,6 +757,15 @@ void CWallet::AddToSproutSpends(const uint256& nullifier, const uint256& wtxid) SyncMetaData(range); } +void CWallet::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) +{ + mapTxSaplingNullifiers.insert(make_pair(nullifier, wtxid)); + + pair range; + range = mapTxSaplingNullifiers.equal_range(nullifier); + SyncMetaData(range); +} + void CWallet::AddToSpends(const uint256& wtxid) { assert(mapWallet.count(wtxid)); @@ -778,15 +786,6 @@ void CWallet::AddToSpends(const uint256& wtxid) } } -void CWallet::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) -{ - mapTxSaplingNullifiers.insert(make_pair(nullifier, wtxid)); - - pair range; - range = mapTxSaplingNullifiers.equal_range(nullifier); - SyncMetaData(range); -} - void CWallet::ClearNoteWitnessCache() { LOCK(cs_wallet); @@ -1226,14 +1225,14 @@ void CWallet::UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx) { SaplingOutPoint op = item.first; SaplingNoteData nd = item.second; - if (nd.witnesses.size() == 0) { + if (nd.witnesses.empty()) { // If there are no witnesses, erase the nullifier and associated mapping. if (item.second.nullifier) { mapSaplingNullifiersToNotes.erase(item.second.nullifier.get()); } item.second.nullifier = boost::none; } - else if (nd.witnesses.size() > 0) { + else { uint64_t position = nd.witnesses.front().position(); SaplingFullViewingKey fvk = mapSaplingFullViewingKeys.at(nd.ivk); OutputDescription output = wtx.vShieldedOutput[op.n]; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a43a53b8e..c8aac9181 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -732,8 +732,8 @@ private: void AddToTransparentSpends(const COutPoint& outpoint, const uint256& wtxid); void AddToSproutSpends(const uint256& nullifier, const uint256& wtxid); - void AddToSpends(const uint256& wtxid); void AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid); + void AddToSpends(const uint256& wtxid); public: /* From 2f6481f8353b537a415c813a2518bfcd48477835 Mon Sep 17 00:00:00 2001 From: Simon Date: Thu, 2 Aug 2018 23:56:31 -0700 Subject: [PATCH 25/40] Add two new wallet tests: FindMySaplingNotes, SaplingNullifierIsSpent. --- src/wallet/gtest/test_wallet.cpp | 109 +++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 18bda4a65..498725a17 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -7,6 +7,7 @@ #include "main.h" #include "primitives/block.h" #include "random.h" +#include "transaction_builder.h" #include "utiltest.h" #include "wallet/wallet.h" #include "zcash/JoinSplit.hpp" @@ -394,6 +395,49 @@ TEST(WalletTests, GetSproutNoteNullifier) { EXPECT_EQ(nullifier, ret); } +TEST(WalletTests, FindMySaplingNotes) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate dummy Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto pk = sk.default_address(); + + // Generate dummy Sapling note + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree tree; + tree.append(cm); + auto anchor = tree.root(); + auto witness = tree.witness(); + + // Generate transaction + auto builder = TransactionBuilder(consensusParams, 1); + ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); + builder.AddSaplingOutput(fvk, pk, 25000, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + + // No Sapling notes can be found in tx which belong to the wallet + CWalletTx wtx {&wallet, tx}; + ASSERT_FALSE(wallet.HaveSaplingSpendingKey(fvk)); + auto noteMap = wallet.FindMySaplingNotes(wtx); + EXPECT_EQ(0, noteMap.size()); + + // Add spending key to wallet, so Sapling notes can be found + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + noteMap = wallet.FindMySaplingNotes(wtx); + EXPECT_EQ(2, noteMap.size()); +} + TEST(WalletTests, FindMySproutNotes) { CWallet wallet; @@ -520,6 +564,71 @@ TEST(WalletTests, NullifierIsSpent) { mapBlockIndex.erase(blockHash); } +TEST(WalletTests, SaplingNullifierIsSpent) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate dummy Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto pk = sk.default_address(); + + // Generate dummy Sapling note + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree tree; + tree.append(cm); + auto anchor = tree.root(); + auto witness = tree.witness(); + + // Generate transaction + auto builder = TransactionBuilder(consensusParams, 1); + ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); + builder.AddSaplingOutput(fvk, pk, 25000, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + + CWalletTx wtx {&wallet, tx}; + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + + // Manually compute the nullifier based on the known position + auto nf = note.nullifier(fvk, witness.position()); + ASSERT_TRUE(nf); + uint256 nullifier = nf.get(); + + // Verify nullifier is unused + EXPECT_FALSE(wallet.IsSaplingSpent(nullifier)); + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + CBlock block; + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + EXPECT_TRUE(chainActive.Contains(&fakeIndex)); + EXPECT_EQ(0, chainActive.Height()); + + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); + + // Verify nullifier is unused + EXPECT_TRUE(wallet.IsSaplingSpent(nullifier)); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); +} + TEST(WalletTests, NavigateFromNullifierToNote) { CWallet wallet; From e5df6ec5cc0d0c2b348e9c6a159ec435bdc41e74 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 3 Aug 2018 11:31:59 -0700 Subject: [PATCH 26/40] Add new wallet test: NavigateFromSaplingNullifierToNote Checks caching of Sapling nullifier and mapping to its SaplingOutPoint. --- src/wallet/gtest/test_wallet.cpp | 100 +++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 498725a17..4631571de 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -655,6 +655,106 @@ TEST(WalletTests, NavigateFromNullifierToNote) { EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes[nullifier].n); } +TEST(WalletTests, NavigateFromSaplingNullifierToNote) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate dummy Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto pk = sk.default_address(); + + // Generate dummy Sapling note + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree saplingTree; + saplingTree.append(cm); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); + + // Generate transaction + auto builder = TransactionBuilder(consensusParams, 1); + ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); + builder.AddSaplingOutput(fvk, pk, 25000, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + + CWalletTx wtx {&wallet, tx}; + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + + // Manually compute the nullifier based on the expected position + auto nf = note.nullifier(fvk, witness.position()); + ASSERT_TRUE(nf); + uint256 nullifier = nf.get(); + + // Verify dummy note is unspent + EXPECT_FALSE(wallet.IsSaplingSpent(nullifier)); + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + EXPECT_TRUE(chainActive.Contains(&fakeIndex)); + EXPECT_EQ(0, chainActive.Height()); + + // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe + wtx.SetMerkleBranch(block); + auto saplingNoteData = wallet.FindMySaplingNotes(wtx); + ASSERT_TRUE(saplingNoteData.size() > 0); + wtx.SetSaplingNoteData(saplingNoteData); + wallet.AddToWallet(wtx, true, NULL); + + // Verify dummy note is now spent, as AddToWallet invokes AddToSpends() + EXPECT_TRUE(wallet.IsSaplingSpent(nullifier)); + + // Test invariant: no witnesses means no nullifier. + EXPECT_EQ(0, wallet.mapSaplingNullifiersToNotes.size()); + for (mapSaplingNoteData_t::value_type &item : wtx.mapSaplingNoteData) { + SaplingNoteData nd = item.second; + ASSERT_TRUE(nd.witnesses.empty()); + ASSERT_FALSE(nd.nullifier); + } + + // Simulate receiving new block and ChainTip signal + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + + // Retrieve the updated wtx from wallet + uint256 hash = wtx.GetHash(); + wtx = wallet.mapWallet[hash]; + + // Verify Sapling nullifiers map to SaplingOutPoints + EXPECT_EQ(2, wallet.mapSaplingNullifiersToNotes.size()); + for (mapSaplingNoteData_t::value_type &item : wtx.mapSaplingNoteData) { + SaplingOutPoint op = item.first; + SaplingNoteData nd = item.second; + EXPECT_EQ(hash, op.hash); + EXPECT_EQ(1, nd.witnesses.size()); + ASSERT_TRUE(nd.nullifier); + auto nf = nd.nullifier.get(); + EXPECT_EQ(1, wallet.mapSaplingNullifiersToNotes.count(nf)); + EXPECT_EQ(op.hash, wallet.mapSaplingNullifiersToNotes[nf].hash); + EXPECT_EQ(op.n, wallet.mapSaplingNullifiersToNotes[nf].n); + } + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); +} + TEST(WalletTests, SpentNoteIsFromMe) { CWallet wallet; From fba41680494695afef9f59c0d5deed72965a62a9 Mon Sep 17 00:00:00 2001 From: Simon Date: Fri, 3 Aug 2018 22:10:57 -0700 Subject: [PATCH 27/40] Add new wallet test: UpdatedSaplingNoteData. --- src/wallet/gtest/test_wallet.cpp | 115 +++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 4631571de..36d071560 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -1328,6 +1328,121 @@ TEST(WalletTests, UpdatedNoteData) { // TODO: The new note should get witnessed (but maybe not here) (#1350) } +TEST(WalletTests, UpdatedSaplingNoteData) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate dummy Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto pk = sk.default_address(); + + // Generate dummy recipient Sapling address + auto sk2 = libzcash::SaplingSpendingKey::random(); + auto fvk2 = sk2.full_viewing_key(); + auto pk2 = sk2.default_address(); + + // Generate dummy Sapling note + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree saplingTree; + saplingTree.append(cm); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); + + // Generate transaction + auto builder = TransactionBuilder(consensusParams, 1); + ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); + builder.AddSaplingOutput(fvk, pk2, 25000, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + + // Wallet contains fvk1 but not fvk2 + CWalletTx wtx {&wallet, tx}; + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + ASSERT_FALSE(wallet.HaveSaplingSpendingKey(fvk2)); + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + EXPECT_TRUE(chainActive.Contains(&fakeIndex)); + EXPECT_EQ(0, chainActive.Height()); + + // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe + auto saplingNoteData = wallet.FindMySaplingNotes(wtx); + ASSERT_TRUE(saplingNoteData.size() == 1); // wallet only has key for change output + wtx.SetSaplingNoteData(saplingNoteData); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); + + // Simulate receiving new block and ChainTip signal + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + + // Retrieve the updated wtx from wallet + uint256 hash = wtx.GetHash(); + wtx = wallet.mapWallet[hash]; + + // Now lets add key fvk2 so wallet can find the payment note sent to pk2 + ASSERT_TRUE(wallet.AddSaplingZKey(sk2)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk2)); + CWalletTx wtx2 = wtx; + auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2); + ASSERT_TRUE(saplingNoteData2.size() == 2); + wtx2.SetSaplingNoteData(saplingNoteData2); + + // The payment note has not been witnessed yet, so let's fake the witness. + SaplingOutPoint sop0(wtx2.GetHash(), 0); + SaplingOutPoint sop1(wtx2.GetHash(), 1); + wtx2.mapSaplingNoteData[sop0].witnesses.push_front(saplingTree.witness()); + wtx2.mapSaplingNoteData[sop0].witnessHeight = 0; + + // The txs are different as wtx is aware of just the change output, + // whereas wtx2 is aware of both payment and change outputs. + EXPECT_NE(wtx.mapSaplingNoteData, wtx2.mapSaplingNoteData); + EXPECT_EQ(1, wtx.mapSaplingNoteData.size()); + EXPECT_EQ(1, wtx.mapSaplingNoteData[sop1].witnesses.size()); // wtx has witness for change + + EXPECT_EQ(2, wtx2.mapSaplingNoteData.size()); + EXPECT_EQ(1, wtx2.mapSaplingNoteData[sop0].witnesses.size()); // wtx2 has fake witness for payment output + EXPECT_EQ(0, wtx2.mapSaplingNoteData[sop1].witnesses.size()); // wtx2 never had incrementnotewitness called + + // After updating, they should be the same + EXPECT_TRUE(wallet.UpdatedNoteData(wtx2, wtx)); + + // We can't do this: + // EXPECT_EQ(wtx.mapSaplingNoteData, wtx2.mapSaplingNoteData); + // because nullifiers (if part of == comparator) have not all been computed + // Also note that mapwallet[hash] is not updated with the updated wtx. + // wtx = wallet.mapWallet[hash]; + + EXPECT_EQ(2, wtx.mapSaplingNoteData.size()); + EXPECT_EQ(2, wtx2.mapSaplingNoteData.size()); + // wtx copied over the fake witness from wtx2 for the payment output + EXPECT_EQ(wtx.mapSaplingNoteData[sop0].witnesses.front(), wtx2.mapSaplingNoteData[sop0].witnesses.front()); + // wtx2 never had its change output witnessed even though it has been in wtx + EXPECT_EQ(0, wtx2.mapSaplingNoteData[sop1].witnesses.size()); + EXPECT_EQ(wtx.mapSaplingNoteData[sop1].witnesses.front(), saplingTree.witness()); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); +} + TEST(WalletTests, MarkAffectedTransactionsDirty) { TestWallet wallet; From f13387486e774eb1de16db20ad5746a037cc502b Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 6 Aug 2018 20:11:03 -0700 Subject: [PATCH 28/40] Add new wallet tests: SpentSaplingNoteIsFromMe. --- src/wallet/gtest/test_wallet.cpp | 145 +++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 36d071560..d126be89e 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -783,6 +783,151 @@ TEST(WalletTests, SpentNoteIsFromMe) { EXPECT_TRUE(wallet.IsFromMe(wtx2)); } +// Create note A, spend A to create note B, spend and verify note B is from me. +TEST(WalletTests, SpentSaplingNoteIsFromMe) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + auto pk = sk.default_address(); + + // Generate Sapling note A + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree saplingTree; + saplingTree.append(cm); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); + + // 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, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + + CWalletTx wtx {&wallet, tx}; + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + EXPECT_TRUE(chainActive.Contains(&fakeIndex)); + EXPECT_EQ(0, chainActive.Height()); + + auto saplingNoteData = wallet.FindMySaplingNotes(wtx); + ASSERT_TRUE(saplingNoteData.size() > 0); + wtx.SetSaplingNoteData(saplingNoteData); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); + + // Simulate receiving new block and ChainTip signal. + // This triggers calculation of nullifiers for notes belonging to this wallet + // in the output descriptions of wtx. + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + + // Retrieve the updated wtx from wallet + wtx = wallet.mapWallet[wtx.GetHash()]; + + // The test wallet never received the fake note which is being spent, so there + // is no mapping from nullifier to notedata stored in mapSaplingNullifiersToNotes. + // Therefore the wallet does not know the tx belongs to the wallet. + EXPECT_FALSE(wallet.IsFromMe(wtx)); + + // Manually compute the nullifier and check map entry does not exist + auto nf = note.nullifier(fvk, witness.position()); + ASSERT_TRUE(nf); + ASSERT_FALSE(wallet.mapSaplingNullifiersToNotes.count(nf.get())); + + // Decrypt note B + auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + wtx.vShieldedOutput[0].encCiphertext, + ivk, + wtx.vShieldedOutput[0].ephemeralKey, + wtx.vShieldedOutput[0].cm); + ASSERT_EQ(static_cast(maybe_pt), true); + auto maybe_note = maybe_pt.get().note(ivk); + ASSERT_EQ(static_cast(maybe_note), true); + auto note2 = maybe_note.get(); + + // Get witness to retrieve position of note B we want to spend + SaplingOutPoint sop0(wtx.GetHash(), 0); + auto spend_note_witness = wtx.mapSaplingNoteData[sop0].witnesses.front(); + auto maybe_nf = note2.nullifier(fvk, spend_note_witness.position()); + ASSERT_EQ(static_cast(maybe_nf), true); + auto nullifier2 = maybe_nf.get(); + + // NOTE: Not updating the anchor results in a core dump. Shouldn't builder just return error? + // *** Error in `./zcash-gtest': double free or corruption (out): 0x00007ffd8755d990 *** + anchor = saplingTree.root(); + + // 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, {}); + auto maybe_tx2 = builder2.Build(); + ASSERT_EQ(static_cast(maybe_tx2), true); + auto tx2 = maybe_tx2.get(); + EXPECT_EQ(tx2.vin.size(), 0); + EXPECT_EQ(tx2.vout.size(), 0); + EXPECT_EQ(tx2.vjoinsplit.size(), 0); + EXPECT_EQ(tx2.vShieldedSpend.size(), 1); + EXPECT_EQ(tx2.vShieldedOutput.size(), 2); + EXPECT_EQ(tx2.valueBalance, 10000); + + CWalletTx wtx2 {&wallet, tx2}; + + // Fake-mine this tx into the next block + EXPECT_EQ(0, chainActive.Height()); + CBlock block2; + block2.vtx.push_back(wtx2); + block2.hashMerkleRoot = block2.BuildMerkleTree(); + block2.hashPrevBlock = blockHash; + auto blockHash2 = block2.GetHash(); + CBlockIndex fakeIndex2 {block2}; + mapBlockIndex.insert(std::make_pair(blockHash2, &fakeIndex2)); + fakeIndex2.nHeight = 1; + chainActive.SetTip(&fakeIndex2); + EXPECT_TRUE(chainActive.Contains(&fakeIndex2)); + EXPECT_EQ(1, chainActive.Height()); + + auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2); + ASSERT_TRUE(saplingNoteData2.size() > 0); + wtx2.SetSaplingNoteData(saplingNoteData2); + wtx2.SetMerkleBranch(block2); + wallet.AddToWallet(wtx2, true, NULL); + + // Verify note B is spent. AddToWallet invokes AddToSpends which updates mapTxSaplingNullifiers + EXPECT_TRUE(wallet.IsSaplingSpent(nullifier2)); + + // Verify note B belongs to wallet. + EXPECT_TRUE(wallet.IsFromMe(wtx2)); + ASSERT_TRUE(wallet.mapSaplingNullifiersToNotes.count(nullifier2)); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); + mapBlockIndex.erase(blockHash2); +} + TEST(WalletTests, CachedWitnessesEmptyChain) { TestWallet wallet; From 992a82c64981e7aed07869039b6a77015b9b6df4 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 09:54:49 -0700 Subject: [PATCH 29/40] Rename wallet tests for clarity between Sprout and Sapling. --- src/wallet/gtest/test_wallet.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index d126be89e..caee4c316 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -139,7 +139,7 @@ TEST(WalletTests, SetupDatadirLocationRunAsFirstTest) { mapArgs["-datadir"] = pathTemp.string(); } -TEST(WalletTests, NoteDataSerialisation) { +TEST(WalletTests, SproutNoteDataSerialisation) { auto sk = libzcash::SproutSpendingKey::random(); auto wtx = GetValidReceive(sk, 10, true); auto note = GetNote(sk, wtx, 0, 1); @@ -163,7 +163,7 @@ TEST(WalletTests, NoteDataSerialisation) { } -TEST(WalletTests, FindUnspentNotes) { +TEST(WalletTests, FindUnspentSproutNotes) { SelectParams(CBaseChainParams::TESTNET); CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -322,7 +322,7 @@ TEST(WalletTests, FindUnspentNotes) { } -TEST(WalletTests, SetNoteAddrsInCWalletTx) { +TEST(WalletTests, SetSproutNoteAddrsInCWalletTx) { auto sk = libzcash::SproutSpendingKey::random(); auto wtx = GetValidReceive(sk, 10, true); auto note = GetNote(sk, wtx, 0, 1); @@ -338,7 +338,7 @@ TEST(WalletTests, SetNoteAddrsInCWalletTx) { EXPECT_EQ(noteData, wtx.mapSproutNoteData); } -TEST(WalletTests, SetInvalidNoteAddrsInCWalletTx) { +TEST(WalletTests, SetSproutInvalidNoteAddrsInCWalletTx) { CWalletTx wtx; EXPECT_EQ(0, wtx.mapSproutNoteData.size()); @@ -493,7 +493,7 @@ TEST(WalletTests, FindMySproutNotesInEncryptedWallet) { EXPECT_EQ(nd, noteMap[jsoutpt]); } -TEST(WalletTests, GetConflictedNotes) { +TEST(WalletTests, GetConflictedSproutNotes) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -524,7 +524,7 @@ TEST(WalletTests, GetConflictedNotes) { EXPECT_EQ(std::set({hash2, hash3}), c3); } -TEST(WalletTests, NullifierIsSpent) { +TEST(WalletTests, SproutNullifierIsSpent) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -629,7 +629,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { mapBlockIndex.erase(blockHash); } -TEST(WalletTests, NavigateFromNullifierToNote) { +TEST(WalletTests, NavigateFromSproutNullifierToNote) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -755,7 +755,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { mapBlockIndex.erase(blockHash); } -TEST(WalletTests, SpentNoteIsFromMe) { +TEST(WalletTests, SpentSproutNoteIsFromMe) { CWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -1391,7 +1391,7 @@ TEST(WalletTests, WriteWitnessCache) { wallet.SetBestChain(walletdb, loc); } -TEST(WalletTests, UpdateNullifierNoteMap) { +TEST(WalletTests, UpdateSproutNullifierNoteMap) { TestWallet wallet; uint256 r {GetRandHash()}; CKeyingMaterial vMasterKey (r.begin(), r.end()); @@ -1426,7 +1426,7 @@ TEST(WalletTests, UpdateNullifierNoteMap) { EXPECT_EQ(1, wallet.mapSproutNullifiersToNotes[nullifier].n); } -TEST(WalletTests, UpdatedNoteData) { +TEST(WalletTests, UpdatedSproutNoteData) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -1619,7 +1619,7 @@ TEST(WalletTests, MarkAffectedTransactionsDirty) { EXPECT_FALSE(wallet.mapWallet[hash].fDebitCached); } -TEST(WalletTests, NoteLocking) { +TEST(WalletTests, SproutNoteLocking) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); From 668ca2974a849821d7ae6f16a1f7414e6af53985 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 09:57:24 -0700 Subject: [PATCH 30/40] Fix typo in variable name in test. --- src/wallet/gtest/test_wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index caee4c316..4e4def5e3 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -1304,7 +1304,7 @@ TEST(WalletTests, ClearNoteWitnessCache) { // After clearing, we should not have a witness for either note wallet.ClearNoteWitnessCache(); - auto anchros2 = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); + auto anchors2 = GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, sproutWitnesses, saplingWitnesses); EXPECT_FALSE((bool) sproutWitnesses[0]); EXPECT_FALSE((bool) sproutWitnesses[1]); EXPECT_FALSE((bool) saplingWitnesses[0]); From 9fe34549b1920f03fd915c2adf8c5a57365b002e Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 09:59:23 -0700 Subject: [PATCH 31/40] Fix inaccurate comments in test. --- src/wallet/gtest/test_wallet.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 4e4def5e3..6a2bfcdf2 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -603,7 +603,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { ASSERT_TRUE(nf); uint256 nullifier = nf.get(); - // Verify nullifier is unused + // Verify note has not been spent EXPECT_FALSE(wallet.IsSaplingSpent(nullifier)); // Fake-mine the transaction @@ -621,7 +621,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { wtx.SetMerkleBranch(block); wallet.AddToWallet(wtx, true, NULL); - // Verify nullifier is unused + // Verify note has been spent EXPECT_TRUE(wallet.IsSaplingSpent(nullifier)); // Tear down From 6d8ea4fa826669aeff2d5704ebed1237f5b7d5d8 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 10:18:21 -0700 Subject: [PATCH 32/40] Fix typo in parameter name. --- src/wallet/wallet.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c8aac9181..b1655eebd 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1053,7 +1053,7 @@ public: void MarkDirty(); bool UpdateNullifierNoteMap(); void UpdateNullifierNoteMapWithTx(const CWalletTx& wtx); - void UpdateSaplingNullifierNoteMapWithTx(CWalletTx& tx); + void UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx); void UpdateSaplingNullifierNoteMapForBlock(const CBlock* pblock); bool AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletDB* pwalletdb); void SyncTransaction(const CTransaction& tx, const CBlock* pblock); From 3afc6ce21909ef892182d2720b003ce8e3a43f9e Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 10:55:40 -0700 Subject: [PATCH 33/40] Update CWallet::GetConflicts for Sapling. --- src/wallet/wallet.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0d127ccbd..7d0b7c9a3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -593,6 +593,19 @@ set CWallet::GetConflicts(const uint256& txid) const } } } + + std::pair range_o; + + for (const SpendDescription &spend : wtx.vShieldedSpend) { + uint256 nullifier = spend.nullifier; + if (mapTxSaplingNullifiers.count(nullifier) <= 1) { + continue; // No conflict if zero or one spends + } + range_o = mapTxSaplingNullifiers.equal_range(nullifier); + for (TxNullifiers::const_iterator it = range_o.first; it != range_o.second; ++it) { + result.insert(it->second); + } + } return result; } From a5ca7967a4d3d3addd347aaef9324a9f20bfaef2 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 15:38:23 -0700 Subject: [PATCH 34/40] Add new wallet test: SetSaplingNoteAddrsInCWalletTx. --- src/wallet/gtest/test_wallet.cpp | 59 ++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 6a2bfcdf2..4523015e2 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -338,6 +338,62 @@ TEST(WalletTests, SetSproutNoteAddrsInCWalletTx) { EXPECT_EQ(noteData, wtx.mapSproutNoteData); } +TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto pk = sk.default_address(); + auto ivk = fvk.in_viewing_key(); + + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree tree; + tree.append(cm); + auto anchor = tree.root(); + auto witness = tree.witness(); + + auto nf = note.nullifier(fvk, witness.position()); + ASSERT_TRUE(nf); + uint256 nullifier = nf.get(); + + auto builder = TransactionBuilder(consensusParams, 1); + ASSERT_TRUE(builder.AddSaplingSpend(expsk, note, anchor, witness)); + builder.AddSaplingOutput(fvk, pk, 50000, {}); + builder.SetFee(0); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + + CWalletTx wtx {&wallet, tx}; + + EXPECT_EQ(0, wtx.mapSaplingNoteData.size()); + mapSaplingNoteData_t noteData; + + SaplingOutPoint op {wtx.GetHash(), 0}; + SaplingNoteData nd; + nd.nullifier = nullifier; + nd.ivk = ivk; + nd.witnesses.push_front(witness); + nd.witnessHeight = 123; + noteData.insert(std::make_pair(op, nd)); + + wtx.SetSaplingNoteData(noteData); + EXPECT_EQ(noteData, wtx.mapSaplingNoteData); + + // Test individual fields in case equality operator is defined/changed. + EXPECT_EQ(ivk, wtx.mapSaplingNoteData[op].ivk); + EXPECT_EQ(nullifier, wtx.mapSaplingNoteData[op].nullifier); + EXPECT_EQ(nd.witnessHeight, wtx.mapSaplingNoteData[op].witnessHeight); + EXPECT_TRUE(witness == wtx.mapSaplingNoteData[op].witnesses.front()); +} + TEST(WalletTests, SetSproutInvalidNoteAddrsInCWalletTx) { CWalletTx wtx; EXPECT_EQ(0, wtx.mapSproutNoteData.size()); @@ -351,6 +407,9 @@ TEST(WalletTests, SetSproutInvalidNoteAddrsInCWalletTx) { EXPECT_THROW(wtx.SetSproutNoteData(noteData), std::logic_error); } +// The following test is the same as SetInvalidSaplingNoteDataInCWalletTx +// TEST(WalletTests, SetSaplingInvalidNoteAddrsInCWalletTx) + // Cannot add note data for an index which does not exist in tx.vShieldedOutput TEST(WalletTests, SetInvalidSaplingNoteDataInCWalletTx) { CWalletTx wtx; From eba096f24ec57a88f1c11684570ffdce48c1b6b3 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 19:07:46 -0700 Subject: [PATCH 35/40] Add new wallet test: GetConflictedSaplingNotes --- src/wallet/gtest/test_wallet.cpp | 124 +++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 4523015e2..02341569d 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -583,6 +583,130 @@ TEST(WalletTests, GetConflictedSproutNotes) { EXPECT_EQ(std::set({hash2, hash3}), c3); } +// Generate note A and spend to create note B, from which we spend to create two conflicting transactions +TEST(WalletTests, GetConflictedSaplingNotes) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + auto pk = sk.default_address(); + + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + + // Generate note A + libzcash::SaplingNote note(pk, 50000); + auto cm = note.cm().value(); + SaplingMerkleTree saplingTree; + saplingTree.append(cm); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); + + // 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, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx = maybe_tx.get(); + CWalletTx wtx {&wallet, tx}; + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + EXPECT_TRUE(chainActive.Contains(&fakeIndex)); + EXPECT_EQ(0, chainActive.Height()); + + // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe + auto saplingNoteData = wallet.FindMySaplingNotes(wtx); + ASSERT_TRUE(saplingNoteData.size() > 0); + wtx.SetSaplingNoteData(saplingNoteData); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); + + // Simulate receiving new block and ChainTip signal + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + + // Retrieve the updated wtx from wallet + uint256 hash = wtx.GetHash(); + wtx = wallet.mapWallet[hash]; + + // Decrypt output note B + auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + wtx.vShieldedOutput[0].encCiphertext, + ivk, + wtx.vShieldedOutput[0].ephemeralKey, + wtx.vShieldedOutput[0].cm); + ASSERT_EQ(static_cast(maybe_pt), true); + auto maybe_note = maybe_pt.get().note(ivk); + ASSERT_EQ(static_cast(maybe_note), true); + auto note2 = maybe_note.get(); + + SaplingOutPoint sop0(wtx.GetHash(), 0); + auto spend_note_witness = wtx.mapSaplingNoteData[sop0].witnesses.front(); + auto maybe_nf = note2.nullifier(fvk, spend_note_witness.position()); + ASSERT_EQ(static_cast(maybe_nf), true); + auto nullifier2 = maybe_nf.get(); + + anchor = saplingTree.root(); + + // 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, {}); + auto maybe_tx2 = builder2.Build(); + ASSERT_EQ(static_cast(maybe_tx2), true); + auto tx2 = maybe_tx2.get(); + + // 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, {}); + auto maybe_tx3 = builder3.Build(); + ASSERT_EQ(static_cast(maybe_tx3), true); + auto tx3 = maybe_tx3.get(); + + CWalletTx wtx2 {&wallet, tx2}; + CWalletTx wtx3 {&wallet, tx3}; + + auto hash2 = wtx2.GetHash(); + auto hash3 = wtx3.GetHash(); + + // No conflicts for no spends (wtx is currently the only transaction in the wallet) + EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); + EXPECT_EQ(0, wallet.GetConflicts(hash3).size()); + + // No conflicts for one spend + wallet.AddToWallet(wtx2, true, NULL); + EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); + + // Conflicts for two spends + wallet.AddToWallet(wtx3, true, NULL); + auto c3 = wallet.GetConflicts(hash2); + EXPECT_EQ(2, c3.size()); + EXPECT_EQ(std::set({hash2, hash3}), c3); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); +} + TEST(WalletTests, SproutNullifierIsSpent) { CWallet wallet; From 52d162319d72d2e67e7730196f43eea4981febc3 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 7 Aug 2018 21:06:03 -0700 Subject: [PATCH 36/40] Add new wallet test: MarkAffectedSaplingTransactionsDirty Also rename MarkAffectedTransactionsDirty to MarkAffectedSproutTransactionsDirty. --- src/wallet/gtest/test_wallet.cpp | 121 ++++++++++++++++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 02341569d..6389282c5 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -4,6 +4,7 @@ #include "base58.h" #include "chainparams.h" +#include "key_io.h" #include "main.h" #include "primitives/block.h" #include "random.h" @@ -24,6 +25,8 @@ ACTION(ThrowLogicError) { throw std::logic_error("Boom"); } +static const std::string tSecretRegtest = "cND2ZvtabDbJ1gucx9GWH6XT9kgTAqfb6cotPt5Q5CyxVDhid2EN"; + class MockWalletDB { public: MOCK_METHOD0(TxnBegin, bool()); @@ -1771,7 +1774,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { mapBlockIndex.erase(blockHash); } -TEST(WalletTests, MarkAffectedTransactionsDirty) { +TEST(WalletTests, MarkAffectedSproutTransactionsDirty) { TestWallet wallet; auto sk = libzcash::SproutSpendingKey::random(); @@ -1802,6 +1805,122 @@ TEST(WalletTests, MarkAffectedTransactionsDirty) { EXPECT_FALSE(wallet.mapWallet[hash].fDebitCached); } +TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { + SelectParams(CBaseChainParams::REGTEST); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto consensusParams = Params().GetConsensus(); + + TestWallet wallet; + + // Generate Sapling address + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto fvk = sk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + auto pk = sk.default_address(); + + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); + + // Set up transparent address + CBasicKeyStore keystore; + CKey tsk = DecodeSecret(tSecretRegtest); + keystore.AddKey(tsk); + auto scriptPubKey = GetScriptForDestination(tsk.GetPubKey().GetID()); + + // Generate shielding tx from transparent to Sapling + // 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, {}); + auto maybe_tx = builder.Build(); + ASSERT_EQ(static_cast(maybe_tx), true); + auto tx1 = maybe_tx.get(); + + EXPECT_EQ(tx1.vin.size(), 1); + EXPECT_EQ(tx1.vout.size(), 0); + EXPECT_EQ(tx1.vjoinsplit.size(), 0); + EXPECT_EQ(tx1.vShieldedSpend.size(), 0); + EXPECT_EQ(tx1.vShieldedOutput.size(), 1); + EXPECT_EQ(tx1.valueBalance, -40000); + + CWalletTx wtx {&wallet, tx1}; + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SaplingMerkleTree saplingTree; + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + block.hashMerkleRoot = block.BuildMerkleTree(); + auto blockHash = block.GetHash(); + CBlockIndex fakeIndex {block}; + mapBlockIndex.insert(std::make_pair(blockHash, &fakeIndex)); + chainActive.SetTip(&fakeIndex); + EXPECT_TRUE(chainActive.Contains(&fakeIndex)); + EXPECT_EQ(0, chainActive.Height()); + + // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe + auto saplingNoteData = wallet.FindMySaplingNotes(wtx); + ASSERT_TRUE(saplingNoteData.size() > 0); + wtx.SetSaplingNoteData(saplingNoteData); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); + + // Simulate receiving new block and ChainTip signal + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + + // Retrieve the updated wtx from wallet + uint256 hash = wtx.GetHash(); + wtx = wallet.mapWallet[hash]; + + // Prepare to spend the note that was just created + auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + tx1.vShieldedOutput[0].encCiphertext, ivk, tx1.vShieldedOutput[0].ephemeralKey, tx1.vShieldedOutput[0].cm); + ASSERT_EQ(static_cast(maybe_pt), true); + auto maybe_note = maybe_pt.get().note(ivk); + ASSERT_EQ(static_cast(maybe_note), true); + auto note = maybe_note.get(); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); + + // Create a Sapling-only transaction + // 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, {}); + auto maybe_tx2 = builder2.Build(); + ASSERT_EQ(static_cast(maybe_tx2), true); + auto tx2 = maybe_tx2.get(); + + EXPECT_EQ(tx2.vin.size(), 0); + EXPECT_EQ(tx2.vout.size(), 0); + EXPECT_EQ(tx2.vjoinsplit.size(), 0); + EXPECT_EQ(tx2.vShieldedSpend.size(), 1); + EXPECT_EQ(tx2.vShieldedOutput.size(), 2); + EXPECT_EQ(tx2.valueBalance, 10000); + + CWalletTx wtx2 {&wallet, tx2}; + auto hash2 = wtx2.GetHash(); + + wallet.MarkAffectedTransactionsDirty(wtx); + + // After getting a cached value, the first tx should be clean + wallet.mapWallet[hash].GetDebit(ISMINE_ALL); + EXPECT_TRUE(wallet.mapWallet[hash].fDebitCached); + + // After adding the note spend, the first tx should be dirty + wallet.AddToWallet(wtx2, true, NULL); + wallet.MarkAffectedTransactionsDirty(wtx2); + EXPECT_FALSE(wallet.mapWallet[hash].fDebitCached); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); +} + TEST(WalletTests, SproutNoteLocking) { TestWallet wallet; From 58a1224d6312d66a64388ed9abb4e183b3a09b95 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 8 Aug 2018 10:55:35 -0700 Subject: [PATCH 37/40] Update wallet unit tests to revert upgraded network parameters. --- src/wallet/gtest/test_wallet.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 6389282c5..350764cbf 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -395,6 +395,10 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { EXPECT_EQ(nullifier, wtx.mapSaplingNoteData[op].nullifier); EXPECT_EQ(nd.witnessHeight, wtx.mapSaplingNoteData[op].witnessHeight); EXPECT_TRUE(witness == wtx.mapSaplingNoteData[op].witnesses.front()); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, SetSproutInvalidNoteAddrsInCWalletTx) { @@ -498,6 +502,10 @@ TEST(WalletTests, FindMySaplingNotes) { ASSERT_TRUE(wallet.HaveSaplingSpendingKey(fvk)); noteMap = wallet.FindMySaplingNotes(wtx); EXPECT_EQ(2, noteMap.size()); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, FindMySproutNotes) { @@ -708,6 +716,10 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, SproutNullifierIsSpent) { @@ -813,6 +825,10 @@ TEST(WalletTests, SaplingNullifierIsSpent) { // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, NavigateFromSproutNullifierToNote) { @@ -939,6 +955,10 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, SpentSproutNoteIsFromMe) { @@ -1112,6 +1132,10 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); mapBlockIndex.erase(blockHash2); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, CachedWitnessesEmptyChain) { @@ -1772,6 +1796,10 @@ TEST(WalletTests, UpdatedSaplingNoteData) { // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, MarkAffectedSproutTransactionsDirty) { @@ -1919,6 +1947,10 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); + + // Revert to default + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } TEST(WalletTests, SproutNoteLocking) { From 3c3d1f0a3835a2035d221a37d31df0defc654fd5 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 8 Aug 2018 11:04:35 -0700 Subject: [PATCH 38/40] Clean up wallet unit tests: replace .value() with .get() for clarity. This is to avoid confusion with note.value(). --- src/wallet/gtest/test_wallet.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 350764cbf..c89cfc921 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -356,7 +356,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto ivk = fvk.in_viewing_key(); libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree tree; tree.append(cm); auto anchor = tree.root(); @@ -477,7 +477,7 @@ TEST(WalletTests, FindMySaplingNotes) { // Generate dummy Sapling note libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree tree; tree.append(cm); auto anchor = tree.root(); @@ -615,7 +615,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Generate note A libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); auto anchor = saplingTree.root(); @@ -778,7 +778,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { // Generate dummy Sapling note libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree tree; tree.append(cm); auto anchor = tree.root(); @@ -873,7 +873,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { // Generate dummy Sapling note libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); auto anchor = saplingTree.root(); @@ -1007,7 +1007,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Generate Sapling note A libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); auto anchor = saplingTree.root(); @@ -1704,7 +1704,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { // Generate dummy Sapling note libzcash::SaplingNote note(pk, 50000); - auto cm = note.cm().value(); + auto cm = note.cm().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); auto anchor = saplingTree.root(); From 303f80fb1e112384933a6403818ead8f30d11dd9 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 8 Aug 2018 11:33:18 -0700 Subject: [PATCH 39/40] Fix comment in CWallet::SyncMetaData. --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7d0b7c9a3..90d6a72e0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -687,7 +687,7 @@ void CWallet::SyncMetaData(pair::iterator, typename TxSpe CWalletTx* copyTo = &mapWallet[hash]; if (copyFrom == copyTo) continue; copyTo->mapValue = copyFrom->mapValue; - // mapSproutNoteData not copied on purpose + // mapSproutNoteData and mapSaplingNoteData not copied on purpose // (it is always set correctly for each CWalletTx) copyTo->vOrderForm = copyFrom->vOrderForm; // fTimeReceivedIsTxTime not copied on purpose From c9339bb61fd98c5e454d34f81afa384bc87918f6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 17 Aug 2018 21:39:16 +0100 Subject: [PATCH 40/40] test: Fix comment in WalletTests.FindMySaplingNotes --- src/wallet/gtest/test_wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index c89cfc921..d79b69ea0 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -491,7 +491,7 @@ TEST(WalletTests, FindMySaplingNotes) { ASSERT_EQ(static_cast(maybe_tx), true); auto tx = maybe_tx.get(); - // No Sapling notes can be found in tx which belong to the wallet + // No Sapling notes can be found in tx which does not belong to the wallet CWalletTx wtx {&wallet, tx}; ASSERT_FALSE(wallet.HaveSaplingSpendingKey(fvk)); auto noteMap = wallet.FindMySaplingNotes(wtx);