Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Narrow return type of signTypedMessage and encryption methods #249

Merged
merged 2 commits into from
Jul 17, 2023

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 17, 2023

Description

The methods signTypedMessage, getEncryptionPublicKey, and decryptMessage always return a string, but the declared return type was Bytes (which is a type union of string along with other things). This made the methods more difficult to use, as callers had to handle the other Bytes types as return values as well.

The methods have been updated to use string as the return type.

Changes

  • Changed: The methods signTypedMessage, getEncryptionPublicKey, and decryptMessage now return string rather than Bytes
    • This is non-breaking because the Bytes type includes string

References

Helps to unblock MetaMask/core#1441

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@Gudahtt Gudahtt requested a review from a team as a code owner July 17, 2023 21:45
mikesposito
mikesposito previously approved these changes Jul 17, 2023
Copy link
Member

@mikesposito mikesposito 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 just noticed we have the same problem with getEncryptionPublicKey anddecryptMessage, we can fix them separately but we could also take this chance

@Gudahtt Gudahtt changed the title Narrow return type of signTypedMessage Narrow return type of signTypedMessage and encryption methods Jul 17, 2023
@Gudahtt
Copy link
Member Author

Gudahtt commented Jul 17, 2023

Oh, good catch! I have updated those two methods as well, and updated the PR title & description accordingly.

Figured we may as well fix them here since it's the same problem.

Gudahtt added 2 commits July 17, 2023 20:06
The method `signTypedMessage` always returns a string, but the declared
return type was `Bytes` (which is a type union of `string` along with
other things). This made the method more difficult to use, as callers
had to handle the other `Bytes` types as return values as well.

The method has been updated to use `string` as the return type.
@Gudahtt Gudahtt force-pushed the fix-sign-typed-message-return-type branch from 242b788 to 2d07efc Compare July 17, 2023 22:36
@Gudahtt Gudahtt merged commit 3e102b5 into main Jul 17, 2023
@Gudahtt Gudahtt deleted the fix-sign-typed-message-return-type branch July 17, 2023 22:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants