Skip to content
This repository has been archived by the owner on Jan 15, 2021. It is now read-only.

Update WalletConnect provider #1039

Merged
merged 21 commits into from
Jun 9, 2020
Merged

Update WalletConnect provider #1039

merged 21 commits into from
Jun 9, 2020

Conversation

Velenir
Copy link
Contributor

@Velenir Velenir commented May 22, 2020

Updates WC provider, with quite a leap in version
Therefore needs thorough testing first

Otherwise some dependencies get deduped and pinned to incompatible versions, and web3@1.2.8 breaks:

@ethersproject/bignumber version is wrong

> npm ls @ethersproject/bignumber
@gnosis.pm/dex-react@0.11.1 /home/velenir/Projects/Gnosis/dex-react
├─┬ @walletconnect/web3-provider@1.0.0-beta.47
│ └─┬ @walletconnect/browser@1.0.0-beta.47
│   └─┬ @walletconnect/utils@1.0.0-beta.47
│     ├─┬ @ethersproject/address@5.0.0-beta.134
│     │ └── @ethersproject/bignumber@5.0.0-beta.135 
│     └─┬ @ethersproject/strings@5.0.0-beta.136
│       └─┬ @ethersproject/constants@5.0.0-beta.133
│         └── @ethersproject/bignumber@5.0.0-beta.135  deduped
└─┬ web3@1.2.8
  └─┬ web3-eth@1.2.8
    └─┬ web3-eth-abi@1.2.8
      └─┬ @ethersproject/abi@5.0.0-beta.153
        ├─┬ @ethersproject/address@5.0.0-beta.135
        │ └── @ethersproject/bignumber@5.0.0-beta.139 
        ├── @ethersproject/bignumber@5.0.0-beta.135  deduped
        └─┬ @ethersproject/strings@5.0.0-beta.137
          └─┬ @ethersproject/constants@5.0.0-beta.134
            └── @ethersproject/bignumber@5.0.0-beta.139 

normally it would be beta.139 but because of walletconnect dependency where it's pinned to beta.135 it got deduped like that

@anxolin
Copy link
Contributor

anxolin commented May 22, 2020

Travis failed, TS unhappynes? https://travis-ci.org/github/gnosis/dex-react/builds/690006009#L779

I'll rebase this branch also with the latest develop and force a recompile, it's outdated

@anxolin anxolin force-pushed the walletconnect_update branch from 5780823 to b5fecc3 Compare May 22, 2020 15:59
@ghost
Copy link

ghost commented May 25, 2020

Travis automatic deployment:
https://pr1039--dexreact.review.gnosisdev.com

@Velenir
Copy link
Contributor Author

Velenir commented May 25, 2020

Latest WalletConnect provider just doesn't work with our setup.
Need to update dapp-ui with new web3modal(web3connect)
Or simply throw dapp-ui away and do everything here.

@anxolin
Copy link
Contributor

anxolin commented May 25, 2020 •
edited
Loading

Latest WalletConnect provider just doesn't work with our setup. Need to update dapp-ui with new web3modal(web3connect). Or simply throw dapp-ui away and do everything here.

For me is fine to throw dapp-ui if it make it simpler

So, I wait for testing this, right?

@Velenir
Copy link
Contributor Author

Velenir commented May 26, 2020

So, I wait for testing this, right?

Yes

@Velenir Velenir force-pushed the walletconnect_update branch from e04a84c to afe80dd Compare June 1, 2020 10:21
@Velenir
Copy link
Contributor Author

Velenir commented Jun 3, 2020

Can test now

alfetopito
alfetopito reviewed Jun 3, 2020
View reviewed changes
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Can't connect with WC. When I click on the button that usually displays the QR code, nothing happens and an error is logged on the console:

screenshot_2020-06-03_09-44-00

As always, using FF.

package.json Outdated
"@gnosis.pm/dex-js": "^0.4.0",
"@hapi/joi": "^17.1.1",
"@hot-loader/react-dom": "^16.11.0",
"@popperjs/core": "^2.0.6",
"@types/react": "^16.9.19",
"@types/react-is": "^16.7.1",
"@types/react-select": "^3.0.10",
"@walletconnect/web3-provider": "^1.0.0-beta.46",
"@walletconnect/web3-provider": "1.0.0-beta.47",
Copy link
Contributor

Choose a reason for hiding this comment

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

LoL that is as far as we can go, 1 beta RC ahead 🤣

src/api/wallet/WalletApi.ts Outdated Show resolved Hide resolved
@Velenir
Copy link
Contributor Author

Velenir commented Jun 4, 2020

Can't connect with WC. When I click on the button that usually displays the QR code, nothing happens and an error is logged on the console:

screenshot_2020-06-03_09-44-00

As always, using FF.

oops, Travis didn't even build, that's why you saw an older version
Was not up to date with develop
Now should be good.

I think it's really great we have at least one developer primarily on FF, otherwise we would just forget about other browsers 😥

@Velenir Velenir requested a review from alfetopito June 4, 2020 09:45
@Velenir
Copy link
Contributor Author

Velenir commented Jun 4, 2020

Check #1085 first, please

alfetopito
alfetopito approved these changes Jun 4, 2020
View reviewed changes
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Working like a charm 👍

anxolin
anxolin reviewed Jun 4, 2020
View reviewed changes
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

I'm afraid I get a random error I don't see in dev:

image

@Velenir
Copy link
Contributor Author

Velenir commented Jun 5, 2020

I'm afraid I get a random error I don't see in dev

Why does it only break in prod, smh
Should be ok now

@Velenir Velenir requested review from anxolin and alfetopito June 5, 2020 12:30
alfetopito
alfetopito approved these changes Jun 5, 2020
View reviewed changes
Copy link
Contributor

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Working 👍 (haven't tested txs though...)

anxolin
anxolin approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

works in mainnet

package.json
@@ -28,15 +28,14 @@
"@fortawesome/free-regular-svg-icons": "^5.12.0",
"@fortawesome/free-solid-svg-icons": "^5.12.0",
"@fortawesome/react-fontawesome": "^0.1.8",
"@gnosis.pm/dapp-ui": "^0.5.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

bye bye 👋

Copy link
Contributor

Choose a reason for hiding this comment

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

hate to see it go

@Velenir
Walletconnect update X (#1085)
107dcc1 
* sticky all @walletconnect/* dependencies

* allow types for @walletconnect/web3-provider/* imports

* extend wcprovider.wc type

* hack for @walletconnect/* deps resolution

* fix WC logo size

* hackaround for broken eth_* requests

* update @WalletConnect deps

* move provider types and utils

* handle WalletConnect provider differently

* fix types
@Velenir Velenir merged commit b8acf8e into develop Jun 9, 2020
@Velenir Velenir deleted the walletconnect_update branch June 9, 2020 10:25
@alfetopito alfetopito mentioned this pull request Jul 16, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@W3stside W3stside W3stside approved these changes

@anxolin anxolin anxolin approved these changes

@alfetopito alfetopito alfetopito approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Velenir @anxolin @alfetopito @W3stside