From e0cc32a5acda16176f3292b5c2a88abbd86ba619 Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 20 Mar 2024 13:44:39 +0530 Subject: [PATCH 1/9] refactor: used regex instead of ks.Accounts to match keystore file --- accounts/accountUtils.go | 21 ----------- accounts/accounts.go | 75 ++++++++++++++++++++++++++++++++-------- 2 files changed, 61 insertions(+), 35 deletions(-) diff --git a/accounts/accountUtils.go b/accounts/accountUtils.go index 11f8a5f10..bfa234ff6 100644 --- a/accounts/accountUtils.go +++ b/accounts/accountUtils.go @@ -6,7 +6,6 @@ import ( "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/crypto" - "os" "razor/core/types" ) @@ -16,24 +15,14 @@ var AccountUtilsInterface AccountInterface type AccountInterface interface { CreateAccount(path string, password string) accounts.Account - GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) SignData(hash []byte, account types.Account, defaultPath string) ([]byte, error) - Accounts(path string) []accounts.Account NewAccount(path string, passphrase string) (accounts.Account, error) - DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) - ReadFile(filename string) ([]byte, error) } type AccountUtils struct{} -//This function returns all the accounts in form of array -func (accountUtils AccountUtils) Accounts(path string) []accounts.Account { - ks := keystore.NewKeyStore(path, keystore.StandardScryptN, keystore.StandardScryptP) - return ks.Accounts() -} - //This function takes path and pass phrase as input and returns the new account func (accountUtils AccountUtils) NewAccount(path string, passphrase string) (accounts.Account, error) { ks := keystore.NewKeyStore(path, keystore.StandardScryptN, keystore.StandardScryptP) @@ -41,17 +30,7 @@ func (accountUtils AccountUtils) NewAccount(path string, passphrase string) (acc return ks.NewAccount(passphrase) } -//This function takes json bytes array and password as input and returns the decrypted key -func (accountUtils AccountUtils) DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) { - return keystore.DecryptKey(jsonBytes, password) -} - //This function takes hash in form of byte array and private key as input and returns signature as byte array func (accountUtils AccountUtils) Sign(digestHash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { return crypto.Sign(digestHash, prv) } - -//This function takes name of the file as input and returns the file data as byte array -func (accountUtils AccountUtils) ReadFile(filename string) ([]byte, error) { - return os.ReadFile(filename) -} diff --git a/accounts/accounts.go b/accounts/accounts.go index b907dfef9..a6742c5bb 100644 --- a/accounts/accounts.go +++ b/accounts/accounts.go @@ -4,10 +4,15 @@ package accounts import ( "crypto/ecdsa" "errors" + "fmt" "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/keystore" + "os" + "path/filepath" "razor/core/types" "razor/logger" "razor/path" + "regexp" "strings" ) @@ -28,29 +33,30 @@ func (AccountUtils) CreateAccount(keystorePath string, password string) accounts return newAcc } -//This function takes and path of keystore and password as input and returns private key of account -func (AccountUtils) GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) { - jsonBytes, err := AccountUtilsInterface.ReadFile(keystorePath) +//This function takes address of account, password and keystore path as input and returns private key of account +func (AccountUtils) GetPrivateKey(address string, password string, keystoreDirPath string) (*ecdsa.PrivateKey, error) { + fileName, err := FindKeystoreFileForAddress(keystoreDirPath, address) + if err != nil { + log.Error("Error in finding keystore file for an address: ", err) + return nil, err + } + + keyJson, err := os.ReadFile(fileName) if err != nil { log.Error("Error in reading keystore: ", err) return nil, err } - key, err := AccountUtilsInterface.DecryptKey(jsonBytes, password) + key, err := keystore.DecryptKey(keyJson, password) if err != nil { - log.Error("Error in fetching private key: ", err) + log.Error("Error in decrypting private key: ", err) return nil, err } - return key.PrivateKey, nil -} -//This function takes address of account, password and keystore path as input and returns private key of account -func (AccountUtils) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) { - allAccounts := AccountUtilsInterface.Accounts(keystorePath) - for _, account := range allAccounts { - if strings.EqualFold(account.Address.Hex(), address) { - return AccountUtilsInterface.GetPrivateKeyFromKeystore(account.URL.Path, password) - } + // Check if the input address matches with address from keystore file in which password was matched + if key.Address.Hex() == address { + return key.PrivateKey, nil } + return nil, errors.New("no keystore file found") } @@ -62,3 +68,44 @@ func (AccountUtils) SignData(hash []byte, account types.Account, defaultPath str } return AccountUtilsInterface.Sign(hash, privateKey) } + +// FindKeystoreFileForAddress matches the keystore file for the given address. +func FindKeystoreFileForAddress(keystoreDirPath, address string) (string, error) { + normalizedAddress := strings.ToLower(address) + if strings.HasPrefix(normalizedAddress, "0x") { + normalizedAddress = normalizedAddress[2:] + } + regexPattern := fmt.Sprintf("^UTC--.*--%s$", regexp.QuoteMeta(normalizedAddress)) + re, err := regexp.Compile(regexPattern) + if err != nil { + log.Errorf("Error in compiling regex: %v", err) + return "", err + } + + var keystoreFilePath string + err = filepath.WalkDir(keystoreDirPath, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err // Propagate errors encountered during traversal + } + if d.IsDir() { + return nil // Skip directories, continue walking + } + if re.MatchString(d.Name()) { // Check if file name matches the regex + keystoreFilePath = path + return filepath.SkipDir // File found, no need to continue + } + return nil + }) + + if err != nil { + log.Errorf("Error walking through keystore directory: %v", err) + return "", err + } + + if keystoreFilePath == "" { + log.Errorf("No matching keystore file found for address %s", address) + return "", errors.New("no matching keystore file found") + } + + return keystoreFilePath, nil +} From 7c7e1ff5dc505f5db246232eec614ca2d3ddea2c Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 20 Mar 2024 13:45:56 +0530 Subject: [PATCH 2/9] refactor: added more extensive realistic testing for fetching of private key --- accounts/accounts_test.go | 187 ++++++++---------- accounts/mocks/account_interface.go | 118 ++--------- ...--911654feb423363fb771e04e18d1e7325ae10a91 | 1 + ...--2f5f59615689b706b6ad13fd03343dca28784989 | 1 + ...--811654feb423363fb771e04e18d1e7325ae10a91 | 1 + ...--811654feb423363fb771e04e18d1e7325ae10a91 | 1 + 6 files changed, 111 insertions(+), 198 deletions(-) create mode 100644 accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 create mode 100644 accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 create mode 100644 accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 create mode 100644 accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go index aa964bf47..de2f4c006 100644 --- a/accounts/accounts_test.go +++ b/accounts/accounts_test.go @@ -4,11 +4,11 @@ import ( "crypto/ecdsa" "errors" "github.com/ethereum/go-ethereum/accounts" - "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" "github.com/magiconair/properties/assert" "github.com/stretchr/testify/mock" "io/fs" + "path/filepath" "razor/accounts/mocks" "razor/core/types" "razor/path" @@ -116,140 +116,68 @@ func TestCreateAccount(t *testing.T) { } } -func TestGetPrivateKeyFromKeystore(t *testing.T) { - var password string - var keystorePath string - var privateKey *ecdsa.PrivateKey - var jsonBytes []byte +func TestGetPrivateKey(t *testing.T) { + password := "Razor@123" + keystoreDirPath := "test_accounts" type args struct { - jsonBytes []byte - jsonBytesErr error - key *keystore.Key - keyErr error + address string + password string + keystoreDirPath string } tests := []struct { name string args args - want *ecdsa.PrivateKey wantErr bool }{ { - name: "Test 1: When GetPrivateKey function executes successfully", + name: "Test 1: When input address with correct password is present in keystore directory", args: args{ - jsonBytes: jsonBytes, - key: &keystore.Key{ - PrivateKey: privateKey, - }, + address: "0x911654feb423363fb771e04e18d1e7325ae10a91", + password: password, + keystoreDirPath: keystoreDirPath, }, - want: privateKey, wantErr: false, }, { - name: "Test 2: When there is an error in reading data from file", + name: "Test 2: When another input address with correct password is present in keystore directory", args: args{ - jsonBytesErr: errors.New("error in reading data"), - key: &keystore.Key{ - PrivateKey: nil, - }, + address: "0x2f5f59615689b706b6ad13fd03343dca28784989", + password: password, + keystoreDirPath: keystoreDirPath, }, - want: nil, - wantErr: true, + wantErr: false, }, { - name: "Test 3: When there is an error in fetching private key", + name: "Test 3: When provided address is not present in keystore directory", args: args{ - jsonBytes: jsonBytes, - key: &keystore.Key{ - PrivateKey: nil, - }, - keyErr: errors.New("private key error"), + address: "0x911654feb423363fb771e04e18d1e7325ae10a91_not_present", + keystoreDirPath: keystoreDirPath, }, - want: privateKey, wantErr: true, }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - accountsMock := new(mocks.AccountInterface) - AccountUtilsInterface = accountsMock - - accountsMock.On("ReadFile", mock.AnythingOfType("string")).Return(tt.args.jsonBytes, tt.args.jsonBytesErr) - accountsMock.On("DecryptKey", mock.Anything, mock.AnythingOfType("string")).Return(tt.args.key, tt.args.keyErr) - - accountUtils := &AccountUtils{} - got, err := accountUtils.GetPrivateKeyFromKeystore(keystorePath, password) - if got != tt.want { - t.Errorf("Private key from GetPrivateKey, got = %v, want %v", got, tt.want) - } - if (err != nil) != tt.wantErr { - t.Errorf("GetPrivateKeyFromKeystore() error = %v, wantErr %v", err, tt.wantErr) - return - } - }) - } -} - -func TestGetPrivateKey(t *testing.T) { - var password string - var keystorePath string - var privateKey *ecdsa.PrivateKey - - accountsList := []accounts.Account{ - {Address: common.HexToAddress("0x000000000000000000000000000000000000dea1"), - URL: accounts.URL{Scheme: "TestKeyScheme", Path: "test/key/path"}, - }, - {Address: common.HexToAddress("0x000000000000000000000000000000000000dea2"), - URL: accounts.URL{Scheme: "TestKeyScheme", Path: "test/key/path"}, - }, - } - - type args struct { - address string - accounts []accounts.Account - privateKey *ecdsa.PrivateKey - } - tests := []struct { - name string - args args - want *ecdsa.PrivateKey - wantErr bool - }{ { - name: "Test 1: When input address is present in accountsList", + name: "Test 4: When input address with incorrect password is present in keystore directory", args: args{ - address: "0x000000000000000000000000000000000000dea1", - accounts: accountsList, - privateKey: privateKey, + address: "0x911654feb423363fb771e04e18d1e7325ae10a91", + password: "incorrect password", + keystoreDirPath: keystoreDirPath, }, - want: privateKey, - wantErr: false, + wantErr: true, }, { - name: "Test 2: When input address is not present in accountsList", + name: "Test 5: When a keystore file is renamed differently from the address to which it belonged", args: args{ - address: "0x000000000000000000000000000000000000dea3", - accounts: accountsList, - privateKey: privateKey, + address: "0x811654feb423363fb771e04e18d1e7325ae10a91", + password: password, + keystoreDirPath: "test_accounts/incorrect_test_accounts", }, - want: nil, - wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - accountsMock := new(mocks.AccountInterface) - AccountUtilsInterface = accountsMock - - accountsMock.On("Accounts", mock.AnythingOfType("string")).Return(tt.args.accounts) - accountsMock.On("GetPrivateKeyFromKeystore", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(tt.args.privateKey, nil) - accountUtils := &AccountUtils{} - got, err := accountUtils.GetPrivateKey(tt.args.address, password, keystorePath) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetPrivateKey() got = %v, want %v", got, tt.want) - } + _, err := accountUtils.GetPrivateKey(tt.args.address, tt.args.password, tt.args.keystoreDirPath) if (err != nil) != tt.wantErr { t.Errorf("GetPrivateKey() error = %v, wantErr %v", err, tt.wantErr) return @@ -319,3 +247,60 @@ func TestSignData(t *testing.T) { }) } } + +func TestFindKeystoreFileForAddress(t *testing.T) { + testAccountsKeystorePath := "test_accounts" + + tests := []struct { + name string + keystoreDir string + address string + expectedFile string + expectErr bool + }{ + { + name: "Matching file exists for an address", + keystoreDir: testAccountsKeystorePath, + address: "0x911654feb423363fb771e04e18d1e7325ae10a91", + expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91"), + expectErr: false, + }, + { + name: "Matching file exists for another address", + keystoreDir: testAccountsKeystorePath, + address: "0x2f5f59615689b706b6ad13fd03343dca28784989", + expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989"), + expectErr: false, + }, + { + name: "No matching file", + keystoreDir: testAccountsKeystorePath, + address: "nonexistentaddress", + expectErr: true, + }, + { + name: "When keystore directory doesnt exists", + keystoreDir: "test_accounts_invalid", + address: "0x2f5f59615689b706b6ad13fd03343dca28784989", + expectErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := FindKeystoreFileForAddress(tc.keystoreDir, tc.address) + if tc.expectErr { + if err == nil { + t.Errorf("Expected an error but got none") + } + } else { + if err != nil { + t.Errorf("Did not expect an error but got one: %v", err) + } + if got != tc.expectedFile { + t.Errorf("Expected file %v, got %v", tc.expectedFile, got) + } + } + }) + } +} diff --git a/accounts/mocks/account_interface.go b/accounts/mocks/account_interface.go index e8e006441..ed1e4b9f5 100644 --- a/accounts/mocks/account_interface.go +++ b/accounts/mocks/account_interface.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.14.0. DO NOT EDIT. +// Code generated by mockery v2.30.1. DO NOT EDIT. package mocks @@ -7,8 +7,6 @@ import ( accounts "github.com/ethereum/go-ethereum/accounts" - keystore "github.com/ethereum/go-ethereum/accounts/keystore" - mock "github.com/stretchr/testify/mock" types "razor/core/types" @@ -19,22 +17,6 @@ type AccountInterface struct { mock.Mock } -// Accounts provides a mock function with given fields: path -func (_m *AccountInterface) Accounts(path string) []accounts.Account { - ret := _m.Called(path) - - var r0 []accounts.Account - if rf, ok := ret.Get(0).(func(string) []accounts.Account); ok { - r0 = rf(path) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]accounts.Account) - } - } - - return r0 -} - // CreateAccount provides a mock function with given fields: path, password func (_m *AccountInterface) CreateAccount(path string, password string) accounts.Account { ret := _m.Called(path, password) @@ -49,34 +31,15 @@ func (_m *AccountInterface) CreateAccount(path string, password string) accounts return r0 } -// DecryptKey provides a mock function with given fields: jsonBytes, password -func (_m *AccountInterface) DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) { - ret := _m.Called(jsonBytes, password) - - var r0 *keystore.Key - if rf, ok := ret.Get(0).(func([]byte, string) *keystore.Key); ok { - r0 = rf(jsonBytes, password) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*keystore.Key) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func([]byte, string) error); ok { - r1 = rf(jsonBytes, password) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // GetPrivateKey provides a mock function with given fields: address, password, keystorePath func (_m *AccountInterface) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) { ret := _m.Called(address, password, keystorePath) var r0 *ecdsa.PrivateKey + var r1 error + if rf, ok := ret.Get(0).(func(string, string, string) (*ecdsa.PrivateKey, error)); ok { + return rf(address, password, keystorePath) + } if rf, ok := ret.Get(0).(func(string, string, string) *ecdsa.PrivateKey); ok { r0 = rf(address, password, keystorePath) } else { @@ -85,7 +48,6 @@ func (_m *AccountInterface) GetPrivateKey(address string, password string, keyst } } - var r1 error if rf, ok := ret.Get(1).(func(string, string, string) error); ok { r1 = rf(address, password, keystorePath) } else { @@ -95,41 +57,21 @@ func (_m *AccountInterface) GetPrivateKey(address string, password string, keyst return r0, r1 } -// GetPrivateKeyFromKeystore provides a mock function with given fields: keystorePath, password -func (_m *AccountInterface) GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) { - ret := _m.Called(keystorePath, password) - - var r0 *ecdsa.PrivateKey - if rf, ok := ret.Get(0).(func(string, string) *ecdsa.PrivateKey); ok { - r0 = rf(keystorePath, password) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*ecdsa.PrivateKey) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(string, string) error); ok { - r1 = rf(keystorePath, password) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // NewAccount provides a mock function with given fields: path, passphrase func (_m *AccountInterface) NewAccount(path string, passphrase string) (accounts.Account, error) { ret := _m.Called(path, passphrase) var r0 accounts.Account + var r1 error + if rf, ok := ret.Get(0).(func(string, string) (accounts.Account, error)); ok { + return rf(path, passphrase) + } if rf, ok := ret.Get(0).(func(string, string) accounts.Account); ok { r0 = rf(path, passphrase) } else { r0 = ret.Get(0).(accounts.Account) } - var r1 error if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(path, passphrase) } else { @@ -139,34 +81,15 @@ func (_m *AccountInterface) NewAccount(path string, passphrase string) (accounts return r0, r1 } -// ReadFile provides a mock function with given fields: filename -func (_m *AccountInterface) ReadFile(filename string) ([]byte, error) { - ret := _m.Called(filename) - - var r0 []byte - if rf, ok := ret.Get(0).(func(string) []byte); ok { - r0 = rf(filename) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(filename) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // Sign provides a mock function with given fields: digestHash, prv func (_m *AccountInterface) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) { ret := _m.Called(digestHash, prv) var r0 []byte + var r1 error + if rf, ok := ret.Get(0).(func([]byte, *ecdsa.PrivateKey) ([]byte, error)); ok { + return rf(digestHash, prv) + } if rf, ok := ret.Get(0).(func([]byte, *ecdsa.PrivateKey) []byte); ok { r0 = rf(digestHash, prv) } else { @@ -175,7 +98,6 @@ func (_m *AccountInterface) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]by } } - var r1 error if rf, ok := ret.Get(1).(func([]byte, *ecdsa.PrivateKey) error); ok { r1 = rf(digestHash, prv) } else { @@ -190,6 +112,10 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default ret := _m.Called(hash, account, defaultPath) var r0 []byte + var r1 error + if rf, ok := ret.Get(0).(func([]byte, types.Account, string) ([]byte, error)); ok { + return rf(hash, account, defaultPath) + } if rf, ok := ret.Get(0).(func([]byte, types.Account, string) []byte); ok { r0 = rf(hash, account, defaultPath) } else { @@ -198,7 +124,6 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default } } - var r1 error if rf, ok := ret.Get(1).(func([]byte, types.Account, string) error); ok { r1 = rf(hash, account, defaultPath) } else { @@ -208,13 +133,12 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default return r0, r1 } -type mockConstructorTestingTNewAccountInterface interface { +// NewAccountInterface creates a new instance of AccountInterface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAccountInterface(t interface { mock.TestingT Cleanup(func()) -} - -// NewAccountInterface creates a new instance of AccountInterface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewAccountInterface(t mockConstructorTestingTNewAccountInterface) *AccountInterface { +}) *AccountInterface { mock := &AccountInterface{} mock.Mock.Test(t) diff --git a/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 new file mode 100644 index 000000000..bc50d72f6 --- /dev/null +++ b/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 @@ -0,0 +1 @@ +{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 b/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 new file mode 100644 index 000000000..c773f086e --- /dev/null +++ b/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 @@ -0,0 +1 @@ +{"address":"2f5f59615689b706b6ad13fd03343dca28784989","crypto":{"cipher":"aes-128-ctr","ciphertext":"e46770162aa3d74c00f1b7dff2a6e255743e61483d3f6a4c266c2697ac05aed3","cipherparams":{"iv":"8941588ce2540c7bef63f0df4a67e6cb"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"e106a7ef7916893083977dbe18cd5edf57491ba21a13adc2db7b79c7c8b85ded"},"mac":"d30b6bf80ea2ddbaca14f7635596e56ac150bda4416e9dc915117b8574cda2ba"},"id":"0accaaa4-2e5c-451b-b813-6551f9c01511","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 new file mode 100644 index 000000000..bc50d72f6 --- /dev/null +++ b/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 @@ -0,0 +1 @@ +{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 new file mode 100644 index 000000000..bc50d72f6 --- /dev/null +++ b/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 @@ -0,0 +1 @@ +{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file From 1c2692b29682d07372d955b15d0b388093c6be4e Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 20 Mar 2024 14:02:26 +0530 Subject: [PATCH 3/9] ci: ignore test-accounts directory in coverage --- .github/workflows/ci.yml | 2 +- .github/workflows/develop.yml | 2 +- .github/workflows/release.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0e2167b2..5296ecdf5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,7 @@ jobs: - name: Execute test case run: > - go-acc ./... --ignore razor/accounts/mocks --ignore razor/cmd/mocks + go-acc ./... --ignore razor/accounts/mocks --ignore razor/accounts/test_accounts --ignore razor/cmd/mocks --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt - name: Run benchmarks diff --git a/.github/workflows/develop.yml b/.github/workflows/develop.yml index e9f4a9baf..d58e6337d 100644 --- a/.github/workflows/develop.yml +++ b/.github/workflows/develop.yml @@ -61,7 +61,7 @@ jobs: golangci-lint run -v --timeout 5m - name: Execute test case run: | - go-acc ./... --ignore razor/accounts/mocks --ignore razor/cmd/mocks --ignore razor/cmd/eventListeners.go --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt + go-acc ./... --ignore razor/accounts/mocks --ignore razor/accounts/test_accounts --ignore razor/cmd/mocks --ignore razor/cmd/eventListeners.go --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt - name: Run benchmarks run: | go test ./... -bench=. -run=^# diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d13c625f4..b4c7e9acd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -58,7 +58,7 @@ jobs: golangci-lint run -v --timeout 5m - name: Execute test case run: | - go-acc ./... --ignore razor/accounts/mocks --ignore razor/cmd/mocks --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt + go-acc ./... --ignore razor/accounts/mocks --ignore razor/accounts/test_accounts --ignore razor/cmd/mocks --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt - name: Run benchmarks run: | go test ./... -bench=. -run=^# From 9be610b5ab34086247593689dce968195ea42b87 Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 20 Mar 2024 14:09:59 +0530 Subject: [PATCH 4/9] refactor: added test when multiple timestamps keystore file present for same account --- accounts/accounts_test.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go index de2f4c006..2a9603ea4 100644 --- a/accounts/accounts_test.go +++ b/accounts/accounts_test.go @@ -259,31 +259,38 @@ func TestFindKeystoreFileForAddress(t *testing.T) { expectErr bool }{ { - name: "Matching file exists for an address", + name: "Test 1: Matching file exists for an address", keystoreDir: testAccountsKeystorePath, address: "0x911654feb423363fb771e04e18d1e7325ae10a91", expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91"), expectErr: false, }, { - name: "Matching file exists for another address", + name: "Test 2: Matching file exists for another address", keystoreDir: testAccountsKeystorePath, address: "0x2f5f59615689b706b6ad13fd03343dca28784989", expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989"), expectErr: false, }, { - name: "No matching file", + name: "Test 3: No matching file", keystoreDir: testAccountsKeystorePath, address: "nonexistentaddress", expectErr: true, }, { - name: "When keystore directory doesnt exists", + name: "Test 4: When keystore directory doesnt exists", keystoreDir: "test_accounts_invalid", address: "0x2f5f59615689b706b6ad13fd03343dca28784989", expectErr: true, }, + { + name: "Test 5: When multiple files for same account is present in the keystore directory", + keystoreDir: "test_accounts/incorrect_test_accounts", + address: "0x811654feb423363fb771e04e18d1e7325ae10a91", + expectedFile: filepath.Join("test_accounts/incorrect_test_accounts", "UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91"), + expectErr: true, + }, } for _, tc := range tests { From b5eeabc8a8660924a9dd5853dc651851344ca07e Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 20 Mar 2024 15:21:16 +0530 Subject: [PATCH 5/9] refactor: covered lower and upper case addresses --- accounts/accounts.go | 7 ++----- accounts/accounts_test.go | 13 +++++++------ 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/accounts/accounts.go b/accounts/accounts.go index a6742c5bb..8cb7de6b4 100644 --- a/accounts/accounts.go +++ b/accounts/accounts.go @@ -53,7 +53,7 @@ func (AccountUtils) GetPrivateKey(address string, password string, keystoreDirPa } // Check if the input address matches with address from keystore file in which password was matched - if key.Address.Hex() == address { + if strings.EqualFold(key.Address.Hex(), address) { return key.PrivateKey, nil } @@ -71,10 +71,7 @@ func (AccountUtils) SignData(hash []byte, account types.Account, defaultPath str // FindKeystoreFileForAddress matches the keystore file for the given address. func FindKeystoreFileForAddress(keystoreDirPath, address string) (string, error) { - normalizedAddress := strings.ToLower(address) - if strings.HasPrefix(normalizedAddress, "0x") { - normalizedAddress = normalizedAddress[2:] - } + normalizedAddress := strings.ToLower(strings.TrimPrefix(address, "0x")) regexPattern := fmt.Sprintf("^UTC--.*--%s$", regexp.QuoteMeta(normalizedAddress)) re, err := regexp.Compile(regexPattern) if err != nil { diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go index 2a9603ea4..7e0b860ca 100644 --- a/accounts/accounts_test.go +++ b/accounts/accounts_test.go @@ -140,9 +140,9 @@ func TestGetPrivateKey(t *testing.T) { wantErr: false, }, { - name: "Test 2: When another input address with correct password is present in keystore directory", + name: "Test 2: When input upper case address with correct password is present in keystore directory", args: args{ - address: "0x2f5f59615689b706b6ad13fd03343dca28784989", + address: "0x2F5F59615689B706B6AD13FD03343DCA28784989", password: password, keystoreDirPath: keystoreDirPath, }, @@ -172,6 +172,7 @@ func TestGetPrivateKey(t *testing.T) { password: password, keystoreDirPath: "test_accounts/incorrect_test_accounts", }, + wantErr: true, }, } for _, tt := range tests { @@ -266,9 +267,9 @@ func TestFindKeystoreFileForAddress(t *testing.T) { expectErr: false, }, { - name: "Test 2: Matching file exists for another address", + name: "Test 2: Matching file exists for upper case address", keystoreDir: testAccountsKeystorePath, - address: "0x2f5f59615689b706b6ad13fd03343dca28784989", + address: "0x2F5F59615689B706B6AD13FD03343DCA28784989", expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989"), expectErr: false, }, @@ -288,8 +289,8 @@ func TestFindKeystoreFileForAddress(t *testing.T) { name: "Test 5: When multiple files for same account is present in the keystore directory", keystoreDir: "test_accounts/incorrect_test_accounts", address: "0x811654feb423363fb771e04e18d1e7325ae10a91", - expectedFile: filepath.Join("test_accounts/incorrect_test_accounts", "UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91"), - expectErr: true, + expectedFile: filepath.Join("test_accounts/incorrect_test_accounts", "UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91"), + expectErr: false, }, } From c0ff52cd27da75d5b9bac70e1a69e8180f4c7efc Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 27 Mar 2024 13:26:12 +0530 Subject: [PATCH 6/9] revert: optimised fetching key from keystore dir --- .github/workflows/ci.yml | 2 +- .github/workflows/develop.yml | 2 +- .github/workflows/release.yml | 2 +- accounts/accountUtils.go | 21 ++ accounts/accounts.go | 72 ++----- accounts/accounts_test.go | 193 +++++++++--------- accounts/mocks/account_interface.go | 118 +++++++++-- ...--911654feb423363fb771e04e18d1e7325ae10a91 | 1 - ...--2f5f59615689b706b6ad13fd03343dca28784989 | 1 - ...--811654feb423363fb771e04e18d1e7325ae10a91 | 1 - ...--811654feb423363fb771e04e18d1e7325ae10a91 | 1 - 11 files changed, 235 insertions(+), 179 deletions(-) delete mode 100644 accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 delete mode 100644 accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 delete mode 100644 accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 delete mode 100644 accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5296ecdf5..c0e2167b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,7 @@ jobs: - name: Execute test case run: > - go-acc ./... --ignore razor/accounts/mocks --ignore razor/accounts/test_accounts --ignore razor/cmd/mocks + go-acc ./... --ignore razor/accounts/mocks --ignore razor/cmd/mocks --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt - name: Run benchmarks diff --git a/.github/workflows/develop.yml b/.github/workflows/develop.yml index d58e6337d..e9f4a9baf 100644 --- a/.github/workflows/develop.yml +++ b/.github/workflows/develop.yml @@ -61,7 +61,7 @@ jobs: golangci-lint run -v --timeout 5m - name: Execute test case run: | - go-acc ./... --ignore razor/accounts/mocks --ignore razor/accounts/test_accounts --ignore razor/cmd/mocks --ignore razor/cmd/eventListeners.go --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt + go-acc ./... --ignore razor/accounts/mocks --ignore razor/cmd/mocks --ignore razor/cmd/eventListeners.go --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt - name: Run benchmarks run: | go test ./... -bench=. -run=^# diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b4c7e9acd..d13c625f4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -58,7 +58,7 @@ jobs: golangci-lint run -v --timeout 5m - name: Execute test case run: | - go-acc ./... --ignore razor/accounts/mocks --ignore razor/accounts/test_accounts --ignore razor/cmd/mocks --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt + go-acc ./... --ignore razor/accounts/mocks --ignore razor/cmd/mocks --ignore razor/utils/mocks --ignore pkg --ignore razor/path/mocks --output coverage.txt - name: Run benchmarks run: | go test ./... -bench=. -run=^# diff --git a/accounts/accountUtils.go b/accounts/accountUtils.go index bfa234ff6..11f8a5f10 100644 --- a/accounts/accountUtils.go +++ b/accounts/accountUtils.go @@ -6,6 +6,7 @@ import ( "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/crypto" + "os" "razor/core/types" ) @@ -15,14 +16,24 @@ var AccountUtilsInterface AccountInterface type AccountInterface interface { CreateAccount(path string, password string) accounts.Account + GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) SignData(hash []byte, account types.Account, defaultPath string) ([]byte, error) + Accounts(path string) []accounts.Account NewAccount(path string, passphrase string) (accounts.Account, error) + DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) + ReadFile(filename string) ([]byte, error) } type AccountUtils struct{} +//This function returns all the accounts in form of array +func (accountUtils AccountUtils) Accounts(path string) []accounts.Account { + ks := keystore.NewKeyStore(path, keystore.StandardScryptN, keystore.StandardScryptP) + return ks.Accounts() +} + //This function takes path and pass phrase as input and returns the new account func (accountUtils AccountUtils) NewAccount(path string, passphrase string) (accounts.Account, error) { ks := keystore.NewKeyStore(path, keystore.StandardScryptN, keystore.StandardScryptP) @@ -30,7 +41,17 @@ func (accountUtils AccountUtils) NewAccount(path string, passphrase string) (acc return ks.NewAccount(passphrase) } +//This function takes json bytes array and password as input and returns the decrypted key +func (accountUtils AccountUtils) DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) { + return keystore.DecryptKey(jsonBytes, password) +} + //This function takes hash in form of byte array and private key as input and returns signature as byte array func (accountUtils AccountUtils) Sign(digestHash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { return crypto.Sign(digestHash, prv) } + +//This function takes name of the file as input and returns the file data as byte array +func (accountUtils AccountUtils) ReadFile(filename string) ([]byte, error) { + return os.ReadFile(filename) +} diff --git a/accounts/accounts.go b/accounts/accounts.go index 8cb7de6b4..b907dfef9 100644 --- a/accounts/accounts.go +++ b/accounts/accounts.go @@ -4,15 +4,10 @@ package accounts import ( "crypto/ecdsa" "errors" - "fmt" "github.com/ethereum/go-ethereum/accounts" - "github.com/ethereum/go-ethereum/accounts/keystore" - "os" - "path/filepath" "razor/core/types" "razor/logger" "razor/path" - "regexp" "strings" ) @@ -33,30 +28,29 @@ func (AccountUtils) CreateAccount(keystorePath string, password string) accounts return newAcc } -//This function takes address of account, password and keystore path as input and returns private key of account -func (AccountUtils) GetPrivateKey(address string, password string, keystoreDirPath string) (*ecdsa.PrivateKey, error) { - fileName, err := FindKeystoreFileForAddress(keystoreDirPath, address) - if err != nil { - log.Error("Error in finding keystore file for an address: ", err) - return nil, err - } - - keyJson, err := os.ReadFile(fileName) +//This function takes and path of keystore and password as input and returns private key of account +func (AccountUtils) GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) { + jsonBytes, err := AccountUtilsInterface.ReadFile(keystorePath) if err != nil { log.Error("Error in reading keystore: ", err) return nil, err } - key, err := keystore.DecryptKey(keyJson, password) + key, err := AccountUtilsInterface.DecryptKey(jsonBytes, password) if err != nil { - log.Error("Error in decrypting private key: ", err) + log.Error("Error in fetching private key: ", err) return nil, err } + return key.PrivateKey, nil +} - // Check if the input address matches with address from keystore file in which password was matched - if strings.EqualFold(key.Address.Hex(), address) { - return key.PrivateKey, nil +//This function takes address of account, password and keystore path as input and returns private key of account +func (AccountUtils) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) { + allAccounts := AccountUtilsInterface.Accounts(keystorePath) + for _, account := range allAccounts { + if strings.EqualFold(account.Address.Hex(), address) { + return AccountUtilsInterface.GetPrivateKeyFromKeystore(account.URL.Path, password) + } } - return nil, errors.New("no keystore file found") } @@ -68,41 +62,3 @@ func (AccountUtils) SignData(hash []byte, account types.Account, defaultPath str } return AccountUtilsInterface.Sign(hash, privateKey) } - -// FindKeystoreFileForAddress matches the keystore file for the given address. -func FindKeystoreFileForAddress(keystoreDirPath, address string) (string, error) { - normalizedAddress := strings.ToLower(strings.TrimPrefix(address, "0x")) - regexPattern := fmt.Sprintf("^UTC--.*--%s$", regexp.QuoteMeta(normalizedAddress)) - re, err := regexp.Compile(regexPattern) - if err != nil { - log.Errorf("Error in compiling regex: %v", err) - return "", err - } - - var keystoreFilePath string - err = filepath.WalkDir(keystoreDirPath, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err // Propagate errors encountered during traversal - } - if d.IsDir() { - return nil // Skip directories, continue walking - } - if re.MatchString(d.Name()) { // Check if file name matches the regex - keystoreFilePath = path - return filepath.SkipDir // File found, no need to continue - } - return nil - }) - - if err != nil { - log.Errorf("Error walking through keystore directory: %v", err) - return "", err - } - - if keystoreFilePath == "" { - log.Errorf("No matching keystore file found for address %s", address) - return "", errors.New("no matching keystore file found") - } - - return keystoreFilePath, nil -} diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go index 7e0b860ca..aa964bf47 100644 --- a/accounts/accounts_test.go +++ b/accounts/accounts_test.go @@ -4,11 +4,11 @@ import ( "crypto/ecdsa" "errors" "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" "github.com/magiconair/properties/assert" "github.com/stretchr/testify/mock" "io/fs" - "path/filepath" "razor/accounts/mocks" "razor/core/types" "razor/path" @@ -116,69 +116,140 @@ func TestCreateAccount(t *testing.T) { } } -func TestGetPrivateKey(t *testing.T) { - password := "Razor@123" - keystoreDirPath := "test_accounts" +func TestGetPrivateKeyFromKeystore(t *testing.T) { + var password string + var keystorePath string + var privateKey *ecdsa.PrivateKey + var jsonBytes []byte type args struct { - address string - password string - keystoreDirPath string + jsonBytes []byte + jsonBytesErr error + key *keystore.Key + keyErr error } tests := []struct { name string args args + want *ecdsa.PrivateKey wantErr bool }{ { - name: "Test 1: When input address with correct password is present in keystore directory", + name: "Test 1: When GetPrivateKey function executes successfully", args: args{ - address: "0x911654feb423363fb771e04e18d1e7325ae10a91", - password: password, - keystoreDirPath: keystoreDirPath, + jsonBytes: jsonBytes, + key: &keystore.Key{ + PrivateKey: privateKey, + }, }, + want: privateKey, wantErr: false, }, { - name: "Test 2: When input upper case address with correct password is present in keystore directory", + name: "Test 2: When there is an error in reading data from file", args: args{ - address: "0x2F5F59615689B706B6AD13FD03343DCA28784989", - password: password, - keystoreDirPath: keystoreDirPath, + jsonBytesErr: errors.New("error in reading data"), + key: &keystore.Key{ + PrivateKey: nil, + }, }, - wantErr: false, + want: nil, + wantErr: true, }, { - name: "Test 3: When provided address is not present in keystore directory", + name: "Test 3: When there is an error in fetching private key", args: args{ - address: "0x911654feb423363fb771e04e18d1e7325ae10a91_not_present", - keystoreDirPath: keystoreDirPath, + jsonBytes: jsonBytes, + key: &keystore.Key{ + PrivateKey: nil, + }, + keyErr: errors.New("private key error"), }, + want: privateKey, wantErr: true, }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + accountsMock := new(mocks.AccountInterface) + AccountUtilsInterface = accountsMock + + accountsMock.On("ReadFile", mock.AnythingOfType("string")).Return(tt.args.jsonBytes, tt.args.jsonBytesErr) + accountsMock.On("DecryptKey", mock.Anything, mock.AnythingOfType("string")).Return(tt.args.key, tt.args.keyErr) + + accountUtils := &AccountUtils{} + got, err := accountUtils.GetPrivateKeyFromKeystore(keystorePath, password) + if got != tt.want { + t.Errorf("Private key from GetPrivateKey, got = %v, want %v", got, tt.want) + } + if (err != nil) != tt.wantErr { + t.Errorf("GetPrivateKeyFromKeystore() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} + +func TestGetPrivateKey(t *testing.T) { + var password string + var keystorePath string + var privateKey *ecdsa.PrivateKey + + accountsList := []accounts.Account{ + {Address: common.HexToAddress("0x000000000000000000000000000000000000dea1"), + URL: accounts.URL{Scheme: "TestKeyScheme", Path: "test/key/path"}, + }, + {Address: common.HexToAddress("0x000000000000000000000000000000000000dea2"), + URL: accounts.URL{Scheme: "TestKeyScheme", Path: "test/key/path"}, + }, + } + + type args struct { + address string + accounts []accounts.Account + privateKey *ecdsa.PrivateKey + } + tests := []struct { + name string + args args + want *ecdsa.PrivateKey + wantErr bool + }{ { - name: "Test 4: When input address with incorrect password is present in keystore directory", + name: "Test 1: When input address is present in accountsList", args: args{ - address: "0x911654feb423363fb771e04e18d1e7325ae10a91", - password: "incorrect password", - keystoreDirPath: keystoreDirPath, + address: "0x000000000000000000000000000000000000dea1", + accounts: accountsList, + privateKey: privateKey, }, - wantErr: true, + want: privateKey, + wantErr: false, }, { - name: "Test 5: When a keystore file is renamed differently from the address to which it belonged", + name: "Test 2: When input address is not present in accountsList", args: args{ - address: "0x811654feb423363fb771e04e18d1e7325ae10a91", - password: password, - keystoreDirPath: "test_accounts/incorrect_test_accounts", + address: "0x000000000000000000000000000000000000dea3", + accounts: accountsList, + privateKey: privateKey, }, + want: nil, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + accountsMock := new(mocks.AccountInterface) + AccountUtilsInterface = accountsMock + + accountsMock.On("Accounts", mock.AnythingOfType("string")).Return(tt.args.accounts) + accountsMock.On("GetPrivateKeyFromKeystore", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(tt.args.privateKey, nil) + accountUtils := &AccountUtils{} - _, err := accountUtils.GetPrivateKey(tt.args.address, tt.args.password, tt.args.keystoreDirPath) + got, err := accountUtils.GetPrivateKey(tt.args.address, password, keystorePath) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetPrivateKey() got = %v, want %v", got, tt.want) + } if (err != nil) != tt.wantErr { t.Errorf("GetPrivateKey() error = %v, wantErr %v", err, tt.wantErr) return @@ -248,67 +319,3 @@ func TestSignData(t *testing.T) { }) } } - -func TestFindKeystoreFileForAddress(t *testing.T) { - testAccountsKeystorePath := "test_accounts" - - tests := []struct { - name string - keystoreDir string - address string - expectedFile string - expectErr bool - }{ - { - name: "Test 1: Matching file exists for an address", - keystoreDir: testAccountsKeystorePath, - address: "0x911654feb423363fb771e04e18d1e7325ae10a91", - expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91"), - expectErr: false, - }, - { - name: "Test 2: Matching file exists for upper case address", - keystoreDir: testAccountsKeystorePath, - address: "0x2F5F59615689B706B6AD13FD03343DCA28784989", - expectedFile: filepath.Join(testAccountsKeystorePath, "UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989"), - expectErr: false, - }, - { - name: "Test 3: No matching file", - keystoreDir: testAccountsKeystorePath, - address: "nonexistentaddress", - expectErr: true, - }, - { - name: "Test 4: When keystore directory doesnt exists", - keystoreDir: "test_accounts_invalid", - address: "0x2f5f59615689b706b6ad13fd03343dca28784989", - expectErr: true, - }, - { - name: "Test 5: When multiple files for same account is present in the keystore directory", - keystoreDir: "test_accounts/incorrect_test_accounts", - address: "0x811654feb423363fb771e04e18d1e7325ae10a91", - expectedFile: filepath.Join("test_accounts/incorrect_test_accounts", "UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91"), - expectErr: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - got, err := FindKeystoreFileForAddress(tc.keystoreDir, tc.address) - if tc.expectErr { - if err == nil { - t.Errorf("Expected an error but got none") - } - } else { - if err != nil { - t.Errorf("Did not expect an error but got one: %v", err) - } - if got != tc.expectedFile { - t.Errorf("Expected file %v, got %v", tc.expectedFile, got) - } - } - }) - } -} diff --git a/accounts/mocks/account_interface.go b/accounts/mocks/account_interface.go index ed1e4b9f5..e8e006441 100644 --- a/accounts/mocks/account_interface.go +++ b/accounts/mocks/account_interface.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.30.1. DO NOT EDIT. +// Code generated by mockery v2.14.0. DO NOT EDIT. package mocks @@ -7,6 +7,8 @@ import ( accounts "github.com/ethereum/go-ethereum/accounts" + keystore "github.com/ethereum/go-ethereum/accounts/keystore" + mock "github.com/stretchr/testify/mock" types "razor/core/types" @@ -17,6 +19,22 @@ type AccountInterface struct { mock.Mock } +// Accounts provides a mock function with given fields: path +func (_m *AccountInterface) Accounts(path string) []accounts.Account { + ret := _m.Called(path) + + var r0 []accounts.Account + if rf, ok := ret.Get(0).(func(string) []accounts.Account); ok { + r0 = rf(path) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]accounts.Account) + } + } + + return r0 +} + // CreateAccount provides a mock function with given fields: path, password func (_m *AccountInterface) CreateAccount(path string, password string) accounts.Account { ret := _m.Called(path, password) @@ -31,15 +49,34 @@ func (_m *AccountInterface) CreateAccount(path string, password string) accounts return r0 } +// DecryptKey provides a mock function with given fields: jsonBytes, password +func (_m *AccountInterface) DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) { + ret := _m.Called(jsonBytes, password) + + var r0 *keystore.Key + if rf, ok := ret.Get(0).(func([]byte, string) *keystore.Key); ok { + r0 = rf(jsonBytes, password) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*keystore.Key) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func([]byte, string) error); ok { + r1 = rf(jsonBytes, password) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetPrivateKey provides a mock function with given fields: address, password, keystorePath func (_m *AccountInterface) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) { ret := _m.Called(address, password, keystorePath) var r0 *ecdsa.PrivateKey - var r1 error - if rf, ok := ret.Get(0).(func(string, string, string) (*ecdsa.PrivateKey, error)); ok { - return rf(address, password, keystorePath) - } if rf, ok := ret.Get(0).(func(string, string, string) *ecdsa.PrivateKey); ok { r0 = rf(address, password, keystorePath) } else { @@ -48,6 +85,7 @@ func (_m *AccountInterface) GetPrivateKey(address string, password string, keyst } } + var r1 error if rf, ok := ret.Get(1).(func(string, string, string) error); ok { r1 = rf(address, password, keystorePath) } else { @@ -57,21 +95,41 @@ func (_m *AccountInterface) GetPrivateKey(address string, password string, keyst return r0, r1 } +// GetPrivateKeyFromKeystore provides a mock function with given fields: keystorePath, password +func (_m *AccountInterface) GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) { + ret := _m.Called(keystorePath, password) + + var r0 *ecdsa.PrivateKey + if rf, ok := ret.Get(0).(func(string, string) *ecdsa.PrivateKey); ok { + r0 = rf(keystorePath, password) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*ecdsa.PrivateKey) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(keystorePath, password) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // NewAccount provides a mock function with given fields: path, passphrase func (_m *AccountInterface) NewAccount(path string, passphrase string) (accounts.Account, error) { ret := _m.Called(path, passphrase) var r0 accounts.Account - var r1 error - if rf, ok := ret.Get(0).(func(string, string) (accounts.Account, error)); ok { - return rf(path, passphrase) - } if rf, ok := ret.Get(0).(func(string, string) accounts.Account); ok { r0 = rf(path, passphrase) } else { r0 = ret.Get(0).(accounts.Account) } + var r1 error if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(path, passphrase) } else { @@ -81,15 +139,34 @@ func (_m *AccountInterface) NewAccount(path string, passphrase string) (accounts return r0, r1 } +// ReadFile provides a mock function with given fields: filename +func (_m *AccountInterface) ReadFile(filename string) ([]byte, error) { + ret := _m.Called(filename) + + var r0 []byte + if rf, ok := ret.Get(0).(func(string) []byte); ok { + r0 = rf(filename) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(filename) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Sign provides a mock function with given fields: digestHash, prv func (_m *AccountInterface) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) { ret := _m.Called(digestHash, prv) var r0 []byte - var r1 error - if rf, ok := ret.Get(0).(func([]byte, *ecdsa.PrivateKey) ([]byte, error)); ok { - return rf(digestHash, prv) - } if rf, ok := ret.Get(0).(func([]byte, *ecdsa.PrivateKey) []byte); ok { r0 = rf(digestHash, prv) } else { @@ -98,6 +175,7 @@ func (_m *AccountInterface) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]by } } + var r1 error if rf, ok := ret.Get(1).(func([]byte, *ecdsa.PrivateKey) error); ok { r1 = rf(digestHash, prv) } else { @@ -112,10 +190,6 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default ret := _m.Called(hash, account, defaultPath) var r0 []byte - var r1 error - if rf, ok := ret.Get(0).(func([]byte, types.Account, string) ([]byte, error)); ok { - return rf(hash, account, defaultPath) - } if rf, ok := ret.Get(0).(func([]byte, types.Account, string) []byte); ok { r0 = rf(hash, account, defaultPath) } else { @@ -124,6 +198,7 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default } } + var r1 error if rf, ok := ret.Get(1).(func([]byte, types.Account, string) error); ok { r1 = rf(hash, account, defaultPath) } else { @@ -133,12 +208,13 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default return r0, r1 } -// NewAccountInterface creates a new instance of AccountInterface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewAccountInterface(t interface { +type mockConstructorTestingTNewAccountInterface interface { mock.TestingT Cleanup(func()) -}) *AccountInterface { +} + +// NewAccountInterface creates a new instance of AccountInterface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewAccountInterface(t mockConstructorTestingTNewAccountInterface) *AccountInterface { mock := &AccountInterface{} mock.Mock.Test(t) diff --git a/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 deleted file mode 100644 index bc50d72f6..000000000 --- a/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 +++ /dev/null @@ -1 +0,0 @@ -{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 b/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 deleted file mode 100644 index c773f086e..000000000 --- a/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 +++ /dev/null @@ -1 +0,0 @@ -{"address":"2f5f59615689b706b6ad13fd03343dca28784989","crypto":{"cipher":"aes-128-ctr","ciphertext":"e46770162aa3d74c00f1b7dff2a6e255743e61483d3f6a4c266c2697ac05aed3","cipherparams":{"iv":"8941588ce2540c7bef63f0df4a67e6cb"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"e106a7ef7916893083977dbe18cd5edf57491ba21a13adc2db7b79c7c8b85ded"},"mac":"d30b6bf80ea2ddbaca14f7635596e56ac150bda4416e9dc915117b8574cda2ba"},"id":"0accaaa4-2e5c-451b-b813-6551f9c01511","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 deleted file mode 100644 index bc50d72f6..000000000 --- a/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-03-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 +++ /dev/null @@ -1 +0,0 @@ -{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 deleted file mode 100644 index bc50d72f6..000000000 --- a/accounts/test_accounts/incorrect_test_accounts/UTC--2024-03-20T07-04-56.358521000Z--811654feb423363fb771e04e18d1e7325ae10a91 +++ /dev/null @@ -1 +0,0 @@ -{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file From 7db8cd267ae7d61bef6015aed14056a5e0b1998d Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 27 Mar 2024 14:27:21 +0530 Subject: [PATCH 7/9] fix: reused same keystore instance for fetching pkey every time --- accounts/accountUtils.go | 14 -------------- accounts/accounts.go | 29 ++++++++++++++++++++++------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/accounts/accountUtils.go b/accounts/accountUtils.go index 11f8a5f10..611b98b1c 100644 --- a/accounts/accountUtils.go +++ b/accounts/accountUtils.go @@ -6,7 +6,6 @@ import ( "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/crypto" - "os" "razor/core/types" ) @@ -16,14 +15,11 @@ var AccountUtilsInterface AccountInterface type AccountInterface interface { CreateAccount(path string, password string) accounts.Account - GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) SignData(hash []byte, account types.Account, defaultPath string) ([]byte, error) Accounts(path string) []accounts.Account NewAccount(path string, passphrase string) (accounts.Account, error) - DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) - ReadFile(filename string) ([]byte, error) } type AccountUtils struct{} @@ -41,17 +37,7 @@ func (accountUtils AccountUtils) NewAccount(path string, passphrase string) (acc return ks.NewAccount(passphrase) } -//This function takes json bytes array and password as input and returns the decrypted key -func (accountUtils AccountUtils) DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) { - return keystore.DecryptKey(jsonBytes, password) -} - //This function takes hash in form of byte array and private key as input and returns signature as byte array func (accountUtils AccountUtils) Sign(digestHash []byte, prv *ecdsa.PrivateKey) (sig []byte, err error) { return crypto.Sign(digestHash, prv) } - -//This function takes name of the file as input and returns the file data as byte array -func (accountUtils AccountUtils) ReadFile(filename string) ([]byte, error) { - return os.ReadFile(filename) -} diff --git a/accounts/accounts.go b/accounts/accounts.go index b907dfef9..36f9d0fe0 100644 --- a/accounts/accounts.go +++ b/accounts/accounts.go @@ -5,13 +5,24 @@ import ( "crypto/ecdsa" "errors" "github.com/ethereum/go-ethereum/accounts" + "github.com/ethereum/go-ethereum/accounts/keystore" + "os" "razor/core/types" "razor/logger" "razor/path" "strings" ) -var log = logger.NewLogger() +var ( + log = logger.NewLogger() + ksInstance *keystore.KeyStore +) + +// InitializeKeystore directly initializes the global keystore instance. +func initializeKeystore(keystorePath string) { + log.Info("Initialising keystoreInstance...") + ksInstance = keystore.NewKeyStore(keystorePath, keystore.StandardScryptN, keystore.StandardScryptP) +} //This function takes path and password as input and returns new account func (AccountUtils) CreateAccount(keystorePath string, password string) accounts.Account { @@ -29,13 +40,13 @@ func (AccountUtils) CreateAccount(keystorePath string, password string) accounts } //This function takes and path of keystore and password as input and returns private key of account -func (AccountUtils) GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) { - jsonBytes, err := AccountUtilsInterface.ReadFile(keystorePath) +func getPrivateKeyFromKeystore(keystoreFilePath string, password string) (*ecdsa.PrivateKey, error) { + jsonBytes, err := os.ReadFile(keystoreFilePath) if err != nil { log.Error("Error in reading keystore: ", err) return nil, err } - key, err := AccountUtilsInterface.DecryptKey(jsonBytes, password) + key, err := keystore.DecryptKey(jsonBytes, password) if err != nil { log.Error("Error in fetching private key: ", err) return nil, err @@ -44,11 +55,15 @@ func (AccountUtils) GetPrivateKeyFromKeystore(keystorePath string, password stri } //This function takes address of account, password and keystore path as input and returns private key of account -func (AccountUtils) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) { - allAccounts := AccountUtilsInterface.Accounts(keystorePath) +func (AccountUtils) GetPrivateKey(address string, password string, keystoreDirPath string) (*ecdsa.PrivateKey, error) { + if ksInstance == nil { + initializeKeystore(keystoreDirPath) + } + + allAccounts := ksInstance.Accounts() for _, account := range allAccounts { if strings.EqualFold(account.Address.Hex(), address) { - return AccountUtilsInterface.GetPrivateKeyFromKeystore(account.URL.Path, password) + return getPrivateKeyFromKeystore(account.URL.Path, password) } } return nil, errors.New("no keystore file found") From 2a20886029e6a438686a8c420f23af41893645db Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 27 Mar 2024 14:28:22 +0530 Subject: [PATCH 8/9] refactor: added realistic tests for fetching of pKey --- accounts/accounts_test.go | 120 +++++++----------- accounts/mocks/account_interface.go | 102 +++------------ ...--911654feb423363fb771e04e18d1e7325ae10a91 | 1 + ...--2f5f59615689b706b6ad13fd03343dca28784989 | 1 + 4 files changed, 68 insertions(+), 156 deletions(-) create mode 100644 accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 create mode 100644 accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 diff --git a/accounts/accounts_test.go b/accounts/accounts_test.go index aa964bf47..3e288ad6f 100644 --- a/accounts/accounts_test.go +++ b/accounts/accounts_test.go @@ -4,7 +4,6 @@ import ( "crypto/ecdsa" "errors" "github.com/ethereum/go-ethereum/accounts" - "github.com/ethereum/go-ethereum/accounts/keystore" "github.com/ethereum/go-ethereum/common" "github.com/magiconair/properties/assert" "github.com/stretchr/testify/mock" @@ -116,17 +115,12 @@ func TestCreateAccount(t *testing.T) { } } -func TestGetPrivateKeyFromKeystore(t *testing.T) { - var password string - var keystorePath string - var privateKey *ecdsa.PrivateKey - var jsonBytes []byte +func Test_getPrivateKeyFromKeystore(t *testing.T) { + password := "Razor@123" type args struct { - jsonBytes []byte - jsonBytesErr error - key *keystore.Key - keyErr error + keystoreFilePath string + password string } tests := []struct { name string @@ -135,54 +129,35 @@ func TestGetPrivateKeyFromKeystore(t *testing.T) { wantErr bool }{ { - name: "Test 1: When GetPrivateKey function executes successfully", + name: "Test 1: When keystore file is present and getPrivateKeyFromKeystore function executes successfully", args: args{ - jsonBytes: jsonBytes, - key: &keystore.Key{ - PrivateKey: privateKey, - }, + keystoreFilePath: "test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91", + password: password, }, - want: privateKey, wantErr: false, }, { - name: "Test 2: When there is an error in reading data from file", + name: "Test 2: When there is no keystore file present at the desired path", args: args{ - jsonBytesErr: errors.New("error in reading data"), - key: &keystore.Key{ - PrivateKey: nil, - }, + keystoreFilePath: "test_accounts/UTC--2024-03-20T07-03-56.358521000Z--211654feb423363fb771e04e18d1e7325ae10a91", + password: password, }, want: nil, wantErr: true, }, { - name: "Test 3: When there is an error in fetching private key", + name: "Test 3: When password is incorrect for the desired keystore file", args: args{ - jsonBytes: jsonBytes, - key: &keystore.Key{ - PrivateKey: nil, - }, - keyErr: errors.New("private key error"), + keystoreFilePath: "test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91", + password: "Razor@456", }, - want: privateKey, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - accountsMock := new(mocks.AccountInterface) - AccountUtilsInterface = accountsMock - - accountsMock.On("ReadFile", mock.AnythingOfType("string")).Return(tt.args.jsonBytes, tt.args.jsonBytesErr) - accountsMock.On("DecryptKey", mock.Anything, mock.AnythingOfType("string")).Return(tt.args.key, tt.args.keyErr) - - accountUtils := &AccountUtils{} - got, err := accountUtils.GetPrivateKeyFromKeystore(keystorePath, password) - if got != tt.want { - t.Errorf("Private key from GetPrivateKey, got = %v, want %v", got, tt.want) - } + _, err := getPrivateKeyFromKeystore(tt.args.keystoreFilePath, tt.args.password) if (err != nil) != tt.wantErr { t.Errorf("GetPrivateKeyFromKeystore() error = %v, wantErr %v", err, tt.wantErr) return @@ -192,64 +167,59 @@ func TestGetPrivateKeyFromKeystore(t *testing.T) { } func TestGetPrivateKey(t *testing.T) { - var password string - var keystorePath string - var privateKey *ecdsa.PrivateKey - - accountsList := []accounts.Account{ - {Address: common.HexToAddress("0x000000000000000000000000000000000000dea1"), - URL: accounts.URL{Scheme: "TestKeyScheme", Path: "test/key/path"}, - }, - {Address: common.HexToAddress("0x000000000000000000000000000000000000dea2"), - URL: accounts.URL{Scheme: "TestKeyScheme", Path: "test/key/path"}, - }, - } + password := "Razor@123" + keystoreDirPath := "test_accounts" type args struct { - address string - accounts []accounts.Account - privateKey *ecdsa.PrivateKey + address string + password string + keystoreDirPath string } tests := []struct { name string args args - want *ecdsa.PrivateKey wantErr bool }{ { - name: "Test 1: When input address is present in accountsList", + name: "Test 1: When input address with correct password is present in keystore directory", args: args{ - address: "0x000000000000000000000000000000000000dea1", - accounts: accountsList, - privateKey: privateKey, + address: "0x911654feb423363fb771e04e18d1e7325ae10a91", + password: password, + keystoreDirPath: keystoreDirPath, }, - want: privateKey, wantErr: false, }, { - name: "Test 2: When input address is not present in accountsList", + name: "Test 2: When input upper case address with correct password is present in keystore directory", args: args{ - address: "0x000000000000000000000000000000000000dea3", - accounts: accountsList, - privateKey: privateKey, + address: "0x2F5F59615689B706B6AD13FD03343DCA28784989", + password: password, + keystoreDirPath: keystoreDirPath, + }, + wantErr: false, + }, + { + name: "Test 3: When provided address is not present in keystore directory", + args: args{ + address: "0x911654feb423363fb771e04e18d1e7325ae10a91_not_present", + keystoreDirPath: keystoreDirPath, + }, + wantErr: true, + }, + { + name: "Test 4: When input address with incorrect password is present in keystore directory", + args: args{ + address: "0x911654feb423363fb771e04e18d1e7325ae10a91", + password: "incorrect password", + keystoreDirPath: keystoreDirPath, }, - want: nil, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - accountsMock := new(mocks.AccountInterface) - AccountUtilsInterface = accountsMock - - accountsMock.On("Accounts", mock.AnythingOfType("string")).Return(tt.args.accounts) - accountsMock.On("GetPrivateKeyFromKeystore", mock.AnythingOfType("string"), mock.AnythingOfType("string")).Return(tt.args.privateKey, nil) - accountUtils := &AccountUtils{} - got, err := accountUtils.GetPrivateKey(tt.args.address, password, keystorePath) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("GetPrivateKey() got = %v, want %v", got, tt.want) - } + _, err := accountUtils.GetPrivateKey(tt.args.address, tt.args.password, tt.args.keystoreDirPath) if (err != nil) != tt.wantErr { t.Errorf("GetPrivateKey() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/accounts/mocks/account_interface.go b/accounts/mocks/account_interface.go index e8e006441..53570c318 100644 --- a/accounts/mocks/account_interface.go +++ b/accounts/mocks/account_interface.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.14.0. DO NOT EDIT. +// Code generated by mockery v2.30.1. DO NOT EDIT. package mocks @@ -7,8 +7,6 @@ import ( accounts "github.com/ethereum/go-ethereum/accounts" - keystore "github.com/ethereum/go-ethereum/accounts/keystore" - mock "github.com/stretchr/testify/mock" types "razor/core/types" @@ -49,34 +47,15 @@ func (_m *AccountInterface) CreateAccount(path string, password string) accounts return r0 } -// DecryptKey provides a mock function with given fields: jsonBytes, password -func (_m *AccountInterface) DecryptKey(jsonBytes []byte, password string) (*keystore.Key, error) { - ret := _m.Called(jsonBytes, password) - - var r0 *keystore.Key - if rf, ok := ret.Get(0).(func([]byte, string) *keystore.Key); ok { - r0 = rf(jsonBytes, password) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*keystore.Key) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func([]byte, string) error); ok { - r1 = rf(jsonBytes, password) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // GetPrivateKey provides a mock function with given fields: address, password, keystorePath func (_m *AccountInterface) GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) { ret := _m.Called(address, password, keystorePath) var r0 *ecdsa.PrivateKey + var r1 error + if rf, ok := ret.Get(0).(func(string, string, string) (*ecdsa.PrivateKey, error)); ok { + return rf(address, password, keystorePath) + } if rf, ok := ret.Get(0).(func(string, string, string) *ecdsa.PrivateKey); ok { r0 = rf(address, password, keystorePath) } else { @@ -85,7 +64,6 @@ func (_m *AccountInterface) GetPrivateKey(address string, password string, keyst } } - var r1 error if rf, ok := ret.Get(1).(func(string, string, string) error); ok { r1 = rf(address, password, keystorePath) } else { @@ -95,41 +73,21 @@ func (_m *AccountInterface) GetPrivateKey(address string, password string, keyst return r0, r1 } -// GetPrivateKeyFromKeystore provides a mock function with given fields: keystorePath, password -func (_m *AccountInterface) GetPrivateKeyFromKeystore(keystorePath string, password string) (*ecdsa.PrivateKey, error) { - ret := _m.Called(keystorePath, password) - - var r0 *ecdsa.PrivateKey - if rf, ok := ret.Get(0).(func(string, string) *ecdsa.PrivateKey); ok { - r0 = rf(keystorePath, password) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*ecdsa.PrivateKey) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(string, string) error); ok { - r1 = rf(keystorePath, password) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // NewAccount provides a mock function with given fields: path, passphrase func (_m *AccountInterface) NewAccount(path string, passphrase string) (accounts.Account, error) { ret := _m.Called(path, passphrase) var r0 accounts.Account + var r1 error + if rf, ok := ret.Get(0).(func(string, string) (accounts.Account, error)); ok { + return rf(path, passphrase) + } if rf, ok := ret.Get(0).(func(string, string) accounts.Account); ok { r0 = rf(path, passphrase) } else { r0 = ret.Get(0).(accounts.Account) } - var r1 error if rf, ok := ret.Get(1).(func(string, string) error); ok { r1 = rf(path, passphrase) } else { @@ -139,34 +97,15 @@ func (_m *AccountInterface) NewAccount(path string, passphrase string) (accounts return r0, r1 } -// ReadFile provides a mock function with given fields: filename -func (_m *AccountInterface) ReadFile(filename string) ([]byte, error) { - ret := _m.Called(filename) - - var r0 []byte - if rf, ok := ret.Get(0).(func(string) []byte); ok { - r0 = rf(filename) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(filename) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - // Sign provides a mock function with given fields: digestHash, prv func (_m *AccountInterface) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) { ret := _m.Called(digestHash, prv) var r0 []byte + var r1 error + if rf, ok := ret.Get(0).(func([]byte, *ecdsa.PrivateKey) ([]byte, error)); ok { + return rf(digestHash, prv) + } if rf, ok := ret.Get(0).(func([]byte, *ecdsa.PrivateKey) []byte); ok { r0 = rf(digestHash, prv) } else { @@ -175,7 +114,6 @@ func (_m *AccountInterface) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]by } } - var r1 error if rf, ok := ret.Get(1).(func([]byte, *ecdsa.PrivateKey) error); ok { r1 = rf(digestHash, prv) } else { @@ -190,6 +128,10 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default ret := _m.Called(hash, account, defaultPath) var r0 []byte + var r1 error + if rf, ok := ret.Get(0).(func([]byte, types.Account, string) ([]byte, error)); ok { + return rf(hash, account, defaultPath) + } if rf, ok := ret.Get(0).(func([]byte, types.Account, string) []byte); ok { r0 = rf(hash, account, defaultPath) } else { @@ -198,7 +140,6 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default } } - var r1 error if rf, ok := ret.Get(1).(func([]byte, types.Account, string) error); ok { r1 = rf(hash, account, defaultPath) } else { @@ -208,13 +149,12 @@ func (_m *AccountInterface) SignData(hash []byte, account types.Account, default return r0, r1 } -type mockConstructorTestingTNewAccountInterface interface { +// NewAccountInterface creates a new instance of AccountInterface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewAccountInterface(t interface { mock.TestingT Cleanup(func()) -} - -// NewAccountInterface creates a new instance of AccountInterface. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -func NewAccountInterface(t mockConstructorTestingTNewAccountInterface) *AccountInterface { +}) *AccountInterface { mock := &AccountInterface{} mock.Mock.Test(t) diff --git a/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 b/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 new file mode 100644 index 000000000..bc50d72f6 --- /dev/null +++ b/accounts/test_accounts/UTC--2024-03-20T07-03-56.358521000Z--911654feb423363fb771e04e18d1e7325ae10a91 @@ -0,0 +1 @@ +{"address":"911654feb423363fb771e04e18d1e7325ae10a91","crypto":{"cipher":"aes-128-ctr","ciphertext":"032e882238c605aa6ede0c54658b1f26a8800e4b41c67349159236e7ffa76955","cipherparams":{"iv":"12cae1b8475b00f92e2eac08e08a6a39"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"bacdad2b595bda0ed52706842b9a1d20ce64210eeadcf7302e6ba9e2ddc22706"},"mac":"722ea7fc4cee367e92bc403a5371aab1e78e66e3876d00d3a40830130c443101"},"id":"e0b6a612-8877-4400-813a-ef8a627d33d8","version":3} \ No newline at end of file diff --git a/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 b/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 new file mode 100644 index 000000000..c773f086e --- /dev/null +++ b/accounts/test_accounts/UTC--2024-03-20T07-04-11.601622000Z--2f5f59615689b706b6ad13fd03343dca28784989 @@ -0,0 +1 @@ +{"address":"2f5f59615689b706b6ad13fd03343dca28784989","crypto":{"cipher":"aes-128-ctr","ciphertext":"e46770162aa3d74c00f1b7dff2a6e255743e61483d3f6a4c266c2697ac05aed3","cipherparams":{"iv":"8941588ce2540c7bef63f0df4a67e6cb"},"kdf":"scrypt","kdfparams":{"dklen":32,"n":262144,"p":1,"r":8,"salt":"e106a7ef7916893083977dbe18cd5edf57491ba21a13adc2db7b79c7c8b85ded"},"mac":"d30b6bf80ea2ddbaca14f7635596e56ac150bda4416e9dc915117b8574cda2ba"},"id":"0accaaa4-2e5c-451b-b813-6551f9c01511","version":3} \ No newline at end of file From 95d4a584d89282cb14d1b1e9b7cf0157659fff2a Mon Sep 17 00:00:00 2001 From: YashK Date: Wed, 27 Mar 2024 18:08:59 +0530 Subject: [PATCH 9/9] refactor: removed unused accounts() from interface --- accounts/accountUtils.go | 7 ------- accounts/mocks/account_interface.go | 16 ---------------- 2 files changed, 23 deletions(-) diff --git a/accounts/accountUtils.go b/accounts/accountUtils.go index 611b98b1c..bfa234ff6 100644 --- a/accounts/accountUtils.go +++ b/accounts/accountUtils.go @@ -17,19 +17,12 @@ type AccountInterface interface { CreateAccount(path string, password string) accounts.Account GetPrivateKey(address string, password string, keystorePath string) (*ecdsa.PrivateKey, error) SignData(hash []byte, account types.Account, defaultPath string) ([]byte, error) - Accounts(path string) []accounts.Account NewAccount(path string, passphrase string) (accounts.Account, error) Sign(digestHash []byte, prv *ecdsa.PrivateKey) ([]byte, error) } type AccountUtils struct{} -//This function returns all the accounts in form of array -func (accountUtils AccountUtils) Accounts(path string) []accounts.Account { - ks := keystore.NewKeyStore(path, keystore.StandardScryptN, keystore.StandardScryptP) - return ks.Accounts() -} - //This function takes path and pass phrase as input and returns the new account func (accountUtils AccountUtils) NewAccount(path string, passphrase string) (accounts.Account, error) { ks := keystore.NewKeyStore(path, keystore.StandardScryptN, keystore.StandardScryptP) diff --git a/accounts/mocks/account_interface.go b/accounts/mocks/account_interface.go index 53570c318..ed1e4b9f5 100644 --- a/accounts/mocks/account_interface.go +++ b/accounts/mocks/account_interface.go @@ -17,22 +17,6 @@ type AccountInterface struct { mock.Mock } -// Accounts provides a mock function with given fields: path -func (_m *AccountInterface) Accounts(path string) []accounts.Account { - ret := _m.Called(path) - - var r0 []accounts.Account - if rf, ok := ret.Get(0).(func(string) []accounts.Account); ok { - r0 = rf(path) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).([]accounts.Account) - } - } - - return r0 -} - // CreateAccount provides a mock function with given fields: path, password func (_m *AccountInterface) CreateAccount(path string, password string) accounts.Account { ret := _m.Called(path, password)