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.

Sign up for GitHub

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

Jump to bottom

Chore(WalletConnect): Upgrade Wallet connect version and migrate to WalletKit #16468

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Chore(WalletConnect): Upgrade Wallet connect version and migrate to WalletKit #16468

merged 1 commit into from
Oct 10, 2024

Conversation

alexjba
Copy link
Contributor

@alexjba alexjba commented Oct 4, 2024

What does the PR do

Closes #15916

The web3Wallet is now WalletKit and WalletConnect is being rebranded to Reown.

Updated the packages based on docs https://docs.reown.com/walletkit/upgrade/from-web3wallet-web

@alexjba alexjba changed the base branch from master to feat/sign-timeout October 4, 2024 13:24
@alexjba alexjba requested review from a team, Cuteivist, anastasiyaig and saledjenic and removed request for a team October 4, 2024 13:25
@status-im-auto
Copy link
Member

status-im-auto commented Oct 4, 2024 •
edited
Loading

Jenkins Builds

Click to see older builds (7)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 1f1c0ce #2 2024-10-04 13:31:44 ~6 min tests/nim 📄log
✔️ 1f1c0ce #2 2024-10-04 13:33:16 ~8 min macos/aarch64 🍎dmg
✔️ 1f1c0ce #2 2024-10-04 13:34:51 ~9 min macos/x86_64 🍎dmg
❌ 1f1c0ce #2 2024-10-04 13:37:40 ~12 min tests/ui 📄log
✔️ 1f1c0ce #2 2024-10-04 13:40:20 ~15 min linux-nix/x86_64 📦tgz
✔️ 1f1c0ce #2 2024-10-04 13:43:19 ~18 min linux/x86_64 📦tgz
❌ 1f1c0ce #3 2024-10-07 09:02:00 ~10 min tests/ui 📄log
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 80390e4 #3 2024-10-08 12:27:47 ~6 min tests/nim 📄log
✔️ 80390e4 #3 2024-10-08 12:30:25 ~9 min macos/aarch64 🍎dmg
❌ 80390e4 #4 2024-10-08 12:32:34 ~11 min tests/ui 📄log
✔️ 80390e4 #3 2024-10-08 12:33:40 ~12 min macos/x86_64 🍎dmg
✔️ 80390e4 #3 2024-10-08 12:33:49 ~12 min linux-nix/x86_64 📦tgz
✔️ 80390e4 #3 2024-10-08 12:38:37 ~17 min linux/x86_64 📦tgz
❌ 80390e4 #5 2024-10-09 07:05:23 ~11 min tests/ui 📄log
✔️ 80390e4 #4 2024-10-10 09:56:07 ~6 min macos/aarch64 🍎dmg
✔️ 80390e4 #4 2024-10-10 09:56:19 ~6 min tests/nim 📄log
❌ 80390e4 #6 2024-10-10 10:01:06 ~11 min tests/ui 📄log
✔️ 80390e4 #4 2024-10-10 10:02:24 ~12 min macos/x86_64 🍎dmg
✔️ 80390e4 #4 2024-10-10 10:07:53 ~18 min linux-nix/x86_64 📦tgz
✔️ 80390e4 #4 2024-10-10 10:08:06 ~18 min linux/x86_64 📦tgz
✔️ 80390e4 #4 2024-10-10 10:11:30 ~21 min windows/x86_64 💿exe
✔️ 16c7d60 #5 2024-10-10 12:37:33 ~5 min macos/aarch64 🍎dmg
✔️ 16c7d60 #5 2024-10-10 12:38:10 ~6 min tests/nim 📄log
✔️ 16c7d60 #7 2024-10-10 12:43:27 ~11 min tests/ui 📄log
✔️ 16c7d60 #5 2024-10-10 12:44:06 ~12 min macos/x86_64 🍎dmg
✔️ 16c7d60 #5 2024-10-10 12:45:14 ~13 min linux-nix/x86_64 📦tgz
✔️ 16c7d60 #5 2024-10-10 12:47:38 ~16 min linux/x86_64 📦tgz
✔️ 16c7d60 #5 2024-10-10 12:57:47 ~26 min windows/x86_64 💿exe

@status-im-auto
Copy link
Member

✔️ status-desktop/prs/windows/x86_64/package/PR-16468#2 🔹 ~26 min 🔹 1f1c0ce 🔹 📦 windows/x86_64 package

@virginiabalducci virginiabalducci self-requested a review October 7, 2024 11:12
@virginiabalducci
Copy link

Hi @alexjba
I tested the build from this PR and the connection is not working as expected.

  1. Go go a dApp page, for example Rarible
  2. Connect via Browser connect

Actual result: the pop up to sign the connection request shows up on the desktop app, user accepts it and app seems to be connected, however on the dApps end, Status app is not connecting.

