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

Replace ChillDKG seed with hostseckey #42

Closed
wants to merge 2 commits into from

Conversation

jonasnick
Copy link
Collaborator

WIP because the markdown is inconsistent and confusing. For example, the "host secret key" is also called "long-term secret key", "host secret key", "device secret key" or just "secret key".

Fixes #28

Copy link
Collaborator

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

Concept ACK for the reasons explained in #28.

perhaps the seed argument in encpedpop participant_step1 should then also be renamed to deckey (because that's what we pass anyway)

WIP because the markdown is inconsistent and confusing. For example, the "host
secret key" is also called "long-term secret key", "host secret key", "device
secret key" or just "secret key".

Urghs yeah. What about calling it "host secret key" always, but adding descriptive prefixes if necessary? See my inline suggestions.

python/chilldkg_ref/util.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -124,44 +125,34 @@ def certeq_coordinator_step(sigs: List[bytes]) -> bytes:
###


def hostseckey(seed: bytes) -> bytes:
return prf(seed, "chilldkg hostseckey")
def hostpubkey_gen(hostseckey: bytes) -> bytes:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def hostpubkey_gen(hostseckey: bytes) -> bytes:
def hostpubkey(hostseckey: bytes) -> bytes:

I like this a bit more.

  • I don't think the public key is "generated" at this point. It's rather that the entire key pair is generated when the hostseckey is created.
  • For consistency with the other names. We also don't use certeq_message_gen or params_id_gen.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The name was chosen to be consistent with BIP 340's def pubkey_gen(seckey: bytes) -> bytes. But I agree with your reasoning.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done. I renamed the local conflicting variables to hostpk.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I missed the local conflict. That's pretty ugly, to be honest.

Well, let me just merge this. Perhaps I'll suggest a more elegant variant later if I find one.

@jonasnick
Copy link
Collaborator Author

perhaps the seed argument in encpedpop participant_step1 should then also be renamed to deckey (because that's what we pass anyway)

seed seems to be more correct in the context of encpedpop because the argument is in principle unrelated to the deckey.

@real-or-random
Copy link
Collaborator

Forgot to push your changes? ;)

@real-or-random real-or-random changed the title WIP: Replace ChillDKG seed with hostseckey Replace ChillDKG seed with hostseckey Oct 1, 2024
@real-or-random real-or-random marked this pull request as ready for review October 1, 2024 13:43
@real-or-random
Copy link
Collaborator

I reverted the naming change. (I think I had a point there, but I really dislike the naming conflict with the local variables...)

Manually pushed to master here as 4617da4

real-or-random added a commit to real-or-random/bip-frost-dkg that referenced this pull request Oct 1, 2024
ChillDKG anyway passes the hostseckey for both, so this was potentially
confusing to readers of the code. This change is in line with BlockstreamResearch#42.
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.

Should we make the hostseckey the cryptographic seed?
2 participants