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

Jump to bottom

feat: add legacy v2-signed keys #125

Merged
merged 4 commits into from
Jan 11, 2024
Merged

feat: add legacy v2-signed keys #125

merged 4 commits into from
Jan 11, 2024

Conversation

richardhuaaa
Copy link
Contributor

Adds the option for v3 keys to be signed by v2 keys, to avoid additional wallet signatures on clients that already have a signed v2 key.

@richardhuaaa richardhuaaa requested a review from neekolas January 11, 2024 12:36
@richardhuaaa richardhuaaa requested a review from a team as a code owner January 11, 2024 12:36
richardhuaaa
richardhuaaa commented Jan 11, 2024
View reviewed changes
proto/mls/message_contents/association.proto Outdated
// 'CreateIdentity' wallet signature
message LegacyCreateIdentityAssociation {
RecoverableEcdsaSignature signature = 1;
SignedPublicKey legacy_create_identity_key = 2;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The creation time and account address can be derived from the nested SignedPublicKey proto

neekolas
neekolas approved these changes Jan 11, 2024
View reviewed changes
Copy link
Collaborator

@neekolas neekolas left a comment

Choose a reason for hiding this comment

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

Looks good to me

@richardhuaaa
Copy link
Contributor Author

@neekolas, I added 059ce56, because I think the revocation must apply to the unsigned legacy identity key bytes, not a proto serialization of the signed key bytes. This is because the proto serialization could be different between implementations. Will land, but let me know if you have feedback or tweaks and I can put up another PR!

@richardhuaaa richardhuaaa merged commit 2769931 into main Jan 11, 2024
8 checks passed
@richardhuaaa richardhuaaa deleted the rich/legacy-create-identity-association branch January 11, 2024 23:54
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 3.37.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@neekolas
Copy link
Collaborator

because I think the revocation must apply to the unsigned legacy identity key bytes, not a proto serialization of the signed key bytes. This is because the proto serialization could be different between implementations. Will land, but let me know if you have feedback or tweaks and I can put up another PR!

That's good thinking. We don't want to rely on a byte-perfect proto serialization anywhere.

@neekolas neekolas mentioned this pull request Jan 12, 2024
Merged
@richardhuaaa richardhuaaa mentioned this pull request Jan 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neekolas neekolas neekolas approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@richardhuaaa @neekolas