Merge branch 'main' into orchard-mainnet-circuit

.github/workflows/bench.yml

@@ -1,29 +1,35 @@
 name: Benchmarks
-
 on:
   push:
     branches:
       - main
-  pull_request:
-    branches:
-      - main
 
-env:
-  CARGO_TERM_COLOR: always
+permissions:
+  contents: write
+  deployments: write
 
 jobs:
-  build:
-    name: Run orchard benchmarks
+  benchmark:
+    name: Performance regression check
     runs-on: ubuntu-latest
-    env:
-      CRITERION_TOKEN: ${{ secrets.CRITERION_TOKEN }}
     steps:
       - uses: actions/checkout@v2
-      - name: Run benchmarks
-        run: |
-          # run benchmarks and save baseline in a directory called "new"
-          cargo bench -- --verbose
-      - name: Upload benchmarks
-        run: |
-          # upload the files
-          bash <(curl -s https://criterion.dev/bash)
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: 1.51.0
+          override: true
+      - name: Run benchmark
+        run: cargo bench -- --output-format bencher | tee output.txt
+      - name: Store benchmark result
+        uses: benchmark-action/github-action-benchmark@v1
+        with:
+          name: Orchard Benchmarks
+          tool: 'cargo'
+          output-file-path: output.txt
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          auto-push: true
+          # Show alert with commit comment on detecting possible performance regression
+          alert-threshold: '200%'
+          comment-on-alert: true
+          fail-on-alert: true
+          alert-comment-cc-users: '@str4d'

Cargo.toml

@@ -1,29 +1,32 @@
 [package]
 name = "orchard"
-version = "0.0.0"
+version = "0.1.0-beta.1"
 authors = [
     "Sean Bowe <sean@electriccoin.co>",
     "Jack Grigg <jack@electriccoin.co>",
+    "Daira Hopwood <daira@jacaranda.org>",
+    "Ying Tong Lai <yingtong@electriccoin.co>",
+    "Kris Nuttycombe <kris@electriccoin.co>",
 ]
 edition = "2018"
-description = "Sapling on stilts!"
+description = "[BETA] The Orchard shielded transaction protocol"
 license-file = "LICENSE-BOSL"
 repository = "https://github.com/zcash/orchard"
 documentation = "https://docs.rs/orchard"
 readme = "README.md"
-
-# We are not publishing this yet.
-publish = false
+categories = ["cryptography::cryptocurrencies"]
+keywords = ["zcash"]
 
 [package.metadata.docs.rs]
-rustdoc-args = [ "--html-in-header", "katex-header.html" ]
+all-features = true
+rustdoc-args = ["--cfg", "docsrs", "--html-in-header", "katex-header.html"]
 
 [dependencies]
 aes = "0.7"
 arrayvec = "0.7.0"
 bigint = "4"
 bitvec = "0.22"
-blake2b_simd = "0.5"
+blake2b_simd = "1"
 ff = "0.11"
 fpe = "0.5"
 group = "0.11"
@@ -33,26 +36,24 @@ memuse = { version = "0.2", features = ["nonempty"] }
 pasta_curves = "0.2.1"
 proptest = { version = "1.0.0", optional = true }
 rand = "0.8"
+reddsa = "0.1"
 nonempty = "0.7"
 serde = { version = "1.0", features = ["derive"] }
 subtle = "2.3"
-zcash_note_encryption = "0.0"
-incrementalmerkletree = "0.1"
+zcash_note_encryption = "0.1"
+incrementalmerkletree = "0.2"
 
 # Developer tooling dependencies
 plotters = { version = "0.3.0", optional = true }
 
-[dependencies.reddsa]
-git = "https://github.com/str4d/redjubjub.git"
-rev = "416a6a8ebf8bd42c114c938883016c04f338de72"
-
 [dev-dependencies]
 criterion = "0.3"
 hex = "0.4"
 proptest = "1.0.0"
+zcash_note_encryption = { version = "0.1", features = ["pre-zip-212"] }
 
 [target.'cfg(unix)'.dev-dependencies]
-pprof = { version = "0.5", features = ["criterion", "flamegraph"] }
+pprof = { version = "0.6", features = ["criterion", "flamegraph"] }
 
 [lib]
 bench = false
@@ -88,6 +89,4 @@ debug = true
 debug = true
 
 [patch.crates-io]
-zcash_note_encryption = { git = "https://github.com/zcash/librustzcash.git", rev = "35e75420657599fdc701cb45704878eb3fa2e59a" }
-incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "b7bd6246122a6e9ace8edb51553fbf5228906cbb" }
 halo2 = { git = "https://github.com/zcash/halo2.git", rev = "afd7bc5469674cd08eae1634225fd02706a36a4f" }

