Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

make Keyset Id's member variables public #564

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

codingpeanut157
Copy link

Description

Aligns with other structures by exposing Id's member variables, allowing developers to create Ids that don't follow the CDK protocol. [1]

[1] https://github.com/cashubtc/nuts/blob/main/02.md#deriving-the-keyset-id

Aligns with other structures by exposing Id's member variables,
allowing developers to create Ids that don't follow the CDK protocol. [1]

[1] https://github.com/cashubtc/nuts/blob/main/02.md#deriving-the-keyset-id
Copy link
Contributor

@ok300 ok300 left a comment

Choose a reason for hiding this comment

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

NACK.

This would allow callers to bypass all the From implementations for Id ( From<&Keys>, FromStr, from_bytes etc) when creating Ids, which is unnecessary and dangerous.

@codingpeanut157
Copy link
Author

codingpeanut157 commented Jan 28, 2025

NACK.

This would allow callers to bypass all the From implementations for Id ( From<&Keys>, FromStr, from_bytes etc) when creating Ids, which is unnecessary and dangerous.

That was the point of this PR: allow others to implement their own keyset Id generator algorithms.
Regarding the potential danger of exposing internals of Id: I don't see any extra danger, as you expose from_bytes method which substantially allows users to put anything that starts with Version00 in the Id.
Moreover, exposing internals of other structures that use Id like MintKeysInfo or MintKeyset do not mitigate that same danger.
And besides that, exposing internals of structs like Proof or MintKeyPair is equally dangerous IMHO.
So why not exposing Id and opening up cdk to custom implementations of eCash protocol...

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants