Skip to content

Conversation

@a1denvalu3
Copy link
Contributor

@a1denvalu3 a1denvalu3 commented Apr 2, 2025

Description

Adding support for keysets v2.

CHANGED

  • Replaced From trait implementation for Id with functions v1_from_keys, v2_from_data.
  • struct Id, id field:
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum IdBytes {
    V1([u8; 7]),
    V2([u8; 31]),
}

pub struct Id {
    version: KeySetVersion,
    id: IdBytes,
}
  • keyset generation functions (generate, generate_from_xpriv, create_new_keyset) take 2 additional arguments: an optional final_expiry and a version to identify the correct version.
  • creating new keysets defaults to KeySetVersion::Version01 (creating new keysets defaults to the newer one).

TODO

  • ShortIdV2 for short IDs in tokens
  • Fix database interactions.

Checklist

@a1denvalu3 a1denvalu3 marked this pull request as draft April 2, 2025 12:58
@a1denvalu3 a1denvalu3 mentioned this pull request Apr 2, 2025
5 tasks
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.

Looking good.

Some tests would be nice as well, converting to and from byte arrays of different lengths.

@a1denvalu3
Copy link
Contributor Author

a1denvalu3 commented Apr 3, 2025

So @ok300 @thesimplekid I thought we could create a new ShortId enum which will be the type of the id field for TokenV4Token. In the case of v1, it will just be the keyset ID itself, while in the case of v2 it will be the first 8 bytes of IDv2.

@thesimplekid
Copy link
Collaborator

So @ok300 @thesimplekid I thought we could create a new ShortId enum which will be the type of the id field for TokenV4Token. In the case of v1, it will just be the keyset ID itself, while in the case of v2 it will be the first 8 bytes of IDv2.

Yes I think that makes sense. It's similar to how we handled the transition from TokenV3 to TokenV4.

@ok300
Copy link
Contributor

ok300 commented Apr 3, 2025

Sounds good. Maybe ShortKeysetId is clearer.

@thesimplekid thesimplekid requested a review from crodas April 23, 2025 07:39
@thesimplekid
Copy link
Collaborator

merging main should fix the current CI issues, I updated the stable rust version.

@a1denvalu3
Copy link
Contributor Author

I think this is good.

@thesimplekid thesimplekid added this to the v0.11 milestone Jun 18, 2025
@a1denvalu3
Copy link
Contributor Author

Do we want to merge this?

@thesimplekid thesimplekid merged commit c61fd38 into cashubtc:main Jun 19, 2025
108 checks passed
@github-project-automation github-project-automation bot moved this from Needs Review to Done in CDK Jun 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants