Skip to content

Conversation

@kclowes
Copy link
Collaborator

@kclowes kclowes commented Apr 3, 2024

What was wrong?

It's not safe to sign random hashes, but occasionally developers want to do so. We were undecided about what to do with the method, but decided to add a new method called unsafe_sign_hash and finally remove signHash

Closes #85

How was it fixed?

Removed signHash, moved functionality to unsafe_sign_hash.

LMK if you think we should add more/different docs

Todo:

  • Clean up commit history

  • Add or update documentation related to these changes

  • Add entry to the release notes

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes force-pushed the unsafe-sign-hash branch from 5f96177 to 10bdd06 Compare April 3, 2024 17:24
@kclowes kclowes added the v0.13.x label Apr 3, 2024
@kclowes kclowes marked this pull request as ready for review April 3, 2024 17:37
@kclowes kclowes requested review from fselmo, pacrob and reedsa April 3, 2024 17:37
@kclowes kclowes force-pushed the unsafe-sign-hash branch from c3c452e to 9bb3ad6 Compare April 3, 2024 17:47
Copy link
Contributor

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm. I think the name change should point most users to reading the documentation.

Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@reedsa reedsa left a comment

Choose a reason for hiding this comment

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

lgtm!

@kclowes kclowes merged commit d5153e2 into ethereum:main Apr 5, 2024
@Uxio0
Copy link

Uxio0 commented Apr 5, 2024

Sounds like a really good solution, thanks @kclowes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

signHash is deprecated in favor of sign_message

5 participants