diff --git a/doc/payment-api.md b/doc/payment-api.md index ccbb5725c..3560add64 100644 --- a/doc/payment-api.md +++ b/doc/payment-api.md @@ -151,7 +151,7 @@ RPC_WALLET_ERROR (-4) | _Unspecified problem with wallet_ ----------------------| ------------------------------------- "Could not find previous JoinSplit anchor" | Try restarting node with `-reindex`. "Error decrypting output note of previous JoinSplit: __" | -"Could not find witness for note commitment" | Try restarting node with `-reindex`. +"Could not find witness for note commitment" | Try restarting node with `-rescan`. "Witness for note commitment is null" | Missing witness for note commitement. "Witness for spendable note does not have same anchor as change input" | Invalid anchor for spendable note witness. "Not enough funds to pay miners fee" | Retry with sufficient funds. diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index c912a04c1..9bcc5f533 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -51,7 +51,7 @@ public: void IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblock, - ZCIncrementalMerkleTree tree) { + ZCIncrementalMerkleTree& tree) { CWallet::IncrementNoteWitnesses(pindex, pblock, tree); } void DecrementNoteWitnesses(const CBlockIndex* pindex) { @@ -82,6 +82,28 @@ CWalletTx GetValidSpend(const libzcash::SpendingKey& sk, return GetValidSpend(*params, sk, note, value); } +JSOutPoint CreateValidBlock(TestWallet& wallet, + const libzcash::SpendingKey& sk, + const CBlockIndex& index, + CBlock& block, + ZCIncrementalMerkleTree& tree) { + auto wtx = GetValidReceive(sk, 50, 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); + + block.vtx.push_back(wtx); + wallet.IncrementNoteWitnesses(&index, &block, tree); + + return jsoutpt; +} + TEST(wallet_tests, setup_datadir_location_run_as_first_test) { // Get temporary and unique path for file. boost::filesystem::path pathTemp = boost::filesystem::temp_directory_path() / boost::filesystem::unique_path(); @@ -572,27 +594,14 @@ TEST(wallet_tests, cached_witnesses_chain_tip) { wallet.AddSpendingKey(sk); { - // First transaction (case tested in _empty_chain) - auto wtx = GetValidReceive(sk, 10, 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); - - std::vector notes {jsoutpt}; - std::vector> witnesses; - // First block (case tested in _empty_chain) - block1.vtx.push_back(wtx); CBlockIndex index1(block1); index1.nHeight = 1; - wallet.IncrementNoteWitnesses(&index1, &block1, tree); + auto jsoutpt = CreateValidBlock(wallet, sk, index1, block1, tree); + // Called to fetch anchor + std::vector notes {jsoutpt}; + std::vector> witnesses; wallet.GetNoteWitnesses(notes, witnesses, anchor1); } @@ -667,47 +676,21 @@ TEST(wallet_tests, CachedWitnessesDecrementFirst) { wallet.AddSpendingKey(sk); { - // First transaction (case tested in _empty_chain) - auto wtx = GetValidReceive(sk, 10, 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); - // First block (case tested in _empty_chain) CBlock block1; - block1.vtx.push_back(wtx); CBlockIndex index1(block1); index1.nHeight = 1; - wallet.IncrementNoteWitnesses(&index1, &block1, tree); + CreateValidBlock(wallet, sk, index1, block1, tree); } { - // Second transaction (case tested in _chain_tip) - auto wtx = GetValidReceive(sk, 50, 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); + // Second block (case tested in _chain_tip) + index2.nHeight = 2; + auto jsoutpt = CreateValidBlock(wallet, sk, index2, block2, tree); + // Called to fetch anchor std::vector notes {jsoutpt}; std::vector> witnesses; - - // Second block (case tested in _chain_tip) - block2.vtx.push_back(wtx); - index2.nHeight = 2; - wallet.IncrementNoteWitnesses(&index2, &block2, tree); - // Called to fetch anchor wallet.GetNoteWitnesses(notes, witnesses, anchor2); } @@ -753,116 +736,77 @@ TEST(wallet_tests, CachedWitnessesDecrementFirst) { TEST(wallet_tests, CachedWitnessesCleanIndex) { TestWallet wallet; - CBlock block1; - CBlock block2; - CBlock block3; - CBlockIndex index1(block1); - CBlockIndex index2(block2); - CBlockIndex index3(block3); + std::vector blocks; + std::vector indices; + std::vector notes; + std::vector anchors; ZCIncrementalMerkleTree tree; + ZCIncrementalMerkleTree riTree = tree; + std::vector> witnesses; auto sk = libzcash::SpendingKey::random(); wallet.AddSpendingKey(sk); - { - // First transaction (case tested in _empty_chain) - auto wtx = GetValidReceive(sk, 10, true); - auto note = GetNote(sk, wtx, 0, 1); - auto nullifier = note.nullifier(sk); + // Generate a chain + size_t numBlocks = WITNESS_CACHE_SIZE + 10; + blocks.resize(numBlocks); + indices.resize(numBlocks); + for (size_t i = 0; i < numBlocks; i++) { + indices[i].nHeight = i; + auto old = tree.root(); + auto jsoutpt = CreateValidBlock(wallet, sk, indices[i], blocks[i], tree); + EXPECT_NE(old, tree.root()); + notes.push_back(jsoutpt); - 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); - - // First block (case tested in _empty_chain) - block1.vtx.push_back(wtx); - index1.nHeight = 1; - wallet.IncrementNoteWitnesses(&index1, &block1, tree); + witnesses.clear(); + uint256 anchor; + wallet.GetNoteWitnesses(notes, witnesses, anchor); + for (size_t j = 0; j <= i; j++) { + EXPECT_TRUE((bool) witnesses[j]); + } + anchors.push_back(anchor); } - { - // Second transaction (case tested in _chain_tip) - auto wtx = GetValidReceive(sk, 50, 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); - - // Second block (case tested in _chain_tip) - block2.vtx.push_back(wtx); - index2.nHeight = 2; - wallet.IncrementNoteWitnesses(&index2, &block2, tree); - } - - { - // Third transaction - 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); - - std::vector notes {jsoutpt}; - std::vector> witnesses; - uint256 anchor3; - - // Third block - block3.vtx.push_back(wtx); - index3.nHeight = 3; - wallet.IncrementNoteWitnesses(&index3, &block3, tree); - wallet.GetNoteWitnesses(notes, witnesses, anchor3); - - // Now pretend we are reindexing: the chain is cleared, and each block is - // used to increment witnesses again. - wallet.IncrementNoteWitnesses(&index1, &block1, tree); - uint256 anchor3a; + // Now pretend we are reindexing: the chain is cleared, and each block is + // used to increment witnesses again. + for (size_t i = 0; i < numBlocks; i++) { + ZCIncrementalMerkleTree riPrevTree {riTree}; + wallet.IncrementNoteWitnesses(&(indices[i]), &(blocks[i]), riTree); witnesses.clear(); - wallet.GetNoteWitnesses(notes, witnesses, anchor3a); - EXPECT_TRUE((bool) witnesses[0]); - // Should equal third anchor because witness cache unaffected - EXPECT_EQ(anchor3, anchor3a); + uint256 anchor; + wallet.GetNoteWitnesses(notes, witnesses, anchor); + for (size_t j = 0; j < numBlocks; j++) { + EXPECT_TRUE((bool) witnesses[j]); + } + // Should equal final anchor because witness cache unaffected + EXPECT_EQ(anchors.back(), anchor); - wallet.IncrementNoteWitnesses(&index2, &block2, tree); - uint256 anchor3b; - witnesses.clear(); - wallet.GetNoteWitnesses(notes, witnesses, anchor3b); - EXPECT_TRUE((bool) witnesses[0]); - EXPECT_EQ(anchor3, anchor3b); + if ((i == 5) || (i == 50)) { + // Pretend a reorg happened that was recorded in the block files + { + wallet.DecrementNoteWitnesses(&(indices[i])); + witnesses.clear(); + uint256 anchor; + wallet.GetNoteWitnesses(notes, witnesses, anchor); + for (size_t j = 0; j < numBlocks; j++) { + EXPECT_TRUE((bool) witnesses[j]); + } + // Should equal final anchor because witness cache unaffected + EXPECT_EQ(anchors.back(), anchor); + } - // Pretend a reorg happened that was recorded in the block files - wallet.DecrementNoteWitnesses(&index2); - uint256 anchor3c; - witnesses.clear(); - wallet.GetNoteWitnesses(notes, witnesses, anchor3c); - EXPECT_TRUE((bool) witnesses[0]); - EXPECT_EQ(anchor3, anchor3c); - - wallet.IncrementNoteWitnesses(&index2, &block2, tree); - uint256 anchor3d; - witnesses.clear(); - wallet.GetNoteWitnesses(notes, witnesses, anchor3d); - EXPECT_TRUE((bool) witnesses[0]); - EXPECT_EQ(anchor3, anchor3d); - - wallet.IncrementNoteWitnesses(&index3, &block3, tree); - uint256 anchor3e; - witnesses.clear(); - wallet.GetNoteWitnesses(notes, witnesses, anchor3e); - EXPECT_TRUE((bool) witnesses[0]); - EXPECT_EQ(anchor3, anchor3e); + { + wallet.IncrementNoteWitnesses(&(indices[i]), &(blocks[i]), riPrevTree); + witnesses.clear(); + uint256 anchor; + wallet.GetNoteWitnesses(notes, witnesses, anchor); + for (size_t j = 0; j < numBlocks; j++) { + EXPECT_TRUE((bool) witnesses[j]); + } + // Should equal final anchor because witness cache unaffected + EXPECT_EQ(anchors.back(), anchor); + } + } } } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c49ca662d..8c32dcb1a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -704,7 +704,8 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, // If this is our note, witness it if (txIsOurs) { JSOutPoint jsoutpt {hash, i, j}; - if (mapWallet[hash].mapNoteData.count(jsoutpt)) { + if (mapWallet[hash].mapNoteData.count(jsoutpt) && + mapWallet[hash].mapNoteData[jsoutpt].witnessHeight < pindex->nHeight) { CNoteData* nd = &(mapWallet[hash].mapNoteData[jsoutpt]); if (nd->witnesses.size() > 0) { // We think this can happen because we write out the