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: handle error in case erc1155 id cannot be converted to BigNumber #3073

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

alfetopito
Copy link
Collaborator

@alfetopito alfetopito commented Aug 23, 2023

Summary

Attempt to fix user reported error

image

This fix parts from the assumption that the ens NFT avatar is passing a bad value onto a BigNumber constructor.

To Test

Unsure, as I have not been able to reproduce it yet

Update: User confirmed it fixes the problem \o/

Background

User's setup:

  • Brave + MM + Ledger, with ens name and ERC721 set as avatar
  • User also tried on Chrome + Rabby + Ledger and faced the same issue

The problem

User's avatar is set to http://eip155:1/erc721:0xed5af388653567af2f388e6224dc7c4b3241c544/0000 (replaced the actual id with 0000 to preserve user's identity)

The hooks that processes the avatar URL is not expecting an url in this format https://github.com/cowprotocol/cowswap/blob/76c1c7c9d1c41934eb8b9762d00389b4917616b4/apps/cowswap-frontend/src/legacy/hooks/useENSAvatar.ts#L78C1-L78C1

When splitting it by :, we get 4 parts:

["http","//eip155","1/erc721","0xed5af388653567af2f388e6224dc7c4b3241c544/3329"]

Due to the unexpected format, we end up with the id being set to erc721 instead of 0000.
Down the line the id is converted to a BigNumber to be converted to a hexString.

That's when it fails, and that's what this fix does.

This fix does not, however, fixes the loading of the different format.
I'll leave that as a task on the backlog as the immediate issue is being addressed, and the user is able to connect to the interface without crashing.

@vercel
Copy link

vercel bot commented Aug 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
swap-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2023 7:33am

@alfetopito alfetopito requested a review from a team August 23, 2023 08:06
@alfetopito alfetopito marked this pull request as ready for review August 23, 2023 08:06
@alfetopito alfetopito merged commit 82a549d into main Aug 23, 2023
13 checks passed
@alfetopito alfetopito deleted the fix-ens-avatar branch August 23, 2023 08:13
@github-actions github-actions bot locked and limited conversation to collaborators Aug 23, 2023
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