Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Update wallet listing policy #6438
Update wallet listing policy #6438
Changes from 30 commits
2882732
8499ca5
e94d027
a03d908
82826e1
ab182e2
a3b2fc1
ac47cc7
39ddd6e
7e053e9
6c37a98
a5f11e0
c9f471a
3b6145a
722f04f
4994d77
569fc85
09e63fd
293634c
11f7342
0b340cd
fa2779c
abf92f8
c2a8183
7494f9f
49c6e0a
2814fd4
887c27d
79f0b21
4acb059
9a0ac4c
d26024e
b206960
23bfea0
cba7469
fdf119d
c1f8876
0c0b6a3
a557f42
b4b5654
c1de7a7
1036f39
2002de2
bec25f3
f52c997
778982d
b615e9b
39d4a75
0e3f089
60a5bff
ee94755
f358eaa
1246f99
1a52f39
08de2bc
193a3eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Open question for discussion: Should an active development team be noted as an absolute requirement?
Also, what defines active?
My take is this should be a requirement to keep users safe. Ethereum is still relatively young and has many upgrades being rolled out, and listed wallets should have developers maintaining the codebase and tending to bug reports.
Not sure what the cutoff should be though for an "inactive" project... Perhaps either an official announcement, or no code commits for > 6 mos?
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 do think an active development team is a requirement. Just last year there was the EIP-1559 upgrade that required wallets to upgrade as a single example where there are changes that wallets would need to support. If anything this is almost the main requirement out of all of this.
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 we going to use this information? We should probably ask for exactly what we want if we are.
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 don't see this explicitly in the design right now. @nloureiro Was there a plan to include these?
For comparison to other areas of the site, we have the socials in the
staking-products.json
for each product, but we ended up not adding these to the product cards that get rendered. Agree if we decide to not include this in the design then we should probably remove the question.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.
Im ok if these arent in v1 of this release, but I do think at some point showing this information would be ideal.
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.
Then perhaps we ask explicitly? It'll save a bunch of time in the future.
Discord server:
Twitter:
what else?
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.
Curious how we would use this info? Wouldn't this one be more of a binary yes/no?
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.
Yeah, this is more binary. My idea here is a link to documentation like this: https://docs.metamask.io/guide/signing-data.html#signing-data-with-metamask
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 wonder how important this feature is given that users could just swap for liquid staking tokens?
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.
Probably less important. There is still a difference between adding more ETH to the pool, compared to just buying someone elses stake, but I hear ya. I think it's still worth leaving this in.
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.
Curious what folks think of "layer 2" vs. "multi-chain"? Or should we include both?
Ultimately I suspect L2 is what we care about. Users may want other EVM chains. There could also be user benefit in e.g. Bitcoin support in a wallet (like Ledger does).
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.
Agree that we don't care about alt L1s like AVAX here but what about sidechains like Polygon?
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.
Perhaps a better question: which L2s do you support? e.g. Argent supports zkSync but not Optimism. But Metamask supports custom RPCs.
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.
Yeah, I see L2 and multi-chain as two different feature sets... one is contained within the Ethereum security bubble, the other is many different security bubbles.
Yes, in general, it's about which networks the wallet can connect to... but someone who uses Ethereum may reasonably want to find a wallet that supports L2s so they can inherit Ethereum's L1 security and still afford gas.
Meanwhile, someone who uses many cryptocurrencies on many chains care that their wallet also supports these other chains.
So my vote would be to include both. We should ask what non-EVM chains are compatible, and also what Ethereum L2s are compatible, imo.
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 agree with the notion here, but my pushback would be from our discussions of what layer 2's are. I think a user would want to look at validiums or optimistic chains as well, which I would say still fall under the bundle of scaling Ethereum, but we don't classify it as a layer 2. That being said, I think that is more captured in the RPC importing question. Down for a discussion, but I think I agree with this suggestion.