Corrections to V7 transaction format and using updated test vectors (#66

)

This PR makes the following changes:
- reordering of the component fields in `read_action` and `write_action`
in issuance in order to make them spec compliant.
- It sets some of the constant values for NU7 in a way that is
consistent with the generated test vectors from the Python reference
implementation (See QED-it/zcash-test-vectors#22
for the changes to the Python reference implementation).
- Some changes are made to function names to reflect the shift from
preparing ZSAs for V7 rather than V6.
- There are changes to the tests to use the `zcash_unstable` flag in
order to cover both ZSA and vanilla Orchard behavior.
- The test vectors are also expanded to include both the V5 vectors, and
the V7 vectors (behind a `zcash_unstable` flag).

Cargo.toml

@@ -124,5 +124,5 @@ panic = 'abort'
 codegen-units = 1
 
 [patch.crates-io]
-zcash_note_encryption = { version = "0.4", git = "https://github.com/QED-it/zcash_note_encryption", branch = "fix-sapling-constants" }
+zcash_note_encryption = { version = "0.4", git = "https://github.com/QED-it/zcash_note_encryption", branch = "note-bytes" }
 sapling = { package = "sapling-crypto", version = "0.1.3", git = "https://github.com/QED-it/sapling-crypto", branch = "orchard-backward-compatibility" }

zcash_primitives/src/transaction/builder.rs

