Refactor burn types and proptest implementations for Orchard types

1. Introduced new type `Burn` that wraps `Vec<BurnItem>`, implemented serialization/deserialization for it - put it
   into `orchard_zsa/burn.rs`, also moved `NoBurn` there from `orchard_flavor_ext.rs` for better code structuring.
2. Renamed the `EncryptedNoteTest` trait to `TestArbitrary` in `orchard_flavor_ext.rs` and used it to constrain
   `BurnType` as well.
3. Renamed `serialize.rs` to `common.rs` in the `orchard_zsa` folder.
4. Refactored transaction generation strategies for V5 and V6 in `transaction/arbitrary.rs` and
  `orchard_zsa/arbitrary.rs` to incorporate the new burn handling and proper issuance handling.

zebra-chain/src/orchard/orchard_flavor_ext.rs

@@ -1,6 +1,6 @@
 //! This module defines traits and structures for supporting the Orchard Shielded Protocol
 //! for `V5` and `V6` versions of the transaction.
-use std::{fmt::Debug, io};
+use std::fmt::Debug;
 
 use serde::{de::DeserializeOwned, Serialize};
 
@@ -9,24 +9,24 @@ use proptest_derive::Arbitrary;
 
 use orchard::{note_encryption::OrchardDomainCommon, orchard_flavor};
 
-use crate::serialization::{SerializationError, ZcashDeserialize, ZcashSerialize};
-
-use super::note;
+use crate::serialization::{ZcashDeserialize, ZcashSerialize};
 
 #[cfg(feature = "tx-v6")]
-use crate::orchard_zsa::burn::BurnItem;
+use crate::orchard_zsa::burn::{Burn, NoBurn};
+
+use super::note;
 
 #[cfg(not(any(test, feature = "proptest-impl")))]
-pub trait EncryptedNoteTest {}
+pub trait TestArbitrary {}
 
 #[cfg(not(any(test, feature = "proptest-impl")))]
-impl<T> EncryptedNoteTest for T {}
+impl<T> TestArbitrary for T {}
 
 #[cfg(any(test, feature = "proptest-impl"))]
-pub trait EncryptedNoteTest: proptest::prelude::Arbitrary {}
+pub trait TestArbitrary: proptest::prelude::Arbitrary {}
 
 #[cfg(any(test, feature = "proptest-impl"))]
-impl<T: proptest::prelude::Arbitrary> EncryptedNoteTest for T {}
+impl<T: proptest::prelude::Arbitrary> TestArbitrary for T {}
 
 /// A trait representing compile-time settings of Orchard Shielded Protocol used in
 /// the transactions `V5` and `V6`.
@@ -40,17 +40,17 @@ pub trait OrchardFlavorExt: Clone + Debug {
         + Serialize
         + ZcashDeserialize
         + ZcashSerialize
-        + EncryptedNoteTest;
+        + TestArbitrary;
 
-    /// FIXME: add doc
+    /// Specifies the Orchard protocol flavor from `orchard` crate used by this implementation.
     type Flavor: orchard_flavor::OrchardFlavor;
 
     /// The size of the encrypted note for this protocol version.
     const ENCRYPTED_NOTE_SIZE: usize = Self::Flavor::ENC_CIPHERTEXT_SIZE;
 
     /// A type representing a burn field for this protocol version.
-    // FIXME: add cfg tx-v6 here?
-    type BurnType: Clone + Debug + Default + ZcashDeserialize + ZcashSerialize;
+    #[cfg(feature = "tx-v6")]
+    type BurnType: Clone + Debug + Default + ZcashDeserialize + ZcashSerialize + TestArbitrary;
 }
 
 /// A structure representing a tag for Orchard protocol variant used for the transaction version `V5`.
@@ -65,33 +65,19 @@ pub struct OrchardVanilla;
 #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
 pub struct OrchardZSA;
 
