-
Notifications
You must be signed in to change notification settings - Fork 928
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
Add Medusa Wallet to Showcase #1344
base: staging
Are you sure you want to change the base?
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.
I was monitoring this as part of the CIP process and observed that it represents good implementation of standards and meets community expectations.
Co-authored-by: Robert Phair <[email protected]>
Withholding approval pending resolution of #1344 (review)
If you have any questions about Medusa, its key management, and so on, here is an article with all the answers: https://x.com/MedusaAdaWallet/status/1852456213604405735 |
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.
@Fell-x27 thanks for explaining your position in the previously linked X blog posting and for including your rationale for the key transmission & storage referenced in #1344 (comment).
- I went forward to tentatively approve this submission if I could find that this material were made available to the user before they obtain & install the Medusa wallet... which I believe is essential considering the unique security posture of transmitting and storing the user's encrypted private key: a factor that users should be aware of 100% of the time to ensure they agree with that behaviour & its rationale.
- However there's no "docs" or "help" on the linked application page, and I found no linked web site with even an "about the app" or FAQ page where such a statement would be posted... not even a place from which the material you wrote on X could be linked.
As I've stated in other Dev Portal submission reviews, when the CF & the Portal reviewing community are practically vouching for a tool by listing it here, I believe the bar is raised a bit higher to require at least a page or two of accessible documentation before agreeing to use it (in your case, by clicking the "Sign up" button on the app front page).
- I've noted from your PM the installation-time tick-box "my private key... encrypted with my spending password... will be stored in a secure vault on Medusa's server" - however I don't think it's proper form for this to be announced to the user only after agreeing to "Sign up" and as # 4 on what is currently a 6-item checklist.
- Likewise I note that you've added a link to the X article on this checklist item: currently the only place your security posture is accessible. I won't indulge in platform chauvinism but I don't think it supports your case to have this material hosted only on a system that is world-renowned for both censorship and aggressive analytics (the latter running against your "privacy" model).
So (though my endorsement is definitely not required, given the number of reviewers here (and though I have no "veto" power) I would agree to sign off on this tool if:
- There is some documentation clearly visible and/or linked on the way to "Sign up" where potential (not yet signing up!) users can see what you've written in the X blog: especially its final section OK, What About Storing Keys on the Server?;
- [as also suggested in private discussion with another reviewer here, though I also strongly believe this] Editing the
Description:
field of your listing to clearly & concisely indicate (whether you consider it a "security feature" or disclaimer) that the arrangement of Medusa wallet + Medusa account provides hosted storage of a user wallet's private key.
In the meantime, I don't think the conversation #1344 (comment) is resolved just because "two weeks" went by... in fact, the lack of agreement so far between you & @SebastienGllmt requires that it be left open (and therefore I'm reopening it): particularly to ensure other reviewers can put the recent blog posting, especially its final section, into context.
@rphair, yeah, I'll probably copy the article to Medusa itself, adding it as a separate section. I was actually thinking about that too. Not even because of privacy concerns (I don't see an issue with Twitter knowing that someone following a bunch of crypto blogs read an article about a crypto wallet), but just for the sake of convenience and reliability. Plus, it would allow for navigation by paragraphs. The consent for key handling is included in the wallet creation form for one reason—it is not universal. When creating a wallet through Ledger or Keystone, this point will not be shown because it makes no sense there. The key management is done by an external device. In that case, indicating specifics about private key processing during account registration could confuse the user. I can add a checkbox with a link to this material (hosted on Medusa) during registration, like, "I acknowledge the principles of the wallet's operation... etc." Similar to a public offer or terms of use. As for But I won't be able to work on it until next week. Right now, I'm busy with the Keystone Wallet integration, and in a couple of days, I'll have other urgent matters to attend to. |
I appreciate @SebastienGllmt’s perspective and recognize the importance of thoroughly vetting projects, especially on matters of security or the wallet category as such. (In the past, several strange wallets have been found and prevented in this way.) However, it’s essential for the discussion to stay constructive and respectful, as outlined in our code of conduct. Given the time without a response, I believe it’s important to close this conversation soon if no further engagement occurs. @Fell-x27 has made substantial efforts to address feedback, and I hope the final steps, particularly around transparency in key storage practices, will satisfy the necessary requirements for inclusion. |
@Fell-x27 would you please either update us on your commitments from #1344 (comment) or resolve the merge conflicts in this branch (resulting from other merges into upstream |
I'll do both in a week or two. There were some family-related problems, but now I'm back in the saddle again. |
Checklist
yarn build
after adding my changes without getting any errors.yarn.lock
(or have removed these changes).Showcase addition
wallet