starknet-react icon indicating copy to clipboard operation
starknet-react copied to clipboard

Improve connector abstraction

Open fracek opened this issue 3 years ago • 5 comments

At the moment the connector abstraction is very bare bone, consisting only of the InjectedConnector class (and the proposed testing connector in #54). Now we have a new wallet (see #77) and so it makes sense to improve what is available.

As a React library we have a few of design goals:

  1. Give maximum control to developers when it comes to the UI and UX of their application. Embedding a modal is sub-optimal IMHO, ready-made wallet pickers should be developed on top of this library and not the other way round.
  2. Support future non-injected wallets like Wallet Connect.
  3. Improve discoverability, so for example show wallets that the user has not installed but the developer wants to push.
  4. Support all kind of injected wallets, without the developer having to manually add them.

Point 3 and 4 are at odds so there's going to be tradeoffs.

fracek avatar Apr 24 '22 14:04 fracek

I think the best way forward is to continue improving on the connectors interface we have. There should be a way to list StarkNet wallets installed by the user (for example, have a global dictionary window.starknetWallet[id] => wallet) that we can leverage when building connectors. Then we would add:

  • Wallet specific classes (ArgentConnector, BraavosConnector, etc).
  • An helper function to automatically detect injected wallets.

Then we can update the current API to something like:

const connectors = [
  new ArgentConnector(),
  new CartridgeConnector(),
  new OtherConnector()
].filter(connector => connector.available())

<StarknetProvider connectors={connectors}>
  <MyApp />
</StarknetProvider>

The modal component would use the available connectors as follows:

const connectors = useConnectors()

<div>
{connectors.map(connector => (
  <button onClick={connector.connect}><img src={connector.iconUrl} /> Connect {connector.name}</button>
))}
</div>

I think having a global injected registry of wallets would also be beneficial to the wallets since they can control branding by providing an icon and homepage url.

fracek avatar Apr 24 '22 14:04 fracek

/cc @tarrencev @janek26 @avimak

fracek avatar Apr 24 '22 14:04 fracek

I believe that refactoring get-starknet "core" APIs into a dedicated package under the same repo (i.e. get-starknet/core) will answer most of the points raised above. flexibility, no UI, lookup installed wallets, discovery for available (non-installed) wallets, wallets order/include/exclude options for dapps, aligning wallet APIs for easier dapps<->wallet integrations, unified types, etc.

avimak avatar May 04 '22 14:05 avimak

I believe that refactoring get-starknet "core" APIs into a dedicated package under the same repo (i.e. get-starknet/core) will answer most of the points raised above. flexibility, no UI, lookup installed wallets, discovery for available (non-installed) wallets, wallets order/include/exclude options for dapps, aligning wallet APIs for easier dapps<->wallet integrations, unified types, etc.

great to hear the core is being broken out. how does get-starknet plan to work with non plugin/injected wallet providers? i.e. architectures similar to wallet connect?

tarrencev avatar May 04 '22 14:05 tarrencev

So I thought more about this and my conclusion is that InjectedConnector needs to be specific to a single injected wallet for it to work well. With the current abstraction it's impossible to escape the modal included in get-starknet. We should then provide an helper function to create a list of injected connectors based on what the user has installed.

The frontend developers are then responsible for:

  • displaying a message if the user has no browser wallet
  • design a modal window that accommodates multiple wallets

fracek avatar Jun 04 '22 22:06 fracek

This has been fixed in #215

fracek avatar Mar 04 '23 13:03 fracek