search

polkadot-js / extension

Simple browser extension for managing Polkadot and Substrate network accounts in a browser. Allows the signing of extrinsics using these accounts. Also provides a simple interface for compliant extensions for dapps.
Apache License 2.0
971 stars 417 forks source link

Add Address Book function to the extension? #646

Open chrisli30 opened 3 years ago

chrisli30 commented 3 years ago

Hi there,

I don't find any reference in Issues so bring up the topic here. Have you considered to add Address Book function the Polkadot.js extenstion? The Address Book tab on polkadot-js/apps will lose contact data if browser cache is cleared which would be a huge hassle if the owner of the wallet isn't aware of it. I think Contacts is important because 99% of the time we are sending funds to the same people over and over again. Since polkdot.js is the official way to send funds on Web I feel like it could use more features like Contacts.

Me and my team will be happy to contribute code if you think it's a good feature.

Please let me know your thoughts. Thanks!

jacogr commented 3 years ago

It is not something I have thought about, but does make sense.

Couple of points here -

Contributions are always great.

@Tbaut Any additional thoughts?

Tbaut commented 3 years ago

I have thought about it very recently related to https://github.com/polkadot-js/extension/issues/628 as an additional security guarantee if we are able to show that an account we're interacting with is known and identified by the extension. I didn't think about the additional portability it allows, but it'd be nice as well. I can immediately see related "issues" that would follow such as the wish to export/import these in batch, which would bloat the interface, and need to be thought through.

As such, in terms of interface, I think it'd be very important to totally separate the owned account and the contacts in a dedicated view available from the menu (like the website management view) and have any button to add/remove contact there instead of from the main view. Having a distinct way of showing those accounts would also be good. With an icon like we do for external, or something even more visible.

chrisli30 commented 3 years ago
  • the ui-keyring does support addresses, so all should be good

Cool

  • we would need a new web3* interface to return these, this is simple - just needs to filter correctly for accounts/addresses

I haven't checked those interface but sounds straightforward.

  • when adding an entry, it would be good to check it against the phishing list as well

Yeah, I feel it a good feature. Is the phishing list managed by polkadot.js server, or something else?

  • like we have for accounts, would be good to tie these to specific networks

Yep, network could be an attribute of a wallet address, and their values are selected from a known network list.

  • we will need some filter on the top to swap between address/account view

Is that the same thing as @Tbaut's suggestion, that I think it'd be very important to totally separate the owned account and the contacts in a dedicated view available from the menu? Yeah I agree it should be in a separate tab so users don't confuse somebody else's address with one's own.

chrisli30 commented 3 years ago

I didn't think about the additional portability it allows, but it'd be nice as well. I can immediately see related "issues" that would follow such as the wish to export/import these in batch, which would bloat the interface, and need to be thought through.

I wouldn't worry about additional features for now. If people ask for it, it means the feature is wanted, and we can keep adding on top.

We are a team of 4 developers previously building on Ethereum ecosystem, a crypto wallet supporting multisig, Naming System and Wallet Connect for RSK specifically. I have been a DOT token holder since 2017 but didn't deeply research on Polkadot tech till Oct 2020. After reading everything I truely believe Polkadot will surpass Ethereum in 2 year's time, so we are here finding important challenges in the ecosystem to solve. Since we were working on an iOS&Andorid crypto wallet, we know that an Address Book is a feature user would want.

I have 2 questions,

  1. Is there any issue more urgent you need us to solve? For the first three months my goal is to get familiar with stuff so I'm open to any challenges. Also small issues would help verify our qualification.
  2. How should we proceed? I create a plan for the feature in the issue, get approved, and PR the solution code?

Thanks for the prompt response!

jacogr commented 3 years ago

The extension is really mostly seen as "feature-complete" atm. That doesn't mean it doesn't get anything new, but it does no have a huge backlog of items that "needs to be in". Over the last 2 months the stuff that have been asked for for ages has been done.

That doesn't means it is where it should be, not completely issue-free :) So really the drive is based on requests that make a lot of sense (like this one), or issues where people keep on asking (or worse), burning. It continues to evolve, but I must admit that here I'm feeling a bit less "cowboy" than in other places, since really trying to keep it focussed on a single task, and do that task really well.

We don't need to make a huge PR, actually I generally prefer if things are rather not done as massive one-off PRs. There is also some leeway here, since 99.9% only run off the published releases and not master, so a PR with some functionality (not 100% complete), doesn't land out n the wild, but is nicely available for testing.

So I'm guessing what I'm saying is the following - if we can break this up into bite-sized chunks first, really happy to go straight to PR there. This also gives familiarity with the codebase - not everything is great and it even takes me some time to "get into it" since I don't spend nearly enough time on it.

If I have to pick 1 existing glaring omission outside of this (which is actually both here and in the apps UI, i.e. it is a combo) is the ability to import/export all accounts on both sides. (As well as contacts). This is useful for 2 reasons -

chrisli30 commented 3 years ago

Jaco, agreed. The extension works well at the moment, but we can start from addressing the backlogs.

The batched import/export will be a neat feature to me. Let me dig into the code and start with that.

Thanks for the advice on the process. That's helpful.

chrisli30 commented 3 years ago

Happy Luna new year! : )

jacogr commented 3 years ago

Happy new year :)