Skip to content

feat(crypto): CRP-2699 adapt vetKD API to use derivation domain blob instead of path#4049

Merged
fspreiss merged 15 commits into
masterfrom
franzstefan/CRP-2699-vetkd-derivation-domain-instead-of-path
Feb 26, 2025
Merged

feat(crypto): CRP-2699 adapt vetKD API to use derivation domain blob instead of path#4049
fspreiss merged 15 commits into
masterfrom
franzstefan/CRP-2699-vetkd-derivation-domain-instead-of-path

Conversation

@fspreiss

@fspreiss fspreiss commented Feb 21, 2025

Copy link
Copy Markdown
Contributor

Changes the hierarchical vetKD derivation_path: vec blob to a non-hierarchical derivation_domain: blob because the vetKD public key derivation algorithm is actually non-hierarchical. This simplifies the API. Callers can still embed hierarchical structure into the derivation domain blob if they wish.

The respective change in the IC spec PR is dfinity/portal@3bbb8f4.

The name derivation domain is not set in stone and is up for discussion. One possible alternative is derivation context, but there may be others.

Note that the SignWithThresholdContext currently contains a field SignWithThresholdContext::derivation_path: Vec<Vec<u8>> that currently applies no matter if the context relates to ECDSA, Schnorr, or VetKD (based on the SignWithThresholdContext:args: ThresholdArguments). Now that the VetKD variant uses just a blob instead of a vec of blobs, the derivation path is flattened to form the derivation domain. In a later step, we may consider moving SignWithThresholdContext::derivation_path into {Ecdsa|Schnorr}Arguments and add VetKdArguments::derivation_domain, given that not all three arguments don't use the hierarchical derivation path any more. This, however, is not strictly necessary and rather a cleanup action, which would require a multi-step migration because the SignWithThresholdContext is persisted in the state.

@github-actions github-actions Bot added the feat label Feb 21, 2025
@fspreiss fspreiss marked this pull request as ready for review February 21, 2025 15:19
@fspreiss fspreiss requested a review from a team February 21, 2025 15:20
@fspreiss fspreiss requested review from a team as code owners February 21, 2025 15:20

@maksymar maksymar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM execution part

Comment thread rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs Outdated

@randombit randombit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some questions regarding the derivation itself, otherwise lgtm

Comment thread rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs Outdated
Comment thread rs/crypto/internal/crypto_lib/bls12_381/vetkd/src/lib.rs
@fspreiss fspreiss added this pull request to the merge queue Feb 26, 2025
Merged via the queue into master with commit bf87411 Feb 26, 2025
@fspreiss fspreiss deleted the franzstefan/CRP-2699-vetkd-derivation-domain-instead-of-path branch February 26, 2025 10:05
@fspreiss

Copy link
Copy Markdown
Contributor Author

Created CRP-2709 for the potential cleanup of moving of SignWithThresholdContext::derivation_path into {Ecdsa|Schnorr}Arguments and the addition of VetKdArguments::derivation_domain

github-merge-queue Bot pushed a commit that referenced this pull request Mar 13, 2025
Performs the following renamings in the vetKD API in accordance with the
[latest
changes](dfinity/portal@add1c71)
in the [spec PR](dfinity/portal#3763):
* `derivation_id` --> `input`
* The name `derivation_id` often caused confusion and `input` is a more
standard name in the context of key derivation schemes.
* `derivation_domain`
([previously](#4049)
`derivation_path`) --> `context`
* The main use case for the derivation domain/path is to do domain
separation, i.e., to specify the context in which the derived keys are
to be used. Given this, directly calling it context seems beneficial in
that it makes the meaning of the field more clear and intuitive, and
thus the API easier to use.
* `vetkd_derive_encrypted_key` --> `vetkd_derive_key`
* Although the fact that the returned key is encrypted is relevant in
that it ensures that nodes cannot see the key in clear text, this can be
considered an implementation detail. Also, the name
`vetkd_derive_encrypted_key` is somewhat long. In any case, in the
returned struct the (single) field is still called `encrypted_key`, so
it is still explicit that the returned key is encrypted.
* `encryption_public_key` --> `transport_public_key`
* Everyone everywhere (in publications, slides, demos, etc.) called this
"transport public key". The reason this was not called
transport_public_key in the API so far was because the containing API
method was called `vetkd_derive_encrypted_key` and the name
`encryption_public_key` should have made it clear that it is this very
public key under which the _encrypted key_ is encrypted. Now that we are
removing the part `encrypted_` from the API name, this reason is
obsolete and we are free to call it `transport_public_key`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants