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

[RCD-47] Fix 'hasSpendingPassword' metadata recovery during migration #3876

Merged

Conversation

KtorZ
Copy link
Contributor

@KtorZ KtorZ commented Nov 20, 2018

Description

When I create a wallet without a password on the 1.3.1 build and then upgrade\migrate to 2.0.0; the resulting wallet has a spending password enforced.

When migrating from an old data-layer, we don't have much information about whether there was a password defined on a wallet. So, we use to apply some heuristic based on the password last upate timestamp.

This is rather unreliable and can return misleading metadata for wallets which don't have passwords. Because there's no such thing as "no password" (secret keys are actually encrypted with an empty passphrase), it is possible to assess whether a key was encrypted with a password by trying to decrypt with an empty one. If it succeeds, then we can tell for sure that there's no password defined for the given key.

Linked issue

RCD-47

Type of change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🛠 New feature (non-breaking change which adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
  • 🔨 New or improved tests for existing code
  • ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

  • I have read the style guide document, and my code follows the code style of this project.
  • If my code deals with exceptions, it follows the guidelines.
  • I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
  • CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.

QA Steps

  • Create wallets using the legacy data layer with and without passwords
  • Migrate them to the new data layer
  • Query the API and control the hasSpendingPassword metadata attribute
[2018-11-20 14:05:21.03 UTC] Starting acid state migration
[2018-11-20 14:05:21.03 UTC] Starting wallet worker.
[2018-11-20 14:05:21.04 UTC] Found 2 rootAddress(es) to migrate.
[2018-11-20 14:05:21.11 UTC] Migrating HdRoot { id: HdRootId Ae2...8DD, name: With Password, hasPassword: updated 1542722721058256, assurance: normal, createdAt: 1542722721058256} with balance 0 coin(s)
[2018-11-20 14:05:21.15 UTC] Migrating HdRoot { id: HdRootId Ae2...jxk, name: Without Password, hasPassword: no, assurance: normal, createdAt: 1542722721113537} with balance 0 coin(s)
[2018-11-20 14:05:21.15 UTC] acid state migration succeeded. Old db backup can be found at ./state-demo/wallet-db/wallet-backup-2018-11-20T14_05_21

Screenshots (if available)

How to merge

Send the message bors r+ to merge this PR. For more information, see
docs/how-to/bors.md.

When migrating from an old data-layer, we don't have much information
about whether there was a password defined on a wallet. So, we use to
apply some heuristic based on the password last upate timestamp.

This is rather unreliable and can return misleading metadata for wallets
which don't have passwords. Because there's no such thing as "no
password" (secret keys are actually encrypted with an empty passphrase),
it is possible to assess whether a key was encrypted with a password by
trying to decrypt with an empty one. If it succeeds, then we can tell
for sure that there's no password defined for the given key.
@KtorZ KtorZ requested a review from dcoutts November 20, 2018 14:15
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@KtorZ KtorZ merged commit fce5cd0 into release/2.0.0 Nov 20, 2018
@KtorZ KtorZ deleted the KtorZ/RCD-47/fix-hasSpendingPassword-metadata-migration branch November 20, 2018 18:54
@KtorZ KtorZ mentioned this pull request Jan 4, 2019
12 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants