Auto merge of #4674 - therealyingtong:zip-212-ncc-fixes, r=therealyingtong

ZIP 212 NCC fixes

Fixes to the ZIP 212 implementation as suggested in the NCC audit.

src/main.cpp

@@ -966,10 +966,11 @@ 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()
+                // ZIP 212: after ZIP 212 any Sapling output of a coinbase tx that is
+                // decrypted to a note plaintext, MUST have note plaintext lead byte equal
+                // to 0x02. This applies even during the grace period, and also applies to
+                // funding stream outputs sent to shielded payment addresses, if any.
+                // https://zips.z.cash/zip-0212#consensus-rule-change-for-coinbase-transactions
                 if (canopyActive != (encPlaintext->get_leadbyte() == 0x02)) {
                     return state.DoS(
                         DOS_LEVEL_BLOCK,

src/wallet/rpcwallet.cpp

@@ -3804,7 +3804,10 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp)
         auto wtxPrev = pwalletMain->mapWallet.at(op.hash);
 
         // We don't need to check the leadbyte here: if wtx exists in
-        // the wallet, it must have already passed the leadbyte check
+        // the wallet, it must have been successfully decrypted. This
+        // means the plaintext leadbyte was valid at the block height
+        // where the note was received.
+        // https://zips.z.cash/zip-0212#changes-to-the-process-of-receiving-sapling-notes
         auto decrypted = wtxPrev.DecryptSaplingNoteWithoutLeadByteCheck(op).get();
         auto notePt = decrypted.first;
         auto pa = decrypted.second;
@@ -3834,7 +3837,10 @@ UniValue z_viewtransaction(const UniValue& params, bool fHelp)
         bool isOutgoing;
 
         // We don't need to check the leadbyte here: if wtx exists in
-        // the wallet, it must have already passed the leadbyte check
+        // the wallet, it must have been successfully decrypted. This
+        // means the plaintext leadbyte was valid at the block height
+        // where the note was received.
+        // https://zips.z.cash/zip-0212#changes-to-the-process-of-receiving-sapling-notes
         auto decrypted = wtx.DecryptSaplingNoteWithoutLeadByteCheck(op);
         if (decrypted) {
             notePt = decrypted->first;

src/zcash/Note.cpp

@@ -173,6 +173,7 @@ boost::optional<SaplingNote> SaplingNotePlaintext::note(const SaplingIncomingVie
     if (addr) {
         Zip212Enabled zip_212_enabled = Zip212Enabled::BeforeZip212;
         if (leadbyte != 0x01) {
+            assert(leadbyte == 0x02);
             zip_212_enabled = Zip212Enabled::AfterZip212;
         };
         auto tmp = SaplingNote(d, addr.get().pk_d, value_, rseed, zip_212_enabled);
@@ -292,6 +293,7 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::plaintext_checks_wit
     }
 
     if (plaintext.get_leadbyte() != 0x01) {
+        assert(plaintext.get_leadbyte() == 0x02);
         // ZIP 212: Check that epk is consistent to guard against linkability
         // attacks without relying on the soundness of the SNARK.
         uint256 expected_epk;
@@ -369,7 +371,17 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::plaintext_checks_wit
     const uint256 &cmu
 )
 {
-    // Check that epk is consistent with esk
+    if (plaintext.get_leadbyte() != 0x01) {
+        assert(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_or_derive_esk()) {
+            return boost::none;
+        }
+    }
+
+    // ZIP 212: The recipient MUST derive esk and check that epk is consistent with it.
+    // https://zips.z.cash/zip-0212#changes-to-the-process-of-receiving-sapling-notes
     uint256 expected_epk;
     if (!librustzcash_sapling_ka_derivepublic(plaintext.d.data(), esk.begin(), expected_epk.begin())) {
         return boost::none;
@@ -395,14 +407,6 @@ boost::optional<SaplingNotePlaintext> SaplingNotePlaintext::plaintext_checks_wit
         return boost::none;
     }
 
-    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_or_derive_esk()) {
-            return boost::none;
-        }
-    }
-
     return plaintext;
 }
 
@@ -450,6 +454,7 @@ SaplingOutCiphertext SaplingOutgoingPlaintext::encrypt(
 
 uint256 SaplingNotePlaintext::rcm() const {
     if (leadbyte != 0x01) {
+        assert(leadbyte == 0x02);
         return PRF_rcm(rseed);
     } else {
         return rseed;
@@ -466,6 +471,7 @@ uint256 SaplingNote::rcm() const {
 
 uint256 SaplingNotePlaintext::generate_or_derive_esk() const {
     if (leadbyte != 0x01) {
+        assert(leadbyte == 0x02);
         return PRF_esk(rseed);
     } else {
         uint256 esk;

src/zcash/prf.cpp

@@ -27,7 +27,7 @@ std::array<unsigned char, 64> PRF_expand(const uint256& sk, unsigned char t)
 uint256 PRF_rcm(const uint256& rseed)
 {
     uint256 rcm;
-    auto tmp = PRF_expand(rseed, 4);
+    auto tmp = PRF_expand(rseed, PRF_RCM_TAG);
     librustzcash_to_scalar(tmp.data(), rcm.begin());
     return rcm;
 }
@@ -35,7 +35,7 @@ uint256 PRF_rcm(const uint256& rseed)
 uint256 PRF_esk(const uint256& rseed)
 {
     uint256 esk;
-    auto tmp = PRF_expand(rseed, 5);
+    auto tmp = PRF_expand(rseed, PRF_ESK_TAG);
     librustzcash_to_scalar(tmp.data(), esk.begin());
     return esk;
 }
@@ -43,7 +43,7 @@ uint256 PRF_esk(const uint256& rseed)
 uint256 PRF_ask(const uint256& sk)
 {
     uint256 ask;
-    auto tmp = PRF_expand(sk, 0);
+    auto tmp = PRF_expand(sk, PRF_ASK_TAG);
     librustzcash_to_scalar(tmp.data(), ask.begin());
     return ask;
 }
@@ -51,7 +51,7 @@ uint256 PRF_ask(const uint256& sk)
 uint256 PRF_nsk(const uint256& sk)
 {
     uint256 nsk;
-    auto tmp = PRF_expand(sk, 1);
+    auto tmp = PRF_expand(sk, PRF_NSK_TAG);
     librustzcash_to_scalar(tmp.data(), nsk.begin());
     return nsk;
 }
@@ -59,7 +59,7 @@ uint256 PRF_nsk(const uint256& sk)
 uint256 PRF_ovk(const uint256& sk)
 {
     uint256 ovk;
-    auto tmp = PRF_expand(sk, 2);
+    auto tmp = PRF_expand(sk, PRF_OVK_TAG);
     memcpy(ovk.begin(), tmp.data(), 32);
     return ovk;
 }

src/zcash/prf.h

@@ -25,6 +25,12 @@ uint256 PRF_ovk(const uint256& sk);
 uint256 PRF_rcm(const uint256& rseed);
 uint256 PRF_esk(const uint256& rseed);
 
+const char PRF_ASK_TAG = 0;
+const char PRF_NSK_TAG = 1;
+const char PRF_OVK_TAG = 2;
+const char PRF_RCM_TAG = 4;
+const char PRF_ESK_TAG = 5;
+
 std::array<unsigned char, 11> default_diversifier(const uint256& sk);
 
 #endif // ZC_PRF_H_