From dec49d1f8226d02445912ce255d3d66dbdab7caa Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 27 Sep 2016 11:14:49 -0700 Subject: [PATCH 1/2] Fix GetFilteredNotes to use int for minDepth like upstream and avoid casting problems. Don't use FindMyNotes as mapNoteData has already been set on wallet tx. --- src/wallet/rpcwallet.cpp | 4 ++-- src/wallet/wallet.cpp | 8 +++----- src/wallet/wallet.h | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 0fc34966..ba04bdfe 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2833,7 +2833,7 @@ Value z_listaddresses(const Array& params, bool fHelp) return ret; } -CAmount getBalanceTaddr(std::string transparentAddress, size_t minDepth=1) { +CAmount getBalanceTaddr(std::string transparentAddress, int minDepth=1) { set setAddress; vector vecOutputs; CAmount balance = 0; @@ -2872,7 +2872,7 @@ CAmount getBalanceTaddr(std::string transparentAddress, size_t minDepth=1) { return balance; } -CAmount getBalanceZaddr(std::string address, size_t minDepth = 1) { +CAmount getBalanceZaddr(std::string address, int minDepth = 1) { CAmount balance = 0; std::vector entries; LOCK2(cs_main, pwalletMain->cs_wallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2e1449fc..e0b2e52a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3187,7 +3187,7 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee) return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, fRejectAbsurdFee); } -bool CWallet::GetFilteredNotes(std::vector & outEntries, std::string address, size_t minDepth = 1, bool ignoreSpent) +bool CWallet::GetFilteredNotes(std::vector & outEntries, std::string address, int minDepth, bool ignoreSpent) { bool fFilterAddress = false; libzcash::PaymentAddress filterPaymentAddress; @@ -3206,13 +3206,11 @@ bool CWallet::GetFilteredNotes(std::vector & outEntries, st continue; } - mapNoteData_t mapNoteData = FindMyNotes(wtx); - - if (mapNoteData.size() == 0) { + if (wtx.mapNoteData.size() == 0) { continue; } - for (auto & pair : mapNoteData) { + for (auto & pair : wtx.mapNoteData) { JSOutPoint jsop = pair.first; CNoteData nd = pair.second; PaymentAddress pa = nd.address; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1bfb7579..b130705e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -905,7 +905,7 @@ public: void SetBroadcastTransactions(bool broadcast) { fBroadcastTransactions = broadcast; } /* Find notes filtered by payment address, min depth, ability to spend */ - bool GetFilteredNotes(std::vector & outEntries, std::string address, size_t minDepth, bool ignoreSpent=true); + bool GetFilteredNotes(std::vector & outEntries, std::string address, int minDepth=1, bool ignoreSpent=true); }; From eaccc007c91ab8336905355a6fe47fb40f532c66 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 27 Sep 2016 22:43:13 -0700 Subject: [PATCH 2/2] Update test to filter and find notes. --- src/wallet/gtest/test_wallet.cpp | 160 +++++++++++++++++++++++-------- 1 file changed, 121 insertions(+), 39 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 6ebaef97..f0bab297 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -175,12 +175,10 @@ TEST(wallet_tests, note_data_serialisation) { EXPECT_EQ(noteData[jsoutpt].witnesses, noteData2[jsoutpt].witnesses); } + TEST(wallet_tests, find_unspent_notes) { - SelectParams(CBaseChainParams::TESTNET); - CWallet wallet; - auto sk = libzcash::SpendingKey::random(); wallet.AddSpendingKey(sk); @@ -188,44 +186,28 @@ TEST(wallet_tests, find_unspent_notes) { auto note = GetNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); - auto noteMap = wallet.FindMyNotes(wtx); - EXPECT_EQ(2, noteMap.size()); - + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetHash(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + noteData[jsoutpt] = nd; + + wtx.SetNoteData(noteData); wallet.AddToWallet(wtx, true, NULL); EXPECT_FALSE(wallet.IsSpent(nullifier)); - auto wtx2 = GetValidSpend(sk, note, 5); - wallet.AddToWallet(wtx2, true, NULL); - - // two notes, one is spent but wallet doesn't see it as spent yet until mined + // We currently have an unspent and unconfirmed note in the wallet (depth of -1) std::vector entries; wallet.GetFilteredNotes(entries, "", 0); - EXPECT_EQ(2, entries.size()); - - // Create new payment address, add new note, and filter - auto user2_sk = libzcash::SpendingKey::random(); - wallet.AddSpendingKey(user2_sk); - auto user2_pa = user2_sk.address(); - auto user2_payment_address = CZCPaymentAddress(user2_pa).ToString(); - auto wtx3 = GetValidReceive(user2_sk, 15, true); // adds 2 more notes - auto note2 = GetNote(user2_sk, wtx3, 0, 1); - auto nullifier2 = note2.nullifier(user2_sk); - wallet.AddToWallet(wtx3, true, NULL); - EXPECT_FALSE(wallet.IsSpent(nullifier2)); + EXPECT_EQ(0, entries.size()); + entries.clear(); + wallet.GetFilteredNotes(entries, "", -1); + EXPECT_EQ(1, entries.size()); + entries.clear(); - // 4 notes in wallet, 1 spent (not seen), 1 is the new payment address - entries.clear(); - wallet.GetFilteredNotes(entries, "", 0); - EXPECT_EQ(4, entries.size()); - entries.clear(); - wallet.GetFilteredNotes(entries, user2_payment_address, 0); - EXPECT_EQ(2, entries.size()); - // Fake-mine the transaction EXPECT_EQ(-1, chainActive.Height()); - CBlock block; - block.vtx.push_back(wtx2); + block.vtx.push_back(wtx); block.hashMerkleRoot = block.BuildMerkleTree(); auto blockHash = block.GetHash(); CBlockIndex fakeIndex {block}; @@ -234,22 +216,122 @@ TEST(wallet_tests, find_unspent_notes) { EXPECT_TRUE(chainActive.Contains(&fakeIndex)); EXPECT_EQ(0, chainActive.Height()); - wtx2.SetMerkleBranch(block); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); + EXPECT_FALSE(wallet.IsSpent(nullifier)); + + + // We now have an unspent and confirmed note in the wallet (depth of 1) + wallet.GetFilteredNotes(entries, "", 0); + EXPECT_EQ(1, entries.size()); + entries.clear(); + wallet.GetFilteredNotes(entries, "", 1); + EXPECT_EQ(1, entries.size()); + entries.clear(); + wallet.GetFilteredNotes(entries, "", 2); + EXPECT_EQ(0, entries.size()); + entries.clear(); + + + // Let's spend the note. + auto wtx2 = GetValidSpend(sk, note, 5); + wallet.AddToWallet(wtx2, true, NULL); + EXPECT_FALSE(wallet.IsSpent(nullifier)); + + // Fake-mine a spend transaction + 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()); + + wtx2.SetMerkleBranch(block2); wallet.AddToWallet(wtx2, true, NULL); EXPECT_TRUE(wallet.IsSpent(nullifier)); - // 4 notes, 1 spent (now seen) - entries.clear(); + // The note has been spent. By default, GetFilteredNotes() ignores spent notes. wallet.GetFilteredNotes(entries, "", 0); - EXPECT_EQ(3, entries.size()); + EXPECT_EQ(0, entries.size()); entries.clear(); - // no change to user2 and their two notes. - wallet.GetFilteredNotes(entries, user2_payment_address, 0); + // Let's include spent notes to retrieve it. + wallet.GetFilteredNotes(entries, "", 0, false); + EXPECT_EQ(1, entries.size()); + entries.clear(); + // The spent note has two confirmations. + wallet.GetFilteredNotes(entries, "", 2, false); + EXPECT_EQ(1, entries.size()); + entries.clear(); + // It does not have 3 confirmations. + wallet.GetFilteredNotes(entries, "", 3, false); + EXPECT_EQ(0, entries.size()); + entries.clear(); + + + // Let's receive a new note + CWalletTx wtx3; + { + auto wtx = GetValidReceive(sk, 20, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetHash(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + noteData[jsoutpt] = nd; + + wtx.SetNoteData(noteData); + wallet.AddToWallet(wtx, true, NULL); + EXPECT_FALSE(wallet.IsSpent(nullifier)); + + wtx3 = wtx; + } + + // Fake-mine the new transaction + EXPECT_EQ(1, chainActive.Height()); + CBlock block3; + block3.vtx.push_back(wtx3); + block3.hashMerkleRoot = block3.BuildMerkleTree(); + block3.hashPrevBlock = blockHash2; + auto blockHash3 = block3.GetHash(); + CBlockIndex fakeIndex3 {block3}; + mapBlockIndex.insert(std::make_pair(blockHash3, &fakeIndex3)); + fakeIndex3.nHeight = 2; + chainActive.SetTip(&fakeIndex3); + EXPECT_TRUE(chainActive.Contains(&fakeIndex3)); + EXPECT_EQ(2, chainActive.Height()); + + wtx3.SetMerkleBranch(block3); + wallet.AddToWallet(wtx3, true, NULL); + + // We now have an unspent note which has one confirmation, in addition to our spent note. + wallet.GetFilteredNotes(entries, "", 1); + EXPECT_EQ(1, entries.size()); + entries.clear(); + // Let's return the spent note too. + wallet.GetFilteredNotes(entries, "", 1, false); EXPECT_EQ(2, entries.size()); - + entries.clear(); + // Increasing number of confirmations will exclude our new unspent note. + wallet.GetFilteredNotes(entries, "", 2, false); + EXPECT_EQ(1, entries.size()); + entries.clear(); + // If we also ignore spent notes at thie depth, we won't find any notes. + wallet.GetFilteredNotes(entries, "", 2, true); + EXPECT_EQ(0, entries.size()); + entries.clear(); + // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); + mapBlockIndex.erase(blockHash2); + mapBlockIndex.erase(blockHash3); }