URL: https://appkit-lab.reown.com/library/wagmi-siwe/
Console log:
app_20241007_220608.log

Screen.Recording.2024-10-07.at.10.17.50.PM.mov

dApp: Rarible

Screen.Recording.2024-10-07.at.10.20.36.PM.mov 
WRN 2024-10-08 01:21:28.362Z qt warning                                 topics="qt" tid=54837088 text="Warning: Multi-selection is disabled, but multiple items are selected. Automatically selecting the first inserted item." file=qrc:/app/AppLayouts/Wallet/views/NetworkSelectorView.qml:43 category=qml
WRN 2024-10-08 01:21:28.420Z qt warning                                 topics="qt" tid=54837088 text="QML Binding: Binding loop detected for property \"value\"" file=qrc:/imports/shared/controls/AccountSelector.qml:136 category=default
DBG 2024-10-08 01:21:33.560Z NewBE_callPrivateRPC                       topics="rpc" tid=54837088 file=core.nim:27 rpc_method=wakuext_slowdownArchivesImport
DBG 2024-10-08 01:21:33.646Z NewBE_callPrivateRPC                       topics="rpc" tid=54837088 file=core.nim:27 rpc_method=connector_requestAccountsAccepted
ERR 2024-10-08 01:21:33.650Z qt error                                   topics="qt" tid=54837088 text="addSession: missing required parameters" file=qrc:/app/AppLayouts/Wallet/services/dapps/ConnectorDAppsListProvider.qml:17 category=qml
DBG 2024-10-08 01:21:35.442Z NewBE_callPrivateRPC                       topics="rpc" tid=54837088 file=core.nim:27 rpc_method=wakuext_speedupArchivesImport

Console log:
app_20241007_220608.log

geth.log.zip

Let me know in case I can provide any other information, thanks!

@alexjba
Copy link
Contributor Author

alexjba commented Oct 8, 2024

Hi @alexjba I tested the build from this PR and the connection is not working as expected.

  1. Go go a dApp page, for example Rarible
  2. Connect via Browser connect

Actual result: the pop up to sign the connection request shows up on the desktop app, user accepts it and app seems to be connected, however on the dApps end, Status app is not connecting.

URL: https://appkit-lab.reown.com/library/wagmi-siwe/ Console log: app_20241007_220608.log

Screen.Recording.2024-10-07.at.10.17.50.PM.mov

dApp: Rarible

Screen.Recording.2024-10-07.at.10.20.36.PM.mov

WRN 2024-10-08 01:21:28.362Z qt warning                                 topics="qt" tid=54837088 text="Warning: Multi-selection is disabled, but multiple items are selected. Automatically selecting the first inserted item." file=qrc:/app/AppLayouts/Wallet/views/NetworkSelectorView.qml:43 category=qml
WRN 2024-10-08 01:21:28.420Z qt warning                                 topics="qt" tid=54837088 text="QML Binding: Binding loop detected for property \"value\"" file=qrc:/imports/shared/controls/AccountSelector.qml:136 category=default
DBG 2024-10-08 01:21:33.560Z NewBE_callPrivateRPC                       topics="rpc" tid=54837088 file=core.nim:27 rpc_method=wakuext_slowdownArchivesImport
DBG 2024-10-08 01:21:33.646Z NewBE_callPrivateRPC                       topics="rpc" tid=54837088 file=core.nim:27 rpc_method=connector_requestAccountsAccepted
ERR 2024-10-08 01:21:33.650Z qt error                                   topics="qt" tid=54837088 text="addSession: missing required parameters" file=qrc:/app/AppLayouts/Wallet/services/dapps/ConnectorDAppsListProvider.qml:17 category=qml
DBG 2024-10-08 01:21:35.442Z NewBE_callPrivateRPC                       topics="rpc" tid=54837088 file=core.nim:27 rpc_method=wakuext_speedupArchivesImport

Console log: app_20241007_220608.log

geth.log.zip

Let me know in case I can provide any other information, thanks!

Thanks for testing. There are a few mentions on your findings. The first one is that the test is targeting BC and BC is not covered in this PR. Srry, should have provided more details here.. What I'm doing in this PR is to update the WalletConnect version we're using in the Status app.
The second one is that in your tests the dApps tries to use SIWE on BrowserConnect. First we'll need to implement the personal sign (#16046 and #16015). SIWE will be implemented at a later stage.

@virginiabalducci
Copy link

Hey @alexjba thanks for the clarification

It seems WC is also presenting issues. On the build app of this PR, I'm unable to connect to any dApp. I tested with 3 test users with the same results.
I also deleted my browsing history and tried again with the same results.

Tested in https://lido.fi/, https://react-app.walletconnect.com/, https://app.uniswap.org/, https://rarible.com/

