From 6aa6746de2f65cdc69d7bab673401af4e35920d3 Mon Sep 17 00:00:00 2001 From: Roman Chornii Date: Mon, 5 Aug 2024 16:41:20 +0300 Subject: [PATCH] fix(dApps): Improved handling of connected dApps. (#15985) 1. Hiding DApps button on not supported wallet account selection 2. Filtering DApps in connected dApps list based on account selection closes: #15589 closes: #15647 --- storybook/pages/DAppsWorkflowPage.qml | 2 + storybook/pages/DappsComboBoxPage.qml | 19 ++++- .../qmlTests/tests/tst_DAppsWorkflow.qml | 53 ++++++++----- .../AppLayouts/Wallet/panels/WalletHeader.qml | 5 +- .../services/dapps/DAppsListProvider.qml | 77 ++++++++++++++++--- .../services/dapps/WalletConnectService.qml | 35 +++++++-- .../Wallet/services/dapps/helpers.js | 11 ++- .../popups/walletconnect/DAppsListPopup.qml | 17 ++++ 8 files changed, 177 insertions(+), 42 deletions(-) diff --git a/storybook/pages/DAppsWorkflowPage.qml b/storybook/pages/DAppsWorkflowPage.qml index 6e54e05b74..af223aeaa9 100644 --- a/storybook/pages/DAppsWorkflowPage.qml +++ b/storybook/pages/DAppsWorkflowPage.qml @@ -427,6 +427,8 @@ Item { // Name mismatch between storybook and production readonly property var groupedAccountAssetsModel: groupedAccountsAssetsModel } + + readonly property string selectedAddress: "" } onDisplayToastMessage: (message, isErr) => { diff --git a/storybook/pages/DappsComboBoxPage.qml b/storybook/pages/DappsComboBoxPage.qml index 870b1883f8..81c8f6d669 100644 --- a/storybook/pages/DappsComboBoxPage.qml +++ b/storybook/pages/DappsComboBoxPage.qml @@ -17,7 +17,7 @@ SplitView { id: connectedDappComboBox anchors.top: parent.top anchors.horizontalCenter: parent.horizontalCenter - model: emptyModelCheckbox.checked ? emptyModel : dappsModel + model: emptyModelCheckbox.checked ? emptyModel : smallModelCheckbox.checked ? smallModel: dappsModel popup.visible: true } @@ -25,6 +25,15 @@ SplitView { id: emptyModel } + ListModel { + id: smallModel + ListElement { + name: "SMALL Model" + url: "https://dapp.test/1" + iconUrl: "https://se-sdk-dapp.vercel.app/assets/eip155:1.png" + } + } + ListModel { id: dappsModel ListElement { @@ -96,10 +105,16 @@ SplitView { text: "Empty model" checked: false } + + CheckBox { + id: smallModelCheckbox + text: "Small model" + checked: false + } } } } // category: Controls -// https://www.figma.com/design/HrmZp1y4S77QJezRFRl6ku/dApp-Interactions---Milestone-1?node-id=130-31949&t=hnzB58fTnEnx2z84-0 \ No newline at end of file +// https://www.figma.com/design/HrmZp1y4S77QJezRFRl6ku/dApp-Interactions---Milestone-1?node-id=130-31949&t=hnzB58fTnEnx2z84-0 diff --git a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml index 49ab7cf5f2..f666441f10 100644 --- a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml +++ b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml @@ -182,16 +182,26 @@ Item { } readonly property ListModel nonWatchAccounts: ListModel { - ListElement {address: "0x1"} + ListElement { + address: "0x1" + keycardAccount: false + } ListElement { address: "0x2" name: "helloworld" emoji: "😋" color: "#2A4AF5" + keycardAccount: false + } + ListElement { + address: "0x3a" + keycardAccount: false } - ListElement { address: "0x3a" } // Account from GroupedAccountsAssetsModel - ListElement { address: "0x7F47C2e18a4BBf5487E6fb082eC2D9Ab0E6d7240" } + ListElement { + address: "0x7F47C2e18a4BBf5487E6fb082eC2D9Ab0E6d7240" + keycardAccount: false + } } function getNetworkShortNames(chainIds) { return "eth:oeth:arb" @@ -605,7 +615,7 @@ Item { const address = ModelUtils.get(provider.supportedAccountsModel, 0, "address") let session = JSON.parse(Testing.formatApproveSessionResponse([1, 2], [address], {dappMetadataJsonString: Testing.noIconsDappMetadataJsonString})) callback({"b536a": session, "b537b": session}) - compare(provider.dappsModel.count, 1, "expected dappsModel have the SDK's reported dapps") + compare(provider.dappsModel.count, 1, "expected dappsModel have the SDK's reported dapp, 2 sessions of the same dApp per 2 wallet account, meaning 1 dApp model entry") compare(provider.dappsModel.get(0).iconUrl, "", "expected iconUrl to be missing") let updateCalls = provider.store.updateWalletConnectSessionsCalls compare(updateCalls.length, 1, "expected a call to store.updateWalletConnectSessions") @@ -693,26 +703,27 @@ Item { compare(eip155.events.length, 2) } - function test_filterActiveSessionsForKnownAccounts() { + function test_getAccountsInSession() { const account1 = accountsModel.get(0) const account2 = accountsModel.get(1) const chainIds = [chainsModel.get(0).chainId, chainsModel.get(1).chainId] - const knownSession = JSON.parse(Testing.formatApproveSessionResponse(chainIds, [account2.address])) - // Allow the unlikely unknown accounts to cover for the deleted accounts case - const unknownSessionWithKnownAccount = JSON.parse(Testing.formatApproveSessionResponse(chainIds, ['0x03acc', account1.address])) - const unknownSession1 = JSON.parse(Testing.formatApproveSessionResponse(chainIds, ['0x83acc'])) - const unknownSession2 = JSON.parse(Testing.formatApproveSessionResponse(chainIds, ['0x12acc'])) - let testSessions = { - "b536a": knownSession, - "b537b": unknownSession1, - "b538c": unknownSession2, - "b539d": unknownSessionWithKnownAccount - } - const res = DAppsHelpers.filterActiveSessionsForKnownAccounts(testSessions, accountsModel) - compare(Object.keys(res).length, 2, "expected two sessions to be returned") - // Also test that order is stable - compare(res["b536a"], knownSession, "expected the known session to be returned") - compare(res["b539d"], unknownSessionWithKnownAccount, "expected the known session to be returned") + + const oneAccountSession = JSON.parse(Testing.formatApproveSessionResponse(chainIds, [account2.address])) + const twoAccountsSession = JSON.parse(Testing.formatApproveSessionResponse(chainIds, ['0x03acc', account1.address])) + const duplicateAccountsSession = JSON.parse(Testing.formatApproveSessionResponse(chainIds, ['0x83acb', '0x83acb'])) + + const res = DAppsHelpers.getAccountsInSession(oneAccountSession) + compare(res.length, 1, "expected the only account to be returned") + compare(res[0], account2.address, "expected the only account to be the one in the session") + + const res2 = DAppsHelpers.getAccountsInSession(twoAccountsSession) + compare(res2.length, 2, "expected the two accounts to be returned") + compare(res2[0], '0x03acc', "expected the first account to be the one in the session") + compare(res2[1], account1.address, "expected the second account to be the one in the session") + + const res3 = DAppsHelpers.getAccountsInSession(duplicateAccountsSession) + compare(res3.length, 1, "expected the only account to be returned") + compare(res3[0], '0x83acb', "expected the duplicated account") } } diff --git a/ui/app/AppLayouts/Wallet/panels/WalletHeader.qml b/ui/app/AppLayouts/Wallet/panels/WalletHeader.qml index 342d8ba513..c03d2f5cff 100644 --- a/ui/app/AppLayouts/Wallet/panels/WalletHeader.qml +++ b/ui/app/AppLayouts/Wallet/panels/WalletHeader.qml @@ -133,9 +133,12 @@ Item { spacing: 8 - visible: !root.walletStore.showSavedAddresses && Global.featureFlags.dappsEnabled + visible: !root.walletStore.showSavedAddresses + && Global.featureFlags.dappsEnabled + && Global.walletConnectService.isServiceAvailableForAddressSelection enabled: !!Global.walletConnectService + wcService: Global.walletConnectService loginType: root.store.loginType selectedAccountAddress: root.walletStore.selectedAddress diff --git a/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml b/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml index 26cd50e36c..ac43169266 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml @@ -1,7 +1,10 @@ import QtQuick 2.15 +import StatusQ 0.1 import StatusQ.Core.Utils 0.1 +import SortFilterProxyModel 0.2 + import AppLayouts.Wallet.services.dapps 1.0 import shared.stores 1.0 @@ -15,7 +18,24 @@ QObject { required property DAppsStore store required property var supportedAccountsModel - readonly property alias dappsModel: d.dappsModel + property string selectedAddress: "" + + readonly property SortFilterProxyModel dappsModel: SortFilterProxyModel { + objectName: "DAppsModelFiltered" + sourceModel: d.dappsModel + + filters: FastExpressionFilter { + enabled: !!root.selectedAddress + + function isAddressIncluded(accountAddressesSubModel, selectedAddress) { + const addresses = ModelUtils.modelToFlatArray(accountAddressesSubModel, "address") + return addresses.includes(root.selectedAddress) + } + expression: isAddressIncluded(model.accountAddresses, root.selectedAddress) + + expectedRoles: "accountAddresses" + } + } function updateDapps() { d.updateDappsModel() @@ -26,6 +46,7 @@ QObject { property ListModel dappsModel: ListModel { id: dapps + objectName: "DAppsModel" } property var dappsListReceivedFn: null @@ -33,10 +54,24 @@ QObject { function updateDappsModel() { dappsListReceivedFn = (dappsJson) => { + root.store.dappsListReceived.disconnect(dappsListReceivedFn); dapps.clear(); let dappsList = JSON.parse(dappsJson); for (let i = 0; i < dappsList.length; i++) { + const cachedEntry = dappsList[i]; + let accountAddresses = cachedEntry.accountAddresses + if (!accountAddresses) { + accountAddresses = [{address: ''}]; + } + + const dappEntryWithRequiredRoles = { + description: cachedEntry.description, + url: cachedEntry.url, + name: cachedEntry.name, + iconUrl: cachedEntry.url, + accountAddresses: cachedEntry.accountAddresses + } dapps.append(dappsList[i]); } } @@ -49,27 +84,45 @@ QObject { } getActiveSessionsFn = () => { - sdk.getActiveSessions((allSessions) => { + sdk.getActiveSessions((allSessionsAllProfiles) => { root.store.dappsListReceived.disconnect(dappsListReceivedFn); - let tmpMap = {} - var topics = [] - const sessions = DAppsHelpers.filterActiveSessionsForKnownAccounts(allSessions, root.supportedAccountsModel) - for (const key in sessions) { - const dapp = sessions[key].peer.metadata + const dAppsMap = {} + const topics = [] + const sessions = DAppsHelpers.filterActiveSessionsForKnownAccounts(allSessionsAllProfiles, supportedAccountsModel) + for (const sessionID in sessions) { + const session = sessions[sessionID] + const dapp = session.peer.metadata if (!!dapp.icons && dapp.icons.length > 0) { dapp.iconUrl = dapp.icons[0] } else { dapp.iconUrl = "" } - tmpMap[dapp.url] = dapp; - topics.push(key) + const accounts = DAppsHelpers.getAccountsInSession(session) + const existingDApp = dAppsMap[dapp.url] + if (existingDApp) { + // In Qt5.15.2 this is the way to make a "union" of two arrays + // more modern syntax (ES-6) is not available yet + const combinedAddresses = new Set(existingDApp.accountAddresses.concat(dapp.accountAddresses)); + existingDApp.accountAddresses = Array.from(combinedAddresses); + } else { + dapp.accountAddresses = accounts + dAppsMap[dapp.url] = dapp + } + + topics.push(sessionID) } + // TODO #15075: on SDK dApps refresh update the model that has data source from persistence instead of using reset dapps.clear(); - // Iterate tmpMap and fill dapps - for (const key in tmpMap) { - dapps.append(tmpMap[key]); + + // Iterate dAppsMap and fill dapps + for (const topic in dAppsMap) { + const dAppEntry = dAppsMap[topic]; + // Due to ListModel converting flat array to empty nested ListModel + // having array of key value pair fixes the problem + dAppEntry.accountAddresses = dAppEntry.accountAddresses.filter(account => (!!account)).map(account => ({address: account})); + dapps.append(dAppEntry); } root.store.updateWalletConnectSessions(JSON.stringify(topics)) diff --git a/ui/app/AppLayouts/Wallet/services/dapps/WalletConnectService.qml b/ui/app/AppLayouts/Wallet/services/dapps/WalletConnectService.qml index dfca0c10d5..ccc2269311 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/WalletConnectService.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/WalletConnectService.qml @@ -35,6 +35,8 @@ QObject { readonly property alias dappsModel: dappsProvider.dappsModel readonly property alias requestHandler: requestHandler + readonly property bool isServiceAvailableForAddressSelection: dappsProvider.supportedAccountsModel.ModelCount.count + readonly property var validAccounts: SortFilterProxyModel { sourceModel: d.supportedAccountsModel proxyRoles: [ @@ -109,14 +111,19 @@ QObject { } function disconnectDapp(url) { - wcSDK.getActiveSessions((allSessions) => { - const sessions = DAppsHelpers.filterActiveSessionsForKnownAccounts(allSessions, d.supportedAccountsModel) + wcSDK.getActiveSessions((allSessionsAllProfiles) => { + const sessions = DAppsHelpers.filterActiveSessionsForKnownAccounts(allSessionsAllProfiles, validAccounts) for (const sessionID in sessions) { const session = sessions[sessionID] + const accountsInSession = DAppsHelpers.getAccountsInSession(session) const dapp = session.peer.metadata const topic = session.topic if (dapp.url === url) { - wcSDK.disconnectSession(topic) + if (!dappsProvider.selectedAddress || + (accountsInSession.includes(dappsProvider.selectedAddress))) + { + wcSDK.disconnectSession(topic) + } } } }); @@ -226,7 +233,13 @@ QObject { QObject { id: d - readonly property var supportedAccountsModel: root.walletRootStore.nonWatchAccounts + readonly property var supportedAccountsModel: SortFilterProxyModel { + sourceModel: root.walletRootStore.nonWatchAccounts + filters: ValueFilter { + roleName: "keycardAccount" + value: false + } + } property var currentSessionProposal: null property var acceptedSessionProposal: null @@ -265,7 +278,19 @@ QObject { sdk: root.wcSDK store: root.store - supportedAccountsModel: d.supportedAccountsModel + supportedAccountsModel: SortFilterProxyModel { + objectName: "SelectedAddressModelForDAppsListProvider" + sourceModel: d.supportedAccountsModel + filters: FastExpressionFilter { + enabled: !root.walletRootStore.showAllAccounts + + expression: root.walletRootStore.selectedAddress.toLowerCase() === model.address.toLowerCase() + + expectedRoles: ["address"] + } + } + + selectedAddress: root.walletRootStore.selectedAddress } // Timeout for the corner case where the URL was already dismissed and the SDK doesn't respond with an error nor advances with the proposal diff --git a/ui/app/AppLayouts/Wallet/services/dapps/helpers.js b/ui/app/AppLayouts/Wallet/services/dapps/helpers.js index b67d98730c..fab211a86d 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/helpers.js +++ b/ui/app/AppLayouts/Wallet/services/dapps/helpers.js @@ -120,4 +120,13 @@ function filterActiveSessionsForKnownAccounts(sessions, accountsModel) { knownSessions[topic] = session }) return knownSessions -} \ No newline at end of file +} + +function getAccountsInSession(session) { + const eip155Addresses = session.namespaces.eip155.accounts + const accountSet = new Set( + eip155Addresses.map(eip155Address => eip155Address.split(':').pop().trim()) + ); + const uniqueAddresses = Array.from(accountSet); + return uniqueAddresses +} diff --git a/ui/imports/shared/popups/walletconnect/DAppsListPopup.qml b/ui/imports/shared/popups/walletconnect/DAppsListPopup.qml index 82e7a2d7cb..ce5df38d80 100644 --- a/ui/imports/shared/popups/walletconnect/DAppsListPopup.qml +++ b/ui/imports/shared/popups/walletconnect/DAppsListPopup.qml @@ -1,4 +1,5 @@ import QtQuick 2.15 +import QtQml 2.15 import QtQuick.Controls 2.15 import QtQml.Models 2.15 import QtQuick.Layouts 1.15 @@ -46,9 +47,25 @@ Popup { } } + // workaround for https://bugreports.qt.io/browse/QTBUG-87804 + Binding on margins { + id: workaroundBinding + + when: false + restoreMode: Binding.RestoreBindingOrValue + } + + onImplicitContentHeightChanged: { + workaroundBinding.value = root.margins + 1 + workaroundBinding.when = true + workaroundBinding.when = false + } + contentItem: ColumnLayout { id: mainLayout + spacing: 0 + ShapeRectangle { id: listPlaceholder