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

refactor!: make verifyDidSignature accept Uint8Array #666

Merged
merged 6 commits into from
Oct 19, 2022
Merged

refactor!: make verifyDidSignature accept Uint8Array #666

merged 6 commits into from
Oct 19, 2022

Conversation

rflechtner
Copy link
Contributor

@rflechtner rflechtner commented Oct 18, 2022 •
edited
Loading

fixes KILTProtocol/ticket#2236

As per the ticket, this changes verifyDidSignature such that it takes signatures as a byte array instead of a hex string.
It also removes the makeDidSignature helper.

How to test:

Unit tests have been adapted.

Checklist:

  • I have verified that the code works
  • I have verified that the code is easy to understand
    • If not, I have left a well-balanced amount of inline comments
  • I have left the code in a better state
  • I have documented the changes (where applicable)

@rflechtner rflechtner requested a review from arty-name October 18, 2022 11:19
@rflechtner rflechtner self-assigned this Oct 18, 2022
@rflechtner rflechtner requested a review from ntn-x2 October 18, 2022 11:20
@rflechtner
Copy link
Contributor Author

@arty-name is that along the lines of what you had in mind?

ntn-x2
ntn-x2 reviewed Oct 18, 2022
View reviewed changes
Copy link
Member

@ntn-x2 ntn-x2 left a comment

Choose a reason for hiding this comment

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

Can we maybe remove any leftover of byte array to HEX conversions? I feel we still do it quite few times and it does not seem to be necessary.

@rflechtner rflechtner merged commit 8b6e57f into develop Oct 19, 2022
@rflechtner rflechtner deleted the rf-refactor-verify-did-signature branch October 19, 2022 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arty-name arty-name arty-name approved these changes

@ntn-x2 ntn-x2 ntn-x2 approved these changes

Assignees

@rflechtner rflechtner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rflechtner @arty-name @ntn-x2