Skip to content
This repository was archived by the owner on Aug 18, 2020. It is now read-only.

[CO-407] Remove legacy-wallet dependency from new wallet Migration #3836

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

uroboros
Copy link
Contributor

@uroboros uroboros commented Nov 6, 2018

Description

This PR changes the wallet migration code in order to remove the last dependency on the legacy wallet.

In order to achieve this, the following changes were made to wallet migration:

  • previously, we extracted metadata from the legacy DB for the purposes of migration. This included migrating a "default address" for each migrated wallets
  • instead of migrating from the legacy db, we now migrate all wallets that have an encrypted secret key in the wallet Keystore
  • however, this does not give us the wallet spending password, which prevents us from being able to create a default address for each migrated wallet
  • the solution is to remove the invariant that new wallets be created/restored with a default address (and instead make the address optional)
  • existing code paths that create/restore wallets can still create and provide a default address, while the migration code can omit the address for migration

Linked issue

https://iohk.myjetbrains.com/youtrack/issue/CO-407

Type of change

  • 🛠 New feature (non-breaking change which adds functionality)
  • 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc

Testing checklist

  • All new and existing tests passed.

QA Steps

Re-test wallet migration

@uroboros uroboros requested a review from KtorZ November 6, 2018 20:42
@KtorZ
Copy link
Contributor

KtorZ commented Nov 7, 2018

Manual testing, trying to migrate an old database full of genesis wallets with an extra wallet.

[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:21.96 UTC] Starting wallet(s) migration
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:21.97 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZ5YjF9WuDoWfCZLPQ56MdQC6CZa2VKwMVRVqBBfTLPNcPvET4, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521101974860} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:21.97 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZLYkm8WxTJUYcFe5r5MUTctmQ9N8JLo3H25a4c6DgJUVTBJB9, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521101979676} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:21.99 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZAmUBbrY5WyqKkDCotcfE2zqxdFG5GMRGT5q5DU6zkVjTudRo, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521101990334} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:21.99 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZLvVMHizH9GiKYEtMRN7jd4mDGRENTAinAvvWrH3FFRHfHvmm, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521101996314} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.01 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZ31wZ1ATCCNS8MdgkvWTxY7uwHRNyMAUp2BZPmB61Ne4x1WCH, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102004674} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.01 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZGMy2HXqBQ5v14g8ovcUo39D6znq7nZ5j4pwaqzfqoVSz8tAo, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102013379} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.02 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEYzAcda9sjDRRH5Xrf5er1VfzQ6eyAsjH899oP1DzwHS1RLMdW, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102022193} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.02 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEYw64wSoH85Ynwa3XNM1gyh6VDrYu5DFRK4d1NPtX5Are3NHWz, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102029492} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.04 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZEK5DvxPMtnTnUfQg8coWAAbNfLEvQ4GqWTe9h8d6AEkBDMce, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102038354} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.06 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZ3QWzPxjM4ai83iV129DiCnT4eSQGtjCpwmcvQFB5a8KsLheE, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102062146} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.06 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZA3tszYyNZsCGXadb63QPgCEdR4ZSsYWTrwew9JenNhedr3Sf, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102073059} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.08 UTC] Migrating HdRoot { id:          HdRootId Ae2tdPwUPEZCq7M3jAHupa1VRTVqBMH1M6b1y8Bm8GaJndfMxkVBqHqPUJC, name:        <Migrated Wallet>, hasPassword: no, assurance:   strict, createdAt:   1541521102084750} with balance 0 coin(s)
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.08 UTC] Migration succeeded, restoration of migrated wallets in progress...
[cardano-sl.wallet:Info:ThreadId 320][2018-11-06 16:18:22.08 UTC] acid state migration succeeded. Old db backup can be found at 
./state-demo/wallet-db/wallet-backup-2018-11-06T16_18_22
$ bash list-wallets.sh 
"<Migrated Wallet>"@"Ae2tdPwUPEYw64wSoH85Ynwa3XNM1gyh6VDrYu5DFRK4d1NPtX5Are3NHWz"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEYzAcda9sjDRRH5Xrf5er1VfzQ6eyAsjH899oP1DzwHS1RLMdW"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZ31wZ1ATCCNS8MdgkvWTxY7uwHRNyMAUp2BZPmB61Ne4x1WCH"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZ3QWzPxjM4ai83iV129DiCnT4eSQGtjCpwmcvQFB5a8KsLheE"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZ5YjF9WuDoWfCZLPQ56MdQC6CZa2VKwMVRVqBBfTLPNcPvET4"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZ9YfGu85TwZTWMMcufFbycRkACUTHn6i3kM3PBXU6nafNsQDN"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZA3tszYyNZsCGXadb63QPgCEdR4ZSsYWTrwew9JenNhedr3Sf"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZAmUBbrY5WyqKkDCotcfE2zqxdFG5GMRGT5q5DU6zkVjTudRo"
Balance: 37499999999166

"<Migrated Wallet>"@"Ae2tdPwUPEZCq7M3jAHupa1VRTVqBMH1M6b1y8Bm8GaJndfMxkVBqHqPUJC"
Balance: 0

"<Migrated Wallet>"@"Ae2tdPwUPEZEK5DvxPMtnTnUfQg8coWAAbNfLEvQ4GqWTe9h8d6AEkBDMce"
Balance: 37499999999166
$ ls state-demo/wallet-db/
wallet-acid  wallet-backup-2018-11-06T16_18_22  wallet-sqlite.sqlite3

On a second run:

[cardano-sl.wallet:Info:ThreadId 322] [2018-11-06 16:23:47.62 UTC] Starting wallet(s) migration
[cardano-sl.wallet:Info:ThreadId 322] [2018-11-06 16:23:47.62 UTC] No legacy DB at ./state-demo/wallet-db/wallet, migration is not needed.

@uroboros uroboros force-pushed the feature/co-407-remove-legacy-wallet-deps branch from a02e5b9 to f3c6e2b Compare November 7, 2018 09:31
@KtorZ KtorZ merged commit 7866f28 into develop Nov 7, 2018
@KtorZ KtorZ deleted the feature/co-407-remove-legacy-wallet-deps branch November 7, 2018 12:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants