From 9ce6753909057ba13dabc3c11cca826a6c4e3e77 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 1 Feb 2023 21:20:11 +0000 Subject: [PATCH] Retroactively enable ZIP 216 before NU5 activation This completes the work started in zcash/zcash#6000. Closes zcash/zcash#6396. --- src/rust/include/librustzcash.h | 2 -- src/rust/src/rustzcash.rs | 21 +++++---------------- src/rust/src/tests/key_agreement.rs | 2 -- src/rust/src/tests/notes.rs | 1 - src/wallet/wallet.cpp | 5 +---- src/zcash/Note.cpp | 23 +++++++++-------------- src/zcash/Note.hpp | 2 -- src/zcash/NoteEncryption.cpp | 19 +++++++++++-------- src/zcash/NoteEncryption.hpp | 1 - 9 files changed, 26 insertions(+), 50 deletions(-) diff --git a/src/rust/include/librustzcash.h b/src/rust/include/librustzcash.h index 83f107b6929..24333ba235f 100644 --- a/src/rust/include/librustzcash.h +++ b/src/rust/include/librustzcash.h @@ -97,7 +97,6 @@ extern "C" { /// The result is also of length 32 and placed in `result`. /// Returns false if the diversifier or pk_d is not valid bool librustzcash_sapling_compute_cmu( - bool zip216_enabled, const unsigned char *diversifier, const unsigned char *pk_d, const uint64_t value, @@ -111,7 +110,6 @@ extern "C" { /// the result is written to the 32-byte /// `result` buffer. bool librustzcash_sapling_ka_agree( - bool zip216_enabled, const unsigned char *p, const unsigned char *sk, unsigned char *result diff --git a/src/rust/src/rustzcash.rs b/src/rust/src/rustzcash.rs index 747ca5bfb2c..cc843467700 100644 --- a/src/rust/src/rustzcash.rs +++ b/src/rust/src/rustzcash.rs @@ -356,7 +356,6 @@ pub extern "C" fn librustzcash_sapling_generate_r(result: *mut [c_uchar; 32]) { // Private utility function to get Note from C parameters fn priv_get_note( - zip216_enabled: bool, diversifier: *const [c_uchar; 11], pk_d: *const [c_uchar; 32], value: u64, @@ -365,12 +364,7 @@ fn priv_get_note( let diversifier = Diversifier(unsafe { *diversifier }); let g_d = diversifier.g_d().ok_or(())?; - let pk_d = de_ct(if zip216_enabled { - jubjub::ExtendedPoint::from_bytes(unsafe { &*pk_d }) - } else { - jubjub::AffinePoint::from_bytes_pre_zip216_compatibility(unsafe { *pk_d }).map(|p| p.into()) - }) - .ok_or(())?; + let pk_d = de_ct(jubjub::ExtendedPoint::from_bytes(unsafe { &*pk_d })).ok_or(())?; let pk_d = de_ct(pk_d.into_subgroup()).ok_or(())?; @@ -407,7 +401,8 @@ pub extern "C" fn librustzcash_sapling_compute_nf( ) -> bool { // ZIP 216: Nullifier derivation is not consensus-critical // (nullifiers are revealed, not calculated by consensus). - let note = match priv_get_note(true, diversifier, pk_d, value, rcm) { + // In any case, ZIP 216 is now enabled retroactively. + let note = match priv_get_note(diversifier, pk_d, value, rcm) { Ok(p) => p, Err(_) => return false, }; @@ -437,14 +432,13 @@ pub extern "C" fn librustzcash_sapling_compute_nf( /// Returns false if `diversifier` or `pk_d` is not valid. #[no_mangle] pub extern "C" fn librustzcash_sapling_compute_cmu( - zip216_enabled: bool, diversifier: *const [c_uchar; 11], pk_d: *const [c_uchar; 32], value: u64, rcm: *const [c_uchar; 32], result: *mut [c_uchar; 32], ) -> bool { - let note = match priv_get_note(zip216_enabled, diversifier, pk_d, value, rcm) { + let note = match priv_get_note(diversifier, pk_d, value, rcm) { Ok(p) => p, Err(_) => return false, }; @@ -461,17 +455,12 @@ pub extern "C" fn librustzcash_sapling_compute_cmu( /// the 32-byte `result` buffer. #[no_mangle] pub extern "C" fn librustzcash_sapling_ka_agree( - zip216_enabled: bool, p: *const [c_uchar; 32], sk: *const [c_uchar; 32], result: *mut [c_uchar; 32], ) -> bool { // Deserialize p - let p = match de_ct(if zip216_enabled { - jubjub::ExtendedPoint::from_bytes(unsafe { &*p }) - } else { - jubjub::AffinePoint::from_bytes_pre_zip216_compatibility(unsafe { *p }).map(|p| p.into()) - }) { + let p = match de_ct(jubjub::ExtendedPoint::from_bytes(unsafe { &*p })) { Some(p) => p, None => return false, }; diff --git a/src/rust/src/tests/key_agreement.rs b/src/rust/src/tests/key_agreement.rs index ab31ea14cf6..c999156ed8e 100644 --- a/src/rust/src/tests/key_agreement.rs +++ b/src/rust/src/tests/key_agreement.rs @@ -43,7 +43,6 @@ fn test_key_agreement() { let addr_pk_d = addr.pk_d().to_bytes(); assert!(librustzcash_sapling_ka_agree( - true, &addr_pk_d, &esk, &mut shared_secret_sender @@ -61,7 +60,6 @@ fn test_key_agreement() { // Create sharedSecret with ephemeral key let mut shared_secret_recipient = [0u8; 32]; assert!(librustzcash_sapling_ka_agree( - true, &epk, &ivk_serialized, &mut shared_secret_recipient diff --git a/src/rust/src/tests/notes.rs b/src/rust/src/tests/notes.rs index 271e77bdffb..1441e5b697a 100644 --- a/src/rust/src/tests/notes.rs +++ b/src/rust/src/tests/notes.rs @@ -649,7 +649,6 @@ fn notes() { // Compute commitment and compare with test vector let mut result = [0u8; 32]; assert!(librustzcash_sapling_compute_cmu( - true, &tv.default_d, &tv.default_pk_d, tv.note_v, diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1c251f0d937..c289b088321 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4415,8 +4415,6 @@ std::optional> CWalletTx::RecoverSaplingNoteWithoutLeadByteCheck(SaplingOutPoint op, std::set& ovks) const { auto output = this->vShieldedOutput[op.n]; - // ZIP 216: This wallet method is not called from consensus rules. - bool zip216Enabled = true; for (auto ovk : ovks) { auto outPt = SaplingOutgoingPlaintext::decrypt( @@ -4431,14 +4429,13 @@ std::optionalesk, outPt->pk_d); + output.encCiphertext, output.ephemeralKey, outPt->esk, outPt->pk_d); // The transaction would not have entered the wallet unless // its plaintext had been successfully decrypted previously. assert(optDeserialized != std::nullopt); auto maybe_pt = SaplingNotePlaintext::plaintext_checks_without_height( - zip216Enabled, *optDeserialized, output.ephemeralKey, outPt->esk, diff --git a/src/zcash/Note.cpp b/src/zcash/Note.cpp index 6e318b4f8ed..b0284327326 100644 --- a/src/zcash/Note.cpp +++ b/src/zcash/Note.cpp @@ -66,9 +66,10 @@ SaplingNote::SaplingNote( std::optional SaplingNote::cmu() const { uint256 result; uint256 rcm_tmp = rcm(); - // ZIP 216: This method is only called from test code. + // We consider ZIP 216 active all of the time because blocks prior to NU5 + // activation (on mainnet and testnet) did not contain Sapling transactions + // that violated its canonicity rule. if (!librustzcash_sapling_compute_cmu( - true, d.data(), pk_d.begin(), value(), @@ -286,7 +287,6 @@ std::optional SaplingNotePlaintext::plaintext_checks_witho uint256 cmu_expected; uint256 rcm = plaintext.rcm(); if (!librustzcash_sapling_compute_cmu( - true, plaintext.d.data(), pk_d.begin(), plaintext.value(), @@ -328,12 +328,10 @@ std::optional SaplingNotePlaintext::decrypt( const uint256 &cmu ) { - // The nu5Active flag passed in here enables the new consensus rules from ZIP 216 - // (https://zips.z.cash/zip-0216#specification) on the following fields: - // - // - pk_d in the outCiphertext field of Sapling coinbase outputs. - bool nu5Active = params.NetworkUpgradeActive(height, Consensus::UPGRADE_NU5); - auto ret = attempt_sapling_enc_decryption_deserialization(nu5Active, ciphertext, epk, esk, pk_d); + // We consider ZIP 216 active all of the time because blocks prior to NU5 + // activation (on mainnet and testnet) did not contain Sapling transactions + // that violated its canonicity rule. + auto ret = attempt_sapling_enc_decryption_deserialization(ciphertext, epk, esk, pk_d); if (!ret) { return std::nullopt; @@ -347,19 +345,18 @@ std::optional SaplingNotePlaintext::decrypt( return std::nullopt; } - return plaintext_checks_without_height(nu5Active, plaintext, epk, esk, pk_d, cmu); + return plaintext_checks_without_height(plaintext, epk, esk, pk_d, cmu); } } std::optional SaplingNotePlaintext::attempt_sapling_enc_decryption_deserialization( - bool zip216Enabled, const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, const uint256 &pk_d ) { - auto encPlaintext = AttemptSaplingEncDecryption(zip216Enabled, ciphertext, epk, esk, pk_d); + auto encPlaintext = AttemptSaplingEncDecryption(ciphertext, epk, esk, pk_d); if (!encPlaintext) { return std::nullopt; @@ -381,7 +378,6 @@ std::optional SaplingNotePlaintext::attempt_sapling_enc_de } std::optional SaplingNotePlaintext::plaintext_checks_without_height( - bool zip216Enabled, const SaplingNotePlaintext &plaintext, const uint256 &epk, const uint256 &esk, @@ -411,7 +407,6 @@ std::optional SaplingNotePlaintext::plaintext_checks_witho uint256 cmu_expected; uint256 rcm = plaintext.rcm(); if (!librustzcash_sapling_compute_cmu( - zip216Enabled, plaintext.d.data(), pk_d.begin(), plaintext.value(), diff --git a/src/zcash/Note.hpp b/src/zcash/Note.hpp index 4b3f4e972df..ed231f41147 100644 --- a/src/zcash/Note.hpp +++ b/src/zcash/Note.hpp @@ -193,7 +193,6 @@ class SaplingNotePlaintext : public BaseNotePlaintext { ); static std::optional plaintext_checks_without_height( - bool zip216Enabled, const SaplingNotePlaintext &plaintext, const uint256 &epk, const uint256 &esk, @@ -202,7 +201,6 @@ class SaplingNotePlaintext : public BaseNotePlaintext { ); static std::optional attempt_sapling_enc_decryption_deserialization( - bool zip216Enabled, const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, diff --git a/src/zcash/NoteEncryption.cpp b/src/zcash/NoteEncryption.cpp index 367fd512794..6c427cfae25 100644 --- a/src/zcash/NoteEncryption.cpp +++ b/src/zcash/NoteEncryption.cpp @@ -115,9 +115,9 @@ std::optional SaplingNoteEncryption::encrypt_to_recipient( uint256 dhsecret; // The new consensus rules from ZIP 216 (https://zips.z.cash/zip-0216#specification) - // on pk_d are enabled unconditionally, as they MAY be enforced in advance of NU5 - // activation. - if (!librustzcash_sapling_ka_agree(true, pk_d.begin(), esk.begin(), dhsecret.begin())) { + // on pk_d were enabled unconditionally, even before we started to apply them + // retroactively. + if (!librustzcash_sapling_ka_agree(pk_d.begin(), esk.begin(), dhsecret.begin())) { return std::nullopt; } @@ -150,9 +150,10 @@ std::optional AttemptSaplingEncDecryption( { uint256 dhsecret; - // ZIP 216: We can enable the rules unconditionally, because ephemeralKey has always - // been required to not be small-order (https://zips.z.cash/zip-0216#specification). - if (!librustzcash_sapling_ka_agree(true, epk.begin(), ivk.begin(), dhsecret.begin())) { + // We consider ZIP 216 active all of the time because blocks prior to NU5 + // activation (on mainnet and testnet) did not contain Sapling transactions + // that violated its canonicity rule. + if (!librustzcash_sapling_ka_agree(epk.begin(), ivk.begin(), dhsecret.begin())) { return std::nullopt; } @@ -180,7 +181,6 @@ std::optional AttemptSaplingEncDecryption( } std::optional AttemptSaplingEncDecryption ( - bool zip216Enabled, const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk, @@ -189,7 +189,10 @@ std::optional AttemptSaplingEncDecryption ( { uint256 dhsecret; - if (!librustzcash_sapling_ka_agree(zip216Enabled, pk_d.begin(), esk.begin(), dhsecret.begin())) { + // We consider ZIP 216 active all of the time because blocks prior to NU5 + // activation (on mainnet and testnet) did not contain Sapling transactions + // that violated its canonicity rule. + if (!librustzcash_sapling_ka_agree(pk_d.begin(), esk.begin(), dhsecret.begin())) { return std::nullopt; } diff --git a/src/zcash/NoteEncryption.hpp b/src/zcash/NoteEncryption.hpp index 91d3adb7987..a4484004b54 100644 --- a/src/zcash/NoteEncryption.hpp +++ b/src/zcash/NoteEncryption.hpp @@ -77,7 +77,6 @@ std::optional AttemptSaplingEncDecryption( // Attempts to decrypt a Sapling note using outgoing plaintext. // This will not check that the contents of the ciphertext are correct. std::optional AttemptSaplingEncDecryption ( - bool zip216Enabled, const SaplingEncCiphertext &ciphertext, const uint256 &epk, const uint256 &esk,