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

Properly implement 2.0 authorize method in MobileWalletAdapterClient #569

Merged
merged 7 commits into from
Oct 16, 2023

Conversation

Funkatronics
Copy link
Contributor

No description provided.

@Funkatronics
Copy link
Contributor Author

@creativedrewy this will unblock you but I am not loving this solution.

The MobileWalletAdapterClient in clientlib does not know about the session properties and because both chain and cluster are strings, there is no way to disambiguate the call.

some other ideas:

  • make a MobileWalletAdapterClient.authorizeV2 method that takes chain, and revert the MobileWalletAdapterClient.authorize method to use cluster so clients know exactly what they are calling.
  • make an Identifier or ChainIdentifier object instead of using strings for chains. This would help disambiguate a legacy authorize method from a new one that takes the chain object

@Funkatronics
Copy link
Contributor Author

Summary of discussions with @creativedrewy

These changes will work, tho may not be ideal.

clientlib.MoibleWalletAdapterClient exposes the new 2.0 authorize functionality, while preserving the legacy behavior. The downside of this is that a client might inappropriately call the 2.0 authorize on a 1.0 wallet endpoint. The client will need to be aware of the reported session properties and do the right thing. Ideally, we'd like the scenario/session to only expose the new 2.0 functionality when a 2.0 session has been established, and only expose 1.0/legacy functionality otherwise.

@Funkatronics Funkatronics changed the title Fix client chains/cluster usage Properly implement 2.0 authorize method in MobileWalletAdapterClient Oct 12, 2023
@Funkatronics
Copy link
Contributor Author

tests pass locally, flakiness on ci. merging @creativedrewy

@Funkatronics Funkatronics merged commit 1a327af into main Oct 16, 2023
5 of 6 checks passed
@Funkatronics Funkatronics deleted the fix-client-chains-cluster-usage branch October 16, 2023 19:33
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.

1 participant