Support wa key signature verification #115
Conversation
apporc
changed the title
apporc
changed the title
| */ | ||
| export function recover(signature: Uint8Array, message: Uint8Array, type: string) { | ||
| if (type === KeyType.WA) { | ||
| throw new Error(`can't recover webauthn public keys, please use @wharfkit/webauthn.`) |
src/chain/public-key.ts
Outdated
| * This is suitable for cryptographic operations like verification. | ||
| */ | ||
| getCompressedKeyBytes(): Uint8Array { | ||
| return this.data.array.subarray(0, 33) |
There was a problem hiding this comment.
Just so the result is mutable, I would probably do this instead:
| return this.data.array.subarray(0, 33) | |
| return new Uint8Array(this.data.array.subarray(0, 33)) |
There was a problem hiding this comment.
I don't think one would need the result as mutable.
There was a problem hiding this comment.
It's good practice to always return something that is mutable in a situation like this one because other developers might expect it to be. With your code, if someone takes the returned compressed key and modifies it for whatever reason, that will also modify the this.data value which will modify the public key itself. I don't think we ever want that to happen so I would return a new array like in my above suggestion.
There was a problem hiding this comment.
Yeah, you are right, we should make the code robust. I didn't realize that you can unintentionally mutate the original Uint8Array while mutating the returned value. I mean developers wouldn't need to mutate the returned value, which is functionally meaningless, but you are right, what if he made a mistake.
There was a problem hiding this comment.
Since the PR is merged, i added another PR to switch to use slice, i think it will solve the issue.
#117
| * Returns the core 33-byte compressed public key data as a Uint8Array. | ||
| * This is suitable for cryptographic operations like verification. | ||
| */ | ||
| getCompressedKeyBytes(): Uint8Array { |
There was a problem hiding this comment.
is it worth throwing an error when this.data.array is less than 33 bytes? If not, maybe we add a check in the constructor that throws an error when this.data is not the correct length?
There was a problem hiding this comment.
I am not sure whether we should improve the error handling in this PR. It would change the designed behavior of the library. Illegal bytes passed in situation was not considered at all in this library.
There was a problem hiding this comment.
Yeah, maybe it should be something that this library checks, but you're right that doesn't need to be part of this PR 👍
No description provided.