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

Support tagged ZIP 32 child derivation #7

Merged
merged 4 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from 3 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
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ and this library adheres to Rust's notion of
[Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Added
- `zcash_spec::PrfExpand::REGISTERED_ZIP32_CHILD` (for tagged ZIP 32 child
derivation).
- `zcash_spec::PrfExpand::ADHOC_ZIP32_CHILD` (renamed and retyped per ZIP 32).
- `zcash_spec::VariableLengthSlice`

### Changed
- `zcash_spec::PrfExpand::ORCHARD_ZIP32_CHILD` now has type
`PrfExpand<([u8; 32], [u8; 4], Option<(u8, VariableLengthSlice)>)>` due to
ZIP 32 changes.

### Removed
- `zcash_spec::PrfExpand::ARBITRARY_ZIP32_CHILD` (use `ADHOC_ZIP32_CHILD`
instead).

## [0.1.2] - 2024-10-01
### Added
Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@
#![deny(rustdoc::broken_intra_doc_links)]

mod prf_expand;
pub use prf_expand::PrfExpand;
pub use prf_expand::{PrfExpand, VariableLengthSlice};
20 changes: 18 additions & 2 deletions src/prf_expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ impl<T> PrfExpand<T> {
}
}

/// Marker type used for `PrfExpand` instantiations that permit a variable-length input.
pub struct VariableLengthSlice;

macro_rules! with_inputs {
($($arr:ident, $arrlen:ident),*) => {
#[allow(unused_parens)]
Expand Down Expand Up @@ -94,8 +97,6 @@ with_inputs!(a, A);

impl PrfExpand<([u8; 32], [u8; 4])> {
pub const SPROUT_ZIP32_CHILD: Self = Self::new(0x80);
pub const ORCHARD_ZIP32_CHILD: Self = Self::new(0x81);
pub const ARBITRARY_ZIP32_CHILD: Self = Self::new(0xAB);
}
impl PrfExpand<([u8; 32], [u8; 32])> {
pub const ORCHARD_DK_OVK: Self = Self::new(0x82);
Expand All @@ -108,3 +109,18 @@ impl PrfExpand<([u8; 96], [u8; 32], [u8; 4])> {
pub const SAPLING_ZIP32_CHILD_NON_HARDENED: Self = Self::new(0x12);
}
with_inputs!(a, A, b, B, c, C);

impl PrfExpand<([u8; 32], [u8; 4], Option<(u8, VariableLengthSlice)>)> {
pub const ORCHARD_ZIP32_CHILD: Self = Self::new(0x81);
pub const ADHOC_ZIP32_CHILD: Self = Self::new(0xAB);
pub const REGISTERED_ZIP32_CHILD: Self = Self::new(0xAC);

/// Expands the given secret key in this domain.
pub fn with(self, sk: &[u8], a: &[u8; 32], b: &[u8; 4], c: Option<(u8, &[u8])>) -> [u8; 64] {
Copy link
Collaborator

@nuttycom nuttycom Feb 19, 2025

Choose a reason for hiding this comment

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

Is it intended that Some((0, &[])) be a valid input to this function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These APIs are not intended to exclude bad encodings in general. For example, many of the values are field elements for which non-canonical encodings are invalid. They're supposed to be low-level APIs taking already-encoded values that are assumed to be correct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question for Some((0, &[0xab, 0xcd]))

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. That being said, if we wanted to instead encode here that for these three specific PrfExpand domain separators that (0, &[]) is the canonical None then we could do that instead (in which case, usage in e.g orchard would pass that instead of None).

If we do this, then we should also remove the tuple and switch back to [u8; 1] in the API (so that future auto-derived arguments can be done correctly).

if let Some((d, e)) = c {
self.apply(sk, &[a, b, &[d], e])
} else {
self.apply(sk, &[a, b])
}
}
}
Loading