Skip to content

Conversation

@k-moonblade
Copy link
Contributor

@k-moonblade k-moonblade commented Jul 4, 2022

Project Abstract

Choko Wallet is a user-friendly multi-chain crypto wallet for the Polkadot ecosystem. The Polkadot ecosystem has been known for its complicated nature to interact with. Therefore, we want to build a user-friendly and easy to integrate wallet for all projects in the Polkadot ecosystem.

For which grant level are you applying?

  • Level 1: Up to $10,000, 2 approvals
  • Level 2: Up to $30,000, 3 approvals
  • Level 3: Unlimited, 5 approvals (for > $100k Web3 Foundation Council approval)

Application Checklist

  • The application template has been copied, renamed ( project_name.md) and updated.
  • I have read and understood the FAQs, application guidelines and announcement guidelines.
  • A BTC, Ethereum (USDT/USDC/DAI) or Polkadot/Kusama (aUSD) address for the payment of the milestones is provided inside the application.
  • I have read and acknowledge the terms and conditions.
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The initial PR contains only one commit (squash and force-push if needed).
  • The grant will only be announced once the first milestone has been accepted.
  • I prefer the discussion of this application to be in a private Element/Matrix channel. My username is: @_______

How Did You Hear About our grants program?

  • Social Media
  • Hackathon
  • Personal Recommendation
  • Substrate Builders Program
  • Investor/VC
  • Online Search
  • Other: _______

@semuelle
Copy link
Member

semuelle commented Jul 5, 2022

Thanks for the application, @RoyTimes.

I agree that wallets in the ecosystem still have a way to go regarding user experience. But as a user, I'd be rather worried about the use of localStorage. For example, the Cookie AutoDelete extension on Firefox is set to delete localStorage by default when you leave a site. Do you know of any similar wallets that might offer some insight into the safety, use or misuse of such a wallet or some of the features you are planning to implement? I remember Burner Wallet using cookies, but that wallet is clearly marketed as for temporary use.

@k-moonblade
Copy link
Contributor Author

@semuelle as I have mentioned in the application, Near Wallet w/ code base is handling the private key this way and create a very smooth user experience. As a matter of fact, the storage items in the localStorage are not even encrypted.

Given that the wallet is the most popular Near wallet, I think It's far more dangerous for uneducated users to misuse the product (or prevent them from trying to understand the problem at all). As I said in the application, we are at the more "easy to use" end for spectrum of wallets over "secure".

@Noc2 Noc2 self-assigned this Jul 5, 2022
@semuelle
Copy link
Member

semuelle commented Jul 6, 2022

Thanks for the reply, @RoyTimes. I felt like the application wasn't quite clear on the "easy to use" part, but I tested the NEAR wallet and found the balance between usability and warning messages quite good.

Regarding the SDK and the SDK standard: if I understand correctly, you want to build a wrapper around @polkadot/extension-dapp. However, there have been a number of other projects building "generic" Substrate wallet SDKs, like WalletConnect v2, WalletBeacon or talisman-connect. Have you put any thought into (a) adopting one of the other libraries and (b) how to create a standard that might be adopted by others, such as posting on a forum for discussion of such? Usually, when one party decides to create a standard, it often just adds a standard to the list of existing standards.

EDIT: also still very much a draft, but might be useful: w3f/PSPs#29

@k-moonblade
Copy link
Contributor Author

k-moonblade commented Jul 6, 2022

@semuelle

Yup. We like NEAR wallet so we want it to work beyond the near ecosystem.

