Allow SpendValidatingKey to be constructed from bytes #428
Conversation
There was a problem hiding this comment.
I'm also interested this since we also need this for our FROST demo signing tool (we are currently using Ywallet code that uses modified versions of the crates so we just edited them to support this, but we're in the process of replacing that code).
I left a couple of random comments while I was taking a look, but it's probably better to wait for the maintainers to chime in first before changing anything.
|
I'm fine with adding this kind of API behind an unstable cfg option, but I think that this API is not what we will want in the final production release, because I suspect we will not want people to pass around To figure out the production API, we need to first finish the |
Yes this is exactly what we don't want for non-FROST users. @str4d, What about adding the visibility crate to handle the API changes as @conradoplg suggested? We'd be adding |
src/keys.rs
Outdated
|
|
||
| /// Attempts to convert these bytes into a spend validating key | ||
| /// from its serialized form, I2LEOSP_256(ak). Returns None if | ||
| /// it can't be created. | ||
| #[cfg(feature = "frost")] | ||
| pub fn with_bytes(bytes: &[u8]) -> Option<Self> { | ||
| SpendValidatingKey::from_bytes(bytes) | ||
| } |
There was a problem hiding this comment.
This isn't needed if #[cfg_attr(feature = "frost", visibility::make(pub))] is used. In any case the method that is conditionally exposed should be called from_bytes.
There was a problem hiding this comment.
Cool I'll get visibility crate in then. Thank you!
closes zcash#427 This adds a FROST feature flag so that public interface of the crate is not altered in its meaning for non-FROST applications. Key derivation is intended to be done on a specific way for usual applications. Interface changes introduce by FROST can pose a risk for the rest of the use cases that don't involve the assumptions of the FROST signature scheme protocol. we don't want non-FROST cases to make use the helper functions FROST needs to derive the needed key elements. PR suggestions: Make "frost" feature be under the "unstable" set of features correct the unstable feature definition use visibility crate
I opened zcash/zips#883 as that separate PR. @conradoplg @pacu let's figure out and specify the FROST-Orchard key tree interaction there. |
Co-authored-by: Jack Grigg <[email protected]> Co-authored-by: Daira-Emma Hopwood <[email protected]>
Just to be very clear here, the FROST book is not normative, except as it is incorporated by reference into the protocol spec or a ZIP (which to my knowledge it is currently not). It is very informative, but any FROST logic we put into the public crates needs to be tied to a normative reference. In this case, that is ZIP 312, so anything we do needs to be specified there (i.e. in the PR I linked above). |
According to the FROST book the ASK is split using FROST and its verifying key is the ak that should be used to generate the Orchard Full Viewing Key.
Allow SpendValidatingKey to be constructed from bytes
closes #427
As we agreed with @daira at Z|ECC, this adds a FROST feature flag so that public interface of the
crate is not altered in its meaning for non-FROST applications.
Key derivation is intended to be done on a specific way for usual
applications. Interface changes introduce by FROST can pose a risk
for the rest of the use cases that don't involve the assumptions
of the FROST signature scheme protocol. we don't want non-FROST
cases to make use the helper functions FROST needs to derive the
needed key elements.
There are other changes pending to make FROST work with Orchard.
Those changes will also be under this same feature flag