-/// A special marker type indicating the absence of a burn field in Orchard ShieldedData for `V5` transactions.
-/// Useful for unifying ShieldedData serialization and deserialization implementations across various
-/// Orchard protocol variants (i.e. various transaction versions).
-#[derive(Default, Clone, Debug, PartialEq, Eq, Serialize)]
-pub struct NoBurn;
-
-impl ZcashSerialize for NoBurn {
-    fn zcash_serialize<W: io::Write>(&self, mut _writer: W) -> Result<(), io::Error> {
-        Ok(())
-    }
-}
-
-impl ZcashDeserialize for NoBurn {
-    fn zcash_deserialize<R: io::Read>(mut _reader: R) -> Result<Self, SerializationError> {
-        Ok(Self)
-    }
-}
-
 impl OrchardFlavorExt for OrchardVanilla {
     type Flavor = orchard_flavor::OrchardVanilla;
     type EncryptedNote = note::EncryptedNote<{ Self::ENCRYPTED_NOTE_SIZE }>;
+
+    #[cfg(feature = "tx-v6")]
     type BurnType = NoBurn;
 }
 
 #[cfg(feature = "tx-v6")]
 impl OrchardFlavorExt for OrchardZSA {
     type Flavor = orchard_flavor::OrchardZSA;
     type EncryptedNote = note::EncryptedNote<{ Self::ENCRYPTED_NOTE_SIZE }>;
-    type BurnType = Vec<BurnItem>;
+
+    #[cfg(feature = "tx-v6")]
+    type BurnType = Burn;
 }

zebra-chain/src/orchard_zsa.rs

@@ -4,8 +4,9 @@
 #[cfg(any(test, feature = "proptest-impl"))]
 pub(crate) mod arbitrary;
 
+mod common;
+
 pub mod burn;
 pub mod issuance;
-pub mod serialize;
 
 pub use burn::BurnItem;

zebra-chain/src/orchard_zsa/arbitrary.rs

@@ -5,31 +5,12 @@ use proptest::prelude::*;
 use orchard::{bundle::testing::BundleArb, issuance::testing::arb_signed_issue_bundle};
 
 // FIXME: consider using another value, i.e. define MAX_BURN_ITEMS constant for that
-use crate::{
-    orchard::{OrchardFlavorExt, OrchardVanilla, OrchardZSA},
-    transaction::arbitrary::MAX_ARBITRARY_ITEMS,
-};
-
-use super::{burn::BurnItem, issuance::IssueData};
-
-pub(crate) trait ArbitraryBurn: OrchardFlavorExt {
-    fn arbitrary_burn() -> BoxedStrategy<Self::BurnType>;
-    // FIXME: remove the following lines
-    //   where
-    //       Self: Sized
-}
-
-impl ArbitraryBurn for OrchardVanilla {
-    fn arbitrary_burn() -> BoxedStrategy<Self::BurnType> {
-        Just(Default::default()).boxed()
-    }
-}
+use crate::transaction::arbitrary::MAX_ARBITRARY_ITEMS;
 
-impl ArbitraryBurn for OrchardZSA {
-    fn arbitrary_burn() -> BoxedStrategy<Self::BurnType> {
-        prop::collection::vec(any::<BurnItem>(), 0..MAX_ARBITRARY_ITEMS).boxed()
-    }
-}
+use super::{
+    burn::{Burn, BurnItem, NoBurn},
+    issuance::IssueData,
+};
 
 impl Arbitrary for BurnItem {
     type Parameters = ();
@@ -51,6 +32,29 @@ impl Arbitrary for BurnItem {
     type Strategy = BoxedStrategy<Self>;
 }
 
+impl Arbitrary for NoBurn {
+    type Parameters = ();
+
+    fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
+        // FIXME: consider using this instead, for clarity: any::<()>().prop_map(|_| NoBurn).boxed()
+        Just(Self).boxed()
+    }
+
+    type Strategy = BoxedStrategy<Self>;
+}
+
+impl Arbitrary for Burn {
+    type Parameters = ();
+
+    fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
+        prop::collection::vec(any::<BurnItem>(), 0..MAX_ARBITRARY_ITEMS)
+            .prop_map(|inner| inner.into())
+            .boxed()
+    }
+
+    type Strategy = BoxedStrategy<Self>;
+}
+
 impl Arbitrary for IssueData {
     type Parameters = ();
 

zebra-chain/src/orchard_zsa/burn.rs

@@ -10,14 +10,15 @@ use crate::{
 
 use orchard::{note::AssetBase, value::NoteValue};
 
-use super::serialize::ASSET_BASE_SIZE;
+use super::common::ASSET_BASE_SIZE;
 
 // Sizes of the serialized values for types in bytes (used for TrustedPreallocate impls)
 const AMOUNT_SIZE: u64 = 8;
+
 // FIXME: is this a correct way to calculate (simple sum of sizes of components)?
 const BURN_ITEM_SIZE: u64 = ASSET_BASE_SIZE + AMOUNT_SIZE;
 
-/// Represents an Orchard ZSA burn item.
+/// Orchard ZSA burn item.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct BurnItem(AssetBase, Amount);
 
@@ -59,18 +60,16 @@ impl TrustedPreallocate for BurnItem {
     }
 }
 
-#[cfg(any(test, feature = "proptest-impl"))]
 impl serde::Serialize for BurnItem {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
-        // FIXME: return custom error with a meaningful description?
+        // FIXME: return a custom error with a meaningful description?
         (self.0.to_bytes(), &self.1).serialize(serializer)
     }
 }
 
