Only allow construction of Unified keys & addresses with at least one…

… data item.

zcash_client_backend/src/data_api/testing/orchard.rs

@@ -75,6 +75,7 @@ impl ShieldedPoolTester for OrchardPoolTester {
             None,
             None,
         )
+        .expect("Address construction is valid")
         .into()
     }
 
@@ -154,7 +155,9 @@ impl ShieldedPoolTester for OrchardPoolTester {
                 return result.map(|(note, addr, memo)| {
                     (
                         Note::Orchard(note),
-                        UnifiedAddress::from_receivers(Some(addr), None, None).into(),
+                        UnifiedAddress::from_receivers(Some(addr), None, None)
+                            .expect("Address construction is valid")
+                            .into(),
                         MemoBytes::from_bytes(&memo).expect("correct length"),
                     )
                 });

zcash_keys/CHANGELOG.md

@@ -24,6 +24,7 @@ and this library adheres to Rust's notion of
    unknown_data, unknown_metadata
  }`
 - `zcash_keys::address::UnifiedAddressRequest::unsafe_new_without_expiry`
+- `zcash_keys::keys::UnifiedKeyError`
 
 ### Changed
 - `zcash_keys::address::UnifiedAddressRequest::new` now takes additional
@@ -38,17 +39,28 @@ and this library adheres to Rust's notion of
   - `zcash_keys::keys::UnifiedIncomingViewingKey::default_address`
 - `zcash_keys::address::UnifiedAddress::from_receivers` is now only available
   under the `test-dependencies` feature flag. It should not be used for
-  non-test purposes as it can potentially generate addresses that contain no
-  receivers.
+  non-test purposes.
+- `zcash_keys::address::UnifiedAddress` can now represent ZIP 316 Revision 1
+  unified addresses, which may lack any shielded receivers; addresses are now
+  constrained to include at least one data item, but are not otherwise
+  restricted in their receiver composition.
 - `zcash_keys::keys::UnifiedSpendingKey::default_address` is now failable, and
   now returns a `Result<(UnifiedAddress, DiversifierIndex), AddressGenerationError>`.
 - `zcash_keys::address::Address` variants now `Box` their contents to
   avoid large discrepancies in enum variant sizing.
+- `zcash_keys::keys::DecodingError` has added variant `ConstraintViolation`.
+- The following methods now return `Result<_, UnifiedKeyError>` instead of
+  `Result<_, DerivationError>`
+  - `zcash_keys::keys::UnifiedSpendingKey::from_seed`
+  - `zcash_keys::keys::UnifiedFullViewingKey::new`
+  - `zcash_keys::keys::UnifiedFullViewingKey::from_orchard_fvk`
+  - `zcash_keys::keys::UnifiedFullViewingKey::from_sapling_extended_full_viewing_key`
 
 ### Removed
 - `zcash_keys::address::UnifiedAddress::unknown` (use `unknown_data` instead.)
-- `zcash_keys::address::UnifiedAddressRequest::unsafe_new` (use 
+- `zcash_keys::address::UnifiedAddressRequest::unsafe_new` (use
   `UnifiedAddressRequest::unsafe_new_without_expiry` instead)
+- `zcash_keys::keys::DerivationError` (use `UnifiedKeyError` instead).
 
 ## [0.5.0] - 2024-11-14
 

zcash_keys/src/address.rs

@@ -93,7 +93,7 @@ impl TryFrom<unified::Address> for UnifiedAddress {
             }
         }
 
-        Ok(Self {
+        Self::from_checked_parts(
             #[cfg(feature = "orchard")]
             orchard,
             #[cfg(feature = "sapling")]
@@ -103,7 +103,8 @@ impl TryFrom<unified::Address> for UnifiedAddress {
             expiry_height,
             expiry_time,
             unknown_metadata,
-        })
+        )
+        .ok_or("Unified addresses without data fields are not permitted.")
     }
 }
 
@@ -118,36 +119,55 @@ impl UnifiedAddress {
         #[cfg(feature = "orchard")] orchard: Option<orchard::Address>,
         #[cfg(feature = "sapling")] sapling: Option<PaymentAddress>,
         transparent: Option<TransparentAddress>,
-    ) -> Self {
-        Self::new_internal(
+    ) -> Option<Self> {
+        Self::from_checked_parts(
             #[cfg(feature = "orchard")]
             orchard,
             #[cfg(feature = "sapling")]
             sapling,
             transparent,
+            vec![],
             None,
             None,
+            vec![],
         )
     }
 
-    pub(crate) fn new_internal(
+    pub(crate) fn from_checked_parts(
         #[cfg(feature = "orchard")] orchard: Option<orchard::Address>,
         #[cfg(feature = "sapling")] sapling: Option<PaymentAddress>,
         transparent: Option<TransparentAddress>,
+        unknown_data: Vec<(u32, Vec<u8>)>,
         expiry_height: Option<BlockHeight>,
         expiry_time: Option<u64>,
-    ) -> Self {
-        Self {
+        unknown_metadata: Vec<(u32, Vec<u8>)>,
+    ) -> Option<Self> {
+        let has_transparent = transparent.is_some();
+
+        #[allow(unused_mut)]
+        let mut has_shielded = false;
+        #[cfg(feature = "sapling")]
+        {
+            has_shielded = has_shielded || sapling.is_some();
+        }
+        #[cfg(feature = "orchard")]
+        {
+            has_shielded = has_shielded || orchard.is_some();
+        }
+
+        let has_unknown = !unknown_data.is_empty();
+
+        (has_transparent || has_shielded || has_unknown).then_some(Self {
             #[cfg(feature = "orchard")]
             orchard,
             #[cfg(feature = "sapling")]
             sapling,
             transparent,
-            unknown_data: vec![],
+            unknown_data,
             expiry_height,
             expiry_time,
-            unknown_metadata: vec![],
-        }
+            unknown_metadata,
+        })
     }
 
     /// Returns whether this address has an Orchard receiver.
@@ -599,15 +619,25 @@ mod tests {
         let transparent = None;
 
         #[cfg(all(feature = "orchard", feature = "sapling"))]
-        let ua = UnifiedAddress::new_internal(orchard, sapling, transparent, None, None);
+        let ua = UnifiedAddress::from_checked_parts(
+            orchard,
+            sapling,
+            transparent,
+            vec![],
+            None,
+            None,
+            vec![],
+        );
 
         #[cfg(all(not(feature = "orchard"), feature = "sapling"))]
-        let ua = UnifiedAddress::new_internal(sapling, transparent, None, None);
+        let ua =
+            UnifiedAddress::from_checked_parts(sapling, transparent, vec![], None, None, vec![]);
 
         #[cfg(all(feature = "orchard", not(feature = "sapling")))]
-        let ua = UnifiedAddress::new_internal(orchard, transparent, None, None);
+        let ua =
+            UnifiedAddress::from_checked_parts(orchard, transparent, vec![], None, None, vec![]);
 
-        let addr = Address::from(ua);
+        let addr = Address::from(ua.expect("test UAs are constructed in valid configurations"));
         let addr_str = addr.encode(&MAIN_NETWORK);
         assert_eq!(Address::decode(&MAIN_NETWORK, &addr_str), Some(addr));
     }
@@ -616,7 +646,10 @@ mod tests {
     #[cfg(not(any(feature = "orchard", feature = "sapling")))]
     fn ua_round_trip() {
         let transparent = None;
-        assert_eq!(UnifiedAddress::new_internal(transparent, None, None), None)
+        assert_eq!(
+            UnifiedAddress::from_checked_parts(transparent, vec![], None, None, vec![]),
+            None
+        )
     }
 
     #[test]