Auto merge of #1445 - bitcartel:master_fix_filtered_notes, r=str4d
Fix casting error in GetFilteredNotes Use int for minDepth like upstream instead of size_t which can lead to casting problems if a wallet tx has a depth of -1. Also don't use FindMyNotes as mapNoteData has already been set on wallet tx. @str4d As dicussed. This should be merged before other PRs related to wallet.
This commit is contained in:
commit
42941c9fd4
|
@ -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<CNotePlaintextEntry> 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);
|
||||
}
|
||||
|
||||
|
||||
|
|
|
@ -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<CBitcoinAddress> setAddress;
|
||||
vector<COutput> 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<CNotePlaintextEntry> entries;
|
||||
LOCK2(cs_main, pwalletMain->cs_wallet);
|
||||
|
|
|
@ -3205,7 +3205,7 @@ bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectAbsurdFee)
|
|||
return ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, NULL, fRejectAbsurdFee);
|
||||
}
|
||||
|
||||
bool CWallet::GetFilteredNotes(std::vector<CNotePlaintextEntry> & outEntries, std::string address, size_t minDepth = 1, bool ignoreSpent)
|
||||
bool CWallet::GetFilteredNotes(std::vector<CNotePlaintextEntry> & outEntries, std::string address, int minDepth, bool ignoreSpent)
|
||||
{
|
||||
bool fFilterAddress = false;
|
||||
libzcash::PaymentAddress filterPaymentAddress;
|
||||
|
@ -3224,13 +3224,11 @@ bool CWallet::GetFilteredNotes(std::vector<CNotePlaintextEntry> & 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;
|
||||
|
|
|
@ -906,7 +906,7 @@ public:
|
|||
void SetBroadcastTransactions(bool broadcast) { fBroadcastTransactions = broadcast; }
|
||||
|
||||
/* Find notes filtered by payment address, min depth, ability to spend */
|
||||
bool GetFilteredNotes(std::vector<CNotePlaintextEntry> & outEntries, std::string address, size_t minDepth, bool ignoreSpent=true);
|
||||
bool GetFilteredNotes(std::vector<CNotePlaintextEntry> & outEntries, std::string address, int minDepth=1, bool ignoreSpent=true);
|
||||
|
||||
};
|
||||
|
||||
|
|
Loading…
Reference in New Issue