From e2fb5e6402abfff4028896cf1d0dc0c870d66302 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@electriccoin.co>
Date: Sun, 6 Jun 2021 23:45:15 +0100
Subject: [PATCH] Impl ZIP 216 on SaplingVerificationContext and
 redjubjub::PublicKey

`PublicKey::verify` now always uses post-ZIP 216 validation rules, which
is fine in non-consensus contexts.

`SaplingVerificationContext` is used by `zcashd`'s consensus rules.
---
 zcash_primitives/CHANGELOG.md             |  3 +++
 zcash_primitives/src/sapling/redjubjub.rs | 18 ++++++++++++++++--
 zcash_proofs/CHANGELOG.md                 |  6 ++++++
 zcash_proofs/src/sapling/verifier.rs      | 20 +++++++++++---------
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md
index c0d4aee49..9bb93f146 100644
--- a/zcash_primitives/CHANGELOG.md
+++ b/zcash_primitives/CHANGELOG.md
@@ -7,6 +7,9 @@ and this library adheres to Rust's notion of
 
 ## [Unreleased]
 ### Added
+- `zcash_primitives::sapling::redjubjub::PublicKey::verify_with_zip216`, for
+  controlling how RedJubjub signatures are validated. `PublicKey::verify` has
+  been altered to always use post-ZIP 216 validation rules.
 - `zcash_primitives::transaction::Builder::with_progress_notifier`, for setting
   a notification channel on which transaction build progress updates will be
   sent.
diff --git a/zcash_primitives/src/sapling/redjubjub.rs b/zcash_primitives/src/sapling/redjubjub.rs
index 8d5939a84..4cc29f09f 100644
--- a/zcash_primitives/src/sapling/redjubjub.rs
+++ b/zcash_primitives/src/sapling/redjubjub.rs
@@ -5,7 +5,7 @@
 
 use ff::{Field, PrimeField};
 use group::GroupEncoding;
-use jubjub::{ExtendedPoint, SubgroupPoint};
+use jubjub::{AffinePoint, ExtendedPoint, SubgroupPoint};
 use rand_core::RngCore;
 use std::io::{self, Read, Write};
 use std::ops::{AddAssign, MulAssign, Neg};
@@ -123,13 +123,27 @@ impl PublicKey {
     }
 
     pub fn verify(&self, msg: &[u8], sig: &Signature, p_g: SubgroupPoint) -> bool {
+        self.verify_with_zip216(msg, sig, p_g, true)
+    }
+
+    pub fn verify_with_zip216(
+        &self,
+        msg: &[u8],
+        sig: &Signature,
+        p_g: SubgroupPoint,
+        zip216_enabled: bool,
+    ) -> bool {
         // c = H*(Rbar || M)
         let c = h_star(&sig.rbar[..], msg);
 
         // Signature checks:
         // R != invalid
         let r = {
-            let r = ExtendedPoint::from_bytes(&sig.rbar);
+            let r = if zip216_enabled {
+                ExtendedPoint::from_bytes(&sig.rbar)
+            } else {
+                AffinePoint::from_bytes_pre_zip216_compatibility(sig.rbar).map(|p| p.to_extended())
+            };
             if r.is_none().into() {
                 return false;
             }
diff --git a/zcash_proofs/CHANGELOG.md b/zcash_proofs/CHANGELOG.md
index ff0305fb4..b89e8e273 100644
--- a/zcash_proofs/CHANGELOG.md
+++ b/zcash_proofs/CHANGELOG.md
@@ -8,11 +8,17 @@ and this library adheres to Rust's notion of
 ## [Unreleased]
 ### Changed
 - MSRV is now 1.51.0.
+- `zcash_proofs::sapling::SaplingVerificationContext::new` now takes a
+  `zip216_enabled` boolean; this is used to control how RedJubjub signatures are
+  validated.
 - Renamed the following in `zcash_proofs::circuit::sprout` to use lower-case
   abbreviations (matching Rust naming conventions):
   - `JSInput` to `JsInput`
   - `JSOutput` to `JsOutput`
 
+### Removed
+- `zcash_proofs::sapling::SaplingVerificationContext: Default`
+
 ## [0.5.0] - 2021-03-26
 ### Added
 - `zcash_proofs::ZcashParameters`
diff --git a/zcash_proofs/src/sapling/verifier.rs b/zcash_proofs/src/sapling/verifier.rs
index bd7bee7ac..3084a9501 100644
--- a/zcash_proofs/src/sapling/verifier.rs
+++ b/zcash_proofs/src/sapling/verifier.rs
@@ -16,19 +16,15 @@ use super::compute_value_balance;
 pub struct SaplingVerificationContext {
     // (sum of the Spend value commitments) - (sum of the Output value commitments)
     cv_sum: jubjub::ExtendedPoint,
-}
-
-impl Default for SaplingVerificationContext {
-    fn default() -> Self {
-        SaplingVerificationContext::new()
-    }
+    zip216_enabled: bool,
 }
 
 impl SaplingVerificationContext {
     /// Construct a new context to be used with a single transaction.
-    pub fn new() -> Self {
+    pub fn new(zip216_enabled: bool) -> Self {
         SaplingVerificationContext {
             cv_sum: jubjub::ExtendedPoint::identity(),
+            zip216_enabled,
         }
     }
 
@@ -62,7 +58,12 @@ impl SaplingVerificationContext {
         (&mut data_to_be_signed[32..64]).copy_from_slice(&sighash_value[..]);
 
         // Verify the spend_auth_sig
-        if !rk.verify(&data_to_be_signed, &spend_auth_sig, SPENDING_KEY_GENERATOR) {
+        if !rk.verify_with_zip216(
+            &data_to_be_signed,
+            &spend_auth_sig,
+            SPENDING_KEY_GENERATOR,
+            self.zip216_enabled,
+        ) {
             return false;
         }
 
@@ -161,10 +162,11 @@ impl SaplingVerificationContext {
         (&mut data_to_be_signed[32..64]).copy_from_slice(&sighash_value[..]);
 
         // Verify the binding_sig
-        bvk.verify(
+        bvk.verify_with_zip216(
             &data_to_be_signed,
             &binding_sig,
             VALUE_COMMITMENT_RANDOMNESS_GENERATOR,
+            self.zip216_enabled,
         )
     }
 }