signMessage interface not matching example in NEP

[lostpebble]

https://github.com/near/wallet-selector/pull/779#issuecomment-1700933919

Hi @Elabar,

The signMessage is not working correctly on Meteor Wallet. Can you guys check if the implementation of the signMessage is as in the example of the NEP: https://github.com/gagdiez/near-login/blob/main/tests/authentication/wallet.ts#L60

@erditkurteshiSQA would love a little more context here- why does the interface have to look exactly like the example? We've implemented the API in the Wallet Selector, so it works fine there as far as I know.

Is this perhaps related more to the Injected Wallet NEP ? https://github.com/near/NEPs/pull/408 - so, in that case, using our Wallet without making use of our SDK's interface, but rather the one defined there?

[erditkurteshiSQA]

Hi, thank you for taking a look at this.

We think that there was something that we missed while reviewing the signMessage PR in the verifySignature function.

So in this line here: https://github.com/near/wallet-selector/pull/779/files#diff-be0cd0f34e9432b7f604d8c997d479ebbfdf9d5abd6ebde0d02b42e7a5ab2c20R22

We think that this needs to be hashed exactly as in the NEP example with sha256.array because all other wallets that implement this feature will do the same if they follow the example: https://github.com/gagdiez/near-login/blob/main/authenticate/wallet-authenticate.js#L21

The above is an assumption because we don't really know the implementation of signMessage inside the meteor wallet sdk.

We have updated the verifySignature in our wallet-selector dev branch to match exactly what the example referenced in the NEP suggests, would you mind testing the signMessage from this branch?

[lostpebble]

Thanks, will check out the dev branch and diagnose things a bit.

[erditkurteshiSQA]

Hi @lostpebble ,

Just checking if there is any update regarding this issue. Thank you.

[lostpebble]

Hi @erditkurteshiSQA -

I think the issue is that you've gone with a slightly different implementation of how to sign the message.

We implemented this ages ago and was working fine- but there is a slight difference now in how things are implemented, which I do worry could cause issues if others have done the same.

According to the NEP itself (https://github.com/gutsyphilip/NEPs/blob/8b0b05c3727f0a90b70c6f88791152f54bf5b77f/neps/nep-0413.md#example):

It is telling us to create a hash of the payload, but your implementation is slightly different:

const hashedPayload = js_sha256.sha256.array(borshPayload) // returns a number array (byte array)

Compared to the example, and our implementation:

const hashedPayload = js_sha256.sha256(borshPayload); // returns a hash hex string

When we change this one line, the Wallet Selector response is verified successfully in the new version.

So I'm not sure what the next steps are here. Does the NEP need to be updated with your new way of doing it and we all have to update our code? I think everyone has been referencing the NEP, so perhaps the verification code needs to change to the correct implementation.

[lostpebble]

Or perhaps I'm incorrect- the NEP doesn't actually specify directly what kind of data the sha256.hash function returns. We assumed it meant a "hash" string as implemented by the base js_sha256.sha256 function, but perhaps that is a byte array.

[lostpebble]

Actually- this is our mistake. We forgot to set the type of encoding when making a buffer out of the hex hash string:

const signedPayload = keyPair.sign(Buffer.from(hashedPayload, "hex"));

So this should then act exactly the same as your way of doing it- which is just more direct to the Buffer / UInt8Array.

[lostpebble]

I'm updating everything now and will push a fix asap.

[lostpebble]

This fix is currently live on our web app version. Extension is in the process of being reviewed and updated (can take a day or two).