benches/circuit.rs

@@ -25,7 +25,7 @@ fn criterion_benchmark(c: &mut Criterion) {
     let vk = VerifyingKey::build();
     let pk = ProvingKey::build();
 
-    for num_recipients in 1..=4 {
+    let create_bundle = |num_recipients| {
         let mut builder = Builder::new(
             Flags::from_parts(true, true),
             Anchor::from_bytes([0; 32]).unwrap(),
@@ -43,9 +43,16 @@ fn criterion_benchmark(c: &mut Criterion) {
             .map(|a| a.to_instance(*bundle.flags(), *bundle.anchor()))
             .collect();
 
-        {
-            let mut group = c.benchmark_group("proving");
-            group.sample_size(10);
+        (bundle, instances)
+    };
+
+    let recipients_range = 1..=4;
+
+    {
+        let mut group = c.benchmark_group("proving");
+        group.sample_size(10);
+        for num_recipients in recipients_range.clone() {
+            let (bundle, instances) = create_bundle(num_recipients);
             group.bench_function(BenchmarkId::new("bundle", num_recipients), |b| {
                 b.iter(|| {
                     bundle
@@ -55,9 +62,12 @@ fn criterion_benchmark(c: &mut Criterion) {
                 });
             });
         }
+    }
 
-        {
-            let mut group = c.benchmark_group("verifying");
+    {
+        let mut group = c.benchmark_group("verifying");
+        for num_recipients in recipients_range {
+            let (bundle, instances) = create_bundle(num_recipients);
             let bundle = bundle
                 .create_proof(&pk)
                 .unwrap()

src/address.rs

@@ -67,6 +67,7 @@ impl Address {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use proptest::prelude::*;
 

src/builder.rs

@@ -568,6 +568,7 @@ impl<V> Bundle<InProgress<Proof, PartiallyAuthorized>, V> {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use incrementalmerkletree::{bridgetree::BridgeTree, Frontier, Tree};
 

src/bundle.rs

@@ -506,6 +506,7 @@ pub struct BundleAuthorizingCommitment(pub Blake2bHash);
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use nonempty::NonEmpty;
     use pasta_curves::{arithmetic::FieldExt, pallas};

src/circuit.rs

@@ -969,6 +969,17 @@ mod tests {
             );
         }
 
+        // Test that the proof size is as expected.
+        let expected_proof_size = {
+            let circuit_cost = halo2::dev::CircuitCost::<pasta_curves::vesta::Point, _>::measure(
+                K as usize,
+                &circuits[0],
+            );
+            assert_eq!(usize::from(circuit_cost.proof_size(1)), 4992);
+            assert_eq!(usize::from(circuit_cost.proof_size(2)), 7264);
+            usize::from(circuit_cost.proof_size(instances.len()))
+        };
+
         for (circuit, instance) in circuits.iter().zip(instances.iter()) {
             assert_eq!(
                 MockProver::run(
@@ -989,6 +1000,7 @@ mod tests {
         let pk = ProvingKey::build();
         let proof = Proof::create(&pk, &circuits, &instances).unwrap();
         assert!(proof.verify(&vk, &instances).is_ok());
+        assert_eq!(proof.0.len(), expected_proof_size);
     }
 
     #[cfg(feature = "dev-graph")]

src/keys.rs

@@ -35,9 +35,15 @@ const ZIP32_PURPOSE: u32 = 32;
 /// Defined in [Zcash Protocol Spec § 4.2.3: Orchard Key Components][orchardkeycomponents].
 ///
 /// [orchardkeycomponents]: https://zips.z.cash/protocol/nu5.pdf#orchardkeycomponents
-#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Copy, Clone)]
 pub struct SpendingKey([u8; 32]);
 
+impl ConstantTimeEq for SpendingKey {
+    fn ct_eq(&self, other: &Self) -> Choice {
+        self.to_bytes().ct_eq(other.to_bytes())
+    }
+}
+
 impl SpendingKey {
     /// Generates a random spending key.
     ///
@@ -178,14 +184,17 @@ impl SpendValidatingKey {
     pub(crate) fn from_bytes(bytes: &[u8]) -> Option<Self> {
         <[u8; 32]>::try_from(bytes)
             .ok()
-            .and_then(|b|
-                // check that the sign of the y-coordinate is positive
-                if b[31] & 0x80 == 0 {
+            .and_then(|b| {
+                // Structural validity checks for ak_P:
+                // - The point must not be the identity
+                //   (which for Pallas is canonically encoded as all-zeroes).
+                // - The sign of the y-coordinate must be positive.
+                if b != [0; 32] && b[31] & 0x80 == 0 {
                     <redpallas::VerificationKey<SpendAuth>>::try_from(b).ok()
                 } else {
                     None
                 }
-            )
+            })
             .map(SpendValidatingKey)
     }
 }
@@ -425,6 +434,12 @@ di_from!(u32);
 di_from!(u64);
 di_from!(usize);
 
+impl From<[u8; 11]> for DiversifierIndex {
+    fn from(j_bytes: [u8; 11]) -> Self {
+        DiversifierIndex(j_bytes)
+    }
+}
+
 impl DiversifierKey {
     /// Returns the diversifier at index 0.
     pub fn default_diversifier(&self) -> Diversifier {
@@ -780,6 +795,7 @@ impl SharedSecret {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use proptest::prelude::*;
 
@@ -829,6 +845,12 @@ mod tests {
         Note,
     };
 
+    #[test]
+    fn spend_validating_key_from_bytes() {
+        // ak_P must not be the identity.
+        assert!(SpendValidatingKey::from_bytes(&[0; 32]).is_none());
+    }
+
     #[test]
     fn parsers_reject_invalid() {
         assert!(bool::from(

src/note.rs

@@ -249,6 +249,7 @@ pub struct TransmittedNoteCiphertext {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use proptest::prelude::*;
 

src/note/nullifier.rs

@@ -60,6 +60,7 @@ impl Nullifier {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use group::Group;
     use pasta_curves::{arithmetic::FieldExt, pallas};

src/note_encryption.rs

@@ -5,8 +5,9 @@ use std::{convert::TryInto, fmt};
 use blake2b_simd::{Hash, Params};
 use group::ff::PrimeField;
 use zcash_note_encryption::{
-    BatchDomain, Domain, EphemeralKeyBytes, NotePlaintextBytes, NoteValidity, OutPlaintextBytes,
-    OutgoingCipherKey, ShieldedOutput, COMPACT_NOTE_SIZE, NOTE_PLAINTEXT_SIZE, OUT_PLAINTEXT_SIZE,
+    BatchDomain, Domain, EphemeralKeyBytes, NotePlaintextBytes, OutPlaintextBytes,
+    OutgoingCipherKey, ShieldedOutput, COMPACT_NOTE_SIZE, ENC_CIPHERTEXT_SIZE, NOTE_PLAINTEXT_SIZE,
+    OUT_PLAINTEXT_SIZE,
 };
 
 use crate::{
@@ -182,13 +183,6 @@ impl Domain for OrchardDomain {
         EphemeralPublicKey::from_bytes(&ephemeral_key.0).into()
     }
 
-    fn check_epk_bytes<F: Fn(&Self::EphemeralSecretKey) -> NoteValidity>(
-        note: &Self::Note,
-        check: F,
-    ) -> NoteValidity {
-        check(&note.esk())
-    }
-
     fn cmstar(note: &Self::Note) -> Self::ExtractedCommitment {
         note.commitment().into()
     }
@@ -208,9 +202,9 @@ impl Domain for OrchardDomain {
         pk_d: &Self::DiversifiedTransmissionKey,
         esk: &Self::EphemeralSecretKey,
         ephemeral_key: &EphemeralKeyBytes,
-        plaintext: &[u8],
+        plaintext: &NotePlaintextBytes,
     ) -> Option<(Self::Note, Self::Recipient)> {
-        orchard_parse_note_plaintext_without_memo(self, plaintext, |diversifier| {
+        orchard_parse_note_plaintext_without_memo(self, &plaintext.0, |diversifier| {
             if esk
                 .derive_public(diversify_hash(diversifier.as_array()))
                 .to_bytes()
@@ -224,20 +218,18 @@ impl Domain for OrchardDomain {
         })
     }
 
-    fn extract_memo(&self, plaintext: &[u8]) -> Self::Memo {
-        plaintext[COMPACT_NOTE_SIZE..NOTE_PLAINTEXT_SIZE]
+    fn extract_memo(&self, plaintext: &NotePlaintextBytes) -> Self::Memo {
+        plaintext.0[COMPACT_NOTE_SIZE..NOTE_PLAINTEXT_SIZE]
             .try_into()
             .unwrap()
     }
 
-    fn extract_pk_d(
-        out_plaintext: &[u8; OUT_PLAINTEXT_SIZE],
-    ) -> Option<Self::DiversifiedTransmissionKey> {
-        DiversifiedTransmissionKey::from_bytes(out_plaintext[0..32].try_into().unwrap()).into()
+    fn extract_pk_d(out_plaintext: &OutPlaintextBytes) -> Option<Self::DiversifiedTransmissionKey> {
+        DiversifiedTransmissionKey::from_bytes(out_plaintext.0[0..32].try_into().unwrap()).into()
     }
 
-    fn extract_esk(out_plaintext: &[u8; OUT_PLAINTEXT_SIZE]) -> Option<Self::EphemeralSecretKey> {
-        EphemeralSecretKey::from_bytes(out_plaintext[32..OUT_PLAINTEXT_SIZE].try_into().unwrap())
+    fn extract_esk(out_plaintext: &OutPlaintextBytes) -> Option<Self::EphemeralSecretKey> {
+        EphemeralSecretKey::from_bytes(out_plaintext.0[32..OUT_PLAINTEXT_SIZE].try_into().unwrap())
             .into()
     }
 }
@@ -260,7 +252,7 @@ impl BatchDomain for OrchardDomain {
 /// Implementation of in-band secret distribution for Orchard bundles.
 pub type OrchardNoteEncryption = zcash_note_encryption::NoteEncryption<OrchardDomain>;
 
-impl<T> ShieldedOutput<OrchardDomain> for Action<T> {
+impl<T> ShieldedOutput<OrchardDomain, ENC_CIPHERTEXT_SIZE> for Action<T> {
     fn ephemeral_key(&self) -> EphemeralKeyBytes {
         EphemeralKeyBytes(self.encrypted_note().epk_bytes)
     }
@@ -269,7 +261,7 @@ impl<T> ShieldedOutput<OrchardDomain> for Action<T> {
         self.cmx().to_bytes()
     }
 
-    fn enc_ciphertext(&self) -> &[u8] {
+    fn enc_ciphertext(&self) -> &[u8; ENC_CIPHERTEXT_SIZE] {
         &self.encrypted_note().enc_ciphertext
     }
 }
@@ -299,7 +291,7 @@ impl<T> From<&Action<T>> for CompactAction {
     }
 }
 
-impl ShieldedOutput<OrchardDomain> for CompactAction {
+impl ShieldedOutput<OrchardDomain, COMPACT_NOTE_SIZE> for CompactAction {
     fn ephemeral_key(&self) -> EphemeralKeyBytes {
         EphemeralKeyBytes(self.ephemeral_key.0)
     }
@@ -308,7 +300,7 @@ impl ShieldedOutput<OrchardDomain> for CompactAction {
         self.cmx.to_bytes()
     }
 
-    fn enc_ciphertext(&self) -> &[u8] {
+    fn enc_ciphertext(&self) -> &[u8; COMPACT_NOTE_SIZE] {
         &self.enc_ciphertext
     }
 }

src/primitives/redpallas.rs

@@ -175,6 +175,7 @@ pub(crate) mod private {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use std::convert::TryFrom;
 

src/tree.rs

@@ -81,6 +81,7 @@ pub struct MerklePath {
 }
 
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 impl From<(incrementalmerkletree::Position, Vec<MerkleHashOrchard>)> for MerklePath {
     fn from(path: (incrementalmerkletree::Position, Vec<MerkleHashOrchard>)) -> Self {
         use std::convert::TryInto;
@@ -265,6 +266,7 @@ impl<'de> Deserialize<'de> for MerkleHashOrchard {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     #[cfg(test)]
     use incrementalmerkletree::{

src/value.rs

@@ -296,6 +296,7 @@ impl ValueCommitment {
 
 /// Generators for property testing.
 #[cfg(any(test, feature = "test-dependencies"))]
+#[cfg_attr(docsrs, doc(cfg(feature = "test-dependencies")))]
 pub mod testing {
     use pasta_curves::{arithmetic::FieldExt, pallas};
     use proptest::prelude::*;

src/zip32.rs

@@ -6,6 +6,7 @@ use std::{
 };
 
 use blake2b_simd::Params as Blake2bParams;
+use subtle::{Choice, ConstantTimeEq};
 
 use crate::{
     keys::{FullViewingKey, SpendingKey},
@@ -101,13 +102,13 @@ pub(crate) struct ExtendedSpendingKey {
     sk: SpendingKey,
 }
 
-impl std::cmp::PartialEq for ExtendedSpendingKey {
-    fn eq(&self, rhs: &ExtendedSpendingKey) -> bool {
-        self.depth == rhs.depth
-            && self.parent_fvk_tag == rhs.parent_fvk_tag
-            && self.child_index == rhs.child_index
-            && self.chain_code == rhs.chain_code
-            && self.sk == rhs.sk
+impl ConstantTimeEq for ExtendedSpendingKey {
+    fn ct_eq(&self, rhs: &Self) -> Choice {
+        self.depth.ct_eq(&rhs.depth)
+            & self.parent_fvk_tag.0.ct_eq(&rhs.parent_fvk_tag.0)
+            & self.child_index.0.ct_eq(&rhs.child_index.0)
+            & self.chain_code.0.ct_eq(&rhs.chain_code.0)
+            & self.sk.ct_eq(&rhs.sk)
     }
 }
 
@@ -229,16 +230,17 @@ mod tests {
         let xsk_m = ExtendedSpendingKey::master(&seed).unwrap();
 
         let xsk_5h = xsk_m.derive_child(5.try_into().unwrap()).unwrap();
-        assert_eq!(
-            ExtendedSpendingKey::from_path(&seed, &[5.try_into().unwrap()]).unwrap(),
-            xsk_5h
-        );
+        assert!(bool::from(
+            ExtendedSpendingKey::from_path(&seed, &[5.try_into().unwrap()])
+                .unwrap()
+                .ct_eq(&xsk_5h)
+        ));
 
         let xsk_5h_7 = xsk_5h.derive_child(7.try_into().unwrap()).unwrap();
-        assert_eq!(
+        assert!(bool::from(
             ExtendedSpendingKey::from_path(&seed, &[5.try_into().unwrap(), 7.try_into().unwrap()])
-                .unwrap(),
-            xsk_5h_7
-        );
+                .unwrap()
+                .ct_eq(&xsk_5h_7)
+        ));
     }
 }