Zondax / ledger-stacks

Apache License 2.0
17 stars 7 forks source link

Adjust `Stacks Signed Message` length prefix #120

Closed kyranjamie closed 1 year ago

kyranjamie commented 1 year ago

In implementing message signing for the Hiro Wallet, I've noticed a discrepancy between our software implementation.

Software implementation:

// 'Stacks Message Signing:\n'.length //  = 24
// 'Stacks Message Signing:\n'.length.toString(16) //  = 18
const chainPrefix = '\x18Stacks Message Signing:\n';

https://github.com/blockstack/stacks.js/blob/5ac9385069ce48bde452520d5168c84e31b5b961/packages/encryption/src/messageSignature.ts#L5-L8

(This uses the wrong phrasing and needs to be updated to Stacks Signed Message)

Zondax implementation: https://github.com/Zondax/ledger-blockstack/blob/32d8f12fe93bfc9e4c4a65605a88f0a9048695ec/js/src/index.ts#L251-L254

Hiro Wallet is currently using \x18 and Zondax uses \x19. There was some discussion about the prefix in the issue @MarvinJanssen created.

Stacks Signed Message:\n is 24 chars, so wouldn't that make the prefix \x18 in little-endian encoding

~I believe we should be using \x18, unless I misunderstand how the prefix length works.~

@neithanmo did say here that it should indeed by 19, though I don't understand why.

Edit: actually both wrong:

"Ethereum Signed Message:\n".length.toString(16) // = 19, same used in Eth prefix `\x19`
"Stacks Signed Message:\n".length.toString(16) // = 17

In previous calculations I was counting \n manually as two chars, so believe that value should instead be:

\x17Stacks Signed Message:\n

:link: zboto Link

neithanmo commented 1 year ago

closed via #121