fix: improve multichain SDK to use ConnectionRequests as QRCodes#1347
Conversation
b1eef31 to
ace6bcd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1347 +/- ##
=======================================
Coverage 74.93% 74.93%
=======================================
Files 184 184
Lines 4513 4513
Branches 1105 1105
=======================================
Hits 3382 3382
Misses 1131 1131 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| "updateLink": any; | ||
| "updateExpiresIn": any; |
There was a problem hiding this comment.
would typing these as string and number respectively cause us problems downstream, or would it be okay ?
| * Abstract Modal class with shared functionality across all models | ||
| */ | ||
| export abstract class Modal<R = unknown, T = unknown> { | ||
| export abstract class Modal<R extends OTPCode | QRLink = OTPCode | QRLink, T extends ModalData = ModalData> { |
There was a problem hiding this comment.
Does it make sense to use more descriptive names for the generic parameters here (R and T)? Something like SignInConfirmationMethod for the first one, don't have a great name for the second one. No is an okay answer.
There was a problem hiding this comment.
Options + Data type though but improved them
| () => { | ||
| return new Promise<SessionRequest>((resolveSession) => { | ||
| return new Promise<ConnectionRequest>((resolveConnectionRequest) => { | ||
| this.dappClient.on('session_request', (sessionRequest: SessionRequest) => { |
There was a problem hiding this comment.
Could we use an enum for event names like session_request?
There was a problem hiding this comment.
on is currently not typed thats a good point, I think @chakra-guy is aware of this. It won't be with an enum though but it knows perfectly which events we have
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|



Explanation
Multichain SDK was previous using sessionRequests to generate QRCodes, during a while this was enough for us to start testing the Dapps (node, react).
Now the Mobile app can already establish connections with the Mobile wallet protocol relay server, and we need to integrate this ConnectionRequests into the multichain SDK so the QRCodes are created properly.
Difference is really not much (types extracted from MWP)
References
Checklist