@@ -54,11 +54,11 @@ use crate::{
 use crate::transaction::builder::Error::{IssuanceBuilder, IssuanceBundle};
 use orchard::note::AssetBase;
 use orchard::orchard_flavor::OrchardVanilla;
-#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */ use orchard::orchard_flavor::OrchardZSA;
 #[cfg(zcash_unstable = "nu6")] /* TODO nu7 */
 use orchard::{
     issuance::{IssueBundle, IssueInfo},
     keys::{IssuanceAuthorizingKey, IssuanceValidatingKey},
+    orchard_flavor::OrchardZSA,
 };
 #[cfg(zcash_unstable = "nu6")] /* TODO nu7 */ use rand_core::OsRng;
 

zcash_primitives/src/transaction/components/issuance.rs

@@ -57,10 +57,10 @@ fn read_authorization<R: Read>(mut reader: R) -> io::Result<Signed> {
 }
 
 fn read_action<R: Read>(mut reader: R) -> io::Result<IssueAction> {
-    let finalize = reader.read_u8()? != 0;
-    let notes = Vector::read(&mut reader, |r| read_note(r))?;
     let asset_descr_bytes = Vector::read(&mut reader, |r| r.read_u8())?;
     let asset_descr: String = String::from_utf8(asset_descr_bytes).unwrap();
+    let notes = Vector::read(&mut reader, |r| read_note(r))?;
+    let finalize = reader.read_u8()? != 0;
     Ok(IssueAction::from_parts(asset_descr, notes, finalize))
 }
 
@@ -131,11 +131,11 @@ pub fn write_v7_bundle<W: Write>(
 
 fn write_action<W: Write>(action: &IssueAction, mut writer: W) -> io::Result<()> {
     let is_finalized_u8: u8 = if action.is_finalized() { 1 } else { 0 };
-    writer.write_u8(is_finalized_u8)?;
-    Vector::write(&mut writer, action.notes(), |w, note| write_note(note, w))?;
     Vector::write(&mut writer, action.asset_desc().as_bytes(), |w, b| {
         w.write_u8(*b)
     })?;
+    Vector::write(&mut writer, action.notes(), |w, note| write_note(note, w))?;
+    writer.write_u8(is_finalized_u8)?;
     Ok(())
 }
 

zcash_primitives/src/transaction/components/orchard.rs

@@ -86,8 +86,8 @@ pub fn read_v5_bundle<R: Read>(
     }
 }
 
-/// Reads an [`orchard::Bundle`] from a v5 transaction format.
-pub fn read_v6_bundle<R: Read>(
+/// Reads an [`orchard::Bundle`] from a v7 transaction format.
+pub fn read_v7_bundle<R: Read>(
     mut reader: R,
 ) -> io::Result<Option<orchard::Bundle<Authorized, Amount, OrchardZSA>>> {
     #[allow(clippy::redundant_closure)]
@@ -312,8 +312,8 @@ pub fn write_v5_bundle<W: Write>(
     Ok(())
 }
 
-/// Writes an [`orchard::Bundle`] in the v6 transaction format.
-pub fn write_v6_bundle<W: Write>(
+/// Writes an [`orchard::Bundle`] in the v7 transaction format.
+pub fn write_v7_bundle<W: Write>(
     bundle: Option<&orchard::Bundle<Authorized, Amount, OrchardZSA>>,
     mut writer: W,
 ) -> io::Result<()> {

zcash_primitives/src/transaction/components/sapling.rs

@@ -6,7 +6,7 @@ use zcash_protocol::consensus::{BlockHeight, NetworkUpgrade, Parameters, ZIP212_
 use std::io::{self, Read, Write};
 
 use zcash_encoding::{Array, CompactSize, Vector};
-use zcash_note_encryption::{EphemeralKeyBytes, ENC_CIPHERTEXT_SIZE, OUT_CIPHERTEXT_SIZE};
+use zcash_note_encryption::{EphemeralKeyBytes, OUT_CIPHERTEXT_SIZE};
 
 use crate::{
     sapling::{
@@ -15,6 +15,7 @@ use crate::{
             OutputDescriptionV5, SpendDescription, SpendDescriptionV5,
         },
         note::ExtractedNoteCommitment,
+        note_encryption::ENC_CIPHERTEXT_SIZE,
         value::ValueCommitment,
         Nullifier,
     },

zcash_primitives/src/transaction/mod.rs

@@ -58,7 +58,7 @@ const V5_VERSION_GROUP_ID: u32 = 0x26A7270A;
 #[cfg(zcash_unstable = "nu6")] /* TODO nu7 */
 const V7_TX_VERSION: u32 = 7;
 #[cfg(zcash_unstable = "nu6")] /* TODO nu7 */
-const V7_VERSION_GROUP_ID: u32 = 0x26A7270C; // TODO ???
+const V7_VERSION_GROUP_ID: u32 = 0x124A69F8; // TODO ???
 
 /// These versions are used exclusively for in-development transaction
 /// serialization, and will never be active under the consensus rules.
@@ -930,7 +930,7 @@ impl Transaction {
             Self::read_v5_header_fragment(&mut reader)?;
         let transparent_bundle = Self::read_transparent(&mut reader)?;
         let sapling_bundle = sapling_serialization::read_v5_bundle(&mut reader)?;
-        let orchard_zsa_bundle = orchard_serialization::read_v6_bundle(&mut reader)?;
+        let orchard_zsa_bundle = orchard_serialization::read_v7_bundle(&mut reader)?;
         let issue_bundle = issuance::read_v7_bundle(&mut reader)?;
 
         #[cfg(zcash_unstable = "zfuture")]
@@ -1079,13 +1079,13 @@ impl Transaction {
         if self.sprout_bundle.is_some() {
             return Err(io::Error::new(
                 io::ErrorKind::InvalidInput,
-                "Sprout components cannot be present when serializing to the V5 transaction format.",
+                "Sprout components cannot be present when serializing to the V7 transaction format.",
             ));
         }
         self.write_v5_header(&mut writer)?;
         self.write_transparent(&mut writer)?;
         self.write_v5_sapling(&mut writer)?;
-        orchard_serialization::write_v6_bundle(self.orchard_zsa_bundle.as_ref(), &mut writer)?;
+        orchard_serialization::write_v7_bundle(self.orchard_zsa_bundle.as_ref(), &mut writer)?;
         issuance::write_v7_bundle(self.issue_bundle.as_ref(), &mut writer)?;
         #[cfg(zcash_unstable = "zfuture")]
         self.write_tze(&mut writer)?;

zcash_primitives/src/transaction/tests.rs

@@ -236,7 +236,12 @@ fn zip_0244() {
     fn to_test_txdata(
         tv: &self::data::zip_0244::TestVector,
     ) -> (TransactionData<TestUnauthorized>, TxDigests<Blake2bHash>) {
-        let tx = Transaction::read(&tv.tx[..], BranchId::Nu5).unwrap();
+        let tx = Transaction::read(
+            &tv.tx[..],
+            #[cfg(not(zcash_unstable = "nu6"))] /* TODO nu7 */ BranchId::Nu5,
+            #[cfg(zcash_unstable = "nu6")] /* TODO nu7 */ BranchId::Nu7,
+        )
+        .unwrap();
 
         assert_eq!(tx.txid.as_ref(), &tv.txid);
         assert_eq!(tx.auth_commitment().as_ref(), &tv.auth_digest);