search

JoinColony / purser

Interact with Ethereum wallets easily
https://joincolony.github.io/purser
MIT License
84 stars 21 forks source link

Common wallet interface fields are not very common #304

Open KiChjang opened 4 years ago

KiChjang commented 4 years ago

As we expand the scope on the kinds of wallets that we support on purser, some functionalities that are considered common denominators across all wallets are in fact applicable only to hardware wallets. Here are some of the fields that do not fit nicely to the interface:

  1. address

The getter function for this field is and should be common, but what's not common is the setter. We've actually sort of encountered this problem in the Metamask integration, whereby the address setter is only intended to be used by the StateObserver watching over Metamask state changes and not by users of purser -- in fact, the behaviour of setting the address by users is not known to me personally, and is possibly undefined. Other software wallet providers such as Authereum and Fortmatic do not expect users to have multiple addresses with their account, so it would make little sense for them to have an address setter.

  1. privateKey

To my knowledge, software wallet providers generally own the private keys and would not allow access to the private keys for the wallet.

  1. mnemonic

Same with privateKey, though I'm less certain -- I maybe wrong on this.

  1. keystore

The instantiation process for software wallet providers is usually not open to public. It is therefore weird and unnatural to be able to access internal fields of these software wallets from purser. It's also not clear to me what purpose a user would have to have access to the keystore string.

There may be more fields/methods, but these are the ones that I can list off the top of my head.

rdig commented 4 years ago

Common wallet interface fields are not very common

Due to no "real" standard existing here (not that it would make much sense), a truly common interface cannot exist, especially in libraries like purser, which try to unify very different implementations of wallets, with wildly different approaches to doing so.

applicable only to hardware wallets

I'm sorry but I'll have to disagree with you here. It's rather applicable only to software wallets, since most of what you listed below is strictly available only through purser-software, as all other wallets don't expose either the privateKey, mnemonic or keystore props

address

Yes, a setter is only available to the purser-metamask module, the others use the derivation path to get the address, and setting it by hand would not make sense. These do not apply to metamask, hence the need for a setter

privateKey To my knowledge, software wallet providers generally own the private keys and would not allow access to the private keys for the wallet.

If by "software" you mean metamask, or metamask-like wallets then yes, they don't, but otherwise, software wallets like ethersjs's one most certainly provides access to all values and ways of accessing said wallet instance: private keys, mnemonics, and the json keystore file

mnemonic

See above

keystore The instantiation process for software wallet providers is usually not open to public

Why? The purser library is intended to be used by developers to facilitate faster wallet integration within they're dapps. This is not intended to be used, directly, by end-users of said that.

What the developer chooses to allow or disallow as a wallet instantiation method is strictly up to them (Ie: we at Colony disallow both privateKey or keystore wallet instantiation, but that won't prevent purser-software from exposing that to the developer)

Also, please note that most (I think the docs need some updates) interface props in the docs list which ones you can expect in which wallet types: https://github.com/JoinColony/purser/blob/master/docs/_Interface_Common_Wallet_Interface.md#otheraddresses

Note: This prop is only available on Hardware Wallet types (Eg: Trezor).