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: add name and symbol to getDetails #1727

Merged
merged 9 commits into from
Nov 8, 2023
Merged

fix: add name and symbol to getDetails #1727

merged 9 commits into from
Nov 8, 2023

Conversation

salimtb
Copy link
Contributor

@salimtb salimtb commented Sep 26, 2023

Explanation

  • What is the current state of things and why does it need to change?
    the function get details is not returning name and symbol , this mean that this information is not available in the extension for contract of type ERC1155, with this PR those two properties will be available

  • What is the solution your changes offer and how does it work?
    the solution is to call the 2 method name() and symbol() in parallel , and if we get promise resolved we will include the result in the object returned.

  • Are there any changes whose purpose might not obvious to those unfamiliar with the domain?
    no

References

Changelog

@metamask/assets-controllers

  • : FIXED
  • Add name and symbol to the payload returned by the function getDetails for erc1155 contract
  • this package is now returning name and symbol if the contract have it

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

@salimtb salimtb requested a review from a team as a code owner September 26, 2023 14:16
bergeron
bergeron previously approved these changes Sep 26, 2023
sahar-fehri
sahar-fehri previously approved these changes Sep 28, 2023
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

LGTM

@salimtb salimtb merged commit c2b9b32 into main Nov 8, 2023
128 checks passed
@salimtb salimtb deleted the MMASSETS-28 branch November 8, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Error retrieving NFT collection name with setApprovalForAll function on ERC1155 contract
4 participants