Closed berendsliedrecht closed 1 year ago
How does this work compare/relate to the work being done in the hyperledger/aries-acapy-wallet-upgrade project?
How does this work compare/relate to the work being done in the hyperledger/aries-acapy-wallet-upgrade project?
There is some overlap between the two. For the migration we have to do a generic (non-(aca-py|afj|af-go)-specific) conversion. This mainly converting the data to the new tables, but not the record transformation.
The record transformation is dependent upon the framework it will be used in, so that part is not implemented here and still has to be done in the framework itself.
The script from aca-py also handles postgres wallets and multi-tenancy properly, this one does not. However, when those fixes are applied in the future, the aca-py script can depend on this function for the preprocessing.
Hope that helps!
@andrewwhitehead There might be some duplicate code (related to kdf and more), but I wanted to modify as little of the rest of the library as possible. This means that removing the script would revert perfectly back to the state before the script without having to remove some code in specific places.
If there is some functionality I can use from askar, without changing anything, let me know.
Running into some issues with the tags. Shouldn't merge this yet.
I'm working on a few updates for this as well
@andrewwhitehead I merged your PR. Just one question about the name removal of _sqlite
as a suffix. Was there a specific reason for this?
@blu3beri I removed the _sqlite
suffix because it looks like we (with this change) wouldn't need to update the foreign function interface again when Postgres migration support is added.
I'm not sure why the sqlite rekey test seems to be failing sometimes on Linux. It's running into a database locked error when provisioning a new database with a random filename. It's not related to these changes.
I added the migration but not the conversion script to aries-askar itself.
It is added as a feature (default right now, but can be removed). After a while we can probably remove it from the default features as most people have migrated, but then it can always be opted-in of course.
I was working on the migration script for React Native Sqlite wallets and I was running into a lof of issues regarding sqlite and cryptography dependencies. Every sqlite dependency came with a list of issues and it since we can not patch them (as the sqlite dependency has to be a direct dependency of the wallet), it seemed unfeasible.
I tried to depend on the values / structs / functions from aries-askar itself as much as possible, but I might have created some duplicate code. @andrewwhitehead you can probably spot this a lot better than me here.
The python script could also depend on this function, but it is not required.
The script also only supports indy-sdk sqlite -> aries-askar sqlite. There is no additional support for multiple wallets (although the script can be be called multiple times for this to work, however untested). Postgres has been omitted for now as it did not fit the direct use case. I don't think that adding support for Postgres would be all too complex.
Curious to hear if there are any objections to this PR or if there is a very specific reason why it was not done for the Python script.
cc: @TimoGlastra @andrewwhitehead @swcurran
NOTE: It does not really have a test, yet. I would have to create an indy-sdk wallet and I am not too sure what the quickest way is to do that.