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.

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

Implement Message Signing WASM #281

Merged
merged 5 commits into from
Oct 26, 2023

Conversation

coderofstuff
Copy link
Collaborator

New JS functions:

  • signMessage(message, privkey) - privkey can be a hex string or PrivateKey object
  • verifyMessage(message, signature, pubkey) - pubkey can be a hex string or PublicKey object

@coderofstuff coderofstuff force-pushed the coderofstuff/message-wasm branch from ee4dbbf to debc147 Compare September 20, 2023 05:36
@coderofstuff coderofstuff requested a review from aspect September 20, 2023 05:37
@someone235 someone235 marked this pull request as draft September 20, 2023 08:20
@someone235 someone235 marked this pull request as ready for review September 21, 2023 10:23
@coderofstuff coderofstuff requested a review from aspect September 22, 2023 05:40
@KaffinPX
Copy link
Contributor

I think signMessage should be on private key class?

@coderofstuff coderofstuff force-pushed the coderofstuff/message-wasm branch from fb4f893 to cd6488d Compare October 2, 2023 19:20
@aspect
Copy link
Collaborator

aspect commented Oct 4, 2023

@coderofstuff Before moving forward with merging this PR, I wanted to discuss/suggest perhaps converting these functions to receive not multiple arguments but a single JsValue object containing these arguments. I have been converting all WASM functions (where applicable), removing arguments to have them receive a single Object argument.

For example, the following function receives arguments via an object: https://github.com/kaspanet/rusty-kaspa/blob/master/wallet/core/src/wasm/utxo/context.rs#L114-L128

This allows us to "future proof" these functions, allowing them to, in the future, if the need arises, to receive different types of arguments, without having to change function signatures.

For example, if in the future, we want to add other types of signatures (as discussed during our KIP-005 debate) the function can decide what type of signature is to be used by examining the supplied object data...

Arguments would need to be documented in the rustdoc section for each function.

--

One more note - if one wants to document types on JS side, instead of receiving a JsValue, the function can receive a custom data type which is declared as JS type on the Rust side (example: https://github.com/kaspanet/rusty-kaspa/blob/master/wallet/core/src/wasm/tx/generator/generator.rs#L22)

The only reason this is done is that all type documentation propagated to TypeScript, so when using such type in TypeScript/JavaScript, the user immediately gets full inline documentation of the data type in question.

This is not currently used in many places but the long-term goal is to convert the majority of API calls to use custom types in such manner. This last part is not really needed at this point, just letting you know that this can be done.

@aspect
Copy link
Collaborator

aspect commented Oct 4, 2023

I think signMessage should be on private key class?
@KaffinPX if you see my recent feedback, I am trying to convert and make most API calls to accept a single Object argument that contains different properties as arguments. This allows us to make these functions accept additional arguments in the future without changing their signatures.

There have been a number of side discussions to support other types of signatures in the future, such approach allows us to extend these fns in the future.

However, affixing these functions to ProvateKey plays against that (for example, in the future such function may accept a PrivateKey parameter or a wallet Account parameter).

So I prefer to keep these functions disassociated from any specific data types (classes).

@coderofstuff coderofstuff force-pushed the coderofstuff/message-wasm branch 2 times, most recently from e817dbf to 041a15b Compare October 7, 2023 05:13
@coderofstuff coderofstuff force-pushed the coderofstuff/message-wasm branch from 041a15b to 038ccb8 Compare October 22, 2023 16:58
Copy link
Collaborator

@aspect aspect left a comment

Choose a reason for hiding this comment

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

Looks great. Merging.

@aspect aspect merged commit 11cc019 into kaspanet:master Oct 26, 2023
6 checks passed
@coderofstuff coderofstuff deleted the coderofstuff/message-wasm branch October 26, 2023 18:58
smartgoo pushed a commit to smartgoo/rusty-kaspa that referenced this pull request Jun 18, 2024
* Implement Message Signing WASM

* Simplify getting structs from JsValue

* Update sign/verify to use single object input

* Result handling optimizations

---------

Co-authored-by: aspect <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants