walletlib + fakewallet updates #574
Conversation
# Conflicts: # android/walletlib/src/main/java/com/solana/mobilewalletadapter/walletlib/protocol/MobileWalletAdapterConfig.java
|
ran automated tests locally and passed. |
| @@ -353,14 +368,25 @@ class MobileWalletAdapterViewModel(application: Application) : AndroidViewModel( | |||
| } | |||
| } | |||
|
|
|||
| private fun clusterToRpcUri(cluster: String?): Uri { | |||
| return when (cluster) { | |||
| private fun buildAccount(publicKey: ByteArray, label: String, icon: Uri? = null, | |||
There was a problem hiding this comment.
Whats the need for this method if it just returns an AuthorizedAccount constructor?
| protected final String[] mFeatures; | ||
|
|
||
| @Nullable | ||
| protected final String[] mAddresses; |
There was a problem hiding this comment.
addresses is how the way for dApp to request authorization for certain bs58 pubkeys/addresses correct?
Does this act as a signal for the wallet to attempt to reauthorize the dapp for these addresses?
There was a problem hiding this comment.
yes, tho its not the same as MWA spec reauthorize. its a clue to the wallet as to what addresses the dapp is looking to use for the session.
| this.features = features; | ||
| } | ||
|
|
||
| public AuthorizedAccount(@NonNull byte[] publicKey, |
There was a problem hiding this comment.
What is the usecase for the two different constructors?
There was a problem hiding this comment.
displayAddress/Format are more a a multichain relevancy thing, so today no one will be using them. so adding the extra constructor is just a convenience
| @@ -333,75 +325,6 @@ private void doReauthorize(@NonNull MobileWalletAdapterServer.AuthorizeRequest r | |||
| authRecord.chain, authRecord.scope))); | |||
| } | |||
|
|
|||
| @Override | |||
There was a problem hiding this comment.
I thought reauthorize was still used internally? But here we're deleting it?
Can you outline the new flow for reauthorize from the consumer dapp -> wallet?
There was a problem hiding this comment.
Nevermind, answered my own question. authorize now calls doReauthorize under the hood, and this public reauthorize method isn't needed.
| // ============================================================================================= | ||
|
|
||
| // NOTE: future may be either AuthorizeRequest or ReauthorizeRequest | ||
| private void handleAuthorize(@Nullable Object id, @Nullable Object params) throws IOException { |
There was a problem hiding this comment.
This is the entrypoint for where the authorize request enters the wallet.
There was a problem hiding this comment.
yeah this is where we handle the RPC request
| @NonNull String chain) { | ||
| @NonNull String chain, | ||
| @Nullable String[] features, | ||
| @Nullable String[] addresses) { |
There was a problem hiding this comment.
Why is the authToken parameter gone from this AuthorizeRequest?
There was a problem hiding this comment.
it was always null, so it made no sense to pass it to the client. If the authToken is present, the reauthorize callback is triggered instead.
| @Nullable String chain, | ||
| @Nullable String[] features, | ||
| @Nullable String[] addresses, | ||
| @Nullable String authToken) { |
There was a problem hiding this comment.
authToken is present in this AuthorizeRequest
There was a problem hiding this comment.
the authorize method now takes an optional authToken param in the 2.0 spec, so that needs to be added here so that we can pass it to the scenario
This PR has been used to cut alpha builds quickly. This PR only touches walletlib (and fakewallet)
summary of changes:
MobileWalletAdapterServercompletely. The scenario still exposes reauthorize and handles authorize/reauthorize internally so things remain the same for consumers.Most relevant files:
MobileWalletAdapterSession.java(now handles session properties)MobileWalletAdapterServer.java(removal of reauthorize stuff, now all routes through authorize)LocalScenario.java(handles disambiguating authorize/reauthorize)