sdk: fix personal message to use bcs #12997
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
sdk/typescript/src/verify/index.ts
Outdated
| export async function verifySignature( | ||
| bytes: Uint8Array, | ||
| signature: SerializedSignature, | ||
| ): Promise<false | PublicKey> { |
There was a problem hiding this comment.
Not sure if this is a good way to handle returns of these methods, but I think getting back the public key is useful here so you can also validate that the key used to sign a message was the expected one
There was a problem hiding this comment.
i dont have strong preference but slight preference just having it to return true/false is more intuitive and easier to return to the outer level
There was a problem hiding this comment.
Rather than false | publicKey I wonder if this should just throw? I do agree getting the public key back is pretty useful, because often you want to validate ownership of an address and without the return here it becomes multi-step.
There was a problem hiding this comment.
updated this to throw
|
@Jordan-Mysten I updated this with the changes we talked about today. I forgot we were going to do this as 2 separate changes, I can split it up if thats easier, but I think most of the changes here are pretty straight forward |
sdk/typescript/src/verify/index.ts
Outdated
| export async function verifySignature( | ||
| bytes: Uint8Array, | ||
| signature: SerializedSignature, | ||
| ): Promise<false | PublicKey> { |
There was a problem hiding this comment.
Rather than false | publicKey I wonder if this should just throw? I do agree getting the public key back is pretty useful, because often you want to validate ownership of an address and without the return here it becomes multi-step.
## Description Closes #12931. Add feature to verify a multisig in typescript. depends on #12997 ## Test Plan unit tests. --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes --------- Co-authored-by: Michael Hayes <michael.hayes@mystenlabs.com>
Description
fix a bug that the signMessage in rust vs typescript has a discrepancy. this is make typescript use bcs serialization for a vector instead of just a plain vector.
Test Plan
existing tests in ts still passes (bc verify and sign are both changed to the new format).
tested against rust that a ts produced sig now verifies in rust.
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes