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

Api updates corresponding to post-0.2 rust-umbral PRs #272

Merged
merged 6 commits into from
Aug 19, 2021

Conversation

fjarri
Copy link
Contributor

@fjarri fjarri commented Jul 19, 2021

Already merged rust-umbral PRs:

sk = skf.secret_key_by_label(label)
return bytes(skf), bytes(sk)
derived_skf = skf.secret_key_factory_by_label(skf_label)
sk = derived_skf.secret_key_by_label(key_label)
Copy link
Member

Choose a reason for hiding this comment

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

Should it be by_label or from_label?

Choose a reason for hiding this comment

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

That would also be my guess, but I think from prefix is widely accepted as a naming convention for From<> trait implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"by_label" is a leftover from PyUmbral 0.1. from is indeed used generally for constructors.

Come to think of it, perhaps make_secret_key()/make_secret_key_factory() (or derive_, borrowing from PyUmbral 0.1) would be a better name for these methods. by_label in my mind implies that there is some lookup going on and not creation.

@@ -53,7 +50,7 @@ def serialized_size(cls):
def _from_exact_bytes(cls, data: bytes):
return cls(CurveScalar._from_exact_bytes(data))

def __bytes__(self) -> bytes:
def to_secret_bytes(self) -> bytes:
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -125,6 +145,14 @@ def secret_key_by_label(self, label: bytes) -> SecretKey:

Copy link
Member

@cygnusv cygnusv Jul 20, 2021

Choose a reason for hiding this comment

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

Not related to this PR but we discussed this extra hoop before in discord (https://discord.com/channels/411401661714792449/411401661714792451/860193967923527730)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I remember - see my concerns in nucypher/rust-umbral#64

@fjarri
Copy link
Contributor Author

fjarri commented Aug 19, 2021

The discussion about hashing to scalars is happening in nucypher/rust-umbral#35 . The rest is ok to mere since nucypher/rust-umbral#64 is merged.

@fjarri fjarri merged commit 5c9fb53 into nucypher:master Aug 19, 2021
@fjarri fjarri deleted the api-updates branch August 19, 2021 16:07
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.

3 participants