For SDK, we will not create a wrapper around @polkadot/extension-dapp nor want to build any standard for connector. It would be quite straightforwards: the DApp will be redirected to the wallet with URL (i.e. https://wallet.url/sign?transaction=XXXXX), where XXX is the encoded form of the transaction parameter. The wallet will then initiate a transaction with pure @polkadot/api. I have created a sandbox to demonstrate this (also included in the application): https://github.com/RoyTimes/wallet-sandbox-demo . For any parts regarding the "standard" for SDK, think about how teams submit PR to @polkadot/app to add in their network URL, color schema and custom types.

Ref to: https://github.com/RoyTimes/wallet-sandbox-demo/blob/master/wallet/src/App.js For wallet side
https://github.com/RoyTimes/wallet-sandbox-demo/blob/master/dapp/src/App.js for DApp side.

Copy link
Contributor

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the application. I have a couple of questions:

  • Could you integrate the “Features: In Form of {Milestones}_{Features} - {Description}:” into the deliverables/tables of the milestones, because the milestones are actually what we evaluate as part of the deliveries and according to the contract?
  • Did you consider using the social recovery pallet for your project?
  • Could also add the localstorage part into the milestone?
  • Regarding browser encryption for localstroage, I worked on something similar a long time ago. Maybe this is helpful for you: https://github.com/PACTCare/Dweb.page/tree/master/src/js/crypto, but I’m not up to date regarding the latest status of it and browser support.

@Noc2 Noc2 added the changes requested The team needs to clarify a few things first. label Jul 6, 2022
@k-moonblade
Copy link
Contributor Author

k-moonblade commented Jul 6, 2022

@Noc2

  • Could you integrate the “Features: In Form of {Milestones}_{Features} - {Description}:” into the deliverables/tables of the milestones, because the milestones are actually what we evaluate as part of the deliveries and according to the contract?

Done. Sorry for the confusion for the original structure.

Not yet and it sounds like a good idea. I'm including it into future plans for now. There are tons of great features like this one and we would submit additional grant application for them.

  • Could also add the localstorage part into the milestone?

Done. Now, milestone 1 explicitly says that we will store encrypted user key in localStorage.

For cryptographic, we would use the crypto wrappers we created with SkyeKiwi Protocol @skyekiwi/crypto - tweetnacl style crypto primitives instead of the WebCrypto API. And nothing unencrypted escape local tab memory unless the users want them to (i.e. setup email recovery, setup on another device). Might gotta add additional sr25519 ECDH from @polkadot/wasm-crypto to @skyekiwi/crypto tho. Let me know if you'd like me to include these explicitly in the application.

Copy link
Contributor

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. Feel free to also integrate the crypto part. I’m happy to mark the application already as ready for review. However, my recommendation would be to also remove milestone 3 for now. On the one hand the “DeFi Integration” isn’t very clear at this stage on the other hand it would be easier to accept the grant at a lower price, see/experience an initial version of the wallet and then potentially continue to fund it via grants or treasury. In general, if possible feel free to reduce the price a little bit more. The discussion in the committee will probably be about if it makes sense to fund another wallet, so a lower price always helps with this decision.

@Noc2 Noc2 added ready for review The project is ready to be reviewed by the committee members. and removed changes requested The team needs to clarify a few things first. labels Jul 7, 2022
Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For SDK, we will not create a wrapper around @polkadot/extension-dapp nor want to build any standard for connector. It would be quite straightforwards: the DApp will be redirected to the wallet with URL (i.e. https://wallet.url/sign?transaction=XXXXX), where XXX is the encoded form of the transaction parameter. The wallet will then initiate a transaction with pure @polkadot/api. For any parts regarding the "standard" for SDK, think about how teams submit PR to @polkadot/app to add in their network URL, color schema and custom types.

In that case, I would agree with @Noc2 and suggest to remove M3 and reduce the price accordingly.

@k-moonblade
Copy link
Contributor Author

@Noc2 @semuelle Done. Please review. Thanks.

Noc2
Noc2 previously approved these changes Jul 8, 2022
Copy link
Contributor

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. I’m happy to go ahead with it, even though it would be nice if you could reduce the price a little bit more. But let's wait what the rest of the committee say

semuelle
semuelle previously approved these changes Jul 8, 2022
Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not particularly keen on the wallet having a pallet dependency, but—if I understand correctly—all but the linkdrop will work without the pallet. Also, I think a web wallet could be really beneficial, so I'm happy to approve.

laboon
laboon previously approved these changes Jul 15, 2022
Copy link
Collaborator

@laboon laboon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit wary of some of the features of the wallet (e.g. email seed recovery), but I also realize that I am not the target audience. And anything that can help with ease of use of the network, I am generally in favor of - it's a common complaint.

@k-moonblade
Copy link
Contributor Author

@alxs @cruikshankss Please take a look. Feedbacks welcomed. Thanks.

@ashlink11
Copy link
Contributor

@alxs @cruikshankss Please take a look. Feedbacks welcomed. Thanks.

Hi @RoyTimes, I have had a look at your application. Wallet security is one of the most complicated topics for me to study and think through & often takes me quite a while to understand all the wallet ideals that need to be met to make a truly secure wallet. I've actually already made a note a couple days ago to ask my team some questions to learn more about wallet security on our team call next week (specifically regarding your application). If you have any more insight on the ratio of all wallet ideals / the ideals you plan to achieve with this grant, that would be highly appreciated. Thank you!

Copy link
Contributor

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @RoyTimes

I have a couple of questions:

  1. How will you connect to the networks? Are you going to run your own node or will you use the public endpoints?
  2. In case you're not running your own node(s), why limit the wallet to connect to just a few networks? It's not a lot of extra work to add the other networks and endpoints, if you make this part of your wallet configurable (which I'd assume you'd do anyway):

    a almost blank dashboard but allow switching between networks (support at least Polkadot, Kusama and SkyeKiwi Network)

  3. Since you're going to fork the existing NEAR wallet, I think 9 months (6 FTE x 1.5 months) is a rather high estimation for the completion of M1. I'd assume, you'll be able to reuse most of the logic. Feel free to correct me, if that's not the case.

@k-moonblade
Copy link
Contributor Author

@cruikshankss The very core of this grant application is to bring an very easy to use wallet for the Polkadot ecosystem, which prioritize usability over security. This was made in response to the common criticism on the Polkadot ecosystem for hard to use and hard to understand the system. I made an argument earlier on that the NEAR wallet that we fork is a semi-centralized non-secure wallet but gain huge adoption and gain the blockchain a reputation for easy to use. We build this wallet to serve the under-served users for the Polkadot ecosystem with very little assets at stake but want to play with the ecosystem. For tightly secure wallets, there are good options like Parity Signer, Ledger Wallet with PolkadotJS Extension. And we are filling the gap for less security demanding users with a decently secure wallet option.

@k-moonblade
Copy link
Contributor Author

@takahser

  1. You are right. We would user public endpoints and configuration from PolkadotJS App.
  2. It will be mostly UI/UX concerns. We don’t want it to be overwhelming when a user click the network selection drop down and see a long list of network to choose from. Moreover, we won’t be sure how the color schema will fit well on our UI (i.e would a specific color of a network blends in with the buttons and make the webpage looks bad). It will be a lots of work on the visual side but indeed not a lots of work on the data flow underneath. We also want to create some pre-built transaction calls into some networks in the SDK (i.e. teleport assets from Polkadot to XXX network) which will be part of the network configuration.
  3. That won’t be the case. Near wallet is not a completely decentralized product, therefore, it offloads some of its logic to their private server. Because of the nature of the network, lots of the actions are hardcoded to wire to specific contracts on the network. The network selection is rather fixed because there are only a Testnet and a Mainnet of NEAR, which means lots of the underlying data flow is hard-coded as well. There is not password to encrypt the seed. Etc. Our engineers have been reviewing the code base for more than a week now and we made the decision to start fresh and port over UI and functionalities as needed. Most of the dataflow will be completely reworked.

@takahser
Copy link
Contributor

@RoyTimes thanks for your answers.

  1. I understand your concern. But maybe you could add an advanced mode, where users are allowed to add custom networks. Maybe with a disclaimer, that it's still in some kind of test/beta stage and hence the colors may not perfectly match. Just an idea. Anyway, I think it'd be a shame not to add the capability to add more networks when technically you can implement it with minimal additional effort since it's mostly a matter of configuration.
  2. Does that mean you will do a complete rewrite? My understanding of "forking the repo" was, that you would build on top of it. Mostly keeping the UI, but replacing all the bindings and logic.

@k-moonblade
Copy link
Contributor Author

  1. Sure. We will experiment with it and ship it maybe with M2. I agree, it's not technically additional effort at all. But can I not include it in the delivery contract mostly on UI/UX concerns. Will add a note on delivery if we actually sort it out (kinda as a soft promise :)
  2. By complete rewrite - the near wallet had a big mixture of redux logic with routers. As they keep adding features to the wallet, the state management came to a point where it takes more effort to create something from new than re-use the state management code. And the naming convention is horrible (i.e. there are SetupRecoveryMethodWithRouter SetupRecoveryImplicitAccountWrapper SetupPassphraseNewAccountWrapper CreateImplicitAccountWrapper SetupSeedPhraseWithRouter SetupImplicitWithRouter for account creation with big pile of redux mixture). As I said, tons of hard coded contract address, hard coded account actions. UI of the wallet is not complicated and we are actually porting over but replacing bindings and logic does not worth the effort than creating from scratch with reference to the original repo.

@takahser
Copy link
Contributor

@RoyTimes

  1. in that case, could you add this information to the application document?

  2. Quote:

    the state management came to a point where it takes more effort to create something from new than re-use the state management code.

    Did you mean to write "less effort" here?;) I agree that all these Wrappers and the tight coupling with the Router are red flags, so a rewrite seems sensible. Maybe you can make this a bit clearer in the application doc.

In general, I still think M1 is a bit pricy for connecting to some RPC nodes, signing transactions using an already-existent library, and the corresponding UI work, that you can copy over from the NEAR repo (just talking about the template & styles, not the state management and business logic parts). But given the value proposition of a user-friendly web-based wallet, I'm willing to approve if you update your application doc in correspondence to what we've discussed in 1 & 2.

@CLAassistant
Copy link

CLAassistant commented Jul 18, 2022

CLA assistant check
All committers have signed the CLA.

@alxs
Copy link
Contributor

alxs commented Jul 19, 2022

I agree that a modern wallet for the Polkadot ecosystem would ideally be multi- and cross-chain first. I also think it would make sense to deliver some version of a polkadot.{js}-color scheme or custom node solution with the grant, but feel free to leave it as optional as discussed with @takahser.

As for the linkdrop pallet, I think very similar functionality was implemented in this grant and you may be able to reuse the code from their deliveries. The use case is different, but the requirements seem basically the same. Could you look into it?

I also agree that the first milestone seems very expensive, especially since you'll be able to reuse a lot of functionality from NEAR wallet and polkadot.{js}. I would actually like to ask you to reduce the price, also for M2 if you can indeed reuse the existing code I shared. But feel free to wait for the 5th approval by somebody else instead.

Could you please also sign the T&Cs again, since we updated them yesterday?

@Noc2
Copy link
Contributor

Noc2 commented Jul 28, 2022

@RoyTimes any updates here? @alxs and @takahser are waiting for your reply.

@takahser
Copy link
Contributor

@RoyTimes sounds good. In that case, feel free to ping us again when you've updated the application.

@k-moonblade k-moonblade dismissed stale reviews from laboon, semuelle, and Noc2 via 4c46f31 August 2, 2022 12:54
@k-moonblade
Copy link
Contributor Author

Hi All. CLA Signed.

TL;DR for updates:

  1. A detailed description on request handling mechanism (a website wallet version of wallet RPC). Current draft implementation included.
  2. A detailed reasoning of why not just fork the Near Wallet and why it will be a lots of work for M1.
  3. Moved some of the M2 items to M1.
  4. Added to M2: include a built-in DAPP to teleport limited number of assets between Parachains/relay chains.
  5. Added to M2: multi-chain assets balance viewer.

Please review and comment @takahser @semuelle @Noc2 @laboon @alxs Thanks!

Copy link
Contributor

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update. I’m still happy to go ahead with it.

@Noc2 Noc2 requested a review from laboon August 3, 2022 19:41
@laboon
Copy link
Collaborator

laboon commented Aug 9, 2022

Sorry for the delay, not sure how I missed the email that a re-review was requested.

Copy link
Contributor

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RoyTimes thanks for the update. Happy to go ahead with it as well.

@k-moonblade
Copy link
Contributor Author

@semuelle In case you missed this. Please review.

@k-moonblade
Copy link
Contributor Author

@semuelle @cruikshankss Hi, in case you missed the notification. Please review at your convenience. Thanks!

@Noc2 Noc2 merged commit defdf09 into w3f:master Aug 25, 2022
@github-actions
Copy link
Contributor

Congratulations and welcome to the Web3 Foundation Grants Program! Please refer to our Milestone Delivery repository for instructions on how to submit milestones and invoices, our FAQ for frequently asked questions and the support section of our README for more ways to find answers to your questions.

Before you start, take a moment to read through our announcement guidelines for all communications related to the grant or make them known to the right person in your organisation. In particular, please don't announce the grant publicly before at least the first milestone of your project has been approved. At that point or shortly before, you can get in touch with us at grantsPR@web3.foundation and we'll be happy to collaborate on an announcement about the work you’re doing.

Lastly, please remember to let us know in case you run into any delays or deviate from the deliverables in your application. You can either leave a comment here or directly request to amend your application via PR. We wish you luck with your project! 🚀

@keeganquigley
Copy link
Collaborator

Hi @RoyTimes how is milestone 2 coming along?

@keeganquigley
Copy link
Collaborator

Hi @RoyTimes just a friendly reminder, I see you are still actively working on the project but just curious if you can give us any updates on M2 since it has been almost 6 months. Thanks!

@k-moonblade
Copy link
Contributor Author

Hi @RoyTimes just a friendly reminder, I see you are still actively working on the project but just curious if you can give us any updates on M2 since it has been almost 6 months. Thanks!

Hi. Sorry for the delayed response. Yes. We are very actively working not the project. The actual plan is indeed a bit different than the original plan tho.

So for optimized user experience and security, we have made several major changes to make:

  1. No need for gas fee for new users.
  2. No need to keep track of seed phrase but only login with an OAuth provider.

Therefore,

  1. As previously concerned in the original application discussion - a wallet with a pallet dependency is not optimal but we have not been able to figure out a better solution than LinkDrop yet.
  2. For seedless features and the concerns about storing user secret in browser localStorage - we do have it resolved by using an MPC network to authenticate and generate/sign user messages for the user. Therefore, user will only need to store a shard of the whole keep and able to rotate the key pretty effortlessly. So, it creates a whole experience of log in with Google/Github then have everything set up in the wallet for the user.

However,

  1. We still have not draw out the optimal engineering arch for the LinkDrop pallet yet due to higher priority for the MPC node impl.
  2. We have spent the last 3 months to build an MPC node implementation from scratch and have the first version of it deployed this week. Because of the lack of underlying cryptography library support - the implementation only supports Secp256k1 for now and we want to bring Ed25519 support and integrate with Polkadot style chains. There are two major issues with MPC to go with this application: a. based on business decisions, it's pretty rare and hardcore tech and might be delayed in open sourcing it but we will eventually open source it. b. because it is licensed GPL3.0 from the same license for some of the underlying cryptography.

I think it might be a good idea to connect offline on Matrix for a detailed discussion if you are up to it. My id is "@songzhou:matrix.org".

@keeganquigley
Copy link
Collaborator

Thanks for the update @RoyTimes sounds good! Feel free to file an amendment to extend the timeline if the delivery will be further delayed. That way you can also integrate your new plans/changes and update the scope accordingly.

@keeganquigley
Copy link
Collaborator

Hi @RoyTimes we can connect on Matrix if necessary but I think it might be a good idea to go ahead and submit an amendment to add the changes above as well as extend the timeline. Let me know if you have any questions regarding this, we just want to make sure it is still being worked on.

@k-moonblade
Copy link
Contributor Author

@keeganquigley Sorry for the delay. We will compose an amendment, ideally, by end of next week.

@keeganquigley
Copy link
Collaborator

Thanks @RoyTimes do you still plan to submit this soon?

@keeganquigley
Copy link
Collaborator

Hi @RoyTimes pinging you once more. Please note that if we don't hear from you after 2 weeks, the committee will likely terminate the grant due to inactivity. Let us know if you have any questions.

@k-moonblade
Copy link
Contributor Author

Hi @RoyTimes pinging you once more. Please note that if we don't hear from you after 2 weeks, the committee will likely terminate the grant due to inactivity. Let us know if you have any questions.

Sorry for the delayed response. Just submitted an amendment to remove Milestone2 and try to mark this grant application as completed. Happy to discuss our plans further in the amendment PR!

Noc2 added a commit that referenced this pull request Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review The project is ready to be reviewed by the committee members.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants