diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 7f7fd13e2..f8dedbe63 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -37,6 +37,7 @@ and this library adheres to Rust's notion of - `TransactionRequest::new` for constructing a request from `Vec`. - `TransactionRequest::payments` for accessing the `Payments` that make up a request. +- `zcash_client_backend::encoding::KeyError` - New experimental APIs that should be considered unstable, and are likely to be modified and/or moved to a different module in a future release: @@ -113,7 +114,11 @@ and this library adheres to Rust's notion of derived from ZIP 316 UFVKs and UIVKs. - `welding_rig::scan_block` now uses batching for trial-decryption of transaction outputs. - +- The return type of the following methods in `zcash_client_backend::encoding` + have been changed to improve error reporting: + - `decode_extended_spending_key` + - `decode_extended_full_viewing_key` + - `decode_payment_address` ### Removed - `zcash_client_backend::data_api`: diff --git a/zcash_client_backend/examples/diversify-address.rs b/zcash_client_backend/examples/diversify-address.rs index de20a456e..1071566c9 100644 --- a/zcash_client_backend/examples/diversify-address.rs +++ b/zcash_client_backend/examples/diversify-address.rs @@ -9,16 +9,12 @@ use zcash_primitives::{ fn parse_viewing_key(s: &str) -> Result<(ExtendedFullViewingKey, bool), &'static str> { decode_extended_full_viewing_key(mainnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY, s) - .ok() - .flatten() .map(|vk| (vk, true)) - .or_else(|| { + .or_else(|_| { decode_extended_full_viewing_key(testnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY, s) - .ok() - .flatten() .map(|vk| (vk, false)) }) - .ok_or("Invalid Sapling viewing key") + .map_err(|_| "Invalid Sapling viewing key") } fn parse_diversifier_index(s: &str) -> Result { diff --git a/zcash_client_backend/src/encoding.rs b/zcash_client_backend/src/encoding.rs index 1bebbf5cf..c36e8c778 100644 --- a/zcash_client_backend/src/encoding.rs +++ b/zcash_client_backend/src/encoding.rs @@ -26,15 +26,55 @@ where bech32::encode(hrp, data.to_base32(), Variant::Bech32).expect("hrp is invalid") } -fn bech32_decode(hrp: &str, s: &str, read: F) -> Result, Error> +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Bech32DecodeError { + Bech32Error(Error), + IncorrectVariant(Variant), + ReadError, + HrpMismatch { expected: String, actual: String }, +} + +impl From for Bech32DecodeError { + fn from(err: Error) -> Self { + Bech32DecodeError::Bech32Error(err) + } +} + +impl fmt::Display for Bech32DecodeError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match &self { + Bech32DecodeError::Bech32Error(e) => write!(f, "{}", e), + Bech32DecodeError::IncorrectVariant(variant) => write!( + f, + "Incorrect bech32 encoding (wrong variant: {:?})", + variant + ), + Bech32DecodeError::ReadError => { + write!(f, "Failed to decode key from its binary representation.") + } + Bech32DecodeError::HrpMismatch { expected, actual } => write!( + f, + "Key was encoded for a different network: expected {}, got {}.", + expected, actual + ), + } + } +} + +fn bech32_decode(hrp: &str, s: &str, read: F) -> Result where F: Fn(Vec) -> Option, { - match bech32::decode(s)? { - (decoded_hrp, data, Variant::Bech32) if decoded_hrp == hrp => { - Vec::::from_base32(&data).map(read) - } - _ => Ok(None), + let (decoded_hrp, data, variant) = bech32::decode(s)?; + if variant != Variant::Bech32 { + Err(Bech32DecodeError::IncorrectVariant(variant)) + } else if decoded_hrp != hrp { + Err(Bech32DecodeError::HrpMismatch { + expected: hrp.to_string(), + actual: decoded_hrp, + }) + } else { + read(Vec::::from_base32(&data)?).ok_or(Bech32DecodeError::ReadError) } } @@ -121,7 +161,7 @@ pub fn encode_extended_spending_key(hrp: &str, extsk: &ExtendedSpendingKey) -> S pub fn decode_extended_spending_key( hrp: &str, s: &str, -) -> Result, Error> { +) -> Result { bech32_decode(hrp, s, |data| ExtendedSpendingKey::read(&data[..]).ok()) } @@ -155,7 +195,7 @@ pub fn encode_extended_full_viewing_key(hrp: &str, extfvk: &ExtendedFullViewingK pub fn decode_extended_full_viewing_key( hrp: &str, s: &str, -) -> Result, Error> { +) -> Result { bech32_decode(hrp, s, |data| ExtendedFullViewingKey::read(&data[..]).ok()) } @@ -240,11 +280,11 @@ pub fn encode_payment_address_p( /// HRP_SAPLING_PAYMENT_ADDRESS, /// "ztestsapling1qqqqqqqqqqqqqqqqqqcguyvaw2vjk4sdyeg0lc970u659lvhqq7t0np6hlup5lusxle75ss7jnk", /// ), -/// Ok(Some(pa)), +/// Ok(pa), /// ); /// ``` /// [`PaymentAddress`]: zcash_primitives::sapling::PaymentAddress -pub fn decode_payment_address(hrp: &str, s: &str) -> Result, Error> { +pub fn decode_payment_address(hrp: &str, s: &str) -> Result { bech32_decode(hrp, s, |data| { if data.len() != 43 { return None; @@ -392,6 +432,7 @@ mod tests { use super::{ decode_extended_full_viewing_key, decode_extended_spending_key, decode_payment_address, encode_extended_full_viewing_key, encode_extended_spending_key, encode_payment_address, + Bech32DecodeError, }; #[test] @@ -414,7 +455,7 @@ mod tests { encoded_main ) .unwrap(), - Some(extsk.clone()) + extsk ); assert_eq!( @@ -430,7 +471,7 @@ mod tests { encoded_test ) .unwrap(), - Some(extsk) + extsk ); } @@ -454,7 +495,7 @@ mod tests { encoded_main ) .unwrap(), - Some(extfvk.clone()) + extfvk ); assert_eq!( @@ -470,7 +511,7 @@ mod tests { encoded_test ) .unwrap(), - Some(extfvk) + extfvk ); } @@ -500,7 +541,7 @@ mod tests { encoded_main ) .unwrap(), - Some(addr.clone()) + addr ); assert_eq!( @@ -513,7 +554,7 @@ mod tests { encoded_test ) .unwrap(), - Some(addr) + addr ); } @@ -535,9 +576,8 @@ mod tests { decode_payment_address( constants::mainnet::HRP_SAPLING_PAYMENT_ADDRESS, &encoded_main - ) - .unwrap(), - None + ), + Err(Bech32DecodeError::ReadError) ); } } diff --git a/zcash_client_backend/src/zip321.rs b/zcash_client_backend/src/zip321.rs index 7d78b17f0..75f5a0374 100644 --- a/zcash_client_backend/src/zip321.rs +++ b/zcash_client_backend/src/zip321.rs @@ -793,7 +793,7 @@ mod tests { let expected = TransactionRequest { payments: vec![ Payment { - recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap().unwrap()), + recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()), amount: Amount::from_u64(376876902796286).unwrap(), memo: None, label: None, @@ -811,7 +811,7 @@ mod tests { let req = TransactionRequest { payments: vec![ Payment { - recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap().unwrap()), + recipient_address: RecipientAddress::Shielded(decode_payment_address(TEST_NETWORK.hrp_sapling_payment_address(), "ztestsapling1n65uaftvs2g7075q2x2a04shfk066u3lldzxsrprfrqtzxnhc9ps73v4lhx4l9yfxj46sl0q90k").unwrap()), amount: Amount::from_u64(0).unwrap(), memo: None, label: None, diff --git a/zcash_client_sqlite/src/error.rs b/zcash_client_sqlite/src/error.rs index 33e21c2bf..a45a66a08 100644 --- a/zcash_client_sqlite/src/error.rs +++ b/zcash_client_sqlite/src/error.rs @@ -3,7 +3,10 @@ use std::error; use std::fmt; -use zcash_client_backend::{data_api, encoding::TransparentCodecError}; +use zcash_client_backend::{ + data_api, + encoding::{Bech32DecodeError, TransparentCodecError}, +}; use zcash_primitives::consensus::BlockHeight; use crate::{NoteId, PRUNING_HEIGHT}; @@ -27,8 +30,8 @@ pub enum SqliteClientError { /// Illegal attempt to reinitialize an already-initialized wallet database. TableNotEmpty, - /// Bech32 decoding error - Bech32(bech32::Error), + /// A Bech32-encoded key or address decoding error + Bech32DecodeError(Bech32DecodeError), /// Base58 decoding error Base58(bs58::decode::Error), @@ -58,7 +61,7 @@ impl error::Error for SqliteClientError { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match &self { SqliteClientError::InvalidMemo(e) => Some(e), - SqliteClientError::Bech32(e) => Some(e), + SqliteClientError::Bech32DecodeError(Bech32DecodeError::Bech32Error(e)) => Some(e), SqliteClientError::DbError(e) => Some(e), SqliteClientError::Io(e) => Some(e), _ => None, @@ -78,7 +81,7 @@ impl fmt::Display for SqliteClientError { write!(f, "The note ID associated with an inserted witness must correspond to a received note."), SqliteClientError::RequestedRewindInvalid(h, r) => write!(f, "A rewind must be either of less than {} blocks, or at least back to block {} for your wallet; the requested height was {}.", PRUNING_HEIGHT, h, r), - SqliteClientError::Bech32(e) => write!(f, "{}", e), + SqliteClientError::Bech32DecodeError(e) => write!(f, "{}", e), SqliteClientError::Base58(e) => write!(f, "{}", e), SqliteClientError::TransparentAddress(e) => write!(f, "{}", e), SqliteClientError::TableNotEmpty => write!(f, "Table is not empty"), @@ -102,9 +105,9 @@ impl From for SqliteClientError { } } -impl From for SqliteClientError { - fn from(e: bech32::Error) -> Self { - SqliteClientError::Bech32(e) +impl From for SqliteClientError { + fn from(e: Bech32DecodeError) -> Self { + SqliteClientError::Bech32DecodeError(e) } } diff --git a/zcash_client_sqlite/src/wallet/init.rs b/zcash_client_sqlite/src/wallet/init.rs index a4235e1f7..18772a0dd 100644 --- a/zcash_client_sqlite/src/wallet/init.rs +++ b/zcash_client_sqlite/src/wallet/init.rs @@ -197,13 +197,7 @@ impl RusqliteMigration for WalletMigration2

{ for row in rows { let (account, address) = row?; let decoded_address = - decode_payment_address(self.params.hrp_sapling_payment_address(), &address)? - .ok_or_else(|| { - SqliteClientError::CorruptedData(format!( - "Not a valid Sapling payment address: {}", - address - )) - })?; + decode_payment_address(self.params.hrp_sapling_payment_address(), &address)?; let usk = UnifiedSpendingKey::from_seed(&self.params, self.seed.expose_secret(), account)