-#[cfg(any(test, feature = "proptest-impl"))]
 impl<'de> serde::Deserialize<'de> for BurnItem {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
@@ -87,3 +86,43 @@ impl<'de> serde::Deserialize<'de> for BurnItem {
         ))
     }
 }
+
+/// A special marker type indicating the absence of a burn field in Orchard ShieldedData for `V5` transactions.
+/// Useful for unifying ShieldedData serialization and deserialization implementations across various
+/// Orchard protocol variants (i.e. various transaction versions).
+#[derive(Default, Clone, Debug, PartialEq, Eq, Serialize)]
+pub struct NoBurn;
+
+impl ZcashSerialize for NoBurn {
+    fn zcash_serialize<W: io::Write>(&self, mut _writer: W) -> Result<(), io::Error> {
+        Ok(())
+    }
+}
+
+impl ZcashDeserialize for NoBurn {
+    fn zcash_deserialize<R: io::Read>(mut _reader: R) -> Result<Self, SerializationError> {
+        Ok(Self)
+    }
+}
+
+/// Orchard ZSA burn items (assets intended for burning)
+#[derive(Default, Clone, Debug, PartialEq, Eq, Serialize)]
+pub struct Burn(Vec<BurnItem>);
+
+impl From<Vec<BurnItem>> for Burn {
+    fn from(inner: Vec<BurnItem>) -> Self {
+        Self(inner)
+    }
+}
+
+impl ZcashSerialize for Burn {
+    fn zcash_serialize<W: io::Write>(&self, writer: W) -> Result<(), io::Error> {
+        self.0.zcash_serialize(writer)
+    }
+}
+
+impl ZcashDeserialize for Burn {
+    fn zcash_deserialize<R: io::Read>(reader: R) -> Result<Self, SerializationError> {
+        Ok(Burn(Vec::<BurnItem>::zcash_deserialize(reader)?))
+    }
+}

zebra-chain/src/orchard_zsa/serialize.rs → zebra-chain/src/orchard_zsa/common.rs

@@ -7,7 +7,7 @@ use crate::serialization::{ReadZcashExt, SerializationError, ZcashDeserialize, Z
 use orchard::note::AssetBase;
 
 // The size of the serialized AssetBase in bytes (used for TrustedPreallocate impls)
-pub(crate) const ASSET_BASE_SIZE: u64 = 32;
+pub(super) const ASSET_BASE_SIZE: u64 = 32;
 
 impl ZcashSerialize for AssetBase {
     fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {

zebra-chain/src/orchard_zsa/issuance.rs

@@ -26,7 +26,7 @@ use orchard::{
     Address, Note,
 };
 
-use super::serialize::ASSET_BASE_SIZE;
+use super::common::ASSET_BASE_SIZE;
 
 /// Wrapper for `IssueBundle` used in the context of Transaction V6. This allows the implementation of
 /// a Serde serializer for unit tests within this crate.