From 476240fbd107fe193096350f5181c32c0539eff0 Mon Sep 17 00:00:00 2001 From: Dmitry Date: Wed, 31 Jul 2019 10:50:40 +0300 Subject: [PATCH] Initialize status keystore instance before starting ethereum node --- Makefile | 1 - account/accounts.go | 86 +++++--- account/accounts_mock.go | 65 ------ account/accounts_test.go | 300 ++++++++------------------ account/keystore.go | 25 +++ account/utils.go | 2 - api/backend.go | 10 +- api/backend_subs_test.go | 28 ++- api/backend_test.go | 16 +- api/utils_test.go | 1 + lib/library.go | 7 + lib/library_test_utils.go | 8 +- mobile/status.go | 6 + node/node.go | 24 ++- node/node_api_test.go | 5 +- node/node_test.go | 7 +- node/status_node.go | 48 ++--- node/status_node_rpc_client_test.go | 2 +- node/status_node_test.go | 32 +-- t/devtests/devnode.go | 1 + t/e2e/accounts/accounts_test.go | 10 +- t/e2e/api/api_test.go | 5 +- t/e2e/api/backend_test.go | 3 +- t/e2e/node/manager_test.go | 34 +-- t/e2e/rpc/rpc_test.go | 6 +- t/e2e/services/base_api_test.go | 1 + t/e2e/suites.go | 4 +- t/e2e/whisper/whisper_ext_test.go | 2 +- t/e2e/whisper/whisper_mailbox_test.go | 4 + t/e2e/whisper/whisper_test.go | 2 +- 30 files changed, 304 insertions(+), 441 deletions(-) delete mode 100644 account/accounts_mock.go create mode 100644 account/keystore.go diff --git a/Makefile b/Makefile index 127f28cbe..d36895cd1 100644 --- a/Makefile +++ b/Makefile @@ -232,7 +232,6 @@ mock-install: ##@other Install mocking tools mock: ##@other Regenerate mocks mockgen -package=fcm -destination=notifications/push/fcm/client_mock.go -source=notifications/push/fcm/client.go mockgen -package=fake -destination=transactions/fake/mock.go -source=transactions/fake/txservice.go - mockgen -package=account -destination=account/accounts_mock.go -source=account/accounts.go mockgen -package=status -destination=services/status/account_mock.go -source=services/status/service.go mockgen -package=peer -destination=services/peer/discoverer_mock.go -source=services/peer/service.go diff --git a/account/accounts.go b/account/accounts.go index 64f9c7177..73af4753a 100644 --- a/account/accounts.go +++ b/account/accounts.go @@ -30,21 +30,16 @@ var ( ErrInvalidMasterKeyCreated = errors.New("can not create master extended key") ErrOnboardingNotStarted = errors.New("onboarding must be started before choosing an account") ErrOnboardingAccountNotFound = errors.New("cannot find onboarding account with the given id") + ErrAccountKeyStoreMissing = errors.New("account key store is not set") ) var zeroAddress = common.Address{} -// GethServiceProvider provides required geth services. -type GethServiceProvider interface { - AccountManager() (*accounts.Manager, error) - AccountKeyStore() (*keystore.KeyStore, error) -} - // Manager represents account manager interface. type Manager struct { - geth GethServiceProvider - - mu sync.RWMutex + mu sync.RWMutex + keystore *keystore.KeyStore + manager *accounts.Manager accountsGenerator *generator.Generator onboarding *Onboarding @@ -55,15 +50,43 @@ type Manager struct { } // NewManager returns new node account manager. -func NewManager(geth GethServiceProvider) *Manager { - manager := &Manager{ - geth: geth, +func NewManager() *Manager { + m := &Manager{} + m.accountsGenerator = generator.New(m) + return m +} + +// InitKeystore sets key manager and key store. +func (m *Manager) InitKeystore(keydir string) error { + m.mu.Lock() + defer m.mu.Unlock() + manager, err := makeAccountManager(keydir) + if err != nil { + return err } + m.manager = manager + backends := manager.Backends(keystore.KeyStoreType) + if len(backends) == 0 { + return ErrAccountKeyStoreMissing + } + keyStore, ok := backends[0].(*keystore.KeyStore) + if !ok { + return ErrAccountKeyStoreMissing + } + m.keystore = keyStore + return nil +} - accountsGenerator := generator.New(manager) - manager.accountsGenerator = accountsGenerator +func (m *Manager) GetKeystore() *keystore.KeyStore { + m.mu.RLock() + defer m.mu.RUnlock() + return m.keystore +} - return manager +func (m *Manager) GetManager() *accounts.Manager { + m.mu.RLock() + defer m.mu.RUnlock() + return m.manager } // AccountsGenerator returns accountsGenerator. @@ -270,24 +293,22 @@ func (m *Manager) Logout() { // ImportAccount imports the account specified with privateKey. func (m *Manager) ImportAccount(privateKey *ecdsa.PrivateKey, password string) (common.Address, error) { - keyStore, err := m.geth.AccountKeyStore() - if err != nil { - return common.Address{}, err + if m.keystore == nil { + return common.Address{}, ErrAccountKeyStoreMissing } - account, err := keyStore.ImportECDSA(privateKey, password) + account, err := m.keystore.ImportECDSA(privateKey, password) return account.Address, err } func (m *Manager) ImportSingleExtendedKey(extKey *extkeys.ExtendedKey, password string) (address, pubKey string, err error) { - keyStore, err := m.geth.AccountKeyStore() - if err != nil { - return "", "", err + if m.keystore == nil { + return "", "", ErrAccountKeyStoreMissing } // imports extended key, create key file (if necessary) - account, err := keyStore.ImportSingleExtendedKey(extKey, password) + account, err := m.keystore.ImportSingleExtendedKey(extKey, password) if err != nil { return "", "", err } @@ -295,7 +316,7 @@ func (m *Manager) ImportSingleExtendedKey(extKey *extkeys.ExtendedKey, password address = account.Address.Hex() // obtain public key to return - account, key, err := keyStore.AccountDecryptedKey(account, password) + account, key, err := m.keystore.AccountDecryptedKey(account, password) if err != nil { return address, "", err } @@ -308,20 +329,19 @@ func (m *Manager) ImportSingleExtendedKey(extKey *extkeys.ExtendedKey, password // importExtendedKey processes incoming extended key, extracts required info and creates corresponding account key. // Once account key is formed, that key is put (if not already) into keystore i.e. key is *encoded* into key file. func (m *Manager) importExtendedKey(keyPurpose extkeys.KeyPurpose, extKey *extkeys.ExtendedKey, password string) (address, pubKey string, err error) { - keyStore, err := m.geth.AccountKeyStore() - if err != nil { - return "", "", err + if m.keystore == nil { + return "", "", ErrAccountKeyStoreMissing } // imports extended key, create key file (if necessary) - account, err := keyStore.ImportExtendedKeyForPurpose(keyPurpose, extKey, password) + account, err := m.keystore.ImportExtendedKeyForPurpose(keyPurpose, extKey, password) if err != nil { return "", "", err } address = account.Address.Hex() // obtain public key to return - account, key, err := keyStore.AccountDecryptedKey(account, password) + account, key, err := m.keystore.AccountDecryptedKey(account, password) if err != nil { return address, "", err } @@ -335,7 +355,6 @@ func (m *Manager) importExtendedKey(keyPurpose extkeys.KeyPurpose, extKey *extke func (m *Manager) Accounts() ([]gethcommon.Address, error) { m.mu.RLock() defer m.mu.RUnlock() - addresses := make([]gethcommon.Address, 0) if m.mainAccountAddress != zeroAddress { addresses = append(addresses, m.mainAccountAddress) @@ -397,9 +416,8 @@ func (m *Manager) ImportOnboardingAccount(id string, password string) (Info, str // The running node, has a keystore directory which is loaded on start. Key file // for a given address is expected to be in that directory prior to node start. func (m *Manager) AddressToDecryptedAccount(address, password string) (accounts.Account, *keystore.Key, error) { - keyStore, err := m.geth.AccountKeyStore() - if err != nil { - return accounts.Account{}, nil, err + if m.keystore == nil { + return accounts.Account{}, nil, ErrAccountKeyStoreMissing } account, err := ParseAccountString(address) @@ -408,7 +426,7 @@ func (m *Manager) AddressToDecryptedAccount(address, password string) (accounts. } var key *keystore.Key - account, key, err = keyStore.AccountDecryptedKey(account, password) + account, key, err = m.keystore.AccountDecryptedKey(account, password) if err != nil { err = fmt.Errorf("%s: %s", ErrAccountToKeyMappingFailure, err) } diff --git a/account/accounts_mock.go b/account/accounts_mock.go deleted file mode 100644 index 7d04f53db..000000000 --- a/account/accounts_mock.go +++ /dev/null @@ -1,65 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: account/accounts.go - -// Package account is a generated GoMock package. -package account - -import ( - accounts "github.com/ethereum/go-ethereum/accounts" - keystore "github.com/ethereum/go-ethereum/accounts/keystore" - gomock "github.com/golang/mock/gomock" - reflect "reflect" -) - -// MockGethServiceProvider is a mock of GethServiceProvider interface -type MockGethServiceProvider struct { - ctrl *gomock.Controller - recorder *MockGethServiceProviderMockRecorder -} - -// MockGethServiceProviderMockRecorder is the mock recorder for MockGethServiceProvider -type MockGethServiceProviderMockRecorder struct { - mock *MockGethServiceProvider -} - -// NewMockGethServiceProvider creates a new mock instance -func NewMockGethServiceProvider(ctrl *gomock.Controller) *MockGethServiceProvider { - mock := &MockGethServiceProvider{ctrl: ctrl} - mock.recorder = &MockGethServiceProviderMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockGethServiceProvider) EXPECT() *MockGethServiceProviderMockRecorder { - return m.recorder -} - -// AccountManager mocks base method -func (m *MockGethServiceProvider) AccountManager() (*accounts.Manager, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AccountManager") - ret0, _ := ret[0].(*accounts.Manager) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// AccountManager indicates an expected call of AccountManager -func (mr *MockGethServiceProviderMockRecorder) AccountManager() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AccountManager", reflect.TypeOf((*MockGethServiceProvider)(nil).AccountManager)) -} - -// AccountKeyStore mocks base method -func (m *MockGethServiceProvider) AccountKeyStore() (*keystore.KeyStore, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AccountKeyStore") - ret0, _ := ret[0].(*keystore.KeyStore) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// AccountKeyStore indicates an expected call of AccountKeyStore -func (mr *MockGethServiceProviderMockRecorder) AccountKeyStore() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AccountKeyStore", reflect.TypeOf((*MockGethServiceProvider)(nil).AccountKeyStore)) -} diff --git a/account/accounts_test.go b/account/accounts_test.go index d1133f0be..b5db22677 100644 --- a/account/accounts_test.go +++ b/account/accounts_test.go @@ -9,20 +9,16 @@ import ( "reflect" "testing" - "github.com/ethereum/go-ethereum/accounts" - "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" - "github.com/golang/mock/gomock" . "github.com/status-im/status-go/t/utils" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) func TestVerifyAccountPassword(t *testing.T) { - accManager := NewManager(nil) + accManager := NewManager() keyStoreDir, err := ioutil.TempDir(os.TempDir(), "accounts") require.NoError(t, err) defer os.RemoveAll(keyStoreDir) //nolint: errcheck @@ -108,70 +104,22 @@ func TestVerifyAccountPasswordWithAccountBeforeEIP55(t *testing.T) { err = ImportTestAccount(keyStoreDir, "test-account3-before-eip55.pk") require.NoError(t, err) - accManager := NewManager(nil) + accManager := NewManager() address := gethcommon.HexToAddress(TestConfig.Account3.WalletAddress) _, err = accManager.VerifyAccountPassword(keyStoreDir, address.Hex(), TestConfig.Account3.Password) require.NoError(t, err) } -var errKeyStore = errors.New("Can't return a key store") - func TestManagerTestSuite(t *testing.T) { - gethServiceProvider := newMockGethServiceProvider(t) - accManager := NewManager(gethServiceProvider) - - keyStoreDir, err := ioutil.TempDir(os.TempDir(), "accounts") - require.NoError(t, err) - keyStore := keystore.NewKeyStore(keyStoreDir, keystore.LightScryptN, keystore.LightScryptP) - defer os.RemoveAll(keyStoreDir) //nolint: errcheck - - testPassword := "test-password" - - // Initial test - create test account - gethServiceProvider.EXPECT().AccountKeyStore().Return(keyStore, nil) - accountInfo, mnemonic, err := accManager.CreateAccount(testPassword) - require.NoError(t, err) - require.NotEmpty(t, accountInfo.WalletAddress) - require.NotEmpty(t, accountInfo.WalletPubKey) - require.NotEmpty(t, accountInfo.ChatAddress) - require.NotEmpty(t, accountInfo.ChatPubKey) - require.NotEmpty(t, mnemonic) - - // Before the complete decoupling of the keys, wallet and chat keys are the same - assert.Equal(t, accountInfo.WalletAddress, accountInfo.ChatAddress) - assert.Equal(t, accountInfo.WalletPubKey, accountInfo.ChatPubKey) - - s := &ManagerTestSuite{ - testAccount: testAccount{ - "test-password", - accountInfo.WalletAddress, - accountInfo.WalletPubKey, - accountInfo.ChatAddress, - accountInfo.ChatPubKey, - mnemonic, - }, - gethServiceProvider: gethServiceProvider, - accManager: accManager, - keyStore: keyStore, - gethAccManager: accounts.NewManager(), - } - - suite.Run(t, s) -} - -func newMockGethServiceProvider(t *testing.T) *MockGethServiceProvider { - ctrl := gomock.NewController(t) - return NewMockGethServiceProvider(ctrl) + suite.Run(t, new(ManagerTestSuite)) } type ManagerTestSuite struct { suite.Suite testAccount - gethServiceProvider *MockGethServiceProvider - accManager *Manager - keyStore *keystore.KeyStore - gethAccManager *accounts.Manager + accManager *Manager + keydir string } type testAccount struct { @@ -183,43 +131,52 @@ type testAccount struct { mnemonic string } -// reinitMock is for reassigning a new mock node manager to account manager. -// Stating the amount of times for mock calls kills the flexibility for -// development so this is a good workaround to use with EXPECT().Func().AnyTimes() -func (s *ManagerTestSuite) reinitMock() { - s.gethServiceProvider = newMockGethServiceProvider(s.T()) - s.accManager.geth = s.gethServiceProvider -} - // SetupTest is used here for reinitializing the mock before every // test function to avoid faulty execution. func (s *ManagerTestSuite) SetupTest() { - s.reinitMock() + s.accManager = NewManager() + + keyStoreDir, err := ioutil.TempDir(os.TempDir(), "accounts") + s.Require().NoError(err) + s.Require().NoError(s.accManager.InitKeystore(keyStoreDir)) + s.keydir = keyStoreDir + + testPassword := "test-password" + + // Initial test - create test account + accountInfo, mnemonic, err := s.accManager.CreateAccount(testPassword) + s.Require().NoError(err) + s.Require().NotEmpty(accountInfo.WalletAddress) + s.Require().NotEmpty(accountInfo.WalletPubKey) + s.Require().NotEmpty(accountInfo.ChatAddress) + s.Require().NotEmpty(accountInfo.ChatPubKey) + s.Require().NotEmpty(mnemonic) + + // Before the complete decoupling of the keys, wallet and chat keys are the same + s.Equal(accountInfo.WalletAddress, accountInfo.ChatAddress) + s.Equal(accountInfo.WalletPubKey, accountInfo.ChatPubKey) + + s.testAccount = testAccount{ + testPassword, + accountInfo.WalletAddress, + accountInfo.WalletPubKey, + accountInfo.ChatAddress, + accountInfo.ChatPubKey, + mnemonic, + } } -func (s *ManagerTestSuite) TestCreateAccount() { - // Don't fail on empty password - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(s.keyStore, nil) - _, _, err := s.accManager.CreateAccount(s.password) - s.NoError(err) - - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(nil, errKeyStore) - _, _, err = s.accManager.CreateAccount(s.password) - s.Equal(errKeyStore, err) +func (s *ManagerTestSuite) TearDownTest() { + s.Require().NoError(os.RemoveAll(s.keydir)) } func (s *ManagerTestSuite) TestRecoverAccount() { - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(s.keyStore, nil) accountInfo, err := s.accManager.RecoverAccount(s.password, s.mnemonic) s.NoError(err) s.Equal(s.walletAddress, accountInfo.WalletAddress) s.Equal(s.walletPubKey, accountInfo.WalletPubKey) s.Equal(s.chatAddress, accountInfo.ChatAddress) s.Equal(s.chatPubKey, accountInfo.ChatPubKey) - - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(nil, errKeyStore) - _, err = s.accManager.RecoverAccount(s.password, s.mnemonic) - s.Equal(errKeyStore, err) } func (s *ManagerTestSuite) TestOnboarding() { @@ -240,7 +197,6 @@ func (s *ManagerTestSuite) TestOnboarding() { // choose one account and encrypt it with password password := "test-onboarding-account" account := accounts[0] - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(s.keyStore, nil) info, mnemonic, err := s.accManager.ImportOnboardingAccount(account.ID, password) s.Require().NoError(err) s.Equal(account.Info, info) @@ -248,7 +204,6 @@ func (s *ManagerTestSuite) TestOnboarding() { s.Nil(s.accManager.onboarding) // try to decrypt it with password to check if it's been imported correctly - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(s.keyStore, nil) decAccount, _, err := s.accManager.AddressToDecryptedAccount(info.WalletAddress, password) s.Require().NoError(err) s.Equal(info.WalletAddress, decAccount.Address.Hex()) @@ -262,79 +217,43 @@ func (s *ManagerTestSuite) TestOnboarding() { s.Nil(s.accManager.onboarding) } -func (s *ManagerTestSuite) TestSelectAccount() { - testCases := []struct { - name string - accountKeyStoreReturn []interface{} - walletAddress string - chatAddress string - password string - expectedError error - }{ - { - "success", - []interface{}{s.keyStore, nil}, - s.walletAddress, - s.chatAddress, - s.password, - nil, - }, - { - "fail_keyStore", - []interface{}{nil, errKeyStore}, - s.walletAddress, - s.chatAddress, - s.password, - errKeyStore, - }, - { - "fail_wrongChatAddress", - []interface{}{s.keyStore, nil}, - s.walletAddress, - "0x0000000000000000000000000000000000000001", - s.password, - errors.New("cannot retrieve a valid key for a given account: no key for given address or file"), - }, - { - "fail_wrongPassword", - []interface{}{s.keyStore, nil}, - s.walletAddress, - s.chatAddress, - "wrong-password", - errors.New("cannot retrieve a valid key for a given account: could not decrypt key with given passphrase"), - }, +func (s *ManagerTestSuite) TestSelectAccountSuccess() { + s.testSelectAccount(common.HexToAddress(s.testAccount.chatAddress), common.HexToAddress(s.testAccount.walletAddress), s.testAccount.password, nil) +} + +func (s *ManagerTestSuite) TestSelectAccountWrongAddress() { + s.testSelectAccount(common.HexToAddress("0x0000000000000000000000000000000000000001"), common.HexToAddress(s.testAccount.walletAddress), s.testAccount.password, errors.New("cannot retrieve a valid key for a given account: no key for given address or file")) +} + +func (s *ManagerTestSuite) TestSelectAccountWrongPassword() { + s.testSelectAccount(common.HexToAddress(s.testAccount.chatAddress), common.HexToAddress(s.testAccount.walletAddress), "wrong", errors.New("cannot retrieve a valid key for a given account: could not decrypt key with given passphrase")) +} + +func (s *ManagerTestSuite) testSelectAccount(chat, wallet common.Address, password string, expErr error) { + loginParams := LoginParams{ + ChatAddress: chat, + MainAccount: wallet, + Password: password, + } + err := s.accManager.SelectAccount(loginParams) + s.Require().Equal(expErr, err) + + selectedMainAccountAddress, walletErr := s.accManager.MainAccountAddress() + selectedChatAccount, chatErr := s.accManager.SelectedChatAccount() + + if expErr == nil { + s.Require().NoError(walletErr) + s.Require().NoError(chatErr) + s.Equal(wallet, selectedMainAccountAddress) + s.Equal(chat, crypto.PubkeyToAddress(selectedChatAccount.AccountKey.PrivateKey.PublicKey)) + } else { + s.Equal(common.Address{}, selectedMainAccountAddress) + s.Nil(selectedChatAccount) + s.Equal(walletErr, ErrNoAccountSelected) + s.Equal(chatErr, ErrNoAccountSelected) } - for _, testCase := range testCases { - s.T().Run(testCase.name, func(t *testing.T) { - s.reinitMock() - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(testCase.accountKeyStoreReturn...).AnyTimes() - loginParams := LoginParams{ - ChatAddress: common.HexToAddress(testCase.chatAddress), - MainAccount: common.HexToAddress(testCase.walletAddress), - Password: testCase.password, - } - err := s.accManager.SelectAccount(loginParams) - s.Equal(testCase.expectedError, err) - - selectedMainAccountAddress, walletErr := s.accManager.MainAccountAddress() - selectedChatAccount, chatErr := s.accManager.SelectedChatAccount() - - if testCase.expectedError == nil { - s.Equal(testCase.walletAddress, selectedMainAccountAddress.String()) - s.Equal(testCase.chatAddress, crypto.PubkeyToAddress(selectedChatAccount.AccountKey.PrivateKey.PublicKey).Hex()) - s.NoError(walletErr) - s.NoError(chatErr) - } else { - s.Equal(common.Address{}, selectedMainAccountAddress) - s.Nil(selectedChatAccount) - s.Equal(walletErr, ErrNoAccountSelected) - s.Equal(chatErr, ErrNoAccountSelected) - } - - s.accManager.Logout() - }) - } + s.accManager.Logout() } func (s *ManagerTestSuite) TestSetChatAccount() { @@ -367,7 +286,6 @@ func (s *ManagerTestSuite) TestLogout() { // TestAccounts tests cases for (*Manager).Accounts. func (s *ManagerTestSuite) TestAccounts() { // Select the test account - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(s.keyStore, nil).AnyTimes() loginParams := LoginParams{ MainAccount: common.HexToAddress(s.walletAddress), ChatAddress: common.HexToAddress(s.chatAddress), @@ -377,70 +295,36 @@ func (s *ManagerTestSuite) TestAccounts() { s.NoError(err) // Success - s.gethServiceProvider.EXPECT().AccountManager().Return(s.gethAccManager, nil) accs, err := s.accManager.Accounts() s.NoError(err) s.NotNil(accs) - // Selected main account address is zero address but doesn't fail s.accManager.mainAccountAddress = common.Address{} - s.gethServiceProvider.EXPECT().AccountManager().Return(s.gethAccManager, nil) accs, err = s.accManager.Accounts() s.NoError(err) s.NotNil(accs) } -func (s *ManagerTestSuite) TestAddressToDecryptedAccount() { - testCases := []struct { - name string - accountKeyStoreReturn []interface{} - walletAddress string - password string - expectedError error - }{ - { - "success", - []interface{}{s.keyStore, nil}, - s.walletAddress, - s.password, - nil, - }, - { - "fail_keyStore", - []interface{}{nil, errKeyStore}, - s.walletAddress, - s.password, - errKeyStore, - }, - { - "fail_wrongWalletAddress", - []interface{}{s.keyStore, nil}, - "wrong-wallet-address", - s.password, - ErrAddressToAccountMappingFailure, - }, - { - "fail_wrongPassword", - []interface{}{s.keyStore, nil}, - s.walletAddress, - "wrong-password", - errors.New("cannot retrieve a valid key for a given account: could not decrypt key with given passphrase"), - }, - } +func (s *ManagerTestSuite) TestAddressToDecryptedAccountSuccess() { + s.testAddressToDecryptedAccount(s.walletAddress, s.password, nil) +} - for _, testCase := range testCases { - s.T().Run(testCase.name, func(t *testing.T) { - s.reinitMock() - s.gethServiceProvider.EXPECT().AccountKeyStore().Return(testCase.accountKeyStoreReturn...).AnyTimes() - acc, key, err := s.accManager.AddressToDecryptedAccount(testCase.walletAddress, testCase.password) - if testCase.expectedError != nil { - s.Equal(testCase.expectedError, err) - } else { - s.NoError(err) - s.NotNil(acc) - s.NotNil(key) - s.Equal(acc.Address, key.Address) - } - }) +func (s *ManagerTestSuite) TestAddressToDecryptedAccountWrongAddress() { + s.testAddressToDecryptedAccount("0x0001", s.password, ErrAddressToAccountMappingFailure) +} + +func (s *ManagerTestSuite) TestAddressToDecryptedAccountWrongPassword() { + s.testAddressToDecryptedAccount(s.walletAddress, "wrong", errors.New("cannot retrieve a valid key for a given account: could not decrypt key with given passphrase")) +} + +func (s *ManagerTestSuite) testAddressToDecryptedAccount(wallet, password string, expErr error) { + acc, key, err := s.accManager.AddressToDecryptedAccount(wallet, password) + if expErr != nil { + s.Equal(expErr, err) + } else { + s.Require().NoError(err) + s.Require().NotNil(acc) + s.Require().NotNil(key) + s.Equal(acc.Address, key.Address) } } diff --git a/account/keystore.go b/account/keystore.go new file mode 100644 index 000000000..40e4f8971 --- /dev/null +++ b/account/keystore.go @@ -0,0 +1,25 @@ +package account + +import ( + "io/ioutil" + "os" + + "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/keystore" +) + +// makeAccountManager creates ethereum accounts.Manager with single disk backend and lightweight kdf. +// If keydir is empty new temporary directory with go-ethereum-keystore will be intialized. +func makeAccountManager(keydir string) (manager *accounts.Manager, err error) { + if keydir == "" { + // There is no datadir. + keydir, err = ioutil.TempDir("", "go-ethereum-keystore") + } + if err != nil { + return nil, err + } + if err := os.MkdirAll(keydir, 0700); err != nil { + return nil, err + } + return accounts.NewManager(keystore.NewKeyStore(keydir, keystore.LightScryptN, keystore.LightScryptP)), nil +} diff --git a/account/utils.go b/account/utils.go index bddd92da5..703ed64c6 100644 --- a/account/utils.go +++ b/account/utils.go @@ -42,7 +42,6 @@ func ParseLoginParams(paramsJSON string) (LoginParams, error) { params LoginParams zeroAddress common.Address ) - if err := json.Unmarshal([]byte(paramsJSON), ¶ms); err != nil { return params, err } @@ -60,7 +59,6 @@ func ParseLoginParams(paramsJSON string) (LoginParams, error) { return params, newErrZeroAddress("WatchAddresses") } } - return params, nil } diff --git a/api/backend.go b/api/backend.go index cb34d2473..78daf230b 100644 --- a/api/backend.go +++ b/api/backend.go @@ -70,7 +70,7 @@ func NewStatusBackend() *StatusBackend { defer log.Info("Status backend initialized", "version", params.Version, "commit", params.GitCommit) statusNode := node.New() - accountManager := account.NewManager(statusNode) + accountManager := account.NewManager() transactor := transactions.NewTransactor() personalAPI := personal.NewAPI() notificationManager := fcm.NewNotification(fcmServerKey) @@ -149,12 +149,17 @@ func (b *StatusBackend) startNode(config *params.NodeConfig) (err error) { services = appendIf(config.UpstreamConfig.Enabled, services, b.rpcFiltersService()) services = append(services, b.subscriptionService()) + manager := b.accountManager.GetManager() + if manager == nil { + return errors.New("ethereum accounts.Manager is nil") + } if err = b.statusNode.StartWithOptions(config, node.StartOptions{ Services: services, // The peers discovery protocols are started manually after // `node.ready` signal is sent. // It was discussed in https://github.com/status-im/status-go/pull/1333. - StartDiscovery: false, + StartDiscovery: false, + AccountsManager: manager, }); err != nil { return } @@ -622,7 +627,6 @@ func (b *StatusBackend) startWallet(password string) error { allAddresses := make([]common.Address, len(watchAddresses)+1) allAddresses[0] = mainAccountAddress copy(allAddresses[1:], watchAddresses) - path := path.Join(b.statusNode.Config().DataDir, fmt.Sprintf("wallet-%x.sql", mainAccountAddress)) return wallet.StartReactor(path, password, b.statusNode.RPCClient().Ethclient(), diff --git a/api/backend_subs_test.go b/api/backend_subs_test.go index af013573b..a776e5dd5 100644 --- a/api/backend_subs_test.go +++ b/api/backend_subs_test.go @@ -8,6 +8,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/status-im/status-go/account" + "github.com/status-im/status-go/node" "github.com/status-im/status-go/params" "github.com/status-im/status-go/signal" "github.com/status-im/status-go/t/utils" @@ -23,11 +24,16 @@ const ( func TestSubscriptionEthWithParamsDict(t *testing.T) { // a simple test to check the parameter parsing for eth_* filter subscriptions backend := NewStatusBackend() + // initNodeAndLogin can fail and terminate the test, in that case stopNode must be executed anyway. + defer func() { + err := backend.StopNode() + if err != node.ErrNoRunningNode { + require.NoError(t, err) + } + }() initNodeAndLogin(t, backend) - defer func() { require.NoError(t, backend.StopNode()) }() - createSubscription(t, backend, fmt.Sprintf(`"eth_newFilter", [ { "fromBlock":"earliest", @@ -40,11 +46,15 @@ func TestSubscriptionEthWithParamsDict(t *testing.T) { func TestSubscriptionPendingTransaction(t *testing.T) { backend := NewStatusBackend() backend.allowAllRPC = true + defer func() { + err := backend.StopNode() + if err != node.ErrNoRunningNode { + require.NoError(t, err) + } + }() account, _ := initNodeAndLogin(t, backend) - defer func() { require.NoError(t, backend.StopNode()) }() - signals := make(chan string) defer func() { signal.ResetDefaultNodeNotificationHandler() @@ -88,11 +98,15 @@ func TestSubscriptionPendingTransaction(t *testing.T) { func TestSubscriptionWhisperEnvelopes(t *testing.T) { backend := NewStatusBackend() + defer func() { + err := backend.StopNode() + if err != node.ErrNoRunningNode { + require.NoError(t, err) + } + }() initNodeAndLogin(t, backend) - defer func() { require.NoError(t, backend.StopNode()) }() - signals := make(chan string) defer func() { signal.ResetDefaultNodeNotificationHandler() @@ -222,9 +236,9 @@ func initNodeAndLogin(t *testing.T, backend *StatusBackend) (string, string) { config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) - info, _, err := backend.AccountManager().CreateAccount(password) require.NoError(t, err) diff --git a/api/backend_test.go b/api/backend_test.go index 7cdcc8eff..6f6a3b196 100644 --- a/api/backend_test.go +++ b/api/backend_test.go @@ -25,6 +25,7 @@ func TestBackendStartNodeConcurrently(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) count := 2 resultCh := make(chan error) @@ -58,9 +59,8 @@ func TestBackendRestartNodeConcurrently(t *testing.T) { config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) count := 3 - - err = backend.StartNode(config) - require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) + require.NoError(t, backend.StartNode(config)) defer func() { require.NoError(t, backend.StopNode()) }() @@ -84,7 +84,7 @@ func TestBackendGettersConcurrently(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) - + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) defer func() { @@ -136,7 +136,7 @@ func TestBackendAccountsConcurrently(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) - + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) defer func() { @@ -196,6 +196,7 @@ func TestBackendInjectChatAccount(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) defer func() { @@ -269,6 +270,7 @@ func TestBackendCallRPCConcurrently(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) count := 3 err = backend.StartNode(config) @@ -342,6 +344,7 @@ func TestBlockedRPCMethods(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) defer func() { require.NoError(t, backend.StopNode()) }() @@ -379,6 +382,7 @@ func TestStartStopMultipleTimes(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) config.NoDiscovery = false // doesn't have to be running. just any valid enode to bypass validation. config.ClusterConfig.BootNodes = []string{ @@ -395,6 +399,7 @@ func TestSignHash(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) require.NoError(t, backend.StartNode(config)) defer func() { @@ -432,6 +437,7 @@ func TestHashTypedData(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) defer func() { diff --git a/api/utils_test.go b/api/utils_test.go index c90ae7e03..f95ff6b61 100644 --- a/api/utils_test.go +++ b/api/utils_test.go @@ -16,6 +16,7 @@ func TestHashMessage(t *testing.T) { backend := NewStatusBackend() config, err := utils.MakeTestNodeConfig(params.StatusChainNetworkID) require.NoError(t, err) + require.NoError(t, backend.AccountManager().InitKeystore(config.KeyStoreDir)) err = backend.StartNode(config) require.NoError(t, err) defer func() { diff --git a/lib/library.go b/lib/library.go index 90281685a..2f83e0ec6 100644 --- a/lib/library.go +++ b/lib/library.go @@ -337,6 +337,13 @@ func Login(loginParamsJSON *C.char) *C.char { return makeJSONResponse(err) } +// InitKeystore initialize keystore before doing any operations with keys. +//export InitKeystore +func InitKeystore(keydir *C.char) *C.char { + err := statusBackend.AccountManager().InitKeystore(C.GoString(keydir)) + return makeJSONResponse(err) +} + // LoginWithKeycard initializes an account with a chat key and encryption key used for PFS. // It purges all the previous identities from Whisper, and injects the key as shh identity. //export LoginWithKeycard diff --git a/lib/library_test_utils.go b/lib/library_test_utils.go index 2fa2355a4..bf58bd12c 100644 --- a/lib/library_test_utils.go +++ b/lib/library_test_utils.go @@ -102,6 +102,7 @@ func testExportedAPI(t *testing.T, done chan struct{}) { if err := ImportTestAccount(testKeyDir, GetAccount2PKFile()); err != nil { panic(err) } + _ = InitKeystore(C.CString(testKeyDir)) // FIXME(tiabc): All of that is done because usage of cgo is not supported in tests. // Probably, there should be a cleaner way, for example, test cgo bindings in e2e tests @@ -391,7 +392,11 @@ func testCallPrivateRPCWithPrivateAPI(t *testing.T) bool { } func testRecoverAccount(t *testing.T) bool { //nolint: gocyclo - keyStore, _ := statusBackend.StatusNode().AccountKeyStore() + keyStore := statusBackend.AccountManager().GetKeystore() + if keyStore == nil { + t.Errorf("keystore is nil") + return false + } // create an account accountInfo, mnemonic, err := statusBackend.AccountManager().CreateAccount(TestConfig.Account1.Password) @@ -845,6 +850,7 @@ func startTestNode(t *testing.T) <-chan struct{} { if err := ImportTestAccount(testKeyDir, GetAccount2PKFile()); err != nil { panic(err) } + _ = InitKeystore(C.CString(testKeyDir)) waitForNodeStart := make(chan struct{}, 1) signal.SetDefaultNodeNotificationHandler(func(jsonEvent string) { diff --git a/mobile/status.go b/mobile/status.go index 18382b3f4..d8a8d5d15 100644 --- a/mobile/status.go +++ b/mobile/status.go @@ -336,6 +336,12 @@ func Login(loginParamsJSON string) string { return makeJSONResponse(err) } +// InitKeystore initialize keystore before doing any operations with keys. +func InitKeystore(keydir string) string { + err := statusBackend.AccountManager().InitKeystore(keydir) + return makeJSONResponse(err) +} + // LoginWithKeycard initializes an account with a chat key and encryption key used for PFS. // It purges all the previous identities from Whisper, and injects the key as shh identity. func LoginWithKeycard(chatKeyData, encryptionKeyData string) string { diff --git a/node/node.go b/node/node.go index 882a65429..140a10e91 100644 --- a/node/node.go +++ b/node/node.go @@ -9,6 +9,7 @@ import ( "path/filepath" "time" + "github.com/ethereum/go-ethereum/accounts" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/crypto" @@ -57,7 +58,7 @@ var ( var logger = log.New("package", "status-go/node") // MakeNode creates a geth node entity -func MakeNode(config *params.NodeConfig, db *leveldb.DB) (*node.Node, error) { +func MakeNode(config *params.NodeConfig, accs *accounts.Manager, db *leveldb.DB) (*node.Node, error) { // If DataDir is empty, it means we want to create an ephemeral node // keeping data only in memory. if config.DataDir != "" { @@ -82,17 +83,17 @@ func MakeNode(config *params.NodeConfig, db *leveldb.DB) (*node.Node, error) { return nil, fmt.Errorf(ErrNodeMakeFailureFormat, err.Error()) } - err = activateServices(stack, config, db) + err = activateServices(stack, config, accs, db) if err != nil { return nil, err } return stack, nil } -func activateServices(stack *node.Node, config *params.NodeConfig, db *leveldb.DB) error { +func activateServices(stack *node.Node, config *params.NodeConfig, accs *accounts.Manager, db *leveldb.DB) error { // start Ethereum service if we are not expected to use an upstream server if !config.UpstreamConfig.Enabled { - if err := activateLightEthService(stack, config); err != nil { + if err := activateLightEthService(stack, accs, config); err != nil { return fmt.Errorf("%v: %v", ErrLightEthRegistrationFailure, err) } } else { @@ -107,7 +108,7 @@ func activateServices(stack *node.Node, config *params.NodeConfig, db *leveldb.D // Usually, they are provided by an ETH or a LES service, but when using // upstream, we don't start any of these, so we need to start our own // implementation. - if err := activatePersonalService(stack, config); err != nil { + if err := activatePersonalService(stack, accs, config); err != nil { return fmt.Errorf("%v: %v", ErrPersonalServiceRegistrationFailure, err) } } @@ -248,7 +249,7 @@ func defaultStatusChainGenesisBlock() (*core.Genesis, error) { } // activateLightEthService configures and registers the eth.Ethereum service with a given node. -func activateLightEthService(stack *node.Node, config *params.NodeConfig) error { +func activateLightEthService(stack *node.Node, accs *accounts.Manager, config *params.NodeConfig) error { if !config.LightEthConfig.Enabled { logger.Info("LES protocol is disabled") return nil @@ -269,13 +270,18 @@ func activateLightEthService(stack *node.Node, config *params.NodeConfig) error MinTrustedFraction: config.LightEthConfig.MinTrustedFraction, } return stack.Register(func(ctx *node.ServiceContext) (node.Service, error) { - return les.New(ctx, ðConf) + // NOTE(dshulyak) here we set our instance of the accounts manager. + // without sharing same instance selected account won't be visible for personal_* methods. + nctx := &node.ServiceContext{} + *nctx = *ctx + nctx.AccountManager = accs + return les.New(nctx, ðConf) }) } -func activatePersonalService(stack *node.Node, config *params.NodeConfig) error { +func activatePersonalService(stack *node.Node, accs *accounts.Manager, config *params.NodeConfig) error { return stack.Register(func(*node.ServiceContext) (node.Service, error) { - svc := personal.New(stack.AccountManager()) + svc := personal.New(accs) return svc, nil }) } diff --git a/node/node_api_test.go b/node/node_api_test.go index f5a2c06fa..7ab62ebbc 100644 --- a/node/node_api_test.go +++ b/node/node_api_test.go @@ -3,6 +3,7 @@ package node import ( "testing" + "github.com/ethereum/go-ethereum/accounts" whisper "github.com/status-im/whisper/whisperv6" "github.com/status-im/status-go/params" @@ -17,7 +18,7 @@ func TestWhisperLightModeEnabledSetsEmptyBloomFilter(t *testing.T) { }, } node := New() - require.NoError(t, node.Start(&config)) + require.NoError(t, node.Start(&config, &accounts.Manager{})) defer func() { require.NoError(t, node.Stop()) }() @@ -39,7 +40,7 @@ func TestWhisperLightModeEnabledSetsNilBloomFilter(t *testing.T) { }, } node := New() - require.NoError(t, node.Start(&config)) + require.NoError(t, node.Start(&config, &accounts.Manager{})) defer func() { require.NoError(t, node.Stop()) }() diff --git a/node/node_test.go b/node/node_test.go index 553d4ab58..c8fcb6640 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -4,6 +4,7 @@ import ( "net" "testing" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/status-im/status-go/params" @@ -23,7 +24,7 @@ func TestMakeNodeDefaultConfig(t *testing.T) { db, err := leveldb.Open(storage.NewMemStorage(), nil) require.NoError(t, err) - _, err = MakeNode(config, db) + _, err = MakeNode(config, &accounts.Manager{}, db) require.NoError(t, err) } @@ -40,7 +41,7 @@ func TestMakeNodeWellFormedBootnodes(t *testing.T) { db, err := leveldb.Open(storage.NewMemStorage(), nil) require.NoError(t, err) - _, err = MakeNode(config, db) + _, err = MakeNode(config, &accounts.Manager{}, db) require.NoError(t, err) } @@ -58,7 +59,7 @@ func TestMakeNodeMalformedBootnodes(t *testing.T) { db, err := leveldb.Open(storage.NewMemStorage(), nil) require.NoError(t, err) - _, err = MakeNode(config, db) + _, err = MakeNode(config, &accounts.Manager{}, db) require.NoError(t, err) } diff --git a/node/status_node.go b/node/status_node.go index 23bc5358a..280acc286 100644 --- a/node/status_node.go +++ b/node/status_node.go @@ -13,7 +13,6 @@ import ( "time" "github.com/ethereum/go-ethereum/accounts" - "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/les" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" @@ -105,17 +104,19 @@ func (n *StatusNode) Server() *p2p.Server { // Start starts current StatusNode, failing if it's already started. // It accepts a list of services that should be added to the node. -func (n *StatusNode) Start(config *params.NodeConfig, services ...node.ServiceConstructor) error { +func (n *StatusNode) Start(config *params.NodeConfig, accs *accounts.Manager, services ...node.ServiceConstructor) error { return n.StartWithOptions(config, StartOptions{ - Services: services, - StartDiscovery: true, + Services: services, + StartDiscovery: true, + AccountsManager: accs, }) } // StartOptions allows to control some parameters of Start() method. type StartOptions struct { - Services []node.ServiceConstructor - StartDiscovery bool + Services []node.ServiceConstructor + StartDiscovery bool + AccountsManager *accounts.Manager } // StartWithOptions starts current StatusNode, failing if it's already started. @@ -133,12 +134,12 @@ func (n *StatusNode) StartWithOptions(config *params.NodeConfig, options StartOp db, err := db.Create(config.DataDir, params.StatusDatabase) if err != nil { - return err + return fmt.Errorf("failed to create database at %s: %v", config.DataDir, err) } n.db = db - err = n.startWithDB(config, db, options.Services) + err = n.startWithDB(config, options.AccountsManager, db, options.Services) // continue only if there was no error when starting node with a db if err == nil && options.StartDiscovery && n.discoveryEnabled() { @@ -156,8 +157,8 @@ func (n *StatusNode) StartWithOptions(config *params.NodeConfig, options StartOp return nil } -func (n *StatusNode) startWithDB(config *params.NodeConfig, db *leveldb.DB, services []node.ServiceConstructor) error { - if err := n.createNode(config, db); err != nil { +func (n *StatusNode) startWithDB(config *params.NodeConfig, accs *accounts.Manager, db *leveldb.DB, services []node.ServiceConstructor) error { + if err := n.createNode(config, accs, db); err != nil { return err } n.config = config @@ -173,8 +174,8 @@ func (n *StatusNode) startWithDB(config *params.NodeConfig, db *leveldb.DB, serv return nil } -func (n *StatusNode) createNode(config *params.NodeConfig, db *leveldb.DB) (err error) { - n.gethNode, err = MakeNode(config, db) +func (n *StatusNode) createNode(config *params.NodeConfig, accs *accounts.Manager, db *leveldb.DB) (err error) { + n.gethNode, err = MakeNode(config, accs, db) return err } @@ -638,29 +639,6 @@ func (n *StatusNode) AccountManager() (*accounts.Manager, error) { return n.gethNode.AccountManager(), nil } -// AccountKeyStore exposes reference to accounts key store -func (n *StatusNode) AccountKeyStore() (*keystore.KeyStore, error) { - n.mu.RLock() - defer n.mu.RUnlock() - - if n.gethNode == nil { - return nil, ErrNoGethNode - } - - accountManager := n.gethNode.AccountManager() - backends := accountManager.Backends(keystore.KeyStoreType) - if len(backends) == 0 { - return nil, ErrAccountKeyStoreMissing - } - - keyStore, ok := backends[0].(*keystore.KeyStore) - if !ok { - return nil, ErrAccountKeyStoreMissing - } - - return keyStore, nil -} - // RPCClient exposes reference to RPC client connected to the running node. func (n *StatusNode) RPCClient() *rpc.Client { n.mu.RLock() diff --git a/node/status_node_rpc_client_test.go b/node/status_node_rpc_client_test.go index a7aa81bdf..b29c2803d 100644 --- a/node/status_node_rpc_client_test.go +++ b/node/status_node_rpc_client_test.go @@ -58,7 +58,7 @@ func createAndStartStatusNode(config *params.NodeConfig) (*node.StatusNode, erro }, } statusNode := node.New() - return statusNode, statusNode.Start(config, services...) + return statusNode, statusNode.Start(config, nil, services...) } func TestNodeRPCClientCallOnlyPublicAPIs(t *testing.T) { diff --git a/node/status_node_test.go b/node/status_node_test.go index 283399bf5..9fd879cda 100644 --- a/node/status_node_test.go +++ b/node/status_node_test.go @@ -36,13 +36,8 @@ func TestStatusNodeStart(t *testing.T) { require.Nil(t, n.Config()) require.Nil(t, n.RPCClient()) require.Equal(t, 0, n.PeerCount()) - _, err = n.AccountManager() - require.EqualError(t, err, ErrNoGethNode.Error()) - _, err = n.AccountKeyStore() - require.EqualError(t, err, ErrNoGethNode.Error()) - // start node - require.NoError(t, n.Start(config)) + require.NoError(t, n.Start(config, nil)) // checks after node is started require.True(t, n.IsRunning()) @@ -53,11 +48,8 @@ func TestStatusNodeStart(t *testing.T) { accountManager, err := n.AccountManager() require.Nil(t, err) require.NotNil(t, accountManager) - keyStore, err := n.AccountKeyStore() - require.Nil(t, err) - require.NotNil(t, keyStore) // try to start already started node - require.EqualError(t, n.Start(config), ErrNodeRunning.Error()) + require.EqualError(t, n.Start(config, nil), ErrNodeRunning.Error()) // stop node require.NoError(t, n.Stop()) @@ -68,10 +60,6 @@ func TestStatusNodeStart(t *testing.T) { require.Nil(t, n.GethNode()) require.Nil(t, n.RPCClient()) require.Equal(t, 0, n.PeerCount()) - _, err = n.AccountManager() - require.EqualError(t, err, ErrNoGethNode.Error()) - _, err = n.AccountKeyStore() - require.EqualError(t, err, ErrNoGethNode.Error()) } func TestStatusNodeWithDataDir(t *testing.T) { @@ -94,7 +82,7 @@ func TestStatusNodeWithDataDir(t *testing.T) { } n := New() - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) require.NoError(t, n.Stop()) } @@ -140,7 +128,7 @@ func TestStatusNodeServiceGetters(t *testing.T) { require.Nil(t, instance) // start node - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) // checks after node is started instance, err = service.getter() @@ -184,7 +172,7 @@ func TestStatusNodeAddPeer(t *testing.T) { config := params.NodeConfig{ MaxPeers: math.MaxInt32, } - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) defer func() { require.NoError(t, n.Stop()) }() errCh := helpers.WaitForPeerAsync(n.Server(), peerURL, p2p.PeerEventTypeAdd, time.Second*5) @@ -226,7 +214,7 @@ func TestStatusNodeReconnectStaticPeers(t *testing.T) { StaticNodes: []string{peerURL}, }, } - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) defer func() { require.NoError(t, n.Stop()) }() // checks after node is started @@ -286,7 +274,7 @@ func TestStatusNodeRendezvousDiscovery(t *testing.T) { AdvertiseAddr: "127.0.0.1", } n := New() - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) require.NotNil(t, n.discovery) require.True(t, n.discovery.Running()) require.IsType(t, &discovery.Rendezvous{}, n.discovery) @@ -320,7 +308,7 @@ func TestStatusNodeDiscoverNode(t *testing.T) { ListenAddr: "127.0.0.1:0", } n := New() - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) node, err := n.discoverNode() require.NoError(t, err) require.Equal(t, net.ParseIP("127.0.0.1").To4(), node.IP()) @@ -331,7 +319,7 @@ func TestStatusNodeDiscoverNode(t *testing.T) { ListenAddr: "127.0.0.1:0", } n = New() - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) node, err = n.discoverNode() require.NoError(t, err) require.Equal(t, net.ParseIP("127.0.0.2").To4(), node.IP()) @@ -357,7 +345,7 @@ func TestChaosModeCheckRPCClientsUpstreamURL(t *testing.T) { }, } n := New() - require.NoError(t, n.Start(&config)) + require.NoError(t, n.Start(&config, nil)) defer func() { require.NoError(t, n.Stop()) }() require.NotNil(t, n.RPCClient()) diff --git a/t/devtests/devnode.go b/t/devtests/devnode.go index 89b6a97e9..674ccc309 100644 --- a/t/devtests/devnode.go +++ b/t/devtests/devnode.go @@ -55,6 +55,7 @@ func (s *DevNodeSuite) SetupTest() { config.WalletConfig.Enabled = true config.UpstreamConfig.URL = s.miner.IPCEndpoint() s.backend = api.NewStatusBackend() + s.Require().NoError(s.backend.AccountManager().InitKeystore(config.KeyStoreDir)) s.Require().NoError(s.backend.StartNode(config)) s.Remote, err = s.miner.Attach() s.Require().NoError(err) diff --git a/t/e2e/accounts/accounts_test.go b/t/e2e/accounts/accounts_test.go index 9f379f8fe..2b1746517 100644 --- a/t/e2e/accounts/accounts_test.go +++ b/t/e2e/accounts/accounts_test.go @@ -67,8 +67,7 @@ func (s *AccountsTestSuite) TestImportSingleExtendedKey() { s.StartTestBackend() defer s.StopTestBackend() - keyStore, err := s.Backend.StatusNode().AccountKeyStore() - s.NoError(err) + keyStore := s.Backend.AccountManager().GetKeystore() s.NotNil(keyStore) // create a master extended key @@ -95,8 +94,7 @@ func (s *AccountsTestSuite) TestImportAccount() { s.StartTestBackend() defer s.StopTestBackend() - keyStore, err := s.Backend.StatusNode().AccountKeyStore() - s.NoError(err) + keyStore := s.Backend.AccountManager().GetKeystore() s.NotNil(keyStore) // create a private key @@ -119,8 +117,8 @@ func (s *AccountsTestSuite) TestRecoverAccount() { s.StartTestBackend() defer s.StopTestBackend() - keyStore, err := s.Backend.StatusNode().AccountKeyStore() - s.NoError(err) + keyStore := s.Backend.AccountManager().GetKeystore() + s.NotNil(keyStore) // create an acc accountInfo, mnemonic, err := s.Backend.AccountManager().CreateAccount(TestConfig.Account1.Password) diff --git a/t/e2e/api/api_test.go b/t/e2e/api/api_test.go index 9186ac7d8..fd2e7a8d7 100644 --- a/t/e2e/api/api_test.go +++ b/t/e2e/api/api_test.go @@ -90,6 +90,7 @@ func (s *APITestSuite) TestRaceConditions() { if rnd.Intn(100) > 75 { // introduce random delays time.Sleep(500 * time.Millisecond) } + s.NoError(s.backend.AccountManager().InitKeystore(randConfig.KeyStoreDir)) go randFunc(randConfig) } @@ -124,6 +125,7 @@ func (s *APITestSuite) TestEventsNodeStartStop() { nodeConfig, err := MakeTestNodeConfig(GetNetworkID()) s.NoError(err) + s.NoError(s.backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) s.Require().NoError(s.backend.StartNode(nodeConfig)) s.NoError(s.backend.StopNode()) s.verifyEnvelopes(envelopes, signal.EventNodeStarted, signal.EventNodeReady, signal.EventNodeStopped) @@ -168,12 +170,13 @@ func (s *APITestSuite) TestNodeStartCrash() { defer func() { s.NoError(db.Close()) }() // start node outside the manager (on the same port), so that manager node.Start() method fails - outsideNode, err := node.MakeNode(nodeConfig, db) + outsideNode, err := node.MakeNode(nodeConfig, nil, db) s.NoError(err) err = outsideNode.Start() s.NoError(err) // now try starting using node manager, it should fail (error is irrelevant as it is implementation detail) + s.NoError(s.backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) s.Error(<-api.RunAsync(func() error { return s.backend.StartNode(nodeConfig) })) select { diff --git a/t/e2e/api/backend_test.go b/t/e2e/api/backend_test.go index c91a1bf07..4c65c93a7 100644 --- a/t/e2e/api/backend_test.go +++ b/t/e2e/api/backend_test.go @@ -25,7 +25,7 @@ func (s *APIBackendTestSuite) TestNetworkSwitching() { // Get test node configuration. nodeConfig, err := MakeTestNodeConfig(GetNetworkID()) s.NoError(err) - + s.NoError(s.Backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) s.False(s.Backend.IsNodeRunning()) s.Require().NoError(s.Backend.StartNode(nodeConfig)) s.True(s.Backend.IsNodeRunning()) @@ -87,6 +87,7 @@ func (s *APIBackendTestSuite) TestRestartNode() { // get config nodeConfig, err := MakeTestNodeConfig(GetNetworkID()) s.NoError(err) + s.NoError(s.Backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) s.False(s.Backend.IsNodeRunning()) require.NoError(s.Backend.StartNode(nodeConfig)) diff --git a/t/e2e/node/manager_test.go b/t/e2e/node/manager_test.go index 4b10efd14..1c47c2946 100644 --- a/t/e2e/node/manager_test.go +++ b/t/e2e/node/manager_test.go @@ -5,7 +5,6 @@ import ( "time" "github.com/ethereum/go-ethereum/accounts" - "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/les" gethnode "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/rpc" @@ -71,20 +70,6 @@ func (s *ManagerTestSuite) TestReferencesWithoutStartedNode() { }, node.ErrNoRunningNode, }, - { - "non-null manager, no running node, get AccountManager", - func() (interface{}, error) { - return s.StatusNode.AccountManager() - }, - node.ErrNoGethNode, - }, - { - "non-null manager, no running node, get AccountKeyStore", - func() (interface{}, error) { - return s.StatusNode.AccountKeyStore() - }, - node.ErrNoGethNode, - }, { "non-null manager, no running node, get RPC Client", func() (interface{}, error) { @@ -148,13 +133,6 @@ func (s *ManagerTestSuite) TestReferencesWithStartedNode() { }, &accounts.Manager{}, }, - { - "node is running, get AccountKeyStore", - func() (interface{}, error) { - return s.StatusNode.AccountKeyStore() - }, - &keystore.KeyStore{}, - }, { "node is running, get RPC Client", func() (interface{}, error) { @@ -183,12 +161,12 @@ func (s *ManagerTestSuite) TestNodeStartStop() { // start node s.False(s.StatusNode.IsRunning()) - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) // wait till node is started s.True(s.StatusNode.IsRunning()) // try starting another node (w/o stopping the previously started node) - s.Equal(node.ErrNodeRunning, s.StatusNode.Start(nodeConfig)) + s.Equal(node.ErrNodeRunning, s.StatusNode.Start(nodeConfig, nil)) // now stop node time.Sleep(100 * time.Millisecond) //https://github.com/status-im/status-go/issues/429#issuecomment-339663163 @@ -196,7 +174,7 @@ func (s *ManagerTestSuite) TestNodeStartStop() { s.False(s.StatusNode.IsRunning()) // start new node with exactly the same config - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) s.True(s.StatusNode.IsRunning()) // finally stop the node @@ -209,7 +187,7 @@ func (s *ManagerTestSuite) TestNetworkSwitching() { nodeConfig, err := MakeTestNodeConfig(GetNetworkID()) s.NoError(err) s.False(s.StatusNode.IsRunning()) - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) // wait till node is started s.Require().True(s.StatusNode.IsRunning()) @@ -225,7 +203,7 @@ func (s *ManagerTestSuite) TestNetworkSwitching() { // start new node with completely different config nodeConfig, err = MakeTestNodeConfig(params.RinkebyNetworkID) s.NoError(err) - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) s.True(s.StatusNode.IsRunning()) // make sure we are on another network indeed @@ -251,7 +229,7 @@ func (s *ManagerTestSuite) TestStartWithUpstreamEnabled() { nodeConfig.UpstreamConfig.Enabled = true nodeConfig.UpstreamConfig.URL = networkURL - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) s.True(s.StatusNode.IsRunning()) time.Sleep(100 * time.Millisecond) //https://github.com/status-im/status-go/issues/429#issuecomment-339663163 diff --git a/t/e2e/rpc/rpc_test.go b/t/e2e/rpc/rpc_test.go index b194312c0..b6e13504c 100644 --- a/t/e2e/rpc/rpc_test.go +++ b/t/e2e/rpc/rpc_test.go @@ -49,7 +49,7 @@ func (s *RPCTestSuite) TestCallRPC() { nodeConfig.UpstreamConfig.URL = networkURL } - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) rpcClient := s.StatusNode.RPCClient() s.NotNil(rpcClient) @@ -127,7 +127,7 @@ func (s *RPCTestSuite) TestCallRawResult() { nodeConfig, err := MakeTestNodeConfig(GetNetworkID()) s.NoError(err) - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) client := s.StatusNode.RPCPrivateClient() s.NotNil(client) @@ -145,7 +145,7 @@ func (s *RPCTestSuite) TestCallRawResultGetTransactionReceipt() { nodeConfig, err := MakeTestNodeConfig(GetNetworkID()) s.NoError(err) - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) client := s.StatusNode.RPCClient() s.NotNil(client) diff --git a/t/e2e/services/base_api_test.go b/t/e2e/services/base_api_test.go index bbe115734..a77f1cbc1 100644 --- a/t/e2e/services/base_api_test.go +++ b/t/e2e/services/base_api_test.go @@ -73,6 +73,7 @@ func (s *BaseJSONRPCSuite) SetupTest(upstreamEnabled, statusServiceEnabled, debu nodeConfig, err := utils.MakeTestNodeConfig(utils.GetNetworkID()) s.NoError(err) + s.NoError(s.Backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) nodeConfig.IPCEnabled = false nodeConfig.EnableStatusService = statusServiceEnabled diff --git a/t/e2e/suites.go b/t/e2e/suites.go index 161f88e59..aa9613a29 100644 --- a/t/e2e/suites.go +++ b/t/e2e/suites.go @@ -51,7 +51,7 @@ func (s *StatusNodeTestSuite) StartTestNode(opts ...TestNodeOption) { s.NoError(importTestAccounts(nodeConfig.KeyStoreDir)) s.False(s.StatusNode.IsRunning()) - s.NoError(s.StatusNode.Start(nodeConfig)) + s.NoError(s.StatusNode.Start(nodeConfig, nil)) s.True(s.StatusNode.IsRunning()) } @@ -90,7 +90,7 @@ func (s *BackendTestSuite) StartTestBackend(opts ...TestNodeOption) { for i := range opts { opts[i](nodeConfig) } - + s.NoError(s.Backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) // import account keys s.NoError(importTestAccounts(nodeConfig.KeyStoreDir)) diff --git a/t/e2e/whisper/whisper_ext_test.go b/t/e2e/whisper/whisper_ext_test.go index 34e95cf69..aeb6292de 100644 --- a/t/e2e/whisper/whisper_ext_test.go +++ b/t/e2e/whisper/whisper_ext_test.go @@ -35,7 +35,7 @@ func (s *WhisperExtensionSuite) SetupTest() { cfg, err := utils.MakeTestNodeConfigWithDataDir(fmt.Sprintf("test-shhext-%d", i), dir, 777) s.Require().NoError(err) s.nodes[i] = node.New() - s.Require().NoError(s.nodes[i].Start(cfg)) + s.Require().NoError(s.nodes[i].Start(cfg, nil)) } } diff --git a/t/e2e/whisper/whisper_mailbox_test.go b/t/e2e/whisper/whisper_mailbox_test.go index d51db01c6..24dd3ec1b 100644 --- a/t/e2e/whisper/whisper_mailbox_test.go +++ b/t/e2e/whisper/whisper_mailbox_test.go @@ -708,7 +708,10 @@ func (s *WhisperMailboxSuite) startBackend(name string) (*api.StatusBackend, fun backend := api.NewStatusBackend() nodeConfig, err := utils.MakeTestNodeConfig(utils.GetNetworkID()) nodeConfig.DataDir = datadir + nodeConfig.KeyStoreDir = filepath.Join(datadir, "keystore") s.Require().NoError(err) + s.Require().NoError(backend.AccountManager().InitKeystore(nodeConfig.KeyStoreDir)) + s.Require().False(backend.IsNodeRunning()) nodeConfig.WhisperConfig.LightClient = true @@ -748,6 +751,7 @@ func (s *WhisperMailboxSuite) startMailboxBackendWithCallback( s.Require().NoError(err) mailboxBackend := api.NewStatusBackend() + s.Require().NoError(mailboxBackend.AccountManager().InitKeystore(mailboxConfig.KeyStoreDir)) datadir := filepath.Join(utils.RootDir, ".ethereumtest/mailbox", name) mailboxConfig.LightEthConfig.Enabled = false diff --git a/t/e2e/whisper/whisper_test.go b/t/e2e/whisper/whisper_test.go index 1521a0b77..1e8bf98d5 100644 --- a/t/e2e/whisper/whisper_test.go +++ b/t/e2e/whisper/whisper_test.go @@ -40,7 +40,7 @@ func (s *WhisperTestSuite) TestWhisperFilterRace() { whisperService, err := s.Backend.StatusNode().WhisperService() s.NoError(err) - accountManager := account.NewManager(s.Backend.StatusNode()) + accountManager := s.Backend.AccountManager() s.NotNil(accountManager) whisperAPI := whisper.NewPublicWhisperAPI(whisperService)