From 25045b3914ed539cab375509741e1037445a3f64 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 14 Mar 2024 15:23:40 -0600 Subject: [PATCH] zcash_keys: Keep the Ufvk and Uivk encodings private. --- zcash_client_sqlite/src/wallet.rs | 18 ++------- .../init/migrations/full_account_ids.rs | 4 +- zcash_keys/src/keys.rs | 40 ++++++++++++++----- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 90d2616fc0..90dc840aed 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -74,7 +74,6 @@ use std::io::{self, Cursor}; use std::num::NonZeroU32; use std::ops::RangeInclusive; use tracing::debug; -use zcash_address::unified::{Encoding, Uivk}; use zcash_keys::keys::{ AddressGenerationError, HdSeedFingerprint, UnifiedAddressRequest, UnifiedIncomingViewingKey, }; @@ -120,7 +119,7 @@ use { crate::UtxoId, rusqlite::Row, std::collections::BTreeSet, - zcash_address::unified::Ivk, + zcash_address::unified::{Encoding, Ivk, Uivk}, zcash_client_backend::wallet::{TransparentAddressMetadata, WalletTransparentOutput}, zcash_primitives::{ legacy::{ @@ -347,7 +346,7 @@ pub(crate) fn add_account( ":hd_seed_fingerprint": hd_seed_fingerprint.as_ref().map(|fp| fp.as_bytes()), ":hd_account_index": hd_account_index.map(u32::from), ":ufvk": viewing_key.ufvk().map(|ufvk| ufvk.encode(params)), - ":uivk": viewing_key.uivk().to_uivk().encode(¶ms.network_type()), + ":uivk": viewing_key.uivk().encode(params), ":orchard_fvk_item_cache": orchard_item, ":sapling_fvk_item_cache": sapling_item, ":p2pkh_fvk_item_cache": transparent_item, @@ -1503,18 +1502,9 @@ pub(crate) fn get_account( )) } else { let uivk_str: String = row.get("uivk")?; - let (network, uivk) = Uivk::decode(&uivk_str).map_err(|e| { - SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) - })?; - if network != params.network_type() { - return Err(SqliteClientError::CorruptedData( - "UIVK network type does not match wallet network type".to_string(), - )); - } ViewingKey::Incoming(Box::new( - UnifiedIncomingViewingKey::from_uivk(&uivk).map_err(|e| { - SqliteClientError::CorruptedData(format!("Failure to decode UIVK: {e}")) - })?, + UnifiedIncomingViewingKey::decode(params, &uivk_str[..]) + .map_err(SqliteClientError::BadAccountData)?, )) }; diff --git a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs index 07689421d3..fa019be21e 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/full_account_ids.rs @@ -5,7 +5,6 @@ use rusqlite::{named_params, Transaction}; use schemer_rusqlite::RusqliteMigration; use secrecy::{ExposeSecret, SecretVec}; use uuid::Uuid; -use zcash_address::unified::Encoding; use zcash_client_backend::{data_api::AccountKind, keys::UnifiedSpendingKey}; use zcash_keys::keys::{HdSeedFingerprint, UnifiedFullViewingKey}; use zcash_primitives::consensus; @@ -122,8 +121,7 @@ impl RusqliteMigration for Migration

{ let uivk = ufvk_parsed .to_unified_incoming_viewing_key() - .to_uivk() - .encode(&self.params.network_type()); + .encode(&self.params); #[cfg(feature = "transparent-inputs")] let transparent_item = ufvk_parsed.transparent().map(|k| k.serialize()); diff --git a/zcash_keys/src/keys.rs b/zcash_keys/src/keys.rs index aac28cda32..d03b5852c9 100644 --- a/zcash_keys/src/keys.rs +++ b/zcash_keys/src/keys.rs @@ -740,13 +740,13 @@ impl UnifiedFullViewingKey { )); } - Self::from_ufvk(&ufvk).map_err(|e| e.to_string()) + Self::parse(&ufvk).map_err(|e| e.to_string()) } /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. /// /// [ZIP 316]: https://zips.z.cash/zip-0316 - pub fn from_ufvk(ufvk: &Ufvk) -> Result { + pub fn parse(ufvk: &Ufvk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -823,7 +823,7 @@ impl UnifiedFullViewingKey { } /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. - pub fn to_ufvk(&self) -> Ufvk { + fn to_ufvk(&self) -> Ufvk { let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { unified::Fvk::Unknown { typecode: *typecode, @@ -973,8 +973,24 @@ impl UnifiedIncomingViewingKey { } } + /// Parses a `UnifiedFullViewingKey` from its [ZIP 316] string encoding. + /// + /// [ZIP 316]: https://zips.z.cash/zip-0316 + pub fn decode(params: &P, encoding: &str) -> Result { + let (net, ufvk) = unified::Uivk::decode(encoding).map_err(|e| e.to_string())?; + let expected_net = params.network_type(); + if net != expected_net { + return Err(format!( + "UIVK is for network {:?} but we expected {:?}", + net, expected_net, + )); + } + + Self::parse(&ufvk).map_err(|e| e.to_string()) + } + /// Constructs a unified incoming viewing key from a parsed unified encoding. - pub fn from_uivk(uivk: &Uivk) -> Result { + fn parse(uivk: &Uivk) -> Result { #[cfg(feature = "orchard")] let mut orchard = None; #[cfg(feature = "sapling")] @@ -1041,8 +1057,13 @@ impl UnifiedIncomingViewingKey { }) } + /// Returns the string encoding of this `UnifiedFullViewingKey` for the given network. + pub fn encode(&self, params: &P) -> String { + self.render().encode(¶ms.network_type()) + } + /// Converts this unified incoming viewing key to a unified encoding. - pub fn to_uivk(&self) -> Uivk { + fn render(&self) -> Uivk { let items = std::iter::empty().chain(self.unknown.iter().map(|(typecode, data)| { unified::Ivk::Unknown { typecode: *typecode, @@ -1530,7 +1551,7 @@ mod tests { orchard, ); - let encoded = uivk.to_uivk().encode(&NetworkType::Main); + let encoded = uivk.render().encode(&NetworkType::Main); // Test encoded form against known values; these test vectors contain Orchard receivers // that will be treated as unknown if the `orchard` feature is not enabled. @@ -1548,9 +1569,8 @@ mod tests { assert_eq!(encoded, _encoded_no_t); } - let decoded = - UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(&encoded).unwrap().1).unwrap(); - let reencoded = decoded.to_uivk().encode(&NetworkType::Main); + let decoded = UnifiedIncomingViewingKey::parse(&Uivk::decode(&encoded).unwrap().1).unwrap(); + let reencoded = decoded.render().encode(&NetworkType::Main); assert_eq!(encoded, reencoded); #[cfg(feature = "transparent-inputs")] @@ -1570,7 +1590,7 @@ mod tests { ); let decoded_with_t = - UnifiedIncomingViewingKey::from_uivk(&Uivk::decode(encoded_with_t).unwrap().1).unwrap(); + UnifiedIncomingViewingKey::parse(&Uivk::decode(encoded_with_t).unwrap().1).unwrap(); #[cfg(feature = "transparent-inputs")] assert_eq!( decoded_with_t.transparent.map(|t| t.serialize()),