-
Notifications
You must be signed in to change notification settings - Fork 180
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
Autonamespace fix #1305
Autonamespace fix #1305
Conversation
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.
LGTM! Just a minor comment 👌
@@ -11,8 +11,12 @@ final class SessionProposalInteractor { | |||
|
|||
let supportedRequiredChains = proposal.requiredNamespaces["eip155"]?.chains | |||
let supportedOptionalChains = proposal.optionalNamespaces?["eip155"]?.chains ?? [] | |||
let supportedChains = (supportedRequiredChains ?? []).union(supportedOptionalChains) ?? [] | |||
|
|||
var supportedChains = (supportedRequiredChains ?? []).union(supportedOptionalChains) ?? [] |
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.
Prolly a nitpick but, would be useful to make supportedChains
a Set in case supportedRequiredChains
and supportedOptionalChains
have repeated chains? Like [eip155:1, eip155:137, eip155:10]
and [eip155:137, eip155:10, eip155:56, .....]
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.
union is a Set method :D
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.
Nice! I've told you I'm rusty :D
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.
I think it applies to every language
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.
To every language that has it maybe :P
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.
LGTM!
case unsupportedChains | ||
case unsupportedMethods | ||
case unsupportedAccounts | ||
case upsupportedEvents |
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.
upsupportedEvents
misspelled word
Description
fixes a bug with required chains validation
Resolves # (issue)
How Has This Been Tested?
Due Dilligence