-
Notifications
You must be signed in to change notification settings - Fork 984
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
Only show the Wallet Connect requests for the logged in user #20815
Conversation
Jenkins BuildsClick to see older builds (4)
|
How will this play with my PR that filter's sessions based on testnet or mainnet mode ? |
@shivekkhurana it has nothing to do with that, it's only about the issue that is described in #20155 where you might observe getting requests that are for another user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't test, but looks good.
(defn filter-operable-accounts | ||
[accounts] | ||
(filter #(and (:operable? %) | ||
(not (:watch-only? %))) | ||
accounts)) | ||
|
||
(defn filter-sessions-for-account-addresses | ||
[account-addresses sessions] | ||
(filter (fn [{:keys [accounts]}] | ||
(some (fn [account] | ||
(some (fn [account-address] | ||
(clojure.string/includes? account account-address)) | ||
account-addresses)) | ||
accounts)) | ||
sessions)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these helpers available elsewhere as well? Like in a sub or smth. Could be extracted into a helper maybe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more like a function designed to help with processing sessions we've just got from SDK, that's why it's not a sub.
But isn't it already like a helper? I moved it to here because it can potentially be useful in other places as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah no, I though you copied it from a sub or smth and this logic exists elsewhere as well. If that's not the case, then it's all good!
86% of end-end tests have passed
Expected to fail tests (1)Click to expandClass TestWalletOneDevice:
Passed tests (6)Click to expandClass TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
|
@@ -127,3 +127,19 @@ | |||
:url (get-in session [:peer :metadata :url]) | |||
:accounts (get-in session [:namespaces :eip155 :accounts]) | |||
:disconnected false}) | |||
|
|||
(defn filter-operable-accounts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a good idea to keep this logic somewhere it can be reused. There's this function status-im.contexts.communities.utils/sorted-operable-non-watch-only-accounts
that could reuse it for example. Maybe a good place to move it is in wallet.data-store
?
@alwx thanks for the fix. Tested and ready for merge. |
fixes #20155
fixes #20800 (but that needs verification from QA)
The issue was the incorrect filtering of sessions that we get from Wallet Connect SDK.
Platforms
status: ready