web3.js icon indicating copy to clipboard operation
web3.js copied to clipboard

Add raw hash signing function (without the wrapping in Ethereum message: ...)

Open mconnelly8 opened this issue 1 year ago • 7 comments

mconnelly8 avatar Apr 04 '24 19:04 mconnelly8

https://docs.web3js.org/libdocs/Accounts#sign

jdevcs avatar May 28 '24 14:05 jdevcs

scope: add function, doc, unit test ( with recovery )

jdevcs avatar May 28 '24 14:05 jdevcs

@jdevcs Hey, is anyone working on this? I'd love to try this one. I plan to add one more option parameter into the hashMessage to add support for whether there is a prefix or not. https://github.com/web3/web3.js/blob/f70140693b14edb20891a4c8554f5fcc58dc6dd3/packages/web3-eth-accounts/src/account.ts#L159

Overall, should we introduce new function pairs, such as signRaw/recoverRaw, or modify the logic of the existing sign/recover?

The latter might introduce breaking changes, because the current prefixed param of recover actually combines whether to hash and add a prefix. If we modify the existing functionality, this logic might change. https://github.com/web3/web3.js/blob/f70140693b14edb20891a4c8554f5fcc58dc6dd3/packages/web3-eth-accounts/src/account.ts#L424

blackmoshui avatar Oct 10 '24 07:10 blackmoshui

@blackmoshui thanks for your contribution, We will prefer to avoid breaking change.

jdevcs avatar Oct 14 '24 13:10 jdevcs

Hey @blackmoshui, just wanted to confirm that you want to implement this? Let us know if you have any questions. The main thing we want to ensure is that we want to avoid breaking changes.

mconnelly8 avatar Oct 15 '24 20:10 mconnelly8

Hey @blackmoshui, just wanted to confirm that you want to implement this? Let us know if you have any questions. The main thing we want to ensure is that we want to avoid breaking changes.

I understand that avoiding breaking changes in a client-side SDK is very important.

I plan to add an optional parameter to hashMessage to support not adding a prefix, then add a new signRaw function along with corresponding tests and documentation.

In the initial PR, I won't add functions like recoverRaw because users can combine existing hashMessage and recover functions to implement recovery themselves. I believe this approach won't introduce any breaking changes.

What do you think? I can prepare a draft PR first.

blackmoshui avatar Oct 16 '24 15:10 blackmoshui

@jdevcs?

mconnelly8 avatar Oct 17 '24 18:10 mconnelly8

I prepared a PR, but after completing it I realized that we could also achieve this functionality by modifying the existing sign function instead of adding a new function. I can modify the PR if needed.

blackmoshui avatar Oct 22 '24 06:10 blackmoshui