This repository has been archived by the owner on Jan 30, 2024. It is now read-only.
@osdnk/t #119
Open
@osdnk/t #119
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: codyborn <codyborn@outlook.com>
## Overview This PR includes changes relating to storing private shares and signers in the Async Storage. I decided to put the storing logic inside the `Wallet` entity. This is a different solution that is known from other wallets, but I believe this is a reasonable approach here because, together with signers and shares, we will also introduce biometric storage. Therefore, we have a bit more data to store and we should not rely on wallets to integrate with every storage properly. I decided to make the approach modular. I decided to create abstract classes for a different type of storage (for private keys and for signers) and immediately follow up with an opinionated implementation tailored for React Native. However, from the user's perspective, that is perfectly fine to replace our implementation with any other, better suited for the product. ### Signers storage That is the store with public information about signers connected with the given account. Those are stored in the Local Storage, being a default (although maybe suboptimal) solution for React Native Projects. ### Private keys storage That is the storage, which is designed to include sensitive information. Namely, we store private shares (secp256k1 shards), which should be hidden under some security measures. Temporarily, I did that also with Local Storage, which is _the wrong_ solution. However, I did it to avoid dealing with the technicalities of native code with accessing the keychain. If that PR is approved and we will have 100% confidence around every piece of the architecture, we will proceed with purely technical work of migrating those data into secure storage. ### `CapsuleReactNativeWallet` and `CapsuleBaseWallet`. That is purely aesthetical change, but it's worth motivating. Since we are not opinionated toward any storage implementation and allow for replacing our implementations and predicting support for different platforms, we extracted out RN-related components. We created the `CapsuleBaseWallet` abstract class with some methods (RN-specific) implemented in the class `CapsuleReactNativeWallet`. ### `BiometricSessionManager` Biometrics-related things were extracted out with opinionated stuff moved to `CapsuleReactNativeWallet` ## Testing In order to test it, please navigate to `contracts.ts` and uncomment `await testSigning(newWallet)`. After opening the app for the second time, it should be possible to sign the transaction immediately (and see the output in the terminal)
## Motivation Now, we need to migrate from a hardcoded user-id to a generic mechanism. ## Changes First, note that we created a workaround to accept any code to verify a user whose email ends with `@test.usecapsule.com`. We did it to replicate the flow of creating users in the Kolektive without the tedious (and pointless) work of creating temporary react components for inputting email and verification codes. Obviously, this will need to change while making a real integration. Similarly, as in #6 and #7, I added an extra abstract method to be implemented in an actual wallet class and provided a default implementation for React Native which relies (again) on `AsynsStorage`. We assume that we can access the correct user-id before initiating the wallet instance. Similarly, our decision and design can be easily altered with overring methods. ## Cookies We needed to move around a few things to ensure that while initializing sessions manager, we already know the userId
## Motivation We want to make the complete flow work with cookies refreshing if needed. ## Changes I decided to make a different approach than previously. Before, I was expecting to analyze cookies on the front end; if it's outdated, we renew the cookie and take action. Obviously, on top of that, we still had to prepare the logic to prevent the case if the cookie expired in the meantime and the server rejected us due to cookie expiration. The motivation for that was not to make the request that would certainly fail if we knew upfront that the cookie was expired. However, for the sake of MVP, we are OK with one more request, and I decided to always optimistically try to make a request, and only if the backend rejects the cookie, we make the renewal. That solution is enormously more straightforward because now we avoid all the "manual" cookie parsing, and we can safely rely on built-in React Native logic for cookie manipulation. We don't even directly store the cookie. In the further iteration, we should implement analyzing cookies on the front end. We now have a wrapper for server fetches that automatically renews sessions if the wrapped function fails due to the cookies policy. ~I cannot test it now due to https://capsule-labs.slack.com/archives/C042W1BSG59/p1672606731886599~ To test it, I added a 5 min delay before pre sign of tx to ensure the cookie outdated ` await new Promise((r) => setTimeout(r, 300000))`
* Expose new methods * address: string,
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
A few sentences describing the overall effects and goals of the pull request's commits.
What is the current behavior, and what is the updated/expected behavior with this PR?
Other changes
Describe any minor or "drive-by" changes here.
Tested
An explanation of how the changes were tested or an explanation as to why they don't need to be.
How others should test
Does this need to be tested by QA in the next release cycle? If so please give a brief explanation of how to test these changes.
Related issues
Backwards compatibility
Brief explanation of why these changes are/are not backwards compatible.