Skip to content

Fix issuance key derivation#74

Merged
ConstanceBeguier merged 9 commits intozsa1from
issue_keys_update
Jun 20, 2023
Merged

Fix issuance key derivation#74
ConstanceBeguier merged 9 commits intozsa1from
issue_keys_update

Conversation

@ConstanceBeguier
Copy link
Collaborator

@ConstanceBeguier ConstanceBeguier commented Jun 15, 2023

Updated constants for master (extended) issuance key according to ZIP 227. Previously, we used the same personalization for the master extended spending key and the master extended issuance key, as well as the same purpose constant for the spending master key and the issuance master key.

Now, the following updates have been made:

  • Personalization for the master extended issuance key: ZIP32ZSAIssue_V1
  • Purpose constant for the issuance master key: 227"

@QED-it QED-it deleted a comment from what-the-diff bot Jun 15, 2023
Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

one change is required to make it perfect.

Also,

  • please fix comment on line 190: /// Converts this issuance validating key to its serialized form
    to "spend key"

src/keys.rs Outdated
pub fn from_bytes(sk_iss: [u8; 32]) -> CtOption<Self> {
CtOption::new(
IssuanceKey(sk_iss),
SpendingKey::from_bytes(sk_iss).is_some(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need this check.

Copy link
Collaborator

@PaulLaux PaulLaux Jun 20, 2023

Choose a reason for hiding this comment

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

what we need is to make sure we can derive IssuanceValidatingKey, IssuanceAuthorizingKey potentially by calling

impl From<&IssuanceKey> for IssuanceAuthorizingKey {
    fn from(sk_iss: &IssuanceKey) -> Self {
        let isk = to_scalar(PrfExpand::ZsaIsk.expand(&sk_iss.0));
        // IssuanceAuthorizingKey cannot be constructed such that this assertion would fail.
        assert!(!bool::from(isk.is_zero()));
        IssuanceAuthorizingKey(conditionally_negate(isk))
    }
}

/// and...

IssuanceValidatingKey {

    /// Constructs an Orchard issuance validating key from uniformly-random bytes.
    ///
    /// Returns `None` if the bytes do not correspond to a valid key.
    pub fn from_bytes(bytes: &[u8]) -> Option<Self> {
        <[u8; 32]>::try_from(bytes)
            .ok()
            .and_then(check_structural_validity)
            .map(IssuanceValidatingKey)
    }
}

Copy link
Collaborator Author

@ConstanceBeguier ConstanceBeguier Jun 20, 2023

Choose a reason for hiding this comment

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

I updated the check for IssuanceAuthorizingKey.
I think deriving IssuanceValidatingKey from IssuanceAuthorizingKey never fails (=> no check required for this part)

Copy link
Collaborator

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

Approved with one comment.

@ConstanceBeguier ConstanceBeguier merged commit aa1d895 into zsa1 Jun 20, 2023
@PaulLaux PaulLaux mentioned this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants