From 7ddd03d86d48777604d5997a7352997845a99a10 Mon Sep 17 00:00:00 2001 From: DanS Date: Thu, 15 Oct 2026 21:12:39 -0500 Subject: [PATCH] zcash/wallet: decrypt a Sapling note's plaintext and note in one pass SaplingNotePlaintext::decrypt(ciphertext, ivk, epk, cmu) derives pk_d = [ivk]g_d to check the note commitment, then throws it away. The wallet then calls ivk.address(d), which derives pk_d again, and plaintext.note(ivk), which derives it a third time. Each derivation is a Jubjub scalar multiplication, ~115us, measured. Including the key agreement and the commitment check, the whole sequence is ~775us per note. decrypt_with_note() runs the same decryption and checks and returns the note built from that pk_d. ivk_to_pkd succeeding already implies a valid diversifier, which is all ivk.address() checks on top. It gets its own name rather than a decrypt() overload: an overload taking a fifth argument by non-const reference would capture calls meant for the (epk, esk, pk_d, cmu) variant. The cmu check stays. GetFilteredNotes and FindMySaplingNotes' spending-key loop use it. That saves two of the three derivations per note in every balance walk, and one per found note during scans and rescans (trial decryption of outputs that aren't ours still fails at the AEAD, as before). FindMySaplingNotes also no longer copies each OutputDescription. New tests: MatchesDecryptThenNote checks decrypt_with_note field for field against decrypt() + note(ivk), over default and diversified addresses, a wrong ivk and a wrong cmu. FindMySaplingNotes pins what discovery returns: notes per key, diversified addresses to register, foreign outputs ignored. Both pass before and after the switch. Co-Authored-By: Claude Opus 5.5 --- src/gtest/test_wallet_notecache.cpp | 94 +++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 24 ++++---- src/zcash/Note.cpp | 40 +++++++++++- src/zcash/Note.hpp | 11 ++++ 4 files changed, 154 insertions(+), 15 deletions(-) diff --git a/src/gtest/test_wallet_notecache.cpp b/src/gtest/test_wallet_notecache.cpp index a6362d576..0415bc657 100644 --- a/src/gtest/test_wallet_notecache.cpp +++ b/src/gtest/test_wallet_notecache.cpp @@ -494,3 +494,97 @@ TEST_F(WalletNoteCacheEncrypted, UnlockedWalletFindsSpendingKeys) { EXPECT_TRUE(sk.DefaultAddress() == paC); EXPECT_FALSE(wallet.GetSaplingSpendingKey(TestKey(9).expsk.full_viewing_key(), sk)); } + +namespace { + +// The default address plus a few diversified addresses of the same key. +std::vector AddressesOf(const SaplingExtendedSpendingKey& sk) +{ + std::vector addrs = {sk.DefaultAddress()}; + for (unsigned char start : {7, 90, 201}) { + diversifier_index_t j; + j.begin()[0] = start; + auto found = sk.ToXFVK().Address(j); + if (found) + addrs.push_back(found->second); + } + return addrs; +} + +} + +// decrypt_with_note returns exactly decrypt() plus decrypt()->note(ivk), and fails exactly when they do. +TEST(SaplingDecryptWithNote, MatchesDecryptThenNote) { + auto sk = TestKey(0); + const SaplingIncomingViewingKey ivk = sk.expsk.full_viewing_key().in_viewing_key(); + const SaplingIncomingViewingKey otherIvk = TestKey(1).expsk.full_viewing_key().in_viewing_key(); + auto addrs = AddressesOf(sk); + ASSERT_GE(addrs.size(), 3u) << "need diversified addresses too"; + + uint64_t value = 100; + for (const SaplingPaymentAddress& pa : addrs) { + OutputDescription od = MakeOutput(pa, ++value); + auto legacyPt = SaplingNotePlaintext::decrypt(od.encCiphertext, ivk, od.ephemeralKey, od.cm); + ASSERT_TRUE(static_cast(legacyPt)); + auto legacyNote = legacyPt->note(ivk); + ASSERT_TRUE(static_cast(legacyNote)); + + auto both = SaplingNotePlaintext::decrypt_with_note(od.encCiphertext, ivk, od.ephemeralKey, od.cm); + ASSERT_TRUE(static_cast(both)); + const SaplingNotePlaintext& pt = both->first; + const SaplingNote& note = both->second; + EXPECT_TRUE(pt.d == legacyPt->d); + EXPECT_EQ(legacyPt->rcm, pt.rcm); + EXPECT_EQ(legacyPt->value(), pt.value()); + EXPECT_TRUE(pt.memo() == legacyPt->memo()); + EXPECT_TRUE(note.d == legacyNote->d); + EXPECT_EQ(legacyNote->pk_d, note.pk_d); + EXPECT_EQ(legacyNote->r, note.r); + EXPECT_EQ(legacyNote->value(), note.value()); + EXPECT_TRUE(SaplingPaymentAddress(note.d, note.pk_d) == pa); + EXPECT_EQ(value, note.value()); + + EXPECT_FALSE(static_cast(SaplingNotePlaintext::decrypt_with_note(od.encCiphertext, otherIvk, od.ephemeralKey, od.cm))); + uint256 badCm = od.cm; + badCm.begin()[0] ^= 1; + EXPECT_FALSE(static_cast(SaplingNotePlaintext::decrypt(od.encCiphertext, ivk, od.ephemeralKey, badCm))); + EXPECT_FALSE(static_cast(SaplingNotePlaintext::decrypt_with_note(od.encCiphertext, ivk, od.ephemeralKey, badCm))); + } +} + +// What FindMySaplingNotes discovers: notes to a spending key's default and diversified addresses, and to +// a viewing-key-only address, each with the key that found it; a diversified address the wallet hasn't +// seen yet is reported for registration; outputs to other wallets are ignored. +TEST(WalletNoteCacheDiscovery, FindMySaplingNotes) { + NoteCacheWallet wallet; + auto skA = TestKey(0), skB = TestKey(1); + const SaplingIncomingViewingKey ivkA = skA.expsk.full_viewing_key().in_viewing_key(); + const SaplingIncomingViewingKey ivkB = skB.expsk.full_viewing_key().in_viewing_key(); + auto addrsA = AddressesOf(skA); + const SaplingPaymentAddress paB = skB.DefaultAddress(); + + LOCK2(cs_main, wallet.cs_wallet); + ASSERT_TRUE(wallet.AddSaplingZKey(skA, addrsA[0])); + ASSERT_TRUE(wallet.AddSaplingIncomingViewingKey(ivkB, paB)); + + CTransaction tx = MakeTx({MakeOutput(addrsA[0], 1), MakeOutput(addrsA[1], 2), MakeOutput(paB, 3), + MakeOutput(TestKey(9).DefaultAddress(), 4), MakeOutput(addrsA[2], 5)}); + auto found = wallet.FindMySaplingNotes(tx); + const mapSaplingNoteData_t& notes = found.first; + ASSERT_EQ(4u, notes.size()); + EXPECT_TRUE(notes.at(SaplingOutPoint(tx.GetHash(), 0)).ivk == ivkA); + EXPECT_TRUE(notes.at(SaplingOutPoint(tx.GetHash(), 1)).ivk == ivkA); + EXPECT_TRUE(notes.at(SaplingOutPoint(tx.GetHash(), 2)).ivk == ivkB); + EXPECT_EQ(0u, notes.count(SaplingOutPoint(tx.GetHash(), 3))); + EXPECT_TRUE(notes.at(SaplingOutPoint(tx.GetHash(), 4)).ivk == ivkA); + for (const auto& item : notes) { + EXPECT_FALSE(static_cast(item.second.nullifier)); + EXPECT_TRUE(item.second.witnesses.empty()); + EXPECT_EQ(-1, item.second.witnessHeight); + } + + const SaplingIncomingViewingKeyMap& toAdd = found.second; + ASSERT_EQ(2u, toAdd.size()) << "the two diversified addresses, not the registered default one"; + EXPECT_TRUE(toAdd.at(addrsA[1]) == ivkA); + EXPECT_TRUE(toAdd.at(addrsA[2]) == ivkA); +} diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5e4d54640..9cf5161f2 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2263,15 +2263,17 @@ std::pair CWallet::FindMySap // Protocol Spec: 4.19 Block Chain Scanning (Sapling) for (uint32_t i = 0; i < nZouts; ++i) { - const OutputDescription output = tx.vShieldedOutput[i]; + const OutputDescription& output = tx.vShieldedOutput[i]; bool found = false; for (auto it = mapSaplingFullViewingKeys.begin(); it != mapSaplingFullViewingKeys.end(); ++it) { SaplingIncomingViewingKey ivk = it->first; - auto result = SaplingNotePlaintext::decrypt(output.encCiphertext, ivk, output.ephemeralKey, output.cm); + auto result = SaplingNotePlaintext::decrypt_with_note(output.encCiphertext, ivk, output.ephemeralKey, output.cm); if (result) { - auto address = ivk.address(result.get().d); - if (address && mapSaplingIncomingViewingKeys.count(address.get()) == 0) { - viewingKeysToAdd[address.get()] = ivk; + // The note's (d, pk_d) is the address it was sent to, exactly what ivk.address(d) would + // derive again: decrypt_with_note already computed pk_d from this ivk and d. + const libzcash::SaplingPaymentAddress address(result->second.d, result->second.pk_d); + if (mapSaplingIncomingViewingKeys.count(address) == 0) { + viewingKeysToAdd[address] = ivk; } // We don't cache the nullifier here as computing it requires knowledge of the note position // in the commitment tree, which can only be determined when the transaction has been mined. @@ -5816,17 +5818,16 @@ void CWallet::GetFilteredNotes( continue; } - auto maybe_pt = SaplingNotePlaintext::decrypt( + // One key agreement and one pk_d derivation; the note and its address come with it. + auto maybe_pt = SaplingNotePlaintext::decrypt_with_note( wtx.vShieldedOutput[op.n].encCiphertext, nd.ivk, wtx.vShieldedOutput[op.n].ephemeralKey, wtx.vShieldedOutput[op.n].cm); 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(); + const SaplingNotePlaintext& notePt = maybe_pt->first; + const libzcash::SaplingNote& note = maybe_pt->second; + const libzcash::SaplingPaymentAddress pa(note.d, note.pk_d); // skip notes which belong to a different payment address in the wallet if (!(filterAddresses.empty() || filterAddresses.count(pa))) { @@ -5844,7 +5845,6 @@ void CWallet::GetFilteredNotes( } } - auto note = notePt.note(nd.ivk).get(); saplingEntries.push_back(SaplingNoteEntry { op, pa, note, notePt.memo(), nDepth }); } diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 7aa08536f..1bf2b746b 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -115,11 +115,16 @@ boost::optional SaplingOutgoingPlaintext::decrypt( } } -boost::optional SaplingNotePlaintext::decrypt( +namespace { + +// SaplingNotePlaintext::decrypt with an incoming viewing key, also handing back the pk_d = [ivk]g_d +// it derives to check cmu. +boost::optional DecryptWithIvk( const SaplingEncCiphertext &ciphertext, const uint256 &ivk, const uint256 &epk, - const uint256 &cmu + const uint256 &cmu, + uint256 &pk_d ) { auto pt = AttemptSaplingEncDecryption(ciphertext, ivk, epk); @@ -140,7 +145,6 @@ boost::optional SaplingNotePlaintext::decrypt( return boost::none; } - uint256 pk_d; if (!librustzcash_ivk_to_pkd(ivk.begin(), ret.d.data(), pk_d.begin())) { return boost::none; } @@ -164,6 +168,36 @@ boost::optional SaplingNotePlaintext::decrypt( return ret; } +} + +boost::optional SaplingNotePlaintext::decrypt( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu +) +{ + uint256 pk_d; + return DecryptWithIvk(ciphertext, ivk, epk, cmu, pk_d); +} + +boost::optional> SaplingNotePlaintext::decrypt_with_note( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu +) +{ + uint256 pk_d; + auto pt = DecryptWithIvk(ciphertext, ivk, epk, cmu, pk_d); + if (!pt) { + return boost::none; + } + // ivk_to_pkd succeeding implies d is a valid diversifier, which is all note(ivk) checks on top. + SaplingNote note(pt->d, pk_d, pt->value(), pt->rcm); + return std::make_pair(*pt, note); +} + boost::optional SaplingNotePlaintext::decrypt( const SaplingEncCiphertext &ciphertext, const uint256 &epk, diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index 47bebe740..72e7dc3f7 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -85,6 +85,17 @@ public: const uint256 &cmu ); + // decrypt(ciphertext, ivk, epk, cmu) that also returns the note. Building the note needs + // pk_d = [ivk]g_d, which the decryption derives anyway to check cmu; note(ivk) would derive it + // again (another Jubjub scalar multiplication). A distinct name rather than an overload, so it + // can never capture a call meant for the (epk, esk, pk_d, cmu) one. + static boost::optional> decrypt_with_note( + const SaplingEncCiphertext &ciphertext, + const uint256 &ivk, + const uint256 &epk, + const uint256 &cmu + ); + boost::optional note(const SaplingIncomingViewingKey& ivk) const; virtual ~SaplingNotePlaintext() {}