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

Wallet Connect #357

Merged
merged 12 commits into from
Jan 7, 2021
Merged

Wallet Connect #357

merged 12 commits into from
Jan 7, 2021

Conversation

jessgusclark
Copy link
Member

@jessgusclark jessgusclark commented Dec 9, 2020

Add WalletConnect as a web3Provider to rLogin.

Operations that require more than one transaction are disabled as they don't work with the existing wallets. A warning is showed on the Domain Information screen as well as the two places where operations do not work.

Was tested with rWallet and defiant wallet.

testing:

This can be tested by using this link: https://rnsdomains.github.io/rns-manager-react/

Since this test is to Github pages, you will probably experience 404 errors and broken images. Please navigate using the header.

@jessgusclark jessgusclark marked this pull request as draft December 10, 2020 10:39
@jessgusclark jessgusclark marked this pull request as ready for review January 4, 2021 12:43
The previous version was not compatible with wallet connect and was showing the error `this.send is not a function`. This behavior is documented in this issue here: web3/web3.js#3790. The reccomendation was to downgrade to 1.2.x but in this case, we are upgrading to it.
- This code gets executed if a provider is present. Therefor hasMetamask is always true. Remove if statement.
- Use
- Metamask & Nifty return an integer where as WalletConnect returns a boolean.
- Redux and connections
@sonarcloud
Copy link

sonarcloud bot commented Jan 4, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Comment on lines +408 to +409
"is_wallet_connect_warning": "WalletConnect is a experimental provider and not all RNS manager operations work.",
"wallet_connect_feature": "This feature does not work with WalletConnect."
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not forget the translations 🙏

Copy link
Contributor

@javiesses javiesses left a comment

Choose a reason for hiding this comment

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

Like it, I've tested it from GH pages. Managed to create a domain and then a subdomain.
Had some issues with the subdomain creation, it seems it is not receiving well the event when the tx is confirmed, so it never shows the new subdomain in the list. Then I refresh, disconnect from Wallet Connect and tried again and it worked like a charm

@javiesses javiesses merged commit 6401d8a into develop Jan 7, 2021
juan-rsk pushed a commit to juan-rsk/rns-manager-react that referenced this pull request Apr 6, 2021
* Add wallet-connect package and connect to rLogin init.

* Bump web3 version

The previous version was not compatible with wallet connect and was showing the error `this.send is not a function`. This behavior is documented in this issue here: web3/web3.js#3790. The reccomendation was to downgrade to 1.2.x but in this case, we are upgrading to it.

* Update auth/operations start

- This code gets executed if a provider is present. Therefor hasMetamask is always true. Remove if statement.
- Use

* Transaction listener should return success on true

- Metamask & Nifty return an integer where as WalletConnect returns a boolean.

* Listen to disconnect event, move disconnect above login.

* Fix propTypes error.

* Remove console.log, fix lint.

* Remove commented out code.

* Rename hasMetamask to hasWeb3Provider

- Redux and connections

* Add isWalletConnect redux prop.

* Add messages where wallet connect functionality does not work.

* Fix lint issue.
@ilanolkies ilanolkies deleted the wallet-connect branch June 28, 2021 18:07
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

2 participants