Skip to content
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 network bounds / is hardware to accounts.subscribe callback #1192

Open
1 task done
rossbulat opened this issue Jan 20, 2023 · 5 comments
Open
1 task done

Add network bounds / is hardware to accounts.subscribe callback #1192

rossbulat opened this issue Jan 20, 2023 · 5 comments

Comments

@rossbulat
Copy link

rossbulat commented Jan 20, 2023

  • Feature request

There is a limitation with hardware-wallet imported accounts that's not only subject to Polkadot JS extension, but to the injectedWeb3 accounts subscribe spec being used.

Ledger devices sandbox accounts to 1 network, and will therefore only work for that 1 network. The issue is that the network(s) an address is "bound to" are not communicated in the injectedWeb3 accounts.subscribe API. When I subscribe to the accounts of the extension, apps need to know the supported networks in order to display or ignore the account correctly, depending on the active network within the app.

Proposed solution would be to include optional hardware?: ledger_nano_s | <other> to signify account is from a hardware wallet, and networks?: Array<string> properties within the returned account list that returns a list of networks supported by the address. Perhaps the names adhere to the same spec as Ledger Substrate JS.

This would also be useful if we wanted to roll out Ledger support on only one network - say Polkadot, whereby the Polkadot-only accounts only use the Polkadot Ledger app & accounts will only be displayed when on that network.

A practical use case: The Polkadot Ledger app just rolled out nesting support for all the staking functions used in staking dashboard. Kusama does not support such yet - we need to therefore silo Polkadot hw support from Kusama hw support.

@rossbulat rossbulat changed the title Add network bounds to accounts.subscribe object Add network bounds / is hardware to accounts.subscribe object Jan 20, 2023
@rossbulat rossbulat changed the title Add network bounds / is hardware to accounts.subscribe object Add network bounds / is hardware to accounts.subscribe callback Jan 20, 2023
@jacogr
Copy link
Member

jacogr commented Jan 20, 2023

Generally the focus is on web3Accounts and web3AccountsSubscribe and it is the recommended approach to retrieve as opposed to the lower-level per-extension .subscribe which is actually not controlled in the web3* interfaces. (These are provided by all extensions themselves so it is a really low-level interface)

Both these web3* methods do allow the filtering capabilities, e.g. on genesisHash as per the options passed

export async function web3Accounts ({ accountType, extensions, genesisHash, ss58Format }: Web3AccountsOptions = {}): Promise<InjectedAccountWithMeta[]> {

export async function web3AccountsSubscribe (cb: (accounts: InjectedAccountWithMeta[]) => void | Promise<void>, { accountType, extensions, genesisHash, ss58Format }: Web3AccountsOptions = {}): Promise<Unsubcall> {

I agree it could be useful to somehow roll this out to eg. lower-level subscribe as well, however that obviously has some thinking required for backwards compatibility. (And obviously if different extensions implement their own subscribe, aka it is not polyfilled like we do for those that don't in web3Enable, it would actually be be quite a hit-and miss exercise.)

So maybe for the future if we can find some workarounds for the various incarnations.

@rossbulat
Copy link
Author

rossbulat commented Jan 20, 2023

Thank you, this is useful. The issue I faced with web3Enable (separate to the above) is that it summons all available extension pop-ups; this has prevented me from using it. Would an additional optional argument to the function allowing us to give IDs to a particular extension(s) to enable be too much of a stretch to add? Or an additional web3EnableById function so the existing ones are not touched?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@jacogr @rossbulat and others