Native Module Refactor #473
Conversation
Funkatronics
commented
May 24, 2023
•
Funkatronics
commented
- Request Correlation, Handle Multiple Requests #474
- Establish React Request Completion API #458
- Native Module Cleanup #475
…-adapter into martini-native-module-refactor
|
marked as wip, need to update the PR with the new resolve API |
…nto martini-native-module-refactor
…nto martini-native-module-refactor
js/packages/mobile-wallet-adapter-walletlib/src/useMobileWalletAdapterSession.ts
Outdated
Show resolved
Hide resolved
| this@SolanaMobileWalletAdapterWalletLibModule.request = | ||
| MobileWalletAdapterRemoteRequest.SignMessages(request) | ||
| val request = MobileWalletAdapterRemoteRequest.SignMessages(request) | ||
| pendingRequests.put(request.id, request) |
There was a problem hiding this comment.
where does the .id field come from?
There was a problem hiding this comment.
its a default constructor parameter on the request. It defaults to a randomly generated UUID if no id is provided.
see here:
| SolanaMobileWalletAdapterWalletLib.completeSignPayloadsWithDecline(request.sessionId, request.requestId) | ||
| else if ((response as TooManyPayloadsResponse).failReason === MWARequestFailReason.TooManyPayloads) | ||
| SolanaMobileWalletAdapterWalletLib.completeSignPayloadsWithTooManyPayloads(request.sessionId, request.requestId) | ||
| else if ((response as AuthorizationNotValidResponse).failReason === MWARequestFailReason.AuthorizationNotValid) | ||
| SolanaMobileWalletAdapterWalletLib.completeSignPayloadsWithAuthorizationNotValid(request.sessionId, request.requestId) | ||
| else if ((response as InvalidSignaturesResponse).failReason === MWARequestFailReason.InvalidSignatures) | ||
| SolanaMobileWalletAdapterWalletLib.completeWithInvalidPayloads(request.sessionId, request.requestId, | ||
| (response as InvalidSignaturesResponse).valid) |
There was a problem hiding this comment.
Future Consideration:
Is it possible to combine the "failure response" handling into a single helper function then check for the failure response case at the top of each case?
Something like:
case MWARequestType.SignTransactionsRequest:
if (response.failReason) {
handleFailResponse(request, response)
break;
}
....
Guessing would be hard because you lose type signal on which exact completion to call. Not a blocker at all.
There was a problem hiding this comment.
yeah I have plans to improve all of this. I am working on a more robust serialization approach that will make it easier to pass objects back and forth across the bridge. With that, these switch statements will improve and possibly even go away.
| @@ -36,6 +36,13 @@ export enum MobileWalletAdapterServiceRequestEventType { | |||
| // Requests that come from the dApp for Authorization, Signing, Sending services. | |||
| export abstract class MobileWalletAdapterServiceRequest { | |||
There was a problem hiding this comment.
We can probably delete all these deprecated files, especially if the full example app UI is working now.
| val uri = Uri.parse(uriStr) | ||
|
|
||
| // TODO: this is dirty, need some stateful object/data to know what state we are in. |
There was a problem hiding this comment.
Definitely a TODO for now. Maybe this is just something we can follow up once we get feedback from first wallets that implement.
| authRequest.completeWithAuthorize( | ||
| publicKey.toByteArray(), | ||
| accountLabel, | ||
| null, // walletUriBase, |
There was a problem hiding this comment.
TODO: Fill in these null fields.
walletUriBase is the custom "scheme" wallets can return to dapps to use (not sure if anyone actually uses this lol)
authorizationScope is the auth key that we implement during ClientTrustUseCase. Maybe we can just add a dummy String/bytearray?
There was a problem hiding this comment.
agreed, tho lets tackle in a separate PR
| ) = launch { | ||
| checkSessionId(sessionId) { | ||
| Log.d(TAG, "completeWithAuthorize: authorized public key = $publicKey") | ||
| (pendingRequests.remove(requestId) as? MobileWalletAdapterRemoteRequest.AuthorizeDapp)?.request?.let { authRequest -> |
There was a problem hiding this comment.
Question: What happens if the type check here fails for some reason?
We've already executed `pendingRequests.remove(requestId). Does that get lost forever in the fail case? Is that even an issue?
There was a problem hiding this comment.
yeah so you are correct that the remove will still occur no matter what. Probably would be safer to only remove if we actually complete the request. I will update.
| private fun checkSessionId(sessionId: String, doIfValid: (() -> Unit)) = | ||
| if (sessionId == scenarioId) doIfValid() | ||
| else sendSessionEventToReact(MobileWalletAdapterSessionEvent.ScenarioError( | ||
| "Invalid session ($sessionId). This session does not exist/is no longer active." |
