fix: Fix multibase [DEV-2307] #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mirceanis
reviewed
Mar 6, 2023
| @@ -48,7 +48,7 @@ export class VeramoEd25519Signature2020 extends VeramoLdSignature { | |||
| const verificationKey = new Ed25519VerificationKey2020({ | |||
| id, | |||
| controller, | |||
| publicKeyMultibase: this.preSigningKeyModification(u8a.fromString(key.publicKeyHex, 'hex')), | |||
| publicKeyMultibase: u8a.toString(u8a.fromString(key.publicKeyHex, 'hex'), 'base58btc'), | |||
There was a problem hiding this comment.
I'm not sure that's enough for it to be a multibase public key representation.
publicKeyMultibase is supposed to be self descriptive, meaning that the key type (Ed25519) should be determined by a prefix.
The internal publicKeyHex is supposed to be the raw public key data, without such a prefix.
The encoding you are doing here seems to be more suitable for publicKeyBase58 rather than publicKeyMultibase.
If you're having trouble matching a Verification Method from a DID document against the locally managed key (decentralized-identity#1139), it's more likely that there's a bug in extractPublicKeyBytes. Perhaps try to check if this line is doing the right thing.
Eengineer1
approved these changes
Mar 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What issue is this PR fixing
fixes decentralized-identity#1139
What is being changed
This PR slices the two-byte prefix 0xed01 in publicKeyMultibase according to the spec in extractPublicKeyHex function
Quality
Check all that apply:
Details
If applicable, add screen captures, error messages or stack traces to help explain your problem.