rpc: implement personal_importRawKey (#552)

Author: fedekunze

CHANGELOG.md

@@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## Unreleased
 
+### Features
+
+* (rpc) [\#552](https://github.com/ChainSafe/ethermint/pull/552) Implement Eth Personal namespace `personal_importRawKey`.
+
 ### Bug fixes
 
 * (keys) [\#554](https://github.com/ChainSafe/ethermint/pull/554) Fix private key derivation.

crypto/algorithm.go

@@ -18,8 +18,10 @@ import (
 )
 
 const (
+	// EthSecp256k1Type string constant for the EthSecp256k1 algorithm
+	EthSecp256k1Type = "eth_secp256k1"
 	// EthSecp256k1 defines the ECDSA secp256k1 used on Ethereum
-	EthSecp256k1 = keys.SigningAlgo("eth_secp256k1")
+	EthSecp256k1 = keys.SigningAlgo(EthSecp256k1Type)
 )
 
 // SupportedAlgorithms defines the list of signing algorithms used on Ethermint:

rpc/apis.go

@@ -40,7 +40,7 @@ func GetRPCAPIs(cliCtx context.CLIContext, keys []emintcrypto.PrivKeySecp256k1)
 		{
 			Namespace: PersonalNamespace,
 			Version:   apiVersion,
-			Service:   NewPersonalEthAPI(cliCtx, ethAPI, nonceLock, keys),
+			Service:   NewPersonalEthAPI(ethAPI),
 			Public:    false,
 		},
 		{

rpc/eth_api.go

@@ -46,7 +46,7 @@ type PublicEthAPI struct {
 	chainIDEpoch *big.Int
 	logger       log.Logger
 	backend      Backend
-	keys         []crypto.PrivKeySecp256k1
+	keys         []crypto.PrivKeySecp256k1 // unlocked keys
 	nonceLock    *AddrLocker
 	keybaseLock  sync.Mutex
 }

rpc/personal_api.go

@@ -5,43 +5,34 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"sync"
 	"time"
 
-	sdkcontext "github.com/cosmos/cosmos-sdk/client/context"
+	"github.com/spf13/viper"
+
 	"github.com/cosmos/cosmos-sdk/client/flags"
 	"github.com/cosmos/cosmos-sdk/crypto/keys"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/mintkey"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	emintcrypto "github.com/cosmos/ethermint/crypto"
-	params "github.com/cosmos/ethermint/rpc/args"
-	"github.com/spf13/viper"
-	"github.com/tendermint/tendermint/libs/log"
 
 	"github.com/ethereum/go-ethereum/accounts"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	"github.com/ethereum/go-ethereum/crypto"
+
+	emintcrypto "github.com/cosmos/ethermint/crypto"
+	params "github.com/cosmos/ethermint/rpc/args"
 )
 
-// PersonalEthAPI is the eth_ prefixed set of APIs in the Web3 JSON-RPC spec.
+// PersonalEthAPI is the personal_ prefixed set of APIs in the Web3 JSON-RPC spec.
 type PersonalEthAPI struct {
-	logger      log.Logger
-	cliCtx      sdkcontext.CLIContext
-	ethAPI      *PublicEthAPI
-	nonceLock   *AddrLocker
-	keys        []emintcrypto.PrivKeySecp256k1
-	keyInfos    []keys.Info
-	keybaseLock sync.Mutex
+	ethAPI   *PublicEthAPI
+	keyInfos []keys.Info // all keys, both locked and unlocked. unlocked keys are stored in ethAPI.keys
 }
 
-// NewPersonalEthAPI creates an instance of the public ETH Web3 API.
-func NewPersonalEthAPI(cliCtx sdkcontext.CLIContext, ethAPI *PublicEthAPI, nonceLock *AddrLocker, keys []emintcrypto.PrivKeySecp256k1) *PersonalEthAPI {
+// NewPersonalEthAPI creates an instance of the public Personal Eth API.
+func NewPersonalEthAPI(ethAPI *PublicEthAPI) *PersonalEthAPI {
 	api := &PersonalEthAPI{
-		logger:    log.NewTMLogger(log.NewSyncWriter(os.Stdout)).With("module", "json-rpc"),
-		cliCtx:    cliCtx,
-		ethAPI:    ethAPI,
-		nonceLock: nonceLock,
-		keys:      keys,
+		ethAPI: ethAPI,
 	}
 
 	infos, err := api.getKeybaseInfo()
@@ -54,43 +45,68 @@ func NewPersonalEthAPI(cliCtx sdkcontext.CLIContext, ethAPI *PublicEthAPI, nonce
 }
 
 func (e *PersonalEthAPI) getKeybaseInfo() ([]keys.Info, error) {
-	e.keybaseLock.Lock()
-	defer e.keybaseLock.Unlock()
+	e.ethAPI.keybaseLock.Lock()
+	defer e.ethAPI.keybaseLock.Unlock()
 
-	if e.cliCtx.Keybase == nil {
+	if e.ethAPI.cliCtx.Keybase == nil {
 		keybase, err := keys.NewKeyring(
 			sdk.KeyringServiceName(),
 			viper.GetString(flags.FlagKeyringBackend),
 			viper.GetString(flags.FlagHome),
-			e.cliCtx.Input,
+			e.ethAPI.cliCtx.Input,
 			emintcrypto.EthSecp256k1Options()...,
 		)
 		if err != nil {
 			return nil, err
 		}
 
-		e.cliCtx.Keybase = keybase
+		e.ethAPI.cliCtx.Keybase = keybase
 	}
 
-	return e.cliCtx.Keybase.List()
+	return e.ethAPI.cliCtx.Keybase.List()
 }
 
-// ImportRawKey stores the given hex encoded ECDSA key into the key directory,
-// encrypting it with the passphrase.
-// Currently, this is not implemented since the feature is not supported by the keys.
+// ImportRawKey armors and encrypts a given raw hex encoded ECDSA key and stores it into the key directory.
+// The name of the key will have the format "personal_<length-keys>", where <length-keys> is the total number of
+// keys stored on the keyring.
+// NOTE: The key will be both armored and encrypted using the same passphrase.
 func (e *PersonalEthAPI) ImportRawKey(privkey, password string) (common.Address, error) {
-	e.logger.Debug("personal_importRawKey", "error", "not implemented")
-	_, err := crypto.HexToECDSA(privkey)
+	e.ethAPI.logger.Debug("personal_importRawKey")
+	priv, err := crypto.HexToECDSA(privkey)
 	if err != nil {
 		return common.Address{}, err
 	}
 
-	return common.Address{}, nil
+	privKey := emintcrypto.PrivKeySecp256k1(crypto.FromECDSA(priv))
+
+	armor := mintkey.EncryptArmorPrivKey(privKey, password, emintcrypto.EthSecp256k1Type)
+
+	// ignore error as we only care about the length of the list
+	list, _ := e.ethAPI.cliCtx.Keybase.List()
+	privKeyName := fmt.Sprintf("personal_%d", len(list))
+
+	if err := e.ethAPI.cliCtx.Keybase.ImportPrivKey(privKeyName, armor, password); err != nil {
+		return common.Address{}, err
+	}
+
+	addr := common.BytesToAddress(privKey.PubKey().Address().Bytes())
+
+	info, err := e.ethAPI.cliCtx.Keybase.Get(privKeyName)
+	if err != nil {
+		return common.Address{}, err
+	}
+
+	// append key and info to be able to lock and list the account
+	//e.ethAPI.keys = append(e.ethAPI.keys, privKey)
+	e.keyInfos = append(e.keyInfos, info)
+	e.ethAPI.logger.Info("key successfully imported", "name", privKeyName, "address", addr.String())
+
+	return addr, nil
 }
 
 // ListAccounts will return a list of addresses for accounts this node manages.
 func (e *PersonalEthAPI) ListAccounts() ([]common.Address, error) {
-	e.logger.Debug("personal_listAccounts")
+	e.ethAPI.logger.Debug("personal_listAccounts")
 	addrs := []common.Address{}
 	for _, info := range e.keyInfos {
 		addressBytes := info.GetPubKey().Address().Bytes()
@@ -103,18 +119,7 @@ func (e *PersonalEthAPI) ListAccounts() ([]common.Address, error) {
 // LockAccount will lock the account associated with the given address when it's unlocked.
 // It removes the key corresponding to the given address from the API's local keys.
 func (e *PersonalEthAPI) LockAccount(address common.Address) bool {
-	e.logger.Debug("personal_lockAccount", "address", address)
-	for i, key := range e.keys {
-		if !bytes.Equal(key.PubKey().Address().Bytes(), address.Bytes()) {
-			continue
-		}
-
-		tmp := make([]emintcrypto.PrivKeySecp256k1, len(e.keys)-1)
-		copy(tmp[:i], e.keys[:i])
-		copy(tmp[i:], e.keys[i+1:])
-		e.keys = tmp
-		return true
-	}
+	e.ethAPI.logger.Debug("personal_lockAccount", "address", address.String())
 
 	for i, key := range e.ethAPI.keys {
 		if !bytes.Equal(key.PubKey().Address().Bytes(), address.Bytes()) {
@@ -125,6 +130,8 @@ func (e *PersonalEthAPI) LockAccount(address common.Address) bool {
 		copy(tmp[:i], e.ethAPI.keys[:i])
 		copy(tmp[i:], e.ethAPI.keys[i+1:])
 		e.ethAPI.keys = tmp
+
+		e.ethAPI.logger.Debug("account unlocked", "address", address.String())
 		return true
 	}
 
@@ -133,62 +140,55 @@ func (e *PersonalEthAPI) LockAccount(address common.Address) bool {
 
 // NewAccount will create a new account and returns the address for the new account.
 func (e *PersonalEthAPI) NewAccount(password string) (common.Address, error) {
-	e.logger.Debug("personal_newAccount")
+	e.ethAPI.logger.Debug("personal_newAccount")
 	_, err := e.getKeybaseInfo()
 	if err != nil {
 		return common.Address{}, err
 	}
 
 	name := "key_" + time.Now().UTC().Format(time.RFC3339)
-	info, _, err := e.cliCtx.Keybase.CreateMnemonic(name, keys.English, password, emintcrypto.EthSecp256k1)
+	info, _, err := e.ethAPI.cliCtx.Keybase.CreateMnemonic(name, keys.English, password, emintcrypto.EthSecp256k1)
 	if err != nil {
 		return common.Address{}, err
 	}
 
 	e.keyInfos = append(e.keyInfos, info)
 
-	// update ethAPI
-	privKey, err := e.cliCtx.Keybase.ExportPrivateKeyObject(name, password)
-	if err != nil {
-		return common.Address{}, err
-	}
-
-	emintKey, ok := privKey.(emintcrypto.PrivKeySecp256k1)
-	if !ok {
-		return common.Address{}, fmt.Errorf("invalid private key type: %T", privKey)
-	}
-	e.ethAPI.keys = append(e.ethAPI.keys, emintKey)
-	e.logger.Debug("personal_newAccount", "address", fmt.Sprintf("0x%x", emintKey.PubKey().Address().Bytes()))
-
 	addr := common.BytesToAddress(info.GetPubKey().Address().Bytes())
-	e.logger.Info("Your new key was generated", "address", addr)
-	e.logger.Info("Please backup your key file!", "path", os.Getenv("HOME")+"/.ethermintcli/"+name)
-	e.logger.Info("Please remember your password!")
+	e.ethAPI.logger.Info("Your new key was generated", "address", addr.String())
+	e.ethAPI.logger.Info("Please backup your key file!", "path", os.Getenv("HOME")+"/.ethermintcli/"+name)
+	e.ethAPI.logger.Info("Please remember your password!")
 	return addr, nil
 }
 
 // UnlockAccount will unlock the account associated with the given address with
 // the given password for duration seconds. If duration is nil it will use a
 // default of 300 seconds. It returns an indication if the account was unlocked.
 // It exports the private key corresponding to the given address from the keyring and stores it in the API's local keys.
-func (e *PersonalEthAPI) UnlockAccount(ctx context.Context, addr common.Address, password string, _ *uint64) (bool, error) {
-	e.logger.Debug("personal_unlockAccount", "address", addr)
+func (e *PersonalEthAPI) UnlockAccount(_ context.Context, addr common.Address, password string, _ *uint64) (bool, error) { // nolint: interfacer
+	e.ethAPI.logger.Debug("personal_unlockAccount", "address", addr.String())
 	// TODO: use duration
 
-	name := ""
+	var keyInfo keys.Info
+
 	for _, info := range e.keyInfos {
 		addressBytes := info.GetPubKey().Address().Bytes()
 		if bytes.Equal(addressBytes, addr[:]) {
-			name = info.GetName()
+			keyInfo = info
+			break
 		}
 	}
 
-	if name == "" {
-		return false, fmt.Errorf("cannot find key with given address")
+	if keyInfo == nil {
+		return false, fmt.Errorf("cannot find key with given address %s", addr.String())
 	}
 
-	// TODO: this only works on local keys
-	privKey, err := e.cliCtx.Keybase.ExportPrivateKeyObject(name, password)
+	// exporting private key only works on local keys
+	if keyInfo.GetType() != keys.TypeLocal {
+		return false, fmt.Errorf("key type must be %s, got %s", keys.TypeLedger.String(), keyInfo.GetType().String())
+	}
+
+	privKey, err := e.ethAPI.cliCtx.Keybase.ExportPrivateKeyObject(keyInfo.GetName(), password)
 	if err != nil {
 		return false, err
 	}
@@ -198,17 +198,15 @@ func (e *PersonalEthAPI) UnlockAccount(ctx context.Context, addr common.Address,
 		return false, fmt.Errorf("invalid private key type: %T", privKey)
 	}
 
-	e.keys = append(e.keys, emintKey)
 	e.ethAPI.keys = append(e.ethAPI.keys, emintKey)
-	e.logger.Debug("personal_unlockAccount", "address", fmt.Sprintf("0x%x", emintKey.PubKey().Address().Bytes()))
-
+	e.ethAPI.logger.Debug("account unlocked", "address", addr.String())
 	return true, nil
 }
 
 // SendTransaction will create a transaction from the given arguments and
 // tries to sign it with the key associated with args.To. If the given password isn't
 // able to decrypt the key it fails.
-func (e *PersonalEthAPI) SendTransaction(ctx context.Context, args params.SendTxArgs, passwd string) (common.Hash, error) {
+func (e *PersonalEthAPI) SendTransaction(_ context.Context, args params.SendTxArgs, _ string) (common.Hash, error) {
 	return e.ethAPI.SendTransaction(args)
 }
 
@@ -221,12 +219,12 @@ func (e *PersonalEthAPI) SendTransaction(ctx context.Context, args params.SendTx
 // The key used to calculate the signature is decrypted with the given password.
 //
 // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_sign
-func (e *PersonalEthAPI) Sign(ctx context.Context, data hexutil.Bytes, addr common.Address, passwd string) (hexutil.Bytes, error) {
-	e.logger.Debug("personal_sign", "data", data, "address", addr)
+func (e *PersonalEthAPI) Sign(_ context.Context, data hexutil.Bytes, addr common.Address, _ string) (hexutil.Bytes, error) {
+	e.ethAPI.logger.Debug("personal_sign", "data", data, "address", addr.String())
 
-	key, ok := checkKeyInKeyring(e.keys, addr)
+	key, ok := checkKeyInKeyring(e.ethAPI.keys, addr)
 	if !ok {
-		return nil, fmt.Errorf("cannot find key with given address")
+		return nil, fmt.Errorf("cannot find key with address %s", addr.String())
 	}
 
 	sig, err := crypto.Sign(accounts.TextHash(data), key.ToECDSA())
@@ -248,8 +246,8 @@ func (e *PersonalEthAPI) Sign(ctx context.Context, data hexutil.Bytes, addr comm
 // the V value must be 27 or 28 for legacy reasons.
 //
 // https://github.com/ethereum/go-ethereum/wiki/Management-APIs#personal_ecRecove
-func (e *PersonalEthAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes) (common.Address, error) {
-	e.logger.Debug("personal_ecRecover", "data", data, "sig", sig)
+func (e *PersonalEthAPI) EcRecover(_ context.Context, data, sig hexutil.Bytes) (common.Address, error) {
+	e.ethAPI.logger.Debug("personal_ecRecover", "data", data, "sig", sig)
 
 	if len(sig) != crypto.SignatureLength {
 		return common.Address{}, fmt.Errorf("signature must be %d bytes long", crypto.SignatureLength)
@@ -259,9 +257,9 @@ func (e *PersonalEthAPI) EcRecover(ctx context.Context, data, sig hexutil.Bytes)
 	}
 	sig[crypto.RecoveryIDOffset] -= 27 // Transform yellow paper V from 27/28 to 0/1
 
-	rpk, err := crypto.SigToPub(accounts.TextHash(data), sig)
+	pubkey, err := crypto.SigToPub(accounts.TextHash(data), sig)
 	if err != nil {
 		return common.Address{}, err
 	}
-	return crypto.PubkeyToAddress(*rpk), nil
+	return crypto.PubkeyToAddress(*pubkey), nil
 }

tests/personal_test.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
+	ethcrypto "github.com/ethereum/go-ethereum/crypto"
 
 	"github.com/stretchr/testify/require"
 )
@@ -42,6 +43,24 @@ func TestPersonal_Sign(t *testing.T) {
 	// TODO: check that signature is same as with geth, requires importing a key
 }
 
+func TestPersonal_ImportRawKey(t *testing.T) {
+	privkey, err := ethcrypto.GenerateKey()
+	require.NoError(t, err)
+
+	// parse priv key to hex
+	hexPriv := common.Bytes2Hex(ethcrypto.FromECDSA(privkey))
+	rpcRes := call(t, "personal_importRawKey", []string{hexPriv, "password"})
+
+	var res hexutil.Bytes
+	err = json.Unmarshal(rpcRes.Result, &res)
+	require.NoError(t, err)
+
+	addr := ethcrypto.PubkeyToAddress(privkey.PublicKey)
+	resAddr := common.BytesToAddress(res)
+
+	require.Equal(t, addr.String(), resAddr.String())
+}
+
 func TestPersonal_EcRecover(t *testing.T) {
 	data := hexutil.Bytes{0x88}
 	rpcRes := call(t, "personal_sign", []interface{}{data, hexutil.Bytes(from), ""})
@@ -67,7 +86,7 @@ func TestPersonal_UnlockAccount(t *testing.T) {
 
 	// try to sign, should be locked
 	_, err = callWithError("personal_sign", []interface{}{hexutil.Bytes{0x88}, addr, ""})
-	require.NotNil(t, err)
+	require.Error(t, err)
 
 	rpcRes = call(t, "personal_unlockAccount", []interface{}{addr, ""})
 	var unlocked bool
@@ -104,5 +123,5 @@ func TestPersonal_LockAccount(t *testing.T) {
 
 	// try to sign, should be locked
 	_, err = callWithError("personal_sign", []interface{}{hexutil.Bytes{0x88}, addr, ""})
-	require.NotNil(t, err)
+	require.Error(t, err)
 }