From c8994fe175de382f2ffa3d34bea77912ee49a0fb Mon Sep 17 00:00:00 2001 From: Sale Djenic Date: Thu, 26 Jan 2023 21:03:01 +0100 Subject: [PATCH] test: `TestConvertAccount` test update This tests the entire process of converting a regular account to a keycard account and then converting that keycard account back to a regular account. For the need of this test I had to improve `DeleteAccount` function, cause the previous implementation didn't remove account from the keystore cache, but only from the keystore. --- account/accounts.go | 46 +------- api/backend_test.go | 207 ++++++++++++++++++++++++---------- api/geth_backend.go | 12 +- mobile/status.go | 4 +- services/accounts/accounts.go | 4 +- 5 files changed, 157 insertions(+), 116 deletions(-) diff --git a/account/accounts.go b/account/accounts.go index a02d6c71c..db2024801 100644 --- a/account/accounts.go +++ b/account/accounts.go @@ -8,7 +8,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strings" "sync" "github.com/google/uuid" @@ -607,47 +606,6 @@ func (m *Manager) ReEncryptKeyStoreDir(keyDirPath, oldPass, newPass string) erro return nil } -func (m *Manager) DeleteAccount(keyDirPath string, address types.Address, ignoreCase bool) error { - var err error - var foundKeyFile string - err = filepath.Walk(keyDirPath, func(path string, fileInfo os.FileInfo, err error) error { - if err != nil { - return err - } - if len(foundKeyFile) > 0 || fileInfo.IsDir() { - return nil - } - - rawKeyFile, e := ioutil.ReadFile(path) - if e != nil { - return fmt.Errorf("invalid account key file: %v", e) - } - - var accountKey struct { - Address string `json:"address"` - } - if e := json.Unmarshal(rawKeyFile, &accountKey); e != nil { - return fmt.Errorf("failed to read key file: %s", e) - } - - if ignoreCase { - if strings.EqualFold("0x"+accountKey.Address, address.String()) { - foundKeyFile = path - } - } else { - if types.HexToAddress("0x"+accountKey.Address).Hex() == address.Hex() { - foundKeyFile = path - } - } - - return nil - }) - if err != nil { - return fmt.Errorf("cannot traverse key store folder: %v", err) - } - - if len(foundKeyFile) == 0 { - return ErrCannotLocateKeyFile{fmt.Sprintf("cannot locate account for address: %s", address.Hex())} - } - return os.Remove(foundKeyFile) +func (m *Manager) DeleteAccount(address types.Address, password string) error { + return m.keystore.Delete(types.Account{Address: address}, password) } diff --git a/api/backend_test.go b/api/backend_test.go index f1578c508..0f4f1937e 100644 --- a/api/backend_test.go +++ b/api/backend_test.go @@ -614,46 +614,125 @@ func TestDeleteMultiaccount(t *testing.T) { } func TestConvertAccount(t *testing.T) { - backend := NewGethStatusBackend() - password := "123123" + const mnemonic = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" + const password = "111111" // represents password for a regular user + const keycardPassword = "222222" // represents password for a keycard user + const pathMaster = "m" + const pathWalletRoot = "m/44'/60'/0'/0" + const pathEIP1581Root = "m/43'/60'/1581'" + const pathEIP1581Chat = "m/43'/60'/1581'/0/0" + const pathDefaultWalletAccount = "m/44'/60'/0'/0/0" + const customWalletPath1 = "m/44'/60'/0'/0/1" + const customWalletPath2 = "m/44'/60'/0'/0/2" + var allGeneratedPaths []string + allGeneratedPaths = append(allGeneratedPaths, customWalletPath1, customWalletPath2, pathWalletRoot, pathEIP1581Root, pathDefaultWalletAccount) + + var err error + + keystoreContainsFileForAccount := func(keyStoreDir string, hexAddress string) bool { + addrWithoutPrefix := strings.ToLower(hexAddress[2:]) + found := false + err = filepath.Walk(keyStoreDir, func(path string, fileInfo os.FileInfo, err error) error { + if err != nil { + return err + } + if !fileInfo.IsDir() && strings.Contains(strings.ToUpper(path), strings.ToUpper(addrWithoutPrefix)) { + found = true + } + return nil + }) + return found + } + rootDataDir, err := os.MkdirTemp("", "test-keystore-dir") require.NoError(t, err) defer os.Remove(rootDataDir) keyStoreDir := filepath.Join(rootDataDir, "keystore") + utils.Init() + backend := NewGethStatusBackend() backend.rootDataDir = rootDataDir - - err = backend.AccountManager().InitKeystore(keyStoreDir) + config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) - - const path = "m/44'/60'/0'/0" - backend.AccountManager() - accs, err := backend.AccountManager(). - AccountsGenerator(). - GenerateAndDeriveAddresses(12, 1, "", []string{path}) + config.DataDir = rootDataDir + config.KeyStoreDir = keyStoreDir + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) + err = backend.StartNode(config) require.NoError(t, err) + defer func() { + require.NoError(t, backend.StopNode()) + }() - generateAccount := accs[0] - accountInfo, err := backend.AccountManager(). - AccountsGenerator(). - StoreAccount(generateAccount.ID, password) + genAccInfo, err := backend.AccountManager().AccountsGenerator().ImportMnemonic(mnemonic, "") + assert.NoError(t, err) + + accountInfo, err := backend.AccountManager().AccountsGenerator().StoreAccount(genAccInfo.ID, password) + assert.NoError(t, err) + + found := keystoreContainsFileForAccount(keyStoreDir, accountInfo.Address) require.NoError(t, err) + require.True(t, found) + + derivedAccounts, err := backend.AccountManager().AccountsGenerator().StoreDerivedAccounts(genAccInfo.ID, password, allGeneratedPaths) + assert.NoError(t, err) + + accs, err := backend.AccountManager().AccountsGenerator().DeriveAddresses(genAccInfo.ID, []string{pathMaster}) + require.NoError(t, err) + require.Equal(t, 1, len(accs)) + masterAddress := accs[pathMaster].Address + found = keystoreContainsFileForAccount(keyStoreDir, masterAddress) + require.NoError(t, err) + require.True(t, found) + + var accountsToStore []*accounts.Account + accountsToStore = append(accountsToStore, &accounts.Account{ + Address: types.HexToAddress(masterAddress), + KeyUID: genAccInfo.KeyUID, + Type: accounts.AccountTypeGenerated, + PublicKey: types.Hex2Bytes(accountInfo.PublicKey), + Path: pathEIP1581Chat, + Wallet: false, + Chat: false, + Name: "GeneratedAccount", + }) + + for p, dAccInfo := range derivedAccounts { + found = keystoreContainsFileForAccount(keyStoreDir, dAccInfo.Address) + require.NoError(t, err) + require.True(t, found) + + if p == pathDefaultWalletAccount || + p == customWalletPath1 || + p == customWalletPath2 { + accountsToStore = append(accountsToStore, &accounts.Account{ + Address: types.HexToAddress(dAccInfo.Address), + KeyUID: genAccInfo.KeyUID, + Wallet: false, + Chat: false, + Path: p, + Name: "derivacc" + p, + Hidden: false, + DerivedFrom: masterAddress, + Removed: false, + }) + } + } account := multiaccounts.Account{ Name: "foo", Timestamp: 1, - KeyUID: generateAccount.KeyUID, + KeyUID: genAccInfo.KeyUID, } err = backend.ensureAppDBOpened(account, password) require.NoError(t, err) s := settings.Settings{ - Address: types.HexToAddress(accountInfo.Address), + Address: types.HexToAddress(masterAddress), CurrentNetwork: "mainnet_rpc", - DappsAddress: types.HexToAddress(accountInfo.Address), - EIP1581Address: types.HexToAddress(accountInfo.Address), + DappsAddress: types.HexToAddress(derivedAccounts[pathDefaultWalletAccount].Address), + EIP1581Address: types.HexToAddress(derivedAccounts[pathEIP1581Root].Address), InstallationID: "d3efcff6-cffa-560e-a547-21d3858cbc51", KeyUID: account.KeyUID, LatestDerivedPath: 0, @@ -663,24 +742,13 @@ func TestConvertAccount(t *testing.T) { PreviewPrivacy: false, PublicKey: accountInfo.PublicKey, SigningPhrase: "yurt joey vibe", - WalletRootAddress: types.HexToAddress(accountInfo.Address)} - - acc := accounts.Account{ - Address: types.HexToAddress(generateAccount.Address), - KeyUID: generateAccount.KeyUID, - Type: accounts.AccountTypeGenerated, - PublicKey: types.Hex2Bytes(generateAccount.PublicKey), - Path: path, - Wallet: false, - Chat: false, - Name: "GeneratedAccount", + WalletRootAddress: types.HexToAddress(derivedAccounts[pathWalletRoot].Address), } - accounts := []*accounts.Account{&acc} err = backend.saveAccountsAndSettings( s, ¶ms.NodeConfig{}, - accounts) + accountsToStore) require.NoError(t, err) err = backend.OpenAccounts() @@ -693,8 +761,6 @@ func TestConvertAccount(t *testing.T) { require.NoError(t, err) require.NotEqual(t, 3, len(files)) - keycardPassword := "0xcafecafe" - keycardAccount := account keycardAccount.KeycardPairing = "pairing" @@ -704,42 +770,59 @@ func TestConvertAccount(t *testing.T) { KeycardPairing: "pairing", } - addrWithoutPrefix := strings.ToLower(generateAccount.Address[2:len(generateAccount.Address)]) - found := false - err = filepath.Walk(keyStoreDir, func(path string, fileInfo os.FileInfo, err error) error { - if err != nil { - return err - } - if !fileInfo.IsDir() && strings.Contains(path, addrWithoutPrefix) { - found = true - } - return nil - }) - - require.NoError(t, err) - require.True(t, found) - - err = backend.ConvertToKeycardAccount(keyStoreDir, keycardAccount, keycardSettings, password, keycardPassword) + // Converting to a keycard account + err = backend.ConvertToKeycardAccount(keycardAccount, keycardSettings, password, keycardPassword) require.NoError(t, err) - err = filepath.Walk(keyStoreDir, func(path string, fileInfo os.FileInfo, err error) error { - if err != nil { - return err - } - if !fileInfo.IsDir() && strings.Contains(path, addrWithoutPrefix) { - found = false - } - return nil - }) - + // Validating results of converting to a keycard account. + // All keystore files for the account which is migrated need to be removed. + found = keystoreContainsFileForAccount(keyStoreDir, accountInfo.Address) require.NoError(t, err) - require.True(t, found) + require.False(t, found) + for _, dAccInfo := range derivedAccounts { + found = keystoreContainsFileForAccount(keyStoreDir, dAccInfo.Address) + require.NoError(t, err) + require.False(t, found) + } + + found = keystoreContainsFileForAccount(keyStoreDir, masterAddress) + require.NoError(t, err) + require.False(t, found) + + // Ensure we're able to open the DB err = backend.ensureAppDBOpened(keycardAccount, keycardPassword) require.NoError(t, err) - b := NewGethStatusBackend() - require.NoError(t, b.OpenAccounts()) + b1 := NewGethStatusBackend() + require.NoError(t, b1.OpenAccounts()) + + // Converting to a regular account + err = backend.ConvertToRegularAccount(mnemonic, keycardPassword, password) + require.NoError(t, err) + + // Validating results of converting to a regular account. + // All keystore files for need to be created. + found = keystoreContainsFileForAccount(keyStoreDir, accountInfo.Address) + require.NoError(t, err) + require.True(t, found) + + for _, dAccInfo := range derivedAccounts { + found = keystoreContainsFileForAccount(keyStoreDir, dAccInfo.Address) + require.NoError(t, err) + require.True(t, found) + } + + found = keystoreContainsFileForAccount(keyStoreDir, masterAddress) + require.NoError(t, err) + require.True(t, found) + + // Ensure we're able to open the DB + err = backend.ensureAppDBOpened(keycardAccount, password) + require.NoError(t, err) + + b2 := NewGethStatusBackend() + require.NoError(t, b2.OpenAccounts()) } func copyFile(srcFolder string, dstFolder string, fileName string, t *testing.T) { diff --git a/api/geth_backend.go b/api/geth_backend.go index 74b8ae8d4..8df7327e9 100644 --- a/api/geth_backend.go +++ b/api/geth_backend.go @@ -564,7 +564,7 @@ func (b *GethStatusBackend) ChangeDatabasePassword(keyUID string, password strin return nil } -func (b *GethStatusBackend) ConvertToKeycardAccount(keyStoreDir string, account multiaccounts.Account, s settings.Settings, password string, newPassword string) error { +func (b *GethStatusBackend) ConvertToKeycardAccount(account multiaccounts.Account, s settings.Settings, password string, newPassword string) error { err := b.multiaccountsDB.UpdateAccountKeycardPairing(account.KeyUID, account.KeycardPairing) if err != nil { return err @@ -639,13 +639,13 @@ func (b *GethStatusBackend) ConvertToKeycardAccount(keyStoreDir string, account // whichever reason the account is still successfully migrated for _, acc := range knownAccounts { if account.KeyUID == acc.KeyUID { - _ = b.accountManager.DeleteAccount(keyStoreDir, acc.Address, true) + _ = b.accountManager.DeleteAccount(acc.Address, newPassword) } } - _ = b.accountManager.DeleteAccount(keyStoreDir, masterAddress, true) - _ = b.accountManager.DeleteAccount(keyStoreDir, dappsAddress, true) - _ = b.accountManager.DeleteAccount(keyStoreDir, eip1581Address, true) - _ = b.accountManager.DeleteAccount(keyStoreDir, walletRootAddress, true) + _ = b.accountManager.DeleteAccount(masterAddress, newPassword) + _ = b.accountManager.DeleteAccount(dappsAddress, newPassword) + _ = b.accountManager.DeleteAccount(eip1581Address, newPassword) + _ = b.accountManager.DeleteAccount(walletRootAddress, newPassword) return nil } diff --git a/mobile/status.go b/mobile/status.go index 8a503f301..3ec688095 100644 --- a/mobile/status.go +++ b/mobile/status.go @@ -828,7 +828,7 @@ func ChangeDatabasePassword(KeyUID, password, newPassword string) string { return makeJSONResponse(nil) } -func ConvertToKeycardAccount(keyStoreDir, accountData, settingsJSON, password, newPassword string) string { +func ConvertToKeycardAccount(accountData, settingsJSON, password, newPassword string) string { var account multiaccounts.Account err := json.Unmarshal([]byte(accountData), &account) if err != nil { @@ -840,7 +840,7 @@ func ConvertToKeycardAccount(keyStoreDir, accountData, settingsJSON, password, n return makeJSONResponse(err) } - err = statusBackend.ConvertToKeycardAccount(keyStoreDir, account, settings, password, newPassword) + err = statusBackend.ConvertToKeycardAccount(account, settings, password, newPassword) if err != nil { return makeJSONResponse(err) } diff --git a/services/accounts/accounts.go b/services/accounts/accounts.go index bf86e5b2b..7362b365f 100644 --- a/services/accounts/accounts.go +++ b/services/accounts/accounts.go @@ -78,7 +78,7 @@ func (api *API) DeleteAccount(ctx context.Context, address types.Address) error return err } if acc.Type != accounts.AccountTypeWatch { - err = api.manager.DeleteAccount(api.config.KeyStoreDir, address, true) + err = api.manager.DeleteAccount(address, "") var e *account.ErrCannotLocateKeyFile if err != nil && !errors.As(err, &e) { return err @@ -444,7 +444,7 @@ func (api *API) AddMigratedKeyPair(ctx context.Context, kcUID string, kpName str for _, addr := range addresses { // This action deletes an account from the keystore, no need to check for error in this context here, cause if this // action fails from whichever reason the account is still successfully migrated since keystore won't be used any more. - _ = api.manager.DeleteAccount(keyStoreDir, addr, true) + _ = api.manager.DeleteAccount(addr, "") } return nil }