Screen.Recording.2024-10-08.at.8.29.28.AM.rarible.mov
Screen.Recording.2024-10-08.at.8.27.38.AM.stake.lido.mov 
WRN 2024-10-08 11:28:46.641Z qt warning                                 topics="qt" tid=55267654 text="QML Connections: Detected function \"onContactRenamed\" in Connections element. This is probably intended to be a signal handler but no signal of the target matches the name." file=qrc:/app/mainui/activitycenter/views/ActivityNotificationMessage.qml:72 category=default
DBG 2024-10-08 11:28:47.373Z NewBE_callPrivateRPC                       topics="rpc" tid=55267654 file=core.nim:27 rpc_method=wakuext_markAsSeenActivityCenterNotifications
DBG 2024-10-08 11:28:51.373Z NewBE_callPrivateRPC                       topics="rpc" tid=55267654 file=core.nim:27 rpc_method=wakuext_speedupArchivesImport
DBG 2024-10-08 11:29:35.077Z NewBE_callPrivateRPC                       topics="rpc" tid=55267654 file=core.nim:27 rpc_method=wakuext_slowdownArchivesImport
DBG 2024-10-08 11:29:38.192Z qt message                                 topics="qt" tid=55267654 text="WC WalletConnectSDK.wcCall.getActiveSessions;" file=qrc:/app/AppLayouts/Wallet/services/dapps/WalletConnectSDK.qml:112 category=qml
ERR 2024-10-08 11:29:38.198Z qt error                                   topics="qt" tid=55267654 text="Uncaught TypeError: Cannot read property 'getActiveSessions' of null" file=qrc:/app/AppLayouts/Wallet/services/dapps/sdk/generated/bundle.js:2 category=js
DBG 2024-10-08 11:29:40.293Z NewBE_callPrivateRPC                       topics="rpc" tid=55267654 file=core.nim:27 rpc_method=wakuext_speedupArchivesImport
DBG 2024-10-08 11:29:45.070Z NewBE_callPrivateRPC                       topics="rpc" tid=55267654 file=core.nim:27 rpc_method=wakuext_slowdownArchivesImport
DBG 2024-10-08 11:29:48.102Z NewBE_callPrivateRPC                       topics="rpc" tid=55267654 file=core.nim:27 rpc_method=wakuext_speedupArchivesImport

@alexjba
Copy link
Contributor Author

alexjba commented Oct 8, 2024

Thanks @virginiabalducci! Will have a look. It looks like a code error somewhere. Probably in the parent PRs

@alexjba
Copy link
Contributor Author

alexjba commented Oct 8, 2024

@virginiabalducci Fixed! There's been an issue on the upgrade

@status-im-auto
Copy link
Member

✔️ status-desktop/prs/windows/x86_64/package/PR-16468#3 🔹 ~25 min 🔹 80390e4 🔹 📦 windows/x86_64 package

@alexjba alexjba force-pushed the feat/sign-timeout branch from bf61332 to c498580 Compare October 9, 2024 09:52
@alexjba alexjba requested review from micieslak, a team and caybro as code owners October 9, 2024 09:52
@caybro
Copy link
Member

caybro commented Oct 9, 2024

@alexjba conflicts :)

@alexjba alexjba force-pushed the feat/sign-timeout branch 3 times, most recently from 9178c8c to 4a5bacb Compare October 10, 2024 07:40
Base automatically changed from feat/sign-timeout to master October 10, 2024 09:49
@alexjba
chore: Migrate to the new WC packages
16c7d60 
Migrate from `Web3Wallet` to `WalletKit`.
More details in the WC docs https://docs.reown.com/walletkit/upgrade/from-web3wallet-web
caybro
caybro approved these changes Oct 10, 2024
View reviewed changes
Copy link
Member

@caybro caybro left a comment

Choose a reason for hiding this comment

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

LGTM

virginiabalducci
virginiabalducci approved these changes Oct 10, 2024
View reviewed changes
Copy link

@virginiabalducci virginiabalducci left a comment

Choose a reason for hiding this comment

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

tested a couple of dApps and looks good!

@alexjba alexjba merged commit e99f817 into master Oct 10, 2024
9 checks passed
@alexjba alexjba deleted the feat/upgrade-WC branch October 10, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caybro caybro caybro approved these changes

@virginiabalducci virginiabalducci virginiabalducci approved these changes

@saledjenic saledjenic saledjenic approved these changes

@Cuteivist Cuteivist Awaiting requested review from Cuteivist Cuteivist is a code owner automatically assigned from status-im/desktop-wallet

@anastasiyaig anastasiyaig Awaiting requested review from anastasiyaig anastasiyaig is a code owner automatically assigned from status-im/desktop-wallet

@micieslak micieslak Awaiting requested review from micieslak micieslak is a code owner

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

Successfully merging this pull request may close these issues.

Update Wallet Connect SDK
5 participants
@alexjba @status-im-auto @virginiabalducci @caybro @saledjenic