search

novasamatech / parity-signer

Air-gapped crypto wallet.
https://vault.novasama.io
GNU General Public License v3.0
557 stars 167 forks source link

Make derivation system clear and self-explanatory to avoid further user's confusion. #833

Closed SvenMeyer closed 1 year ago

SvenMeyer commented 3 years ago

What am I missing ?

12ukAcCa1.... Parity Signer 16UX6GQU.... Browser-plugin & Polkadot.js

Slesarew commented 3 years ago

Are you sure the crypto algorithm in polkadot.js is set to sr25519? Which version of Signer is this? Did you check that derivation path matches? Old Signer was creating something like //polkadot by default, not sure what current browser plugin proposes.

Tbaut commented 3 years ago

This is 99% a derivation issue, as we got many such reports in the past

Old Signer was creating something like //polkadot by default

I wouldn't call it old, this is what's live, and what ppl have access to today! The Polkadot apps/extension, subkey, and any other wallet out there don't have derivation per default, unlike Signer.

TL;DR @SvenMeyer Parity Signer adds a derivation "//[netowrk]" by default when you create an account. Polkadot.js doesn't hence the different address. So either you should make sure to create a custom derived account from the root, with no derivation, or you should add this derivation manually in the extension, by adding at the end of the mnemonic //[network], and replacing [network] by kusama or polkadot.

SvenMeyer commented 3 years ago

@Tbaut Thanks for the insights. Looks like exactly as you described :

Parity Signer shows //polkadot next to the user icon - I would have never come up with the idea that this is a derivation path, I always thought it has to be in a format like m/44’/60’/0’/0

Looking into the Polkadot browser extension, I could not find any option for the derivation path (why should I bother anyway, in Metamask I do not have to deal with it as well, it 'just works' after entering a seed) ... but looking closer there was a "Advanced" dropdown which revealed an empty input field ... who, even an early adopter in crypto would know what to put in there (and there should be no need to bother in the standard use case).

@Slesarew Of course I was looking for a solution to secure my funds, now it looks really messy and with all the workarounds I don't think that at the moment it will make the setup more secure and prevent loss of funds better than just using the browser plugin. Although , the multi-sig approach might be an alternative, thanks for pointing that out.

I really appreciate the work which has been done, but I thought we would be further along the way to a secure setup , now that Kusama and Polkadot & parachains all go live and Millions, if not Billions are being handled ... what use is an outstanding blockchain if the funds get lost in the UI ?

(mass) user adoption quite far off ...

P.S.: Sorry for "just complaining" and not contributing code (at this stage), but happy to try out new releases and do QA.

Slesarew commented 3 years ago

@SvenMeyer please have a look at this document - IMO the best explanation of the derivations out there. https://substrate.dev/docs/en/knowledgebase/integrate/subkey Yes, these are different thing from the old good bitcoin format, they are confusingly similar, but that's because that Substrate's derivations are somewhat backwards-compatible with normal format yet more versatile.

Browser extensions system is being developed synchronously with Signer now (by pretty much same people in different roles), so some changes towards usability should be released soon as well. Thanks for your suggestions.

@AndreiEres @goldsteinsveta @vas3k calling you to read this user experience story. It's quite typical and we should have a clear and easy to find FAQ at least and self-explanatory UI/UX everywhere ideally.

Slesarew commented 3 years ago

P.S.: Sorry for "just complaining" and not contributing code (at this stage), but happy to try out new releases and do QA.

And these user reports are much more valuable for the project at this moment than any code contributions. Thank you.

krodak commented 1 year ago

I think this should be improved with new UI, let us know @SvenMeyer if this is still confusing in current v6.0 builds and share with us any feedback you might have regarding updated UI 🙏🏻

SvenMeyer commented 1 year ago

@krodak Ser, wen and where can I find Parity Signer v6 ?

krodak commented 1 year ago

@SvenMeyer

SvenMeyer commented 1 year ago

@krodak @dvdplm Thanks, an Android version would be great. Not every person working on building a new decentralised permissionless finance infrastructure is happy to use a centralised, permissioned, walled garden ecosystem.

Tbaut commented 1 year ago

I'll copy what I wrote elsewhere:

The latest build can be obtained in the artifacts from the github actions called "Build unsigned APKs". E.g here, scroll down https://github.com/paritytech/parity-signer/actions/runs/4514552449

Note that this shouldn't be used in production, it's only for testing as it has not been signed.

Tbaut commented 1 year ago

BTW, I've been saying it before, but having 1 release/year doesn't help anyone. If you're thinking, "This app should never be updated by ppl, so it's fine only releasing once a year", this is a false sense of the reality.

The truth is: users install the app whenever they want, or can. You will never be able to time this. On the other hand, someone installing the app the day before you release a new version gets rekt, because they're missing out a lot of fixes.

Signer/Vault, like any other app, should have frequent releases, so that users installing it get the latest state. There is no good reason to have been waiting for 1 year for this release.

Dmitry-Borodin commented 1 year ago

current release candidate version is available here https://drive.google.com/file/d/1ZgGWlM78Iv6QbDJffQjXecxkT3T61NV-/view?usp=share_link\

I hope in future we will release more often and important fixed will be available in play store much faster.

Beta channel for current master is not there for now.

krodak commented 1 year ago

@Tbaut completely agree, I think it’s slowly changes as we’ve already released some patch releases for 6.0 iOS app and will post more in coming weeks. I think most issues arise from the fact that currently there is no automatic way to do migration for local database if models change to new ones, but all UI / UX improvements and bug fixes will be rolling out in quick successions. If there are changes related to database structure, that’s a different story but I think we’ll need to tackle it at some point

Tbaut commented 1 year ago

I think most issues arise from the fact that currently there is no automatic way to do migration

If there are changes related to database structure, that’s a different story but I think we’ll need to tackle it at some point

I didn't mean it in this way. Users will have a version and stick to it. They won't upgrade often, or as often as you do. And that's ok. However, if you release often, at least no-one will get a super old release, like it's the case now.

There should never be an automatic way to migrate, users should wipe the device in any case. This makes the dev much easier, because you shouldn't care too much about breaking changes (beside the fact that there's a breaking change linked to the hot device, but this is super rare).

krodak commented 1 year ago

@Tbaut got it, I think most of changes that are happening in short term is related to UI / UX and not necessarily touching anything related to data storage and dev on Rust side, but still could be solving bugs / issues, leading to better experience and not require any actions from user.