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.

Sign up for GitHub

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

make Keyset Id's member variables public #564

Closed
wants to merge 1 commit into from
Closed

make Keyset Id's member variables public #564

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/cashu/src/nuts/nut02.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,13 @@ impl fmt::Display for KeySetVersion {
#[serde(into = "String", try_from = "String")]
#[cfg_attr(feature = "swagger", derive(utoipa::ToSchema), schema(as = String))]
pub struct Id {
version: KeySetVersion,
id: [u8; Self::BYTELEN],
pub version: KeySetVersion,
pub id: [u8; Self::BYTELEN],
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it is worth making it a trait or something with a default implementation, something more idiomatic and easy to extend.

Also, FYI, perhaps this should be part of the signatory, and the mintd just uses whatever is given to it by the signatory, as it may eventually run as its crate either in the same memory space or with an external service through grpc (#509) /cc @thesimplekid

Copy link
Contributor Author

@codingpeanut157 codingpeanut157 Feb 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

as the codebase currently is, I reckon, converting Id into a trait is unfeasible: this small struct is used in so many places that converting all of them into generic code over a parameter impl<ID> [...] where ID: Id would be a bloodbath.

Side note: if ( and I hope it will) the signatory and key management subsystem will become an external service communicating with the mint via grpc, protecting the internals of the keyset Id and how it is built is going to become even less relevant for the security level of the mint itself

Copy link
Collaborator

@thesimplekid thesimplekid Feb 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I do agree with codingpeanut that this would end up being a pretty big change. However, we are going to need to something to support keysetv2 whether that be making it a trait or an enum like we do with token, if I remember correctly I started trying to do token as a trait and ended up with an enum forget exactly why.

Also, FYI, perhaps this should be part of the signatory

We still need the type in cashu as we want this crate to be an implementation of all the cashu types and can be used without the other crates.

}

impl Id {
const STRLEN: usize = 14;
const BYTELEN: usize = 7;
pub const STRLEN: usize = 14;
pub const BYTELEN: usize = 7;

/// [`Id`] to bytes
pub fn to_bytes(&self) -> Vec<u8> {
Expand Down
Loading