Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: only allow hex addresses when creating notifications #5343

Merged

Conversation

Prithpal-Sooriya
Copy link
Contributor

@Prithpal-Sooriya Prithpal-Sooriya commented Feb 14, 2025

Explanation

we do not support solana or non evm addresses. If we want to support this, then we would need a backend update to prevent a 400 bad requests

References

Changelog

@metamask/notification-services-controller

  • FIXED: Added isValidHexAddress when fetching keyring accounts to ensure we only get hex addresses
  • FIXED: Added cleanup logic to remote saved notification user storage blobs to only support hex addresses.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

we do not support solana or non evm addresses. If we want to support this, then we would need a backend update to prevent a 400 bad request
@Prithpal-Sooriya Prithpal-Sooriya requested a review from a team as a code owner February 14, 2025 19:46
@Prithpal-Sooriya Prithpal-Sooriya enabled auto-merge (squash) February 14, 2025 19:52
@Prithpal-Sooriya Prithpal-Sooriya merged commit 0223bed into main Feb 15, 2025
133 checks passed
@Prithpal-Sooriya Prithpal-Sooriya deleted the fix/remove-non-hex-addresses-from-notifications branch February 15, 2025 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants