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

feat: you can now configure the SolanaMobileWalletAdapter with an address selector #171

Merged
merged 1 commit into from
Jul 27, 2022
Merged

feat: you can now configure the SolanaMobileWalletAdapter with an address selector #171

merged 1 commit into from
Jul 27, 2022

Conversation

steveluscher
Copy link
Contributor

@steveluscher steveluscher commented Jul 26, 2022

Background

Mobile wallet adapter allows us to authorize multiple addresses from a single click of the ‘connect’ button. This causes an impedance mismatch between MWA and @solana/wallet-adapter that doesn't have the concept of multiple addresses.

Problem statement

The existing wallet-adapter API is single-key, like this:

const {publicKey, signTransaction} = useWallet();

How, in this world, should wallet-adapter make the choice of publicKey, if the user decides to authorize multiple addresses in a mobile wallet adapter session?

Summary of changes

  • Added a configuration param to SolanaMobileWalletAdapter called addressSelector. Developers can provide an implementation of this to make a sensible selection of address given multiple addresses.
  • Made a createDefaultAddressSelector helper, that just chooses the first address in the list.

Note, that because the selector is async, app developers have a great amount of freedom to pause execution of the wallet adapter plugin to show – for instance – an address selection UI to their users.

Addresses #44.

@steveluscher steveluscher requested a review from jordaaash July 26, 2022 06:19
// TODO(#44): support multiple addresses
this._authorizationResult.addresses[0],
);
this._publicKey = getPublicKeyFromAddress(this._selectedAddress);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing something here, but how can you reliably get a public key from an address? Addresses (if not on Solana) may not be reversible to pubkeys.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I presumed that the Solana wallet-adapter would only ever work with Solana wallets. We’d have to change the return type of useWallet, which right now only has the concept of publicKey.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's a fair assumption then! I think I was not seeing that the context of this code is wallet-adapter-mobile and not mobile-wallet-adapter

@@ -29,6 +24,10 @@ export interface AuthorizationResultCache {
set(authorizationResult: AuthorizationResult): Promise<void>;
}

export interface AddressSelector {
select(addresses: AuthorizationResult['addresses']): Promise<AuthorizationResult['addresses'][number]>;
Copy link

@jordaaash jordaaash Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a stylistic choice since the resolved type will be the same, but AuthorizationResult['addresses'], and definitely AuthorizationResult['addresses'][number], are less clear than their Base64EncodedAddress[] / Base64EncodedAddress counterparts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I think there are two positions you can take here:

  1. This is a thing that turns T[] into T
  2. This is a thing that takes ThisParticularType[] and turns it into ThisParticularType

My goal with making it a lookup on addresses was to make Typescript do more work for me; if the type changes then that change automatically flows through this function.

My implementation actually hits somewhere in between option 1 and 2, and maybe I should go for option 1 which just says ‘I’m going to give you a list of who knows what and you give me a single item back.’

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you like writing Java? Name every AsyncSelector { select<T>(items: T[]): Promise<T> }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like not writing types at all and making Typescript work for the money I spent on it.

Uh… wait.

@steveluscher steveluscher merged commit 103d52f into solana-mobile:main Jul 27, 2022
@steveluscher steveluscher deleted the address-selector-for-wallet-adapter branch July 27, 2022 01:08
Copy link
Contributor

@sdlaver sdlaver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better late than never. Approved!

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

Successfully merging this pull request may close these issues.

3 participants