From 5db5e42ec325d45f40a5f91fd8e454a9714ad0ba Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:49:38 +1200 Subject: [PATCH 01/21] Add optional bool to disable computation of proof in JSDescription constructor --- src/primitives/transaction.cpp | 10 +++++++--- src/primitives/transaction.h | 3 ++- src/zcash/JoinSplit.cpp | 9 +++++++-- src/zcash/JoinSplit.hpp | 3 ++- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp index 854a0be66..5c1f94758 100644 --- a/src/primitives/transaction.cpp +++ b/src/primitives/transaction.cpp @@ -15,11 +15,14 @@ JSDescription::JSDescription(ZCJoinSplit& params, const boost::array& inputs, const boost::array& outputs, CAmount vpub_old, - CAmount vpub_new) : vpub_old(vpub_old), vpub_new(vpub_new), anchor(anchor) + CAmount vpub_new, + bool computeProof) : vpub_old(vpub_old), vpub_new(vpub_new), anchor(anchor) { boost::array notes; - params.loadProvingKey(); + if (computeProof) { + params.loadProvingKey(); + } proof = params.prove( inputs, outputs, @@ -33,7 +36,8 @@ JSDescription::JSDescription(ZCJoinSplit& params, commitments, vpub_old, vpub_new, - anchor + anchor, + computeProof ); } diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h index c88b26d17..44375fc63 100644 --- a/src/primitives/transaction.h +++ b/src/primitives/transaction.h @@ -74,7 +74,8 @@ public: const boost::array& inputs, const boost::array& outputs, CAmount vpub_old, - CAmount vpub_new + CAmount vpub_new, + bool computeProof = true // Set to false in some tests ); // Verifies that the JoinSplit proof is correct. diff --git a/src/zcash/JoinSplit.cpp b/src/zcash/JoinSplit.cpp index b103581b3..71c1ae0ad 100644 --- a/src/zcash/JoinSplit.cpp +++ b/src/zcash/JoinSplit.cpp @@ -173,9 +173,10 @@ public: boost::array& out_commitments, uint64_t vpub_old, uint64_t vpub_new, - const uint256& rt + const uint256& rt, + bool computeProof ) { - if (!pk) { + if (computeProof && !pk) { throw std::runtime_error("JoinSplit proving key not loaded"); } @@ -231,6 +232,10 @@ public: out_macs[i] = PRF_pk(inputs[i].key, i, h_sig); } + if (!computeProof) { + return ZCProof(); + } + protoboard pb; { joinsplit_gadget g(pb); diff --git a/src/zcash/JoinSplit.hpp b/src/zcash/JoinSplit.hpp index e9e89c62d..1b655728d 100644 --- a/src/zcash/JoinSplit.hpp +++ b/src/zcash/JoinSplit.hpp @@ -73,7 +73,8 @@ public: boost::array& out_commitments, uint64_t vpub_old, uint64_t vpub_new, - const uint256& rt + const uint256& rt, + bool computeProof = true ) = 0; virtual bool verify( From 02e674555ef2f9ecf669503f106414badda77ae7 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:50:45 +1200 Subject: [PATCH 02/21] Add wallet method for finding spendable notes in a CTransaction --- src/Makefile.gtest.include | 3 +- src/keystore.cpp | 4 +- src/keystore.h | 29 ++++++++++ src/wallet/gtest/test_wallet.cpp | 98 ++++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 43 ++++++++++++++ src/wallet/wallet.h | 84 +++++++++++++++++++++++++++ src/zcash/Address.hpp | 1 + src/zcash/NoteEncryption.hpp | 4 ++ 8 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 src/wallet/gtest/test_wallet.cpp diff --git a/src/Makefile.gtest.include b/src/Makefile.gtest.include index 8e22ed397..e69deb89f 100644 --- a/src/Makefile.gtest.include +++ b/src/Makefile.gtest.include @@ -16,7 +16,8 @@ zcash_gtest_SOURCES = \ gtest/test_txid.cpp \ gtest/test_wallet_zkeys.cpp \ gtest/test_libzcash_utils.cpp \ - gtest/test_proofs.cpp + gtest/test_proofs.cpp \ + wallet/gtest/test_wallet.cpp zcash_gtest_CPPFLAGS = -DMULTICORE -fopenmp -DBINARY_OUTPUT -DCURVE_ALT_BN128 -DSTATIC diff --git a/src/keystore.cpp b/src/keystore.cpp index 7240cd747..251e47e26 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -87,6 +87,8 @@ bool CBasicKeyStore::HaveWatchOnly() const bool CBasicKeyStore::AddSpendingKey(const libzcash::SpendingKey &sk) { LOCK(cs_SpendingKeyStore); - mapSpendingKeys[sk.address()] = sk; + auto address = sk.address(); + mapSpendingKeys[address] = sk; + mapNoteDecryptors[address] = ZCNoteDecryption(sk.viewing_key()); return true; } diff --git a/src/keystore.h b/src/keystore.h index 987f32070..478c06cb8 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -12,6 +12,7 @@ #include "script/standard.h" #include "sync.h" #include "zcash/Address.hpp" +#include "zcash/NoteEncryption.hpp" #include #include @@ -60,6 +61,7 @@ typedef std::map KeyMap; typedef std::map ScriptMap; typedef std::set WatchOnlySet; typedef std::map SpendingKeyMap; +typedef std::map NoteDecryptorMap; /** Basic key store, that keeps keys in an address->secret map */ class CBasicKeyStore : public CKeyStore @@ -69,6 +71,7 @@ protected: ScriptMap mapScripts; WatchOnlySet setWatchOnly; SpendingKeyMap mapSpendingKeys; + NoteDecryptorMap mapNoteDecryptors; public: bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey); @@ -139,6 +142,32 @@ public: } return false; } + bool GetNoteDecryptor(const libzcash::PaymentAddress &address, ZCNoteDecryption &decOut) const + { + { + LOCK(cs_KeyStore); + NoteDecryptorMap::const_iterator mi = mapNoteDecryptors.find(address); + if (mi != mapNoteDecryptors.end()) + { + decOut = mi->second; + return true; + } + } + return false; + } + void GetNoteDecryptors(std::set &setDec) const + { + setDec.clear(); + { + LOCK(cs_SpendingKeyStore); + NoteDecryptorMap::const_iterator mi = mapNoteDecryptors.begin(); + while (mi != mapNoteDecryptors.end()) + { + setDec.insert(*mi); + mi++; + } + } + } void GetPaymentAddresses(std::set &setAddress) const { setAddress.clear(); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp new file mode 100644 index 000000000..bbb0501bb --- /dev/null +++ b/src/wallet/gtest/test_wallet.cpp @@ -0,0 +1,98 @@ +#include +#include + +#include "base58.h" +#include "chainparams.h" +#include "random.h" +#include "wallet/wallet.h" +#include "zcash/JoinSplit.hpp" +#include "zcash/Note.hpp" +#include "zcash/NoteEncryption.hpp" + +ZCJoinSplit* params = ZCJoinSplit::Unopened(); + +CWalletTx GetValidReceive(const libzcash::SpendingKey& sk, CAmount value, bool randomInputs) { + CMutableTransaction mtx; + mtx.nVersion = 2; // Enable JoinSplits + mtx.vin.resize(2); + if (randomInputs) { + mtx.vin[0].prevout.hash = GetRandHash(); + mtx.vin[1].prevout.hash = GetRandHash(); + } else { + mtx.vin[0].prevout.hash = uint256S("0000000000000000000000000000000000000000000000000000000000000001"); + mtx.vin[1].prevout.hash = uint256S("0000000000000000000000000000000000000000000000000000000000000002"); + } + mtx.vin[0].prevout.n = 0; + mtx.vin[1].prevout.n = 0; + + // Generate an ephemeral keypair. + uint256 joinSplitPubKey; + unsigned char joinSplitPrivKey[crypto_sign_SECRETKEYBYTES]; + crypto_sign_keypair(joinSplitPubKey.begin(), joinSplitPrivKey); + mtx.joinSplitPubKey = joinSplitPubKey; + + boost::array inputs = { + libzcash::JSInput(), // dummy input + libzcash::JSInput() // dummy input + }; + + boost::array outputs = { + libzcash::JSOutput(), // dummy output + libzcash::JSOutput(sk.address(), value) + }; + + boost::array output_notes; + + // Prepare JoinSplits + uint256 rt; + JSDescription jsdesc {*params, mtx.joinSplitPubKey, rt, + inputs, outputs, value, 0, false}; + mtx.vjoinsplit.push_back(jsdesc); + + // Empty output script. + CScript scriptCode; + CTransaction signTx(mtx); + uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL); + + // Add the signature + assert(crypto_sign_detached(&mtx.joinSplitSig[0], NULL, + dataToBeSigned.begin(), 32, + joinSplitPrivKey + ) == 0); + + CTransaction tx {mtx}; + CWalletTx wtx {NULL, tx}; + return wtx; +} + +libzcash::Note GetNote(const libzcash::SpendingKey& sk, + const CTransaction& tx, size_t js, size_t n) { + ZCNoteDecryption decryptor {sk.viewing_key()}; + auto hSig = tx.vjoinsplit[js].h_sig(*params, tx.joinSplitPubKey); + auto note_pt = libzcash::NotePlaintext::decrypt( + decryptor, + tx.vjoinsplit[js].ciphertexts[n], + tx.vjoinsplit[js].ephemeralKey, + hSig, + (unsigned char) n); + return note_pt.note(sk.address()); +} + +TEST(wallet_tests, find_note_in_tx) { + CWallet wallet; + + auto sk = libzcash::SpendingKey::random(); + wallet.AddSpendingKey(sk); + + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + auto noteMap = wallet.FindMyNotes(wtx); + EXPECT_EQ(1, noteMap.size()); + + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + EXPECT_EQ(1, noteMap.count(jsoutpt)); + EXPECT_EQ(nd, noteMap[jsoutpt]); +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0f6eeff49..cb977ec8e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -10,6 +10,7 @@ #include "coincontrol.h" #include "consensus/consensus.h" #include "consensus/validation.h" +#include "init.h" #include "main.h" #include "net.h" #include "script/script.h" @@ -17,6 +18,7 @@ #include "timedata.h" #include "util.h" #include "utilmoneystr.h" +#include "zcash/Note.hpp" #include @@ -57,6 +59,11 @@ struct CompareValueOnly } }; +std::string JSOutPoint::ToString() const +{ + return strprintf("JSOutPoint(%s, %d, %d)", hash.ToString().substr(0,10), js, n); +} + std::string COutput::ToString() const { return strprintf("COutput(%s, %d, %d) [%s]", tx->GetTxid().ToString(), i, nDepth, FormatMoney(tx->vout[i].nValue)); @@ -843,6 +850,42 @@ void CWallet::EraseFromWallet(const uint256 &hash) } +mapNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const +{ + uint256 hash = tx.GetTxid(); + + mapNoteData_t noteData; + std::set decryptors; + GetNoteDecryptors(decryptors); + libzcash::SpendingKey key; + for (size_t i = 0; i < tx.vjoinsplit.size(); i++) { + auto hSig = tx.vjoinsplit[i].h_sig(*pzcashParams, tx.joinSplitPubKey); + for (uint8_t j = 0; j < tx.vjoinsplit[i].ciphertexts.size(); j++) { + for (const NoteDecryptorMap::value_type& item : decryptors) { + try { + auto note_pt = libzcash::NotePlaintext::decrypt( + item.second, + tx.vjoinsplit[i].ciphertexts[j], + tx.vjoinsplit[i].ephemeralKey, + hSig, + (unsigned char) j); + auto address = item.first; + // Decryptors are only cached when SpendingKeys are added + assert(GetSpendingKey(address, key)); + auto note = note_pt.note(address); + JSOutPoint jsoutpt {hash, i, j}; + CNoteData nd {address, note.nullifier(key)}; + noteData.insert(std::make_pair(jsoutpt, nd)); + break; + } catch (const std::exception &) { + // Couldn't decrypt with this spending key + } + } + } + } + return noteData; +} + isminetype CWallet::IsMine(const CTxIn &txin) const { { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0651c6404..4bbbe2aa0 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -146,6 +146,88 @@ struct COutputEntry int vout; }; +/** An note outpoint */ +class JSOutPoint +{ +public: + // Transaction hash + uint256 hash; + // Index into CTransaction.vjoinsplit + size_t js; + // Index into JSDescription fields of length ZC_NUM_JS_OUTPUTS + uint8_t n; + + JSOutPoint() { SetNull(); } + JSOutPoint(uint256 h, size_t js, uint8_t n) : hash {h}, js {js}, n {n} { } + + ADD_SERIALIZE_METHODS; + + template + inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { + READWRITE(hash); + READWRITE(js); + READWRITE(n); + } + + void SetNull() { hash.SetNull(); } + bool IsNull() const { return hash.IsNull(); } + + friend bool operator<(const JSOutPoint& a, const JSOutPoint& b) { + return (a.hash < b.hash || + (a.hash == b.hash && a.js < b.js) || + (a.hash == b.hash && a.js == b.js && a.n < b.n)); + } + + friend bool operator==(const JSOutPoint& a, const JSOutPoint& b) { + return (a.hash == b.hash && a.js == b.js && a.n == b.n); + } + + friend bool operator!=(const JSOutPoint& a, const JSOutPoint& b) { + return !(a == b); + } + + std::string ToString() const; +}; + +class CNoteData +{ +public: + libzcash::PaymentAddress address; + + // It's okay to cache the nullifier in the wallet, because we are storing + // the spending key there too, which could be used to derive this. + // If PR #1210 is merged, we need to revisit the threat model and decide + // whether it is okay to store this unencrypted while the spending key is + // encrypted. + uint256 nullifier; + + CNoteData() : address(), nullifier() { } + CNoteData(libzcash::PaymentAddress a, uint256 n) : address {a}, nullifier {n} { } + + ADD_SERIALIZE_METHODS; + + template + inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { + READWRITE(address); + READWRITE(nullifier); + } + + friend bool operator<(const CNoteData& a, const CNoteData& b) { + return (a.address < b.address || + (a.address == b.address && a.nullifier < b.nullifier)); + } + + friend bool operator==(const CNoteData& a, const CNoteData& b) { + return (a.address == b.address && a.nullifier == b.nullifier); + } + + friend bool operator!=(const CNoteData& a, const CNoteData& b) { + return !(a == b); + } +}; + +typedef std::map mapNoteData_t; + /** A transaction with a merkle branch linking it to the block chain. */ class CMerkleTx : public CTransaction { @@ -667,6 +749,8 @@ public: std::set GetAccountAddresses(std::string strAccount) const; + mapNoteData_t FindMyNotes(const CTransaction& tx) const; + isminetype IsMine(const CTxIn& txin) const; CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; isminetype IsMine(const CTxOut& txout) const; diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index 36b9402a3..d967aef27 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -23,6 +23,7 @@ public: READWRITE(pk_enc); } + friend inline bool operator==(const PaymentAddress& a, const PaymentAddress& b) { return a.a_pk == b.a_pk && a.pk_enc == b.pk_enc; } friend inline bool operator<(const PaymentAddress& a, const PaymentAddress& b) { return a.a_pk < b.a_pk; } }; diff --git a/src/zcash/NoteEncryption.hpp b/src/zcash/NoteEncryption.hpp index 7161d5a20..1cd1a9b27 100644 --- a/src/zcash/NoteEncryption.hpp +++ b/src/zcash/NoteEncryption.hpp @@ -61,6 +61,8 @@ public: typedef boost::array Ciphertext; typedef boost::array Plaintext; + // Unused default constructor to make allocators happy + NoteDecryption() { } NoteDecryption(uint256 sk_enc); Plaintext decrypt(const Ciphertext &ciphertext, @@ -68,6 +70,8 @@ public: const uint256 &hSig, unsigned char nonce ) const; + + friend inline bool operator<(const NoteDecryption& a, const NoteDecryption& b) { return a.pk_enc < b.pk_enc; } }; uint256 random_uint256(); From c3a7307a69dcf94539e5354d0a1d723f6dd59e05 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:51:09 +1200 Subject: [PATCH 03/21] Store mapping between notes and PaymentAddresses in CWalletTx --- src/wallet/gtest/test_wallet.cpp | 29 +++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 29 ++++++++++++++++++++++++++++- src/wallet/wallet.h | 5 +++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index bbb0501bb..8db093e7f 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -78,6 +78,35 @@ libzcash::Note GetNote(const libzcash::SpendingKey& sk, return note_pt.note(sk.address()); } +TEST(wallet_tests, set_note_addrs_in_cwallettx) { + auto sk = libzcash::SpendingKey::random(); + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + EXPECT_EQ(0, wtx.mapNoteData.size()); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + noteData[jsoutpt] = nd; + + wtx.SetNoteData(noteData); + EXPECT_EQ(noteData, wtx.mapNoteData); +} + +TEST(wallet_tests, set_invalid_note_addrs_in_cwallettx) { + CWalletTx wtx; + EXPECT_EQ(0, wtx.mapNoteData.size()); + + mapNoteData_t noteData; + auto sk = libzcash::SpendingKey::random(); + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), uint256()}; + noteData[jsoutpt] = nd; + + EXPECT_THROW(wtx.SetNoteData(noteData), std::runtime_error); +} + TEST(wallet_tests, find_note_in_tx) { CWallet wallet; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cb977ec8e..0d287dac7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -481,6 +481,8 @@ void CWallet::SyncMetaData(pair range) CWalletTx* copyTo = &mapWallet[hash]; if (copyFrom == copyTo) continue; copyTo->mapValue = copyFrom->mapValue; + // mapNoteData not copied on purpose + // (it is always set correctly for each CWalletTx) copyTo->vOrderForm = copyFrom->vOrderForm; // fTimeReceivedIsTxTime not copied on purpose // nTimeReceived not copied on purpose @@ -758,6 +760,11 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD wtx.nIndex = wtxIn.nIndex; fUpdated = true; } + if (!wtxIn.mapNoteData.empty() && wtxIn.mapNoteData != wtx.mapNoteData) + { + wtx.mapNoteData = wtxIn.mapNoteData; + fUpdated = true; + } if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) { wtx.fFromMe = wtxIn.fFromMe; @@ -803,10 +810,14 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetTxid()) != 0; if (fExisted && !fUpdate) return false; - if (fExisted || IsMine(tx) || IsFromMe(tx)) + auto noteData = FindMyNotes(tx); + if (fExisted || IsMine(tx) || IsFromMe(tx) || noteData.size() > 0) { CWalletTx wtx(this,tx); + if (noteData.size() > 0) + wtx.SetNoteData(noteData); + // Get merkle branch if transaction was found in a block if (pblock) wtx.SetMerkleBranch(*pblock); @@ -1007,6 +1018,22 @@ CAmount CWallet::GetChange(const CTransaction& tx) const return nChange; } +void CWalletTx::SetNoteData(mapNoteData_t ¬eData) +{ + mapNoteData.clear(); + for (const std::pair nd : noteData) { + if (nd.first.js < vjoinsplit.size() && + nd.first.n < vjoinsplit[nd.first.js].ciphertexts.size()) { + // Store the address and nullifier for the Note + mapNoteData[nd.first] = nd.second; + } else { + // If FindMyNotes() was used to obtain noteData, + // this should never happen + throw std::runtime_error("CWalletTc::SetNoteData(): Invalid note"); + } + } +} + int64_t CWalletTx::GetTxTime() const { int64_t n = nTimeSmart; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4bbbe2aa0..6b118b9c6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -298,6 +298,7 @@ private: public: mapValue_t mapValue; + mapNoteData_t mapNoteData; std::vector > vOrderForm; unsigned int fTimeReceivedIsTxTime; unsigned int nTimeReceived; //! time received by this node @@ -350,6 +351,7 @@ public: { pwallet = pwalletIn; mapValue.clear(); + mapNoteData.clear(); vOrderForm.clear(); fTimeReceivedIsTxTime = false; nTimeReceived = 0; @@ -399,6 +401,7 @@ public: std::vector vUnused; //! Used to be vtxPrev READWRITE(vUnused); READWRITE(mapValue); + READWRITE(mapNoteData); READWRITE(vOrderForm); READWRITE(fTimeReceivedIsTxTime); READWRITE(nTimeReceived); @@ -440,6 +443,8 @@ public: MarkDirty(); } + void SetNoteData(mapNoteData_t ¬eData); + //! filter decides which addresses will count towards the debit CAmount GetDebit(const isminefilter& filter) const; CAmount GetCredit(const isminefilter& filter) const; From 0f1060478fdf7ba36a90613877d8dcd4c2263554 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:51:40 +1200 Subject: [PATCH 04/21] Keep track of spent notes, and detect and report conflicts --- src/wallet/gtest/test_wallet.cpp | 121 +++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 58 +++++++++++++-- src/wallet/wallet.h | 16 +++- 3 files changed, 188 insertions(+), 7 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 8db093e7f..56ec63373 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -3,6 +3,7 @@ #include "base58.h" #include "chainparams.h" +#include "main.h" #include "random.h" #include "wallet/wallet.h" #include "zcash/JoinSplit.hpp" @@ -78,6 +79,55 @@ libzcash::Note GetNote(const libzcash::SpendingKey& sk, return note_pt.note(sk.address()); } +CWalletTx GetValidSpend(const libzcash::SpendingKey& sk, + const libzcash::Note& note, CAmount value) { + CMutableTransaction mtx; + mtx.vout.resize(2); + mtx.vout[0].nValue = value; + mtx.vout[1].nValue = 0; + + // Generate an ephemeral keypair. + uint256 joinSplitPubKey; + unsigned char joinSplitPrivKey[crypto_sign_SECRETKEYBYTES]; + crypto_sign_keypair(joinSplitPubKey.begin(), joinSplitPrivKey); + mtx.joinSplitPubKey = joinSplitPubKey; + + // Fake tree for the unused witness + ZCIncrementalMerkleTree tree; + + boost::array inputs = { + libzcash::JSInput(tree.witness(), note, sk), + libzcash::JSInput() // dummy input + }; + + boost::array outputs = { + libzcash::JSOutput(), // dummy output + libzcash::JSOutput() // dummy output + }; + + boost::array output_notes; + + // Prepare JoinSplits + uint256 rt; + JSDescription jsdesc {*params, mtx.joinSplitPubKey, rt, + inputs, outputs, 0, value, false}; + mtx.vjoinsplit.push_back(jsdesc); + + // Empty output script. + CScript scriptCode; + CTransaction signTx(mtx); + uint256 dataToBeSigned = SignatureHash(scriptCode, signTx, NOT_AN_INPUT, SIGHASH_ALL); + + // Add the signature + assert(crypto_sign_detached(&mtx.joinSplitSig[0], NULL, + dataToBeSigned.begin(), 32, + joinSplitPrivKey + ) == 0); + CTransaction tx {mtx}; + CWalletTx wtx {NULL, tx}; + return wtx; +} + TEST(wallet_tests, set_note_addrs_in_cwallettx) { auto sk = libzcash::SpendingKey::random(); auto wtx = GetValidReceive(sk, 10, true); @@ -125,3 +175,74 @@ TEST(wallet_tests, find_note_in_tx) { EXPECT_EQ(1, noteMap.count(jsoutpt)); EXPECT_EQ(nd, noteMap[jsoutpt]); } + +TEST(wallet_tests, get_conflicted_notes) { + CWallet wallet; + + auto sk = libzcash::SpendingKey::random(); + wallet.AddSpendingKey(sk); + + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + auto wtx2 = GetValidSpend(sk, note, 5); + auto wtx3 = GetValidSpend(sk, note, 10); + auto hash2 = wtx2.GetTxid(); + auto hash3 = wtx3.GetTxid(); + + // No conflicts for no spends + EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); + wallet.AddToWallet(wtx, true, NULL); + EXPECT_EQ(0, wallet.GetConflicts(hash2).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); +} + +TEST(wallet_tests, nullifier_is_spent) { + CWallet wallet; + + auto sk = libzcash::SpendingKey::random(); + wallet.AddSpendingKey(sk); + + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + EXPECT_FALSE(wallet.IsSpent(nullifier)); + + wallet.AddToWallet(wtx, true, NULL); + EXPECT_FALSE(wallet.IsSpent(nullifier)); + + auto wtx2 = GetValidSpend(sk, note, 5); + wallet.AddToWallet(wtx2, true, NULL); + EXPECT_FALSE(wallet.IsSpent(nullifier)); + + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + CBlock block; + block.vtx.push_back(wtx2); + 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()); + + wtx2.SetMerkleBranch(block); + wallet.AddToWallet(wtx2, true, NULL); + EXPECT_TRUE(wallet.IsSpent(nullifier)); + + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0d287dac7..7da567eef 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -401,6 +401,20 @@ set CWallet::GetConflicts(const uint256& txid) const for (TxSpends::const_iterator it = range.first; it != range.second; ++it) result.insert(it->second); } + + std::pair range_n; + + for (const JSDescription& jsdesc : wtx.vjoinsplit) { + for (const uint256& nullifier : jsdesc.nullifiers) { + if (mapTxNullifiers.count(nullifier) <= 1) { + continue; // No conflict if zero or one spends + } + range_n = mapTxNullifiers.equal_range(nullifier); + for (TxNullifiers::const_iterator it = range_n.first; it != range_n.second; ++it) { + result.insert(it->second); + } + } + } return result; } @@ -456,7 +470,8 @@ bool CWallet::Verify(const string& walletFile, string& warningString, string& er return true; } -void CWallet::SyncMetaData(pair range) +template +void CWallet::SyncMetaData(pair::iterator, typename TxSpendMap::iterator> range) { // We want all the wallet transactions in range to have the same metadata as // the oldest (smallest nOrderPos). @@ -464,7 +479,7 @@ void CWallet::SyncMetaData(pair range) int nMinOrderPos = std::numeric_limits::max(); const CWalletTx* copyFrom = NULL; - for (TxSpends::iterator it = range.first; it != range.second; ++it) + for (typename TxSpendMap::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; int n = mapWallet[hash].nOrderPos; @@ -475,7 +490,7 @@ void CWallet::SyncMetaData(pair range) } } // Now copy data from copyFrom to rest: - for (TxSpends::iterator it = range.first; it != range.second; ++it) + for (typename TxSpendMap::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; CWalletTx* copyTo = &mapWallet[hash]; @@ -514,15 +529,42 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const return false; } +/** + * Note is spent if any non-conflicted transaction + * spends it: + */ +bool CWallet::IsSpent(const uint256& nullifier) const +{ + pair range; + range = mapTxNullifiers.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 + } + } + return false; +} + void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) { mapTxSpends.insert(make_pair(outpoint, wtxid)); pair range; range = mapTxSpends.equal_range(outpoint); - SyncMetaData(range); + SyncMetaData(range); } +void CWallet::AddToSpends(const uint256& nullifier, const uint256& wtxid) +{ + mapTxNullifiers.insert(make_pair(nullifier, wtxid)); + + pair range; + range = mapTxNullifiers.equal_range(nullifier); + SyncMetaData(range); +} void CWallet::AddToSpends(const uint256& wtxid) { @@ -531,8 +573,14 @@ void CWallet::AddToSpends(const uint256& wtxid) if (thisTx.IsCoinBase()) // Coinbases don't spend anything! return; - BOOST_FOREACH(const CTxIn& txin, thisTx.vin) + for (const CTxIn& txin : thisTx.vin) { AddToSpends(txin.prevout, wtxid); + } + for (const JSDescription& jsdesc : thisTx.vjoinsplit) { + for (const uint256& nullifier : jsdesc.nullifiers) { + AddToSpends(nullifier, wtxid); + } + } } bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 6b118b9c6..5c94655e7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -548,17 +548,28 @@ private: int64_t nLastResend; bool fBroadcastTransactions; + template + using TxSpendMap = std::multimap; /** * Used to keep track of spent outpoints, and * detect and report conflicts (double-spends or * mutated transactions where the mutant gets mined). */ - typedef std::multimap TxSpends; + typedef TxSpendMap TxSpends; TxSpends mapTxSpends; + /** + * Used to keep track of spent Notes, and + * detect and report conflicts (double-spends). + */ + typedef TxSpendMap TxNullifiers; + TxNullifiers mapTxNullifiers; + void AddToSpends(const COutPoint& outpoint, const uint256& wtxid); + void AddToSpends(const uint256& nullifier, const uint256& wtxid); void AddToSpends(const uint256& wtxid); - void SyncMetaData(std::pair); + template + void SyncMetaData(std::pair::iterator, typename TxSpendMap::iterator>); public: /* @@ -636,6 +647,7 @@ 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 IsLockedCoin(uint256 hash, unsigned int n) const; void LockCoin(COutPoint& output); From 8db7e25c3f2248fcca2b57b0c5f59186f29e4e01 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:51:56 +1200 Subject: [PATCH 05/21] Create mapping from nullifiers to received notes This is used in the same way as CTxIn.prevout (e.g. to mark transactions dirty). --- src/wallet/gtest/test_wallet.cpp | 26 ++++++++++++++++++++++++++ src/wallet/wallet.cpp | 20 ++++++++++++++++++++ src/wallet/wallet.h | 2 ++ 3 files changed, 48 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 56ec63373..a11d7db50 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -246,3 +246,29 @@ TEST(wallet_tests, nullifier_is_spent) { chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); } + +TEST(wallet_tests, navigate_from_nullifier_to_note) { + CWallet wallet; + + auto sk = libzcash::SpendingKey::random(); + wallet.AddSpendingKey(sk); + + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + noteData[jsoutpt] = nd; + + wtx.SetNoteData(noteData); + + EXPECT_EQ(0, wallet.mapNullifiers.count(nullifier)); + + wallet.AddToWallet(wtx, true, NULL); + EXPECT_EQ(1, wallet.mapNullifiers.count(nullifier)); + EXPECT_EQ(wtx.GetTxid(), wallet.mapNullifiers[nullifier].hash); + EXPECT_EQ(0, wallet.mapNullifiers[nullifier].js); + EXPECT_EQ(1, wallet.mapNullifiers[nullifier].n); +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7da567eef..55e042ec6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -722,6 +722,16 @@ void CWallet::MarkDirty() } } +void CWallet::UpdateNullifierNoteMap(const CWalletTx& wtx) +{ + { + LOCK(cs_wallet); + for (const mapNoteData_t::value_type& item : wtx.mapNoteData) { + mapNullifiers[item.second.nullifier] = item.first; + } + } +} + bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletDB* pwalletdb) { uint256 hash = wtxIn.GetTxid(); @@ -730,6 +740,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD { mapWallet[hash] = wtxIn; mapWallet[hash].BindWallet(this); + UpdateNullifierNoteMap(mapWallet[hash]); AddToSpends(hash); } else @@ -739,6 +750,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD pair::iterator, bool> ret = mapWallet.insert(make_pair(hash, wtxIn)); CWalletTx& wtx = (*ret.first).second; wtx.BindWallet(this); + UpdateNullifierNoteMap(wtx); bool fInsertedNew = ret.second; if (fInsertedNew) { @@ -894,6 +906,14 @@ void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock) if (mapWallet.count(txin.prevout.hash)) mapWallet[txin.prevout.hash].MarkDirty(); } + for (const JSDescription& jsdesc : tx.vjoinsplit) { + for (const uint256& nullifier : jsdesc.nullifiers) { + if (mapNullifiers.count(nullifier) && + mapWallet.count(mapNullifiers[nullifier].hash)) { + mapWallet[mapNullifiers[nullifier].hash].MarkDirty(); + } + } + } } void CWallet::EraseFromWallet(const uint256 &hash) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5c94655e7..511bb9c02 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -625,6 +625,7 @@ public: fBroadcastTransactions = false; } + std::map mapNullifiers; std::map mapWallet; int64_t nOrderPosNext; @@ -727,6 +728,7 @@ public: TxItems OrderedTxItems(std::list& acentries, std::string strAccount = ""); void MarkDirty(); + void UpdateNullifierNoteMap(const CWalletTx& wtx); 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 be74c80deba702e33a970047f7b589a19c3dc2ea Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:52:27 +1200 Subject: [PATCH 06/21] Add caching of incremental witnesses for spendable notes --- src/serialize.h | 44 +++++++++ src/wallet/gtest/test_wallet.cpp | 154 +++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 130 +++++++++++++++++++++++++- src/wallet/wallet.h | 28 ++++++ src/wallet/walletdb.cpp | 10 ++ src/wallet/walletdb.h | 2 + 6 files changed, 367 insertions(+), 1 deletion(-) diff --git a/src/serialize.h b/src/serialize.h index aca3ed076..34d41bf84 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -544,6 +545,13 @@ template unsigned int GetSerializeSize(co template void Serialize(Stream& os, const std::set& m, int nType, int nVersion); template void Unserialize(Stream& is, std::set& m, int nType, int nVersion); +/** + * list + */ +template unsigned int GetSerializeSize(const std::list& m, int nType, int nVersion); +template void Serialize(Stream& os, const std::list& m, int nType, int nVersion); +template void Unserialize(Stream& is, std::list& m, int nType, int nVersion); + @@ -890,6 +898,42 @@ void Unserialize(Stream& is, std::set& m, int nType, int nVersion) +/** + * list + */ +template +unsigned int GetSerializeSize(const std::list& l, int nType, int nVersion) +{ + unsigned int nSize = GetSizeOfCompactSize(l.size()); + for (typename std::list::const_iterator it = l.begin(); it != l.end(); ++it) + nSize += GetSerializeSize((*it), nType, nVersion); + return nSize; +} + +template +void Serialize(Stream& os, const std::list& l, int nType, int nVersion) +{ + WriteCompactSize(os, l.size()); + for (typename std::list::const_iterator it = l.begin(); it != l.end(); ++it) + Serialize(os, (*it), nType, nVersion); +} + +template +void Unserialize(Stream& is, std::list& l, int nType, int nVersion) +{ + l.clear(); + unsigned int nSize = ReadCompactSize(is); + typename std::list::iterator it = l.begin(); + for (unsigned int i = 0; i < nSize; i++) + { + T item; + Unserialize(is, item, nType, nVersion); + l.push_back(item); + } +} + + + /** * Support for ADD_SERIALIZE_METHODS and READWRITE macro */ diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index a11d7db50..6b7374fb2 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -10,8 +11,32 @@ #include "zcash/Note.hpp" #include "zcash/NoteEncryption.hpp" +using ::testing::_; +using ::testing::Return; + ZCJoinSplit* params = ZCJoinSplit::Unopened(); +class MockCCoinsViewCache : public CCoinsViewCache { +public: + MockCCoinsViewCache() : CCoinsViewCache(NULL) { }; + + MOCK_CONST_METHOD2(GetAnchorAt, bool(const uint256 &rt, ZCIncrementalMerkleTree &tree)); +}; + +class TestWallet : public CWallet { +public: + TestWallet() : CWallet() { } + + void IncrementNoteWitnesses(const CBlockIndex* pindex, + const CBlock* pblock, + const CCoinsViewCache* pcoins) { + CWallet::IncrementNoteWitnesses(pindex, pblock, pcoins); + } + void DecrementNoteWitnesses() { + CWallet::DecrementNoteWitnesses(); + } +}; + CWalletTx GetValidReceive(const libzcash::SpendingKey& sk, CAmount value, bool randomInputs) { CMutableTransaction mtx; mtx.nVersion = 2; // Enable JoinSplits @@ -272,3 +297,132 @@ TEST(wallet_tests, navigate_from_nullifier_to_note) { EXPECT_EQ(0, wallet.mapNullifiers[nullifier].js); EXPECT_EQ(1, wallet.mapNullifiers[nullifier].n); } + +TEST(wallet_tests, cached_witnesses_empty_chain) { + TestWallet wallet; + + auto sk = libzcash::SpendingKey::random(); + wallet.AddSpendingKey(sk); + + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + noteData[jsoutpt] = nd; + wtx.SetNoteData(noteData); + + std::vector notes {jsoutpt}; + std::vector> witnesses; + uint256 anchor; + + wallet.GetNoteWitnesses(notes, witnesses, anchor); + EXPECT_FALSE((bool) witnesses[0]); + + wallet.AddToWallet(wtx, true, NULL); + witnesses.clear(); + wallet.GetNoteWitnesses(notes, witnesses, anchor); + EXPECT_FALSE((bool) witnesses[0]); + + CBlock block; + block.vtx.push_back(wtx); + MockCCoinsViewCache coins; + // Empty chain, so we shouldn't try to fetch an anchor + EXPECT_CALL(coins, GetAnchorAt(_, _)).Times(0); + wallet.IncrementNoteWitnesses(NULL, &block, &coins); + witnesses.clear(); + wallet.GetNoteWitnesses(notes, witnesses, anchor); + EXPECT_TRUE((bool) witnesses[0]); + + wallet.DecrementNoteWitnesses(); + witnesses.clear(); + wallet.GetNoteWitnesses(notes, witnesses, anchor); + EXPECT_FALSE((bool) witnesses[0]); +} + +TEST(wallet_tests, cached_witnesses_chain_tip) { + TestWallet wallet; + MockCCoinsViewCache coins; + uint256 anchor1; + CBlock block1; + + 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); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 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); + EXPECT_CALL(coins, GetAnchorAt(_, _)) + .Times(0); + wallet.IncrementNoteWitnesses(NULL, &block1, &coins); + // Called to fetch anchor + wallet.GetNoteWitnesses(notes, witnesses, anchor1); + } + + { + // Second transaction + auto wtx = GetValidReceive(sk, 50, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 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 anchor2; + + wallet.GetNoteWitnesses(notes, witnesses, anchor2); + EXPECT_FALSE((bool) witnesses[0]); + + // Second block + CBlock block2; + block2.hashPrevBlock = block1.GetHash(); + block2.vtx.push_back(wtx); + EXPECT_CALL(coins, GetAnchorAt(anchor1, _)) + .Times(2) + .WillRepeatedly(Return(true)); + wallet.IncrementNoteWitnesses(NULL, &block2, &coins); + witnesses.clear(); + wallet.GetNoteWitnesses(notes, witnesses, anchor2); + EXPECT_TRUE((bool) witnesses[0]); + EXPECT_NE(anchor1, anchor2); + + // Decrementing should give us the previous anchor + uint256 anchor3; + wallet.DecrementNoteWitnesses(); + witnesses.clear(); + wallet.GetNoteWitnesses(notes, witnesses, anchor3); + EXPECT_FALSE((bool) witnesses[0]); + EXPECT_EQ(anchor1, anchor3); + + // Re-incrementing with the same block should give the same result + uint256 anchor4; + wallet.IncrementNoteWitnesses(NULL, &block2, &coins); + witnesses.clear(); + wallet.GetNoteWitnesses(notes, witnesses, anchor4); + EXPECT_TRUE((bool) witnesses[0]); + EXPECT_EQ(anchor2, anchor4); + } +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 55e042ec6..b16cf3e92 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -583,6 +583,110 @@ void CWallet::AddToSpends(const uint256& wtxid) } } +void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, + const CBlock* pblockIn, + const CCoinsViewCache* pcoins) +{ + { + LOCK(cs_wallet); + for (std::pair& wtxItem : mapWallet) { + for (mapNoteData_t::value_type& item : wtxItem.second.mapNoteData) { + CNoteData* nd = &(item.second); + // Copy the witness for the previous block if we have one + if (nd->witnesses.size() > 0) { + nd->witnesses.push_front(nd->witnesses.front()); + } + if (nd->witnesses.size() > WITNESS_CACHE_SIZE) { + nd->witnesses.pop_back(); + } + } + } + + const CBlock* pblock {pblockIn}; + CBlock block; + if (!pblock) { + ReadBlockFromDisk(block, pindex); + pblock = █ + } + + ZCIncrementalMerkleTree tree; + bool treeInitialised = false; + for (const CTransaction& tx : pblock->vtx) { + if (!treeInitialised && tx.vjoinsplit.size() > 0) { + LOCK(cs_main); + // vAnchorCache will only be empty at the beginning + if (vAnchorCache.size() && !pcoins->GetAnchorAt(vAnchorCache.front(), tree)) { + // This should not happen, because IncrementNoteWitnesses() + // is only called when the chain tip updates, and the + // anchors for the JoinSplits in that block should still be + // cached. + // TODO: Calculate the anchor from scratch? + throw std::runtime_error("CWallet::IncrementNoteWitnesses(): anchor not cached"); + } + treeInitialised = true; + } + + auto hash = tx.GetTxid(); + bool txIsOurs = mapWallet.count(hash); + for (size_t i = 0; i < tx.vjoinsplit.size(); i++) { + const JSDescription& jsdesc = tx.vjoinsplit[i]; + for (uint8_t j = 0; j < jsdesc.commitments.size(); j++) { + const uint256& note_commitment = jsdesc.commitments[j]; + tree.append(note_commitment); + + // Increment existing witnesses + for (std::pair& wtxItem : mapWallet) { + for (mapNoteData_t::value_type& item : wtxItem.second.mapNoteData) { + CNoteData* nd = &(item.second); + if (nd->witnesses.size() > 0) { + nd->witnesses.front().append(note_commitment); + } + } + } + + // If this is our note, witness it + if (txIsOurs) { + JSOutPoint jsoutpt {hash, i, j}; + if (mapWallet[hash].mapNoteData.count(jsoutpt)) { + mapWallet[hash].mapNoteData[jsoutpt].witnesses.push_front( + tree.witness()); + } + } + } + } + } + vAnchorCache.push_front(tree.root()); + if (vAnchorCache.size() > WITNESS_CACHE_SIZE) { + vAnchorCache.pop_back(); + } + if (fFileBacked) { + CWalletDB(strWalletFile).WriteAnchorCache(vAnchorCache); + } + } +} + +void CWallet::DecrementNoteWitnesses() +{ + { + LOCK(cs_wallet); + for (std::pair& wtxItem : mapWallet) { + for (mapNoteData_t::value_type& item : wtxItem.second.mapNoteData) { + CNoteData* nd = &(item.second); + if (nd->witnesses.size() > 0) { + nd->witnesses.pop_front(); + } + } + } + if (vAnchorCache.size() > 0) { + vAnchorCache.pop_front(); + } + // TODO: If vAnchorCache is empty, we need to regenerate the caches (#1302) + if (fFileBacked) { + CWalletDB(strWalletFile).WriteAnchorCache(vAnchorCache); + } + } +} + bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) { if (IsCrypted()) @@ -875,8 +979,9 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl { CWalletTx wtx(this,tx); - if (noteData.size() > 0) + if (noteData.size() > 0) { wtx.SetNoteData(noteData); + } // Get merkle branch if transaction was found in a block if (pblock) @@ -965,6 +1070,29 @@ mapNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const return noteData; } +void CWallet::GetNoteWitnesses(std::vector notes, + std::vector>& witnesses, + uint256 &final_anchor) +{ + { + LOCK(cs_wallet); + witnesses.resize(notes.size()); + int i = 0; + for (JSOutPoint note : notes) { + if (mapWallet.count(note.hash) && + mapWallet[note.hash].mapNoteData.count(note) && + mapWallet[note.hash].mapNoteData[note].witnesses.size() > 0) { + witnesses[i] = mapWallet[note.hash].mapNoteData[note].witnesses.front(); + } + i++; + } + // vAnchorCache should only be empty here before the genesis block (so, never) + if (vAnchorCache.size() > 0) { + final_anchor = vAnchorCache.front(); + } + } +} + isminetype CWallet::IsMine(const CTxIn &txin) const { { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 511bb9c02..4af3de4ba 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -7,6 +7,7 @@ #define BITCOIN_WALLET_WALLET_H #include "amount.h" +#include "coins.h" #include "key.h" #include "keystore.h" #include "primitives/block.h" @@ -52,6 +53,9 @@ static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 2; static const CAmount nHighTransactionMaxFeeWarning = 100 * nHighTransactionFeeWarning; //! Largest (in bytes) free transaction we're willing to create static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000; +//! Size of witness cache +// Should be large enough that we can expect to never reorg beyond our cache. +static const unsigned int WITNESS_CACHE_SIZE = 50; class CAccountingEntry; class CBlockIndex; @@ -201,6 +205,12 @@ public: // encrypted. uint256 nullifier; + /** + * Cached incremental witnesses for spendable Notes. + * Beginning of the list is the most recent witness. + */ + std::list witnesses; + CNoteData() : address(), nullifier() { } CNoteData(libzcash::PaymentAddress a, uint256 n) : address {a}, nullifier {n} { } @@ -568,6 +578,20 @@ private: void AddToSpends(const uint256& nullifier, const uint256& wtxid); void AddToSpends(const uint256& wtxid); +public: + /* + * Cached anchors corresponding to the cached incremental witnesses for the + * notes in our wallet. + */ + std::list vAnchorCache; + +protected: + void IncrementNoteWitnesses(const CBlockIndex* pindex, + const CBlock* pblock, + const CCoinsViewCache* pcoins); + void DecrementNoteWitnesses(); + +private: template void SyncMetaData(std::pair::iterator, typename TxSpendMap::iterator>); @@ -769,6 +793,10 @@ public: std::set GetAccountAddresses(std::string strAccount) const; mapNoteData_t FindMyNotes(const CTransaction& tx) const; + void GetNoteWitnesses( + std::vector notes, + std::vector>& witnesses, + uint256 &final_anchor); isminetype IsMine(const CTxIn& txin) const; CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index ab1d364e4..7a165eeaa 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -162,6 +162,12 @@ bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey) return Write(std::string("defaultkey"), vchPubKey); } +bool CWalletDB::WriteAnchorCache(const std::list& vAnchorCache) +{ + nWalletDBUpdated++; + return Write(std::string("anchorcache"), vAnchorCache); +} + bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) { return Read(std::make_pair(std::string("pool"), nPool), keypool); @@ -631,6 +637,10 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return false; } } + else if (strType == "anchorcache") + { + ssValue >> pwallet->vAnchorCache; + } } catch (...) { return false; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index d261c9644..81e3b094e 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -106,6 +106,8 @@ public: bool WriteDefaultKey(const CPubKey& vchPubKey); + bool WriteAnchorCache(const std::list& vAnchorCache); + bool ReadPool(int64_t nPool, CKeyPool& keypool); bool WritePool(int64_t nPool, const CKeyPool& keypool); bool ErasePool(int64_t nPool); From 769e031c1aba01ce250231f26a29d2547d562dc9 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 24 Aug 2016 15:52:43 +1200 Subject: [PATCH 07/21] Update cached incremental witnesses when the active block chain tip changes --- src/main.cpp | 4 ++++ src/validationinterface.cpp | 3 +++ src/validationinterface.h | 4 ++++ src/wallet/wallet.cpp | 9 +++++++++ src/wallet/wallet.h | 1 + 5 files changed, 21 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index ddeed4e05..f7c7966c4 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2403,6 +2403,8 @@ bool static DisconnectTip(CValidationState &state) { BOOST_FOREACH(const CTransaction &tx, block.vtx) { SyncWithWallets(tx, NULL); } + // Update cached incremental witnesses + GetMainSignals().ChainTip(pindexDelete, &block, false); return true; } @@ -2468,6 +2470,8 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock * BOOST_FOREACH(const CTransaction &tx, pblock->vtx) { SyncWithWallets(tx, pblock); } + // Update cached incremental witnesses + GetMainSignals().ChainTip(pindexNew, pblock, true); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint("bench", " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index aa9aefb0d..6dfefdc0d 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -16,6 +16,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2)); g_signals.EraseTransaction.connect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1)); g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); + g_signals.ChainTip.connect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); @@ -26,6 +27,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); + g_signals.ChainTip.disconnect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3)); g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.EraseTransaction.disconnect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1)); @@ -36,6 +38,7 @@ void UnregisterAllValidationInterfaces() { g_signals.BlockChecked.disconnect_all_slots(); g_signals.Broadcast.disconnect_all_slots(); g_signals.Inventory.disconnect_all_slots(); + g_signals.ChainTip.disconnect_all_slots(); g_signals.SetBestChain.disconnect_all_slots(); g_signals.UpdatedTransaction.disconnect_all_slots(); g_signals.EraseTransaction.disconnect_all_slots(); diff --git a/src/validationinterface.h b/src/validationinterface.h index 2a4c7ecce..89bcbf5e7 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -9,6 +9,7 @@ #include class CBlock; +class CBlockIndex; struct CBlockLocator; class CTransaction; class CValidationInterface; @@ -30,6 +31,7 @@ class CValidationInterface { protected: virtual void SyncTransaction(const CTransaction &tx, const CBlock *pblock) {} virtual void EraseFromWallet(const uint256 &hash) {} + virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, bool added) {} virtual void SetBestChain(const CBlockLocator &locator) {} virtual void UpdatedTransaction(const uint256 &hash) {} virtual void Inventory(const uint256 &hash) {} @@ -47,6 +49,8 @@ struct CMainSignals { boost::signals2::signal EraseTransaction; /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ boost::signals2::signal UpdatedTransaction; + /** Notifies listeners of a change to the tip of the active block chain. */ + boost::signals2::signal ChainTip; /** Notifies listeners of a new active block chain. */ boost::signals2::signal SetBestChain; /** Notifies listeners about an inventory item being seen on the network. */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b16cf3e92..fc10c34b1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -336,6 +336,15 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; } +void CWallet::ChainTip(const CBlockIndex *pindex, const CBlock *pblock, bool added) +{ + if (added) { + IncrementNoteWitnesses(pindex, pblock, pcoinsTip); + } else { + DecrementNoteWitnesses(); + } +} + void CWallet::SetBestChain(const CBlockLocator& loc) { CWalletDB walletdb(strWalletFile); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4af3de4ba..0d116b30f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -810,6 +810,7 @@ public: CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetChange(const CTransaction& tx) const; + void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, bool added); void SetBestChain(const CBlockLocator& loc); DBErrors LoadWallet(bool& fFirstRunRet); From de42390f90362df8ac6913c6bf544d7cd4aa5b12 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 02:00:11 +1200 Subject: [PATCH 08/21] Pass ZCIncrementalMerkleTree to wallet to prevent race conditions --- src/main.cpp | 10 ++++++++-- src/validationinterface.cpp | 4 ++-- src/validationinterface.h | 6 ++++-- src/wallet/gtest/test_wallet.cpp | 24 +++++++++--------------- src/wallet/wallet.cpp | 23 ++++------------------- src/wallet/wallet.h | 4 ++-- 6 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index f7c7966c4..9f850f493 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2398,13 +2398,16 @@ bool static DisconnectTip(CValidationState &state) { mempool.check(pcoinsTip); // Update chainActive and related variables. UpdateTip(pindexDelete->pprev); + // Get the current commitment tree + ZCIncrementalMerkleTree newTree; + assert(pcoinsTip->GetAnchorAt(pcoinsTip->GetBestAnchor(), newTree)); // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: BOOST_FOREACH(const CTransaction &tx, block.vtx) { SyncWithWallets(tx, NULL); } // Update cached incremental witnesses - GetMainSignals().ChainTip(pindexDelete, &block, false); + GetMainSignals().ChainTip(pindexDelete, &block, newTree, false); return true; } @@ -2429,6 +2432,9 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock * return AbortNode(state, "Failed to read block"); pblock = █ } + // Get the current commitment tree + ZCIncrementalMerkleTree oldTree; + assert(pcoinsTip->GetAnchorAt(pcoinsTip->GetBestAnchor(), oldTree)); // Apply the block atomically to the chain state. int64_t nTime2 = GetTimeMicros(); nTimeReadFromDisk += nTime2 - nTime1; int64_t nTime3; @@ -2471,7 +2477,7 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew, CBlock * SyncWithWallets(tx, pblock); } // Update cached incremental witnesses - GetMainSignals().ChainTip(pindexNew, pblock, true); + GetMainSignals().ChainTip(pindexNew, pblock, oldTree, true); int64_t nTime6 = GetTimeMicros(); nTimePostConnect += nTime6 - nTime5; nTimeTotal += nTime6 - nTime1; LogPrint("bench", " - Connect postprocess: %.2fms [%.2fs]\n", (nTime6 - nTime5) * 0.001, nTimePostConnect * 0.000001); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 6dfefdc0d..3df6cd3f2 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -16,7 +16,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2)); g_signals.EraseTransaction.connect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1)); g_signals.UpdatedTransaction.connect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); - g_signals.ChainTip.connect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3)); + g_signals.ChainTip.connect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3, _4)); g_signals.SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); @@ -27,7 +27,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); g_signals.Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1)); g_signals.Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); - g_signals.ChainTip.disconnect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3)); + g_signals.ChainTip.disconnect(boost::bind(&CValidationInterface::ChainTip, pwalletIn, _1, _2, _3, _4)); g_signals.SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.UpdatedTransaction.disconnect(boost::bind(&CValidationInterface::UpdatedTransaction, pwalletIn, _1)); g_signals.EraseTransaction.disconnect(boost::bind(&CValidationInterface::EraseFromWallet, pwalletIn, _1)); diff --git a/src/validationinterface.h b/src/validationinterface.h index 89bcbf5e7..144e716bb 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -8,6 +8,8 @@ #include +#include "zcash/IncrementalMerkleTree.hpp" + class CBlock; class CBlockIndex; struct CBlockLocator; @@ -31,7 +33,7 @@ class CValidationInterface { protected: virtual void SyncTransaction(const CTransaction &tx, const CBlock *pblock) {} virtual void EraseFromWallet(const uint256 &hash) {} - virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, bool added) {} + virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, ZCIncrementalMerkleTree tree, bool added) {} virtual void SetBestChain(const CBlockLocator &locator) {} virtual void UpdatedTransaction(const uint256 &hash) {} virtual void Inventory(const uint256 &hash) {} @@ -50,7 +52,7 @@ struct CMainSignals { /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ boost::signals2::signal UpdatedTransaction; /** Notifies listeners of a change to the tip of the active block chain. */ - boost::signals2::signal ChainTip; + boost::signals2::signal ChainTip; /** Notifies listeners of a new active block chain. */ boost::signals2::signal SetBestChain; /** Notifies listeners about an inventory item being seen on the network. */ diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 6b7374fb2..bd15b73d4 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -29,8 +29,8 @@ public: void IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblock, - const CCoinsViewCache* pcoins) { - CWallet::IncrementNoteWitnesses(pindex, pblock, pcoins); + ZCIncrementalMerkleTree tree) { + CWallet::IncrementNoteWitnesses(pindex, pblock, tree); } void DecrementNoteWitnesses() { CWallet::DecrementNoteWitnesses(); @@ -328,10 +328,8 @@ TEST(wallet_tests, cached_witnesses_empty_chain) { CBlock block; block.vtx.push_back(wtx); - MockCCoinsViewCache coins; - // Empty chain, so we shouldn't try to fetch an anchor - EXPECT_CALL(coins, GetAnchorAt(_, _)).Times(0); - wallet.IncrementNoteWitnesses(NULL, &block, &coins); + ZCIncrementalMerkleTree tree; + wallet.IncrementNoteWitnesses(NULL, &block, tree); witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor); EXPECT_TRUE((bool) witnesses[0]); @@ -344,9 +342,9 @@ TEST(wallet_tests, cached_witnesses_empty_chain) { TEST(wallet_tests, cached_witnesses_chain_tip) { TestWallet wallet; - MockCCoinsViewCache coins; uint256 anchor1; CBlock block1; + ZCIncrementalMerkleTree tree; auto sk = libzcash::SpendingKey::random(); wallet.AddSpendingKey(sk); @@ -369,9 +367,7 @@ TEST(wallet_tests, cached_witnesses_chain_tip) { // First block (case tested in _empty_chain) block1.vtx.push_back(wtx); - EXPECT_CALL(coins, GetAnchorAt(_, _)) - .Times(0); - wallet.IncrementNoteWitnesses(NULL, &block1, &coins); + wallet.IncrementNoteWitnesses(NULL, &block1, tree); // Called to fetch anchor wallet.GetNoteWitnesses(notes, witnesses, anchor1); } @@ -400,10 +396,8 @@ TEST(wallet_tests, cached_witnesses_chain_tip) { CBlock block2; block2.hashPrevBlock = block1.GetHash(); block2.vtx.push_back(wtx); - EXPECT_CALL(coins, GetAnchorAt(anchor1, _)) - .Times(2) - .WillRepeatedly(Return(true)); - wallet.IncrementNoteWitnesses(NULL, &block2, &coins); + ZCIncrementalMerkleTree tree2 {tree}; + wallet.IncrementNoteWitnesses(NULL, &block2, tree2); witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor2); EXPECT_TRUE((bool) witnesses[0]); @@ -419,7 +413,7 @@ TEST(wallet_tests, cached_witnesses_chain_tip) { // Re-incrementing with the same block should give the same result uint256 anchor4; - wallet.IncrementNoteWitnesses(NULL, &block2, &coins); + wallet.IncrementNoteWitnesses(NULL, &block2, tree); witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor4); EXPECT_TRUE((bool) witnesses[0]); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fc10c34b1..227697862 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -336,10 +336,11 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; } -void CWallet::ChainTip(const CBlockIndex *pindex, const CBlock *pblock, bool added) +void CWallet::ChainTip(const CBlockIndex *pindex, const CBlock *pblock, + ZCIncrementalMerkleTree tree, bool added) { if (added) { - IncrementNoteWitnesses(pindex, pblock, pcoinsTip); + IncrementNoteWitnesses(pindex, pblock, tree); } else { DecrementNoteWitnesses(); } @@ -594,7 +595,7 @@ void CWallet::AddToSpends(const uint256& wtxid) void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblockIn, - const CCoinsViewCache* pcoins) + ZCIncrementalMerkleTree tree) { { LOCK(cs_wallet); @@ -618,23 +619,7 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, pblock = █ } - ZCIncrementalMerkleTree tree; - bool treeInitialised = false; for (const CTransaction& tx : pblock->vtx) { - if (!treeInitialised && tx.vjoinsplit.size() > 0) { - LOCK(cs_main); - // vAnchorCache will only be empty at the beginning - if (vAnchorCache.size() && !pcoins->GetAnchorAt(vAnchorCache.front(), tree)) { - // This should not happen, because IncrementNoteWitnesses() - // is only called when the chain tip updates, and the - // anchors for the JoinSplits in that block should still be - // cached. - // TODO: Calculate the anchor from scratch? - throw std::runtime_error("CWallet::IncrementNoteWitnesses(): anchor not cached"); - } - treeInitialised = true; - } - auto hash = tx.GetTxid(); bool txIsOurs = mapWallet.count(hash); for (size_t i = 0; i < tx.vjoinsplit.size(); i++) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 0d116b30f..8b6f3db8e 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -588,7 +588,7 @@ public: protected: void IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblock, - const CCoinsViewCache* pcoins); + ZCIncrementalMerkleTree tree); void DecrementNoteWitnesses(); private: @@ -810,7 +810,7 @@ public: CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetChange(const CTransaction& tx) const; - void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, bool added); + void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, ZCIncrementalMerkleTree tree, bool added); void SetBestChain(const CBlockLocator& loc); DBErrors LoadWallet(bool& fFirstRunRet); From 3fac1020e70e83b1f731aee20b2ad286a8553b53 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 02:09:17 +1200 Subject: [PATCH 09/21] Remove GetNoteDecryptors(), lock inside FindMyNotes() instead --- src/keystore.h | 15 +-------------- src/wallet/gtest/test_wallet.cpp | 7 ------- src/wallet/wallet.cpp | 5 ++--- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/src/keystore.h b/src/keystore.h index 478c06cb8..aa3aefdf2 100644 --- a/src/keystore.h +++ b/src/keystore.h @@ -145,7 +145,7 @@ public: bool GetNoteDecryptor(const libzcash::PaymentAddress &address, ZCNoteDecryption &decOut) const { { - LOCK(cs_KeyStore); + LOCK(cs_SpendingKeyStore); NoteDecryptorMap::const_iterator mi = mapNoteDecryptors.find(address); if (mi != mapNoteDecryptors.end()) { @@ -155,19 +155,6 @@ public: } return false; } - void GetNoteDecryptors(std::set &setDec) const - { - setDec.clear(); - { - LOCK(cs_SpendingKeyStore); - NoteDecryptorMap::const_iterator mi = mapNoteDecryptors.begin(); - while (mi != mapNoteDecryptors.end()) - { - setDec.insert(*mi); - mi++; - } - } - } void GetPaymentAddresses(std::set &setAddress) const { setAddress.clear(); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index bd15b73d4..02c3362e0 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -16,13 +16,6 @@ using ::testing::Return; ZCJoinSplit* params = ZCJoinSplit::Unopened(); -class MockCCoinsViewCache : public CCoinsViewCache { -public: - MockCCoinsViewCache() : CCoinsViewCache(NULL) { }; - - MOCK_CONST_METHOD2(GetAnchorAt, bool(const uint256 &rt, ZCIncrementalMerkleTree &tree)); -}; - class TestWallet : public CWallet { public: TestWallet() : CWallet() { } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 227697862..dd229310b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1030,16 +1030,15 @@ void CWallet::EraseFromWallet(const uint256 &hash) mapNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const { + LOCK(cs_SpendingKeyStore); uint256 hash = tx.GetTxid(); mapNoteData_t noteData; - std::set decryptors; - GetNoteDecryptors(decryptors); libzcash::SpendingKey key; for (size_t i = 0; i < tx.vjoinsplit.size(); i++) { auto hSig = tx.vjoinsplit[i].h_sig(*pzcashParams, tx.joinSplitPubKey); for (uint8_t j = 0; j < tx.vjoinsplit[i].ciphertexts.size(); j++) { - for (const NoteDecryptorMap::value_type& item : decryptors) { + for (const NoteDecryptorMap::value_type& item : mapNoteDecryptors) { try { auto note_pt = libzcash::NotePlaintext::decrypt( item.second, From 4086e5ce98c1b5568b945617b79cc9282b939f62 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 14:13:28 +1200 Subject: [PATCH 10/21] Replace vAnchorCache with a cache size counter The anchor is obtained from the returned witnesses; since all witnesses are to the same point (the latest blockchain tip), they all have the same root. --- src/wallet/gtest/test_wallet.cpp | 3 ++- src/wallet/wallet.cpp | 28 ++++++++++++++++------------ src/wallet/wallet.h | 7 ++++--- src/wallet/walletdb.cpp | 8 ++++---- src/wallet/walletdb.h | 2 +- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 02c3362e0..9c6092683 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -402,7 +402,8 @@ TEST(wallet_tests, cached_witnesses_chain_tip) { witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor3); EXPECT_FALSE((bool) witnesses[0]); - EXPECT_EQ(anchor1, anchor3); + // Should not equal first anchor because none of these notes had witnesses + EXPECT_NE(anchor1, anchor3); // Re-incrementing with the same block should give the same result uint256 anchor4; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index dd229310b..9a1c86abd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -649,12 +649,11 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, } } } - vAnchorCache.push_front(tree.root()); - if (vAnchorCache.size() > WITNESS_CACHE_SIZE) { - vAnchorCache.pop_back(); + if (nWitnessCacheSize < WITNESS_CACHE_SIZE) { + nWitnessCacheSize += 1; } if (fFileBacked) { - CWalletDB(strWalletFile).WriteAnchorCache(vAnchorCache); + CWalletDB(strWalletFile).WriteWitnessCacheSize(nWitnessCacheSize); } } } @@ -671,12 +670,11 @@ void CWallet::DecrementNoteWitnesses() } } } - if (vAnchorCache.size() > 0) { - vAnchorCache.pop_front(); - } - // TODO: If vAnchorCache is empty, we need to regenerate the caches (#1302) + nWitnessCacheSize -= 1; + // TODO: If nWitnessCache is zero, we need to regenerate the caches (#1302) + assert(nWitnessCacheSize > 0); if (fFileBacked) { - CWalletDB(strWalletFile).WriteAnchorCache(vAnchorCache); + CWalletDB(strWalletFile).WriteWitnessCacheSize(nWitnessCacheSize); } } } @@ -1070,18 +1068,24 @@ void CWallet::GetNoteWitnesses(std::vector notes, { LOCK(cs_wallet); witnesses.resize(notes.size()); + boost::optional rt; int i = 0; for (JSOutPoint note : notes) { if (mapWallet.count(note.hash) && mapWallet[note.hash].mapNoteData.count(note) && mapWallet[note.hash].mapNoteData[note].witnesses.size() > 0) { witnesses[i] = mapWallet[note.hash].mapNoteData[note].witnesses.front(); + if (!rt) { + rt = witnesses[i]->root(); + } else { + assert(*rt == witnesses[i]->root()); + } } i++; } - // vAnchorCache should only be empty here before the genesis block (so, never) - if (vAnchorCache.size() > 0) { - final_anchor = vAnchorCache.front(); + // All returned witnesses have the same anchor + if (rt) { + final_anchor = *rt; } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 8b6f3db8e..ae2d3edc6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -580,10 +580,11 @@ private: public: /* - * Cached anchors corresponding to the cached incremental witnesses for the - * notes in our wallet. + * Size of the incremental witness cache for the notes in our wallet. + * This will always be greater than or equal to the size of the largest + * incremental witness cache in any transaction in mapWallet. */ - std::list vAnchorCache; + int64_t nWitnessCacheSize; protected: void IncrementNoteWitnesses(const CBlockIndex* pindex, diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 7a165eeaa..e72ee5e3e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -162,10 +162,10 @@ bool CWalletDB::WriteDefaultKey(const CPubKey& vchPubKey) return Write(std::string("defaultkey"), vchPubKey); } -bool CWalletDB::WriteAnchorCache(const std::list& vAnchorCache) +bool CWalletDB::WriteWitnessCacheSize(int64_t nWitnessCacheSize) { nWalletDBUpdated++; - return Write(std::string("anchorcache"), vAnchorCache); + return Write(std::string("witnesscachesize"), nWitnessCacheSize); } bool CWalletDB::ReadPool(int64_t nPool, CKeyPool& keypool) @@ -637,9 +637,9 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return false; } } - else if (strType == "anchorcache") + else if (strType == "witnesscachesize") { - ssValue >> pwallet->vAnchorCache; + ssValue >> pwallet->nWitnessCacheSize; } } catch (...) { diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 81e3b094e..317f71304 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -106,7 +106,7 @@ public: bool WriteDefaultKey(const CPubKey& vchPubKey); - bool WriteAnchorCache(const std::list& vAnchorCache); + bool WriteWitnessCacheSize(int64_t nWitnessCacheSize); bool ReadPool(int64_t nPool, CKeyPool& keypool); bool WritePool(int64_t nPool, const CKeyPool& keypool); From ad20f2149a4281027b7296912a846a646925f355 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 15:11:35 +1200 Subject: [PATCH 11/21] mapNullifiers -> mapNullifiersToNotes for clarity --- src/wallet/gtest/test_wallet.cpp | 10 +++++----- src/wallet/wallet.cpp | 8 ++++---- src/wallet/wallet.h | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 9c6092683..d06406e27 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -282,13 +282,13 @@ TEST(wallet_tests, navigate_from_nullifier_to_note) { wtx.SetNoteData(noteData); - EXPECT_EQ(0, wallet.mapNullifiers.count(nullifier)); + EXPECT_EQ(0, wallet.mapNullifiersToNotes.count(nullifier)); wallet.AddToWallet(wtx, true, NULL); - EXPECT_EQ(1, wallet.mapNullifiers.count(nullifier)); - EXPECT_EQ(wtx.GetTxid(), wallet.mapNullifiers[nullifier].hash); - EXPECT_EQ(0, wallet.mapNullifiers[nullifier].js); - EXPECT_EQ(1, wallet.mapNullifiers[nullifier].n); + EXPECT_EQ(1, wallet.mapNullifiersToNotes.count(nullifier)); + EXPECT_EQ(wtx.GetTxid(), wallet.mapNullifiersToNotes[nullifier].hash); + EXPECT_EQ(0, wallet.mapNullifiersToNotes[nullifier].js); + EXPECT_EQ(1, wallet.mapNullifiersToNotes[nullifier].n); } TEST(wallet_tests, cached_witnesses_empty_chain) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 9a1c86abd..7f6177337 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -823,7 +823,7 @@ void CWallet::UpdateNullifierNoteMap(const CWalletTx& wtx) { LOCK(cs_wallet); for (const mapNoteData_t::value_type& item : wtx.mapNoteData) { - mapNullifiers[item.second.nullifier] = item.first; + mapNullifiersToNotes[item.second.nullifier] = item.first; } } } @@ -1005,9 +1005,9 @@ void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock) } for (const JSDescription& jsdesc : tx.vjoinsplit) { for (const uint256& nullifier : jsdesc.nullifiers) { - if (mapNullifiers.count(nullifier) && - mapWallet.count(mapNullifiers[nullifier].hash)) { - mapWallet[mapNullifiers[nullifier].hash].MarkDirty(); + if (mapNullifiersToNotes.count(nullifier) && + mapWallet.count(mapNullifiersToNotes[nullifier].hash)) { + mapWallet[mapNullifiersToNotes[nullifier].hash].MarkDirty(); } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ae2d3edc6..46d285010 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -650,7 +650,7 @@ public: fBroadcastTransactions = false; } - std::map mapNullifiers; + std::map mapNullifiersToNotes; std::map mapWallet; int64_t nOrderPosNext; From 38a6e7a74d799e47ba44c11cd6793ecf087bfe82 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 15:26:49 +1200 Subject: [PATCH 12/21] Set witness cache size equal to coinbase maturity duration Both constants have the same implicit assumption: that the blockchain will very rarely undergo a reorganisation of that size. --- src/wallet/wallet.cpp | 1 - src/wallet/wallet.h | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7f6177337..f0ac15df3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -8,7 +8,6 @@ #include "base58.h" #include "checkpoints.h" #include "coincontrol.h" -#include "consensus/consensus.h" #include "consensus/validation.h" #include "init.h" #include "main.h" diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 46d285010..231c4aa49 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -8,6 +8,7 @@ #include "amount.h" #include "coins.h" +#include "consensus/consensus.h" #include "key.h" #include "keystore.h" #include "primitives/block.h" @@ -55,7 +56,7 @@ static const CAmount nHighTransactionMaxFeeWarning = 100 * nHighTransactionFeeWa static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000; //! Size of witness cache // Should be large enough that we can expect to never reorg beyond our cache. -static const unsigned int WITNESS_CACHE_SIZE = 50; +static const unsigned int WITNESS_CACHE_SIZE = COINBASE_MATURITY; class CAccountingEntry; class CBlockIndex; From 1551db870a8b92bd657dbdca50810267f82e1bec Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 18:28:00 +1200 Subject: [PATCH 13/21] Add transactions to wallet if we spend notes in them --- src/wallet/gtest/test_wallet.cpp | 28 ++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 24 +++++++++++++++++++++++- src/wallet/wallet.h | 1 + 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index d06406e27..cf418a596 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -291,6 +291,34 @@ TEST(wallet_tests, navigate_from_nullifier_to_note) { EXPECT_EQ(1, wallet.mapNullifiersToNotes[nullifier].n); } +TEST(wallet_tests, spent_note_is_from_me) { + CWallet wallet; + + auto sk = libzcash::SpendingKey::random(); + wallet.AddSpendingKey(sk); + + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + auto wtx2 = GetValidSpend(sk, note, 5); + + EXPECT_FALSE(wallet.IsFromMe(wtx)); + EXPECT_FALSE(wallet.IsFromMe(wtx2)); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + noteData[jsoutpt] = nd; + + wtx.SetNoteData(noteData); + EXPECT_FALSE(wallet.IsFromMe(wtx)); + EXPECT_FALSE(wallet.IsFromMe(wtx2)); + + wallet.AddToWallet(wtx, true, NULL); + EXPECT_FALSE(wallet.IsFromMe(wtx)); + EXPECT_TRUE(wallet.IsFromMe(wtx2)); +} + TEST(wallet_tests, cached_witnesses_empty_chain) { TestWallet wallet; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f0ac15df3..fe2771e66 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1060,6 +1060,18 @@ mapNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const return noteData; } +bool CWallet::IsFromMe(const uint256& nullifier) const +{ + { + LOCK(cs_wallet); + if (mapNullifiersToNotes.count(nullifier) && + mapWallet.count(mapNullifiersToNotes.at(nullifier).hash)) { + return true; + } + } + return false; +} + void CWallet::GetNoteWitnesses(std::vector notes, std::vector>& witnesses, uint256 &final_anchor) @@ -1171,7 +1183,17 @@ bool CWallet::IsMine(const CTransaction& tx) const bool CWallet::IsFromMe(const CTransaction& tx) const { - return (GetDebit(tx, ISMINE_ALL) > 0); + if (GetDebit(tx, ISMINE_ALL) > 0) { + return true; + } + for (const JSDescription& jsdesc : tx.vjoinsplit) { + for (const uint256& nullifier : jsdesc.nullifiers) { + if (IsFromMe(nullifier)) { + return true; + } + } + } + return false; } CAmount CWallet::GetDebit(const CTransaction& tx, const isminefilter& filter) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 231c4aa49..7fa88ab1d 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -795,6 +795,7 @@ public: std::set GetAccountAddresses(std::string strAccount) const; mapNoteData_t FindMyNotes(const CTransaction& tx) const; + bool IsFromMe(const uint256& nullifier) const; void GetNoteWitnesses( std::vector notes, std::vector>& witnesses, From be86b6c3324fffffd32386966b853565c3bcdd39 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 18:57:32 +1200 Subject: [PATCH 14/21] Add test for GetNoteDecryptor() --- src/gtest/test_keystore.cpp | 12 ++++++++++++ src/zcash/NoteEncryption.hpp | 1 + 2 files changed, 13 insertions(+) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index 8587ba49b..a5a952e9a 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -27,3 +27,15 @@ TEST(keystore_tests, store_and_retrieve_spending_key) { EXPECT_EQ(1, addrs.size()); EXPECT_EQ(1, addrs.count(addr)); } + +TEST(keystore_tests, store_and_retrieve_note_decryptor) { + CBasicKeyStore keyStore; + ZCNoteDecryption decOut; + + auto sk = libzcash::SpendingKey::random(); + auto addr = sk.address(); + keyStore.AddSpendingKey(sk); + + keyStore.GetNoteDecryptor(addr, decOut); + EXPECT_EQ(ZCNoteDecryption(sk.viewing_key()), decOut); +} diff --git a/src/zcash/NoteEncryption.hpp b/src/zcash/NoteEncryption.hpp index 1cd1a9b27..bfeb80efa 100644 --- a/src/zcash/NoteEncryption.hpp +++ b/src/zcash/NoteEncryption.hpp @@ -71,6 +71,7 @@ public: unsigned char nonce ) const; + friend inline bool operator==(const NoteDecryption& a, const NoteDecryption& b) { return a.sk_enc == b.sk_enc && a.pk_enc == b.pk_enc; } friend inline bool operator<(const NoteDecryption& a, const NoteDecryption& b) { return a.pk_enc < b.pk_enc; } }; From 0736fa14fccddaab949cf3114ce9075341624e01 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 31 Aug 2016 21:16:59 +1200 Subject: [PATCH 15/21] Keep any existing cached witnesses when updating transactions --- src/wallet/wallet.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fe2771e66..438de40db 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -917,7 +917,16 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD } if (!wtxIn.mapNoteData.empty() && wtxIn.mapNoteData != wtx.mapNoteData) { - wtx.mapNoteData = wtxIn.mapNoteData; + auto tmp = wtxIn.mapNoteData; + // Ensure we keep any cached witnesses we may already have + for (const std::pair nd : wtx.mapNoteData) { + if (tmp.count(nd.first) && nd.second.witnesses.size() > 0) { + tmp.at(nd.first).witnesses.assign( + nd.second.witnesses.cbegin(), nd.second.witnesses.cend()); + } + } + // Now copy over the updated note data + wtx.mapNoteData = tmp; fUpdated = true; } if (wtxIn.fFromMe && wtxIn.fFromMe != wtx.fFromMe) From 32a103aab7eb343b9b84b070dd36ac9c08b9be07 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 1 Sep 2016 11:38:43 +1200 Subject: [PATCH 16/21] Changes after review --- src/keystore.cpp | 2 +- src/wallet/gtest/test_wallet.cpp | 2 +- src/wallet/wallet.cpp | 10 ++++++++-- src/wallet/wallet.h | 3 ++- src/zcash/Address.hpp | 9 +++++++-- src/zcash/NoteEncryption.hpp | 10 +++++++--- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/keystore.cpp b/src/keystore.cpp index 251e47e26..f32ba0c32 100644 --- a/src/keystore.cpp +++ b/src/keystore.cpp @@ -89,6 +89,6 @@ bool CBasicKeyStore::AddSpendingKey(const libzcash::SpendingKey &sk) LOCK(cs_SpendingKeyStore); auto address = sk.address(); mapSpendingKeys[address] = sk; - mapNoteDecryptors[address] = ZCNoteDecryption(sk.viewing_key()); + mapNoteDecryptors.insert(std::make_pair(address, ZCNoteDecryption(sk.viewing_key()))); return true; } diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index cf418a596..289bd265f 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -172,7 +172,7 @@ TEST(wallet_tests, set_invalid_note_addrs_in_cwallettx) { CNoteData nd {sk.address(), uint256()}; noteData[jsoutpt] = nd; - EXPECT_THROW(wtx.SetNoteData(noteData), std::runtime_error); + EXPECT_THROW(wtx.SetNoteData(noteData), std::logic_error); } TEST(wallet_tests, find_note_in_tx) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 438de40db..3407df53e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -601,6 +601,8 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, for (std::pair& wtxItem : mapWallet) { for (mapNoteData_t::value_type& item : wtxItem.second.mapNoteData) { CNoteData* nd = &(item.second); + // Check the validity of the cache + assert(nWitnessCacheSize >= nd->witnesses.size()); // Copy the witness for the previous block if we have one if (nd->witnesses.size() > 0) { nd->witnesses.push_front(nd->witnesses.front()); @@ -1060,8 +1062,12 @@ mapNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const CNoteData nd {address, note.nullifier(key)}; noteData.insert(std::make_pair(jsoutpt, nd)); break; - } catch (const std::exception &) { + } catch (const std::runtime_error &) { // Couldn't decrypt with this spending key + } catch (const std::exception &exc) { + // Unexpected failure + LogPrintf("FindMyNotes(): Unexpected error while testing decrypt:\n"); + LogPrintf("%s\n", exc.what()); } } } @@ -1252,7 +1258,7 @@ void CWalletTx::SetNoteData(mapNoteData_t ¬eData) } else { // If FindMyNotes() was used to obtain noteData, // this should never happen - throw std::runtime_error("CWalletTc::SetNoteData(): Invalid note"); + throw std::logic_error("CWalletTx::SetNoteData(): Invalid note"); } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7fa88ab1d..b1c816fac 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -55,7 +55,8 @@ static const CAmount nHighTransactionMaxFeeWarning = 100 * nHighTransactionFeeWa //! Largest (in bytes) free transaction we're willing to create static const unsigned int MAX_FREE_TRANSACTION_CREATE_SIZE = 1000; //! Size of witness cache -// Should be large enough that we can expect to never reorg beyond our cache. +// Should be large enough that we can expect not to reorg beyond our cache +// unless there is some exceptional network disruption. static const unsigned int WITNESS_CACHE_SIZE = COINBASE_MATURITY; class CAccountingEntry; diff --git a/src/zcash/Address.hpp b/src/zcash/Address.hpp index d967aef27..58caae772 100644 --- a/src/zcash/Address.hpp +++ b/src/zcash/Address.hpp @@ -23,8 +23,13 @@ public: READWRITE(pk_enc); } - friend inline bool operator==(const PaymentAddress& a, const PaymentAddress& b) { return a.a_pk == b.a_pk && a.pk_enc == b.pk_enc; } - friend inline bool operator<(const PaymentAddress& a, const PaymentAddress& b) { return a.a_pk < b.a_pk; } + friend inline bool operator==(const PaymentAddress& a, const PaymentAddress& b) { + return a.a_pk == b.a_pk && a.pk_enc == b.pk_enc; + } + friend inline bool operator<(const PaymentAddress& a, const PaymentAddress& b) { + return (a.a_pk < b.a_pk || + (a.a_pk == b.a_pk && a.pk_enc < b.pk_enc)); + } }; class ViewingKey : public uint256 { diff --git a/src/zcash/NoteEncryption.hpp b/src/zcash/NoteEncryption.hpp index bfeb80efa..e1f3718b0 100644 --- a/src/zcash/NoteEncryption.hpp +++ b/src/zcash/NoteEncryption.hpp @@ -61,7 +61,6 @@ public: typedef boost::array Ciphertext; typedef boost::array Plaintext; - // Unused default constructor to make allocators happy NoteDecryption() { } NoteDecryption(uint256 sk_enc); @@ -71,8 +70,13 @@ public: unsigned char nonce ) const; - friend inline bool operator==(const NoteDecryption& a, const NoteDecryption& b) { return a.sk_enc == b.sk_enc && a.pk_enc == b.pk_enc; } - friend inline bool operator<(const NoteDecryption& a, const NoteDecryption& b) { return a.pk_enc < b.pk_enc; } + friend inline bool operator==(const NoteDecryption& a, const NoteDecryption& b) { + return a.sk_enc == b.sk_enc && a.pk_enc == b.pk_enc; + } + friend inline bool operator<(const NoteDecryption& a, const NoteDecryption& b) { + return (a.sk_enc < b.sk_enc || + (a.sk_enc == b.sk_enc && a.pk_enc < b.pk_enc)); + } }; uint256 random_uint256(); From ac91ebbe9254779d9a9f08b3b0896a23e3e49922 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 1 Sep 2016 12:47:44 +1200 Subject: [PATCH 17/21] Add test showing that the witness cache isn't being serialised --- src/wallet/gtest/test_wallet.cpp | 23 +++++++++++++++++++ src/zcash/IncrementalMerkleTree.hpp | 35 +++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 289bd265f..c47147b4e 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -146,6 +146,29 @@ CWalletTx GetValidSpend(const libzcash::SpendingKey& sk, return wtx; } +TEST(wallet_tests, note_data_serialisation) { + auto sk = libzcash::SpendingKey::random(); + auto wtx = GetValidReceive(sk, 10, true); + auto note = GetNote(sk, wtx, 0, 1); + auto nullifier = note.nullifier(sk); + + mapNoteData_t noteData; + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + CNoteData nd {sk.address(), nullifier}; + ZCIncrementalMerkleTree tree; + nd.witnesses.push_front(tree.witness()); + noteData[jsoutpt] = nd; + + CDataStream ss(SER_DISK, CLIENT_VERSION); + ss << noteData; + + mapNoteData_t noteData2; + ss >> noteData2; + + EXPECT_EQ(noteData, noteData2); + EXPECT_EQ(noteData[jsoutpt].witnesses, noteData2[jsoutpt].witnesses); +} + TEST(wallet_tests, set_note_addrs_in_cwallettx) { auto sk = libzcash::SpendingKey::random(); auto wtx = GetValidReceive(sk, 10, true); diff --git a/src/zcash/IncrementalMerkleTree.hpp b/src/zcash/IncrementalMerkleTree.hpp index ab4913922..419d74840 100644 --- a/src/zcash/IncrementalMerkleTree.hpp +++ b/src/zcash/IncrementalMerkleTree.hpp @@ -43,10 +43,19 @@ public: Hash empty_root(size_t depth) { return empty_roots.at(depth); } + template + friend bool operator==(const EmptyMerkleRoots& a, + const EmptyMerkleRoots& b); private: boost::array empty_roots; }; +template +bool operator==(const EmptyMerkleRoots& a, + const EmptyMerkleRoots& b) { + return a.empty_roots == b.empty_roots; +} + template class IncrementalWitness; @@ -90,6 +99,10 @@ public: return emptyroots.empty_root(Depth); } + template + friend bool operator==(const IncrementalMerkleTree& a, + const IncrementalMerkleTree& b); + private: static EmptyMerkleRoots emptyroots; boost::optional left; @@ -104,6 +117,15 @@ private: void wfcheck() const; }; +template +bool operator==(const IncrementalMerkleTree& a, + const IncrementalMerkleTree& b) { + return (a.emptyroots == b.emptyroots && + a.left == b.left && + a.right == b.right && + a.parents == b.parents); +} + template class IncrementalWitness { friend class IncrementalMerkleTree; @@ -130,6 +152,10 @@ public: cursor_depth = tree.next_depth(filled.size()); } + template + friend bool operator==(const IncrementalWitness& a, + const IncrementalWitness& b); + private: IncrementalMerkleTree tree; std::vector filled; @@ -139,6 +165,15 @@ private: IncrementalWitness(IncrementalMerkleTree tree) : tree(tree) {} }; +template +bool operator==(const IncrementalWitness& a, + const IncrementalWitness& b) { + return (a.tree == b.tree && + a.filled == b.filled && + a.cursor == b.cursor && + a.cursor_depth == b.cursor_depth); +} + class SHA256Compress : public uint256 { public: SHA256Compress() : uint256() {} From 5abaca1af6870a528a16a4707d149030a9500b36 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 1 Sep 2016 13:00:02 +1200 Subject: [PATCH 18/21] Fix the failing test! --- src/wallet/wallet.h | 1 + src/zcash/IncrementalMerkleTree.hpp | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index b1c816fac..5b55e5c07 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -222,6 +222,7 @@ public: inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { READWRITE(address); READWRITE(nullifier); + READWRITE(witnesses); } friend bool operator<(const CNoteData& a, const CNoteData& b) { diff --git a/src/zcash/IncrementalMerkleTree.hpp b/src/zcash/IncrementalMerkleTree.hpp index 419d74840..1d023168a 100644 --- a/src/zcash/IncrementalMerkleTree.hpp +++ b/src/zcash/IncrementalMerkleTree.hpp @@ -131,6 +131,9 @@ class IncrementalWitness { friend class IncrementalMerkleTree; public: + // Required for Unserialize() + IncrementalWitness() {} + MerklePath path() const { return tree.path(partial_path()); } From 73db0c12b9139f072df91958a2f1c85bb76c8e56 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 1 Sep 2016 14:44:01 +1200 Subject: [PATCH 19/21] Increase coverage of GetNoteDecryptor() --- src/gtest/test_keystore.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/gtest/test_keystore.cpp b/src/gtest/test_keystore.cpp index a5a952e9a..11d967e89 100644 --- a/src/gtest/test_keystore.cpp +++ b/src/gtest/test_keystore.cpp @@ -34,8 +34,10 @@ TEST(keystore_tests, store_and_retrieve_note_decryptor) { auto sk = libzcash::SpendingKey::random(); auto addr = sk.address(); - keyStore.AddSpendingKey(sk); - keyStore.GetNoteDecryptor(addr, decOut); + EXPECT_FALSE(keyStore.GetNoteDecryptor(addr, decOut)); + + keyStore.AddSpendingKey(sk); + EXPECT_TRUE(keyStore.GetNoteDecryptor(addr, decOut)); EXPECT_EQ(ZCNoteDecryption(sk.viewing_key()), decOut); } From 268bd84f9a3d14111257db9da6da4feda2738b82 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 1 Sep 2016 15:04:57 +1200 Subject: [PATCH 20/21] Add coverage of the assertion inside GetNoteWitnesses() --- src/wallet/gtest/test_wallet.cpp | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index c47147b4e..e69eb6e14 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -56,7 +56,7 @@ CWalletTx GetValidReceive(const libzcash::SpendingKey& sk, CAmount value, bool r }; boost::array outputs = { - libzcash::JSOutput(), // dummy output + libzcash::JSOutput(sk.address(), value), libzcash::JSOutput(sk.address(), value) }; @@ -209,7 +209,7 @@ TEST(wallet_tests, find_note_in_tx) { auto nullifier = note.nullifier(sk); auto noteMap = wallet.FindMyNotes(wtx); - EXPECT_EQ(1, noteMap.size()); + EXPECT_EQ(2, noteMap.size()); JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; CNoteData nd {sk.address(), nullifier}; @@ -349,26 +349,33 @@ TEST(wallet_tests, cached_witnesses_empty_chain) { wallet.AddSpendingKey(sk); auto wtx = GetValidReceive(sk, 10, true); - auto note = GetNote(sk, wtx, 0, 1); + auto note = GetNote(sk, wtx, 0, 0); + auto note2 = GetNote(sk, wtx, 0, 1); auto nullifier = note.nullifier(sk); + auto nullifier2 = note2.nullifier(sk); mapNoteData_t noteData; - JSOutPoint jsoutpt {wtx.GetTxid(), 0, 1}; + JSOutPoint jsoutpt {wtx.GetTxid(), 0, 0}; + JSOutPoint jsoutpt2 {wtx.GetTxid(), 0, 1}; CNoteData nd {sk.address(), nullifier}; + CNoteData nd2 {sk.address(), nullifier2}; noteData[jsoutpt] = nd; + noteData[jsoutpt2] = nd2; wtx.SetNoteData(noteData); - std::vector notes {jsoutpt}; + std::vector notes {jsoutpt, jsoutpt2}; std::vector> witnesses; uint256 anchor; wallet.GetNoteWitnesses(notes, witnesses, anchor); EXPECT_FALSE((bool) witnesses[0]); + EXPECT_FALSE((bool) witnesses[1]); wallet.AddToWallet(wtx, true, NULL); witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor); EXPECT_FALSE((bool) witnesses[0]); + EXPECT_FALSE((bool) witnesses[1]); CBlock block; block.vtx.push_back(wtx); @@ -377,11 +384,13 @@ TEST(wallet_tests, cached_witnesses_empty_chain) { witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor); EXPECT_TRUE((bool) witnesses[0]); + EXPECT_TRUE((bool) witnesses[1]); wallet.DecrementNoteWitnesses(); witnesses.clear(); wallet.GetNoteWitnesses(notes, witnesses, anchor); EXPECT_FALSE((bool) witnesses[0]); + EXPECT_FALSE((bool) witnesses[1]); } TEST(wallet_tests, cached_witnesses_chain_tip) { From f7d78fdde1b505374bef0457efc66d2585937c77 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 1 Sep 2016 20:16:18 +1200 Subject: [PATCH 21/21] Fix failing test --- src/wallet/gtest/test_wallet.cpp | 8 +++----- src/wallet/wallet.h | 1 + 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index e69eb6e14..6b35baa2d 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -386,11 +386,9 @@ TEST(wallet_tests, cached_witnesses_empty_chain) { EXPECT_TRUE((bool) witnesses[0]); EXPECT_TRUE((bool) witnesses[1]); - wallet.DecrementNoteWitnesses(); - witnesses.clear(); - wallet.GetNoteWitnesses(notes, witnesses, anchor); - EXPECT_FALSE((bool) witnesses[0]); - EXPECT_FALSE((bool) witnesses[1]); + // Until #1302 is implemented, this should triggger an assertion + EXPECT_DEATH(wallet.DecrementNoteWitnesses(), + "Assertion `nWitnessCacheSize > 0' failed."); } TEST(wallet_tests, cached_witnesses_chain_tip) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 5b55e5c07..b736e6906 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -651,6 +651,7 @@ public: nLastResend = 0; nTimeFirstKey = 0; fBroadcastTransactions = false; + nWitnessCacheSize = 0; } std::map mapNullifiersToNotes;