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

NEP-413 Requires Backwards Compatibility Clarification #941

Closed
sunny0529 opened this issue Sep 21, 2023 · 3 comments
Closed

NEP-413 Requires Backwards Compatibility Clarification #941

sunny0529 opened this issue Sep 21, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@sunny0529
Copy link
Contributor

Summary

There are two signers, NEP-413 and Legacy. I'm wondering if the wallets should maintain backwards compatibility.

Description

The existing Wallet Selecter, before the introduction of NEP-413, has a signMessage, which is a signer that takes a uint8array message and performs the signing.
Its implementation can be found at: https://github.com/near/near-api-js/blob/master/packages/signers/src/signer.ts#L26

NEP-413 introduces a new signer with the same name “signMessage” that takes in an Object, serializes it, and signs it.
Details can be found here:

export interface SignMessageParams {

Then, should the wallets support both methods by overloading signMessage?
I was wondering if there is any guidance on whether the wallet should support both NEP-413 and legacy signer(to maintain backwards compatibility), or just NEP-413.

Furthermore, we are considering proposing a NEP that improves on the existing NEP, by including backwards compatibility in NEP-413. It would be good for other wallet teams to work without misunderstandings if there is clarification on this.

Depending on Wallet Selector’s answer, we would like to work closely with you and propose an NEP.

@sunny0529 sunny0529 added the enhancement New feature or request label Sep 21, 2023
@kujtimprenkuSQA
Copy link
Contributor

Hi, @sunny0529 thank you for raising this issue.

There seems to be a misunderstanding of what the NEP0413 signMessage is meant for, the name of the method is "accidentally" the same as the name of the method on the Signer class.

The signMessage of the NEP is meant to be addded in the public API of a wallet (e.g. window.near.wallet.signMessage) the NEP never suggests that this is an update to the implementation of the signMessage of the Signer class.

If you read the first description of the PR: near/NEPs#413 you will notice that this PR initally was about adding a verifyOwner method to the API of a wallet, but the reviewers and the community in the comments of the PR agreed to change the name from verifyOwner to signMessage that's why I mentioned that the name is almost "accidentally" the same.

The signMessage of the NEP must always be signed inside the wallet with the FullAccess key of the chosen account not by the signer of the dApp (wallet-selector).

You might find the referenced example useful: https://github.com/near/NEPs/blob/master/neps/nep-0413.md#references

@AmmarHumackicSQA
Copy link
Contributor

Hi @sunny0529 let us know if you have any additional questions, or we can close this issue. Thank you!

@sunny0529
Copy link
Contributor Author

Thank you for your response. I understand well, thanks to your explanation.

I think you can close this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants