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

tx.to empty string ENS error potential fix #974

Merged
merged 3 commits into from Apr 26, 2022

Conversation

mubarakone
Copy link

test for potential fix: ethers-io/ethers.js#2750 (comment)

@mubarakone
Copy link
Author

@kyokosdream

@mubarakone mubarakone changed the title Update ethereum.ts tx.to empty string ENS error potential fix Apr 25, 2022
@kyokosdream
Copy link

kyokosdream commented Apr 25, 2022

@mubarakone I'm trying to fork the repo and install the custom fork using npm but I'm not having any success. I think the problem has to do with the monorepo structure and the use of scoped packages. Not sure how to proceed.

@kyokosdream
Copy link

Could we just get this request merged as soon as possible? It's proved exceedingly difficult to install a forked version of the code given the monorepo structure, me and @mubarakone have spent many hours attempting to do so. Contract deployment is currently broken with WalletConnect anyway so this change can't hurt any existing users.

I would recommend just changing this single line:
OLD
to: typeof txData.to === "undefined" ? "" : sanitizeHex(txData.to)
NEW
to: typeof txData.to === "undefined" ? null : sanitizeHex(txData.to)

@mubarakone
Copy link
Author

Could we just get this request merged as soon as possible? It's proved exceedingly difficult to install a forked version of the code given the monorepo structure, me and @mubarakone have spent many hours attempting to do so. Contract deployment is currently broken with WalletConnect anyway so this change can't hurt any existing users.

I would recommend just changing this single line: OLD to: typeof txData.to === "undefined" ? "" : sanitizeHex(txData.to) NEW to: typeof txData.to === "undefined" ? null : sanitizeHex(txData.to)

Will do

Copy link
Author

@mubarakone mubarakone left a comment

Choose a reason for hiding this comment

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

@mubarakone mubarakone marked this pull request as ready for review April 25, 2022 21:37
@kyokosdream
Copy link

The changes we've made don't comply with @walletconnect/types unfortunately. We may need to change the null to undefined.

@pedrouid pedrouid merged commit b35c29e into WalletConnect:v1.0 Apr 26, 2022
@pedrouid
Copy link
Member

@kyokosdream @mubarakone I have published v1.7.8 with these changes

@kyokosdream
Copy link

@pedrouid You're a legend thanks for assisting us on this so quickly. I will test today.

@mubarakone
Copy link
Author

@pedrouid Thanks for the merge! We worked really hard to solve this issue.
If you don't mind, I applied for a Javscript position, and I figured volunteering for you guys would be a good way to get a refferal. I can solve more issues and submit more PRs if I can continue to be apart of your team. Let me know how that sounds! Thanks once again.

mubarakone added a commit to mubarakone/walletconnect-monorepo that referenced this pull request May 4, 2022
A continuation of WalletConnect#974

`gasPrice`, `gas`, `value`, `nonce`, and `data` are left in blank strings, causing errors when deploying/interacting with smart contracts. This could potentially fix some issues, please mention this PR in those issues to inform users.
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.

None yet

3 participants