DragonX/hush3
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Auto merge of #1520 - str4d:1502-cache-nullifiers-on-wallet-unlock, r=str4d

Browse Source 
Cache nullifiers on wallet unlock

Closes #1502
This commit is contained in:
zkbot
2016-10-16 17:28:48 -04:00
parent de064bab95 6e263a5fd3
commit 67ec04e6f4
6 changed files with 435 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
qa/pull-tester/rpc-tests.sh
View File

@@ -12,6 +12,7 @@ export BITCOIND=${REAL_BITCOIND}
testScripts=(
'wallet.py'
'wallet_nullifiers.py'
'listtransactions.py'
'mempool_resurrect_test.py'
'txn_doublespend.py'

162
qa/rpc-tests/wallet_nullifiers.py Executable file
View File

@@ -0,0 +1,162 @@
#!/usr/bin/env python2
# Copyright (c) 2016 The Zcash developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import *
from time import *
class WalletNullifiersTest (BitcoinTestFramework):
def run_test (self):
# add zaddr to node 0
myzaddr0 = self.nodes[0].z_getnewaddress()
# send node 0 taddr to zaddr to get out of coinbase
mytaddr = self.nodes[0].getnewaddress();
recipients = []
recipients.append({"address":myzaddr0, "amount":10.0})
myopid = self.nodes[0].z_sendmany(mytaddr, recipients)
opids = []
opids.append(myopid)
timeout = 120
status = None
for x in xrange(1, timeout):
results = self.nodes[0].z_getoperationresult(opids)
if len(results)==0:
sleep(1)
else:
status = results[0]["status"]
assert_equal("success", status)
mytxid = results[0]["result"]["txid"]
break
self.nodes[0].generate(1)
self.sync_all()
# add zaddr to node 2
myzaddr = self.nodes[2].z_getnewaddress()
# import node 2 zaddr into node 1
myzkey = self.nodes[2].z_exportkey(myzaddr)
self.nodes[1].z_importkey(myzkey)
# encrypt node 1 wallet and wait to terminate
self.nodes[1].encryptwallet("test")
bitcoind_processes[1].wait()
# restart node 1
self.nodes[1] = start_node(1, self.options.tmpdir)
connect_nodes_bi(self.nodes, 0, 1)
connect_nodes_bi(self.nodes, 1, 2)
self.sync_all()
# send node 0 zaddr to note 2 zaddr
recipients = []
recipients.append({"address":myzaddr, "amount":7.0})
myopid = self.nodes[0].z_sendmany(myzaddr0, recipients)
opids = []
opids.append(myopid)
timeout = 120
status = None
for x in xrange(1, timeout):
results = self.nodes[0].z_getoperationresult(opids)
if len(results)==0:
sleep(1)
else:
status = results[0]["status"]
assert_equal("success", status)
mytxid = results[0]["result"]["txid"]
break
self.nodes[0].generate(1)
self.sync_all()
# check zaddr balance
zsendmanynotevalue = Decimal('7.0')
assert_equal(self.nodes[2].z_getbalance(myzaddr), zsendmanynotevalue)
assert_equal(self.nodes[1].z_getbalance(myzaddr), zsendmanynotevalue)
# add zaddr to node 3
myzaddr3 = self.nodes[3].z_getnewaddress()
# send node 2 zaddr to note 3 zaddr
recipients = []
recipients.append({"address":myzaddr3, "amount":2.0})
myopid = self.nodes[2].z_sendmany(myzaddr, recipients)
opids = []
opids.append(myopid)
timeout = 120
status = None
for x in xrange(1, timeout):
results = self.nodes[2].z_getoperationresult(opids)
if len(results)==0:
sleep(1)
else:
status = results[0]["status"]
assert_equal("success", status)
mytxid = results[0]["result"]["txid"]
break
self.nodes[2].generate(1)
self.sync_all()
# check zaddr balance
zsendmany2notevalue = Decimal('2.0')
zsendmanyfee = Decimal('0.0001')
zaddrremaining = zsendmanynotevalue - zsendmany2notevalue - zsendmanyfee
assert_equal(self.nodes[3].z_getbalance(myzaddr3), zsendmany2notevalue)
assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining)
# Parallel encrypted wallet can't cache nullifiers for received notes,
# and therefore can't detect spends. So it sees a balance corresponding
# to the sum of both notes it received (one as change).
# TODO: Devise a way to avoid this issue (#1528)
assert_equal(self.nodes[1].z_getbalance(myzaddr), zsendmanynotevalue + zaddrremaining)
# send node 2 zaddr on node 1 to taddr
# This requires that node 1 be unlocked, which triggers caching of
# uncached nullifiers.
self.nodes[1].walletpassphrase("test", 600)
mytaddr1 = self.nodes[1].getnewaddress();
recipients = []
recipients.append({"address":mytaddr1, "amount":1.0})
myopid = self.nodes[1].z_sendmany(myzaddr, recipients)
opids = []
opids.append(myopid)
timeout = 120
status = None
for x in xrange(1, timeout):
results = self.nodes[1].z_getoperationresult(opids)
if len(results)==0:
sleep(1)
else:
status = results[0]["status"]
assert_equal("success", status)
mytxid = results[0]["result"]["txid"]
break
self.nodes[1].generate(1)
self.sync_all()
# check zaddr balance
# Now that the encrypted wallet has been unlocked, the note nullifiers
# have been cached and spent notes can be detected. Thus the two wallets
# are in agreement once more.
zsendmany3notevalue = Decimal('1.0')
zaddrremaining2 = zaddrremaining - zsendmany3notevalue - zsendmanyfee
assert_equal(self.nodes[1].z_getbalance(myzaddr), zaddrremaining2)
assert_equal(self.nodes[2].z_getbalance(myzaddr), zaddrremaining2)
if __name__ == '__main__':
WalletNullifiersTest().main ()

112
src/wallet/gtest/test_wallet.cpp
View File

@@ -35,6 +35,14 @@ class TestWallet : public CWallet {
public:
TestWallet() : CWallet() { }
bool EncryptKeys(CKeyingMaterial& vMasterKeyIn) {
return CCryptoKeyStore::EncryptKeys(vMasterKeyIn);
}
bool Unlock(const CKeyingMaterial& vMasterKeyIn) {
return CCryptoKeyStore::Unlock(vMasterKeyIn);
}
void IncrementNoteWitnesses(const CBlockIndex* pindex,
const CBlock* pblock,
ZCIncrementalMerkleTree tree) {
@@ -382,12 +390,72 @@ TEST(wallet_tests, set_invalid_note_addrs_in_cwallettx) {
EXPECT_THROW(wtx.SetNoteData(noteData), std::logic_error);
}
TEST(wallet_tests, find_note_in_tx) {
TEST(wallet_tests, GetNoteNullifier) {
CWallet wallet;
auto sk = libzcash::SpendingKey::random();
auto address = sk.address();
auto dec = ZCNoteDecryption(sk.viewing_key());
auto wtx = GetValidReceive(sk, 10, true);
auto note = GetNote(sk, wtx, 0, 1);
auto nullifier = note.nullifier(sk);
auto hSig = wtx.vjoinsplit[0].h_sig(
*params, wtx.joinSplitPubKey);
auto ret = wallet.GetNoteNullifier(
wtx.vjoinsplit[0],
address,
dec,
hSig, 1);
EXPECT_NE(nullifier, ret);
wallet.AddSpendingKey(sk);
ret = wallet.GetNoteNullifier(
wtx.vjoinsplit[0],
address,
dec,
hSig, 1);
EXPECT_EQ(nullifier, ret);
}
TEST(wallet_tests, FindMyNotes) {
CWallet wallet;
auto sk = libzcash::SpendingKey::random();
auto sk2 = libzcash::SpendingKey::random();
wallet.AddSpendingKey(sk2);
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(0, noteMap.size());
wallet.AddSpendingKey(sk);
noteMap = wallet.FindMyNotes(wtx);
EXPECT_EQ(2, noteMap.size());
JSOutPoint jsoutpt {wtx.GetHash(), 0, 1};
CNoteData nd {sk.address(), nullifier};
EXPECT_EQ(1, noteMap.count(jsoutpt));
EXPECT_EQ(nd, noteMap[jsoutpt]);
}
TEST(wallet_tests, FindMyNotesInEncryptedWallet) {
TestWallet wallet;
uint256 r {GetRandHash()};
CKeyingMaterial vMasterKey (r.begin(), r.end());
auto sk = libzcash::SpendingKey::random();
wallet.AddSpendingKey(sk);
ASSERT_TRUE(wallet.EncryptKeys(vMasterKey));
auto wtx = GetValidReceive(sk, 10, true);
auto note = GetNote(sk, wtx, 0, 1);
auto nullifier = note.nullifier(sk);
@@ -398,6 +466,13 @@ TEST(wallet_tests, find_note_in_tx) {
JSOutPoint jsoutpt {wtx.GetHash(), 0, 1};
CNoteData nd {sk.address(), nullifier};
EXPECT_EQ(1, noteMap.count(jsoutpt));
EXPECT_NE(nd, noteMap[jsoutpt]);
ASSERT_TRUE(wallet.Unlock(vMasterKey));
noteMap = wallet.FindMyNotes(wtx);
EXPECT_EQ(2, noteMap.size());
EXPECT_EQ(1, noteMap.count(jsoutpt));
EXPECT_EQ(nd, noteMap[jsoutpt]);
}
@@ -757,6 +832,41 @@ TEST(wallet_tests, WriteWitnessCache) {
wallet.WriteWitnessCache(walletdb);
}
TEST(wallet_tests, UpdateNullifierNoteMap) {
TestWallet wallet;
uint256 r {GetRandHash()};
CKeyingMaterial vMasterKey (r.begin(), r.end());
auto sk = libzcash::SpendingKey::random();
wallet.AddSpendingKey(sk);
ASSERT_TRUE(wallet.EncryptKeys(vMasterKey));
auto wtx = GetValidReceive(sk, 10, true);
auto note = GetNote(sk, wtx, 0, 1);
auto nullifier = note.nullifier(sk);
// Pretend that we called FindMyNotes while the wallet was locked
mapNoteData_t noteData;
JSOutPoint jsoutpt {wtx.GetHash(), 0, 1};
CNoteData nd {sk.address()};
noteData[jsoutpt] = nd;
wtx.SetNoteData(noteData);
wallet.AddToWallet(wtx, true, NULL);
EXPECT_EQ(0, wallet.mapNullifiersToNotes.count(nullifier));
EXPECT_FALSE(wallet.UpdateNullifierNoteMap());
ASSERT_TRUE(wallet.Unlock(vMasterKey));
EXPECT_TRUE(wallet.UpdateNullifierNoteMap());
EXPECT_EQ(1, wallet.mapNullifiersToNotes.count(nullifier));
EXPECT_EQ(wtx.GetHash(), wallet.mapNullifiersToNotes[nullifier].hash);
EXPECT_EQ(0, wallet.mapNullifiersToNotes[nullifier].js);
EXPECT_EQ(1, wallet.mapNullifiersToNotes[nullifier].n);
}
TEST(wallet_tests, UpdatedNoteData) {
TestWallet wallet;

2
src/wallet/rpcwallet.cpp
View File

@@ -1877,6 +1877,8 @@ Value walletpassphrase(const Array& params, bool fHelp)
"walletpassphrase <passphrase> <timeout>\n"
"Stores the wallet decryption key in memory for <timeout> seconds.");
// No need to check return values, because the wallet was unlocked above
pwalletMain->UpdateNullifierNoteMap();
pwalletMain->TopUpKeyPool();
int64_t nSleepTime = params[1].get_int64();

107
src/wallet/wallet.cpp
View File

@@ -863,12 +863,50 @@ void CWallet::MarkDirty()
}
}
void CWallet::UpdateNullifierNoteMap(const CWalletTx& wtx)
/**
* Ensure that every note in the wallet has a cached nullifier.
*/
bool CWallet::UpdateNullifierNoteMap()
{
{
LOCK(cs_wallet);
if (IsLocked())
return false;
ZCNoteDecryption dec;
for (std::pair<const uint256, CWalletTx>& wtxItem : mapWallet) {
for (mapNoteData_t::value_type& item : wtxItem.second.mapNoteData) {
if (!item.second.nullifier) {
auto i = item.first.js;
GetNoteDecryptor(item.second.address, dec);
auto hSig = wtxItem.second.vjoinsplit[i].h_sig(
*pzcashParams, wtxItem.second.joinSplitPubKey);
item.second.nullifier = GetNoteNullifier(
wtxItem.second.vjoinsplit[i],
item.second.address,
dec,
hSig,
item.first.n);
}
}
UpdateNullifierNoteMapWithTx(wtxItem.second);
}
}
return true;
}
/**
* Update mapNullifiersToNotes with the cached nullifiers in this tx.
*/
void CWallet::UpdateNullifierNoteMapWithTx(const CWalletTx& wtx)
{
{
LOCK(cs_wallet);
for (const mapNoteData_t::value_type& item : wtx.mapNoteData) {
mapNullifiersToNotes[item.second.nullifier] = item.first;
if (item.second.nullifier) {
mapNullifiersToNotes[*item.second.nullifier] = item.first;
}
}
}
}
@@ -881,7 +919,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD
{
mapWallet[hash] = wtxIn;
mapWallet[hash].BindWallet(this);
UpdateNullifierNoteMap(mapWallet[hash]);
UpdateNullifierNoteMapWithTx(mapWallet[hash]);
AddToSpends(hash);
}
else
@@ -891,7 +929,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD
pair<map<uint256, CWalletTx>::iterator, bool> ret = mapWallet.insert(make_pair(hash, wtxIn));
CWalletTx& wtx = (*ret.first).second;
wtx.BindWallet(this);
UpdateNullifierNoteMap(wtx);
UpdateNullifierNoteMapWithTx(wtx);
bool fInsertedNew = ret.second;
if (fInsertedNew)
{
@@ -1092,6 +1130,32 @@ void CWallet::EraseFromWallet(const uint256 &hash)
}
/**
* Returns a nullifier if the SpendingKey is available
* Throws std::runtime_error if the decryptor doesn't match this note
*/
boost::optional<uint256> CWallet::GetNoteNullifier(const JSDescription& jsdesc,
const libzcash::PaymentAddress& address,
const ZCNoteDecryption& dec,
const uint256& hSig,
uint8_t n) const
{
boost::optional<uint256> ret;
auto note_pt = libzcash::NotePlaintext::decrypt(
dec,
jsdesc.ciphertexts[n],
jsdesc.ephemeralKey,
hSig,
(unsigned char) n);
auto note = note_pt.note(address);
// SpendingKeys are only available if the wallet is unlocked
libzcash::SpendingKey key;
if (GetSpendingKey(address, key)) {
ret = note.nullifier(key);
}
return ret;
}
/**
* Finds all output notes in the given transaction that have been sent to
* PaymentAddresses in this wallet.
@@ -1106,28 +1170,33 @@ mapNoteData_t CWallet::FindMyNotes(const CTransaction& tx) const
uint256 hash = tx.GetHash();
mapNoteData_t noteData;
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 : mapNoteDecryptors) {
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));
auto nullifier = GetNoteNullifier(
tx.vjoinsplit[i],
address,
item.second,
hSig, j);
if (nullifier) {
CNoteData nd {address, *nullifier};
noteData.insert(std::make_pair(jsoutpt, nd));
} else {
CNoteData nd {address};
noteData.insert(std::make_pair(jsoutpt, nd));
}
break;
} catch (const std::runtime_error &) {
// Couldn't decrypt with this spending key
} catch (const std::runtime_error &err) {
if (memcmp("Could not decrypt message", err.what(), 25) != 0) {
// Unexpected failure
LogPrintf("FindMyNotes(): Unexpected runtime error while testing decrypt:\n");
LogPrintf("%s\n", err.what());
} // else
// Couldn't decrypt with this decryptor
} catch (const std::exception &exc) {
// Unexpected failure
LogPrintf("FindMyNotes(): Unexpected error while testing decrypt:\n");
@@ -3335,7 +3404,7 @@ void CWallet::GetFilteredNotes(std::vector<CNotePlaintextEntry> & outEntries, st
}
// skip note which has been spent
if (ignoreSpent && IsSpent(nd.nullifier)) {
if (ignoreSpent && nd.nullifier && IsSpent(*nd.nullifier)) {
continue;
}

78
src/wallet/wallet.h
View File

@@ -201,12 +201,19 @@ 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;
/**
* Cached note nullifier. May not be set if the wallet was not unlocked when
* this was CNoteData was created. If not set, we always assume that the
* note has not been spent.
*
* 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 the wallet is encrypted, this means that someone with access to the
* locked wallet cannot spend notes, but can connect received notes to the
* transactions they are spent in. This is the same security semantics as
* for transparent addresses.
*/
boost::optional<uint256> nullifier;
/**
* Cached incremental witnesses for spendable Notes.
@@ -215,6 +222,7 @@ public:
std::list<ZCIncrementalWitness> witnesses;
CNoteData() : address(), nullifier() { }
CNoteData(libzcash::PaymentAddress a) : address {a}, nullifier() { }
CNoteData(libzcash::PaymentAddress a, uint256 n) : address {a}, nullifier {n} { }
ADD_SERIALIZE_METHODS;
@@ -704,7 +712,56 @@ public:
nWitnessCacheSize = 0;
}
/**
* The reverse mapping of nullifiers to notes.
*
* The mapping cannot be updated while an encrypted wallet is locked,
* because we need the SpendingKey to create the nullifier (#1502). This has
* several implications for transactions added to the wallet while locked:
*
* - Parent transactions can't be marked dirty when a child transaction that
* spends their output notes is updated.
*
* - We currently don't cache any note values, so this is not a problem,
* yet.
*
* - GetFilteredNotes can't filter out spent notes.
*
* - Per the comment in CNoteData, we assume that if we don't have a
* cached nullifier, the note is not spent.
*
* Another more problematic implication is that the wallet can fail to
* detect transactions on the blockchain that spend our notes. There are two
* possible cases in which this could happen:
*
* - We receive a note when the wallet is locked, and then spend it using a
* different wallet client.
*
* - We spend from a PaymentAddress we control, then we export the
* SpendingKey and import it into a new wallet, and reindex/rescan to find
* the old transactions.
*
* The wallet will only miss "pure" spends - transactions that are only
* linked to us by the fact that they contain notes we spent. If it also
* sends notes to us, or interacts with our transparent addresses, we will
* detect the transaction and add it to the wallet (again without caching
* nullifiers for new notes). As by default JoinSplits send change back to
* the origin PaymentAddress, the wallet should rarely miss transactions.
*
* To work around these issues, whenever the wallet is unlocked, we scan all
* cached notes, and cache any missing nullifiers. Since the wallet must be
* unlocked in order to spend notes, this means that GetFilteredNotes will
* always behave correctly within that context (and any other uses will give
* correct responses afterwards), for the transactions that the wallet was
* able to detect. Any missing transactions can be rediscovered by:
*
* - Unlocking the wallet (to fill all nullifier caches).
*
* - Restarting the node with -reindex (which operates on a locked wallet
* but with the now-cached nullifiers).
*/
std::map<uint256, JSOutPoint> mapNullifiersToNotes;
std::map<uint256, CWalletTx> mapWallet;
int64_t nOrderPosNext;
@@ -810,7 +867,8 @@ public:
TxItems OrderedTxItems(std::list<CAccountingEntry>& acentries, std::string strAccount = "");
void MarkDirty();
void UpdateNullifierNoteMap(const CWalletTx& wtx);
bool UpdateNullifierNoteMap();
void UpdateNullifierNoteMapWithTx(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);
@@ -850,6 +908,12 @@ public:
std::set<CTxDestination> GetAccountAddresses(std::string strAccount) const;
boost::optional<uint256> GetNoteNullifier(
const JSDescription& jsdesc,
const libzcash::PaymentAddress& address,
const ZCNoteDecryption& dec,
const uint256& hSig,
uint8_t n) const;
mapNoteData_t FindMyNotes(const CTransaction& tx) const;
bool IsFromMe(const uint256& nullifier) const;
void GetNoteWitnesses(