From 6581970d59210c0209d9d616769cded933dbae9c Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 10:59:48 -0600 Subject: [PATCH 01/21] Add implementations of PRF_expand calls that obtain esk and rcm. --- src/zcash/prf.cpp | 16 ++++++++++++++++ src/zcash/prf.h | 2 ++ 2 files changed, 18 insertions(+) diff --git a/src/zcash/prf.cpp b/src/zcash/prf.cpp index 2491de83e..1aa8142d8 100644 --- a/src/zcash/prf.cpp +++ b/src/zcash/prf.cpp @@ -24,6 +24,22 @@ std::array PRF_expand(const uint256& sk, unsigned char t) return res; } +uint256 PRF_rcm(const uint256& rseed) +{ + uint256 rcm; + auto tmp = PRF_expand(rseed, 4); + librustzcash_to_scalar(tmp.data(), rcm.begin()); + return rcm; +} + +uint256 PRF_esk(const uint256& rseed) +{ + uint256 esk; + auto tmp = PRF_expand(rseed, 5); + librustzcash_to_scalar(tmp.data(), esk.begin()); + return esk; +} + uint256 PRF_ask(const uint256& sk) { uint256 ask; diff --git a/src/zcash/prf.h b/src/zcash/prf.h index f666cfa23..b9256769a 100644 --- a/src/zcash/prf.h +++ b/src/zcash/prf.h @@ -22,6 +22,8 @@ uint256 PRF_rho(const uint252& phi, size_t i0, const uint256& h_sig); uint256 PRF_ask(const uint256& sk); uint256 PRF_nsk(const uint256& sk); uint256 PRF_ovk(const uint256& sk); +uint256 PRF_rcm(const uint256& rseed); +uint256 PRF_esk(const uint256& rseed); std::array default_diversifier(const uint256& sk); From 1c2c6be872699e9be6d7f91aab1a5c3f9d329375 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 11:36:35 -0600 Subject: [PATCH 02/21] Remove bare SaplingNote constructor. --- src/wallet/test/rpc_wallet_tests.cpp | 2 +- src/zcash/Note.hpp | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 621d7314b..6707c065f 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -1837,7 +1837,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_parameters) std::vector sproutNoteInputs = {MergeToAddressInputSproutNote{JSOutPoint(), SproutNote(), 0, SproutSpendingKey()}}; std::vector saplingNoteInputs = - {MergeToAddressInputSaplingNote{SaplingOutPoint(), SaplingNote(), 0, SaplingExpandedSpendingKey()}}; + {MergeToAddressInputSaplingNote{SaplingOutPoint(), SaplingNote({}, uint256(), 0, uint256()), 0, SaplingExpandedSpendingKey()}}; // Sprout and Sapling inputs -> throw try { diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index dee17bfe4..357e27e48 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -50,8 +50,6 @@ public: SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 r) : BaseNote(value), d(d), pk_d(pk_d), r(r) {} - SaplingNote() {}; - SaplingNote(const SaplingPaymentAddress &address, uint64_t value); virtual ~SaplingNote() {}; From e71d2fba9da0675ecb0df565d7d1e81739a3a541 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 13:18:20 -0600 Subject: [PATCH 03/21] Add a getter method to obtain rcm from a Sapling note plaintext. --- src/gtest/test_noteencryption.cpp | 4 ++-- src/zcash/Note.cpp | 14 ++++++++++---- src/zcash/Note.hpp | 7 +++++-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index d66580ac9..fca0ae1a8 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -78,7 +78,7 @@ TEST(noteencryption, NotePlaintext) ASSERT_TRUE(bar.value() == pt.value()); ASSERT_TRUE(bar.memo() == pt.memo()); ASSERT_TRUE(bar.d == pt.d); - ASSERT_TRUE(bar.rcm == pt.rcm); + ASSERT_TRUE(bar.rcm() == pt.rcm()); auto foobar = bar.note(ivk); @@ -155,7 +155,7 @@ TEST(noteencryption, NotePlaintext) ASSERT_TRUE(bar.value() == pt.value()); ASSERT_TRUE(bar.memo() == pt.memo()); ASSERT_TRUE(bar.d == pt.d); - ASSERT_TRUE(bar.rcm == pt.rcm); + ASSERT_TRUE(bar.rcm() == pt.rcm()); } TEST(noteencryption, SaplingApi) diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 5f8b439fb..a315f428a 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -145,7 +145,7 @@ SaplingNotePlaintext::SaplingNotePlaintext( std::array memo) : BaseNotePlaintext(note, memo) { d = note.d; - rcm = note.r; + rseed = note.r; } @@ -153,7 +153,7 @@ boost::optional SaplingNotePlaintext::note(const SaplingIncomingVie { auto addr = ivk.address(d); if (addr) { - return SaplingNote(d, addr.get().pk_d, value_, rcm); + return SaplingNote(d, addr.get().pk_d, value_, rcm()); } else { return boost::none; } @@ -221,11 +221,12 @@ boost::optional SaplingNotePlaintext::decrypt( } uint256 cmu_expected; + uint256 rcm = ret.rcm(); if (!librustzcash_sapling_compute_cm( ret.d.data(), pk_d.begin(), ret.value(), - ret.rcm.begin(), + rcm.begin(), cmu_expected.begin() )) { @@ -266,11 +267,12 @@ boost::optional SaplingNotePlaintext::decrypt( } uint256 cmu_expected; + uint256 rcm = ret.rcm(); if (!librustzcash_sapling_compute_cm( ret.d.data(), pk_d.begin(), ret.value(), - ret.rcm.begin(), + rcm.begin(), cmu_expected.begin() )) { @@ -325,3 +327,7 @@ SaplingOutCiphertext SaplingOutgoingPlaintext::encrypt( return enc.encrypt_to_ourselves(ovk, cv, cm, pt); } + +uint256 SaplingNotePlaintext::rcm() const { + return rseed; +} diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index 357e27e48..a7c3023c0 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -117,9 +117,10 @@ public: typedef std::pair SaplingNotePlaintextEncryptionResult; class SaplingNotePlaintext : public BaseNotePlaintext { +private: + uint256 rseed; public: diversifier_t d; - uint256 rcm; SaplingNotePlaintext() {} @@ -157,11 +158,13 @@ public: READWRITE(d); // 11 bytes READWRITE(value_); // 8 bytes - READWRITE(rcm); // 32 bytes + READWRITE(rseed); // 32 bytes READWRITE(memo_); // 512 bytes } boost::optional encrypt(const uint256& pk_d) const; + + uint256 rcm() const; }; class SaplingOutgoingPlaintext From 8770a5c53234831f9d814cb63f1e836db6478a44 Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 14:53:12 -0600 Subject: [PATCH 04/21] Add support for receiving v2 Sapling note plaintexts. Co-authored by Ying Tong (yingtong@electriccoin.co) --- src/gtest/test_checktransaction.cpp | 4 +- src/gtest/test_noteencryption.cpp | 12 ++-- src/gtest/test_sapling_note.cpp | 8 +-- src/gtest/test_transaction_builder.cpp | 2 +- src/miner.cpp | 2 +- src/transaction_builder.cpp | 8 ++- src/utiltest.cpp | 2 +- src/wallet/gtest/test_wallet.cpp | 6 +- src/zcash/Note.cpp | 85 +++++++++++++++++++++++--- src/zcash/Note.hpp | 26 +++++--- src/zcash/NoteEncryption.cpp | 15 +++-- src/zcash/NoteEncryption.hpp | 2 +- src/zcbenchmarks.cpp | 10 +-- 13 files changed, 134 insertions(+), 48 deletions(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 9b0ff0ae6..233dcb03c 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -1134,7 +1134,7 @@ TEST(CheckTransaction, HeartwoodAcceptsShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456)); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), 0x01); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); @@ -1217,7 +1217,7 @@ TEST(CheckTransaction, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456)); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), 0x01); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); CMutableTransaction mtx = GetValidTransaction(); diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index fca0ae1a8..083bdeba1 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -34,7 +34,7 @@ TEST(noteencryption, NotePlaintext) memo[i] = (unsigned char) i; } - SaplingNote note(addr, 39393); + SaplingNote note(addr, 39393, 0x01); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -91,7 +91,7 @@ TEST(noteencryption, NotePlaintext) ASSERT_TRUE(note.value() == new_note.value()); ASSERT_TRUE(note.d == new_note.d); ASSERT_TRUE(note.pk_d == new_note.pk_d); - ASSERT_TRUE(note.r == new_note.r); + ASSERT_TRUE(note.rcm() == new_note.rcm()); ASSERT_TRUE(note.cmu() == new_note.cmu()); SaplingOutgoingPlaintext out_pt; @@ -183,10 +183,10 @@ TEST(noteencryption, SaplingApi) } // Invalid diversifier - ASSERT_EQ(boost::none, SaplingNoteEncryption::FromDiversifier({1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0})); + ASSERT_EQ(boost::none, SaplingNoteEncryption::FromDiversifier({1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, boost::none)); // Encrypt to pk_1 - auto enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d); + auto enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d, boost::none); auto ciphertext_1 = *enc.encrypt_to_recipient( pk_1.pk_d, message @@ -208,7 +208,7 @@ TEST(noteencryption, SaplingApi) ); // Encrypt to pk_2 - enc = *SaplingNoteEncryption::FromDiversifier(pk_2.d); + enc = *SaplingNoteEncryption::FromDiversifier(pk_2.d, boost::none); auto ciphertext_2 = *enc.encrypt_to_recipient( pk_2.pk_d, message @@ -226,7 +226,7 @@ TEST(noteencryption, SaplingApi) // Test nonce-reuse resistance of API { - auto tmp_enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d); + auto tmp_enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d, boost::none); tmp_enc.encrypt_to_recipient( pk_1.pk_d, diff --git a/src/gtest/test_sapling_note.cpp b/src/gtest/test_sapling_note.cpp index b6846217a..b1da05da3 100644 --- a/src/gtest/test_sapling_note.cpp +++ b/src/gtest/test_sapling_note.cpp @@ -57,16 +57,16 @@ TEST(SaplingNote, Random) { // Test creating random notes using the same spending key auto address = SaplingSpendingKey::random().default_address(); - SaplingNote note1(address, GetRand(MAX_MONEY)); - SaplingNote note2(address, GetRand(MAX_MONEY)); + SaplingNote note1(address, GetRand(MAX_MONEY), 0x01); + SaplingNote note2(address, GetRand(MAX_MONEY), 0x01); ASSERT_EQ(note1.d, note2.d); ASSERT_EQ(note1.pk_d, note2.pk_d); ASSERT_NE(note1.value(), note2.value()); - ASSERT_NE(note1.r, note2.r); + ASSERT_NE(note1.rcm(), note2.rcm()); // Test diversifier and pk_d are not the same for different spending keys - SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY)); + SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY), 0x01); ASSERT_NE(note1.d, note3.d); ASSERT_NE(note1.pk_d, note3.pk_d); } diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index d97d0287e..3cc382265 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -482,7 +482,7 @@ TEST(TransactionBuilder, CheckSaplingTxVersion) } // Cannot add Sapling spends to a non-Sapling transaction - libzcash::SaplingNote note(pk, 50000); + libzcash::SaplingNote note(pk, 50000, 0x01); SaplingMerkleTree tree; try { builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); diff --git a/src/miner.cpp b/src/miner.cpp index 89f52175a..489e09dbc 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -157,7 +157,7 @@ public: mtx.valueBalance = -value; uint256 ovk; - auto note = libzcash::SaplingNote(pa, value); + auto note = libzcash::SaplingNote(pa, value, 0x01); // TODO auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 811db2740..07b9087ff 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -43,11 +43,12 @@ boost::optional OutputDescriptionInfo::Build(void* ctx) { std::vector addressBytes(ss.begin(), ss.end()); OutputDescription odesc; + uint256 rcm = this->note.rcm(); if (!librustzcash_sapling_output_proof( ctx, encryptor.get_esk().begin(), addressBytes.data(), - this->note.r.begin(), + rcm.begin(), this->note.value(), odesc.cv.begin(), odesc.zkproof.begin())) { @@ -161,7 +162,7 @@ void TransactionBuilder::AddSaplingOutput( throw std::runtime_error("TransactionBuilder cannot add Sapling output to pre-Sapling transaction"); } - auto note = libzcash::SaplingNote(to, value); + auto note = libzcash::SaplingNote(to, value, 0x01); // TODO outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } @@ -324,12 +325,13 @@ TransactionBuilderResult TransactionBuilder::Build() std::vector witness(ss.begin(), ss.end()); SpendDescription sdesc; + uint256 rcm = spend.note.rcm(); if (!librustzcash_sapling_spend_proof( ctx, spend.expsk.full_viewing_key().ak.begin(), spend.expsk.nsk.begin(), spend.note.d.data(), - spend.note.r.begin(), + rcm.begin(), spend.alpha.begin(), spend.note.value(), spend.anchor.begin(), diff --git a/src/utiltest.cpp b/src/utiltest.cpp index bcb47abeb..bb75a9504 100644 --- a/src/utiltest.cpp +++ b/src/utiltest.cpp @@ -289,7 +289,7 @@ CKey AddTestCKeyToKeyStore(CBasicKeyStore& keyStore) { TestSaplingNote GetTestSaplingNote(const libzcash::SaplingPaymentAddress& pa, CAmount value) { // Generate dummy Sapling note - libzcash::SaplingNote note(pa, value); + libzcash::SaplingNote note(pa, value, 0x01); uint256 cm = note.cmu().get(); SaplingMerkleTree tree; tree.append(cm); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 51f233c7d..18865cbcd 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -387,7 +387,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto ivk = fvk.in_viewing_key(); auto pk = sk.DefaultAddress(); - libzcash::SaplingNote note(pk, 50000); + libzcash::SaplingNote note(pk, 50000, 0x01); auto cm = note.cmu().get(); SaplingMerkleTree tree; tree.append(cm); @@ -654,7 +654,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); // Generate note A - libzcash::SaplingNote note(pk, 50000); + libzcash::SaplingNote note(pk, 50000, 0x01); auto cm = note.cmu().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); @@ -1018,7 +1018,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { auto pk = sk.DefaultAddress(); // Generate Sapling note A - libzcash::SaplingNote note(pk, 50000); + libzcash::SaplingNote note(pk, 50000, 0x01); auto cm = note.cmu().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index a315f428a..76a14f9e6 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -41,20 +41,31 @@ uint256 SproutNote::nullifier(const SproutSpendingKey& a_sk) const { } // Construct and populate Sapling note for a given payment address and value. -SaplingNote::SaplingNote(const SaplingPaymentAddress& address, const uint64_t value) : BaseNote(value) { +SaplingNote::SaplingNote( + const SaplingPaymentAddress& address, + const uint64_t value, + unsigned char _leadByte +) : BaseNote(value) { d = address.d; pk_d = address.pk_d; - librustzcash_sapling_generate_r(r.begin()); + leadByte = _leadByte; + if (leadByte == 0x02) { + // Per ZIP 212, the rseed field is 32 random bytes. + rseed = random_uint256(); + } else { + librustzcash_sapling_generate_r(rseed.begin()); + } } // Call librustzcash to compute the commitment boost::optional SaplingNote::cmu() const { uint256 result; + uint256 rcm_tmp = rcm(); if (!librustzcash_sapling_compute_cm( d.data(), pk_d.begin(), value(), - r.begin(), + rcm_tmp.begin(), result.begin() )) { @@ -71,11 +82,12 @@ boost::optional SaplingNote::nullifier(const SaplingFullViewingKey& vk, auto nk = vk.nk; uint256 result; + uint256 rcm_tmp = rcm(); if (!librustzcash_sapling_compute_nf( d.data(), pk_d.begin(), value(), - r.begin(), + rcm_tmp.begin(), ak.begin(), nk.begin(), position, @@ -145,7 +157,8 @@ SaplingNotePlaintext::SaplingNotePlaintext( std::array memo) : BaseNotePlaintext(note, memo) { d = note.d; - rseed = note.r; + rseed = note.rseed; + leadByte = note.leadByte; } @@ -153,7 +166,9 @@ boost::optional SaplingNotePlaintext::note(const SaplingIncomingVie { auto addr = ivk.address(d); if (addr) { - return SaplingNote(d, addr.get().pk_d, value_, rcm()); + auto tmp = SaplingNote(d, addr.get().pk_d, value_, rseed); + tmp.leadByte = leadByte; + return tmp; } else { return boost::none; } @@ -237,6 +252,19 @@ boost::optional SaplingNotePlaintext::decrypt( return boost::none; } + if (ret.leadByte == 0x02) { + // ZIP 212: Check that epk is consistent to prevent against linkability + // attacks without relying on the soundness of the SNARK. + uint256 expected_epk; + uint256 esk = ret.generate_esk(); + if (!librustzcash_sapling_ka_derivepublic(ret.d.data(), esk.begin(), expected_epk.begin())) { + return boost::none; + } + if (expected_epk != epk) { + return boost::none; + } + } + return ret; } @@ -283,13 +311,31 @@ boost::optional SaplingNotePlaintext::decrypt( return boost::none; } + if (ret.leadByte == 0x02) { + // ZIP 212: Check that epk is consistent to prevent against linkability + // attacks without relying on the soundness of the SNARK. + uint256 expected_epk; + if (!librustzcash_sapling_ka_derivepublic(ret.d.data(), esk.begin(), expected_epk.begin())) { + return boost::none; + } + if (expected_epk != epk) { + return boost::none; + } + + // ZIP 212: Additionally check that the esk provided to this function + // is consistent with the esk we can derive + if (esk != ret.generate_esk()) { + return boost::none; + } + } + return ret; } boost::optional SaplingNotePlaintext::encrypt(const uint256& pk_d) const { // Get the encryptor - auto sne = SaplingNoteEncryption::FromDiversifier(d); + auto sne = SaplingNoteEncryption::FromDiversifier(d, generate_esk()); if (!sne) { return boost::none; } @@ -329,5 +375,28 @@ SaplingOutCiphertext SaplingOutgoingPlaintext::encrypt( } uint256 SaplingNotePlaintext::rcm() const { - return rseed; + if (leadByte == 0x02) { + return PRF_rcm(rseed); + } else { + return rseed; + } +} + +uint256 SaplingNote::rcm() const { + if (leadByte == 0x02) { + return PRF_rcm(rseed); + } else { + return rseed; + } +} + +uint256 SaplingNotePlaintext::generate_esk() const { + if (leadByte == 0x02) { + return PRF_esk(rseed); + } else { + uint256 esk; + // Pick random esk + librustzcash_sapling_generate_r(esk.begin()); + return esk; + } } diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index a7c3023c0..a130267a7 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -42,20 +42,24 @@ public: class SaplingNote : public BaseNote { +private: + uint256 rseed; + friend class SaplingNotePlaintext; + unsigned char leadByte; public: diversifier_t d; uint256 pk_d; - uint256 r; - SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 r) - : BaseNote(value), d(d), pk_d(pk_d), r(r) {} + SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed) + : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed) {} - SaplingNote(const SaplingPaymentAddress &address, uint64_t value); + SaplingNote(const SaplingPaymentAddress &address, uint64_t value, unsigned char leadByte); virtual ~SaplingNote() {}; boost::optional cmu() const; boost::optional nullifier(const SaplingFullViewingKey &vk, const uint64_t position) const; + uint256 rcm() const; }; class BaseNotePlaintext { @@ -89,10 +93,10 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - unsigned char leadingByte = 0x00; - READWRITE(leadingByte); + unsigned char leadByte = 0x00; + READWRITE(leadByte); - if (leadingByte != 0x00) { + if (leadByte != 0x00) { throw std::ios_base::failure("lead byte of SproutNotePlaintext is not recognized"); } @@ -119,6 +123,7 @@ typedef std::pair SaplingNotePlaint class SaplingNotePlaintext : public BaseNotePlaintext { private: uint256 rseed; + unsigned char leadByte; public: diversifier_t d; @@ -149,10 +154,10 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - unsigned char leadingByte = 0x01; - READWRITE(leadingByte); + READWRITE(leadByte); - if (leadingByte != 0x01) { + if (leadByte != 0x01 && leadByte != 0x02) { + printf("leadByte: %x\n", leadByte); throw std::ios_base::failure("lead byte of SaplingNotePlaintext is not recognized"); } @@ -165,6 +170,7 @@ public: boost::optional encrypt(const uint256& pk_d) const; uint256 rcm() const; + uint256 generate_esk() const; }; class SaplingOutgoingPlaintext diff --git a/src/zcash/NoteEncryption.cpp b/src/zcash/NoteEncryption.cpp index 63e073265..4af47d42e 100644 --- a/src/zcash/NoteEncryption.cpp +++ b/src/zcash/NoteEncryption.cpp @@ -101,12 +101,19 @@ void KDF(unsigned char K[NOTEENCRYPTION_CIPHER_KEYSIZE], namespace libzcash { -boost::optional SaplingNoteEncryption::FromDiversifier(diversifier_t d) { +boost::optional SaplingNoteEncryption::FromDiversifier( + diversifier_t d, + boost::optional esk_provided +) +{ uint256 epk; uint256 esk; - - // Pick random esk - librustzcash_sapling_generate_r(esk.begin()); + if (esk_provided) { + esk = esk_provided.get(); + } else { + // Pick random esk + librustzcash_sapling_generate_r(esk.begin()); + } // Compute epk given the diversifier if (!librustzcash_sapling_ka_derivepublic(d.begin(), esk.begin(), epk.begin())) { diff --git a/src/zcash/NoteEncryption.hpp b/src/zcash/NoteEncryption.hpp index f6e692028..51b27be17 100644 --- a/src/zcash/NoteEncryption.hpp +++ b/src/zcash/NoteEncryption.hpp @@ -42,7 +42,7 @@ protected: public: - static boost::optional FromDiversifier(diversifier_t d); + static boost::optional FromDiversifier(diversifier_t d, boost::optional esk); boost::optional encrypt_to_recipient( const uint256 &pk_d, diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index cd728eceb..33378f2db 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -594,7 +594,7 @@ double benchmark_create_sapling_spend() auto sk = libzcash::SaplingSpendingKey::random(); auto expsk = sk.expanded_spending_key(); auto address = sk.default_address(); - SaplingNote note(address, GetRand(MAX_MONEY)); + SaplingNote note(address, GetRand(MAX_MONEY), 0x01); SaplingMerkleTree tree; auto maybe_cmu = note.cmu(); tree.append(maybe_cmu.get()); @@ -618,12 +618,13 @@ double benchmark_create_sapling_spend() timer_start(tv_start); SpendDescription sdesc; + uint256 rcm = note.rcm(); bool result = librustzcash_sapling_spend_proof( ctx, expsk.full_viewing_key().ak.begin(), expsk.nsk.begin(), note.d.data(), - note.r.begin(), + rcm.begin(), alpha.begin(), note.value(), anchor.begin(), @@ -646,7 +647,7 @@ double benchmark_create_sapling_output() auto address = sk.default_address(); std::array memo; - SaplingNote note(address, GetRand(MAX_MONEY)); + SaplingNote note(address, GetRand(MAX_MONEY), 0x01); libzcash::SaplingNotePlaintext notePlaintext(note, memo); auto res = notePlaintext.encrypt(note.pk_d); @@ -667,11 +668,12 @@ double benchmark_create_sapling_output() timer_start(tv_start); OutputDescription odesc; + uint256 rcm = note.rcm(); bool result = librustzcash_sapling_output_proof( ctx, encryptor.get_esk().begin(), addressBytes.data(), - note.r.begin(), + rcm.begin(), note.value(), odesc.cv.begin(), odesc.zkproof.begin()); From bf9baf2cb65d5553924d0a3ca3d83d5837af975f Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 16:02:37 -0600 Subject: [PATCH 05/21] Change transaction builder and miner to use v2 Sapling note plaintexts after Canopy activates. Co-authored by Ying Tong (yingtong@electriccoin.co) --- src/miner.cpp | 6 +++++- src/transaction_builder.cpp | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 489e09dbc..ddcdbf2bd 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -157,7 +157,11 @@ public: mtx.valueBalance = -value; uint256 ovk; - auto note = libzcash::SaplingNote(pa, value, 0x01); // TODO + unsigned char leadByte = 0x01; + if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { + leadByte = 0x02; + } + auto note = libzcash::SaplingNote(pa, value, leadByte); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 07b9087ff..7390932af 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -162,7 +162,11 @@ void TransactionBuilder::AddSaplingOutput( throw std::runtime_error("TransactionBuilder cannot add Sapling output to pre-Sapling transaction"); } - auto note = libzcash::SaplingNote(to, value, 0x01); // TODO + unsigned char leadByte = 0x01; + if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { + leadByte = 0x02; + } + auto note = libzcash::SaplingNote(to, value, leadByte); outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } From 4af761121d6d5743a281c56e0ee48263b048dedd Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 16:10:13 -0600 Subject: [PATCH 06/21] Require that shielded coinbase output note plaintexts are version 2 if Canopy is active. Co-authored by Ying Tong (yingtong@electriccoin.co) --- src/main.cpp | 17 ++++++++++++++--- src/zcash/Note.hpp | 3 +++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index d11ca5bae..cbdd8a18d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -792,6 +792,7 @@ bool ContextualCheckTransaction( bool saplingActive = chainparams.GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_SAPLING); bool isSprout = !overwinterActive; bool heartwoodActive = chainparams.GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_HEARTWOOD); + bool canopyActive = chainparams.GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY); // If Sprout rules apply, reject transactions which are intended for Overwinter and beyond if (isSprout && tx.fOverwintered) { @@ -919,19 +920,29 @@ bool ContextualCheckTransaction( } // SaplingNotePlaintext::decrypt() checks note commitment validity. - if (!SaplingNotePlaintext::decrypt( + auto encPlaintext = SaplingNotePlaintext::decrypt( output.encCiphertext, output.ephemeralKey, outPlaintext->esk, outPlaintext->pk_d, - output.cmu) - ) { + output.cmu); + if (!encPlaintext) { return state.DoS( DOS_LEVEL_BLOCK, error("CheckTransaction(): coinbase output description has invalid encCiphertext"), REJECT_INVALID, "bad-cb-output-desc-invalid-encct"); } + + // ZIP 212: Check that the note plaintexts use the v2 note plaintext + // version. + if (canopyActive != (encPlaintext->get_lead_byte() == 0x02)) { + return state.DoS( + DOS_LEVEL_BLOCK, + error("CheckTransaction(): coinbase output description has invalid note plaintext version"), + REJECT_INVALID, + "bad-cb-output-desc-invalid-note-plaintext-version"); + } } } } diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index a130267a7..a56a2310e 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -171,6 +171,9 @@ public: uint256 rcm() const; uint256 generate_esk() const; + bool get_lead_byte() const { + return leadByte; + } }; class SaplingOutgoingPlaintext From 56d4ef8333a5c9db04aee636ec383b85089902ee Mon Sep 17 00:00:00 2001 From: Sean Bowe Date: Wed, 17 Jun 2020 17:09:11 -0600 Subject: [PATCH 07/21] Make transaction builder take the next block height into account for use of v2 note plaintexts. --- src/transaction_builder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 7390932af..52286d00c 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -163,7 +163,7 @@ void TransactionBuilder::AddSaplingOutput( } unsigned char leadByte = 0x01; - if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { + if (Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)) { leadByte = 0x02; } auto note = libzcash::SaplingNote(to, value, leadByte); From e060d598901f815d06b35debbad4a6a6d3ca0834 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Fri, 19 Jun 2020 11:10:19 +0800 Subject: [PATCH 08/21] Reject v1 plaintexts after grace period SaplingNotePlaintext::decrypt() now has to be aware of consensus params and blockheight. Its callers in wallet, rpcwallet, and tests are updated accordingly. TransactionBuilder is also modified to reject invalid leadBytes. Co-authored by Daira Hopwood (daira@jacaranda.org) --- src/consensus/consensus.h | 2 ++ src/gtest/test_noteencryption.cpp | 17 ++++++++++ src/main.cpp | 2 ++ src/transaction_builder.cpp | 6 ++++ src/wallet/gtest/test_wallet.cpp | 7 ++-- src/wallet/rpcwallet.cpp | 10 ++++-- src/wallet/wallet.cpp | 20 +++++++++--- src/wallet/wallet.h | 4 +-- src/zcash/Note.cpp | 53 +++++++++++++++---------------- src/zcash/Note.hpp | 41 +++++++++++++++++++----- 10 files changed, 114 insertions(+), 48 deletions(-) diff --git a/src/consensus/consensus.h b/src/consensus/consensus.h index 847e5d53a..3452cd4b4 100644 --- a/src/consensus/consensus.h +++ b/src/consensus/consensus.h @@ -31,6 +31,8 @@ static const unsigned int MAX_TX_SIZE_AFTER_SAPLING = MAX_BLOCK_SIZE; static const int COINBASE_MATURITY = 100; /** The minimum value which is invalid for expiry height, used by CTransaction and CMutableTransaction */ static constexpr uint32_t TX_EXPIRY_HEIGHT_THRESHOLD = 500000000; +/** The number of blocks after Canopy activation after which v1 plaintexts will be rejected */ +static const unsigned int ZIP212_GRACE_PERIOD = 32256; /** Flags for LockTime() */ enum { diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index 083bdeba1..2d99158b2 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -10,6 +10,8 @@ #include "zcash/Address.hpp" #include "crypto/sha256.h" #include "librustzcash.h" +#include "consensus/params.h" +#include "utiltest.h" class TestNoteDecryption : public ZCNoteDecryption { public: @@ -22,6 +24,13 @@ public: TEST(noteencryption, NotePlaintext) { + SelectParams(CBaseChainParams::REGTEST); + const Consensus::Params& params = Params().GetConsensus(); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + using namespace libzcash; auto xsk = SaplingSpendingKey(uint256()).expanded_spending_key(); auto fvk = xsk.full_viewing_key(); @@ -55,6 +64,8 @@ TEST(noteencryption, NotePlaintext) // Try to decrypt with incorrect commitment ASSERT_FALSE(SaplingNotePlaintext::decrypt( + params, + saplingActivationHeight, ct, ivk, epk, @@ -63,6 +74,8 @@ TEST(noteencryption, NotePlaintext) // Try to decrypt with correct commitment auto foo = SaplingNotePlaintext::decrypt( + params, + saplingActivationHeight, ct, ivk, epk, @@ -129,6 +142,8 @@ TEST(noteencryption, NotePlaintext) // Test sender won't accept invalid commitments ASSERT_FALSE( SaplingNotePlaintext::decrypt( + params, + saplingActivationHeight, ct, epk, decrypted_out_ct_unwrapped.esk, @@ -139,6 +154,8 @@ TEST(noteencryption, NotePlaintext) // Test sender can decrypt the note ciphertext. foo = SaplingNotePlaintext::decrypt( + params, + saplingActivationHeight, ct, epk, decrypted_out_ct_unwrapped.esk, diff --git a/src/main.cpp b/src/main.cpp index cbdd8a18d..8e2ebaed9 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -921,6 +921,8 @@ bool ContextualCheckTransaction( // SaplingNotePlaintext::decrypt() checks note commitment validity. auto encPlaintext = SaplingNotePlaintext::decrypt( + chainparams.GetConsensus(), + nHeight, output.encCiphertext, output.ephemeralKey, outPlaintext->esk, diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 52286d00c..1d6eddbb5 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -9,6 +9,7 @@ #include "rpc/protocol.h" #include "script/sign.h" #include "utilmoneystr.h" +#include "zcash/Note.hpp" #include #include @@ -142,6 +143,11 @@ void TransactionBuilder::AddSaplingSpend( throw std::runtime_error("TransactionBuilder cannot add Sapling spend to pre-Sapling transaction"); } + // ZIP212: check that note plaintext lead byte is valid at height + if (!libzcash::plaintext_version_is_valid(consensusParams, nHeight + 1, note.get_lead_byte())) { + throw std::runtime_error("TransactionBuilder: invalid note plaintext version"); + } + // Consistency check: all anchors must equal the first one if (spends.size() > 0 && spends[0].anchor != anchor) { throw JSONRPCError(RPC_WALLET_ERROR, "Anchor does not match previously-added Sapling spends."); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 18865cbcd..348b2e022 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -698,6 +698,8 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Decrypt output note B auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + consensusParams, + wtx.nExpiryHeight, wtx.vShieldedOutput[0].encCiphertext, ivk, wtx.vShieldedOutput[0].ephemeralKey, @@ -1075,6 +1077,8 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { // Decrypt note B auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + consensusParams, + wtx.nExpiryHeight, wtx.vShieldedOutput[0].encCiphertext, ivk, wtx.vShieldedOutput[0].ephemeralKey, @@ -2005,8 +2009,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { wtx = wallet.mapWallet[hash]; // Prepare to spend the note that was just created - auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( - tx1.vShieldedOutput[0].encCiphertext, ivk, tx1.vShieldedOutput[0].ephemeralKey, tx1.vShieldedOutput[0].cmu); + auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt(consensusParams, fakeIndex.nHeight, tx1.vShieldedOutput[0].encCiphertext, ivk, tx1.vShieldedOutput[0].ephemeralKey, tx1.vShieldedOutput[0].cmu); ASSERT_EQ(static_cast(maybe_pt), true); auto maybe_note = maybe_pt.get().note(ivk); ASSERT_EQ(static_cast(maybe_note), true); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 36838401d..e45aa218e 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -5,6 +5,7 @@ #include "amount.h" #include "consensus/upgrades.h" +#include "consensus/params.h" #include "core_io.h" #include "experimental_features.h" #include "init.h" @@ -3768,7 +3769,8 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp) auto op = res->second; auto wtxPrev = pwalletMain->mapWallet.at(op.hash); - auto decrypted = wtxPrev.DecryptSaplingNote(op).get(); + // TODO: decide which height to use here instead of wtxPrev.nExpiryHeight + auto decrypted = wtxPrev.DecryptSaplingNote(Params().GetConsensus(), wtxPrev.nExpiryHeight, op).get(); auto notePt = decrypted.first; auto pa = decrypted.second; @@ -3796,14 +3798,16 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp) SaplingPaymentAddress pa; bool isOutgoing; - auto decrypted = wtx.DecryptSaplingNote(op); + // TODO: decide which height to use here instead of wtx.nExpiryHeight + auto decrypted = wtx.DecryptSaplingNote(Params().GetConsensus(), wtx.nExpiryHeight, op); if (decrypted) { notePt = decrypted->first; pa = decrypted->second; isOutgoing = false; } else { // Try recovering the output - auto recovered = wtx.RecoverSaplingNote(op, ovks); + // TODO: decide which height to use here instead of wtxPrev.nExpiryHeight + auto recovered = wtx.RecoverSaplingNote(Params().GetConsensus(), wtx.nExpiryHeight, op, ovks); if (recovered) { notePt = recovered->first; pa = recovered->second; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a1c7bb5c0..0ccea2ae0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1495,7 +1495,9 @@ void CWallet::UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx) { uint64_t position = nd.witnesses.front().position(); auto extfvk = mapSaplingFullViewingKeys.at(nd.ivk); OutputDescription output = wtx.vShieldedOutput[op.n]; - auto optPlaintext = SaplingNotePlaintext::decrypt(output.encCiphertext, nd.ivk, output.ephemeralKey, output.cmu); + + // TODO: decide which height to use here instead of wtx.nExpiryHeight + auto optPlaintext = SaplingNotePlaintext::decrypt(Params().GetConsensus(), wtx.nExpiryHeight, output.encCiphertext, nd.ivk, output.ephemeralKey, output.cmu); if (!optPlaintext) { // An item in mapSaplingNoteData must have already been successfully decrypted, // otherwise the item would not exist in the first place. @@ -1901,7 +1903,9 @@ std::pair CWallet::FindMySap const OutputDescription output = tx.vShieldedOutput[i]; for (auto it = mapSaplingFullViewingKeys.begin(); it != mapSaplingFullViewingKeys.end(); ++it) { SaplingIncomingViewingKey ivk = it->first; - auto result = SaplingNotePlaintext::decrypt(output.encCiphertext, ivk, output.ephemeralKey, output.cmu); + + // TODO: decide which height to use here instead of wtx.nExpiryHeight + auto result = SaplingNotePlaintext::decrypt(Params().GetConsensus(), tx.nExpiryHeight, output.encCiphertext, ivk, output.ephemeralKey, output.cmu); if (!result) { continue; } @@ -2300,7 +2304,7 @@ std::pair CWalletTx::DecryptSproutNot boost::optional> CWalletTx::DecryptSaplingNote(SaplingOutPoint op) const + SaplingPaymentAddress>> CWalletTx::DecryptSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op) const { // Check whether we can decrypt this SaplingOutPoint if (this->mapSaplingNoteData.count(op) == 0) { @@ -2311,6 +2315,8 @@ boost::optionalmapSaplingNoteData.at(op); auto maybe_pt = SaplingNotePlaintext::decrypt( + params, + height, output.encCiphertext, nd.ivk, output.ephemeralKey, @@ -2327,8 +2333,7 @@ boost::optional> CWalletTx::RecoverSaplingNote( - SaplingOutPoint op, std::set& ovks) const + SaplingPaymentAddress>> CWalletTx::RecoverSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op, std::set& ovks) const { auto output = this->vShieldedOutput[op.n]; @@ -2344,6 +2349,8 @@ boost::optionalesk, @@ -4975,7 +4982,10 @@ void CWallet::GetFilteredNotes( SaplingOutPoint op = pair.first; SaplingNoteData nd = pair.second; + // TODO: decide which height to use here instead of wtx.nExpiryHeight auto maybe_pt = SaplingNotePlaintext::decrypt( + Params().GetConsensus(), + wtx.nExpiryHeight, wtx.vShieldedOutput[op.n].encCiphertext, nd.ivk, wtx.vShieldedOutput[op.n].ephemeralKey, diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c673dfcea..d0696afd6 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -566,10 +566,10 @@ public: JSOutPoint jsop) const; boost::optional> DecryptSaplingNote(SaplingOutPoint op) const; + libzcash::SaplingPaymentAddress>> DecryptSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op) const; boost::optional> RecoverSaplingNote( + libzcash::SaplingPaymentAddress>> RecoverSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op, std::set& ovks) const; //! filter decides which addresses will count towards the debit diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 76a14f9e6..206b73cd9 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -1,6 +1,7 @@ #include "Note.hpp" #include "prf.h" #include "crypto/sha256.h" +#include "consensus/consensus.h" #include "random.h" #include "version.h" @@ -188,24 +189,20 @@ boost::optional SaplingOutgoingPlaintext::decrypt( } // Deserialize from the plaintext - try { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << pt.get(); + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << pt.get(); - SaplingOutgoingPlaintext ret; - ss >> ret; + SaplingOutgoingPlaintext ret; + ss >> ret; - assert(ss.size() == 0); + assert(ss.size() == 0); - return ret; - } catch (const boost::thread_interrupted&) { - throw; - } catch (...) { - return boost::none; - } + return ret; } boost::optional SaplingNotePlaintext::decrypt( + const Consensus::Params& params, + int height, const SaplingEncCiphertext &ciphertext, const uint256 &ivk, const uint256 &epk, @@ -219,14 +216,13 @@ boost::optional SaplingNotePlaintext::decrypt( // Deserialize from the plaintext SaplingNotePlaintext ret; - try { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << pt.get(); - ss >> ret; - assert(ss.size() == 0); - } catch (const boost::thread_interrupted&) { - throw; - } catch (...) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << pt.get(); + ss >> ret; + assert(ss.size() == 0); + + // Check leadbyte is allowed at block height + if (!plaintext_version_valid(params, height, ret.leadByte)) { return boost::none; } @@ -269,6 +265,8 @@ boost::optional SaplingNotePlaintext::decrypt( } boost::optional SaplingNotePlaintext::decrypt( + const Consensus::Params& params, + int height, const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, @@ -283,14 +281,13 @@ boost::optional SaplingNotePlaintext::decrypt( // Deserialize from the plaintext SaplingNotePlaintext ret; - try { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << pt.get(); - ss >> ret; - assert(ss.size() == 0); - } catch (const boost::thread_interrupted&) { - throw; - } catch (...) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << pt.get(); + ss >> ret; + assert(ss.size() == 0); + + // Check leadbyte is legible at block height + if (!plaintext_version_valid(params, height, ret.leadByte)) { return boost::none; } diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index a56a2310e..134c289eb 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -5,6 +5,8 @@ #include "Zcash.h" #include "Address.hpp" #include "NoteEncryption.hpp" +#include "consensus/params.h" +#include "consensus/consensus.h" #include #include @@ -40,6 +42,27 @@ public: uint256 nullifier(const SproutSpendingKey& a_sk) const; }; +inline bool plaintext_version_is_valid(const Consensus::Params& params, int height, unsigned char leadByte) { + int canopyActivationHeight = params.vUpgrades[Consensus::UPGRADE_CANOPY].nActivationHeight; + + if (height < canopyActivationHeight && leadByte != 0x01) { + // non-0x01 received before Canopy activation height + return false; + } + if (height >= canopyActivationHeight + && height < canopyActivationHeight + ZIP212_GRACE_PERIOD + && leadByte != 0x01 + && leadByte != 0x02) + { + // non-{0x01,0x02} received after Canopy activation and before grace period has elapsed + return false; + } + if (height >= canopyActivationHeight + ZIP212_GRACE_PERIOD && leadByte != 0x02) { + // non-0x02 received past (Canopy activation height + grace period) + return false; + } + return true; +}; class SaplingNote : public BaseNote { private: @@ -60,6 +83,10 @@ public: boost::optional cmu() const; boost::optional nullifier(const SaplingFullViewingKey &vk, const uint64_t position) const; uint256 rcm() const; + + unsigned char get_lead_byte() const { + return leadByte; + } }; class BaseNotePlaintext { @@ -132,6 +159,8 @@ public: SaplingNotePlaintext(const SaplingNote& note, std::array memo); static boost::optional decrypt( + const Consensus::Params& params, + int height, const SaplingEncCiphertext &ciphertext, const uint256 &ivk, const uint256 &epk, @@ -139,6 +168,8 @@ public: ); static boost::optional decrypt( + const Consensus::Params& params, + int height, const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, @@ -154,13 +185,7 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(leadByte); - - if (leadByte != 0x01 && leadByte != 0x02) { - printf("leadByte: %x\n", leadByte); - throw std::ios_base::failure("lead byte of SaplingNotePlaintext is not recognized"); - } - + READWRITE(leadByte); // 1 byte READWRITE(d); // 11 bytes READWRITE(value_); // 8 bytes READWRITE(rseed); // 32 bytes @@ -171,7 +196,7 @@ public: uint256 rcm() const; uint256 generate_esk() const; - bool get_lead_byte() const { + unsigned char get_lead_byte() const { return leadByte; } }; From 3c8e9703589bedf5c1d17438a9f1701d4fd7035c Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Fri, 19 Jun 2020 11:09:27 +0800 Subject: [PATCH 09/21] Check epk vs esk whenever caller has esk --- src/zcash/Note.cpp | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 206b73cd9..76fcb1be2 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -222,7 +222,7 @@ boost::optional SaplingNotePlaintext::decrypt( assert(ss.size() == 0); // Check leadbyte is allowed at block height - if (!plaintext_version_valid(params, height, ret.leadByte)) { + if (!plaintext_version_is_valid(params, height, ret.leadByte)) { return boost::none; } @@ -287,7 +287,16 @@ boost::optional SaplingNotePlaintext::decrypt( assert(ss.size() == 0); // Check leadbyte is legible at block height - if (!plaintext_version_valid(params, height, ret.leadByte)) { + if (!plaintext_version_is_valid(params, height, ret.leadByte)) { + return boost::none; + } + + // Check that epk is consistent with esk + uint256 expected_epk; + if (!librustzcash_sapling_ka_derivepublic(ret.d.data(), esk.begin(), expected_epk.begin())) { + return boost::none; + } + if (expected_epk != epk) { return boost::none; } @@ -309,16 +318,6 @@ boost::optional SaplingNotePlaintext::decrypt( } if (ret.leadByte == 0x02) { - // ZIP 212: Check that epk is consistent to prevent against linkability - // attacks without relying on the soundness of the SNARK. - uint256 expected_epk; - if (!librustzcash_sapling_ka_derivepublic(ret.d.data(), esk.begin(), expected_epk.begin())) { - return boost::none; - } - if (expected_epk != epk) { - return boost::none; - } - // ZIP 212: Additionally check that the esk provided to this function // is consistent with the esk we can derive if (esk != ret.generate_esk()) { From 6402c589c6cb00185e14d4f8c4b6bb9acc981489 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Sat, 20 Jun 2020 15:35:16 +0800 Subject: [PATCH 10/21] Refactor SaplingNotePlaintext::decrypt Break up plaintext decryption into height-dependent and non-height-dependent parts. --- src/wallet/rpcwallet.cpp | 13 ++-- src/wallet/wallet.cpp | 110 +++++++++++++++++++++++++++--- src/wallet/wallet.h | 8 ++- src/zcash/Note.cpp | 144 +++++++++++++++++++++++++++------------ src/zcash/Note.hpp | 28 ++++++++ src/zcbenchmarks.cpp | 2 +- 6 files changed, 243 insertions(+), 62 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index e45aa218e..08bf81662 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3769,8 +3769,9 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp) auto op = res->second; auto wtxPrev = pwalletMain->mapWallet.at(op.hash); - // TODO: decide which height to use here instead of wtxPrev.nExpiryHeight - auto decrypted = wtxPrev.DecryptSaplingNote(Params().GetConsensus(), wtxPrev.nExpiryHeight, op).get(); + // We don't need to check the leadbyte here: if wtx exists in + // the wallet, it must have already passed the leadbyte check + auto decrypted = wtxPrev.DecryptSaplingNoteWithoutLeadByteCheck(op).get(); auto notePt = decrypted.first; auto pa = decrypted.second; @@ -3798,16 +3799,16 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp) SaplingPaymentAddress pa; bool isOutgoing; - // TODO: decide which height to use here instead of wtx.nExpiryHeight - auto decrypted = wtx.DecryptSaplingNote(Params().GetConsensus(), wtx.nExpiryHeight, op); + // We don't need to check the leadbyte here: if wtx exists in + // the wallet, it must have already passed the leadbyte check + auto decrypted = wtx.DecryptSaplingNoteWithoutLeadByteCheck(op); if (decrypted) { notePt = decrypted->first; pa = decrypted->second; isOutgoing = false; } else { // Try recovering the output - // TODO: decide which height to use here instead of wtxPrev.nExpiryHeight - auto recovered = wtx.RecoverSaplingNote(Params().GetConsensus(), wtx.nExpiryHeight, op, ovks); + auto recovered = wtx.RecoverSaplingNoteWithoutLeadByteCheck(op, ovks); if (recovered) { notePt = recovered->first; pa = recovered->second; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0ccea2ae0..d43881256 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1496,8 +1496,15 @@ void CWallet::UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx) { auto extfvk = mapSaplingFullViewingKeys.at(nd.ivk); OutputDescription output = wtx.vShieldedOutput[op.n]; - // TODO: decide which height to use here instead of wtx.nExpiryHeight - auto optPlaintext = SaplingNotePlaintext::decrypt(Params().GetConsensus(), wtx.nExpiryHeight, output.encCiphertext, nd.ivk, output.ephemeralKey, output.cmu); + auto optDeserialized = SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization(output.encCiphertext, nd.ivk, output.ephemeralKey); + + if (!optDeserialized) { + // The transaction would not have entered the wallet unless + // its plaintest had been succesfully decrypted previously. + assert(false); + } + + auto optPlaintext = SaplingNotePlaintext::plaintext_checks_without_height(*optDeserialized, nd.ivk, output.ephemeralKey, output.cmu); if (!optPlaintext) { // An item in mapSaplingNoteData must have already been successfully decrypted, // otherwise the item would not exist in the first place. @@ -1716,7 +1723,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; auto sproutNoteData = FindMySproutNotes(tx); - auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx); + auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx, chainActive.Height()); auto saplingNoteData = saplingNoteDataAndAddressesToAdd.first; auto addressesToAdd = saplingNoteDataAndAddressesToAdd.second; for (const auto &addressToAdd : addressesToAdd) { @@ -1890,7 +1897,7 @@ mapSproutNoteData_t CWallet::FindMySproutNotes(const CTransaction &tx) const * the result of FindMySaplingNotes (for the addresses available at the time) will * already have been cached in CWalletTx.mapSaplingNoteData. */ -std::pair CWallet::FindMySaplingNotes(const CTransaction &tx) const +std::pair CWallet::FindMySaplingNotes(const CTransaction &tx, int height) const { LOCK(cs_KeyStore); uint256 hash = tx.GetHash(); @@ -1904,8 +1911,7 @@ std::pair CWallet::FindMySap for (auto it = mapSaplingFullViewingKeys.begin(); it != mapSaplingFullViewingKeys.end(); ++it) { SaplingIncomingViewingKey ivk = it->first; - // TODO: decide which height to use here instead of wtx.nExpiryHeight - auto result = SaplingNotePlaintext::decrypt(Params().GetConsensus(), tx.nExpiryHeight, output.encCiphertext, ivk, output.ephemeralKey, output.cmu); + auto result = SaplingNotePlaintext::decrypt(Params().GetConsensus(), height, output.encCiphertext, ivk, output.ephemeralKey, output.cmu); if (!result) { continue; } @@ -2331,6 +2337,41 @@ boost::optional> CWalletTx::DecryptSaplingNoteWithoutLeadByteCheck(SaplingOutPoint op) const +{ + // Check whether we can decrypt this SaplingOutPoint + if (this->mapSaplingNoteData.count(op) == 0) { + return boost::none; + } + + auto output = this->vShieldedOutput[op.n]; + auto nd = this->mapSaplingNoteData.at(op); + + auto optDeserialized = SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization(output.encCiphertext, nd.ivk, output.ephemeralKey); + + if (!optDeserialized) { + // The transaction would not have entered the wallet unless + // its plaintest had been succesfully decrypted previously. + assert(false); + } + + auto maybe_pt = SaplingNotePlaintext::plaintext_checks_without_height( + *optDeserialized, + nd.ivk, + output.ephemeralKey, + output.cmu); + assert(static_cast(maybe_pt)); + auto notePt = maybe_pt.get(); + + auto maybe_pa = nd.ivk.address(notePt.d); + assert(static_cast(maybe_pa)); + auto pa = maybe_pa.get(); + + return std::make_pair(notePt, pa); +} + boost::optional> CWalletTx::RecoverSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op, std::set& ovks) const @@ -2366,6 +2407,47 @@ boost::optional> CWalletTx::RecoverSaplingNoteWithoutLeadByteCheck(SaplingOutPoint op, std::set& ovks) const +{ + auto output = this->vShieldedOutput[op.n]; + + for (auto ovk : ovks) { + auto outPt = SaplingOutgoingPlaintext::decrypt( + output.outCiphertext, + ovk, + output.cv, + output.cmu, + output.ephemeralKey); + if (!outPt) { + continue; + } + + auto optDeserialized = SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization(output.encCiphertext, output.ephemeralKey, outPt->esk, outPt->pk_d); + + if (!optDeserialized) { + // The transaction would not have entered the wallet unless + // its plaintest had been succesfully decrypted previously. + assert(false); + } + + auto maybe_pt = SaplingNotePlaintext::plaintext_checks_without_height( + *optDeserialized, + output.ephemeralKey, + outPt->esk, + outPt->pk_d, + output.cmu); + assert(static_cast(maybe_pt)); + auto notePt = maybe_pt.get(); + + return std::make_pair(notePt, SaplingPaymentAddress(notePt.d, outPt->pk_d)); + } + + // Couldn't recover with any of the provided OutgoingViewingKeys + return boost::none; +} + int64_t CWalletTx::GetTxTime() const { int64_t n = nTimeSmart; @@ -4982,11 +5064,17 @@ void CWallet::GetFilteredNotes( SaplingOutPoint op = pair.first; SaplingNoteData nd = pair.second; - // TODO: decide which height to use here instead of wtx.nExpiryHeight - auto maybe_pt = SaplingNotePlaintext::decrypt( - Params().GetConsensus(), - wtx.nExpiryHeight, - wtx.vShieldedOutput[op.n].encCiphertext, + auto optDeserialized = SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization(wtx.vShieldedOutput[op.n].encCiphertext, nd.ivk, wtx.vShieldedOutput[op.n].ephemeralKey); + + if (!optDeserialized) { + // The transaction would not have entered the wallet unless + // its plaintest had been succesfully decrypted previously. + assert(false); + } + // We don't need to check the leadbyte here: if wtx exists in + // the wallet, it must have already passed the leadbyte check + auto maybe_pt = SaplingNotePlaintext::plaintext_checks_without_height( + *optDeserialized, nd.ivk, wtx.vShieldedOutput[op.n].ephemeralKey, wtx.vShieldedOutput[op.n].cmu); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d0696afd6..4d6bcb95a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -567,10 +567,16 @@ public: boost::optional> DecryptSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op) const; + boost::optional> DecryptSaplingNoteWithoutLeadByteCheck(SaplingOutPoint op) const; boost::optional> RecoverSaplingNote(const Consensus::Params& params, int height, SaplingOutPoint op, std::set& ovks) const; + boost::optional> RecoverSaplingNoteWithoutLeadByteCheck(SaplingOutPoint op, std::set& ovks) const; //! filter decides which addresses will count towards the debit CAmount GetDebit(const isminefilter& filter) const; @@ -1221,7 +1227,7 @@ public: const uint256& hSig, uint8_t n) const; mapSproutNoteData_t FindMySproutNotes(const CTransaction& tx) const; - std::pair FindMySaplingNotes(const CTransaction& tx) const; + std::pair FindMySaplingNotes(const CTransaction& tx, int height) const; bool IsSproutNullifierFromMe(const uint256& nullifier) const; bool IsSaplingNullifierFromMe(const uint256& nullifier) const; diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 76fcb1be2..08da32ed6 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -209,34 +209,40 @@ boost::optional SaplingNotePlaintext::decrypt( const uint256 &cmu ) { - auto pt = AttemptSaplingEncDecryption(ciphertext, ivk, epk); - if (!pt) { - return boost::none; - } - - // Deserialize from the plaintext - SaplingNotePlaintext ret; - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << pt.get(); - ss >> ret; - assert(ss.size() == 0); - - // Check leadbyte is allowed at block height - if (!plaintext_version_is_valid(params, height, ret.leadByte)) { + auto ret = attempt_sapling_enc_decryption_deserialization(ciphertext, ivk, epk); + + if (!ret) { return boost::none; + } else { + const SaplingNotePlaintext plaintext = *ret; + + // Check leadbyte is allowed at block height + if (!plaintext_version_is_valid(params, height, plaintext.leadByte)) { + return boost::none; + } + + return plaintext_checks_without_height(plaintext, ivk, epk, cmu); } +} +boost::optional SaplingNotePlaintext::plaintext_checks_without_height( + const SaplingNotePlaintext &plaintext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu +) +{ uint256 pk_d; - if (!librustzcash_ivk_to_pkd(ivk.begin(), ret.d.data(), pk_d.begin())) { + if (!librustzcash_ivk_to_pkd(ivk.begin(), plaintext.d.data(), pk_d.begin())) { return boost::none; } uint256 cmu_expected; - uint256 rcm = ret.rcm(); + uint256 rcm = plaintext.rcm(); if (!librustzcash_sapling_compute_cm( - ret.d.data(), + plaintext.d.data(), pk_d.begin(), - ret.value(), + plaintext.value(), rcm.begin(), cmu_expected.begin() )) @@ -248,12 +254,12 @@ boost::optional SaplingNotePlaintext::decrypt( return boost::none; } - if (ret.leadByte == 0x02) { + if (plaintext.leadByte == 0x02) { // ZIP 212: Check that epk is consistent to prevent against linkability // attacks without relying on the soundness of the SNARK. uint256 expected_epk; - uint256 esk = ret.generate_esk(); - if (!librustzcash_sapling_ka_derivepublic(ret.d.data(), esk.begin(), expected_epk.begin())) { + uint256 esk = plaintext.generate_esk(); + if (!librustzcash_sapling_ka_derivepublic(plaintext.d.data(), esk.begin(), expected_epk.begin())) { return boost::none; } if (expected_epk != epk) { @@ -261,7 +267,29 @@ boost::optional SaplingNotePlaintext::decrypt( } } - return ret; + return plaintext; +} + +boost::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk +) +{ + auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, ivk, epk); + + if (!encPlaintext) { + return boost::none; + }; + + // Deserialize from the plaintext + SaplingNotePlaintext plaintext; + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << encPlaintext.get(); + ss >> plaintext; + assert(ss.size() == 0); + + return plaintext; } boost::optional SaplingNotePlaintext::decrypt( @@ -274,26 +302,33 @@ boost::optional SaplingNotePlaintext::decrypt( const uint256 &cmu ) { - auto pt = AttemptSaplingEncDecryption(ciphertext, epk, esk, pk_d); - if (!pt) { - return boost::none; - } - - // Deserialize from the plaintext - SaplingNotePlaintext ret; - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << pt.get(); - ss >> ret; - assert(ss.size() == 0); - - // Check leadbyte is legible at block height - if (!plaintext_version_is_valid(params, height, ret.leadByte)) { + auto ret = attempt_sapling_enc_decryption_deserialization(ciphertext, epk, esk, pk_d); + + if (!ret) { return boost::none; + } else { + SaplingNotePlaintext plaintext = *ret; + + // Check leadbyte is allowed at block height + if (!plaintext_version_is_valid(params, height, plaintext.leadByte)) { + return boost::none; + } + + return plaintext_checks_without_height(plaintext, epk, esk, pk_d, cmu); } +} +boost::optional SaplingNotePlaintext::plaintext_checks_without_height( + const SaplingNotePlaintext &plaintext, + const uint256 &epk, + const uint256 &esk, + const uint256 &pk_d, + const uint256 &cmu +) +{ // Check that epk is consistent with esk uint256 expected_epk; - if (!librustzcash_sapling_ka_derivepublic(ret.d.data(), esk.begin(), expected_epk.begin())) { + if (!librustzcash_sapling_ka_derivepublic(plaintext.d.data(), esk.begin(), expected_epk.begin())) { return boost::none; } if (expected_epk != epk) { @@ -301,11 +336,11 @@ boost::optional SaplingNotePlaintext::decrypt( } uint256 cmu_expected; - uint256 rcm = ret.rcm(); + uint256 rcm = plaintext.rcm(); if (!librustzcash_sapling_compute_cm( - ret.d.data(), + plaintext.d.data(), pk_d.begin(), - ret.value(), + plaintext.value(), rcm.begin(), cmu_expected.begin() )) @@ -317,15 +352,38 @@ boost::optional SaplingNotePlaintext::decrypt( return boost::none; } - if (ret.leadByte == 0x02) { + if (plaintext.leadByte == 0x02) { // ZIP 212: Additionally check that the esk provided to this function // is consistent with the esk we can derive - if (esk != ret.generate_esk()) { + if (esk != plaintext.generate_esk()) { return boost::none; } } - return ret; + return plaintext; +} + +boost::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( + const SaplingEncCiphertext &ciphertext, + const uint256 &epk, + const uint256 &esk, + const uint256 &pk_d +) +{ + auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, epk, esk, pk_d); + + if (!encPlaintext) { + return boost::none; + }; + + // Deserialize from the plaintext + SaplingNotePlaintext plaintext; + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << encPlaintext.get(); + ss >> plaintext; + assert(ss.size() == 0); + + return plaintext; } boost::optional SaplingNotePlaintext::encrypt(const uint256& pk_d) const diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index 134c289eb..b9df41547 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -167,6 +167,19 @@ public: const uint256 &cmu ); + static boost::optional plaintext_checks_without_height( + const SaplingNotePlaintext &plaintext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu + ); + + static boost::optional attempt_sapling_enc_decryption_deserialization( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk + ); + static boost::optional decrypt( const Consensus::Params& params, int height, @@ -177,6 +190,21 @@ public: const uint256 &cmu ); + static boost::optional plaintext_checks_without_height( + const SaplingNotePlaintext &plaintext, + const uint256 &epk, + const uint256 &esk, + const uint256 &pk_d, + const uint256 &cmu + ); + + static boost::optional attempt_sapling_enc_decryption_deserialization( + const SaplingEncCiphertext &ciphertext, + const uint256 &epk, + const uint256 &esk, + const uint256 &pk_d + ); + boost::optional note(const SaplingIncomingViewingKey& ivk) const; virtual ~SaplingNotePlaintext() {} diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index 33378f2db..4311f7bf7 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -307,7 +307,7 @@ double benchmark_try_decrypt_sapling_notes(size_t nKeys) struct timeval tv_start; timer_start(tv_start); - auto noteDataMapAndAddressesToAdd = wallet.FindMySaplingNotes(tx); + auto noteDataMapAndAddressesToAdd = wallet.FindMySaplingNotes(tx, 1); assert(noteDataMapAndAddressesToAdd.first.empty()); return timer_stop(tv_start); } From 7a1d1191707c44cc6addfb47d4678d796a357f8a Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Sat, 20 Jun 2020 00:28:06 +0800 Subject: [PATCH 11/21] Add gtests for v2 plaintexts --- src/gtest/test_checktransaction.cpp | 77 ++++ src/gtest/test_noteencryption.cpp | 535 ++++++++++++++++++------ src/gtest/test_transaction_builder.cpp | 121 ++++++ src/wallet/gtest/test_wallet.cpp | 557 +++++++++++++------------ 4 files changed, 905 insertions(+), 385 deletions(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 233dcb03c..0879bfb01 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -1284,3 +1284,80 @@ TEST(CheckTransaction, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { RegtestDeactivateHeartwood(); } + +// Check that the consensus rules relevant to valueBalance, vShieldedOutput, and +// bindingSig from https://zips.z.cash/protocol/protocol.pdf#txnencoding are +// applied to coinbase transactions. +TEST(CheckTransaction, CanopyEnforcesSaplingRulesOnShieldedCoinbase) { + RegtestActivateCanopy(false, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); + auto chainparams = Params(); + + uint256 ovk; + auto note = libzcash::SaplingNote( + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), 0x02); + auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); + + CMutableTransaction mtx = GetValidTransaction(); + mtx.fOverwintered = true; + mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; + mtx.nVersion = SAPLING_TX_VERSION; + + mtx.vin.resize(1); + mtx.vin[0].prevout.SetNull(); + mtx.vin[0].scriptSig << 123; + mtx.vJoinSplit.resize(0); + mtx.valueBalance = -1000; + + // Coinbase transaction should fail non-contextual checks with no shielded + // outputs and non-zero valueBalance. + { + CTransaction tx(mtx); + EXPECT_TRUE(tx.IsCoinBase()); + + MockCValidationState state; + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-valuebalance-nonzero", false)).Times(1); + EXPECT_FALSE(CheckTransactionWithoutProofVerification(tx, state)); + } + + // Add a Sapling output. + auto ctx = librustzcash_sapling_proving_ctx_init(); + auto odesc = output.Build(ctx).get(); + librustzcash_sapling_proving_ctx_free(ctx); + mtx.vShieldedOutput.push_back(odesc); + + // Coinbase transaction should fail non-contextual checks with valueBalance + // out of range. + { + mtx.valueBalance = MAX_MONEY + 1; + CTransaction tx(mtx); + EXPECT_TRUE(tx.IsCoinBase()); + + MockCValidationState state; + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-valuebalance-toolarge", false)).Times(1); + EXPECT_FALSE(CheckTransactionWithoutProofVerification(tx, state)); + } + { + mtx.valueBalance = -MAX_MONEY - 1; + CTransaction tx(mtx); + EXPECT_TRUE(tx.IsCoinBase()); + + MockCValidationState state; + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-valuebalance-toolarge", false)).Times(1); + EXPECT_FALSE(CheckTransactionWithoutProofVerification(tx, state)); + } + + mtx.valueBalance = -1000; + CTransaction tx(mtx); + EXPECT_TRUE(tx.IsCoinBase()); + + // Coinbase transaction should now pass non-contextual checks. + MockCValidationState state; + EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state)); + + // Coinbase transaction does not pass contextual checks, as bindingSig + // consensus rule is enforced. + EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-sapling-binding-signature-invalid", false)).Times(1); + ContextualCheckTransaction(tx, state, chainparams, 10, 57); + + RegtestDeactivateCanopy(); +} diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index 2d99158b2..22357d0ed 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -22,14 +22,19 @@ public: } }; -TEST(noteencryption, NotePlaintext) +TEST(NoteEncryption, NotePlaintext) { SelectParams(CBaseChainParams::REGTEST); - const Consensus::Params& params = Params().GetConsensus(); int overwinterActivationHeight = 5; int saplingActivationHeight = 30; + int canopyActivationHeight = 70; UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto params = Params().GetConsensus(); + + unsigned char leadBytes[] = {0x01, 0x02}; + int decryptionHeights[] = {saplingActivationHeight, canopyActivationHeight}; using namespace libzcash; auto xsk = SaplingSpendingKey(uint256()).expanded_spending_key(); @@ -43,139 +48,417 @@ TEST(noteencryption, NotePlaintext) memo[i] = (unsigned char) i; } - SaplingNote note(addr, 39393, 0x01); - auto cmu_opt = note.cmu(); - if (!cmu_opt) { - FAIL(); - } - uint256 cmu = cmu_opt.get(); - SaplingNotePlaintext pt(note, memo); + for (int ver = 0; ver < sizeof(leadBytes); ver++){ + SaplingNote note(addr, 39393, leadBytes[ver]); + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); - auto res = pt.encrypt(addr.pk_d); - if (!res) { - FAIL(); - } + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } - auto enc = res.get(); + auto enc = res.get(); - auto ct = enc.first; - auto encryptor = enc.second; - auto epk = encryptor.get_epk(); + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); - // Try to decrypt with incorrect commitment - ASSERT_FALSE(SaplingNotePlaintext::decrypt( - params, - saplingActivationHeight, - ct, - ivk, - epk, - uint256() - )); - - // Try to decrypt with correct commitment - auto foo = SaplingNotePlaintext::decrypt( - params, - saplingActivationHeight, - ct, - ivk, - epk, - cmu - ); - - if (!foo) { - FAIL(); - } - - auto bar = foo.get(); - - ASSERT_TRUE(bar.value() == pt.value()); - ASSERT_TRUE(bar.memo() == pt.memo()); - ASSERT_TRUE(bar.d == pt.d); - ASSERT_TRUE(bar.rcm() == pt.rcm()); - - auto foobar = bar.note(ivk); - - if (!foobar) { - FAIL(); - } - - auto new_note = foobar.get(); - - ASSERT_TRUE(note.value() == new_note.value()); - ASSERT_TRUE(note.d == new_note.d); - ASSERT_TRUE(note.pk_d == new_note.pk_d); - ASSERT_TRUE(note.rcm() == new_note.rcm()); - ASSERT_TRUE(note.cmu() == new_note.cmu()); - - SaplingOutgoingPlaintext out_pt; - out_pt.pk_d = note.pk_d; - out_pt.esk = encryptor.get_esk(); - - auto ovk = random_uint256(); - auto cv = random_uint256(); - auto cm = random_uint256(); - - auto out_ct = out_pt.encrypt( - ovk, - cv, - cm, - encryptor - ); - - auto decrypted_out_ct = out_pt.decrypt( - out_ct, - ovk, - cv, - cm, - encryptor.get_epk() - ); - - if (!decrypted_out_ct) { - FAIL(); - } - - auto decrypted_out_ct_unwrapped = decrypted_out_ct.get(); - - ASSERT_TRUE(decrypted_out_ct_unwrapped.pk_d == out_pt.pk_d); - ASSERT_TRUE(decrypted_out_ct_unwrapped.esk == out_pt.esk); - - // Test sender won't accept invalid commitments - ASSERT_FALSE( - SaplingNotePlaintext::decrypt( + // Try to decrypt with incorrect commitment + ASSERT_FALSE(SaplingNotePlaintext::decrypt( params, - saplingActivationHeight, + decryptionHeights[ver], + ct, + ivk, + epk, + uint256() + )); + + // Try to decrypt with correct commitment + auto foo = SaplingNotePlaintext::decrypt( + params, + decryptionHeights[ver], + ct, + ivk, + epk, + cmu + ); + + if (!foo) { + FAIL(); + } + + auto bar = foo.get(); + + ASSERT_TRUE(bar.value() == pt.value()); + ASSERT_TRUE(bar.memo() == pt.memo()); + ASSERT_TRUE(bar.d == pt.d); + ASSERT_TRUE(bar.rcm() == pt.rcm()); + + auto foobar = bar.note(ivk); + + if (!foobar) { + FAIL(); + } + + auto new_note = foobar.get(); + + ASSERT_TRUE(note.value() == new_note.value()); + ASSERT_TRUE(note.d == new_note.d); + ASSERT_TRUE(note.pk_d == new_note.pk_d); + ASSERT_TRUE(note.rcm() == new_note.rcm()); + ASSERT_TRUE(note.cmu() == new_note.cmu()); + + SaplingOutgoingPlaintext out_pt; + out_pt.pk_d = note.pk_d; + out_pt.esk = encryptor.get_esk(); + + auto ovk = random_uint256(); + auto cv = random_uint256(); + auto cm = random_uint256(); + + auto out_ct = out_pt.encrypt( + ovk, + cv, + cm, + encryptor + ); + + auto decrypted_out_ct = out_pt.decrypt( + out_ct, + ovk, + cv, + cm, + encryptor.get_epk() + ); + + if (!decrypted_out_ct) { + FAIL(); + } + + auto decrypted_out_ct_unwrapped = decrypted_out_ct.get(); + + ASSERT_TRUE(decrypted_out_ct_unwrapped.pk_d == out_pt.pk_d); + ASSERT_TRUE(decrypted_out_ct_unwrapped.esk == out_pt.esk); + + // Test sender won't accept invalid commitments + ASSERT_FALSE( + SaplingNotePlaintext::decrypt( + params, + decryptionHeights[ver], + ct, + epk, + decrypted_out_ct_unwrapped.esk, + decrypted_out_ct_unwrapped.pk_d, + uint256() + ) + ); + + // Test sender can decrypt the note ciphertext. + foo = SaplingNotePlaintext::decrypt( + params, + decryptionHeights[ver], ct, epk, decrypted_out_ct_unwrapped.esk, decrypted_out_ct_unwrapped.pk_d, - uint256() - ) - ); + cmu + ); - // Test sender can decrypt the note ciphertext. - foo = SaplingNotePlaintext::decrypt( - params, - saplingActivationHeight, - ct, - epk, - decrypted_out_ct_unwrapped.esk, - decrypted_out_ct_unwrapped.pk_d, - cmu - ); + if (!foo) { + FAIL(); + } - if (!foo) { - FAIL(); + bar = foo.get(); + + ASSERT_TRUE(bar.value() == pt.value()); + ASSERT_TRUE(bar.memo() == pt.memo()); + ASSERT_TRUE(bar.d == pt.d); + ASSERT_TRUE(bar.rcm() == pt.rcm()); } - bar = foo.get(); - - ASSERT_TRUE(bar.value() == pt.value()); - ASSERT_TRUE(bar.memo() == pt.memo()); - ASSERT_TRUE(bar.d == pt.d); - ASSERT_TRUE(bar.rcm() == pt.rcm()); + // Revert to test default + RegtestDeactivateCanopy(); + RegtestDeactivateHeartwood(); + RegtestDeactivateSapling(); } -TEST(noteencryption, SaplingApi) +TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) +{ + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto params = Params().GetConsensus(); + + using namespace libzcash; + auto xsk = SaplingSpendingKey(uint256()).expanded_spending_key(); + auto fvk = xsk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + SaplingPaymentAddress addr = *ivk.address({0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}); + + std::array memo; + for (size_t i = 0; i < ZC_MEMO_SIZE; i++) { + // Fill the message with dummy data + memo[i] = (unsigned char) i; + } + + { + // non-0x01 received before Canopy activation height + SaplingNote note(addr, 39393, 0x02); + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); + + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } + + auto enc = res.get(); + + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); + + ASSERT_FALSE(SaplingNotePlaintext::decrypt( + params, + canopyActivationHeight - 1, + ct, + ivk, + epk, + cmu + )); + } + + { + // non-{0x01,0x02} received after Canopy activation and before grace period has elapsed + SaplingNote note(addr, 39393, 0x03); + int height1 = canopyActivationHeight; + int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 1; + int heights[] = {height1, height2}; + + for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); + + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } + + auto enc = res.get(); + + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); + + ASSERT_FALSE(SaplingNotePlaintext::decrypt( + params, + heights[j], + ct, + ivk, + epk, + cmu + )); + } + } + + { + // non-0x02 received past (Canopy activation height + grace period) + SaplingNote note(addr, 39393, 0x01); + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); + + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } + + auto enc = res.get(); + + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); + + ASSERT_FALSE(SaplingNotePlaintext::decrypt( + params, + canopyActivationHeight + ZIP212_GRACE_PERIOD, + ct, + ivk, + epk, + cmu + )); + } + + // Revert to test default + RegtestDeactivateCanopy(); + RegtestDeactivateHeartwood(); + RegtestDeactivateSapling(); +} + +TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) +{ + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto params = Params().GetConsensus(); + + using namespace libzcash; + auto xsk = SaplingSpendingKey(uint256()).expanded_spending_key(); + auto fvk = xsk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + SaplingPaymentAddress addr = *ivk.address({0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}); + + std::array memo; + for (size_t i = 0; i < ZC_MEMO_SIZE; i++) { + // Fill the message with dummy data + memo[i] = (unsigned char) i; + } + + { + // 0x01 received before Canopy activation height + SaplingNote note(addr, 39393, 0x01); + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); + + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } + + auto enc = res.get(); + + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); + + auto plaintext = SaplingNotePlaintext::decrypt( + params, + canopyActivationHeight - 1, + ct, + ivk, + epk, + cmu + ); + + if (!plaintext) { + FAIL(); + } + } + + { + // {0x01,0x02} received after Canopy activation and before grace period has elapsed + unsigned char leadBytes[] = {0x01, 0x02}; + int height1 = canopyActivationHeight; + int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 1; + int heights[] = {height1, height2}; + + for (int i = 0; i < sizeof(leadBytes); i++) { + for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { + SaplingNote note(addr, 39393, leadBytes[i]); + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); + + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } + + auto enc = res.get(); + + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); + + auto plaintext = SaplingNotePlaintext::decrypt( + params, + heights[j], + ct, + ivk, + epk, + cmu + ); + + if (!plaintext) { + FAIL(); + } + } + } + } + + { + // 0x02 received past (Canopy activation height + grace period) + SaplingNote note(addr, 39393, 0x02); + auto cmu_opt = note.cmu(); + if (!cmu_opt) { + FAIL(); + } + uint256 cmu = cmu_opt.get(); + SaplingNotePlaintext pt(note, memo); + + auto res = pt.encrypt(addr.pk_d); + if (!res) { + FAIL(); + } + + auto enc = res.get(); + + auto ct = enc.first; + auto encryptor = enc.second; + auto epk = encryptor.get_epk(); + + auto plaintext = SaplingNotePlaintext::decrypt( + params, + canopyActivationHeight + ZIP212_GRACE_PERIOD, + ct, + ivk, + epk, + cmu + ); + + if (!plaintext) { + FAIL(); + } + } + + // Revert to test default + RegtestDeactivateCanopy(); + RegtestDeactivateHeartwood(); + RegtestDeactivateSapling(); +} + +TEST(NoteEncryption, SaplingApi) { using namespace libzcash; @@ -358,7 +641,7 @@ TEST(noteencryption, SaplingApi) )); } -TEST(noteencryption, api) +TEST(NoteEncryption, api) { uint256 sk_enc = ZCNoteEncryption::generate_privkey(uint252(uint256S("21035d60bc1983e37950ce4803418a8fb33ea68d5b937ca382ecbae7564d6a07"))); uint256 pk_enc = ZCNoteEncryption::generate_pubkey(sk_enc); @@ -463,7 +746,7 @@ uint256 test_prf( return ret; } -TEST(noteencryption, PrfAddr) +TEST(NoteEncryption, PrfAddr) { for (size_t i = 0; i < 100; i++) { uint252 a_sk = libzcash::random_uint252(); @@ -483,7 +766,7 @@ TEST(noteencryption, PrfAddr) } } -TEST(noteencryption, PrfNf) +TEST(NoteEncryption, PrfNf) { for (size_t i = 0; i < 100; i++) { uint252 a_sk = libzcash::random_uint252(); @@ -494,7 +777,7 @@ TEST(noteencryption, PrfNf) } } -TEST(noteencryption, PrfPk) +TEST(NoteEncryption, PrfPk) { for (size_t i = 0; i < 100; i++) { uint252 a_sk = libzcash::random_uint252(); @@ -517,7 +800,7 @@ TEST(noteencryption, PrfPk) ASSERT_THROW(PRF_pk(dummy_a, 2, dummy_b), std::domain_error); } -TEST(noteencryption, PrfRho) +TEST(NoteEncryption, PrfRho) { for (size_t i = 0; i < 100; i++) { uint252 phi = libzcash::random_uint252(); @@ -540,7 +823,7 @@ TEST(noteencryption, PrfRho) ASSERT_THROW(PRF_rho(dummy_a, 2, dummy_b), std::domain_error); } -TEST(noteencryption, uint252) +TEST(NoteEncryption, uint252) { ASSERT_THROW(uint252(uint256S("f6da8716682d600f74fc16bd0187faad6a26b4aa4c24d5c055b216d94516847e")), std::domain_error); } diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index 3cc382265..633b83915 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -1,5 +1,6 @@ #include "chainparams.h" #include "consensus/params.h" +#include "consensus/consensus.h" #include "consensus/validation.h" #include "key_io.h" #include "main.h" @@ -495,3 +496,123 @@ TEST(TransactionBuilder, CheckSaplingTxVersion) // Revert to default UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } + +TEST(TransactionBuilder, RejectsInvalidNotePlaintextVersion) +{ + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto consensusParams = Params().GetConsensus(); + + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto pk = sk.default_address(); + + SaplingMerkleTree tree; + + { + // non-0x01 received before Canopy activation height + auto builder = TransactionBuilder(consensusParams, canopyActivationHeight - 1); + libzcash::SaplingNote note(pk, 50000, 0x02); + try { + builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); + } catch (std::runtime_error const & err) { + EXPECT_EQ(err.what(), std::string("TransactionBuilder: invalid note plaintext version")); + } catch(...) { + FAIL() << "Expected std::runtime_error"; + } + } + + { + // non-{0x01,0x02} received after Canopy activation and before grace period has elapsed + libzcash::SaplingNote note(pk, 50000, 0x03); + int height1 = canopyActivationHeight - 1; + int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 2; + int heights[] = {height1, height2}; + + for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { + auto builder = TransactionBuilder(consensusParams, heights[j]); + try { + builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); + } catch (std::runtime_error const & err) { + EXPECT_EQ(err.what(), std::string("TransactionBuilder: invalid note plaintext version")); + } catch(...) { + FAIL() << "Expected std::runtime_error"; + } + } + } + + { + // non-0x02 received past (Canopy activation height + grace period) + auto builder = TransactionBuilder(consensusParams, canopyActivationHeight + ZIP212_GRACE_PERIOD); + libzcash::SaplingNote note(pk, 50000, 0x01); + try { + builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); + } catch (std::runtime_error const & err) { + EXPECT_EQ(err.what(), std::string("TransactionBuilder: invalid note plaintext version")); + } catch(...) { + FAIL() << "Expected std::runtime_error"; + } + } + + // Revert to default + RegtestDeactivateCanopy(); + RegtestDeactivateSapling(); +} + +TEST(TransactionBuilder, AcceptsValidNotePlaintextVersion) +{ + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto consensusParams = Params().GetConsensus(); + + auto sk = libzcash::SaplingSpendingKey::random(); + auto expsk = sk.expanded_spending_key(); + auto pk = sk.default_address(); + + SaplingMerkleTree tree; + + { + // 0x01 received before Canopy activation height + auto builder = TransactionBuilder(consensusParams, canopyActivationHeight - 1); + libzcash::SaplingNote note(pk, 50000, 0x01); + ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); + } + + { + // {0x01,0x02} received after Canopy activation and before grace period has elapsed + unsigned char leadBytes[] = {0x01, 0x02}; + int height1 = canopyActivationHeight - 1; + int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 2; + int heights[] = {height1, height2}; + + for (int i = 0; i < sizeof(leadBytes); i++) { + for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { + printf("height %d: %d\n", j, heights[j]); + auto builder = TransactionBuilder(consensusParams, heights[j]); + libzcash::SaplingNote note(pk, 50000, leadBytes[i]); + ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); + } + } + } + + { + // 0x02 received past (Canopy activation height + grace period) + auto builder = TransactionBuilder(consensusParams, canopyActivationHeight + ZIP212_GRACE_PERIOD - 1); + libzcash::SaplingNote note(pk, 50000, 0x02); + ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); + } + + // Revert to default + RegtestDeactivateCanopy(); + RegtestDeactivateSapling(); +} diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 348b2e022..bb29793cb 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -376,57 +376,70 @@ TEST(WalletTests, SetSproutNoteAddrsInCWalletTx) { } TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { - auto consensusParams = RegtestActivateSapling(); + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto consensusParams = Params().GetConsensus(); - TestWallet wallet; - LOCK(wallet.cs_wallet); + unsigned char leadBytes[] = {0x01, 0x02}; + int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - auto sk = GetTestMasterSaplingSpendingKey(); - auto expsk = sk.expsk; - auto fvk = expsk.full_viewing_key(); - auto ivk = fvk.in_viewing_key(); - auto pk = sk.DefaultAddress(); + for (int ver = 0; ver < sizeof(leadBytes); ver++) { + TestWallet wallet; + LOCK(wallet.cs_wallet); - libzcash::SaplingNote note(pk, 50000, 0x01); - auto cm = note.cmu().get(); - SaplingMerkleTree tree; - tree.append(cm); - auto anchor = tree.root(); - auto witness = tree.witness(); + auto sk = GetTestMasterSaplingSpendingKey(); + auto expsk = sk.expsk; + auto fvk = expsk.full_viewing_key(); + auto ivk = fvk.in_viewing_key(); + auto pk = sk.DefaultAddress(); - auto nf = note.nullifier(fvk, witness.position()); - ASSERT_TRUE(nf); - uint256 nullifier = nf.get(); + libzcash::SaplingNote note(pk, 50000, leadBytes[ver]); + auto cm = note.cmu().get(); + SaplingMerkleTree tree; + tree.append(cm); + auto anchor = tree.root(); + auto witness = tree.witness(); - auto builder = TransactionBuilder(consensusParams, 1); - builder.AddSaplingSpend(expsk, note, anchor, witness); - builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); - builder.SetFee(0); - auto tx = builder.Build().GetTxOrThrow(); + auto nf = note.nullifier(fvk, witness.position()); + ASSERT_TRUE(nf); + uint256 nullifier = nf.get(); - CWalletTx wtx {&wallet, tx}; + auto builder = TransactionBuilder(consensusParams, builderHeights[ver]); + builder.AddSaplingSpend(expsk, note, anchor, witness); + builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); + builder.SetFee(0); + auto tx = builder.Build().GetTxOrThrow(); - EXPECT_EQ(0, wtx.mapSaplingNoteData.size()); - mapSaplingNoteData_t noteData; + CWalletTx wtx {&wallet, tx}; - SaplingOutPoint op {wtx.GetHash(), 0}; - SaplingNoteData nd; - nd.nullifier = nullifier; - nd.ivk = ivk; - nd.witnesses.push_front(witness); - nd.witnessHeight = 123; - noteData.insert(std::make_pair(op, nd)); + EXPECT_EQ(0, wtx.mapSaplingNoteData.size()); + mapSaplingNoteData_t noteData; - wtx.SetSaplingNoteData(noteData); - EXPECT_EQ(noteData, wtx.mapSaplingNoteData); + SaplingOutPoint op {wtx.GetHash(), 0}; + SaplingNoteData nd; + nd.nullifier = nullifier; + nd.ivk = ivk; + nd.witnesses.push_front(witness); + nd.witnessHeight = 123; + noteData.insert(std::make_pair(op, nd)); - // Test individual fields in case equality operator is defined/changed. - EXPECT_EQ(ivk, wtx.mapSaplingNoteData[op].ivk); - EXPECT_EQ(nullifier, wtx.mapSaplingNoteData[op].nullifier); - EXPECT_EQ(nd.witnessHeight, wtx.mapSaplingNoteData[op].witnessHeight); - EXPECT_TRUE(witness == wtx.mapSaplingNoteData[op].witnesses.front()); + wtx.SetSaplingNoteData(noteData); + EXPECT_EQ(noteData, wtx.mapSaplingNoteData); + + // Test individual fields in case equality operator is defined/changed. + EXPECT_EQ(ivk, wtx.mapSaplingNoteData[op].ivk); + EXPECT_EQ(nullifier, wtx.mapSaplingNoteData[op].nullifier); + EXPECT_EQ(nd.witnessHeight, wtx.mapSaplingNoteData[op].witnessHeight); + EXPECT_TRUE(witness == wtx.mapSaplingNoteData[op].witnesses.front()); + } // Revert to default + RegtestDeactivateCanopy(); RegtestDeactivateSapling(); } @@ -534,13 +547,13 @@ TEST(WalletTests, FindMySaplingNotes) { // No Sapling notes can be found in tx which does not belong to the wallet CWalletTx wtx {&wallet, tx}; ASSERT_FALSE(wallet.HaveSaplingSpendingKey(extfvk)); - auto noteMap = wallet.FindMySaplingNotes(wtx).first; + auto noteMap = wallet.FindMySaplingNotes(wtx, 1).first; EXPECT_EQ(0, noteMap.size()); // Add spending key to wallet, so Sapling notes can be found ASSERT_TRUE(wallet.AddSaplingZKey(sk)); ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); - noteMap = wallet.FindMySaplingNotes(wtx).first; + noteMap = wallet.FindMySaplingNotes(wtx, 1).first; EXPECT_EQ(2, noteMap.size()); // Revert to default @@ -638,123 +651,136 @@ TEST(WalletTests, GetConflictedSproutNotes) { // Generate note A and spend to create note B, from which we spend to create two conflicting transactions TEST(WalletTests, GetConflictedSaplingNotes) { - auto consensusParams = RegtestActivateSapling(); + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto consensusParams = Params().GetConsensus(); - TestWallet wallet; - LOCK2(cs_main, wallet.cs_wallet); + unsigned char leadBytes[] = {0x01, 0x02}; + int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - // Generate Sapling address - auto sk = GetTestMasterSaplingSpendingKey(); - auto expsk = sk.expsk; - auto extfvk = sk.ToXFVK(); - auto ivk = extfvk.fvk.in_viewing_key(); - auto pk = sk.DefaultAddress(); + for (int ver = 0; ver < sizeof(leadBytes); ver++) { + TestWallet wallet; + LOCK2(cs_main, wallet.cs_wallet); - ASSERT_TRUE(wallet.AddSaplingZKey(sk)); - ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); + // Generate Sapling address + auto sk = GetTestMasterSaplingSpendingKey(); + auto expsk = sk.expsk; + auto extfvk = sk.ToXFVK(); + auto ivk = extfvk.fvk.in_viewing_key(); + auto pk = sk.DefaultAddress(); - // Generate note A - libzcash::SaplingNote note(pk, 50000, 0x01); - auto cm = note.cmu().get(); - SaplingMerkleTree saplingTree; - saplingTree.append(cm); - auto anchor = saplingTree.root(); - auto witness = saplingTree.witness(); + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); - // Generate tx to create output note B - auto builder = TransactionBuilder(consensusParams, 1); - builder.AddSaplingSpend(expsk, note, anchor, witness); - builder.AddSaplingOutput(extfvk.fvk.ovk, pk, 35000, {}); - auto tx = builder.Build().GetTxOrThrow(); - CWalletTx wtx {&wallet, tx}; + // Generate note A + libzcash::SaplingNote note(pk, 50000, leadBytes[ver]); + auto cm = note.cmu().get(); + SaplingMerkleTree saplingTree; + saplingTree.append(cm); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); - // Fake-mine the transaction - EXPECT_EQ(-1, chainActive.Height()); - SproutMerkleTree sproutTree; - CBlock block; - block.vtx.push_back(wtx); - 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()); + // Generate tx to create output note B + auto builder = TransactionBuilder(consensusParams, builderHeights[ver]); + builder.AddSaplingSpend(expsk, note, anchor, witness); + builder.AddSaplingOutput(extfvk.fvk.ovk, pk, 35000, {}); + auto tx = builder.Build().GetTxOrThrow(); + CWalletTx wtx {&wallet, tx}; - // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe - auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first; - ASSERT_TRUE(saplingNoteData.size() > 0); - wtx.SetSaplingNoteData(saplingNoteData); - wtx.SetMerkleBranch(block); - wallet.AddToWallet(wtx, true, NULL); + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + 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()); - // Simulate receiving new block and ChainTip signal - wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); - wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, builderHeights[ver]).first; + ASSERT_TRUE(saplingNoteData.size() > 0); + wtx.SetSaplingNoteData(saplingNoteData); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); - // Retrieve the updated wtx from wallet - uint256 hash = wtx.GetHash(); - wtx = wallet.mapWallet[hash]; + // Simulate receiving new block and ChainTip signal + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); - // Decrypt output note B - auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( - consensusParams, - wtx.nExpiryHeight, - wtx.vShieldedOutput[0].encCiphertext, - ivk, - wtx.vShieldedOutput[0].ephemeralKey, - wtx.vShieldedOutput[0].cmu); - ASSERT_EQ(static_cast(maybe_pt), true); - auto maybe_note = maybe_pt.get().note(ivk); - ASSERT_EQ(static_cast(maybe_note), true); - auto note2 = maybe_note.get(); + // Retrieve the updated wtx from wallet + uint256 hash = wtx.GetHash(); + wtx = wallet.mapWallet[hash]; - SaplingOutPoint sop0(wtx.GetHash(), 0); - auto spend_note_witness = wtx.mapSaplingNoteData[sop0].witnesses.front(); - auto maybe_nf = note2.nullifier(extfvk.fvk, spend_note_witness.position()); - ASSERT_EQ(static_cast(maybe_nf), true); - auto nullifier2 = maybe_nf.get(); + // Decrypt output note B + auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + consensusParams, + wtx.nExpiryHeight, + wtx.vShieldedOutput[0].encCiphertext, + ivk, + wtx.vShieldedOutput[0].ephemeralKey, + wtx.vShieldedOutput[0].cmu); + ASSERT_EQ(static_cast(maybe_pt), true); + auto maybe_note = maybe_pt.get().note(ivk); + ASSERT_EQ(static_cast(maybe_note), true); + auto note2 = maybe_note.get(); - anchor = saplingTree.root(); + SaplingOutPoint sop0(wtx.GetHash(), 0); + auto spend_note_witness = wtx.mapSaplingNoteData[sop0].witnesses.front(); + auto maybe_nf = note2.nullifier(extfvk.fvk, spend_note_witness.position()); + ASSERT_EQ(static_cast(maybe_nf), true); + auto nullifier2 = maybe_nf.get(); - // Create transaction to spend note B - auto builder2 = TransactionBuilder(consensusParams, 2); - builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); - builder2.AddSaplingOutput(extfvk.fvk.ovk, pk, 20000, {}); - auto tx2 = builder2.Build().GetTxOrThrow(); + anchor = saplingTree.root(); - // Create conflicting transaction which also spends note B - auto builder3 = TransactionBuilder(consensusParams, 2); - builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); - builder3.AddSaplingOutput(extfvk.fvk.ovk, pk, 19999, {}); - auto tx3 = builder3.Build().GetTxOrThrow(); + // Create transaction to spend note B + auto builder2 = TransactionBuilder(consensusParams, builderHeights[ver] + 1); + builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); + builder2.AddSaplingOutput(extfvk.fvk.ovk, pk, 20000, {}); + auto tx2 = builder2.Build().GetTxOrThrow(); - CWalletTx wtx2 {&wallet, tx2}; - CWalletTx wtx3 {&wallet, tx3}; + // Create conflicting transaction which also spends note B + auto builder3 = TransactionBuilder(consensusParams, builderHeights[ver] + 1); + builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); + builder3.AddSaplingOutput(extfvk.fvk.ovk, pk, 19999, {}); + auto tx3 = builder3.Build().GetTxOrThrow(); - auto hash2 = wtx2.GetHash(); - auto hash3 = wtx3.GetHash(); + CWalletTx wtx2 {&wallet, tx2}; + CWalletTx wtx3 {&wallet, tx3}; - // No conflicts for no spends (wtx is currently the only transaction in the wallet) - EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); - EXPECT_EQ(0, wallet.GetConflicts(hash3).size()); + auto hash2 = wtx2.GetHash(); + auto hash3 = wtx3.GetHash(); - // No conflicts for one spend - wallet.AddToWallet(wtx2, true, NULL); - EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); + // No conflicts for no spends (wtx is currently the only transaction in the wallet) + EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); + EXPECT_EQ(0, wallet.GetConflicts(hash3).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); + // No conflicts for one spend + wallet.AddToWallet(wtx2, true, NULL); + EXPECT_EQ(0, wallet.GetConflicts(hash2).size()); - // Tear down - chainActive.SetTip(NULL); - mapBlockIndex.erase(blockHash); + // 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); - // Revert to default - RegtestDeactivateSapling(); + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); + } + + // Revert to test default + RegtestDeactivateCanopy(); + RegtestActivateSapling(); } TEST(WalletTests, SproutNullifierIsSpent) { @@ -930,7 +956,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe wtx.SetMerkleBranch(block); - auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first; + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, chainActive.Height()).first; ASSERT_TRUE(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); wallet.AddToWallet(wtx, true, NULL); @@ -1007,146 +1033,159 @@ TEST(WalletTests, SpentSproutNoteIsFromMe) { // Create note A, spend A to create note B, spend and verify note B is from me. TEST(WalletTests, SpentSaplingNoteIsFromMe) { - auto consensusParams = RegtestActivateSapling(); + SelectParams(CBaseChainParams::REGTEST); + int overwinterActivationHeight = 5; + int saplingActivationHeight = 30; + int canopyActivationHeight = 70; + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); + UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); + auto consensusParams = Params().GetConsensus(); - TestWallet wallet; - LOCK2(cs_main, wallet.cs_wallet); + unsigned char leadBytes[] = {0x01, 0x02}; + int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - // Generate Sapling address - auto sk = GetTestMasterSaplingSpendingKey(); - auto expsk = sk.expsk; - auto extfvk = sk.ToXFVK(); - auto ivk = extfvk.fvk.in_viewing_key(); - auto pk = sk.DefaultAddress(); + for (int ver = 0; ver < sizeof(leadBytes); ver++) { + TestWallet wallet; + LOCK2(cs_main, wallet.cs_wallet); - // Generate Sapling note A - libzcash::SaplingNote note(pk, 50000, 0x01); - auto cm = note.cmu().get(); - SaplingMerkleTree saplingTree; - saplingTree.append(cm); - auto anchor = saplingTree.root(); - auto witness = saplingTree.witness(); + // Generate Sapling address + auto sk = GetTestMasterSaplingSpendingKey(); + auto expsk = sk.expsk; + auto extfvk = sk.ToXFVK(); + auto ivk = extfvk.fvk.in_viewing_key(); + auto pk = sk.DefaultAddress(); - // Generate transaction, which sends funds to note B - auto builder = TransactionBuilder(consensusParams, 1); - builder.AddSaplingSpend(expsk, note, anchor, witness); - builder.AddSaplingOutput(extfvk.fvk.ovk, pk, 25000, {}); - auto tx = builder.Build().GetTxOrThrow(); + // Generate Sapling note A + libzcash::SaplingNote note(pk, 50000, leadBytes[ver]); + auto cm = note.cmu().get(); + SaplingMerkleTree saplingTree; + saplingTree.append(cm); + auto anchor = saplingTree.root(); + auto witness = saplingTree.witness(); - CWalletTx wtx {&wallet, tx}; - ASSERT_TRUE(wallet.AddSaplingZKey(sk)); - ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); + // Generate transaction, which sends funds to note B + auto builder = TransactionBuilder(consensusParams, builderHeights[ver]); + builder.AddSaplingSpend(expsk, note, anchor, witness); + builder.AddSaplingOutput(extfvk.fvk.ovk, pk, 25000, {}); + auto tx = builder.Build().GetTxOrThrow(); - // Fake-mine the transaction - EXPECT_EQ(-1, chainActive.Height()); - SproutMerkleTree sproutTree; - CBlock block; - block.vtx.push_back(wtx); - 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()); + CWalletTx wtx {&wallet, tx}; + ASSERT_TRUE(wallet.AddSaplingZKey(sk)); + ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); - auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first; - ASSERT_TRUE(saplingNoteData.size() > 0); - wtx.SetSaplingNoteData(saplingNoteData); - wtx.SetMerkleBranch(block); - wallet.AddToWallet(wtx, true, NULL); + // Fake-mine the transaction + EXPECT_EQ(-1, chainActive.Height()); + SproutMerkleTree sproutTree; + CBlock block; + block.vtx.push_back(wtx); + 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()); - // Simulate receiving new block and ChainTip signal. - // This triggers calculation of nullifiers for notes belonging to this wallet - // in the output descriptions of wtx. - wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); - wallet.UpdateSaplingNullifierNoteMapForBlock(&block); + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, builderHeights[ver]).first; + ASSERT_TRUE(saplingNoteData.size() > 0); + wtx.SetSaplingNoteData(saplingNoteData); + wtx.SetMerkleBranch(block); + wallet.AddToWallet(wtx, true, NULL); - // Retrieve the updated wtx from wallet - wtx = wallet.mapWallet[wtx.GetHash()]; + // Simulate receiving new block and ChainTip signal. + // This triggers calculation of nullifiers for notes belonging to this wallet + // in the output descriptions of wtx. + wallet.IncrementNoteWitnesses(&fakeIndex, &block, sproutTree, saplingTree); + wallet.UpdateSaplingNullifierNoteMapForBlock(&block); - // The test wallet never received the fake note which is being spent, so there - // is no mapping from nullifier to notedata stored in mapSaplingNullifiersToNotes. - // Therefore the wallet does not know the tx belongs to the wallet. - EXPECT_FALSE(wallet.IsFromMe(wtx)); + // Retrieve the updated wtx from wallet + wtx = wallet.mapWallet[wtx.GetHash()]; - // Manually compute the nullifier and check map entry does not exist - auto nf = note.nullifier(extfvk.fvk, witness.position()); - ASSERT_TRUE(nf); - ASSERT_FALSE(wallet.mapSaplingNullifiersToNotes.count(nf.get())); + // The test wallet never received the fake note which is being spent, so there + // is no mapping from nullifier to notedata stored in mapSaplingNullifiersToNotes. + // Therefore the wallet does not know the tx belongs to the wallet. + EXPECT_FALSE(wallet.IsFromMe(wtx)); - // Decrypt note B - auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( - consensusParams, - wtx.nExpiryHeight, - wtx.vShieldedOutput[0].encCiphertext, - ivk, - wtx.vShieldedOutput[0].ephemeralKey, - wtx.vShieldedOutput[0].cmu); - ASSERT_EQ(static_cast(maybe_pt), true); - auto maybe_note = maybe_pt.get().note(ivk); - ASSERT_EQ(static_cast(maybe_note), true); - auto note2 = maybe_note.get(); + // Manually compute the nullifier and check map entry does not exist + auto nf = note.nullifier(extfvk.fvk, witness.position()); + ASSERT_TRUE(nf); + ASSERT_FALSE(wallet.mapSaplingNullifiersToNotes.count(nf.get())); - // Get witness to retrieve position of note B we want to spend - SaplingOutPoint sop0(wtx.GetHash(), 0); - auto spend_note_witness = wtx.mapSaplingNoteData[sop0].witnesses.front(); - auto maybe_nf = note2.nullifier(extfvk.fvk, spend_note_witness.position()); - ASSERT_EQ(static_cast(maybe_nf), true); - auto nullifier2 = maybe_nf.get(); + // Decrypt note B + auto maybe_pt = libzcash::SaplingNotePlaintext::decrypt( + consensusParams, + wtx.nExpiryHeight, + wtx.vShieldedOutput[0].encCiphertext, + ivk, + wtx.vShieldedOutput[0].ephemeralKey, + wtx.vShieldedOutput[0].cmu); + ASSERT_EQ(static_cast(maybe_pt), true); + auto maybe_note = maybe_pt.get().note(ivk); + ASSERT_EQ(static_cast(maybe_note), true); + auto note2 = maybe_note.get(); - // NOTE: Not updating the anchor results in a core dump. Shouldn't builder just return error? - // *** Error in `./zcash-gtest': double free or corruption (out): 0x00007ffd8755d990 *** - anchor = saplingTree.root(); + // Get witness to retrieve position of note B we want to spend + SaplingOutPoint sop0(wtx.GetHash(), 0); + auto spend_note_witness = wtx.mapSaplingNoteData[sop0].witnesses.front(); + auto maybe_nf = note2.nullifier(extfvk.fvk, spend_note_witness.position()); + ASSERT_EQ(static_cast(maybe_nf), true); + auto nullifier2 = maybe_nf.get(); - // Create transaction to spend note B - auto builder2 = TransactionBuilder(consensusParams, 2); - builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); - builder2.AddSaplingOutput(extfvk.fvk.ovk, pk, 12500, {}); - auto tx2 = builder2.Build().GetTxOrThrow(); - EXPECT_EQ(tx2.vin.size(), 0); - EXPECT_EQ(tx2.vout.size(), 0); - EXPECT_EQ(tx2.vJoinSplit.size(), 0); - EXPECT_EQ(tx2.vShieldedSpend.size(), 1); - EXPECT_EQ(tx2.vShieldedOutput.size(), 2); - EXPECT_EQ(tx2.valueBalance, 10000); + // NOTE: Not updating the anchor results in a core dump. Shouldn't builder just return error? + // *** Error in `./zcash-gtest': double free or corruption (out): 0x00007ffd8755d990 *** + anchor = saplingTree.root(); - CWalletTx wtx2 {&wallet, tx2}; + // Create transaction to spend note B + auto builder2 = TransactionBuilder(consensusParams, builderHeights[ver] + 1); + builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); + builder2.AddSaplingOutput(extfvk.fvk.ovk, pk, 12500, {}); + auto tx2 = builder2.Build().GetTxOrThrow(); + EXPECT_EQ(tx2.vin.size(), 0); + EXPECT_EQ(tx2.vout.size(), 0); + EXPECT_EQ(tx2.vJoinSplit.size(), 0); + EXPECT_EQ(tx2.vShieldedSpend.size(), 1); + EXPECT_EQ(tx2.vShieldedOutput.size(), 2); + EXPECT_EQ(tx2.valueBalance, 10000); - // Fake-mine this tx into the next block - EXPECT_EQ(0, chainActive.Height()); - CBlock block2; - block2.vtx.push_back(wtx2); - block2.hashMerkleRoot = block2.BuildMerkleTree(); - block2.hashPrevBlock = blockHash; - auto blockHash2 = block2.GetHash(); - CBlockIndex fakeIndex2 {block2}; - mapBlockIndex.insert(std::make_pair(blockHash2, &fakeIndex2)); - fakeIndex2.nHeight = 1; - chainActive.SetTip(&fakeIndex2); - EXPECT_TRUE(chainActive.Contains(&fakeIndex2)); - EXPECT_EQ(1, chainActive.Height()); + CWalletTx wtx2 {&wallet, tx2}; - auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2).first; - ASSERT_TRUE(saplingNoteData2.size() > 0); - wtx2.SetSaplingNoteData(saplingNoteData2); - wtx2.SetMerkleBranch(block2); - wallet.AddToWallet(wtx2, true, NULL); + // Fake-mine this tx into the next block + EXPECT_EQ(0, chainActive.Height()); + CBlock block2; + block2.vtx.push_back(wtx2); + block2.hashMerkleRoot = block2.BuildMerkleTree(); + block2.hashPrevBlock = blockHash; + auto blockHash2 = block2.GetHash(); + CBlockIndex fakeIndex2 {block2}; + mapBlockIndex.insert(std::make_pair(blockHash2, &fakeIndex2)); + fakeIndex2.nHeight = 1; + chainActive.SetTip(&fakeIndex2); + EXPECT_TRUE(chainActive.Contains(&fakeIndex2)); + EXPECT_EQ(1, chainActive.Height()); - // Verify note B is spent. AddToWallet invokes AddToSpends which updates mapTxSaplingNullifiers - EXPECT_TRUE(wallet.IsSaplingSpent(nullifier2)); + auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2, builderHeights[ver]).first; + ASSERT_TRUE(saplingNoteData2.size() > 0); + wtx2.SetSaplingNoteData(saplingNoteData2); + wtx2.SetMerkleBranch(block2); + wallet.AddToWallet(wtx2, true, NULL); - // Verify note B belongs to wallet. - EXPECT_TRUE(wallet.IsFromMe(wtx2)); - ASSERT_TRUE(wallet.mapSaplingNullifiersToNotes.count(nullifier2)); + // Verify note B is spent. AddToWallet invokes AddToSpends which updates mapTxSaplingNullifiers + EXPECT_TRUE(wallet.IsSaplingSpent(nullifier2)); - // Tear down - chainActive.SetTip(NULL); - mapBlockIndex.erase(blockHash); - mapBlockIndex.erase(blockHash2); + // Verify note B belongs to wallet. + EXPECT_TRUE(wallet.IsFromMe(wtx2)); + ASSERT_TRUE(wallet.mapSaplingNullifiersToNotes.count(nullifier2)); - // Revert to default - RegtestDeactivateSapling(); + // Tear down + chainActive.SetTip(NULL); + mapBlockIndex.erase(blockHash); + mapBlockIndex.erase(blockHash2); + } + + // Revert to test default + RegtestDeactivateCanopy(); + RegtestActivateSapling(); } TEST(WalletTests, CachedWitnessesEmptyChain) { @@ -1847,7 +1886,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { EXPECT_EQ(0, chainActive.Height()); // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe - auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first; + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, chainActive.Height()).first; ASSERT_TRUE(saplingNoteData.size() == 1); // wallet only has key for change output wtx.SetSaplingNoteData(saplingNoteData); wtx.SetMerkleBranch(block); @@ -1865,7 +1904,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { ASSERT_TRUE(wallet.AddSaplingZKey(sk2)); ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk2)); CWalletTx wtx2 = wtx; - auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2).first; + auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2, chainActive.Height()).first; ASSERT_TRUE(saplingNoteData2.size() == 2); wtx2.SetSaplingNoteData(saplingNoteData2); @@ -1994,7 +2033,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { EXPECT_EQ(0, chainActive.Height()); // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe - auto saplingNoteData = wallet.FindMySaplingNotes(wtx).first; + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, chainActive.Height()).first; ASSERT_TRUE(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); wtx.SetMerkleBranch(block); From f24e706079972225c048ca55a8dd700c59a2e939 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Tue, 30 Jun 2020 05:04:48 +0800 Subject: [PATCH 12/21] Replace leadByte in SaplingNote with is_zip_212 --- qa/rpc-tests/test_framework/util.py | 4 +- src/gtest/test_checktransaction.cpp | 6 +-- src/gtest/test_noteencryption.cpp | 57 +++++--------------------- src/gtest/test_sapling_note.cpp | 6 +-- src/gtest/test_transaction_builder.cpp | 36 ++++------------ src/main.cpp | 2 +- src/miner.cpp | 6 +-- src/transaction_builder.cpp | 13 ++++-- src/utiltest.cpp | 2 +- src/wallet/gtest/test_wallet.cpp | 18 ++++---- src/wallet/wallet.cpp | 8 ++-- src/zcash/Note.cpp | 31 ++++++++------ src/zcash/Note.hpp | 32 +++++++-------- src/zcbenchmarks.cpp | 4 +- 14 files changed, 88 insertions(+), 137 deletions(-) diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index c7773d0b5..05885d122 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -223,7 +223,7 @@ def initialize_chain(test_dir): print("initialize_chain: bitcoind started, waiting for RPC to come up") wait_for_bitcoind_start(bitcoind_processes[i], rpc_url(i), i) if os.getenv("PYTHON_DEBUG", ""): - print("initialize_chain: RPC succesfully started") + print("initialize_chain: RPC successfully started") rpcs = [] for i in range(4): @@ -313,7 +313,7 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary= url = rpc_url(i, rpchost) wait_for_bitcoind_start(bitcoind_processes[i], url, i) if os.getenv("PYTHON_DEBUG", ""): - print("start_node: RPC succesfully started") + print("start_node: RPC successfully started") proxy = get_rpc_proxy(url, i, timeout=timewait) if COVERAGE_DIR: diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 0879bfb01..2ae731ae0 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -1134,7 +1134,7 @@ TEST(CheckTransaction, HeartwoodAcceptsShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), 0x01); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), false); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); @@ -1217,7 +1217,7 @@ TEST(CheckTransaction, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), 0x01); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), false); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); CMutableTransaction mtx = GetValidTransaction(); @@ -1294,7 +1294,7 @@ TEST(CheckTransaction, CanopyEnforcesSaplingRulesOnShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), 0x02); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), true); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); CMutableTransaction mtx = GetValidTransaction(); diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index 22357d0ed..9d8ef2feb 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -33,7 +33,7 @@ TEST(NoteEncryption, NotePlaintext) UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto params = Params().GetConsensus(); - unsigned char leadBytes[] = {0x01, 0x02}; + bool is_zip_212[] = {false, true}; int decryptionHeights[] = {saplingActivationHeight, canopyActivationHeight}; using namespace libzcash; @@ -48,8 +48,8 @@ TEST(NoteEncryption, NotePlaintext) memo[i] = (unsigned char) i; } - for (int ver = 0; ver < sizeof(leadBytes); ver++){ - SaplingNote note(addr, 39393, leadBytes[ver]); + for (int ver = 0; ver < sizeof(is_zip_212); ver++){ + SaplingNote note(addr, 39393, is_zip_212[ver]); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -212,7 +212,7 @@ TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) { // non-0x01 received before Canopy activation height - SaplingNote note(addr, 39393, 0x02); + SaplingNote note(addr, 39393, true); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -241,46 +241,9 @@ TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) )); } - { - // non-{0x01,0x02} received after Canopy activation and before grace period has elapsed - SaplingNote note(addr, 39393, 0x03); - int height1 = canopyActivationHeight; - int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 1; - int heights[] = {height1, height2}; - - for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { - auto cmu_opt = note.cmu(); - if (!cmu_opt) { - FAIL(); - } - uint256 cmu = cmu_opt.get(); - SaplingNotePlaintext pt(note, memo); - - auto res = pt.encrypt(addr.pk_d); - if (!res) { - FAIL(); - } - - auto enc = res.get(); - - auto ct = enc.first; - auto encryptor = enc.second; - auto epk = encryptor.get_epk(); - - ASSERT_FALSE(SaplingNotePlaintext::decrypt( - params, - heights[j], - ct, - ivk, - epk, - cmu - )); - } - } - { // non-0x02 received past (Canopy activation height + grace period) - SaplingNote note(addr, 39393, 0x01); + SaplingNote note(addr, 39393, false); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -340,7 +303,7 @@ TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) { // 0x01 received before Canopy activation height - SaplingNote note(addr, 39393, 0x01); + SaplingNote note(addr, 39393, false); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -375,14 +338,14 @@ TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) { // {0x01,0x02} received after Canopy activation and before grace period has elapsed - unsigned char leadBytes[] = {0x01, 0x02}; + bool is_zip_212[] = {false, true}; int height1 = canopyActivationHeight; int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 1; int heights[] = {height1, height2}; - for (int i = 0; i < sizeof(leadBytes); i++) { + for (int i = 0; i < sizeof(is_zip_212); i++) { for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { - SaplingNote note(addr, 39393, leadBytes[i]); + SaplingNote note(addr, 39393, is_zip_212[i]); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -419,7 +382,7 @@ TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) { // 0x02 received past (Canopy activation height + grace period) - SaplingNote note(addr, 39393, 0x02); + SaplingNote note(addr, 39393, true); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); diff --git a/src/gtest/test_sapling_note.cpp b/src/gtest/test_sapling_note.cpp index b1da05da3..59abbfd5a 100644 --- a/src/gtest/test_sapling_note.cpp +++ b/src/gtest/test_sapling_note.cpp @@ -57,8 +57,8 @@ TEST(SaplingNote, Random) { // Test creating random notes using the same spending key auto address = SaplingSpendingKey::random().default_address(); - SaplingNote note1(address, GetRand(MAX_MONEY), 0x01); - SaplingNote note2(address, GetRand(MAX_MONEY), 0x01); + SaplingNote note1(address, GetRand(MAX_MONEY), false); + SaplingNote note2(address, GetRand(MAX_MONEY), false); ASSERT_EQ(note1.d, note2.d); ASSERT_EQ(note1.pk_d, note2.pk_d); @@ -66,7 +66,7 @@ TEST(SaplingNote, Random) ASSERT_NE(note1.rcm(), note2.rcm()); // Test diversifier and pk_d are not the same for different spending keys - SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY), 0x01); + SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY), false); ASSERT_NE(note1.d, note3.d); ASSERT_NE(note1.pk_d, note3.pk_d); } diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index 633b83915..2f3006461 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -483,7 +483,7 @@ TEST(TransactionBuilder, CheckSaplingTxVersion) } // Cannot add Sapling spends to a non-Sapling transaction - libzcash::SaplingNote note(pk, 50000, 0x01); + libzcash::SaplingNote note(pk, 50000, false); SaplingMerkleTree tree; try { builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); @@ -517,7 +517,7 @@ TEST(TransactionBuilder, RejectsInvalidNotePlaintextVersion) { // non-0x01 received before Canopy activation height auto builder = TransactionBuilder(consensusParams, canopyActivationHeight - 1); - libzcash::SaplingNote note(pk, 50000, 0x02); + libzcash::SaplingNote note(pk, 50000, true); try { builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); } catch (std::runtime_error const & err) { @@ -527,29 +527,10 @@ TEST(TransactionBuilder, RejectsInvalidNotePlaintextVersion) } } - { - // non-{0x01,0x02} received after Canopy activation and before grace period has elapsed - libzcash::SaplingNote note(pk, 50000, 0x03); - int height1 = canopyActivationHeight - 1; - int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 2; - int heights[] = {height1, height2}; - - for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { - auto builder = TransactionBuilder(consensusParams, heights[j]); - try { - builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); - } catch (std::runtime_error const & err) { - EXPECT_EQ(err.what(), std::string("TransactionBuilder: invalid note plaintext version")); - } catch(...) { - FAIL() << "Expected std::runtime_error"; - } - } - } - { // non-0x02 received past (Canopy activation height + grace period) auto builder = TransactionBuilder(consensusParams, canopyActivationHeight + ZIP212_GRACE_PERIOD); - libzcash::SaplingNote note(pk, 50000, 0x01); + libzcash::SaplingNote note(pk, 50000, false); try { builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); } catch (std::runtime_error const & err) { @@ -584,22 +565,21 @@ TEST(TransactionBuilder, AcceptsValidNotePlaintextVersion) { // 0x01 received before Canopy activation height auto builder = TransactionBuilder(consensusParams, canopyActivationHeight - 1); - libzcash::SaplingNote note(pk, 50000, 0x01); + libzcash::SaplingNote note(pk, 50000, false); ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); } { // {0x01,0x02} received after Canopy activation and before grace period has elapsed - unsigned char leadBytes[] = {0x01, 0x02}; + unsigned char is_zip_212[] = {false, true}; int height1 = canopyActivationHeight - 1; int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 2; int heights[] = {height1, height2}; - for (int i = 0; i < sizeof(leadBytes); i++) { + for (int i = 0; i < sizeof(is_zip_212); i++) { for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { - printf("height %d: %d\n", j, heights[j]); auto builder = TransactionBuilder(consensusParams, heights[j]); - libzcash::SaplingNote note(pk, 50000, leadBytes[i]); + libzcash::SaplingNote note(pk, 50000, is_zip_212[i]); ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); } } @@ -608,7 +588,7 @@ TEST(TransactionBuilder, AcceptsValidNotePlaintextVersion) { // 0x02 received past (Canopy activation height + grace period) auto builder = TransactionBuilder(consensusParams, canopyActivationHeight + ZIP212_GRACE_PERIOD - 1); - libzcash::SaplingNote note(pk, 50000, 0x02); + libzcash::SaplingNote note(pk, 50000, true); ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); } diff --git a/src/main.cpp b/src/main.cpp index 8e2ebaed9..08b522cc2 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -938,7 +938,7 @@ bool ContextualCheckTransaction( // ZIP 212: Check that the note plaintexts use the v2 note plaintext // version. - if (canopyActive != (encPlaintext->get_lead_byte() == 0x02)) { + if (canopyActive != (encPlaintext->get_leadbyte() == 0x02)) { return state.DoS( DOS_LEVEL_BLOCK, error("CheckTransaction(): coinbase output description has invalid note plaintext version"), diff --git a/src/miner.cpp b/src/miner.cpp index ddcdbf2bd..1a91ac7d3 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -157,11 +157,7 @@ public: mtx.valueBalance = -value; uint256 ovk; - unsigned char leadByte = 0x01; - if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { - leadByte = 0x02; - } - auto note = libzcash::SaplingNote(pa, value, leadByte); + auto note = libzcash::SaplingNote(pa, value, (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY))); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 1d6eddbb5..613b90add 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -143,8 +143,13 @@ void TransactionBuilder::AddSaplingSpend( throw std::runtime_error("TransactionBuilder cannot add Sapling spend to pre-Sapling transaction"); } + unsigned char leadbyte = 0x01; + if (note.get_is_zip_212() == true) { + leadbyte = 0x02; + } + // ZIP212: check that note plaintext lead byte is valid at height - if (!libzcash::plaintext_version_is_valid(consensusParams, nHeight + 1, note.get_lead_byte())) { + if (!libzcash::plaintext_version_is_valid(consensusParams, nHeight + 1, leadbyte)) { throw std::runtime_error("TransactionBuilder: invalid note plaintext version"); } @@ -168,11 +173,11 @@ void TransactionBuilder::AddSaplingOutput( throw std::runtime_error("TransactionBuilder cannot add Sapling output to pre-Sapling transaction"); } - unsigned char leadByte = 0x01; + bool is_zip_212 = false; if (Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)) { - leadByte = 0x02; + is_zip_212 = true; } - auto note = libzcash::SaplingNote(to, value, leadByte); + auto note = libzcash::SaplingNote(to, value, is_zip_212); outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } diff --git a/src/utiltest.cpp b/src/utiltest.cpp index bb75a9504..6b695e205 100644 --- a/src/utiltest.cpp +++ b/src/utiltest.cpp @@ -289,7 +289,7 @@ CKey AddTestCKeyToKeyStore(CBasicKeyStore& keyStore) { TestSaplingNote GetTestSaplingNote(const libzcash::SaplingPaymentAddress& pa, CAmount value) { // Generate dummy Sapling note - libzcash::SaplingNote note(pa, value, 0x01); + libzcash::SaplingNote note(pa, value, false); uint256 cm = note.cmu().get(); SaplingMerkleTree tree; tree.append(cm); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index bb29793cb..345c28202 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -385,10 +385,10 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto consensusParams = Params().GetConsensus(); - unsigned char leadBytes[] = {0x01, 0x02}; + bool is_zip_212[] = {false, true}; int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - for (int ver = 0; ver < sizeof(leadBytes); ver++) { + for (int ver = 0; ver < sizeof(is_zip_212); ver++) { TestWallet wallet; LOCK(wallet.cs_wallet); @@ -398,7 +398,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto ivk = fvk.in_viewing_key(); auto pk = sk.DefaultAddress(); - libzcash::SaplingNote note(pk, 50000, leadBytes[ver]); + libzcash::SaplingNote note(pk, 50000, is_zip_212[ver]); auto cm = note.cmu().get(); SaplingMerkleTree tree; tree.append(cm); @@ -660,10 +660,10 @@ TEST(WalletTests, GetConflictedSaplingNotes) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto consensusParams = Params().GetConsensus(); - unsigned char leadBytes[] = {0x01, 0x02}; + bool is_zip_212[] = {false, true}; int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - for (int ver = 0; ver < sizeof(leadBytes); ver++) { + for (int ver = 0; ver < sizeof(is_zip_212); ver++) { TestWallet wallet; LOCK2(cs_main, wallet.cs_wallet); @@ -678,7 +678,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); // Generate note A - libzcash::SaplingNote note(pk, 50000, leadBytes[ver]); + libzcash::SaplingNote note(pk, 50000, is_zip_212[ver]); auto cm = note.cmu().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); @@ -1042,10 +1042,10 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto consensusParams = Params().GetConsensus(); - unsigned char leadBytes[] = {0x01, 0x02}; + bool is_zip_212[] = {false, true}; int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - for (int ver = 0; ver < sizeof(leadBytes); ver++) { + for (int ver = 0; ver < sizeof(is_zip_212); ver++) { TestWallet wallet; LOCK2(cs_main, wallet.cs_wallet); @@ -1057,7 +1057,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { auto pk = sk.DefaultAddress(); // Generate Sapling note A - libzcash::SaplingNote note(pk, 50000, leadBytes[ver]); + libzcash::SaplingNote note(pk, 50000, is_zip_212[ver]); auto cm = note.cmu().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d43881256..c9d68c911 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1500,7 +1500,7 @@ void CWallet::UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx) { if (!optDeserialized) { // The transaction would not have entered the wallet unless - // its plaintest had been succesfully decrypted previously. + // its plaintext had been successfully decrypted previously. assert(false); } @@ -2353,7 +2353,7 @@ boost::optional SaplingNotePlaintext::note(const SaplingIncomingVie auto addr = ivk.address(d); if (addr) { auto tmp = SaplingNote(d, addr.get().pk_d, value_, rseed); - tmp.leadByte = leadByte; + tmp.is_zip_212 = false; + if (leadbyte == 0x02) { + tmp.is_zip_212 = true; + } return tmp; } else { return boost::none; @@ -217,7 +224,7 @@ boost::optional SaplingNotePlaintext::decrypt( const SaplingNotePlaintext plaintext = *ret; // Check leadbyte is allowed at block height - if (!plaintext_version_is_valid(params, height, plaintext.leadByte)) { + if (!plaintext_version_is_valid(params, height, plaintext.get_leadbyte())) { return boost::none; } @@ -254,7 +261,7 @@ boost::optional SaplingNotePlaintext::plaintext_checks_wit return boost::none; } - if (plaintext.leadByte == 0x02) { + if (plaintext.get_leadbyte() == 0x02) { // ZIP 212: Check that epk is consistent to prevent against linkability // attacks without relying on the soundness of the SNARK. uint256 expected_epk; @@ -310,7 +317,7 @@ boost::optional SaplingNotePlaintext::decrypt( SaplingNotePlaintext plaintext = *ret; // Check leadbyte is allowed at block height - if (!plaintext_version_is_valid(params, height, plaintext.leadByte)) { + if (!plaintext_version_is_valid(params, height, plaintext.get_leadbyte())) { return boost::none; } @@ -352,7 +359,7 @@ boost::optional SaplingNotePlaintext::plaintext_checks_wit return boost::none; } - if (plaintext.leadByte == 0x02) { + if (plaintext.get_leadbyte() == 0x02) { // ZIP 212: Additionally check that the esk provided to this function // is consistent with the esk we can derive if (esk != plaintext.generate_esk()) { @@ -429,7 +436,7 @@ SaplingOutCiphertext SaplingOutgoingPlaintext::encrypt( } uint256 SaplingNotePlaintext::rcm() const { - if (leadByte == 0x02) { + if (leadbyte == 0x02) { return PRF_rcm(rseed); } else { return rseed; @@ -437,7 +444,7 @@ uint256 SaplingNotePlaintext::rcm() const { } uint256 SaplingNote::rcm() const { - if (leadByte == 0x02) { + if (SaplingNote::get_is_zip_212()) { return PRF_rcm(rseed); } else { return rseed; @@ -445,7 +452,7 @@ uint256 SaplingNote::rcm() const { } uint256 SaplingNotePlaintext::generate_esk() const { - if (leadByte == 0x02) { + if (leadbyte == 0x02) { return PRF_esk(rseed); } else { uint256 esk; diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index b9df41547..bf667d90e 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -42,22 +42,22 @@ public: uint256 nullifier(const SproutSpendingKey& a_sk) const; }; -inline bool plaintext_version_is_valid(const Consensus::Params& params, int height, unsigned char leadByte) { +inline bool plaintext_version_is_valid(const Consensus::Params& params, int height, unsigned char leadbyte) { int canopyActivationHeight = params.vUpgrades[Consensus::UPGRADE_CANOPY].nActivationHeight; - if (height < canopyActivationHeight && leadByte != 0x01) { + if (height < canopyActivationHeight && leadbyte != 0x01) { // non-0x01 received before Canopy activation height return false; } if (height >= canopyActivationHeight && height < canopyActivationHeight + ZIP212_GRACE_PERIOD - && leadByte != 0x01 - && leadByte != 0x02) + && leadbyte != 0x01 + && leadbyte != 0x02) { // non-{0x01,0x02} received after Canopy activation and before grace period has elapsed return false; } - if (height >= canopyActivationHeight + ZIP212_GRACE_PERIOD && leadByte != 0x02) { + if (height >= canopyActivationHeight + ZIP212_GRACE_PERIOD && leadbyte != 0x02) { // non-0x02 received past (Canopy activation height + grace period) return false; } @@ -68,7 +68,7 @@ class SaplingNote : public BaseNote { private: uint256 rseed; friend class SaplingNotePlaintext; - unsigned char leadByte; + bool is_zip_212 = false; // whether the note was generated using ZIP 212 (activated at Canopy) public: diversifier_t d; uint256 pk_d; @@ -76,7 +76,7 @@ public: SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed) : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed) {} - SaplingNote(const SaplingPaymentAddress &address, uint64_t value, unsigned char leadByte); + SaplingNote(const SaplingPaymentAddress &address, uint64_t value, bool is_zip_212); virtual ~SaplingNote() {}; @@ -84,8 +84,8 @@ public: boost::optional nullifier(const SaplingFullViewingKey &vk, const uint64_t position) const; uint256 rcm() const; - unsigned char get_lead_byte() const { - return leadByte; + bool get_is_zip_212() const { + return is_zip_212; } }; @@ -120,10 +120,10 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - unsigned char leadByte = 0x00; - READWRITE(leadByte); + unsigned char leadbyte = 0x00; + READWRITE(leadbyte); - if (leadByte != 0x00) { + if (leadbyte != 0x00) { throw std::ios_base::failure("lead byte of SproutNotePlaintext is not recognized"); } @@ -150,7 +150,7 @@ typedef std::pair SaplingNotePlaint class SaplingNotePlaintext : public BaseNotePlaintext { private: uint256 rseed; - unsigned char leadByte; + unsigned char leadbyte; public: diversifier_t d; @@ -213,7 +213,7 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(leadByte); // 1 byte + READWRITE(leadbyte); // 1 byte READWRITE(d); // 11 bytes READWRITE(value_); // 8 bytes READWRITE(rseed); // 32 bytes @@ -224,8 +224,8 @@ public: uint256 rcm() const; uint256 generate_esk() const; - unsigned char get_lead_byte() const { - return leadByte; + unsigned char get_leadbyte() const { + return leadbyte; } }; diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index 4311f7bf7..13e05ebaa 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -594,7 +594,7 @@ double benchmark_create_sapling_spend() auto sk = libzcash::SaplingSpendingKey::random(); auto expsk = sk.expanded_spending_key(); auto address = sk.default_address(); - SaplingNote note(address, GetRand(MAX_MONEY), 0x01); + SaplingNote note(address, GetRand(MAX_MONEY), false); SaplingMerkleTree tree; auto maybe_cmu = note.cmu(); tree.append(maybe_cmu.get()); @@ -647,7 +647,7 @@ double benchmark_create_sapling_output() auto address = sk.default_address(); std::array memo; - SaplingNote note(address, GetRand(MAX_MONEY), 0x01); + SaplingNote note(address, GetRand(MAX_MONEY), false); libzcash::SaplingNotePlaintext notePlaintext(note, memo); auto res = notePlaintext.encrypt(note.pk_d); From 2f4d7e35c93cbf62dc3b8c703e063e6069e7c8cb Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Tue, 30 Jun 2020 05:50:10 +0800 Subject: [PATCH 13/21] Throw error in plaintext deserialization --- src/zcash/Note.cpp | 59 ++++++++++++++++++++++++++++------------------ src/zcash/Note.hpp | 7 +++++- 2 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 0b5451238..c23c2f1f1 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -196,15 +196,18 @@ boost::optional SaplingOutgoingPlaintext::decrypt( } // Deserialize from the plaintext - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << pt.get(); - - SaplingOutgoingPlaintext ret; - ss >> ret; - - assert(ss.size() == 0); - - return ret; + try { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << pt.get(); + SaplingOutgoingPlaintext ret; + ss >> ret; + assert(ss.size() == 0); + return ret; + } catch (const boost::thread_interrupted&) { + throw; + } catch (...) { + return boost::none; + } } boost::optional SaplingNotePlaintext::decrypt( @@ -290,13 +293,18 @@ boost::optional SaplingNotePlaintext::attempt_sapling_enc_ }; // Deserialize from the plaintext - SaplingNotePlaintext plaintext; - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << encPlaintext.get(); - ss >> plaintext; - assert(ss.size() == 0); - - return plaintext; + SaplingNotePlaintext ret; + try { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << encPlaintext.get(); + ss >> ret; + assert(ss.size() == 0); + return ret; + } catch (const boost::thread_interrupted&) { + throw; + } catch (...) { + return boost::none; + } } boost::optional SaplingNotePlaintext::decrypt( @@ -384,13 +392,18 @@ boost::optional SaplingNotePlaintext::attempt_sapling_enc_ }; // Deserialize from the plaintext - SaplingNotePlaintext plaintext; - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << encPlaintext.get(); - ss >> plaintext; - assert(ss.size() == 0); - - return plaintext; + SaplingNotePlaintext ret; + try { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << encPlaintext.get(); + ss >> ret; + assert(ss.size() == 0); + return ret; + } catch (const boost::thread_interrupted&) { + throw; + } catch (...) { + return boost::none; + } } boost::optional SaplingNotePlaintext::encrypt(const uint256& pk_d) const diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index bf667d90e..bc384588a 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -213,7 +213,12 @@ public: template inline void SerializationOp(Stream& s, Operation ser_action) { - READWRITE(leadbyte); // 1 byte + READWRITE(leadbyte); + + if (leadbyte != 0x01 && leadbyte != 0x02) { + throw std::ios_base::failure("lead byte of SaplingNotePlaintext is not recognized"); + } + READWRITE(d); // 11 bytes READWRITE(value_); // 8 bytes READWRITE(rseed); // 32 bytes From 19d4c47b663f279be7e84d1075e7025ddff67836 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Wed, 1 Jul 2020 03:45:15 +0800 Subject: [PATCH 14/21] Pass pindex to AddToWalletIfInvolvingMe() --- src/main.cpp | 3 +++ src/wallet/wallet.cpp | 18 +++++++++++++----- src/wallet/wallet.h | 4 ++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 08b522cc2..9ff74bee4 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -920,6 +920,7 @@ bool ContextualCheckTransaction( } // SaplingNotePlaintext::decrypt() checks note commitment validity. + auto encPlaintext = SaplingNotePlaintext::decrypt( chainparams.GetConsensus(), nHeight, @@ -938,6 +939,8 @@ bool ContextualCheckTransaction( // ZIP 212: Check that the note plaintexts use the v2 note plaintext // version. + // This check compels miners to switch to the new plaintext version + // and overrides the grace period in plaintext_version_is_valid() if (canopyActive != (encPlaintext->get_leadbyte() == 0x02)) { return state.DoS( DOS_LEVEL_BLOCK, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c9d68c911..e514740e1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1716,14 +1716,22 @@ bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx) * updated; instead, the transaction being in the mempool or conflicted is determined on * the fly in CMerkleTx::GetDepthInMainChain(). */ -bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex, bool fUpdate) { { AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; auto sproutNoteData = FindMySproutNotes(tx); - auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx, chainActive.Height()); + int height; + if (pblock) { + height = pindex->nHeight; + } else { + // assume tx is in mempool + LOCK(cs_main); + height = chainActive.Height(); + } + auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx, height); auto saplingNoteData = saplingNoteDataAndAddressesToAdd.first; auto addressesToAdd = saplingNoteDataAndAddressesToAdd.second; for (const auto &addressToAdd : addressesToAdd) { @@ -1757,10 +1765,10 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl return false; } -void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock) +void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex) { LOCK(cs_wallet); - if (!AddToWalletIfInvolvingMe(tx, pblock, true)) + if (!AddToWalletIfInvolvingMe(tx, pblock, pindex, true)) return; // Not one of ours MarkAffectedTransactionsDirty(tx); @@ -2743,7 +2751,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) ReadBlockFromDisk(block, pindex, Params().GetConsensus()); BOOST_FOREACH(CTransaction& tx, block.vtx) { - if (AddToWalletIfInvolvingMe(tx, &block, fUpdate)) { + if (AddToWalletIfInvolvingMe(tx, &block, pindex, fUpdate)) { myTxHashes.push_back(tx.GetHash()); ret++; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4d6bcb95a..9c0bc2a26 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1162,8 +1162,8 @@ public: void UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx); void UpdateSaplingNullifierNoteMapForBlock(const CBlock* pblock); 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); + void SyncTransaction(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex); + bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex, bool fUpdate); void EraseFromWallet(const uint256 &hash); void WitnessNoteCommitment( std::vector commitments, From ee83424c6fe5ba00bb0b806f410f1cd8bbbd145b Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 2 Jul 2020 07:05:35 +0800 Subject: [PATCH 15/21] Remove plaintext check from AddSaplingSpend Co-authored by Sean Bowe (ewillbefull@gmail.com) --- src/gtest/test_transaction_builder.cpp | 100 ------------------------- src/transaction_builder.cpp | 16 +--- 2 files changed, 1 insertion(+), 115 deletions(-) diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index 2f3006461..3ebdf7f92 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -496,103 +496,3 @@ TEST(TransactionBuilder, CheckSaplingTxVersion) // Revert to default UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); } - -TEST(TransactionBuilder, RejectsInvalidNotePlaintextVersion) -{ - SelectParams(CBaseChainParams::REGTEST); - int overwinterActivationHeight = 5; - int saplingActivationHeight = 30; - int canopyActivationHeight = 70; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); - auto consensusParams = Params().GetConsensus(); - - auto sk = libzcash::SaplingSpendingKey::random(); - auto expsk = sk.expanded_spending_key(); - auto pk = sk.default_address(); - - SaplingMerkleTree tree; - - { - // non-0x01 received before Canopy activation height - auto builder = TransactionBuilder(consensusParams, canopyActivationHeight - 1); - libzcash::SaplingNote note(pk, 50000, true); - try { - builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); - } catch (std::runtime_error const & err) { - EXPECT_EQ(err.what(), std::string("TransactionBuilder: invalid note plaintext version")); - } catch(...) { - FAIL() << "Expected std::runtime_error"; - } - } - - { - // non-0x02 received past (Canopy activation height + grace period) - auto builder = TransactionBuilder(consensusParams, canopyActivationHeight + ZIP212_GRACE_PERIOD); - libzcash::SaplingNote note(pk, 50000, false); - try { - builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); - } catch (std::runtime_error const & err) { - EXPECT_EQ(err.what(), std::string("TransactionBuilder: invalid note plaintext version")); - } catch(...) { - FAIL() << "Expected std::runtime_error"; - } - } - - // Revert to default - RegtestDeactivateCanopy(); - RegtestDeactivateSapling(); -} - -TEST(TransactionBuilder, AcceptsValidNotePlaintextVersion) -{ - SelectParams(CBaseChainParams::REGTEST); - int overwinterActivationHeight = 5; - int saplingActivationHeight = 30; - int canopyActivationHeight = 70; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); - auto consensusParams = Params().GetConsensus(); - - auto sk = libzcash::SaplingSpendingKey::random(); - auto expsk = sk.expanded_spending_key(); - auto pk = sk.default_address(); - - SaplingMerkleTree tree; - - { - // 0x01 received before Canopy activation height - auto builder = TransactionBuilder(consensusParams, canopyActivationHeight - 1); - libzcash::SaplingNote note(pk, 50000, false); - ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); - } - - { - // {0x01,0x02} received after Canopy activation and before grace period has elapsed - unsigned char is_zip_212[] = {false, true}; - int height1 = canopyActivationHeight - 1; - int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 2; - int heights[] = {height1, height2}; - - for (int i = 0; i < sizeof(is_zip_212); i++) { - for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { - auto builder = TransactionBuilder(consensusParams, heights[j]); - libzcash::SaplingNote note(pk, 50000, is_zip_212[i]); - ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); - } - } - } - - { - // 0x02 received past (Canopy activation height + grace period) - auto builder = TransactionBuilder(consensusParams, canopyActivationHeight + ZIP212_GRACE_PERIOD - 1); - libzcash::SaplingNote note(pk, 50000, true); - ASSERT_NO_THROW(builder.AddSaplingSpend(expsk, note, uint256(), tree.witness())); - } - - // Revert to default - RegtestDeactivateCanopy(); - RegtestDeactivateSapling(); -} diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 613b90add..af825f8dd 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -143,16 +143,6 @@ void TransactionBuilder::AddSaplingSpend( throw std::runtime_error("TransactionBuilder cannot add Sapling spend to pre-Sapling transaction"); } - unsigned char leadbyte = 0x01; - if (note.get_is_zip_212() == true) { - leadbyte = 0x02; - } - - // ZIP212: check that note plaintext lead byte is valid at height - if (!libzcash::plaintext_version_is_valid(consensusParams, nHeight + 1, leadbyte)) { - throw std::runtime_error("TransactionBuilder: invalid note plaintext version"); - } - // Consistency check: all anchors must equal the first one if (spends.size() > 0 && spends[0].anchor != anchor) { throw JSONRPCError(RPC_WALLET_ERROR, "Anchor does not match previously-added Sapling spends."); @@ -173,11 +163,7 @@ void TransactionBuilder::AddSaplingOutput( throw std::runtime_error("TransactionBuilder cannot add Sapling output to pre-Sapling transaction"); } - bool is_zip_212 = false; - if (Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)) { - is_zip_212 = true; - } - auto note = libzcash::SaplingNote(to, value, is_zip_212); + auto note = libzcash::SaplingNote(to, value, Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)); outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } From eeda663ff78515bd5537c3bcd63215e9cf4943c9 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 2 Jul 2020 07:18:46 +0800 Subject: [PATCH 16/21] Remove plaintext check from GetFilteredNotes Co-authored by Sean Bowe (ewillbefull@gmail.com) --- src/wallet/wallet.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e514740e1..fd4edc5be 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5079,16 +5079,8 @@ void CWallet::GetFilteredNotes( // its plaintext had been successfully decrypted previously. assert(false); } - // We don't need to check the leadbyte here: if wtx exists in - // the wallet, it must have already passed the leadbyte check - auto maybe_pt = SaplingNotePlaintext::plaintext_checks_without_height( - *optDeserialized, - nd.ivk, - wtx.vShieldedOutput[op.n].ephemeralKey, - wtx.vShieldedOutput[op.n].cmu); - assert(static_cast(maybe_pt)); - auto notePt = maybe_pt.get(); + auto notePt = optDeserialized.get(); auto maybe_pa = nd.ivk.address(notePt.d); assert(static_cast(maybe_pa)); auto pa = maybe_pa.get(); From c4821ddceb5b0bb6b3ffe41ee9c5d5a6ba3c9fbb Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 2 Jul 2020 17:45:02 +0800 Subject: [PATCH 17/21] Refactor bool is_zip_212 to enum Zip212Enabled Co-authored by Kris Nuttycombe (@nuttycom) --- src/gtest/test_checktransaction.cpp | 81 +------------------------- src/gtest/test_noteencryption.cpp | 24 ++++---- src/gtest/test_sapling_note.cpp | 6 +- src/gtest/test_transaction_builder.cpp | 2 +- src/miner.cpp | 6 +- src/transaction_builder.cpp | 7 ++- src/utiltest.cpp | 2 +- src/wallet/gtest/test_wallet.cpp | 18 +++--- src/zcash/Note.cpp | 18 +++--- src/zcash/Note.hpp | 15 +++-- src/zcbenchmarks.cpp | 4 +- 11 files changed, 61 insertions(+), 122 deletions(-) diff --git a/src/gtest/test_checktransaction.cpp b/src/gtest/test_checktransaction.cpp index 2ae731ae0..bf5c20d83 100644 --- a/src/gtest/test_checktransaction.cpp +++ b/src/gtest/test_checktransaction.cpp @@ -1134,7 +1134,7 @@ TEST(CheckTransaction, HeartwoodAcceptsShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), false); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), libzcash::Zip212Enabled::BeforeZip212); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); @@ -1217,7 +1217,7 @@ TEST(CheckTransaction, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { uint256 ovk; auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), false); + libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), libzcash::Zip212Enabled::BeforeZip212); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); CMutableTransaction mtx = GetValidTransaction(); @@ -1284,80 +1284,3 @@ TEST(CheckTransaction, HeartwoodEnforcesSaplingRulesOnShieldedCoinbase) { RegtestDeactivateHeartwood(); } - -// Check that the consensus rules relevant to valueBalance, vShieldedOutput, and -// bindingSig from https://zips.z.cash/protocol/protocol.pdf#txnencoding are -// applied to coinbase transactions. -TEST(CheckTransaction, CanopyEnforcesSaplingRulesOnShieldedCoinbase) { - RegtestActivateCanopy(false, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); - auto chainparams = Params(); - - uint256 ovk; - auto note = libzcash::SaplingNote( - libzcash::SaplingSpendingKey::random().default_address(), CAmount(123456), true); - auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); - - CMutableTransaction mtx = GetValidTransaction(); - mtx.fOverwintered = true; - mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID; - mtx.nVersion = SAPLING_TX_VERSION; - - mtx.vin.resize(1); - mtx.vin[0].prevout.SetNull(); - mtx.vin[0].scriptSig << 123; - mtx.vJoinSplit.resize(0); - mtx.valueBalance = -1000; - - // Coinbase transaction should fail non-contextual checks with no shielded - // outputs and non-zero valueBalance. - { - CTransaction tx(mtx); - EXPECT_TRUE(tx.IsCoinBase()); - - MockCValidationState state; - EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-valuebalance-nonzero", false)).Times(1); - EXPECT_FALSE(CheckTransactionWithoutProofVerification(tx, state)); - } - - // Add a Sapling output. - auto ctx = librustzcash_sapling_proving_ctx_init(); - auto odesc = output.Build(ctx).get(); - librustzcash_sapling_proving_ctx_free(ctx); - mtx.vShieldedOutput.push_back(odesc); - - // Coinbase transaction should fail non-contextual checks with valueBalance - // out of range. - { - mtx.valueBalance = MAX_MONEY + 1; - CTransaction tx(mtx); - EXPECT_TRUE(tx.IsCoinBase()); - - MockCValidationState state; - EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-valuebalance-toolarge", false)).Times(1); - EXPECT_FALSE(CheckTransactionWithoutProofVerification(tx, state)); - } - { - mtx.valueBalance = -MAX_MONEY - 1; - CTransaction tx(mtx); - EXPECT_TRUE(tx.IsCoinBase()); - - MockCValidationState state; - EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-valuebalance-toolarge", false)).Times(1); - EXPECT_FALSE(CheckTransactionWithoutProofVerification(tx, state)); - } - - mtx.valueBalance = -1000; - CTransaction tx(mtx); - EXPECT_TRUE(tx.IsCoinBase()); - - // Coinbase transaction should now pass non-contextual checks. - MockCValidationState state; - EXPECT_TRUE(CheckTransactionWithoutProofVerification(tx, state)); - - // Coinbase transaction does not pass contextual checks, as bindingSig - // consensus rule is enforced. - EXPECT_CALL(state, DoS(100, false, REJECT_INVALID, "bad-txns-sapling-binding-signature-invalid", false)).Times(1); - ContextualCheckTransaction(tx, state, chainparams, 10, 57); - - RegtestDeactivateCanopy(); -} diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index 9d8ef2feb..b5677a210 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -33,7 +33,7 @@ TEST(NoteEncryption, NotePlaintext) UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto params = Params().GetConsensus(); - bool is_zip_212[] = {false, true}; + std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; int decryptionHeights[] = {saplingActivationHeight, canopyActivationHeight}; using namespace libzcash; @@ -48,8 +48,8 @@ TEST(NoteEncryption, NotePlaintext) memo[i] = (unsigned char) i; } - for (int ver = 0; ver < sizeof(is_zip_212); ver++){ - SaplingNote note(addr, 39393, is_zip_212[ver]); + for (int ver = 0; ver < zip_212_enabled.size(); ver++){ + SaplingNote note(addr, 39393, zip_212_enabled[ver]); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -187,7 +187,7 @@ TEST(NoteEncryption, NotePlaintext) RegtestDeactivateSapling(); } -TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) +TEST(NoteEncryption, RejectsInvalidNoteZip212Enabled) { SelectParams(CBaseChainParams::REGTEST); int overwinterActivationHeight = 5; @@ -212,7 +212,7 @@ TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) { // non-0x01 received before Canopy activation height - SaplingNote note(addr, 39393, true); + SaplingNote note(addr, 39393, Zip212Enabled::AfterZip212); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -243,7 +243,7 @@ TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) { // non-0x02 received past (Canopy activation height + grace period) - SaplingNote note(addr, 39393, false); + SaplingNote note(addr, 39393, Zip212Enabled::BeforeZip212); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -278,7 +278,7 @@ TEST(NoteEncryption, RejectsInvalidNotePlaintextVersion) RegtestDeactivateSapling(); } -TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) +TEST(NoteEncryption, AcceptsValidNoteZip212Enabled) { SelectParams(CBaseChainParams::REGTEST); int overwinterActivationHeight = 5; @@ -303,7 +303,7 @@ TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) { // 0x01 received before Canopy activation height - SaplingNote note(addr, 39393, false); + SaplingNote note(addr, 39393, Zip212Enabled::BeforeZip212); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -338,14 +338,14 @@ TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) { // {0x01,0x02} received after Canopy activation and before grace period has elapsed - bool is_zip_212[] = {false, true}; + std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; int height1 = canopyActivationHeight; int height2 = canopyActivationHeight + (ZIP212_GRACE_PERIOD) - 1; int heights[] = {height1, height2}; - for (int i = 0; i < sizeof(is_zip_212); i++) { + for (int i = 0; i < zip_212_enabled.size(); i++) { for (int j = 0; j < sizeof(heights) / sizeof(int); j++) { - SaplingNote note(addr, 39393, is_zip_212[i]); + SaplingNote note(addr, 39393, zip_212_enabled[i]); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); @@ -382,7 +382,7 @@ TEST(NoteEncryption, AcceptsValidNotePlaintextVersion) { // 0x02 received past (Canopy activation height + grace period) - SaplingNote note(addr, 39393, true); + SaplingNote note(addr, 39393, Zip212Enabled::AfterZip212); auto cmu_opt = note.cmu(); if (!cmu_opt) { FAIL(); diff --git a/src/gtest/test_sapling_note.cpp b/src/gtest/test_sapling_note.cpp index 59abbfd5a..6238fe266 100644 --- a/src/gtest/test_sapling_note.cpp +++ b/src/gtest/test_sapling_note.cpp @@ -57,8 +57,8 @@ TEST(SaplingNote, Random) { // Test creating random notes using the same spending key auto address = SaplingSpendingKey::random().default_address(); - SaplingNote note1(address, GetRand(MAX_MONEY), false); - SaplingNote note2(address, GetRand(MAX_MONEY), false); + SaplingNote note1(address, GetRand(MAX_MONEY), Zip212Enabled::BeforeZip212); + SaplingNote note2(address, GetRand(MAX_MONEY), Zip212Enabled::BeforeZip212); ASSERT_EQ(note1.d, note2.d); ASSERT_EQ(note1.pk_d, note2.pk_d); @@ -66,7 +66,7 @@ TEST(SaplingNote, Random) ASSERT_NE(note1.rcm(), note2.rcm()); // Test diversifier and pk_d are not the same for different spending keys - SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY), false); + SaplingNote note3(SaplingSpendingKey::random().default_address(), GetRand(MAX_MONEY), Zip212Enabled::BeforeZip212); ASSERT_NE(note1.d, note3.d); ASSERT_NE(note1.pk_d, note3.pk_d); } diff --git a/src/gtest/test_transaction_builder.cpp b/src/gtest/test_transaction_builder.cpp index 3ebdf7f92..253b3f7ff 100644 --- a/src/gtest/test_transaction_builder.cpp +++ b/src/gtest/test_transaction_builder.cpp @@ -483,7 +483,7 @@ TEST(TransactionBuilder, CheckSaplingTxVersion) } // Cannot add Sapling spends to a non-Sapling transaction - libzcash::SaplingNote note(pk, 50000, false); + libzcash::SaplingNote note(pk, 50000, libzcash::Zip212Enabled::BeforeZip212); SaplingMerkleTree tree; try { builder.AddSaplingSpend(expsk, note, uint256(), tree.witness()); diff --git a/src/miner.cpp b/src/miner.cpp index 1a91ac7d3..638082040 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -157,7 +157,11 @@ public: mtx.valueBalance = -value; uint256 ovk; - auto note = libzcash::SaplingNote(pa, value, (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY))); + libzcash::Zip212Enabled zip_212_enabled = libzcash::Zip212Enabled::BeforeZip212; + if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { + zip_212_enabled = libzcash::Zip212Enabled::AfterZip212; + } + auto note = libzcash::SaplingNote(pa, value, zip_212_enabled); auto output = OutputDescriptionInfo(ovk, note, {{0xF6}}); auto ctx = librustzcash_sapling_proving_ctx_init(); diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index af825f8dd..0ed5998db 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -163,7 +163,12 @@ void TransactionBuilder::AddSaplingOutput( throw std::runtime_error("TransactionBuilder cannot add Sapling output to pre-Sapling transaction"); } - auto note = libzcash::SaplingNote(to, value, Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)); + libzcash::Zip212Enabled zip_212_enabled = libzcash::Zip212Enabled::BeforeZip212; + if (Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)) { + zip_212_enabled = libzcash::Zip212Enabled::AfterZip212; + } + + auto note = libzcash::SaplingNote(to, value, zip_212_enabled); outputs.emplace_back(ovk, note, memo); mtx.valueBalance -= value; } diff --git a/src/utiltest.cpp b/src/utiltest.cpp index 6b695e205..f5ca85ba5 100644 --- a/src/utiltest.cpp +++ b/src/utiltest.cpp @@ -289,7 +289,7 @@ CKey AddTestCKeyToKeyStore(CBasicKeyStore& keyStore) { TestSaplingNote GetTestSaplingNote(const libzcash::SaplingPaymentAddress& pa, CAmount value) { // Generate dummy Sapling note - libzcash::SaplingNote note(pa, value, false); + libzcash::SaplingNote note(pa, value, libzcash::Zip212Enabled::BeforeZip212); uint256 cm = note.cmu().get(); SaplingMerkleTree tree; tree.append(cm); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 345c28202..1c6adaf72 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -385,10 +385,10 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto consensusParams = Params().GetConsensus(); - bool is_zip_212[] = {false, true}; + std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - for (int ver = 0; ver < sizeof(is_zip_212); ver++) { + for (int ver = 0; ver < zip_212_enabled.size(); ver++) { TestWallet wallet; LOCK(wallet.cs_wallet); @@ -398,7 +398,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto ivk = fvk.in_viewing_key(); auto pk = sk.DefaultAddress(); - libzcash::SaplingNote note(pk, 50000, is_zip_212[ver]); + libzcash::SaplingNote note(pk, 50000, zip_212_enabled[ver]); auto cm = note.cmu().get(); SaplingMerkleTree tree; tree.append(cm); @@ -660,10 +660,10 @@ TEST(WalletTests, GetConflictedSaplingNotes) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto consensusParams = Params().GetConsensus(); - bool is_zip_212[] = {false, true}; + std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - for (int ver = 0; ver < sizeof(is_zip_212); ver++) { + for (int ver = 0; ver < zip_212_enabled.size(); ver++) { TestWallet wallet; LOCK2(cs_main, wallet.cs_wallet); @@ -678,7 +678,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { ASSERT_TRUE(wallet.HaveSaplingSpendingKey(extfvk)); // Generate note A - libzcash::SaplingNote note(pk, 50000, is_zip_212[ver]); + libzcash::SaplingNote note(pk, 50000, zip_212_enabled[ver]); auto cm = note.cmu().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); @@ -1042,10 +1042,10 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); auto consensusParams = Params().GetConsensus(); - bool is_zip_212[] = {false, true}; + std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; - for (int ver = 0; ver < sizeof(is_zip_212); ver++) { + for (int ver = 0; ver < zip_212_enabled.size(); ver++) { TestWallet wallet; LOCK2(cs_main, wallet.cs_wallet); @@ -1057,7 +1057,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { auto pk = sk.DefaultAddress(); // Generate Sapling note A - libzcash::SaplingNote note(pk, 50000, is_zip_212[ver]); + libzcash::SaplingNote note(pk, 50000, zip_212_enabled[ver]); auto cm = note.cmu().get(); SaplingMerkleTree saplingTree; saplingTree.append(cm); diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index c23c2f1f1..612b78067 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -45,12 +45,12 @@ uint256 SproutNote::nullifier(const SproutSpendingKey& a_sk) const { SaplingNote::SaplingNote( const SaplingPaymentAddress& address, const uint64_t value, - bool _is_zip_212 + Zip212Enabled _zip_212_enabled ) : BaseNote(value) { d = address.d; pk_d = address.pk_d; - is_zip_212 = _is_zip_212; - if (is_zip_212) { + zip_212_enabled = _zip_212_enabled; + if (zip_212_enabled == Zip212Enabled::AfterZip212) { // Per ZIP 212, the rseed field is 32 random bytes. rseed = random_uint256(); } else { @@ -159,7 +159,7 @@ SaplingNotePlaintext::SaplingNotePlaintext( { d = note.d; rseed = note.rseed; - if (note.get_is_zip_212()) { + if (note.get_zip_212_enabled() == libzcash::Zip212Enabled::AfterZip212) { leadbyte = 0x02; } else { leadbyte = 0x01; @@ -171,11 +171,11 @@ boost::optional SaplingNotePlaintext::note(const SaplingIncomingVie { auto addr = ivk.address(d); if (addr) { - auto tmp = SaplingNote(d, addr.get().pk_d, value_, rseed); - tmp.is_zip_212 = false; + Zip212Enabled zip_212_enabled = Zip212Enabled::BeforeZip212; if (leadbyte == 0x02) { - tmp.is_zip_212 = true; - } + zip_212_enabled = Zip212Enabled::AfterZip212; + }; + auto tmp = SaplingNote(d, addr.get().pk_d, value_, rseed, zip_212_enabled); return tmp; } else { return boost::none; @@ -457,7 +457,7 @@ uint256 SaplingNotePlaintext::rcm() const { } uint256 SaplingNote::rcm() const { - if (SaplingNote::get_is_zip_212()) { + if (SaplingNote::get_zip_212_enabled() == libzcash::Zip212Enabled::AfterZip212) { return PRF_rcm(rseed); } else { return rseed; diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index bc384588a..9d26239e0 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -64,11 +64,15 @@ inline bool plaintext_version_is_valid(const Consensus::Params& params, int heig return true; }; +enum class Zip212Enabled { + BeforeZip212, + AfterZip212 +}; class SaplingNote : public BaseNote { private: uint256 rseed; friend class SaplingNotePlaintext; - bool is_zip_212 = false; // whether the note was generated using ZIP 212 (activated at Canopy) + Zip212Enabled zip_212_enabled; public: diversifier_t d; uint256 pk_d; @@ -76,7 +80,10 @@ public: SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed) : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed) {} - SaplingNote(const SaplingPaymentAddress &address, uint64_t value, bool is_zip_212); + SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed, Zip212Enabled zip_212_enabled) + : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed), zip_212_enabled(zip_212_enabled) {} + + SaplingNote(const SaplingPaymentAddress &address, uint64_t value, Zip212Enabled zip_212_enabled); virtual ~SaplingNote() {}; @@ -84,8 +91,8 @@ public: boost::optional nullifier(const SaplingFullViewingKey &vk, const uint64_t position) const; uint256 rcm() const; - bool get_is_zip_212() const { - return is_zip_212; + Zip212Enabled get_zip_212_enabled() const { + return zip_212_enabled; } }; diff --git a/src/zcbenchmarks.cpp b/src/zcbenchmarks.cpp index 13e05ebaa..d66404814 100644 --- a/src/zcbenchmarks.cpp +++ b/src/zcbenchmarks.cpp @@ -594,7 +594,7 @@ double benchmark_create_sapling_spend() auto sk = libzcash::SaplingSpendingKey::random(); auto expsk = sk.expanded_spending_key(); auto address = sk.default_address(); - SaplingNote note(address, GetRand(MAX_MONEY), false); + SaplingNote note(address, GetRand(MAX_MONEY), libzcash::Zip212Enabled::BeforeZip212); SaplingMerkleTree tree; auto maybe_cmu = note.cmu(); tree.append(maybe_cmu.get()); @@ -647,7 +647,7 @@ double benchmark_create_sapling_output() auto address = sk.default_address(); std::array memo; - SaplingNote note(address, GetRand(MAX_MONEY), false); + SaplingNote note(address, GetRand(MAX_MONEY), libzcash::Zip212Enabled::BeforeZip212); libzcash::SaplingNotePlaintext notePlaintext(note, memo); auto res = notePlaintext.encrypt(note.pk_d); From 31020d6fc9172e347ebd5a5806e4811d9a21a5c1 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Thu, 2 Jul 2020 23:24:03 +0800 Subject: [PATCH 18/21] Minor changes Co-authored by Daira Hopwood (daira@jacaranda.org) and Jack Grigg (jack@electriccoin.co) --- src/gtest/test_noteencryption.cpp | 11 ++- src/transaction_builder.cpp | 3 +- src/wallet/wallet.cpp | 61 ++++++-------- src/zcash/Note.cpp | 134 +++++++++++++++--------------- src/zcash/Note.hpp | 4 +- src/zcash/NoteEncryption.cpp | 9 +- src/zcash/NoteEncryption.hpp | 2 +- 7 files changed, 106 insertions(+), 118 deletions(-) diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index b5677a210..0c8a78c2a 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -445,11 +445,14 @@ TEST(NoteEncryption, SaplingApi) small_message[i] = (unsigned char) i; } + uint256 esk; + librustzcash_sapling_generate_r(esk.begin()); + // Invalid diversifier - ASSERT_EQ(boost::none, SaplingNoteEncryption::FromDiversifier({1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, boost::none)); + ASSERT_EQ(boost::none, SaplingNoteEncryption::FromDiversifier({1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, esk)); // Encrypt to pk_1 - auto enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d, boost::none); + auto enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d, esk); auto ciphertext_1 = *enc.encrypt_to_recipient( pk_1.pk_d, message @@ -471,7 +474,7 @@ TEST(NoteEncryption, SaplingApi) ); // Encrypt to pk_2 - enc = *SaplingNoteEncryption::FromDiversifier(pk_2.d, boost::none); + enc = *SaplingNoteEncryption::FromDiversifier(pk_2.d, esk); auto ciphertext_2 = *enc.encrypt_to_recipient( pk_2.pk_d, message @@ -489,7 +492,7 @@ TEST(NoteEncryption, SaplingApi) // Test nonce-reuse resistance of API { - auto tmp_enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d, boost::none); + auto tmp_enc = *SaplingNoteEncryption::FromDiversifier(pk_1.d, esk); tmp_enc.encrypt_to_recipient( pk_1.pk_d, diff --git a/src/transaction_builder.cpp b/src/transaction_builder.cpp index 0ed5998db..6236d4e9d 100644 --- a/src/transaction_builder.cpp +++ b/src/transaction_builder.cpp @@ -164,7 +164,8 @@ void TransactionBuilder::AddSaplingOutput( } libzcash::Zip212Enabled zip_212_enabled = libzcash::Zip212Enabled::BeforeZip212; - if (Params().GetConsensus().NetworkUpgradeActive(nHeight + 1, Consensus::UPGRADE_CANOPY)) { + // We use nHeight = chainActive.Height() + 1 since the output will be included in the next block + if (Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_CANOPY)) { zip_212_enabled = libzcash::Zip212Enabled::AfterZip212; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fd4edc5be..e7120cdf1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1498,27 +1498,22 @@ void CWallet::UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx) { auto optDeserialized = SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization(output.encCiphertext, nd.ivk, output.ephemeralKey); - if (!optDeserialized) { - // The transaction would not have entered the wallet unless - // its plaintext had been successfully decrypted previously. - assert(false); - } + // The transaction would not have entered the wallet unless + // its plaintext had been successfully decrypted previously. + assert(optDeserialized != boost::none); auto optPlaintext = SaplingNotePlaintext::plaintext_checks_without_height(*optDeserialized, nd.ivk, output.ephemeralKey, output.cmu); - if (!optPlaintext) { - // An item in mapSaplingNoteData must have already been successfully decrypted, - // otherwise the item would not exist in the first place. - assert(false); - } + + // An item in mapSaplingNoteData must have already been successfully decrypted, + // otherwise the item would not exist in the first place. + assert(optPlaintext != boost::none); + auto optNote = optPlaintext.get().note(nd.ivk); - if (!optNote) { - assert(false); - } + assert(optNote != boost::none); + auto optNullifier = optNote.get().nullifier(extfvk.fvk, position); - if (!optNullifier) { - // This should not happen. If it does, maybe the position has been corrupted or miscalculated? - assert(false); - } + // This should not happen. If it does, maybe the position has been corrupted or miscalculated? + assert(optNullifier != boost::none); uint256 nullifier = optNullifier.get(); mapSaplingNullifiersToNotes[nullifier] = op; item.second.nullifier = nullifier; @@ -2335,11 +2330,11 @@ boost::optional(maybe_pt)); + assert(maybe_pt != boost::none); auto notePt = maybe_pt.get(); auto maybe_pa = nd.ivk.address(notePt.d); - assert(static_cast(maybe_pa)); + assert(maybe_pa != boost::none); auto pa = maybe_pa.get(); return std::make_pair(notePt, pa); @@ -2359,18 +2354,16 @@ boost::optional(maybe_pt)); + assert(maybe_pt != boost::none); auto notePt = maybe_pt.get(); auto maybe_pa = nd.ivk.address(notePt.d); @@ -2394,6 +2387,7 @@ boost::optionalesk, outPt->pk_d); - if (!optDeserialized) { - // The transaction would not have entered the wallet unless - // its plaintext had been successfully decrypted previously. - assert(false); - } + // The transaction would not have entered the wallet unless + // its plaintext had been successfully decrypted previously. + assert(optDeserialized != boost::none); auto maybe_pt = SaplingNotePlaintext::plaintext_checks_without_height( *optDeserialized, @@ -5074,11 +5067,9 @@ void CWallet::GetFilteredNotes( auto optDeserialized = SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization(wtx.vShieldedOutput[op.n].encCiphertext, nd.ivk, wtx.vShieldedOutput[op.n].ephemeralKey); - if (!optDeserialized) { - // The transaction would not have entered the wallet unless - // its plaintext had been successfully decrypted previously. - assert(false); - } + // The transaction would not have entered the wallet unless + // its plaintext had been successfully decrypted previously. + assert(optDeserialized != boost::none); auto notePt = optDeserialized.get(); auto maybe_pa = nd.ivk.address(notePt.d); diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 612b78067..23320c07a 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -45,11 +45,11 @@ uint256 SproutNote::nullifier(const SproutSpendingKey& a_sk) const { SaplingNote::SaplingNote( const SaplingPaymentAddress& address, const uint64_t value, - Zip212Enabled _zip_212_enabled + Zip212Enabled zip212Enabled ) : BaseNote(value) { d = address.d; pk_d = address.pk_d; - zip_212_enabled = _zip_212_enabled; + zip_212_enabled = zip212Enabled; if (zip_212_enabled == Zip212Enabled::AfterZip212) { // Per ZIP 212, the rseed field is 32 random bytes. rseed = random_uint256(); @@ -172,7 +172,7 @@ boost::optional SaplingNotePlaintext::note(const SaplingIncomingVie auto addr = ivk.address(d); if (addr) { Zip212Enabled zip_212_enabled = Zip212Enabled::BeforeZip212; - if (leadbyte == 0x02) { + if (leadbyte != 0x01) { zip_212_enabled = Zip212Enabled::AfterZip212; }; auto tmp = SaplingNote(d, addr.get().pk_d, value_, rseed, zip_212_enabled); @@ -235,6 +235,33 @@ boost::optional SaplingNotePlaintext::decrypt( } } +boost::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk +) +{ + auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, ivk, epk); + + if (!encPlaintext) { + return boost::none; + } + + // Deserialize from the plaintext + SaplingNotePlaintext ret; + try { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << encPlaintext.get(); + ss >> ret; + assert(ss.size() == 0); + return ret; + } catch (const boost::thread_interrupted&) { + throw; + } catch (...) { + return boost::none; + } +} + boost::optional SaplingNotePlaintext::plaintext_checks_without_height( const SaplingNotePlaintext &plaintext, const uint256 &ivk, @@ -264,11 +291,11 @@ boost::optional SaplingNotePlaintext::plaintext_checks_wit return boost::none; } - if (plaintext.get_leadbyte() == 0x02) { - // ZIP 212: Check that epk is consistent to prevent against linkability + if (plaintext.get_leadbyte() != 0x01) { + // ZIP 212: Check that epk is consistent to guard against linkability // attacks without relying on the soundness of the SNARK. uint256 expected_epk; - uint256 esk = plaintext.generate_esk(); + uint256 esk = plaintext.generate_or_derive_esk(); if (!librustzcash_sapling_ka_derivepublic(plaintext.d.data(), esk.begin(), expected_epk.begin())) { return boost::none; } @@ -280,33 +307,6 @@ boost::optional SaplingNotePlaintext::plaintext_checks_wit return plaintext; } -boost::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( - const SaplingEncCiphertext &ciphertext, - const uint256 &ivk, - const uint256 &epk -) -{ - auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, ivk, epk); - - if (!encPlaintext) { - return boost::none; - }; - - // Deserialize from the plaintext - SaplingNotePlaintext ret; - try { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << encPlaintext.get(); - ss >> ret; - assert(ss.size() == 0); - return ret; - } catch (const boost::thread_interrupted&) { - throw; - } catch (...) { - return boost::none; - } -} - boost::optional SaplingNotePlaintext::decrypt( const Consensus::Params& params, int height, @@ -333,6 +333,34 @@ boost::optional SaplingNotePlaintext::decrypt( } } +boost::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( + const SaplingEncCiphertext &ciphertext, + const uint256 &epk, + const uint256 &esk, + const uint256 &pk_d +) +{ + auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, epk, esk, pk_d); + + if (!encPlaintext) { + return boost::none; + }; + + // Deserialize from the plaintext + SaplingNotePlaintext ret; + try { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << encPlaintext.get(); + ss >> ret; + assert(ss.size() == 0); + return ret; + } catch (const boost::thread_interrupted&) { + throw; + } catch (...) { + return boost::none; + } +} + boost::optional SaplingNotePlaintext::plaintext_checks_without_height( const SaplingNotePlaintext &plaintext, const uint256 &epk, @@ -367,10 +395,10 @@ boost::optional SaplingNotePlaintext::plaintext_checks_wit return boost::none; } - if (plaintext.get_leadbyte() == 0x02) { + if (plaintext.get_leadbyte() != 0x01) { // ZIP 212: Additionally check that the esk provided to this function // is consistent with the esk we can derive - if (esk != plaintext.generate_esk()) { + if (esk != plaintext.generate_or_derive_esk()) { return boost::none; } } @@ -378,38 +406,10 @@ boost::optional SaplingNotePlaintext::plaintext_checks_wit return plaintext; } -boost::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( - const SaplingEncCiphertext &ciphertext, - const uint256 &epk, - const uint256 &esk, - const uint256 &pk_d -) -{ - auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, epk, esk, pk_d); - - if (!encPlaintext) { - return boost::none; - }; - - // Deserialize from the plaintext - SaplingNotePlaintext ret; - try { - CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); - ss << encPlaintext.get(); - ss >> ret; - assert(ss.size() == 0); - return ret; - } catch (const boost::thread_interrupted&) { - throw; - } catch (...) { - return boost::none; - } -} - boost::optional SaplingNotePlaintext::encrypt(const uint256& pk_d) const { // Get the encryptor - auto sne = SaplingNoteEncryption::FromDiversifier(d, generate_esk()); + auto sne = SaplingNoteEncryption::FromDiversifier(d, generate_or_derive_esk()); if (!sne) { return boost::none; } @@ -449,7 +449,7 @@ SaplingOutCiphertext SaplingOutgoingPlaintext::encrypt( } uint256 SaplingNotePlaintext::rcm() const { - if (leadbyte == 0x02) { + if (leadbyte != 0x01) { return PRF_rcm(rseed); } else { return rseed; @@ -464,8 +464,8 @@ uint256 SaplingNote::rcm() const { } } -uint256 SaplingNotePlaintext::generate_esk() const { - if (leadbyte == 0x02) { +uint256 SaplingNotePlaintext::generate_or_derive_esk() const { + if (leadbyte != 0x01) { return PRF_esk(rseed); } else { uint256 esk; diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index 9d26239e0..f721d5220 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -78,7 +78,7 @@ public: uint256 pk_d; SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed) - : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed) {} + : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed), zip_212_enabled(Zip212Enabled::BeforeZip212) {} SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed, Zip212Enabled zip_212_enabled) : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed), zip_212_enabled(zip_212_enabled) {} @@ -235,7 +235,7 @@ public: boost::optional encrypt(const uint256& pk_d) const; uint256 rcm() const; - uint256 generate_esk() const; + uint256 generate_or_derive_esk() const; unsigned char get_leadbyte() const { return leadbyte; } diff --git a/src/zcash/NoteEncryption.cpp b/src/zcash/NoteEncryption.cpp index 4af47d42e..dee211cba 100644 --- a/src/zcash/NoteEncryption.cpp +++ b/src/zcash/NoteEncryption.cpp @@ -103,17 +103,10 @@ namespace libzcash { boost::optional SaplingNoteEncryption::FromDiversifier( diversifier_t d, - boost::optional esk_provided + uint256 esk ) { uint256 epk; - uint256 esk; - if (esk_provided) { - esk = esk_provided.get(); - } else { - // Pick random esk - librustzcash_sapling_generate_r(esk.begin()); - } // Compute epk given the diversifier if (!librustzcash_sapling_ka_derivepublic(d.begin(), esk.begin(), epk.begin())) { diff --git a/src/zcash/NoteEncryption.hpp b/src/zcash/NoteEncryption.hpp index 51b27be17..308d81dc1 100644 --- a/src/zcash/NoteEncryption.hpp +++ b/src/zcash/NoteEncryption.hpp @@ -42,7 +42,7 @@ protected: public: - static boost::optional FromDiversifier(diversifier_t d, boost::optional esk); + static boost::optional FromDiversifier(diversifier_t d, uint256 esk); boost::optional encrypt_to_recipient( const uint256 &pk_d, From 119bae082ce06bf0db8e48ff645d78064b5c2d14 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Sat, 4 Jul 2020 18:20:37 +0800 Subject: [PATCH 19/21] Remove old SaplingNote() constructor --- src/gtest/test_sapling_note.cpp | 2 +- src/wallet/test/rpc_wallet_tests.cpp | 2 +- src/zcash/Note.hpp | 3 --- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/gtest/test_sapling_note.cpp b/src/gtest/test_sapling_note.cpp index 6238fe266..ca1fece7c 100644 --- a/src/gtest/test_sapling_note.cpp +++ b/src/gtest/test_sapling_note.cpp @@ -44,7 +44,7 @@ TEST(SaplingNote, TestVectors) uint256 nf(v_nf); // Test commitment - SaplingNote note = SaplingNote(diversifier, pk_d, v, r); + SaplingNote note = SaplingNote(diversifier, pk_d, v, r, Zip212Enabled::BeforeZip212); ASSERT_EQ(note.cmu().get(), cm); // Test nullifier diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 6707c065f..0b2ad86b6 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -1837,7 +1837,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_parameters) std::vector sproutNoteInputs = {MergeToAddressInputSproutNote{JSOutPoint(), SproutNote(), 0, SproutSpendingKey()}}; std::vector saplingNoteInputs = - {MergeToAddressInputSaplingNote{SaplingOutPoint(), SaplingNote({}, uint256(), 0, uint256()), 0, SaplingExpandedSpendingKey()}}; + {MergeToAddressInputSaplingNote{SaplingOutPoint(), SaplingNote({}, uint256(), 0, uint256(), Zip212Enabled::BeforeZip212), 0, SaplingExpandedSpendingKey()}}; // Sprout and Sapling inputs -> throw try { diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index f721d5220..0e4cc1ccb 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -77,9 +77,6 @@ public: diversifier_t d; uint256 pk_d; - SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed) - : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed), zip_212_enabled(Zip212Enabled::BeforeZip212) {} - SaplingNote(diversifier_t d, uint256 pk_d, uint64_t value, uint256 rseed, Zip212Enabled zip_212_enabled) : BaseNote(value), d(d), pk_d(pk_d), rseed(rseed), zip_212_enabled(zip_212_enabled) {} From 1020254b6ad87c586e8c54534b3345f6cc072306 Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Sat, 4 Jul 2020 23:33:23 +0800 Subject: [PATCH 20/21] Pass nHeight instead of pindex to AddToWalletIfInvolvingMe() Co-authored by Jack Grigg (jack@electriccoin.co) and Sean Bowe (ewillbefull@gmail.com) --- src/amqp/amqpnotificationinterface.cpp | 2 +- src/amqp/amqpnotificationinterface.h | 2 +- src/validationinterface.cpp | 16 ++++++++-------- src/validationinterface.h | 4 ++-- src/wallet/wallet.cpp | 18 +++++------------- src/wallet/wallet.h | 4 ++-- src/zmq/zmqnotificationinterface.cpp | 2 +- src/zmq/zmqnotificationinterface.h | 2 +- 8 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/amqp/amqpnotificationinterface.cpp b/src/amqp/amqpnotificationinterface.cpp index 0dfae00b9..27f98f690 100644 --- a/src/amqp/amqpnotificationinterface.cpp +++ b/src/amqp/amqpnotificationinterface.cpp @@ -122,7 +122,7 @@ void AMQPNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindex) } } -void AMQPNotificationInterface::SyncTransaction(const CTransaction &tx, const CBlock *pblock) +void AMQPNotificationInterface::SyncTransaction(const CTransaction &tx, const CBlock *pblock, const int nHeight) { for (std::list::iterator i = notifiers.begin(); i != notifiers.end(); ) { AMQPAbstractNotifier *notifier = *i; diff --git a/src/amqp/amqpnotificationinterface.h b/src/amqp/amqpnotificationinterface.h index 77495a85c..ef0ac7f34 100644 --- a/src/amqp/amqpnotificationinterface.h +++ b/src/amqp/amqpnotificationinterface.h @@ -24,7 +24,7 @@ protected: void Shutdown(); // CValidationInterface - void SyncTransaction(const CTransaction &tx, const CBlock *pblock); + void SyncTransaction(const CTransaction &tx, const CBlock *pblock, const int nHeight); void UpdatedBlockTip(const CBlockIndex *pindex); private: diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 49773aee8..db9f8e47e 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -25,7 +25,7 @@ CMainSignals& GetMainSignals() void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.UpdatedBlockTip.connect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1)); - g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2)); + g_signals.SyncTransaction.connect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3)); 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)); @@ -47,7 +47,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { 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)); - g_signals.SyncTransaction.disconnect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2)); + g_signals.SyncTransaction.disconnect(boost::bind(&CValidationInterface::SyncTransaction, pwalletIn, _1, _2, _3)); g_signals.UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1)); } @@ -65,8 +65,8 @@ void UnregisterAllValidationInterfaces() { g_signals.UpdatedBlockTip.disconnect_all_slots(); } -void SyncWithWallets(const CTransaction &tx, const CBlock *pblock) { - g_signals.SyncTransaction(tx, pblock); +void SyncWithWallets(const CTransaction &tx, const CBlock *pblock, const int nHeight) { + g_signals.SyncTransaction(tx, pblock, nHeight); } struct CachedBlockData { @@ -187,7 +187,7 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) // Let wallets know transactions went from 1-confirmed to // 0-confirmed or conflicted: for (const CTransaction &tx : block.vtx) { - SyncWithWallets(tx, NULL); + SyncWithWallets(tx, NULL, pindexLastTip->nHeight); } // Update cached incremental witnesses GetMainSignals().ChainTip(pindexLastTip, &block, boost::none); @@ -214,11 +214,11 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) // Tell wallet about transactions that went from mempool // to conflicted: for (const CTransaction &tx : blockData.txConflicted) { - SyncWithWallets(tx, NULL); + SyncWithWallets(tx, NULL, blockData.pindex->nHeight + 1); } // ... and about transactions that got confirmed: for (const CTransaction &tx : block.vtx) { - SyncWithWallets(tx, &block); + SyncWithWallets(tx, &block, blockData.pindex->nHeight); } // Update cached incremental witnesses GetMainSignals().ChainTip(blockData.pindex, &block, blockData.oldTrees); @@ -230,7 +230,7 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) // Notify transactions in the mempool for (auto tx : recentlyAdded.first) { try { - SyncWithWallets(tx, NULL); + SyncWithWallets(tx, NULL, pindexLastTip->nHeight + 1); } catch (const boost::thread_interrupted&) { throw; } catch (const std::exception& e) { diff --git a/src/validationinterface.h b/src/validationinterface.h index 8dfdd6eb4..f1694a313 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -33,7 +33,7 @@ void UnregisterAllValidationInterfaces(); class CValidationInterface { protected: virtual void UpdatedBlockTip(const CBlockIndex *pindex) {} - virtual void SyncTransaction(const CTransaction &tx, const CBlock *pblock) {} + virtual void SyncTransaction(const CTransaction &tx, const CBlock *pblock, const int nHeight) {} virtual void EraseFromWallet(const uint256 &hash) {} virtual void ChainTip(const CBlockIndex *pindex, const CBlock *pblock, boost::optional> added) {} virtual void SetBestChain(const CBlockLocator &locator) {} @@ -52,7 +52,7 @@ struct CMainSignals { /** Notifies listeners of updated block chain tip */ boost::signals2::signal UpdatedBlockTip; /** Notifies listeners of updated transaction data (transaction, and optionally the block it is found in. */ - boost::signals2::signal SyncTransaction; + boost::signals2::signal SyncTransaction; /** Notifies listeners of an erased transaction (currently disabled, requires transaction replacement). */ boost::signals2::signal EraseTransaction; /** Notifies listeners of an updated transaction without new data (for now: a coinbase potentially becoming visible). */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index e7120cdf1..6e20ae6d4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1711,22 +1711,14 @@ bool CWallet::UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx) * updated; instead, the transaction being in the mempool or conflicted is determined on * the fly in CMerkleTx::GetDepthInMainChain(). */ -bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, const int nHeight, bool fUpdate) { { AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; auto sproutNoteData = FindMySproutNotes(tx); - int height; - if (pblock) { - height = pindex->nHeight; - } else { - // assume tx is in mempool - LOCK(cs_main); - height = chainActive.Height(); - } - auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx, height); + auto saplingNoteDataAndAddressesToAdd = FindMySaplingNotes(tx, nHeight); auto saplingNoteData = saplingNoteDataAndAddressesToAdd.first; auto addressesToAdd = saplingNoteDataAndAddressesToAdd.second; for (const auto &addressToAdd : addressesToAdd) { @@ -1760,10 +1752,10 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl return false; } -void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex) +void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock, const int nHeight) { LOCK(cs_wallet); - if (!AddToWalletIfInvolvingMe(tx, pblock, pindex, true)) + if (!AddToWalletIfInvolvingMe(tx, pblock, nHeight, true)) return; // Not one of ours MarkAffectedTransactionsDirty(tx); @@ -2744,7 +2736,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) ReadBlockFromDisk(block, pindex, Params().GetConsensus()); BOOST_FOREACH(CTransaction& tx, block.vtx) { - if (AddToWalletIfInvolvingMe(tx, &block, pindex, fUpdate)) { + if (AddToWalletIfInvolvingMe(tx, &block, pindex->nHeight, fUpdate)) { myTxHashes.push_back(tx.GetHash()); ret++; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 9c0bc2a26..c41116119 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1162,8 +1162,8 @@ public: void UpdateSaplingNullifierNoteMapWithTx(CWalletTx& wtx); void UpdateSaplingNullifierNoteMapForBlock(const CBlock* pblock); bool AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletDB* pwalletdb); - void SyncTransaction(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex); - bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, const CBlockIndex* pindex, bool fUpdate); + void SyncTransaction(const CTransaction& tx, const CBlock* pblock, const int nHeight); + bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, const int nHeight, bool fUpdate); void EraseFromWallet(const uint256 &hash); void WitnessNoteCommitment( std::vector commitments, diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index e427e6b97..22c3b3901 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -163,7 +163,7 @@ void CZMQNotificationInterface::BlockChecked(const CBlock& block, const CValidat } } -void CZMQNotificationInterface::SyncTransaction(const CTransaction &tx, const CBlock *pblock) +void CZMQNotificationInterface::SyncTransaction(const CTransaction &tx, const CBlock *pblock, const int nHeight) { for (std::list::iterator i = notifiers.begin(); i!=notifiers.end(); ) { diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index 1dc7211d5..fedff60ef 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -25,7 +25,7 @@ protected: void Shutdown(); // CValidationInterface - void SyncTransaction(const CTransaction &tx, const CBlock *pblock); + void SyncTransaction(const CTransaction &tx, const CBlock *pblock, const int nHeight); void UpdatedBlockTip(const CBlockIndex *pindex); void BlockChecked(const CBlock& block, const CValidationState& state); From dde5cc87b7c04a0e0d84525cd8d34921817c595a Mon Sep 17 00:00:00 2001 From: therealyingtong Date: Mon, 6 Jul 2020 13:34:45 +0800 Subject: [PATCH 21/21] Directly call RegtestActivate* in gtests Co-authored by Jack Grigg (jack@electriccoin.co) --- src/gtest/test_noteencryption.cpp | 27 +++++------- src/utiltest.cpp | 4 ++ src/utiltest.h | 1 + src/wallet/gtest/test_wallet.cpp | 72 ++++++++++++------------------- 4 files changed, 42 insertions(+), 62 deletions(-) diff --git a/src/gtest/test_noteencryption.cpp b/src/gtest/test_noteencryption.cpp index 0c8a78c2a..a9b19f184 100644 --- a/src/gtest/test_noteencryption.cpp +++ b/src/gtest/test_noteencryption.cpp @@ -25,16 +25,10 @@ public: TEST(NoteEncryption, NotePlaintext) { SelectParams(CBaseChainParams::REGTEST); - int overwinterActivationHeight = 5; - int saplingActivationHeight = 30; - int canopyActivationHeight = 70; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); - auto params = Params().GetConsensus(); std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; - int decryptionHeights[] = {saplingActivationHeight, canopyActivationHeight}; + const Consensus::Params& (*activations [])() = {RegtestActivateSapling, RegtestActivateCanopy}; + void (*deactivations [])() = {RegtestDeactivateSapling, RegtestDeactivateCanopy}; using namespace libzcash; auto xsk = SaplingSpendingKey(uint256()).expanded_spending_key(); @@ -49,6 +43,8 @@ TEST(NoteEncryption, NotePlaintext) } for (int ver = 0; ver < zip_212_enabled.size(); ver++){ + auto params = (*activations[ver])(); + SaplingNote note(addr, 39393, zip_212_enabled[ver]); auto cmu_opt = note.cmu(); if (!cmu_opt) { @@ -71,7 +67,7 @@ TEST(NoteEncryption, NotePlaintext) // Try to decrypt with incorrect commitment ASSERT_FALSE(SaplingNotePlaintext::decrypt( params, - decryptionHeights[ver], + 1, ct, ivk, epk, @@ -81,7 +77,7 @@ TEST(NoteEncryption, NotePlaintext) // Try to decrypt with correct commitment auto foo = SaplingNotePlaintext::decrypt( params, - decryptionHeights[ver], + 1, ct, ivk, epk, @@ -149,7 +145,7 @@ TEST(NoteEncryption, NotePlaintext) ASSERT_FALSE( SaplingNotePlaintext::decrypt( params, - decryptionHeights[ver], + 1, ct, epk, decrypted_out_ct_unwrapped.esk, @@ -161,7 +157,7 @@ TEST(NoteEncryption, NotePlaintext) // Test sender can decrypt the note ciphertext. foo = SaplingNotePlaintext::decrypt( params, - decryptionHeights[ver], + 1, ct, epk, decrypted_out_ct_unwrapped.esk, @@ -179,12 +175,9 @@ TEST(NoteEncryption, NotePlaintext) ASSERT_TRUE(bar.memo() == pt.memo()); ASSERT_TRUE(bar.d == pt.d); ASSERT_TRUE(bar.rcm() == pt.rcm()); - } - // Revert to test default - RegtestDeactivateCanopy(); - RegtestDeactivateHeartwood(); - RegtestDeactivateSapling(); + (*deactivations[ver])(); + } } TEST(NoteEncryption, RejectsInvalidNoteZip212Enabled) diff --git a/src/utiltest.cpp b/src/utiltest.cpp index f5ca85ba5..24ac06a59 100644 --- a/src/utiltest.cpp +++ b/src/utiltest.cpp @@ -264,6 +264,10 @@ const Consensus::Params& RegtestActivateCanopy(bool updatePow, int canopyActivat return Params().GetConsensus(); } +const Consensus::Params& RegtestActivateCanopy() { + return RegtestActivateCanopy(false, Consensus::NetworkUpgrade::ALWAYS_ACTIVE); +} + void RegtestDeactivateCanopy() { UpdateRegtestPow(0, 0, uint256S("0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f0f")); UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT); diff --git a/src/utiltest.h b/src/utiltest.h index 9166f499c..d22f48af9 100644 --- a/src/utiltest.h +++ b/src/utiltest.h @@ -54,6 +54,7 @@ const Consensus::Params& RegtestActivateHeartwood(bool updatePow, int heartwoodA void RegtestDeactivateHeartwood(); const Consensus::Params& RegtestActivateCanopy(bool updatePow, int canopyActivationHeight = Consensus::NetworkUpgrade::ALWAYS_ACTIVE); +const Consensus::Params& RegtestActivateCanopy(); void RegtestDeactivateCanopy(); diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 1c6adaf72..a0a032140 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -377,18 +377,14 @@ TEST(WalletTests, SetSproutNoteAddrsInCWalletTx) { TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { SelectParams(CBaseChainParams::REGTEST); - int overwinterActivationHeight = 5; - int saplingActivationHeight = 30; - int canopyActivationHeight = 70; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); - auto consensusParams = Params().GetConsensus(); std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; - int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; + const Consensus::Params& (*activations [])() = {RegtestActivateSapling, RegtestActivateCanopy}; + void (*deactivations [])() = {RegtestDeactivateSapling, RegtestDeactivateCanopy}; for (int ver = 0; ver < zip_212_enabled.size(); ver++) { + auto consensusParams = (*activations[ver])(); + TestWallet wallet; LOCK(wallet.cs_wallet); @@ -409,7 +405,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { ASSERT_TRUE(nf); uint256 nullifier = nf.get(); - auto builder = TransactionBuilder(consensusParams, builderHeights[ver]); + auto builder = TransactionBuilder(consensusParams, 1); builder.AddSaplingSpend(expsk, note, anchor, witness); builder.AddSaplingOutput(fvk.ovk, pk, 50000, {}); builder.SetFee(0); @@ -436,11 +432,9 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { EXPECT_EQ(nullifier, wtx.mapSaplingNoteData[op].nullifier); EXPECT_EQ(nd.witnessHeight, wtx.mapSaplingNoteData[op].witnessHeight); EXPECT_TRUE(witness == wtx.mapSaplingNoteData[op].witnesses.front()); - } - // Revert to default - RegtestDeactivateCanopy(); - RegtestDeactivateSapling(); + (*deactivations[ver])(); + } } TEST(WalletTests, SetSproutInvalidNoteAddrsInCWalletTx) { @@ -652,18 +646,14 @@ TEST(WalletTests, GetConflictedSproutNotes) { // Generate note A and spend to create note B, from which we spend to create two conflicting transactions TEST(WalletTests, GetConflictedSaplingNotes) { SelectParams(CBaseChainParams::REGTEST); - int overwinterActivationHeight = 5; - int saplingActivationHeight = 30; - int canopyActivationHeight = 70; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); - auto consensusParams = Params().GetConsensus(); std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; - int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; + const Consensus::Params& (*activations [])() = {RegtestActivateSapling, RegtestActivateCanopy}; + void (*deactivations [])() = {RegtestDeactivateSapling, RegtestDeactivateCanopy}; for (int ver = 0; ver < zip_212_enabled.size(); ver++) { + auto consensusParams = (*activations[ver])(); + TestWallet wallet; LOCK2(cs_main, wallet.cs_wallet); @@ -686,7 +676,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { auto witness = saplingTree.witness(); // Generate tx to create output note B - auto builder = TransactionBuilder(consensusParams, builderHeights[ver]); + auto builder = TransactionBuilder(consensusParams, 1); builder.AddSaplingSpend(expsk, note, anchor, witness); builder.AddSaplingOutput(extfvk.fvk.ovk, pk, 35000, {}); auto tx = builder.Build().GetTxOrThrow(); @@ -706,7 +696,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { EXPECT_EQ(0, chainActive.Height()); // Simulate SyncTransaction which calls AddToWalletIfInvolvingMe - auto saplingNoteData = wallet.FindMySaplingNotes(wtx, builderHeights[ver]).first; + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, 1).first; ASSERT_TRUE(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); wtx.SetMerkleBranch(block); @@ -742,13 +732,13 @@ TEST(WalletTests, GetConflictedSaplingNotes) { anchor = saplingTree.root(); // Create transaction to spend note B - auto builder2 = TransactionBuilder(consensusParams, builderHeights[ver] + 1); + auto builder2 = TransactionBuilder(consensusParams, 2); builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); builder2.AddSaplingOutput(extfvk.fvk.ovk, pk, 20000, {}); auto tx2 = builder2.Build().GetTxOrThrow(); // Create conflicting transaction which also spends note B - auto builder3 = TransactionBuilder(consensusParams, builderHeights[ver] + 1); + auto builder3 = TransactionBuilder(consensusParams, 2); builder3.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); builder3.AddSaplingOutput(extfvk.fvk.ovk, pk, 19999, {}); auto tx3 = builder3.Build().GetTxOrThrow(); @@ -776,11 +766,9 @@ TEST(WalletTests, GetConflictedSaplingNotes) { // Tear down chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); - } - // Revert to test default - RegtestDeactivateCanopy(); - RegtestActivateSapling(); + (*deactivations[ver])(); + } } TEST(WalletTests, SproutNullifierIsSpent) { @@ -1034,18 +1022,14 @@ TEST(WalletTests, SpentSproutNoteIsFromMe) { // Create note A, spend A to create note B, spend and verify note B is from me. TEST(WalletTests, SpentSaplingNoteIsFromMe) { SelectParams(CBaseChainParams::REGTEST); - int overwinterActivationHeight = 5; - int saplingActivationHeight = 30; - int canopyActivationHeight = 70; - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_OVERWINTER, overwinterActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_SAPLING, saplingActivationHeight); - UpdateNetworkUpgradeParameters(Consensus::UPGRADE_CANOPY, canopyActivationHeight); - auto consensusParams = Params().GetConsensus(); std::vector zip_212_enabled = {libzcash::Zip212Enabled::BeforeZip212, libzcash::Zip212Enabled::AfterZip212}; - int builderHeights[] = {saplingActivationHeight, canopyActivationHeight}; + const Consensus::Params& (*activations [])() = {RegtestActivateSapling, RegtestActivateCanopy}; + void (*deactivations [])() = {RegtestDeactivateSapling, RegtestDeactivateCanopy}; for (int ver = 0; ver < zip_212_enabled.size(); ver++) { + auto consensusParams = (*activations[ver])(); + TestWallet wallet; LOCK2(cs_main, wallet.cs_wallet); @@ -1065,7 +1049,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { auto witness = saplingTree.witness(); // Generate transaction, which sends funds to note B - auto builder = TransactionBuilder(consensusParams, builderHeights[ver]); + auto builder = TransactionBuilder(consensusParams, 1); builder.AddSaplingSpend(expsk, note, anchor, witness); builder.AddSaplingOutput(extfvk.fvk.ovk, pk, 25000, {}); auto tx = builder.Build().GetTxOrThrow(); @@ -1087,7 +1071,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { EXPECT_TRUE(chainActive.Contains(&fakeIndex)); EXPECT_EQ(0, chainActive.Height()); - auto saplingNoteData = wallet.FindMySaplingNotes(wtx, builderHeights[ver]).first; + auto saplingNoteData = wallet.FindMySaplingNotes(wtx, 1).first; ASSERT_TRUE(saplingNoteData.size() > 0); wtx.SetSaplingNoteData(saplingNoteData); wtx.SetMerkleBranch(block); @@ -1137,7 +1121,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { anchor = saplingTree.root(); // Create transaction to spend note B - auto builder2 = TransactionBuilder(consensusParams, builderHeights[ver] + 1); + auto builder2 = TransactionBuilder(consensusParams, 2); builder2.AddSaplingSpend(expsk, note2, anchor, spend_note_witness); builder2.AddSaplingOutput(extfvk.fvk.ovk, pk, 12500, {}); auto tx2 = builder2.Build().GetTxOrThrow(); @@ -1164,7 +1148,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { EXPECT_TRUE(chainActive.Contains(&fakeIndex2)); EXPECT_EQ(1, chainActive.Height()); - auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2, builderHeights[ver]).first; + auto saplingNoteData2 = wallet.FindMySaplingNotes(wtx2, 2).first; ASSERT_TRUE(saplingNoteData2.size() > 0); wtx2.SetSaplingNoteData(saplingNoteData2); wtx2.SetMerkleBranch(block2); @@ -1181,11 +1165,9 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { chainActive.SetTip(NULL); mapBlockIndex.erase(blockHash); mapBlockIndex.erase(blockHash2); - } - // Revert to test default - RegtestDeactivateCanopy(); - RegtestActivateSapling(); + (*deactivations[ver])(); + } } TEST(WalletTests, CachedWitnessesEmptyChain) {