Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zcash_keys: Add decode_extfvk_with_network #1523

Merged
merged 3 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions zcash_keys/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ and this library adheres to Rust's notion of

## [Unreleased]

### Added
- `zcash_keys::encoding::decode_extfvk_with_network`
- `impl std::error::Error for Bech32DecodeError`
- `impl std::error::Error for DecodingError`
- `impl std::error::Error for DerivationError`

## [0.3.0] - 2024-08-19
### Notable changes
- `zcash_keys`:
Expand Down
50 changes: 46 additions & 4 deletions zcash_keys/src/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use crate::address::UnifiedAddress;
use bs58::{self, decode::Error as Bs58Error};
use std::fmt;
use zcash_primitives::consensus::NetworkConstants;
use zcash_primitives::consensus::{NetworkConstants, NetworkType};

use zcash_address::unified::{self, Encoding};
use zcash_primitives::{consensus, legacy::TransparentAddress};
Expand Down Expand Up @@ -66,6 +66,16 @@
}
}

#[cfg(feature = "sapling")]
impl std::error::Error for Bech32DecodeError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match &self {
Bech32DecodeError::Bech32Error(e) => Some(e),
_ => None,

Check warning on line 74 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L71-L74

Added lines #L71 - L74 were not covered by tests
}
}
}

#[cfg(feature = "sapling")]
fn bech32_decode<T, F>(hrp: &str, s: &str, read: F) -> Result<T, Bech32DecodeError>
where
Expand Down Expand Up @@ -245,9 +255,8 @@
bech32_encode(hrp, |w| extfvk.write(w))
}

/// Decodes an [`ExtendedFullViewingKey`] from a Bech32-encoded string.
///
/// [`ExtendedFullViewingKey`]: sapling::zip32::ExtendedFullViewingKey
/// Decodes an [`ExtendedFullViewingKey`] from a Bech32-encoded string, verifying that it matches
/// the provided human-readable prefix.
#[cfg(feature = "sapling")]
pub fn decode_extended_full_viewing_key(
hrp: &str,
Expand All @@ -256,6 +265,39 @@
bech32_decode(hrp, s, |data| ExtendedFullViewingKey::read(&data[..]).ok())
}

/// Decodes an [`ExtendedFullViewingKey`] and the [`NetworkType`] that it is intended for use with
/// from a Bech32-encoded string.
#[cfg(feature = "sapling")]
pub fn decode_extfvk_with_network(

Check warning on line 271 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L271

Added line #L271 was not covered by tests
Copy link
Contributor

@daira daira Aug 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know the other functions in this file don't explicitly say in their names that they're for Sapling, but that still seems odd. Non-blocking; I don't think we can do anything about it right now, but we should consider moving them to a zcash_keys::sapling::encoding module if they are never going to be more general.

s: &str,
) -> Result<(NetworkType, ExtendedFullViewingKey), Bech32DecodeError> {
use zcash_protocol::constants::{mainnet, regtest, testnet};

let (decoded_hrp, data, variant) = bech32::decode(s)?;
if variant != Variant::Bech32 {
Err(Bech32DecodeError::IncorrectVariant(variant))

Check warning on line 278 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L276-L278

Added lines #L276 - L278 were not covered by tests
} else {
Comment on lines +276 to +279
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using CheckedHrpstring::new::<Bech32> (although I'm not sure it really comes out any cleaner once error handling has been taken into account).

let network = match &decoded_hrp[..] {
mainnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY => Ok(NetworkType::Main),
testnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY => Ok(NetworkType::Test),
regtest::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY => Ok(NetworkType::Regtest),
other => Err(Bech32DecodeError::HrpMismatch {
expected: format!(
"One of {}, {}, or {}",
mainnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY,
testnet::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY,
regtest::HRP_SAPLING_EXTENDED_FULL_VIEWING_KEY,

Check warning on line 289 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L280-L289

Added lines #L280 - L289 were not covered by tests
),
actual: other.to_string(),

Check warning on line 291 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L291

Added line #L291 was not covered by tests
}),
}?;
let fvk = ExtendedFullViewingKey::read(&Vec::<u8>::from_base32(&data)?[..])
.map_err(|_| Bech32DecodeError::ReadError)?;

Check warning on line 295 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L294-L295

Added lines #L294 - L295 were not covered by tests

Ok((network, fvk))

Check warning on line 297 in zcash_keys/src/encoding.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/encoding.rs#L297

Added line #L297 was not covered by tests
}
}

/// Writes a [`PaymentAddress`] as a Bech32-encoded string.
///
/// # Examples
Expand Down
25 changes: 25 additions & 0 deletions zcash_keys/src/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
#[cfg(feature = "orchard")]
use orchard::{self, keys::Scope};

#[cfg(all(feature = "sapling", feature = "unstable"))]
use ::sapling::zip32::ExtendedFullViewingKey;

#[cfg(feature = "sapling")]
pub mod sapling {
pub use sapling::zip32::{
Expand Down Expand Up @@ -111,6 +114,8 @@
}
}

impl std::error::Error for DerivationError {}

/// A version identifier for the encoding of unified spending keys.
///
/// Each era corresponds to a range of block heights. During an era, the unified spending key
Expand Down Expand Up @@ -176,6 +181,8 @@
}
}

impl std::error::Error for DecodingError {}

#[cfg(feature = "unstable")]
impl Era {
/// Returns the unique identifier for the era.
Expand Down Expand Up @@ -674,6 +681,24 @@
vec![],
)
}

#[cfg(all(feature = "sapling", feature = "unstable"))]
pub fn from_sapling_extended_full_viewing_key(

Check warning on line 686 in zcash_keys/src/keys.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/keys.rs#L686

Added line #L686 was not covered by tests
sapling: ExtendedFullViewingKey,
) -> Result<UnifiedFullViewingKey, DerivationError> {
Self::from_checked_parts(
#[cfg(feature = "transparent-inputs")]
None,
#[cfg(feature = "sapling")]
Some(sapling.to_diversifiable_full_viewing_key()),
#[cfg(feature = "orchard")]
None,

Check warning on line 695 in zcash_keys/src/keys.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/keys.rs#L690-L695

Added lines #L690 - L695 were not covered by tests
// We don't currently allow constructing new UFVKs with unknown items, but we store
// this to allow parsing such UFVKs.
vec![],

Check warning on line 698 in zcash_keys/src/keys.rs

View check run for this annotation

Codecov / codecov/patch

zcash_keys/src/keys.rs#L698

Added line #L698 was not covered by tests
)
}

/// Construct a UFVK from its constituent parts, after verifying that UIVK derivation can
/// succeed.
fn from_checked_parts(
Expand Down
Loading