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

[DEVOPS-1250] backport RCD-47 fix about 'hasSpendingPassword' metadata with extra sanity check #4112

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# CHANGELOG

## Cardano SL 3.0.1

### Fixes

- Fix inconsistent 'hasSpendingPassword' resolution from legacy data layer migration ([DEVOPS-1250](https://iohk.myjetbrains.com/youtrack/issue/DEVOPS-1250) [#4112](https://github.com/input-output-hk/cardano-sl/pull/4112))

## Cardano SL 3.0.0

### Fixes
Expand Down
14 changes: 7 additions & 7 deletions lib/configuration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14851,7 +14851,7 @@ mainnet_wallet_win64: &mainnet_wallet_win64
<<: *mainnet_full
update:
applicationName: csl-daedalus
applicationVersion: 13
applicationVersion: 14
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 1
Expand All @@ -14861,7 +14861,7 @@ mainnet_wallet_macos64: &mainnet_wallet_macos64
<<: *mainnet_full
update:
applicationName: csl-daedalus
applicationVersion: 13
applicationVersion: 14
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 1
Expand All @@ -14871,7 +14871,7 @@ mainnet_wallet_linux64: &mainnet_wallet_linux64
<<: *mainnet_full
update:
applicationName: csl-daedalus
applicationVersion: 13
applicationVersion: 14
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 1
Expand Down Expand Up @@ -14945,7 +14945,7 @@ testnet_wallet: &testnet_wallet
<<: *testnet_full
update: &testnet_wallet_update
applicationName: csl-daedalus
applicationVersion: 9
applicationVersion: 10
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 0
Expand Down Expand Up @@ -14984,7 +14984,7 @@ mainnet_dryrun_wallet_win64: &mainnet_dryrun_wallet_win64
<<: *mainnet_dryrun_full
update:
applicationName: csl-daedalus
applicationVersion: 22
applicationVersion: 23
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 2
Expand All @@ -14994,7 +14994,7 @@ mainnet_dryrun_wallet_macos64: &mainnet_dryrun_wallet_macos64
<<: *mainnet_dryrun_full
update:
applicationName: csl-daedalus
applicationVersion: 22
applicationVersion: 23
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 2
Expand All @@ -15004,7 +15004,7 @@ mainnet_dryrun_wallet_linux64: &mainnet_dryrun_wallet_linux64
<<: *mainnet_dryrun_full
update:
applicationName: csl-daedalus
applicationVersion: 22
applicationVersion: 23
lastKnownBlockVersion:
bvMajor: 0
bvMinor: 2
Expand Down
4 changes: 3 additions & 1 deletion wallet/src/Cardano/Wallet/Action.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import Cardano.Wallet.Kernel (PassiveWallet)
import qualified Cardano.Wallet.Kernel as Kernel
import qualified Cardano.Wallet.Kernel.Internal as Kernel.Internal
import qualified Cardano.Wallet.Kernel.Keystore as Keystore
import Cardano.Wallet.Kernel.Migration (migrateLegacyDataLayer)
import Cardano.Wallet.Kernel.Migration (migrateLegacyDataLayer,
sanityCheckSpendingPassword)
import qualified Cardano.Wallet.Kernel.Mode as Kernel.Mode
import qualified Cardano.Wallet.Kernel.NodeStateAdaptor as NodeStateAdaptor
import Cardano.Wallet.Server.CLI (WalletBackendParams,
Expand Down Expand Up @@ -68,6 +69,7 @@ actionWithWallet params genesisConfig walletConfig txpConfig ntpConfig nodeParam
let pm = configProtocolMagic genesisConfig
WalletLayer.Kernel.bracketPassiveWallet pm dbMode logMessage' keystore nodeState (npFInjects nodeParams) $ \walletLayer passiveWallet -> do
migrateLegacyDataLayer passiveWallet dbPath (getFullMigrationFlag params)
sanityCheckSpendingPassword passiveWallet

let plugs = plugins (walletLayer, passiveWallet) dbMode

Expand Down
57 changes: 53 additions & 4 deletions wallet/src/Cardano/Wallet/Kernel/Migration.hs
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
module Cardano.Wallet.Kernel.Migration (migrateLegacyDataLayer) where
{-# LANGUAGE LambdaCase #-}

module Cardano.Wallet.Kernel.Migration
( migrateLegacyDataLayer
, sanityCheckSpendingPassword
) where

import Universum

import Data.Acid.Advanced (update')
import Data.Text (pack)
import Data.Time (defaultTimeLocale, formatTime, getCurrentTime,
iso8601DateFormat)
import Pos.Crypto.Signing (checkPassMatches)
import System.Directory (doesDirectoryExist, makeAbsolute, renamePath)

import Formatting ((%))
Expand All @@ -15,10 +22,15 @@ import Pos.Crypto (EncryptedSecretKey)
import Pos.Util.Wlog (Severity (..))

import qualified Cardano.Wallet.Kernel as Kernel
import Cardano.Wallet.Kernel.DB.AcidState (UpdateHdRootPassword (..))
import qualified Cardano.Wallet.Kernel.DB.HdWallet as HD
import Cardano.Wallet.Kernel.DB.InDb (InDb (..))
import qualified Cardano.Wallet.Kernel.DB.Read as Kernel
import qualified Cardano.Wallet.Kernel.Internal as Kernel
import Cardano.Wallet.Kernel.Keystore as Keystore
import qualified Cardano.Wallet.Kernel.Read as Kernel
import Cardano.Wallet.Kernel.Restore (restoreWallet)
import Cardano.Wallet.Kernel.Util.Core (getCurrentTimestamp)

{-------------------------------------------------------------------------------
Pure helper functions for migration.
Expand Down Expand Up @@ -104,9 +116,7 @@ restore pw forced esk = do
rootId = HD.eskToHdRootId nm esk

let -- DEFAULTS for wallet restoration
-- we don't have a spending password during migration
hasSpendingPassword = False
-- we cannot derive an address without a spending password
hasSpendingPassword = isNothing $ checkPassMatches mempty esk
defaultAddress = Nothing
defaultWalletName = HD.WalletName "<Migrated Wallet>"
defaultAssuranceLevel = HD.AssuranceLevelStrict
Expand All @@ -132,3 +142,42 @@ restore pw forced esk = do
True -> do
logMsg Error ("Migration failed! " <> msg <> " You are advised to delete the newly created db and try again.")
exitFailure

-- | Verify that the spending password metadata are correctly set. We mistakenly
-- forgot to port a fix from RCD-47 done on 2.0.x onto 3.0.0 and, for a few
-- users, have migrated / restored their wallet with a wrong spending password
-- metadata (arbitrarily set to `False`), making their wallet completely
-- unusable.
--
-- This checks makes sure that the 'hasSpendingPassword' metadata correctly
-- reflects the wallet condition. To be run on each start-up, unfortunately.
sanityCheckSpendingPassword
:: Kernel.PassiveWallet
-> IO ()
sanityCheckSpendingPassword pw = do
let nm = makeNetworkMagic (pw ^. Kernel.walletProtocolMagic)
wKeys <- Keystore.getKeys (pw ^. Kernel.walletKeystore)
db <- Kernel.getWalletSnapshot pw
lastUpdateNow <- InDb <$> getCurrentTimestamp
forM_ wKeys $ \esk -> do
let hasSpendingPassword = case checkPassMatches mempty esk of
Nothing -> HD.HasSpendingPassword lastUpdateNow
Just _ -> HD.NoSpendingPassword
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, ok. So the Just case is when the password does match the empty password, hence NoSpendingPassword.

let rootId = HD.eskToHdRootId nm esk
whenDiscrepancy db rootId hasSpendingPassword restoreTruth >>= \case
Left (HD.UnknownHdRoot _) ->
logMsg Error "Failed to update spending password status, HDRoot is gone?"
Right _ ->
return ()
where
logMsg = pw ^. Kernel.walletLogMessage
whenDiscrepancy db rootId hasSpendingPassword action = do
case (hasSpendingPassword, Kernel.lookupHdRootId db rootId) of
(_, Left e) ->
return $ Left e
(HD.HasSpendingPassword _, Right root) | root ^. HD.hdRootHasPassword == HD.NoSpendingPassword ->
action rootId hasSpendingPassword
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, logic seems right here too.

Copy link
Contributor Author

@KtorZ KtorZ Mar 21, 2019

Choose a reason for hiding this comment

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

It's not entirely necessary but I thought it would be a good idea to only make updates to the DB when necessary.

_ -> -- Avoid making a DB update when there's no need
return $ Right ()
restoreTruth rootId hasSpendingPassword =
void <$> update' (pw ^. Kernel.wallets) (UpdateHdRootPassword rootId hasSpendingPassword)