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

Wrapping Aptos-Wallet-Providers for improving scalability #57

Open
Yoon-Suji opened this issue Jan 18, 2023 · 4 comments
Open

Wrapping Aptos-Wallet-Providers for improving scalability #57

Yoon-Suji opened this issue Jan 18, 2023 · 4 comments

Comments

@Yoon-Suji
Copy link
Contributor

Hi, I have a suggestion about the aptos-wallet-adapter.

For now, an AdapterPlugin must be injected directly under the window as the same name as wallet to detect the wallet. This way is especially uncomfortable for multi-chain wallets because they have to implement Aptos-only provider with the same name as wallet. (For example, in the case of a WELLDONE Wallet, we try to inject window.welldone_aptos provider to prevent conflicts with other chain's provider in the future. But to do so, we must be named WELLDONE_APTOS)

For the above reason, I'd like to suggest below:

  • Assign a property, such as window.aptosWallet or window.aptosProvider, to wrap Aptos Provider objects.
  • Inject Aptos Provider objects under that property. e.g. window.aptosWallet.welldone (I think it might also be in array form.)

In this way, the provider of the wallet is injected under the aptosWallet, so I think it will be convenient to support the aptos-wallet-adapter for multi-chain wallet builder.

In this way, I believe the aptos-wallet-adapter will be more scalable and able to support a wider range of wallets.

@banool
Copy link
Contributor

banool commented Jan 18, 2023

I think this proposal makes a lot of sense, thoughts @0xmaayan?

@0xmaayan
Copy link
Collaborator

Hi @Yoon-Suji , thanks for posting!

Could you elaborate on the proposal?

Do you mean any wallet to wrap its property with a aptosWallet property? for example, window.petra would become window.aptosWallet.petra? or do you imagine this wrapper would be only for multi-chain wallets?

On that topic, there is an open PR from a multi-chain wallet that also tries to handle this issue, what are your thoughts about it? #52
Basically, it adds a provider property to the wallet plugin and the adapter looking for either the name property or the adapter

@scottphc
Copy link
Contributor

There are different kind of wallet providers. For injected provider, this suggestion looks good for me. For SDK type, it won't be injected to window or under aptosWallet, so we also need to consider this case. We can refer to https://github.com/solana-labs/wallet-adapter & https://github.com/wallet-standard/wallet-standard.

@Yoon-Suji
Copy link
Contributor Author

Hey guys, thanks for all your comments! @0xmaayan @scottphc @banool

  1. I mean that wrapping every wallets including multi-chain wallet. Because I think it would be more intuitive to do so and more convenient to detect injected providers.

  2. About Support Loadable wallet #52 , I agree that adding providerName property is good solution and is simpler than my suggestion . However, I think that in the long term, wrapping the Aptos-Wallet-Providers is the more fundamental approach to prevent conflicts with other chains' wallet providers as additional chains and wallets are generated. (For example, Imagine using multiple chains' wallet-adapter in multi-chain dApp. It would be chaos.)

    And also, as all Aptos-Wallet-Providers implement the same standard, I believe it would be easier to understand and maintain if they were all grouped under the same parent(aptosWallet).

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

No branches or pull requests

4 participants