From 130b8b67901d150ca1ce3cf92e6fb076b448ff2f Mon Sep 17 00:00:00 2001 From: Stefan Date: Wed, 19 Jun 2024 18:17:49 +0300 Subject: [PATCH] feat(dapps) fix missing list of dapps in wallet connect Updates: #15126 --- .../wallet_section/activity/controller.nim | 2 - .../service/wallet_connect/service.nim | 2 +- .../qmlTests/tests/helpers/wallet_connect.js | 24 +++++-- .../qmlTests/tests/tst_DAppsWorkflow.qml | 64 +++++++++++++++++-- .../services/dapps/DAppsListProvider.qml | 9 +-- 5 files changed, 85 insertions(+), 16 deletions(-) diff --git a/src/app/modules/main/wallet_section/activity/controller.nim b/src/app/modules/main/wallet_section/activity/controller.nim index 0347e8cace..e2962c97b9 100644 --- a/src/app/modules/main/wallet_section/activity/controller.nim +++ b/src/app/modules/main/wallet_section/activity/controller.nim @@ -249,12 +249,10 @@ QtObject: # setup other event handlers self.eventsHandler.onFilteringDone(proc (jsonObj: JsonNode) = - echo "@dd eventsHandler.onFilteringDone: ", $jsonObj self.processResponse(jsonObj) ) self.eventsHandler.onFilteringUpdateDone(proc (jn: JsonNode) = - echo "@dd eventsHandler.onFilteringUpdateDone: ", $jn if jn.kind != JArray: error "expected an array" diff --git a/src/app_service/service/wallet_connect/service.nim b/src/app_service/service/wallet_connect/service.nim index 68d44956c1..d3277f0958 100644 --- a/src/app_service/service/wallet_connect/service.nim +++ b/src/app_service/service/wallet_connect/service.nim @@ -103,7 +103,7 @@ QtObject: return "" if buildTxResponse.isNil or buildTxResponse.kind != JsonNodeKind.JObject or not buildTxResponse.hasKey("txArgs") or not buildTxResponse.hasKey("messageToSign"): - error "unexpected buildTransaction response" + error "unexpected wallet_buildTransaction response" return "" var txToBeSigned = buildTxResponse["messageToSign"].getStr if txToBeSigned.len != wallet_constants.TX_HASH_LEN_WITH_PREFIX: diff --git a/storybook/qmlTests/tests/helpers/wallet_connect.js b/storybook/qmlTests/tests/helpers/wallet_connect.js index 9a3b26a54e..3afbbb1d11 100644 --- a/storybook/qmlTests/tests/helpers/wallet_connect.js +++ b/storybook/qmlTests/tests/helpers/wallet_connect.js @@ -54,6 +54,13 @@ const dappMetadataJsonString = `{ "url": "${dappUrl}" }` +// https://metamask.github.io/test-dapp/ use case that doesn't have icons +const noIconsDappMetadataJsonString = `{ + "description": "This is the E2e Test Dapp", + "name": "${dappName}", + "url": "${dappUrl}" +}` + const verifiedContextJsonString = `{ "verified": { "origin": "https://app.test.org", @@ -62,7 +69,12 @@ const verifiedContextJsonString = `{ } }` -function formatSessionProposal() { +function formatSessionProposal(custom) { + var dappMetadataJsonStringOverride = dappMetadataJsonString + if (custom && custom.dappMetadataJsonString) { + dappMetadataJsonStringOverride = custom.dappMetadataJsonString + } + return `{ "id": 1715976881734096, "params": { @@ -71,7 +83,7 @@ function formatSessionProposal() { "optionalNamespaces": ${optionalNamespacesJsonString}, "pairingTopic": "50fba141cdb5c015493c2907c46bacf9f7cbd7c8e3d4e97df891f18dddcff69c", "proposer": { - "metadata": ${dappMetadataJsonString}, + "metadata": ${dappMetadataJsonStringOverride}, "publicKey": "095d9992ca0eb6081cabed26faf48919162fd70cc66d639f118a60507ae0463d" }, "relays": [ @@ -98,7 +110,11 @@ function formatBuildApprovedNamespacesResult(networksArray, accountsArray) { }` } -function formatApproveSessionResponse(networksArray, accountsArray) { +function formatApproveSessionResponse(networksArray, accountsArray, custom) { + var dappMetadataJsonStringOverride = dappMetadataJsonString + if (custom && custom.dappMetadataJsonString) { + dappMetadataJsonStringOverride = custom.dappMetadataJsonString + } let chainsStr = networksArray.map(chainId => `"eip155:${chainId}"`).join(',') let accountsStr = accountsArray.map(address => networksArray.map(chainId => `"eip155:${chainId}:${address}"`).join(',')).join(',') return `{ @@ -116,7 +132,7 @@ function formatApproveSessionResponse(networksArray, accountsArray) { "optionalNamespaces": ${optionalNamespacesJsonString}, "pairingTopic": "50fba141cdb5c015493c2907c46bacf9f7cbd7c8e3d4e97df891f18dddcff69c", "peer": { - "metadata": ${dappMetadataJsonString}, + "metadata": ${dappMetadataJsonStringOverride}, "publicKey": "095d9992ca0eb6081cabed26faf48919162fd70cc66d639f118a60507ae0463d" }, "relay": { diff --git a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml index f86082f202..23538c71f5 100644 --- a/storybook/qmlTests/tests/tst_DAppsWorkflow.qml +++ b/storybook/qmlTests/tests/tst_DAppsWorkflow.qml @@ -25,7 +25,7 @@ Item { width: 600 height: 400 - // TODO #15151 fix CI crash and re-enable tests + // // TODO #15151 fix CI crash and re-enable tests // Component { // id: sdkComponent @@ -79,13 +79,15 @@ Item { // id: dappsStoreComponent // DAppsStore { + // property string dappsListReceivedJsonStr: '[]' + // signal dappsListReceived(string dappsJson) // signal userAuthenticated(string topic, string id, string password, string pin) // signal userAuthenticationFailed(string topic, string id) // // By default, return no dapps in store // function getDapps() { - // dappsListReceived('[]') + // dappsListReceived(dappsListReceivedJsonStr) // return true // } @@ -333,8 +335,54 @@ Item { // } // Component { - // id: componentUnderTest - // DAppsWorkflow { + // id: dappsListProviderComponent + // DAppsListProvider { + // } + // } + + // TestCase { + // name: "DAppsListProvider" + + // property DAppsListProvider provider: null + + // readonly property var dappsListReceivedJsonStr: '[{"url":"https://tst1.com","name":"name1","iconUrl":"https://tst1.com/u/1"},{"url":"https://tst2.com","name":"name2","iconUrl":"https://tst2.com/u/2"}]' + + // function init() { + // // Simulate the SDK not being ready + // let sdk = createTemporaryObject(sdkComponent, root, {projectId: "12ab", sdkReady: false}) + // verify(!!sdk) + // let store = createTemporaryObject(dappsStoreComponent, root, { + // dappsListReceivedJsonStr: dappsListReceivedJsonStr + // }) + // verify(!!store) + // provider = createTemporaryObject(dappsListProviderComponent, root, {sdk: sdk, store: store}) + // verify(!!provider) + // } + + // function cleanup() { + // } + + // // Implemented as a regression to metamask not having icons which failed dapps list + // function test_TestUpdateDapps() { + // provider.updateDapps() + + // // Validate that persistance fallback is working + // compare(provider.dappsModel.count, 2, "expected dappsModel have the right number of elements") + // let persistanceList = JSON.parse(dappsListReceivedJsonStr) + // compare(provider.dappsModel.get(0).url, persistanceList[0].url, "expected url to be set") + // compare(provider.dappsModel.get(0).iconUrl, persistanceList[0].iconUrl, "expected iconUrl to be set") + // compare(provider.dappsModel.get(1).name, persistanceList[1].name, "expected name to be set") + + // // Validate that SDK's `getActiveSessions` is not called if not ready + // let sdk = provider.sdk + // compare(sdk.getActiveSessionsCallbacks.length, 0, "expected no calls to sdk.getActiveSessions yet") + // sdk.sdkReady = true + // compare(sdk.getActiveSessionsCallbacks.length, 1, "expected a call to sdk.getActiveSessions when SDK becomes ready") + // let callback = sdk.getActiveSessionsCallbacks[0].callback + // let session = JSON.parse(Testing.formatApproveSessionResponse([1, 2], ["0x1"], {dappMetadataJsonString: Testing.noIconsDappMetadataJsonString})) + // callback({"b536a": session, "b537b": session}) + // compare(provider.dappsModel.count, 1, "expected dappsModel have the SDK's reported dapps") + // compare(provider.dappsModel.get(0).iconUrl, "", "expected iconUrl to be missing") // } // } @@ -418,7 +466,13 @@ Item { // } // } - // TODO #15151: this TestCase if placed before ServiceHelpers was not run with `when: windowShown`. Check if related to the CI crash + // Component { + // id: componentUnderTest + // DAppsWorkflow { + // } + // } + + // // TODO #15151: this TestCase if placed before ServiceHelpers was not run with `when: windowShown`. Check if related to the CI crash // TestCase { // id: dappsWorkflowTest diff --git a/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml b/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml index 9dbb47078d..5c31b06d2b 100644 --- a/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml +++ b/ui/app/AppLayouts/Wallet/services/dapps/DAppsListProvider.qml @@ -54,17 +54,18 @@ QObject { sdk.getActiveSessions((sessions) => { root.store.dappsListReceived.disconnect(dappsListReceivedFn); - // TODO #14755: on SDK dApps refresh update the model that has data source from persistence instead of using reset - dapps.clear(); - let tmpMap = {} for (let key in sessions) { let dapp = sessions[key].peer.metadata - if (dapp.icons.length > 0) { + if (!!dapp.icons && dapp.icons.length > 0) { dapp.iconUrl = dapp.icons[0] + } else { + dapp.iconUrl = "" } tmpMap[dapp.url] = dapp; } + // TODO #14755: 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 (let key in tmpMap) { dapps.append(tmpMap[key]);